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

On 22/09/2021 18:53, Jeff King wrote:

On Wed, Sep 22, 2021 at 05:58:16PM +0100, Ramsay Jones wrote:

quoted

quoted

All three seem pretty reasonable to me.

Heh, interesting. My initial reaction was completely negative! ;-P
(and not just mildly negative either, but 'you must be kidding').

However, I then thought 'I must be missing something, I'm being
stupid and about to embarrass myself in public!'. So, I have
been trying hard to understand what these patches are trying to
accomplish and just what it is I'm missing. But, I'm coming up
blank ...

I think the point is just avoiding repeated work. If you just manually
run "make sparse" once in a while, then caching the result probably
isn't of much value. But if you plan to run, say:

  git rebase -x 'make sparse'

then it would be nice for it to avoid checking the same files over and
over.

I haven't tried, but

    git rebase -x 'make CC=cgcc'

may be a better idea (for some definition of 'better' ;) ).
(if you have been doing all recent builds with CC=cgcc, then
the first commit wouldn't force a complete re-build!).

Using CC=cgcc has a mixed past, sometimes working, sometimes
not (again for some definition of 'working'), for example:

  $ git checkout master
  ...
  $ make clean
  ...
  $ make CC=cgcc >out1 2>&1
  $ ./git version
  git version 2.33.0.514.g99c99ed825
  $ git describe
  v2.33.0-514-g99c99ed825
  $ grep warn out1
  imap-send.c:1461:9: warning: expression using sizeof on a function
  http.c:715:9: warning: expression using sizeof on a function
  http.c:1776:25: warning: expression using sizeof on a function
  http.c:1781:25: warning: expression using sizeof on a function
  http.c:2190:9: warning: expression using sizeof on a function
  http.c:2362:9: warning: expression using sizeof on a function
  http-walker.c:382:9: warning: expression using sizeof on a function
  http-push.c:194:9: warning: expression using sizeof on a function
  http-push.c:205:9: warning: expression using sizeof on a function
  http-push.c:206:9: warning: expression using sizeof on a function
  remote-curl.c:855:9: warning: expression using sizeof on a function
  remote-curl.c:945:17: warning: expression using sizeof on a function
  remote-curl.c:947:17: warning: expression using sizeof on a function
  remote-curl.c:1014:9: warning: expression using sizeof on a function
  $ grep error out1
  $ 

The warnings are due to some gnarly macro magic in the curl headers
which is normally suppressed by setting -DCURL_DISABLE_TYPECHECK in
the SP_EXTRA_FLAGS variable for each of those files. (see e.g the
Makefile:2250).

quoted

At the heart of my unease is dependencies (or rather the lack) for
the 'synthetic object files' *.hco and *.sp. (Also, the addition
of even more 'shrapnel' to the build directories - I wrote a patch
to remove the useless *.hcc files just after commit b503a2d515e was
included, but didn't get around to submitting it).

I don't consider them shrapnel if they're holding useful results. :)

Heh, yes I am a bit of a curmudgeon! :D

But overall, I do agree that if we can't make the dependencies solid
here, this is not worth doing. Sacrificing correctness of the checks for
reduced computation is not a good idea.

Yes, I suspect that 'make the dependencies solid' will be a
challenge, with drip, drip, fixes being required. (Maybe I
am just too pessimistic - maybe we can accept good enough
rather than perfect. Also, the sparse solution may be easier
than the hdr-check solution).

ATB,
Ramsay Jones