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
