Re: [PATCH] Makefile: drop GEN_HDRS

[Junio C Hamano <gitster@pobox.com>]

Jeff King <peff@peff.net> 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@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