Export compile definitions for dependent targets

[gassmoeller]

I think #14491 accidentally removed the export of compile definitions from the cmake system (both from Config.cmake.in and the setup_target macro). They were still correctly set for compiling deal.II itself, but dependent targets would not pick them up and so e.g. algorithms in header files behave differently if they are executed from inside deal.II or outside (e.g. #ifdef DEBUG would be evaluated differently). Also the build output looks different (same flags different definitions) on dependent targets, which let me wonder for a while what we do wrong when outputting the ASPECT build configuration.
ASPECT was also relying on the presence of NDEBUG definition for another dependent target to activate optimized mode (see discussion in #12503).

This change should restore the behavior before #14491.

[tjhei]

I think this makes sense. @tamiko ?

[tjhei]

/rebuild

[tamiko]

@tjhei @gassmoeller @masterleinad Actually, no. We need to add the compile definitions to the dealii::dealii_release and dealii::dealii_debug targets and make sure that they are correctly annotated as an INTERFACE property.

This pull request simply works around the issue with our deal_ii_setup_target() macro which we intend to deprecate and remove in the long run.

[tamiko]

@gassmoeller Please have a look at the target configuration in cmake/deal.II/deal.IITargets.cmake. On my system it correctly annotates all targets with necessary compile definitions, such as DEBUG, NDEBUG, and KOKKOS_DEPENDENCE.

So I am really not sure what issue you are working around here.

I would like this changeset to be reverted and the issue to be fixed correctly.

Edit:

I am terribly sorry that I did not catch this pull request earlier.

Let me elaborate a bit: Our goal is to move to the standard-CMake way of interfacing with external dependencies by using import targets. To this end the build system now supports this:

add_executable(whatever ...)
target_link_libraries(whatever dealii::dealii) # or explicitly dealii::dealii_debug and dealii::dealii_release)

This means that everything necessary to compile correctly against the debug and release version has to be set as a target property on those two targets. (Which I just checked again - it is. Because if we would not set DEBUG and NDEBUG correctly then we simply crash...)

Now, of course we run into a couple of issues because of changing expectations: It is discouraged to set unnecessary compiler/linker flags as target property, because this creates a lot of problems for downstream projects. Thus, we only export the bare minimum of configuration to the two targets dealii::dealii_debug and dealii::dealii_release: C++ language standard, link libraries, include directories, and all compile definitions (!).

But in order to provide backward compatiblity we have a wrapper target dealii::dealii that will set all compiler and linker flags that we chose for compiling deal.II, and our deal_ii_setup_target() macro does the same.

[tamiko]

@gassmoeller How can I reproduce the issue you have?

[gassmoeller]

I dont have time to look into this today, hopefully tomorrow (in the middle of a move), but I am ok reverting this if there is a better way. All I was seeing at the time was that NDEBUG was not picked up correctly by ASPECT when using the macro deal_ii_setup_target() for release mode. Maybe that is a mistake in the ASPECT build system, but I couldnt figure out what it was. I will try to reproduce the problem and report back, in the meantime I am ok if this is reverted in #16924.

[gassmoeller]

@tamiko: The simplest would be to run cmake for ASPECT in DebugRelease mode and check if it picks up NDEBUG correctly for release mode. It is a bit trickier, because I also made some changes to ASPECT CMakeLists in geodynamics/aspect#5621 and I dont remember how the issues were intertwined.

[tamiko]

@gassmoeller Roger. Checking.

Edit: i can confirm that a small number of targets in aspect are not compiled with our compile definitions / compile flags.

I have set up a modified deal.II that export TESTING_COMPILE_DEFINITIONS in deal.IIConfig.cmake (and nothing else) and use your modified deal_ii_setup_target() macro. That way when compiling step 1 I get:

/usr/lib/llvm/18/bin/clang++ -DKOKKOS_DEPENDENCE -DNDEBUG -DTESTING_COMPILE_DEFINITIONS

Here, the first two definitions come from the target definitions in deal.IITargets.cmake and the last one from deal.IIConfig.cmake.

In aspect, a huge majority of targets are also built with all definitions -DKOKKOS_DEPENDENCE -DNDEBUG -DTESTING_COMPILE_DEFINITIONS. The exception is the world_builder target, which is not surprising, it is not linked against deal.II.

[gassmoeller]

Thanks for checking. I will also take another look some time later, but in order to not disrupt the current build system let's move forward with #16924.