diff mbox series

[v2,4/4] Makefile: emulate compile in $(HCO) target better

Message ID 14def72319521d7380fb6a8ec570d014c0f5361b.1569398897.git.liu.denton@gmail.com (mailing list archive)
State New, archived
Headers show
Series fixes related to `make hdr-check` | expand

Commit Message

Denton Liu Sept. 25, 2019, 8:21 a.m. UTC
Currently, when testing headers using `make hdr-check`, headers are
directly compiled. Although this seems to test the headers, this is too
strict since we treat the headers as C sources. As a result, this will
cause warnings to appear that would otherwise not, such as a static
variable definition intended for later use throwing a unused variable
warning.

In addition, on platforms that can run `make hdr-check` but require
custom flags, this target was failing because none of them were being
passed to the compiler. For example, on MacOS, the NO_OPENSSL flag was
being set but it was not being passed into compiler so the check was
failing.

Fix these problems by emulating the compile process better, including
test compiling dummy *.hcc C sources generated from the *.h files and
passing $(ALL_CFLAGS) into the compiler for the $(HCO) target so that
these custom flags can be used.

Helped-by: Jeff King <peff@peff.net>
Signed-off-by: Denton Liu <liu.denton@gmail.com>
---

Peff, thanks for the suggestion! I modified it a little bit so that we
wouldn't have to keep regenerating the *.hcc files unnecessarily.

I also considered piping into the compiler's stdin directly which I know
works for GCC (and _probably_ Clang) but I opted against it because I'm
not sure it's portable for other compilers. Maybe it's alright for this
to be less portable since it's a developer target?

 .gitignore |  1 +
 Makefile   | 12 +++++++++---
 2 files changed, 10 insertions(+), 3 deletions(-)

Comments

Jeff King Oct. 2, 2019, 3:41 p.m. UTC | #1
On Wed, Sep 25, 2019 at 01:21:01AM -0700, Denton Liu wrote:

> Fix these problems by emulating the compile process better, including
> test compiling dummy *.hcc C sources generated from the *.h files and
> passing $(ALL_CFLAGS) into the compiler for the $(HCO) target so that
> these custom flags can be used.
> 
> Helped-by: Jeff King <peff@peff.net>
> Signed-off-by: Denton Liu <liu.denton@gmail.com>
> ---
> 
> Peff, thanks for the suggestion! I modified it a little bit so that we
> wouldn't have to keep regenerating the *.hcc files unnecessarily.

What you have here looks good. I doubt it matters too much compared to
the cost of the compiler, but having them in their own rule makes it
easier to follow, I think.

> I also considered piping into the compiler's stdin directly which I know
> works for GCC (and _probably_ Clang) but I opted against it because I'm
> not sure it's portable for other compilers. Maybe it's alright for this
> to be less portable since it's a developer target?

Yeah, I'd worry slightly about portability. But it would be nice to
avoid generating more cruft. And I agree that most people running this
would be developers. Maybe it would make sense to float a separate patch
on top (that would make it easy to revert if somebody runs into a
problem).

Thanks for working on this (and the whole series looks good, including
the commit message changes you made regarding the bitmap code).

-Peff
diff mbox series

Patch

diff --git a/.gitignore b/.gitignore
index 521d8f4fb4..34efe125cb 100644
--- a/.gitignore
+++ b/.gitignore
@@ -216,6 +216,7 @@ 
 /tags
 /TAGS
 /cscope*
+*.hcc
 *.obj
 *.lib
 *.res
diff --git a/Makefile b/Makefile
index f879697ea3..581cc617e3 100644
--- a/Makefile
+++ b/Makefile
@@ -1872,7 +1872,7 @@  ifndef V
 	QUIET_MSGFMT   = @echo '   ' MSGFMT $@;
 	QUIET_GCOV     = @echo '   ' GCOV $@;
 	QUIET_SP       = @echo '   ' SP $<;
-	QUIET_HDR      = @echo '   ' HDR $<;
+	QUIET_HDR      = @echo '   ' HDR $(<:hcc=h);
 	QUIET_RC       = @echo '   ' RC $@;
 	QUIET_SUBDIR0  = +@subdir=
 	QUIET_SUBDIR1  = ;$(NO_SUBDIR) echo '   ' SUBDIR $$subdir; \
@@ -2771,9 +2771,14 @@  ifndef GCRYPT_SHA256
 endif
 CHK_HDRS = $(filter-out $(EXCEPT_HDRS),$(patsubst ./%,%,$(LIB_H)))
 HCO = $(patsubst %.h,%.hco,$(CHK_HDRS))
+HCC = $(HCO:hco=hcc)
 
-$(HCO): %.hco: %.h FORCE
-	$(QUIET_HDR)$(CC) -include git-compat-util.h -I. -o /dev/null -c -xc $<
+%.hcc: %.h
+	@echo '#include "git-compat-util.h"' >$@
+	@echo '#include "$<"' >>$@
+
+$(HCO): %.hco: %.hcc FORCE
+	$(QUIET_HDR)$(CC) $(ALL_CFLAGS) -o /dev/null -c -xc $<
 
 .PHONY: hdr-check $(HCO)
 hdr-check: $(HCO)
@@ -3082,6 +3087,7 @@  clean: profile-clean coverage-clean cocciclean
 	$(RM) $(ALL_PROGRAMS) $(SCRIPT_LIB) $(BUILT_INS) git$X
 	$(RM) $(TEST_PROGRAMS)
 	$(RM) $(FUZZ_PROGRAMS)
+	$(RM) $(HCC)
 	$(RM) -r bin-wrappers $(dep_dirs)
 	$(RM) -r po/build/
 	$(RM) *.pyc *.pyo */*.pyc */*.pyo command-list.h $(ETAGS_TARGET) tags cscope*