Re: [PATCH 0/3] Makefile: make "sparse" and "hdr-check" non-.PHONY

[Jeff King <peff@peff.net>]

On Wed, Sep 22, 2021 at 12:55:12AM +0200, Ævar Arnfjörð Bjarmason wrote:

> Now that my series to only build "TAGS" when we strictly need to has
> landed in 1b8bd2243e7 (Merge branch 'ab/make-tags-cleanup',
> 2021-09-20), let's do the same for the "sparse" and "hdr-check"
> targets.
> 
> For *.c files we'll now generate corresponding empty *.sp and *.hco
> files when "sparse" and "hdr-check" are run, respectively. If either
> of those errored on the *.c file we'd fail to refresh the
> corresponding generated file.

All three seem pretty reasonable to me.

Though could we be confused in the sparse rule by a header file that
changed? The object files depend on the auto-computed dependencies or on
LIB_H, but the sparse rule doesn't. So, with your patch:

  $ echo '/* ok */' >>git-compat-util.h
  $ make sparse
  [lots of output, everything ok]

  $ echo 'not ok' >>git-compat-util.h
  $ make sparse
  [no output; nothing is run]

  $ touch git.c
  $ make sparse
  git.c: note: in included file (through builtin.h):
  git-compat-util.h:1382:1: error: 'not' has implicit type
  git-compat-util.h:1382:5: error: Expected ; at end of declaration
  [...etc...]

I think it's hard to use the computed dependencies here, because they're
written by the compiler with explicit ".o" targets. But we could either:

  1. make them all depend on LIB_H. That's overly broad, but still
     better than the status quo; or

  2. have "foo.sp" depend on "foo.o". That requires you to build things
     before doing sparse checks, but in practice most people would
     presumably _also_ be compiling anyway, I'd think.

I.e., this works for me (the second "make sparse" in my example above
rebuilds and shows the errors):

diff --git a/Makefile b/Makefile
index e44eb4a62a..a97e52eb19 100644
--- a/Makefile
+++ b/Makefile
@@ -2903,7 +2903,7 @@ check-sha1:: t/helper/test-tool$X
 
 SP_OBJ = $(patsubst %.o,%.sp,$(C_OBJ))
 
-$(SP_OBJ): %.sp: %.c GIT-CFLAGS
+$(SP_OBJ): %.sp: %.c %.o GIT-CFLAGS
 	$(QUIET_SP)cgcc -no-compile $(ALL_CFLAGS) $(EXTRA_CPPFLAGS) \
 		-Wsparse-error \
 		$(SPARSE_FLAGS) $(SP_EXTRA_FLAGS) $< && \

-Peff