build: track header dependencies in the Autoconf build#36
Open
gburd wants to merge 1 commit into
Open
Conversation
The build_unix Makefile listed only .c (and a few .incl) prerequisites per object -- no headers -- so editing a shared header (e.g. src/dbinc/lock.h) and running an incremental `make` silently left dependent objects compiled against the old layout. A struct-field addition then mismatched across translation units (one .o writes a field at the old offset, another reads it at the new one): silent shared-memory corruption, observed as spurious "invalid lock mode" errors. Add the standard autoconf/GCC auto-dependency pattern, gated on compiler support so the portable build is unchanged where it can't be done: - configure.ac probes whether $CC accepts -MMD -MP -MT; if so it sets DEPFLAGS='-MMD -MP -MT $@' and DEP_INCLUDE to pull in the per-object .d fragments, else both are empty (today's behaviour). - Makefile.in threads $(DEPFLAGS) into every compile recipe, includes the .d fragments (@DEP_INCLUDE@), and cleans *.d. - Compile recipes switch from $? (all newer prerequisites) to $< (the source): once -MMD adds header prerequisites, $? would try to compile a header. $< always names the .c and is correct with or without dependency tracking. Verified: touching src/dbinc/lock.h now rebuilds lock_region.o (compiling the right source), and the empty-DEPFLAGS fallback still builds correctly. The CONTRIBUTING note is updated accordingly.
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
1 participant
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.
What
Adds automatic header-dependency tracking to the
build_unixAutoconf build,gated on compiler support (
-MMD -MP -MT), so editing a shared header rebuildsevery dependent object.
Why
The Makefile listed only
.c(and a few.incl) prerequisites per object —no headers. Editing a shared header (e.g.
src/dbinc/lock.h) and running anincremental
makesilently left dependent objects compiled against the oldstruct layout. A struct-field addition then mismatched across translation units
(one
.owrites a field at the old offset, another reads it at the new one) —silent shared-memory corruption, which surfaced as spurious
invalid lock modeerrors during recent SSI work. This is the header-under-tracking hazard already
called out in
CONTRIBUTING.md; this PR fixes it instead of just warning.How
configure.acprobes whether$CCaccepts-MMD -MP -MT. If so it setsDEPFLAGS='-MMD -MP -MT $@'andDEP_INCLUDEto pull in per-object.dfragments; otherwise both are empty and behaviour is unchanged (no
GNU-make-isms leak into the portable build).
Makefile.inthreads$(DEPFLAGS)into every compile recipe, includes the.dfragments via@DEP_INCLUDE@, and adds*.dtoclean.$?→$<: once-MMDadds header prerequisites,$?(all newer prereqs) would try to compile a header;$<always names the.cand is correct in both modes.Testing
touch src/dbinc/lock.hthen incrementalmakenowrebuilds
lock_region.o(compiling the correct source; timestamp advances).DEPFLAGSfallback still builds correctly.configureregenerated with a surgical ~50-line diff (no autoconf-versionchurn). The large
Makefile.indiff is the mechanical$?→$<recipe change.