Draft: Optional CMake target dependencies

Convert target dependencies to cmake generator expressions. If a downstream project does not find a dependency, the generator expression will generate an empty string. If the dependency is found, the generator expression will fill the corresponding dependencies for that target. This has the effect to make dependencies optional from the point of view of the upstream module. Notice that these generator expressions include the corresponding compiler definitions, thus, removing most of the #define problems we have with #234 (closed).

Note: this is an experiment and is not yet fully tested!

added 1 commit

• 3e3913e2 - Convert target dependencies to cmake generator expressions

Compare with previous version

added 1 commit

• f5cc926d - Fix LAPACK compile definitions

Compare with previous version

added buildsystem label

mentioned in merge request !1252 (closed)

added 1 commit

• 29a7cd26 - Fix issues with lists

Compare with previous version

Problem: the dune-py project generator does not like generator expressions.

This is not related to dune-py. It happens also in the regular tests and comes from linking against LAPACK. See my in-source comments.

Since LAPACK_LIBRARIES might be a list of libraries, this generator expression cannot be used. Try the following:

set(LAPACK_LIBRARIES_GENERATORS "")
foreach (lib ${LAPACK_LIBRARIES})
  list(APPEND LAPACK_LIBRARIES_GENERATORS "$<$<BOOL:${LAPACK_FOUND}>:${lib}>")
endforeach(lib)
# ...
dune_register_package_flags(
  LIBRARIES          ${LAPACK_LIBRARIES_GENERATORS}
  COMPILE_OPTIONS     "$<$<BOOL:${LAPACK_FOUND}>:${LAPACK_COMPILE_OPTIONS_FLAGS}>")

Not so nice, though.

I think, it might be related to the escape of the ";" symbol, it seems that the string with multiple libraries inside the generator expression is somewhere interpreted as a list with two string:

The global property ALL_PKG_LIBS contains

[...];$<$<BOOL:TRUE>:/usr/lib/x86_64-linux-gnu/libblas.so>;$<$<BOOL:TRUE>:/usr/lib/x86_64-linux-gnu/liblapack.so;/usr/lib/x86_64-linux-gnu/libblas.so>;

So, it is a list of libraries, but the LAPACK library is a generator expression containing a list. The <...> symbols in cmake do not protect from list separation.

One solution could be to store the ALL_PKG_LIBS not as a regular list, but our own "list" implementation with a custom element separator. This will be difficult. In the long run, I would really love to get rid of these global properties.

Could be related to https://gitlab.kitware.com/cmake/cmake/-/issues/14353

But it is difficult to reproduce in a minimal example.

Yes, I realized about this problem with generators too. I think the solution might be to make an INTERFACE target with all of the dependencies of that library. That way one does not have to deal with lists of flags/definitions/includes. So ideally, ALL_PKG_INCS/ALL_PKG_DEFS/ALL_PKG_OPTS would be empty and ALL_PKG_LIBS would be a list of targets. But I have to experiment a bit with that.

Maybe something like a global accessible interface library, e.g., dune-all-packages that is simply filled with all flags, options, and libraries, and then the user can simply link against this target.

I tried this the other day but it does not entirely work because such library needs to be installed (because the main library may depend on it). The problem comes in downstream modules when you also want a library with the same name, and possible the same alias. Then it will conflict with whatever is defined upstream. But maybe there is a way to solve that.

how about dune-common-all-packages. The name does probably not solve all the problems. We need something like a forwarding target that does not show up as dependency itself. But I think this would only be possible if it would just define link libraries (via imported targets)

we could maybe collect all packages in imported library that contain also the HAVE_CXY flags and then just collect these.

Maybe !1207 (merged) can help. I want to get rid of the automatic linking of all packages. This requires the global ALL_PKG_LIBS properties and is the source of the above issue. If we make this more explicit, we don't have this problem anymore.

It would be great if we could get rid of these global states at least for the core modules. All of these experiments try to go on that direction. One concern I have is that I don't want to install targets that are only internal to a library and do not necessarily need to be exported. So there is a compromise to make there.

Now I see the problem. When I link dunecommon against my dune-common-all-packages INTERFACE target I get the cmake error export called with target "dunecommon" which requires target "dune-common-all-packages" that is not in any export set.

Maybe we just to have to some extra quotation marks somewhere, e.g. in the add_dune_all_flags or dune_target_enable_all_packages functions around the extracted global properties, e.g. link libraries.

Can you push your experiment with dune-common-all-packages? I want to check it out. If it works when installing the target, it could be a good compromise while we get rid of global states/libraries.

NVM I found it.

