Re: [PATCH 1/1] Makefile: use `git ls-files` to list header files, if possible

[Jeff King <peff@peff.net>]

On Wed, Mar 06, 2019 at 12:23:20AM +0000, Ramsay Jones wrote:

> > Yeah, that's what I was hinting at earlier in the thread. Here it is
> > sketched out to an actual working patch. The sub-make bits could
> > actually be a shell script instead of a Makefile; the only point in
> > using make is to use the parent "-j" for parallelism.
> 
> sigh. :(
> 
> I wish my patch removing this target had been picked up now!
> 
> Earlier this evening I spent an hour or so writing an email which
> tried to discourage spending any time on this, because of the
> potential for this to be a huge time-suck. An unrewarding one at
> that! :-D

Heh. I am OK with removing it, too.

My thinking earlier in the thread was that it should go in our bag of
linting tools that people should generally run. But actually, it is kind
of expensive to run, and it does not actually help anything immediately
in practice. I.e., what we really care about is that the C source files
compile, and running "make" does that (and especially running it on
various platforms). This is just checking for a _potential_ problem if
somebody were to include a particular header file at the start of
another C file.

So it's really about 2 steps removed from stopping an actual problem.

> I deleted that email (it's not in my drafts folder anyway)
> because, in the end, it is not up to me to say how people should
> spend their time.

FWIW, it was a fun exercise to build on the compiler dependencies. ;)
And I think I'm done playing with it for now. "make hdr-check" does not
run for me, but I think I've convinced myself that it's not all that
important that I run it.

I did forget to include one file in my earlier patch (the newly added
hdr-check.mak). I imagine one could guess at its contents, but here is
the complete patch, for the sake of reference.

-Peff

---
 Makefile       | 23 ++++++++++++-----------
 compat/bswap.h |  5 +++++
 hdr-check.mak  |  8 ++++++++
 3 files changed, 25 insertions(+), 11 deletions(-)
 create mode 100644 hdr-check.mak

diff --git a/Makefile b/Makefile
index c5240942f2..283e934d7b 100644
--- a/Makefile
+++ b/Makefile
@@ -1852,7 +1852,6 @@ ifndef V
 	QUIET_MSGFMT   = @echo '   ' MSGFMT $@;
 	QUIET_GCOV     = @echo '   ' GCOV $@;
 	QUIET_SP       = @echo '   ' SP $<;
-	QUIET_HDR      = @echo '   ' HDR $<;
 	QUIET_RC       = @echo '   ' RC $@;
 	QUIET_SUBDIR0  = +@subdir=
 	QUIET_SUBDIR1  = ;$(NO_SUBDIR) echo '   ' SUBDIR $$subdir; \
@@ -2735,16 +2734,18 @@ $(SP_OBJ): %.sp: %.c GIT-CFLAGS FORCE
 .PHONY: sparse $(SP_OBJ)
 sparse: $(SP_OBJ)
 
-GEN_HDRS := command-list.h unicode-width.h
-EXCEPT_HDRS := $(GEN_HDRS) compat% xdiff%
-CHK_HDRS = $(filter-out $(EXCEPT_HDRS),$(patsubst ./%,%,$(LIB_H)))
-HCO = $(patsubst %.h,%.hco,$(CHK_HDRS))
-
-$(HCO): %.hco: %.h FORCE
-	$(QUIET_HDR)$(CC) -include git-compat-util.h -I. -o /dev/null -c -xc $<
-
-.PHONY: hdr-check $(HCO)
-hdr-check: $(HCO)
+.PHONY: hdr-check
+hdr-check: all
+ifdef USE_COMPUTED_HEADER_DEPENDENCIES
+	@$(MAKE) -f hdr-check.mak CC="$(CC)" V=$(V) \
+		$$(sed -n 's/^\([^ ]*\)\.h:/\1.hco/p' .depend/* | \
+		   sort -u | \
+		   egrep -v '^(xdiff/|unicode-width.h|command-list.h)' \
+		)
+else
+	@echo >&2 "error: hdr-check supported only on platforms with computed dependencies"
+	@false
+endif
 
 .PHONY: style
 style:
diff --git a/compat/bswap.h b/compat/bswap.h
index 5078ce5ecc..e4e25735ce 100644
--- a/compat/bswap.h
+++ b/compat/bswap.h
@@ -1,3 +1,6 @@
+#ifndef COMPAT_BSWAP_H
+#define COMPAT_BSWAP_H
+
 /*
  * Let's make sure we always have a sane definition for ntohl()/htonl().
  * Some libraries define those as a function call, just to perform byte
@@ -210,3 +213,5 @@ static inline void put_be64(void *ptr, uint64_t value)
 }
 
 #endif
+
+#endif /* COMPAT_BSWAP_H */
diff --git a/hdr-check.mak b/hdr-check.mak
new file mode 100644
index 0000000000..00a3110fda
--- /dev/null
+++ b/hdr-check.mak
@@ -0,0 +1,8 @@
+.PHONY: FORCE
+
+ifndef V
+QUIET_HDR = @echo '   ' HDR $<;
+endif
+
+%.hco: %.h FORCE
+	$(QUIET_HDR)$(CC) -include git-compat-util.h -I. -o /dev/null -c -xc $<
-- 
2.21.0.699.ge1748d4d73