On 03/03/2019 17:19, Jeff King wrote:
> On Sat, Mar 02, 2019 at 09:05:24PM +0100, Johannes Schindelin wrote:
>
>>> That seems reasonable (regardless of whether it is in a script or in the
>>> Makefile). Another option is to use -maxdepth, but that involves
>>> guessing how deep people might actually put header files.
>>
>> It would also fail to work when somebody clones an unrelated repository
>> that contains header files in the top-level directory into the Git
>> worktree. I know somebody like that: me.
>
> Good point.
[Sorry for the late reply - I have been AWOL this weekend and
I am only just catching up with email! :-D ]
So, tl;dr: soon, I will be submitting a patch to remove the
'hdr-check' target completely, for now anyway.
> By the way, "make hdr-check" already fails for me on master, as I do not have
> libgcrypt installed, and it unconditionally checks sha256/gcrypt.h.
Yep, for me too. There is a similar problem if you build with
NO_CURL and don't have the 'curl/curl.h' header file, etc ...
The first version of the 'hdr-check' target re-introduced a static
list of header files, but I didn't think people would appreciate
having to maintain the list manually, so I gave up on that!
The next version was essentially the same patch that started this
thread from Johannes (ie. the 'git ls-files' patch), given that
the 'tags' targets had found it necessary. However, when I did some
'informal' timing tests (ie 5x 'time make ...' and average), this
did not make any noticeable difference for me (_even_ on cygwin!). ;-)
Of course, I had already made the mistake of trying to re-use
something that was 'handy' (ie. LIB_H) and already there.
However, LIB_H wasn't quite what I wanted - I needed a sub-set
of it.
So, I already have a 'hdr-check-fixup' branch (I think I have
already mentioned it), in which the first commit looks like:
diff --git a/Makefile b/Makefile
index c5240942f2..3401d1b9fb 100644
--- a/Makefile
+++ b/Makefile
@@ -2735,9 +2735,10 @@ $(SP_OBJ): %.sp: %.c GIT-CFLAGS FORCE
.PHONY: sparse $(SP_OBJ)
sparse: $(SP_OBJ)
+HC_HDRS := $(wildcard *.h negotiator/*.h block-sha1/*.h ppc/*.h ewah/*.h \
+ sha1dc/*.h refs/*.h vcs-svn/*.h)
GEN_HDRS := command-list.h unicode-width.h
-EXCEPT_HDRS := $(GEN_HDRS) compat% xdiff%
-CHK_HDRS = $(filter-out $(EXCEPT_HDRS),$(patsubst ./%,%,$(LIB_H)))
+CHK_HDRS = $(filter-out $(GEN_HDRS),$(HC_HDRS))
HCO = $(patsubst %.h,%.hco,$(CHK_HDRS))
$(HCO): %.hco: %.h FORCE
... which drops the use of LIB_H entirely.
Now, I have timed this and, for me, it no faster ... (I suspect
that it would be faster for Johannes, but it would still cause
a problem if you have 'top-level header files from some other
repo ...').
So, if we really need to solve the problem, allowing for some
unrelated headers in your worktree, then we can only use a
static list, or a 'git ls-files' approach.
Anyway, for now, since I seem to be the only person using this
target, I think we should just remove it while I think again.
(I can put it in my config.mak, so there will be no loss for me).
ATB,
Ramsay Jones
