Factor out some file-generation machinery #181
Merged
Conversation
This file contains 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
…om the main Makefiles.
|
Now that I've overhauled this PR, this comment is no longer needed |
…s. Autogeneration should now be a lot more transparent.
brittonsmith
reviewed
Apr 8, 2024
|
I reimplemented the core-logic in terms of regex expressions |
This was achieved by tweaking `doc/source/conf.py`. This triggered a few cascading changes: 1. I needed to rename `config/query-version.py` to `config/query_version.py` so that it's contents could be properly imported 2. I needed to slightly tweak the internals of `config/query_version.py` so that the function returned values (instead of directly printing the value). To reflect this change in behavior, I changed a function name from `show_version` to `query_version` 3. While doing this, I also fixed a bug in some functionality from `config/query_version.py` that was intended to strip off a trailing '\n' character from the result of a shell command. In some cases, a trailing line-break might not be present and the functionality accidentally striped off a different character instead. 4. I needed to adjust a path in `src/clib/Makefile` to reflect the new name of the `config/query_version.py` script
This was referenced Apr 28, 2024
brittonsmith
reviewed
May 3, 2024
Previously, `LIB_RELEASE_VERSION` was defined in `src/clib/Make.config.assemble`. - This was fine, when `LIB_RELEASE_VERSION` was assigned a hardcoded value - however, after we started to use a script to retrieve this value, error messages were printed whenever we invoked the `src/example/Makefile` (to build code-examples) - The error messages arise because the path to the script was encoded within the `QUERY_VERSION` variable and that variable was not defined when `src/example/Makefile` included `src/clib/Make.config.assemble` - Since script-path is only defined within `src/clib/Makefile` I decided to move the definition of `LIB_RELEASE_VERSION` to that file. NOTE: since the `LIB_RELEASE_VERSION` wasn't actually used by `src/example/Makefile` the error-messages before this commit were entirely benign.
The required logic to fetch the last line of the file is now implemented in python. The logic loops over all lines from the VERSION file (from beginning to end) and returns the final non-empty line. This is fine for the VERSION file which should really just be 1 line (but is suboptimal for large files)
brittonsmith
approved these changes
May 17, 2024
mabruzzo
added a commit
to mabruzzo/grackle
that referenced
this pull request
May 17, 2024
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:
EDIT: I decided to overhaul this PR significantly. When reviewing it, it should be easier to review the net change rather than the individual commits)
This PR is nominally focused on refactoring some of the machinery related to autogenerating files. We now use more conventional "template" files (this pattern is used in cmake projects, autotools projects, and I've seen it projects with more atypicial build-systems like Athena++).
We now have template files called
src/clib/auto_general.c.inandsrc/clib/grackle_float.h.in. The build system executes the python-script calledconfig/configure_file.pyin order to create copies and perform variable substitution. Any name enclosed by a pair of@symbols in the template file is identified as the variable name. In the output file, it will be replaced by the associated value (specified as command-line argument ofconfig/configure_file.py).This change should make the codebase a lot more approachable. (It also helps pave the way for generating stuff like pkg-config files in a future PR - these files can help downstream applications automatically infer the required compiler args)
Another included change: We also now track the current grackle version number in a dedicated file called
VERSION.