Split config file into public and private config files

Description

This is a proposal to handle the configuration files and solve #234 (closed). With this we will be one step nearer to have a downstream project that can be completely independent of the DUNE build system.

Proposal

1. Generate a public and a private configuration files for each module (easy to automate since config files already have those definitions)
   • Public dune-foo-config.hh: Contains the definitions needed for its usage in downstream modules, including upstream public configuration files (e.g., #include <dune-grid-config.hh>). This header is installed and guarded to be included once.
   • Private dune-foo-config-private.hh: Includes the public header (e.g., #include <dune-grid-config.hh>), and the module information that should not be exported nor be visible outside of the module (same role as #include "config.h"), thus is not installed.
2. The module only installs the public header.
3. Downstream can consume these headers independent of the build system they choose for their project.

Goal

The main driver for this changes is the following snippet:

cmake_minimum_required(VERSION 3.16)
project("dune-foo" CXX)

# find dune-bar project: exports the target Dune::Bar
find_package(dune-bar)

# create a target library with native cmake
add_library(dune-foo INTERFACE)

# link to dune targets without need of dune-isms (e.g., custom generation of config files)
target_link_libraries(dune-foo INTERFACE Dune::Bar)

See !1249 (merged) for more info on other steps to achieve this. Please take into account this when proposing alternative solutions.

Backward compatibility

This MR is intended to be backward compatible. This is achieved by providing config.h with the same content as before, but additionally with pre-prorcessor guards to avoid double definitions.

• Projects that do not need compatibility with DUNE 2.9 can immediately use the new header files.
• Projects with compatibility with DUNE 2.9 need to stick using the config.h header file because the new header files do not exist in older versions. However, once compatibility with DUNE 2.9 is dropped (e.g., in DUNE 2.12), we can deprecate the old legacy headers and inform people that they need to use the new headers.
• Projects that define variables that are later changed (e.g. new packages were found and config depends on it) are backwards compatible, but they need to include config.h first (see documentation). See !1294 (merged) for an alternative of handling this situation.

Closes #234 (closed)

Needs

• !1298 (merged)

---

Python CI: https://github.com/dune-project/dune-testpypi/actions/runs/5350425239

added buildsystem label

changed the description

added 1 commit

• 3da33e5d - Use project name instead of DUNE_MOD_NAME

Compare with previous version

added 1 commit

• 12ca1ce0 - Use project name instead of DUNE_MOD_NAME

Compare with previous version

added 1 commit

• 3eea46c6 - Use project name instead of DUNE_MOD_NAME

Compare with previous version

added 1 commit

• 956b4d68 - Use project name instead of DUNE_MOD_NAME

Compare with previous version

added 1 commit

• 392b8ad3 - Use project name instead of DUNE_MOD_NAME

Compare with previous version

changed the description

mentioned in merge request !924 (closed)

Here we already get an interesting error: https://gitlab.dune-project.org/infrastructure/dune-nightly-test/-/jobs/500680

The variables for SuiteSparse are set in dune-common config file. However, dune-common does not try to find that module at all, thus, it resolves the flags to be false at its own configuration time. These flags need to be set in add_dune_suitesparse_flags rather than in the config file of dune-common. But this will be difficult to do in a backwards compatible way.

changed the description

Thanks for all your work on this. It is really appreciated. I will have a look and then give feedback.

added 1 commit

• 39e61428 - Resolve suite sparse package in dune-common

Compare with previous version

assigned to @santiago.ospina

added 61 commits

• 39e61428...5094a692 - 60 commits from branch master
• b0b35f79 - Split config file into public and private config files

Compare with previous version

added 1 commit

• dcb5a023 - Split config file into public and private config files

Compare with previous version

added 1 commit

• 2f11ee85 - Split config file into public and private config files

Compare with previous version

changed the description

added 1 commit

• c58e1efb - Split config file into public and private config files

Compare with previous version

Maybe a rebase to master is necessary to test it with the other recent modules master versions?

We want the public config file to be "installed" such that it can be included in downstream modules, right? What does "install" mean in this context? The simplest workflow is: configures everything in a build folder and do not call make install. Then we have the dune-common-config.h file only available in the dune-common build folder, that is not exported as include dir to downstream modules. This means that in downstream modules the public config file of dune-common will not be found.

added 7 commits

• c58e1efb...0c98b917 - 6 commits from branch master
• aa91565f - Split config file into public and private config files

Compare with previous version

added 3 commits

• 40b6c003 - Split config file into public and private config files
• e0106aab - Use project name instead of DUNE_MOD_NAME
• e25e33fb - Resolve suite sparse package in dune-common

Compare with previous version

added 29 commits

• e25e33fb...0c98b917 - 28 commits from branch master
• aa91565f - Split config file into public and private config files

Compare with previous version

added 1 commit

• 0673e588 - Generate public config files in a separate folder during build

Compare with previous version

mentioned in merge request !1294 (merged)

changed the description

added 1 commit

• 678818e3 - move config files to generated directory

Compare with previous version

Another full-system pipeline: https://gitlab.dune-project.org/infrastructure/dune-nightly-test/-/pipelines/65292 (with including the dune-common-config.hh in the header files and with the modification of the suitesparse config !1294 (merged))

It seems that this branch is ready now. We should remove the "Draft:" prefix and let everyone test it.

We should document where the public and the private config file should be included. For the public one, I know: the header files of the module, but where to include the private config file?

I added some documentation of how this whole strategy works, could you review it and add that part into it?

I asked, because I really don't know where the private part might be used.

I have the impression that the private config part is not really useful. E.g. the package name is known to the package author already. The package version is better represented in the public part of the config file, e.g. as DUNE_COMMON_VERSION. In the private part we only have the variable VERSION that is very generic. Maybe we should discuss to drop the private config part completely.

Ah, sorry, I misread your question.

I think private sections are useful for choosing options within binaries. Most of our files are headers, but we do ship some binaries and the options made inside them do no need to be public to the consumers of those binaries so long it doesn’t affect the ABI. For example, a quick search tells me that LAPACK_NEEDS_UNDERLINE didn’t need to be in the public section and it would have been fine to have it in the private section. (Note that I use past tense because I don’t know what consequences it has to change this now). This is just an example, but I hope that this clarifies the pattern.

mentioned in merge request copasi/dune-copasi!94 (merged)

added 1 commit

• 4330dd0a - Add some documentation about config files

Compare with previous version

added 1 commit

• 61b35f21 - Format documentation & fix typos

Compare with previous version

added 2 commits

• 5664a6e7 - Move the config files to the build dir include and include_private
• 15bf30b3 - Fix the issue with the configures rst file

Compare with previous version

changed the description

mentioned in merge request !1300 (merged)