cmake: refresh SVNDATE on git reset / commit, not just on branch checkout - #493
Merged
mrjimenez merged 1 commit intoApr 28, 2026
Merged
Conversation
CMAKE_CONFIGURE_DEPENDS on .git/HEAD alone misses the most common
state-change command: `git reset --hard <ref>`. HEAD is a symref
text file ("ref: refs/heads/<branch>"); reset moves the *branch ref*,
not HEAD itself, so HEAD's mtime stays unchanged and ninja never
re-runs cmake. Result: incremental rebuilds after a reset embed the
old SVNDATE in config.h and the binary's --version banner is stale
by however many commits the reset jumped.
Resolve HEAD's symref target at configure time and add the actual
branch ref file to CMAKE_CONFIGURE_DEPENDS too. Also depend on
.git/packed-refs in case the branch ref has been packed away
(`git gc` / `git pack-refs` consolidates loose refs into packed-refs,
after which the loose file no longer exists).
Verified: `touch .git/refs/heads/<branch>` now triggers a
"Configuring done" pass on the next `cmake --build`, and a real
commit/reset that changes the SHA causes config.h to regenerate
and the relevant objects (amule.cpp, amuled.cpp, etc., which embed
SVNDATE) to recompile. When the SHA is unchanged after a touch,
configure_file's content-identical check leaves config.h alone, so
the reconfigure costs ~0.7s and rebuilds nothing.
got3nks
added a commit
to got3nks/amule
that referenced
this pull request
Jul 15, 2026
…sing-override (amule-project#488) (amule-project#493) * style(amule): mark CamuleApp wxApp overrides with `override` (amule-project#488) CamuleApp already marks some of its wxApp overrides `override` (e.g. EnableIP2Country, GetIP2Country), so newer clang flags the ones that don't via -Winconsistent-missing-override. That fires 800+ times across the build because amule.h is included in almost every translation unit (issue amule-project#488, and it grows louder on the fussier clang shipping in Ubuntu 26.10). Mark the five remaining wxApp overrides -- OnInit, OnExit, OnFatalException, OnAssertFailure, OnUnhandledException -- with `override` (dropping the now redundant `virtual`), matching the class's existing style. `override` also makes the compiler verify they really do override a base method. Reproduced locally (830 warnings) and confirmed 0 after the change, clean build. Scope is deliberately just these compiler warnings; the broader tree-wide modernize-use-override sweep + enabling the clang-tidy check is left for a separate one-shot PR. Refs amule-project#488 * build: promote -Winconsistent-missing-override to an error on Clang (amule-project#488) Now that amule.h no longer trips it, guard against the whole class of amule-project#488 warning recurring: when a class marks some virtual overrides `override` but not others, Clang emits -Winconsistent-missing-override once per translation unit, so one header can produce hundreds of warnings (amule.h was 800+). Promote it to an error, mirroring the existing -Werror=deprecated* gate. Clang-only via a generator expression: GCC has no "some marked, some not" variant (only the whole-tree -Wsuggest-override), so an unconditional -Werror= would break the GCC/Ubuntu build. The Clang-based CI jobs (macOS, mingw-w64) are enough to catch a regression. The warning is on by default under Clang, so only the promotion to error is added. Verified: build stays green, and un-marking a single override makes it fail as intended. Refs amule-project#488
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
2 participants
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.
Summary
CMAKE_CONFIGURE_DEPENDSon.git/HEADalone misses the most common state-change command:git reset --hard <ref>. HEAD is a symref text file (ref: refs/heads/<branch>); reset moves the branch ref, not HEAD itself, so HEAD's mtime stays unchanged and ninja never re-runs cmake. The result is that incremental rebuilds after a reset embed the oldSVNDATEinconfig.hand the binary's--versionbanner is stale by however many commits the reset jumped.In practice this bites whenever someone is iterating on a feature branch with
git fetch fork && git reset --hard fork/<branch> && cmake --build build— the build succeeds, the new code is compiled, but the banner still reports the SHA from the previous build.Fix
Resolve HEAD's symref target at configure time and add the actual branch ref file to
CMAKE_CONFIGURE_DEPENDStoo. Also depend on.git/packed-refsin case the branch ref has been packed away (git gc/git pack-refsconsolidates loose refs intopacked-refs, after which the loose file no longer exists).What still works the same
.git:find_package(Git)short-circuits,SVNDATEfalls through to whatever the cache or-DSVNDATE=...provides. Unchanged..git/HEADis depended on, which is exactly what's needed (any new SHA writes to HEAD directly in that mode).git describere-derivesSVNDATE,configure_fileregeneratesconfig.honly if the SHA actually changed (content-identical otherwise), and dependents recompile.Verification
Reconfigure fires;
configure_file's content-identical check leavesconfig.halone when SHA is unchanged after atouch, so the rebuild costs ~0.7 s and rebuilds nothing. A real SHA change would propagate through to a relink.