public inbox for git@vger.kernel.org 
 help / color / mirror / Atom feed
From: Patrick Steinhardt <ps@pks•im>
To: Jeff King <peff@peff•net>
Cc: git@vger•kernel.org, Junio C Hamano <gitster@pobox•com>,
	Kyle Lippincott <spectral@google•com>
Subject: Re: [PATCH v2 4/5] Makefile: respect build info declared in "config.mak"
Date: Fri, 20 Dec 2024 19:18:19 +0100	[thread overview]
Message-ID: <Z2W06zRsQei5E34D@pks.im> (raw)
In-Reply-To: <Z2WxIRcV0LOvx6OX@pks.im>

On Fri, Dec 20, 2024 at 07:02:10PM +0100, Patrick Steinhardt wrote:
> 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

  reply	other threads:[~2024-12-20 18:18 UTC|newest]

Thread overview: 44+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-12-19 15:53 [PATCH 0/2] GIT-VERSION-GEN: fix overriding values Patrick Steinhardt
2024-12-19 15:53 ` [PATCH 1/2] GIT-VERSION-GEN: fix overriding version via environment Patrick Steinhardt
2024-12-19 18:49   ` Junio C Hamano
2024-12-20  7:34   ` Jeff King
2024-12-20  8:45     ` Patrick Steinhardt
2024-12-20  8:56       ` Jeff King
2024-12-20  9:31         ` Patrick Steinhardt
2024-12-20 11:17           ` Jeff King
2024-12-20 12:22             ` Patrick Steinhardt
2024-12-19 15:53 ` [PATCH 2/2] GIT-VERSION-GEN: fix overriding GIT_BUILT_FROM_COMMIT and GIT_DATE Patrick Steinhardt
2024-12-19 21:19   ` Kyle Lippincott
2024-12-19 21:59     ` Junio C Hamano
2024-12-20  7:37   ` Jeff King
2024-12-20 15:04     ` Junio C Hamano
2024-12-20 12:22 ` [PATCH v2 0/5] GIT-VERSION-GEN: fix overriding values Patrick Steinhardt
2024-12-20 12:22   ` [PATCH v2 1/5] GIT-VERSION-GEN: fix overriding version via environment Patrick Steinhardt
2024-12-20 15:52     ` Jeff King
2024-12-20 16:16       ` Patrick Steinhardt
2024-12-20 16:17       ` Junio C Hamano
2024-12-20 16:23         ` Patrick Steinhardt
2024-12-20 12:22   ` [PATCH v2 2/5] GIT-VERSION-GEN: fix overriding GIT_BUILT_FROM_COMMIT and GIT_DATE Patrick Steinhardt
2024-12-20 12:22   ` [PATCH v2 3/5] Makefile: drop unneeded indirection for GIT-VERSION-GEN outputs Patrick Steinhardt
2024-12-20 15:53     ` Jeff King
2024-12-20 12:22   ` [PATCH v2 4/5] Makefile: respect build info declared in "config.mak" Patrick Steinhardt
2024-12-20 15:54     ` Jeff King
2024-12-20 16:47       ` Patrick Steinhardt
2024-12-20 17:51         ` Jeff King
2024-12-20 18:02           ` Patrick Steinhardt
2024-12-20 18:18             ` Patrick Steinhardt [this message]
2024-12-20 18:24             ` Jeff King
2024-12-20 18:39               ` Patrick Steinhardt
2024-12-20 19:07                 ` Patrick Steinhardt
2024-12-28 19:43                   ` Jeff King
2024-12-20 12:22   ` [PATCH v2 5/5] meson: add options to override build information Patrick Steinhardt
2024-12-20 15:56   ` [PATCH v2 0/5] GIT-VERSION-GEN: fix overriding values Jeff King
2024-12-20 19:44 ` [PATCH v3 0/6] " Patrick Steinhardt
2024-12-20 19:44   ` [PATCH v3 1/6] Makefile: stop including "GIT-VERSION-FILE" in docs Patrick Steinhardt
2024-12-20 19:44   ` [PATCH v3 2/6] Makefile: drop unneeded indirection for GIT-VERSION-GEN outputs Patrick Steinhardt
2024-12-20 19:44   ` [PATCH v3 3/6] Makefile: introduce template for GIT-VERSION-GEN Patrick Steinhardt
2024-12-20 19:44   ` [PATCH v3 4/6] GIT-VERSION-GEN: fix overriding GIT_VERSION Patrick Steinhardt
2024-12-20 21:50     ` Junio C Hamano
2024-12-21 10:30       ` Patrick Steinhardt
2024-12-20 19:44   ` [PATCH v3 5/6] GIT-VERSION-GEN: fix overriding GIT_BUILT_FROM_COMMIT and GIT_DATE Patrick Steinhardt
2024-12-20 19:44   ` [PATCH v3 6/6] meson: add options to override build information Patrick Steinhardt

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=Z2W06zRsQei5E34D@pks.im \
    --to=ps@pks$(echo .)im \
    --cc=git@vger$(echo .)kernel.org \
    --cc=gitster@pobox$(echo .)com \
    --cc=peff@peff$(echo .)net \
    --cc=spectral@google$(echo .)com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox