On Fri, Dec 20, 2024 at 02:34:37AM -0500, Jeff King wrote:
> On Thu, Dec 19, 2024 at 04:53:36PM +0100, Patrick Steinhardt wrote:
> > diff --git a/GIT-VERSION-GEN b/GIT-VERSION-GEN
> > index de0e63bdfbac263884e2ea328cc2ef11ace7a238..787c6cfd04f0a43d0c1c8a6690185d26ccf2fc2f 100755
> > --- a/GIT-VERSION-GEN
> > +++ b/GIT-VERSION-GEN
> > @@ -29,7 +29,10 @@ export GIT_CEILING_DIRECTORIES
> >
> > # First see if there is a version file (included in release tarballs),
> > # then try git-describe, then default.
> > -if test -f "$SOURCE_DIR"/version
> > +if test -n "$GIT_VERSION"
> > +then
> > + VN="$GIT_VERSION"
> > +elif test -f "$SOURCE_DIR"/version
> > then
> > VN=$(cat "$SOURCE_DIR"/version) || VN="$DEF_VER"
> > elif {
>
> Later we process $VN into $GIT_VERSION, but after removing the leading
> "v" (which would usually be there from the tag name):
>
> GIT_VERSION=$(expr "$VN" : v*'\(.*\)')
>
> So if I do:
>
> make GIT_VERSION=very-special
>
> with v2.47 I'd end up with the version "very-special". But after your
> patch, it is "ery-special".
>
> I'd guess it's unlikely to come up in practice, but if we are trying to
> restore the old behavior, that's one difference.
Ah, indeed, will fix.
> The second is that the value is read from the environment, but make will
> not always put its variables into the environment. Ones given on the
> command line are, so:
>
> make GIT_VERSION=foo
>
> works as before. But:
>
> echo 'GIT_VERSION = foo' >config.mak
> make
>
> will not, as the variable isn't set in the environment. The invocation
> of GIT-VERSION-GEN already passes along GIT_USER_AGENT explicitly:
>
> version-def.h: version-def.h.in GIT-VERSION-GEN GIT-VERSION-FILE GIT-USER-AGENT
> $(QUIET_GEN)GIT_USER_AGENT="$(GIT_USER_AGENT)" $(SHELL_PATH) ./GIT-VERSION-GEN "$(shell pwd)" $< $@+
> @if cmp $@+ $@ >/dev/null 2>&1; then $(RM) $@+; else mv $@+ $@; fi
>
> Should we do the same thing for GIT_VERSION? And GIT_DATE, etc? If we're
> going to do many of these, it might also be easier to just add "export
> GIT_VERSION", etc, in the Makefile.
I guess. It'll become quite painful to do this at every callsite, so
I'll add another commit on top to introduce a call template that does
all of this for us.
> PS I don't know if meson.build would need something similar. It does not
> even pass through GIT_USER_AGENT now.
It's easy enough to do, so why not?
Patrick
