Re: [PATCH 1/2] GIT-VERSION-GEN: fix overriding version via environment

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Thu, Dec 19, 2024 at 04:53:36PM +0100, Patrick Steinhardt wrote:

> But that subtle mechanism broke with 4838deab65 (Makefile: refactor
> GIT-VERSION-GEN to be reusable, 2024-12-06) and subsequent commits
> because the version information is not propagated via the Makefile
> variable anymore, but instead via the files that `GIT-VERSION-GEN`
> started to write. And as the script never knew about the `GIT_VERSION`
> environment variable in the first place it uses one of the values listed
> above instead of the overridden value.
> 
> Fix this issue by making `GIT-VERSION-GEN` handle the case where
> `GIT_VERSION` has been set via the environment.

I think this is the right direction, but there are two subtleties we
might want to address. The first is that you are adjusting $VN and not
$GIT_VERSION itself:

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


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.

-Peff

PS I don't know if meson.build would need something similar. It does not
   even pass through GIT_USER_AGENT now.




[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]

  Powered by Linux