On 22/09/2021 18:53, Jeff King wrote:
> 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).
>> 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
