Re: [PATCH 1/1] Makefile: use `git ls-files` to list header files, if possible
- From
Jeff King <peff@peff.net>
- Date
- Mar 6, 2019, 04:40 UTC
- Message-ID
- <20190306044006.GA6664@sigill.intra.peff.net>
- In-Reply-To
- <42d125d4-76bf-afc3-8f12-a9fa1296c85c@ramsayjones.plus.com>
On Wed, Mar 06, 2019 at 12:23:20AM +0000, Ramsay Jones wrote:
Show 13 quoted lines
> > 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