| Message ID | 20241220-b4-pks-git-version-via-environment-v3-4-1fd79b52a5fb@pks.im (mailing list archive) |
|---|---|
| State | Accepted |
| Commit | 992bc5618f2879a8d7f00a60489e78c48e661820 |
| Headers | show |
| Series | GIT-VERSION-GEN: fix overriding values | expand |
Patrick Steinhardt <ps@pks.im> writes: > diff --git a/Documentation/Makefile b/Documentation/Makefile > index ff30ab6c4295525757f6a150ec4ff0c72487f440..a89823e1d1ee5042367bdcca6ed426196d49ce89 100644 > --- a/Documentation/Makefile > +++ b/Documentation/Makefile > @@ -181,6 +181,10 @@ endif > -include ../config.mak.autogen > -include ../config.mak > > +# Set GIT_VERSION_OVERRIDE such that version_gen knows to substitute > +# GIT_VERSION in case it was set by the user. > +GIT_VERSION_OVERRIDE := $(GIT_VERSION) > + > ifndef NO_MAN_BOLD_LITERAL > XMLTO_EXTRA += -m manpage-bold-literal.xsl > endif So the idea is that those targets and scripts may have their own GIT_VERION value when they run GIT-VERSION-GEN to cause GIT_VERSION to computed, and in such a case, they should pass the GIT_VERSION they have in GIT_VERSION_OVERRIDE, and thanks to the version_gen thing, this value in GIT_VERSION_OVERRIDE is passed in the environment as GIT_VERSION when GIT-VERSION-GEN is run, and the value in turn is passed intact. Somehow this makes my head spin, as it looks quite convoluted, but the overall flow should yield the desired value. Queued. Thanks.
On Fri, Dec 20, 2024 at 01:50:25PM -0800, Junio C Hamano wrote: > Patrick Steinhardt <ps@pks.im> writes: > > > diff --git a/Documentation/Makefile b/Documentation/Makefile > > index ff30ab6c4295525757f6a150ec4ff0c72487f440..a89823e1d1ee5042367bdcca6ed426196d49ce89 100644 > > --- a/Documentation/Makefile > > +++ b/Documentation/Makefile > > @@ -181,6 +181,10 @@ endif > > -include ../config.mak.autogen > > -include ../config.mak > > > > +# Set GIT_VERSION_OVERRIDE such that version_gen knows to substitute > > +# GIT_VERSION in case it was set by the user. > > +GIT_VERSION_OVERRIDE := $(GIT_VERSION) > > + > > ifndef NO_MAN_BOLD_LITERAL > > XMLTO_EXTRA += -m manpage-bold-literal.xsl > > endif > > So the idea is that those targets and scripts may have their own > GIT_VERION value when they run GIT-VERSION-GEN to cause GIT_VERSION > to computed, and in such a case, they should pass the GIT_VERSION > they have in GIT_VERSION_OVERRIDE, and thanks to the version_gen > thing, this value in GIT_VERSION_OVERRIDE is passed in the > environment as GIT_VERSION when GIT-VERSION-GEN is run, and the > value in turn is passed intact. Somehow this makes my head spin, as > it looks quite convoluted, but the overall flow should yield the > desired value. Agreed, it makes mine spin, as well. Next release cycle I'll have a look at whether I can get rid of the include of "GIT-VERSION-FILE" altogether to make the logic simpler. Patrick
diff --git a/Documentation/Makefile b/Documentation/Makefile index ff30ab6c4295525757f6a150ec4ff0c72487f440..a89823e1d1ee5042367bdcca6ed426196d49ce89 100644 --- a/Documentation/Makefile +++ b/Documentation/Makefile @@ -181,6 +181,10 @@ endif -include ../config.mak.autogen -include ../config.mak +# Set GIT_VERSION_OVERRIDE such that version_gen knows to substitute +# GIT_VERSION in case it was set by the user. +GIT_VERSION_OVERRIDE := $(GIT_VERSION) + ifndef NO_MAN_BOLD_LITERAL XMLTO_EXTRA += -m manpage-bold-literal.xsl endif diff --git a/GIT-VERSION-GEN b/GIT-VERSION-GEN index de0e63bdfbac263884e2ea328cc2ef11ace7a238..9b785da226eff2d7952d3306f45fd2933fdafaca 100755 --- a/GIT-VERSION-GEN +++ b/GIT-VERSION-GEN @@ -27,31 +27,35 @@ fi GIT_CEILING_DIRECTORIES="$SOURCE_DIR/.." 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 -z "$GIT_VERSION" then - VN=$(cat "$SOURCE_DIR"/version) || VN="$DEF_VER" -elif { - test -d "$SOURCE_DIR/.git" || - test -d "${GIT_DIR:-.git}" || - test -f "$SOURCE_DIR"/.git; - } && - VN=$(git -C "$SOURCE_DIR" describe --match "v[0-9]*" HEAD 2>/dev/null) && - case "$VN" in - *$LF*) (exit 1) ;; - v[0-9]*) - git -C "$SOURCE_DIR" update-index -q --refresh - test -z "$(git -C "$SOURCE_DIR" diff-index --name-only HEAD --)" || - VN="$VN-dirty" ;; - esac -then - VN=$(echo "$VN" | sed -e 's/-/./g'); -else - VN="$DEF_VER" + # First see if there is a version file (included in release tarballs), + # then try git-describe, then default. + if test -f "$SOURCE_DIR"/version + then + VN=$(cat "$SOURCE_DIR"/version) || VN="$DEF_VER" + elif { + test -d "$SOURCE_DIR/.git" || + test -d "${GIT_DIR:-.git}" || + test -f "$SOURCE_DIR"/.git; + } && + VN=$(git -C "$SOURCE_DIR" describe --match "v[0-9]*" HEAD 2>/dev/null) && + case "$VN" in + *$LF*) (exit 1) ;; + v[0-9]*) + git -C "$SOURCE_DIR" update-index -q --refresh + test -z "$(git -C "$SOURCE_DIR" diff-index --name-only HEAD --)" || + VN="$VN-dirty" ;; + esac + then + VN=$(echo "$VN" | sed -e 's/-/./g'); + else + VN="$DEF_VER" + fi + + GIT_VERSION=$(expr "$VN" : v*'\(.*\)') fi -GIT_VERSION=$(expr "$VN" : v*'\(.*\)') GIT_BUILT_FROM_COMMIT=$(git -C "$SOURCE_DIR" rev-parse -q --verify HEAD 2>/dev/null) GIT_DATE=$(git -C "$SOURCE_DIR" show --quiet --format='%as' 2>/dev/null) if test -z "$GIT_USER_AGENT" diff --git a/Makefile b/Makefile index 9cfe3d0aa968eff10379d22edff6cc6f4518c2ff..3cdd2278fdaa40feb6139992aa275ad26aaae4a6 100644 --- a/Makefile +++ b/Makefile @@ -591,13 +591,6 @@ include shared.mak # # Disable -pedantic compilation. -GIT-VERSION-FILE: FORCE - @OLD=$$(cat $@ 2>/dev/null || :) && \ - $(call version_gen,"$(shell pwd)",GIT-VERSION-FILE.in,$@) && \ - NEW=$$(cat $@ 2>/dev/null || :) && \ - if test "$$OLD" != "$$NEW"; then echo "$$NEW" >&2; fi --include GIT-VERSION-FILE - # Set our default configuration. # # Among the variables below, these: @@ -1465,6 +1458,18 @@ ifdef DEVELOPER include config.mak.dev endif +GIT-VERSION-FILE: FORCE + @OLD=$$(cat $@ 2>/dev/null || :) && \ + $(call version_gen,"$(shell pwd)",GIT-VERSION-FILE.in,$@) && \ + NEW=$$(cat $@ 2>/dev/null || :) && \ + if test "$$OLD" != "$$NEW"; then echo "$$NEW" >&2; fi + +# We need to set GIT_VERSION_OVERRIDE before including the version file as +# otherwise any user-provided value for GIT_VERSION would have been overridden +# already. +GIT_VERSION_OVERRIDE := $(GIT_VERSION) +-include GIT-VERSION-FILE + # what 'all' will build and 'install' will install in gitexecdir, # excluding programs for built-in commands ALL_PROGRAMS = $(PROGRAMS) $(SCRIPTS) diff --git a/shared.mak b/shared.mak index b23c5505c9692b032cd0b18d3e4ede288614d937..a66f46969e301b35d88650f2c6abc6c0ba1b0f3d 100644 --- a/shared.mak +++ b/shared.mak @@ -122,5 +122,6 @@ endef # as arguments, in that order. define version_gen GIT_USER_AGENT="$(GIT_USER_AGENT)" \ +GIT_VERSION="$(GIT_VERSION_OVERRIDE)" \ $(SHELL_PATH) "$(1)/GIT-VERSION-GEN" "$(1)" "$(2)" "$(3)" endef
GIT-VERSION-GEN tries to derive the version that Git is being built from via multiple different sources in the following order: 1. A file called "version" in the source tree's root directory, if it exists. 2. The current commit in case Git is built from a Git repository. 3. Otherwise, we use a fallback version stored in a variable which is bumped whenever a new Git version is getting tagged. It used to be possible to override the version by overriding the `GIT_VERSION` Makefile variable (e.g. `make GIT_VERSION=foo`). This worked somewhat by chance, only: `GIT-VERSION-GEN` would write the actual Git version into `GIT-VERSION-FILE`, not the overridden value, but when including the file into our Makefile we would not override the `GIT_VERSION` variable because it has already been set by the user. And because our Makefile used the variable to propagate the version to our build tools instead of using `GIT-VERSION-FILE` the resulting build artifacts used the overridden version. 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. Note that this requires us to introduce a new GIT_VERSION_OVERRIDE variable that stores a potential user-provided value, either via the environment or via "config.mak". Ideally we wouldn't need it and could just continue to use GIT_VERSION for this. But unfortunately, Makefiles will first include all sub-Makefiles before figuring out whether it needs to re-make any of them [1]. Consequently, if there already is a GIT-VERSION-FILE, we would have slurped in its value of GIT_VERSION before we call GIT-VERSION-GEN, and because GIT-VERSION-GEN now uses that value as an override it would mean that the first generated value for GIT_VERSION will remain unchanged. Furthermore we have to move the include for "GIT-VERSION-FILE" after the includes for "config.mak" and related so that GIT_VERSION_OVERRIDE can be set to the value provided by "config.mak". [1]: https://www.gnu.org/software/make/manual/html_node/Remaking-Makefiles.html Signed-off-by: Patrick Steinhardt <ps@pks.im> --- Documentation/Makefile | 4 ++++ GIT-VERSION-GEN | 48 ++++++++++++++++++++++++++---------------------- Makefile | 19 ++++++++++++------- shared.mak | 1 + 4 files changed, 43 insertions(+), 29 deletions(-)