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

From: Ramsay Jones <ramsay@ramsayjones.plus.com>
Date: Sep 22, 2021, 16:58 UTC
Message-ID: <c13b050c-abb9-c669-b92c-930f2b43ce10@ramsayjones.plus.com>
In-Reply-To: <YUqQzn5vFDpbF5dM@coredump.intra.peff.net>

On 22/09/2021 03:11, Jeff King wrote:

Show 13 quoted lines

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

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

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

So, lets try something:

  $ make hdr-check
  GIT_VERSION = 2.33.0.517.g53f5cfaf01
      HDR add-interactive.h
  ...
      HDR xdiff-interface.h
  $

OK, that seems to work.
  
  $ find . -iname '*.hcc' | wc -l
  208
  $ find . -iname '*.hco' | wc -l
  200
  $

Hmm, odd:
  
  $ find . -iname '*.hcc' | sed s/.hcc// | sort >zz
  $ find . -iname '*.hco' | sed s/.hco// | sort >xx
  $ diff zz xx
  90d89
  < ./merge-strategies
  137d135
  < ./reftable/slice
  152d149
  < ./sha1-lookup
  198,202d194
  < ./vcs-svn/fast_export
  < ./vcs-svn/line_buffer
  < ./vcs-svn/sliding_window
  < ./vcs-svn/svndiff
  < ./vcs-svn/svndump
  $

... just noticed in passing, I didn't investigate.

Now, by definition, every '*.hcc' file depends on git-compat-util.h, so after changing that header an 'hdr-check' should check every header:

  $ touch git-compat-util.h
  $ make hdr-check
      HDR git-compat-util.h
  $

Hmm, disappointing! Similarly, if I change (say) 'cache.h', then all
the headers that '#include' that file, in addition to 'cache.h', should
also be checked:
  
  $ git grep -n 'include.*cache\.h' -- '*.h' | wc -l
  35
  $ touch cache.h
  $ make hdr-check
      HDR cache.h
  $

Hmm, not quite. So, the sparse target should have similar problems:
  
  $ make sparse
      * new build flags
      SP abspath.c
  ...
      SP remote-curl.c
  $

OK, that works.
  
  $ find . -iname '*.sp' | wc -l
  452
  $ 
  
  $ make sparse
  $ touch git-compat-util.h
  $ make sparse
  $ touch git.h
  $ make sparse
  $ touch git.c
  $ make sparse
      SP git.c
  $ 
  
  $ make clean
  ...
  rm -f GIT-SCRIPT-DEFINES GIT-PERL-DEFINES GIT-PERL-HEADER GIT-PYTHON-VARS
  $ find . -iname '*.sp' | wc -l
  452
  $ 
 
Ah, yes, you may want to add the removal of the 'synthetic objects' to the
make clean target!

As I said, I don't quite understand what these patches want to do, so I can't offer any solutions. :( Well, you could *add* the necessary dependencies, of course, but that could lead to a rabbit hole which I would not want to go down!

ATB, Ramsay Jones