Open
Conversation
Collaborator
Author
|
NOTE: there are some unused configuration options in cholla_config.h.in. I clean those out in the followup to this PR: #484 |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Overview
This PR changes the way the internals of the build-system handles compile-time configuration parameters
Historically, every configuration choice were passed into the compiler as
-D<config-var>or-D<config-var>=<val>flags. Now, we generate a header file calledcholla_config.hthat encodes all of the definitions, and that file is used to communicate the configuration choices to the rest of the source code.Care has been taken so to ensure that the experience is pretty much unchanged (this is only an internal change)
I've also written a lot of website documentation, but I go into some detail below.
How it works
With the changes introduced in this PR, the build-system now
-D<config-var>or-D<config-var>=<val>flags to the newly introducedconfigure_file.pyscript, which generatessrc/cholla_config.hfrom thesrc/cholla_config.h.intemplate file.makethat doesn't invoke a PHONY target, will try to rebuildcholla_config.h. Care is taken to only overwrite the file if the configuration options have changed. (If the file does change,makewill try to regenerate all of the object files)-include cholla_config.h. This option makes the compiler (technically the C preprocessor) act as if the line#include "cholla_config.h"was prepended to the start of each fileNote
I'm frankly not thrilled about the use of the
-include cholla_config.hflag, but after talking it over with Evan, we decided it was the best solution.My reservations pertain to concerns over portability (while g++ and clang++ support the flag, I'm not sure about other compilers) and the fact that the inclusion is implicit.
The alternative was to actually modify ever source file in cholla (that uses a config option) and make sure that it includes the
global/global.hheader (which internally includescholla_config.h) or directly includescholla_config.hbefore the very first check in the file of one of the config-checks.Issues arrive with the alternative since most conditional compilation based on whether or not a config option is defined or not.
Thus, if someone forgot to include
global/global.horcholla_config.h, the source code would just act like all of the relevant options were disabled.1The
configure_file.pytoolThis is a highly modified version of a similar script I previously contributed to grackle. This is modeled after the
configure_filecommands provided by CMake and Meson.A key feature is that the program will abort if an unknown config variable is provided or if a required config variable isn't specified.
Benefits
There are a few benefits of this PR:
-Dflags to just-include cholla_config.h, and we will hopefully be able to get rid of that in the future)Footnotes
There wouldn't be a major problem if all of the compilation options were always defined, and all conditional compilation choices were made based on the value of the option (rather than whether it is defined). In that scenario, the build would simply fail if somebody forgot an include statement. ↩