Jeff King <peff@xxxxxxxx> writes: > On Fri, Dec 13, 2019 at 07:38:21PM -0500, Jeff King wrote: > >> That's because LIB_H is created by running find in the local filesystem. >> So until it's generated, we don't realize it's there to check. I kind of >> wonder if it should be part of LIB_H. I suspect that on some systems, >> we'd fail to notice a rebuild when command-list.txt is updated (but >> nobody noticed, because it is only systems that do not have >> compiler-supported dependency tracking, and most developers are no >> modern platforms that do). > > Actually, this probably isn't true. We have an explicit dependency for > help.o on command-list.h, so it would get built properly then. > > Its inclusion in LIB_H is still wonky, though. It sometimes is included > and sometimes not, depending on whether ls-files or find is used. As long as GENERATED_H is maintained properly to list headers that are actually used (e.g. if we ever start creating and using a header only when some Makefile macro tells us to, we make sure to place the header in GENERATED_H only when we create and use it), I think we should just add it to LIB_H, regardless of what is tracked. LIB_H could contain command-list.h (and other GENERATED_H files) if we did this, but dups in dependency does not hurt in general, and I did not find anything potentially problematic in the existing use of $(LIB_H) in our Makefile. How about doing this as a further clean-up? I am reasonably sure the status-quo description is correct, but I find the justification a bit weak (in other words, I do not have a good answer to "who cares if those that depend on $(LIB_H) are not rebuilt when command-list.h gets rebuilt?") --- >8 --- Makefile: include GENERATED_H in LIB_H $(LIB_H), which is meant to be the list of header files that can affect (hence trigger recompilation) the objects that go in libgit.a, in a directory extracted from a tarball is computed by running "find \*.h" but instead computed with "ls-files \*.h" in a working tree managed by a git repository. The former can include generated header files after a build, and omit them in a clean state. The latter would not, as generated header files are by definition not tracked. Explicitly add $(GENERATED_H) to $(LIB_H) to make things consistent. Signed-off-by: Junio C Hamano <gitster@xxxxxxxxx> --- Makefile | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/Makefile b/Makefile index 9a9d35637d..552c43c3d8 100644 --- a/Makefile +++ b/Makefile @@ -822,6 +822,7 @@ LIB_H := $(sort $(patsubst ./%,%,$(shell git ls-files '*.h' ':!t/' ':!Documentat -name t -prune -o \ -name Documentation -prune -o \ -name '*.h' -print))) +LIB_H += $(GENERATED_H) LIB_OBJS += abspath.o LIB_OBJS += add-interactive.o @@ -2399,7 +2400,7 @@ else # should _not_ be included here, since they are necessary even when # building an object for the first time. -$(OBJECTS): $(LIB_H) $(GENERATED_H) +$(OBJECTS): $(LIB_H) endif exec-cmd.sp exec-cmd.s exec-cmd.o: GIT-PREFIX @@ -2521,7 +2522,7 @@ XGETTEXT_FLAGS_SH = $(XGETTEXT_FLAGS) --language=Shell \ --keyword=gettextln --keyword=eval_gettextln XGETTEXT_FLAGS_PERL = $(XGETTEXT_FLAGS) --language=Perl \ --keyword=__ --keyword=N__ --keyword="__n:1,2" -LOCALIZED_C = $(C_OBJ:o=c) $(LIB_H) $(GENERATED_H) +LOCALIZED_C = $(C_OBJ:o=c) $(LIB_H) LOCALIZED_SH = $(SCRIPT_SH) LOCALIZED_SH += git-parse-remote.sh LOCALIZED_SH += git-rebase--preserve-merges.sh
