Re: [PATCH] Makefile: drop GEN_HDRS
- From
Junio C Hamano <gitster@pobox.com>
- Date
- Dec 16, 2019, 18:55 UTC
- Message-ID
- <xmqqlfrcje1f.fsf@gitster-ct.c.googlers.com>
- In-Reply-To
- <20191214010002.GA945704@coredump.intra.peff.net>
Jeff King <peff@peff.net> writes:
Show 15 quoted lines
> 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@pobox.com> --- 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