Re: [PATCH v2 4/5] Makefile: respect build info declared in "config.mak"

From: Patrick Steinhardt <ps@pks.im>
Date: Dec 20, 2024, 18:18 UTC
Message-ID: <Z2W06zRsQei5E34D@pks.im>
In-Reply-To: <Z2WxIRcV0LOvx6OX@pks.im>

On Fri, Dec 20, 2024 at 07:02:10PM +0100, Patrick Steinhardt wrote:

Show 52 quoted lines

> On Fri, Dec 20, 2024 at 12:51:36PM -0500, Jeff King wrote:
> > On Fri, Dec 20, 2024 at 05:47:14PM +0100, Patrick Steinhardt wrote:
> > 
> > > > This looks good. It fixes the issue, and I am happy that:
> > > > 
> > > > >  asciidoctor-extensions.rb: asciidoctor-extensions.rb.in FORCE
> > > > > -	$(QUIET_GEN)GIT_USER_AGENT="$(GIT_USER_AGENT)" $(SHELL_PATH) ../GIT-VERSION-GEN "$(shell pwd)/.." $< $@
> > > > > +	$(QUIET_GEN)$(SHELL_PATH) ../GIT-VERSION-GEN "$(shell pwd)/.." $< $@
> > > > 
> > > > ...these spots get even simpler.
> > > 
> > > Meh. I just noticed that this doesn't work: we include GIT-VERSION-FILE
> > > and export its value, and consequently any subsequent invocation of
> > > GIT-VERSION-GEN will continue to use the value that we have in
> > > GIT-VERSION-FILE. So it's effectively only computed the first time.
> > 
> > I'm not sure what you mean.
> > 
> > I wondered earlier if we might into a chicken-and-egg problem like that,
> > but I tested and it seemed to work fine. The rule for GIT-VERSION-FILE
> > means we'll build it before make reads it, so that first run of it will
> > get the updated value. And:
> > 
> >   make GIT_VERSION=foo && bin-wrappers/git version
> >   make GIT_VERSION=bar && bin-wrappers/git version
> > 
> > does what you'd expect. And the docs work the same way:
> > 
> >   cd Documentation
> >   make GIT_VERSION=foo git.1 && man -l git.1
> >   make GIT_VERSION=bar git.1 && man -l git.1
> > 
> > Is there a case you found that doesn't work?
> 
> Yes:
> 
>     $ make GIT-VERSION-FILE GIT_VERSION=foo
>     GIT_VERSION=foo
>     make: 'GIT-VERSION-FILE' is up to date.
>     $ cat GIT-VERSION-FILE
>     GIT_VERSION=foo
> 
>     # And now run without GIT_VERSION set.
>     make: 'GIT-VERSION-FILE' is up to date.
>     GIT_VERSION=foo
> 
> So the value remains "sticky" in this case. And that is true whenever
> you don't set GIT_VERSION at all, we always stick with what is currently
> in that file.
> 
> I've got a version now that works for all cases, but I had to use
> Makefile templates and some shuffling to make it work.

The root of the problem is this [1]:

    To this end, after reading in all makefiles make will consider each
    as a goal target, in the order in which they were processed, and
    attempt to update it. If parallel builds (see Parallel Execution)
    are enabled then makefiles will be rebuilt in parallel as well.

So the Makefile will _first_ read in all includes before deciding whether or not it needs to regenerate any of them. So we already export the current GIT_VERSION in case "GIT-VERSION-FILE" exists at the time where we run "GIT-VERSION-GEN".

That behaviour is quite surprising to me, but it seems to work as designed.

Patrick

[1]: https://www.gnu.org/software/make/manual/html_node/Remaking-Makefiles.html