Repository navigation
[cmake] Regenerate git version in config.h at build time - #9299
Conversation
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request refactors the generation of config.h to occur at build time whenever the Git HEAD changes, ensuring the version string stays up-to-date without requiring a full CMake re-run. Feedback on these changes suggests removing the warning when git describe fails to prevent build output spam on shallow clones, and replacing the non-existent __fake_config.h output with a dummy custom target to avoid Ninja build warnings.
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
Previously `git describe` was only executed when configuring CMake, so switching git revisions and running an incremental build left `PROJECT_VERSION` (and `--version` output) stale unless CMake happened to re-run. Inspired by LLVM's `GenerateVersionFromVCS.cmake` / `VCSRevision.h` setup, move `config.h` generation into `scripts/gen-version.cmake` and wire it up via `add_custom_command` depending on `.git/logs/HEAD`. Because `configure_file` only updates `config.h`'s timestamp when its contents actually change, and only `src/support/command-line.cpp` includes `config.h`, incremental rebuilds only recompile that single file when `HEAD` changes. `config.h` is also still generated at configure time so tools like `clang-tidy` that run before a build can find it.
c745635 to
4c08908
Compare
There was a problem hiding this comment.
Code Review
This pull request refactors the version generation logic to update config.h at build time when switching git commits, rather than only at configure time. It introduces a new scripts/gen-version.cmake script and configures a custom build target to handle this process. Feedback on these changes suggests changing a warning message to a status message in the new script to prevent noisy build outputs in shallow-clone CI environments, and marking the fallback fake header file as SYMBOLIC in CMake to avoid IDE warnings and properly handle the missing file.
| OUTPUT_VARIABLE GIT_VERSION | ||
| OUTPUT_STRIP_TRAILING_WHITESPACE) | ||
| if(GIT_VERSION_RESULT) | ||
| message(WARNING "Error running git describe to determine version") |
There was a problem hiding this comment.
Using message(WARNING ...) here can be very noisy, especially in CI environments (like GitHub Actions) where shallow clones are the default and git describe will fail because tags are not fetched. Since this script runs at build time, this warning will be printed repeatedly, cluttering the build output. Changing this to message(STATUS ...) or removing it entirely is recommended.
message(STATUS "Could not run git describe to determine version")
| else() | ||
| # Some checkouts (e.g. 'repo' or reflog-disabled) may not have | ||
| # .git/logs/HEAD; use a non-existent output to force checking git at | ||
| # build time. | ||
| list(APPEND GENERATED_VERSION_FILES "${CMAKE_CURRENT_BINARY_DIR}/__fake_config.h") | ||
| endif() |
There was a problem hiding this comment.
To prevent IDEs (like Visual Studio or Xcode) from displaying warnings about the missing __fake_config.h file, and to explicitly signal to CMake that this file is not expected to be created on disk, you should mark it with the SYMBOLIC source file property. This is the standard CMake way to force a custom command to run on every build.
else()\n # Some checkouts (e.g. 'repo' or reflog-disabled) may not have\n # .git/logs/HEAD; use a non-existent output to force checking git at\n # build time.\n set(FAKE_HEADER "${CMAKE_CURRENT_BINARY_DIR}/__fake_config.h")\n list(APPEND GENERATED_VERSION_FILES "${FAKE_HEADER}")\n set_source_files_properties("${FAKE_HEADER}" PROPERTIES SYMBOLIC TRUE)\n endif()
kripken
left a comment
kripken
left a comment
There was a problem hiding this comment.
Does this cause us to do more work in incremental builds?
|
It means that there will always be at least one object file dirty whenever you change your HEAD ( This is the same in LLVM. Whenever you commit or change your HEAD you get a very minimal forced rebuild. If you prefer we can work around this in emscripten-core/emscripten#28064... but just note that |
kripken
left a comment
kripken
left a comment
There was a problem hiding this comment.
I am not a cmake expert so not sure about the fine details but if this works it sounds good, and seems low risk.
|
Lets see if anyone complains :) |
Previously
git describewas only executed when configuring CMake, so switching git revisions and running an incremental build leftPROJECT_VERSION(and--versionoutput) stale unless CMake happened to re-run.This change (or something like this) is needed to make the version check in emscripten-core/emscripten#28053 work. Right now
--versionlies which is not great.Inspired by LLVM's
GenerateVersionFromVCS.cmake/VCSRevision.hsetup, moveconfig.hgeneration intoscripts/gen-version.cmakeand wire it up viaadd_custom_commanddepending on.git/logs/HEAD.Because
configure_fileonly updatesconfig.h's timestamp when its contents actually change, and onlysrc/support/command-line.cppincludesconfig.h, incremental rebuilds only recompile that single file whenHEADchanges.config.his also still generated at configure time so tools likeclang-tidythat run before a build can find it.