WIP: playground for config.h replacement
This MR is intended for experimenting with ideas in #234 (closed) and not a direct candidate for merge.
Merge request reports
Activity
mentioned in issue #234 (closed)
added 1 commit
- e148022b - Use target and scope in dune_target_add_config_header
added 1 commit
- 6b457588 - reset some changes regarding config.h.cmake file
added 1 commit
- 904f6037 - Add PROJECT_BINARY_DIR to include directories
28 28 LABELS quick) 29 29 30 30 dune_add_test(SOURCES variablesizecommunicatortest.cc 31 LINK_LIBRARIES dunecommon 
This is already proposed in !862 (merged)
Perhaps I share what I noticed while implementing !1262 (merged)
- I don't think one has to include the config in every file. Just on the ones that do not include other dune-common header files. There should not be that many.
- The generated header file needs to be installed, otherwise, we are not solving #234 (closed) where we want to make downstream projects less dependent on the dune build system.
- The order on which the contents of the header files are added is important due to the dune-grid magic macros. See #234 (comment 128665)
- In order to allow mixed
#include <...>of old and new config files between different DUNE versions, we need to add pre-processor macros that avoid them to conflict. So, similar to !1247 (merged), this change needs to be done over several releases.
-
I don't think one has to include the config in every file. Just on the ones that do not include other dune-common header files. There should not be that many.
Sure, we have transitive includes. My concern is that this gets easily forgotten when modifying the header files, e.g. removing some unnecessary include. Maybe then it is also not needed to include the config.h. Actually, it should just be necessary in any file that directly uses some of the macros, right? Because then by transitive includes, if you use a file that needs the macros you include that header file that itself includes the config. The config.h file has no effect on its own, just in combination with functions/classes that need the macro definition to modify their behavior.
So, yes: we only need to include the config file in a few headers actually. Only include what you use! :-)
added buildsystem label
changed milestone to %CMake Modernization
mentioned in merge request !1313 (merged)
1194 1196 endforeach() 1195 1197 endmacro(add_dune_all_flags targets) 1198 1199 1200 # create a module specific config header file and force an include 1201 function(dune_add_config_header _config_hh_in _config_hh) 1202 set(_config_hh_tmp ${CMAKE_CURRENT_BINARY_DIR}/${_config_hh_in}.tmp) 1203 file(READ ${_config_hh_in} _config_hh_content) 1204 file(WRITE ${_config_hh_tmp} "${_config_hh_content}") 1205 1206 # append version information about the current module 1207 dune_module_to_uppercase(PROJECT_NAME ${ProjectName}) 1208 file(APPEND ${_config_hh_tmp} 1209 " 1210 /* Define to the version of ${ProjectName} */ 1211 #define ${PROJECT_NAME}_VERSION \"\${${PROJECT_NAME}_VERSION}\" -
I think this is good to have, and we should definitely provide this in the core modules, but it should not be automatically added IMH. Already having
HAVE_${modules}is a headache that I would like to get rid off !1318 (merged) eventually. We could suggest this indunecontrolgenerated modules though.
-
@simon.praetorius should we close this? I think this is covered by !1318 (merged) and !1313 (merged) which is more up to date.