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

[Patrick Steinhardt <ps@pks.im>]

On Fri, Dec 20, 2024 at 03:56:26AM -0500, Jeff King wrote:
> On Fri, Dec 20, 2024 at 09:45:36AM +0100, Patrick Steinhardt wrote:
> 
> > >   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.
> 
> Is there any reason not to just do:
> 
>   export GIT_VERSION
>   export GIT_DATE
>   export GIT_BUILT_FROM_COMMIT
>   export GIT_USER_AGENT
> 
> in shared.mak? Then you only have to do it once, and no need for
> templates.

You could do that, yeah, but the user needs to be aware that they can.
I'm happy to not go down that path and live with the above solution.
Alternatively, this would be what the call template would look like.

Patrick

diff --git a/Documentation/Makefile b/Documentation/Makefile
index 3392e1ce7e..a7cb885b67 100644
--- a/Documentation/Makefile
+++ b/Documentation/Makefile
@@ -211,12 +211,10 @@ XMLTO_EXTRA += --skip-validation
 XMLTO_EXTRA += -x manpage.xsl
 
 asciidoctor-extensions.rb: asciidoctor-extensions.rb.in FORCE
-	$(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
+	$(QUIET_GEN)$(call version_gen,$(shell pwd)/..,$<,$@)
 else
 asciidoc.conf: asciidoc.conf.in FORCE
-	$(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
+	$(QUIET_GEN)$(call version_gen,$(shell pwd)/..,$<,$@)
 endif
 
 ASCIIDOC_DEPS += docinfo.html
diff --git a/Makefile b/Makefile
index 79739a13d2..9cfe3d0aa9 100644
--- a/Makefile
+++ b/Makefile
@@ -593,7 +593,7 @@ include shared.mak
 
 GIT-VERSION-FILE: FORCE
 	@OLD=$$(cat $@ 2>/dev/null || :) && \
-	$(SHELL_PATH) ./GIT-VERSION-GEN "$(shell pwd)" GIT-VERSION-FILE.in $@ && \
+	$(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
@@ -2512,8 +2512,7 @@ pager.sp pager.s pager.o: EXTRA_CPPFLAGS = \
 	-DPAGER_ENV='$(PAGER_ENV_CQ_SQ)'
 
 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
+	$(QUIET_GEN)$(call version_gen,"$(shell pwd)",$<,$@)
 
 version.sp version.s version.o: version-def.h
 
@@ -2554,8 +2553,7 @@ $(SCRIPT_SH_GEN) $(SCRIPT_LIB) : % : %.sh generate-script.sh GIT-BUILD-OPTIONS G
 	mv $@+ $@
 
 git.rc: git.rc.in GIT-VERSION-GEN GIT-VERSION-FILE
-	$(QUIET_GEN)$(SHELL_PATH) ./GIT-VERSION-GEN "$(shell pwd)" $< $@+
-	@if cmp $@+ $@ >/dev/null 2>&1; then $(RM) $@+; else mv $@+ $@; fi
+	$(QUIET_GEN)$(call version_gen,"$(shell pwd)",$<,$@)
 
 git.res: git.rc GIT-PREFIX
 	$(QUIET_RC)$(RC) -i $< -o $@
diff --git a/shared.mak b/shared.mak
index 29bebd30d8..8e0a19691f 100644
--- a/shared.mak
+++ b/shared.mak
@@ -116,3 +116,14 @@ endef
 define libpath_template
 -L$(1) $(if $(filter-out -L,$(CC_LD_DYNPATH)),$(CC_LD_DYNPATH)$(1))
 endef
+
+# Populate build information into a file via GIT-VERSION-GEN. Requires the
+# absolute path to the root source directory as well as input and output files
+# as arguments, in that order.
+define version_gen
+GIT_BUILT_FROM_COMMIT="$(GIT_BUILT_FROM_COMMIT)" \
+GIT_DATE="$(GIT_DATE)" \
+GIT_USER_AGENT="$(GIT_USER_AGENT)" \
+GIT_VERSION="$(GIT_VERSION)" \
+$(SHELL_PATH) "$(1)/GIT-VERSION-GEN" "$(1)" "$(2)" "$(3)"
+endef