Subject: Re: [PATCH 8/8] hook-list.h: add a generated list of hooks, like config-list.h

From: Ævar Arnfjörð Bjarmason <[email-masked]>
Date: Mon, 15 Nov 2021 23:49:31 +0100

On Tue, Nov 16 2021, Mike Hommey wrote:

> On Mon, Nov 15, 2021 at 11:26:36PM +0100, Ævar Arnfjörð Bjarmason wrote:
>> 
>> On Tue, Nov 16 2021, Mike Hommey wrote:
>> 
>> > On Thu, Sep 23, 2021 at 12:30:03PM +0200, Ævar Arnfjörð Bjarmason wrote:
>> >> -builtin/help.sp builtin/help.s builtin/help.o: config-list.h GIT-PREFIX
>> >> +hook.sp hook.s hook.o: hook-list.h
>> >> +
>> >> +builtin/help.sp builtin/help.s builtin/help.o: config-list.h hook-list.h GIT-PREFIX
>> >
>> > hook-list.h is only included from buitin/bugreport.c, so
>> > builtin/bugreport.o should be the one with the hook-list.h dependency,
>> > shouldn't it?
>> 
>> Well spotted, yes. This is a mistake. I think from some earlier WIP
>> version of the series.
>> 
>> In practice we don't really miss dependencies due to these sorts of
>> mistakes since we use the .depends files, i.e. GCC & Clang figure this
>> out for us:
>> 
>>     $ grep hook-list .depend/* */.depend/*
>>     builtin/.depend/bugreport.o.d: compat/compiler.h git-compat-util.h hook.h hook-list.h
>>     builtin/.depend/bugreport.o.d:hook-list.h:
>
> But aren't those .depends files are only created when compiling object
> files, such that builtin/.depend/bugreport.o.d wouldn't exist until
> bugreport.c is compiled, which would fail if hook-list.h wasn't created
> before that?

Fail how?

I don't think it could fail, because the purpose of these dependency
relationships is to avoid needless *re*builds. So if you're building for
the first time it doesn't matter, your compiler will find the relevant
things to include for you. It doesn't need what's in the Makefile to do
that.

See [1], what I said about LIB_H there applies more generally for the
.depends files.

It will only fail in the sense that it over-depends, i.e. if you do:

    git clean -dxf; make builtin/help.o

We'll generate the hook list, as opposed to for buildin/add.o or
whatever, but I'd say that generally doesn't matter all that much, what
matters is that we don't miss dependency relationships.

Anyway, I'll try to loop around to fixing this, just wanted to gather
some interest in the more general set of fixes I've got. I've got some
fixes queued up after that that fix/improve the *.sp and *.s parts of
the line you quoted (and other similar lines). I'd prefer just to fix
this along with those so the two don't textually conflict.

1. https://lore.kernel.org/git/[email-masked]/