Just an idea: I have create in dune_project() a library with

add_library(${ProjectName}-all-packages INTERFACE)

and then have removed all the dune_register_package_flags calls in favor of an registration of the flags in this target, e.g.

target_link_libraries(${ProjectName}-all-packages INTERFACE
  "$<TARGET_NAME_IF_EXISTS:GMP::gmpxx>")
target_compile_definitions(${ProjectName}-all-packages INTERFACE
  "$<$<TARGET_EXISTS:GMP::gmpxx>:HAVE_GMP=1>")

Then I have modified the function add_dune_all_flags as follows:

macro(add_dune_all_flags targets)
  foreach(target ${targets})
    target_link_libraries(${target} PUBLIC ${ProjectName}-all-packages)
  endforeach()
endmacro(add_dune_all_flags targets)

So far there are no artificial interface target dependencies put into the exported targets. But this is just dune-common. Maybe it crashes further down.

While working on this, I noticed that it would be nice to add package flags with a scope. Therefore, I have introduced a cmake macro called dune_parse_arguments and have added an extra line in all the add_dune_xxx_flags macros, e.g.

function(add_dune_blas_lapack_flags)
  dune_parse_arguments(ARG "" "" "" "SCOPE:PUBLIC:INTERFACE,PRIVATE,PUBLIC" ${ARGN})

  foreach(_target ${ARG_UNPARSED_ARGUMENTS})
    target_link_libraries(${_target}  ${ARG_SCOPE}
      "$<$<BOOL:${LAPACK_FOUND}>:${LAPACK_LIBRARIES}>")
    target_compile_options(${_target} ${ARG_SCOPE}
      "$<$<BOOL:${LAPACK_FOUND}>:${LAPACK_COMPILE_OPTIONS_FLAGS}>")
    target_link_libraries(${_target}  ${ARG_SCOPE}
      "$<$<BOOL:${BLAS_FOUND}>:${BLAS_LIBRARIES}>")
    target_compile_definitions(${_target} ${ARG_SCOPE}
      "$<$<BOOL:${BLAS_FOUND}>:HAVE_BLAS=1>")
  endforeach(_target)
endfunction(add_dune_blas_lapack_flags)

add_dune_blas_lapack_flags(${ProjectName}-all-packages INTERFACE)

Yes! that's very much needed (regardless of this feature). I often can't use these flags directly because they are currently only PUBLIC.

BTW: I think the snippet for add_dune_blas_lapack_flags has the problem of lists passed to generator expressions

yes, but it works when passed directly to the target_xxx_yyy functions.

I have an idea: we could extract all target properties from the interface target dune-xxx-all-packages and forward these properties manually, e.g., in add_dune_all_flags:

function(add_dune_all_flags targets)
  get_target_property(incs ${ProjectName}-all-packages INTERFACE_INCLUDE_DIRECTORIES)
  get_target_property(defs ${ProjectName}-all-packages INTERFACE_COMPILE_DEFINITIONS)
  get_target_property(opts ${ProjectName}-all-packages INTERFACE_COMPILE_OPTIONS)
  get_target_property(libs ${ProjectName}-all-packages INTERFACE_LINK_LIBRARIES)
  get_target_property(libdirs ${ProjectName}-all-packages INTERFACE_LINK_DIRECTORIES)

  foreach(target ${targets})
    if(incs)
      target_include_directories(${target} PUBLIC ${incs})
    endif()
    if(defs)
      target_compile_definitions(${target} PUBLIC ${defs})
    endif()
    if(opts)
      target_compile_options(${target} PUBLIC ${opts})
    endif()
    if(libs)
      target_link_libraries(${target} PUBLIC ${libs})
    endif()
    if(libdirs)
      target_link_directories(${target} PUBLIC ${libdirs})
    endif()
  endforeach()
endfunction(add_dune_all_flags targets)

See https://gitlab.dune-project.org/-/snippets/72 for the dune_parse_arguments function (not very elegantly implemented, though)

See https://gitlab.dune-project.org/-/snippets/73 for a function dune_target_forward_properties to extract all interface properties.

Then I could simplify the add_dune_all_flags macro to

function(add_dune_all_flags targets)
  foreach(target ${targets})
    dune_target_forward_properties(${target} PUBLIC ${ProjectName}-all-packages)
  endforeach()
endfunction(add_dune_all_flags targets)

added 1 commit

• 8a43bdb4 - Add surrounding quotation marks for global target properties

Compare with previous version

The problem with this approach is that finding a library automatically means that it will be added to your target. Maybe that's not what you want. I think !1294 (merged) is a better approach. I will close this.

closed