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

From: Ramsay Jones <ramsay@ramsayjones.plus.com>
Date: Sep 22, 2021, 19:17 UTC
Message-ID: <82c3d9fc-32ff-c38a-cb44-873af1fb83d1@ramsayjones.plus.com>
In-Reply-To: <YUttpgkU6eCOxMj/@coredump.intra.peff.net>

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

Show 21 quoted lines

> On Wed, Sep 22, 2021 at 05:58:16PM +0100, Ramsay Jones wrote:
> 
>>> 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).

Show 7 quoted lines

>> 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