{"thread":{"id":"62670","subject":"[PATCH 0/2] GIT-VERSION-GEN: fix overriding values","startedAt":"2024-12-19T15:55:22Z","lastAt":"2024-12-28T19:43:47Z","messageCount":44,"participants":["Patrick Steinhardt","Junio C Hamano","Kyle Lippincott","Jeff King"],"isPatch":true,"patchVersion":1,"patchTotal":2},"messages":[{"id":"509337","messageId":"20241219-b4-pks-git-version-via-environment-v1-0-9393af058240@pks.im","threadId":"62670","inReplyTo":null,"subject":"[PATCH 0/2] GIT-VERSION-GEN: fix overriding values","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2024-12-19T15:53:35Z","receivedAt":"2024-12-19T15:55:22Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"Hi,\n\nPeff reported that overriding GIT_VERSION and GIT_DATE broke recently\ndue to the refactoring of GIT-VERSION-GEN. This small commit series\nfixes those cases, but also fixes the equivalent issue with\nGIT_BUILT_FROM_COMMIT.\n\nThanks!\n\nPatrick\n\n---\nPatrick Steinhardt (2):\n      GIT-VERSION-GEN: fix overriding version via environment\n      GIT-VERSION-GEN: fix overriding GIT_BUILT_FROM_COMMIT and GIT_DATE\n\n GIT-VERSION-GEN | 19 +++++++++++++++----\n 1 file changed, 15 insertions(+), 4 deletions(-)\n\n\n---\nbase-commit: d882f382b3d939d90cfa58d17b17802338f05d66\nchange-id: 20241219-b4-pks-git-version-via-environment-035490abec26\n\n"},{"id":"509338","messageId":"20241219-b4-pks-git-version-via-environment-v1-2-9393af058240@pks.im","threadId":"62670","inReplyTo":"20241219-b4-pks-git-version-via-environment-v1-0-9393af058240@pks.im","subject":"[PATCH 2/2] GIT-VERSION-GEN: fix overriding GIT_BUILT_FROM_COMMIT and GIT_DATE","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2024-12-19T15:53:37Z","receivedAt":"2024-12-19T15:55:23Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"Same as with the preceding commit, neither GIT_BUILT_FROM_COMMIT nor\nGIT_DATE can be overridden via the environment. Especially the latter is\nof importance given that we set it in our own \"Documentation/doc-diff\"\nscript.\n\nMake the values of both variables overridable.\n\nSigned-off-by: Patrick Steinhardt <ps@pks.im>\n---\n GIT-VERSION-GEN | 14 +++++++++++---\n 1 file changed, 11 insertions(+), 3 deletions(-)\n\ndiff --git a/GIT-VERSION-GEN b/GIT-VERSION-GEN\nindex 787c6cfd04f0a43d0c1c8a6690185d26ccf2fc2f..f8367f6d09ff2ada8868e575d6ec8f1f9b27534d 100755\n--- a/GIT-VERSION-GEN\n+++ b/GIT-VERSION-GEN\n@@ -53,10 +53,18 @@ then\n else\n \tVN=\"$DEF_VER\"\n fi\n-\n GIT_VERSION=$(expr \"$VN\" : v*'\\(.*\\)')\n-GIT_BUILT_FROM_COMMIT=$(git -C \"$SOURCE_DIR\" rev-parse -q --verify HEAD 2>/dev/null)\n-GIT_DATE=$(git -C \"$SOURCE_DIR\" show --quiet --format='%as' 2>/dev/null)\n+\n+if test -z \"$GIT_BUILT_FROM_COMMIT\"\n+then\n+    GIT_BUILT_FROM_COMMIT=$(git -C \"$SOURCE_DIR\" rev-parse -q --verify HEAD 2>/dev/null)\n+fi\n+\n+if test -z \"$GIT_DATE\"\n+then\n+    GIT_DATE=$(git -C \"$SOURCE_DIR\" show --quiet --format='%as' 2>/dev/null)\n+fi\n+\n if test -z \"$GIT_USER_AGENT\"\n then\n \tGIT_USER_AGENT=\"git/$GIT_VERSION\"\n\n-- \n2.47.0\n\n"},{"id":"509339","messageId":"20241219-b4-pks-git-version-via-environment-v1-1-9393af058240@pks.im","threadId":"62670","inReplyTo":"20241219-b4-pks-git-version-via-environment-v1-0-9393af058240@pks.im","subject":"[PATCH 1/2] GIT-VERSION-GEN: fix overriding version via environment","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2024-12-19T15:53:36Z","receivedAt":"2024-12-19T15:55:24Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"GIT-VERSION-GEN tries to derive the version that Git is being built from\nvia multiple different sources in the following order:\n\n  1. A file called \"version\" in the source tree's root directory, if it\n     exists.\n\n  2. The current commit in case Git is built from a Git repository.\n\n  3. Otherwise, we use a fallback version stored in a variable which is\n     bumped whenever a new Git version is getting tagged.\n\nIt used to be possible to override the version by overriding the\n`GIT_VERSION` Makefile variable (e.g. `make GIT_VERSION=foo`). This\nworked somewhat by chance, only: `GIT-VERSION-GEN` would write the\nactual Git version into `GIT-VERSION-FILE`, not the overridden value,\nbut when including the file into our Makefile we would not override the\n`GIT_VERSION` variable because it has already been set by the user. And\nbecause our Makefile used the variable to propagate the version to our\nbuild tools instead of using `GIT-VERSION-FILE` the resulting build\nartifacts used the overridden version.\n\nBut that subtle mechanism broke with 4838deab65 (Makefile: refactor\nGIT-VERSION-GEN to be reusable, 2024-12-06) and subsequent commits\nbecause the version information is not propagated via the Makefile\nvariable anymore, but instead via the files that `GIT-VERSION-GEN`\nstarted to write. And as the script never knew about the `GIT_VERSION`\nenvironment variable in the first place it uses one of the values listed\nabove instead of the overridden value.\n\nFix this issue by making `GIT-VERSION-GEN` handle the case where\n`GIT_VERSION` has been set via the environment.\n\nSigned-off-by: Patrick Steinhardt <ps@pks.im>\n---\n GIT-VERSION-GEN | 5 ++++-\n 1 file changed, 4 insertions(+), 1 deletion(-)\n\ndiff --git a/GIT-VERSION-GEN b/GIT-VERSION-GEN\nindex de0e63bdfbac263884e2ea328cc2ef11ace7a238..787c6cfd04f0a43d0c1c8a6690185d26ccf2fc2f 100755\n--- a/GIT-VERSION-GEN\n+++ b/GIT-VERSION-GEN\n@@ -29,7 +29,10 @@ export GIT_CEILING_DIRECTORIES\n \n # First see if there is a version file (included in release tarballs),\n # then try git-describe, then default.\n-if test -f \"$SOURCE_DIR\"/version\n+if test -n \"$GIT_VERSION\"\n+then\n+    VN=\"$GIT_VERSION\"\n+elif test -f \"$SOURCE_DIR\"/version\n then\n \tVN=$(cat \"$SOURCE_DIR\"/version) || VN=\"$DEF_VER\"\n elif {\n\n-- \n2.47.0\n\n"},{"id":"509343","messageId":"xmqq8qsbik42.fsf@gitster.g","threadId":"62670","inReplyTo":"20241219-b4-pks-git-version-via-environment-v1-1-9393af058240@pks.im","subject":"Re: [PATCH 1/2] GIT-VERSION-GEN: fix overriding version via environment","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2024-12-19T18:49:17Z","receivedAt":"2024-12-19T18:49:19Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Patrick Steinhardt <ps@pks.im> writes:\n\n> worked somewhat by chance, only: ...\n>\n> But that subtle mechanism broke with 4838deab65 (Makefile: refactor\n> GIT-VERSION-GEN to be reusable, 2024-12-06) and subsequent commits\n\nWith such a nice analysis, it does not look like it was \"by chance\"\nworking, though ;-)  And ...\n\n> because the version information is not propagated via the Makefile\n> variable anymore, but instead via the files that `GIT-VERSION-GEN`\n> started to write. And as the script never knew about the `GIT_VERSION`\n> environment variable in the first place it uses one of the values listed\n> above instead of the overridden value.\n>\n> Fix this issue by making `GIT-VERSION-GEN` handle the case where\n> `GIT_VERSION` has been set via the environment.\n\n... the \"fix\" sounds very much the logical and only correct\nsolution.\n\nThanks, queued.\n\n>\n> Signed-off-by: Patrick Steinhardt <ps@pks.im>\n> ---\n>  GIT-VERSION-GEN | 5 ++++-\n>  1 file changed, 4 insertions(+), 1 deletion(-)\n>\n> diff --git a/GIT-VERSION-GEN b/GIT-VERSION-GEN\n> index de0e63bdfbac263884e2ea328cc2ef11ace7a238..787c6cfd04f0a43d0c1c8a6690185d26ccf2fc2f 100755\n> --- a/GIT-VERSION-GEN\n> +++ b/GIT-VERSION-GEN\n> @@ -29,7 +29,10 @@ export GIT_CEILING_DIRECTORIES\n>  \n>  # First see if there is a version file (included in release tarballs),\n>  # then try git-describe, then default.\n> -if test -f \"$SOURCE_DIR\"/version\n> +if test -n \"$GIT_VERSION\"\n> +then\n> +    VN=\"$GIT_VERSION\"\n> +elif test -f \"$SOURCE_DIR\"/version\n>  then\n>  \tVN=$(cat \"$SOURCE_DIR\"/version) || VN=\"$DEF_VER\"\n>  elif {\n"},{"id":"509355","messageId":"CAO_smVhcai9reesTRSrd=hHz2Sa2T7Bs-U1NZ9KcCN5PNv6T5Q@mail.gmail.com","threadId":"62670","inReplyTo":"20241219-b4-pks-git-version-via-environment-v1-2-9393af058240@pks.im","subject":"Re: [PATCH 2/2] GIT-VERSION-GEN: fix overriding GIT_BUILT_FROM_COMMIT and GIT_DATE","fromName":"Kyle Lippincott","fromEmail":"spectral@google.com","sentAt":"2024-12-19T21:19:12Z","receivedAt":"2024-12-19T21:19:25Z","isPatch":true,"sender":{"key":"spectral@google.com","avatar":"https://avatars.githubusercontent.com/u/6371650?v=4"},"body":"On Thu, Dec 19, 2024 at 7:55 AM Patrick Steinhardt <ps@pks.im> wrote:\n>\n> Same as with the preceding commit, neither GIT_BUILT_FROM_COMMIT nor\n> GIT_DATE can be overridden via the environment. Especially the latter is\n> of importance given that we set it in our own \"Documentation/doc-diff\"\n> script.\n>\n> Make the values of both variables overridable.\n>\n> Signed-off-by: Patrick Steinhardt <ps@pks.im>\n> ---\n>  GIT-VERSION-GEN | 14 +++++++++++---\n>  1 file changed, 11 insertions(+), 3 deletions(-)\n\nLooks good, thanks for fixing this and for all the work done on the\ncleanups for the build system changes.\n\n>\n> diff --git a/GIT-VERSION-GEN b/GIT-VERSION-GEN\n> index 787c6cfd04f0a43d0c1c8a6690185d26ccf2fc2f..f8367f6d09ff2ada8868e575d6ec8f1f9b27534d 100755\n> --- a/GIT-VERSION-GEN\n> +++ b/GIT-VERSION-GEN\n> @@ -53,10 +53,18 @@ then\n>  else\n>         VN=\"$DEF_VER\"\n>  fi\n> -\n>  GIT_VERSION=$(expr \"$VN\" : v*'\\(.*\\)')\n> -GIT_BUILT_FROM_COMMIT=$(git -C \"$SOURCE_DIR\" rev-parse -q --verify HEAD 2>/dev/null)\n> -GIT_DATE=$(git -C \"$SOURCE_DIR\" show --quiet --format='%as' 2>/dev/null)\n> +\n> +if test -z \"$GIT_BUILT_FROM_COMMIT\"\n> +then\n> +    GIT_BUILT_FROM_COMMIT=$(git -C \"$SOURCE_DIR\" rev-parse -q --verify HEAD 2>/dev/null)\n> +fi\n> +\n> +if test -z \"$GIT_DATE\"\n> +then\n> +    GIT_DATE=$(git -C \"$SOURCE_DIR\" show --quiet --format='%as' 2>/dev/null)\n> +fi\n> +\n>  if test -z \"$GIT_USER_AGENT\"\n>  then\n>         GIT_USER_AGENT=\"git/$GIT_VERSION\"\n>\n> --\n> 2.47.0\n>\n"},{"id":"509356","messageId":"xmqqzfkrgwps.fsf@gitster.g","threadId":"62670","inReplyTo":"CAO_smVhcai9reesTRSrd=hHz2Sa2T7Bs-U1NZ9KcCN5PNv6T5Q@mail.gmail.com","subject":"Re: [PATCH 2/2] GIT-VERSION-GEN: fix overriding GIT_BUILT_FROM_COMMIT and GIT_DATE","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2024-12-19T21:59:59Z","receivedAt":"2024-12-19T22:00:04Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Kyle Lippincott <spectral@google.com> writes:\n\n> On Thu, Dec 19, 2024 at 7:55 AM Patrick Steinhardt <ps@pks.im> wrote:\n>>\n>> Same as with the preceding commit, neither GIT_BUILT_FROM_COMMIT nor\n>> GIT_DATE can be overridden via the environment. Especially the latter is\n>> of importance given that we set it in our own \"Documentation/doc-diff\"\n>> script.\n>>\n>> Make the values of both variables overridable.\n>>\n>> Signed-off-by: Patrick Steinhardt <ps@pks.im>\n>> ---\n>>  GIT-VERSION-GEN | 14 +++++++++++---\n>>  1 file changed, 11 insertions(+), 3 deletions(-)\n>\n> Looks good, thanks for fixing this and for all the work done on the\n> cleanups for the build system changes.\n\nThanks, all.\n"},{"id":"509361","messageId":"20241220073720.GB2389154@coredump.intra.peff.net","threadId":"62670","inReplyTo":"20241219-b4-pks-git-version-via-environment-v1-2-9393af058240@pks.im","subject":"Re: [PATCH 2/2] GIT-VERSION-GEN: fix overriding GIT_BUILT_FROM_COMMIT and GIT_DATE","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2024-12-20T07:37:20Z","receivedAt":"2024-12-20T07:37:22Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Thu, Dec 19, 2024 at 04:53:37PM +0100, Patrick Steinhardt wrote:\n\n>  GIT_VERSION=$(expr \"$VN\" : v*'\\(.*\\)')\n> -GIT_BUILT_FROM_COMMIT=$(git -C \"$SOURCE_DIR\" rev-parse -q --verify HEAD 2>/dev/null)\n> -GIT_DATE=$(git -C \"$SOURCE_DIR\" show --quiet --format='%as' 2>/dev/null)\n> +\n> +if test -z \"$GIT_BUILT_FROM_COMMIT\"\n> +then\n> +    GIT_BUILT_FROM_COMMIT=$(git -C \"$SOURCE_DIR\" rev-parse -q --verify HEAD 2>/dev/null)\n> +fi\n> +\n> +if test -z \"$GIT_DATE\"\n> +then\n> +    GIT_DATE=$(git -C \"$SOURCE_DIR\" show --quiet --format='%as' 2>/dev/null)\n> +fi\n\nLooks good. I doubt anybody would want to override BUILT_FROM_COMMIT\n(and it was never possible to do so, even before your recent patches),\nbut it's reasonable to include it as well.\n\n-Peff\n"},{"id":"509363","messageId":"20241220073437.GA2389154@coredump.intra.peff.net","threadId":"62670","inReplyTo":"20241219-b4-pks-git-version-via-environment-v1-1-9393af058240@pks.im","subject":"Re: [PATCH 1/2] GIT-VERSION-GEN: fix overriding version via environment","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2024-12-20T07:34:37Z","receivedAt":"2024-12-20T07:41:20Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Thu, Dec 19, 2024 at 04:53:36PM +0100, Patrick Steinhardt wrote:\n\n> But that subtle mechanism broke with 4838deab65 (Makefile: refactor\n> GIT-VERSION-GEN to be reusable, 2024-12-06) and subsequent commits\n> because the version information is not propagated via the Makefile\n> variable anymore, but instead via the files that `GIT-VERSION-GEN`\n> started to write. And as the script never knew about the `GIT_VERSION`\n> environment variable in the first place it uses one of the values listed\n> above instead of the overridden value.\n> \n> Fix this issue by making `GIT-VERSION-GEN` handle the case where\n> `GIT_VERSION` has been set via the environment.\n\nI think this is the right direction, but there are two subtleties we\nmight want to address. The first is that you are adjusting $VN and not\n$GIT_VERSION itself:\n\n> diff --git a/GIT-VERSION-GEN b/GIT-VERSION-GEN\n> index de0e63bdfbac263884e2ea328cc2ef11ace7a238..787c6cfd04f0a43d0c1c8a6690185d26ccf2fc2f 100755\n> --- a/GIT-VERSION-GEN\n> +++ b/GIT-VERSION-GEN\n> @@ -29,7 +29,10 @@ export GIT_CEILING_DIRECTORIES\n>  \n>  # First see if there is a version file (included in release tarballs),\n>  # then try git-describe, then default.\n> -if test -f \"$SOURCE_DIR\"/version\n> +if test -n \"$GIT_VERSION\"\n> +then\n> +    VN=\"$GIT_VERSION\"\n> +elif test -f \"$SOURCE_DIR\"/version\n>  then\n>  \tVN=$(cat \"$SOURCE_DIR\"/version) || VN=\"$DEF_VER\"\n>  elif {\n\nLater we process $VN into $GIT_VERSION, but after removing the leading\n\"v\" (which would usually be there from the tag name):\n\n  GIT_VERSION=$(expr \"$VN\" : v*'\\(.*\\)')\n\nSo if I do:\n\n  make GIT_VERSION=very-special\n\nwith v2.47 I'd end up with the version \"very-special\". But after your\npatch, it is \"ery-special\".\n\nI'd guess it's unlikely to come up in practice, but if we are trying to\nrestore the old behavior, that's one difference.\n\n\nThe second is that the value is read from the environment, but make will\nnot always put its variables into the environment. Ones given on the\ncommand line are, so:\n\n  make GIT_VERSION=foo\n\nworks as before. But:\n\n  echo 'GIT_VERSION = foo' >config.mak\n  make\n\nwill not, as the variable isn't set in the environment. The invocation\nof GIT-VERSION-GEN already passes along GIT_USER_AGENT explicitly:\n\n  version-def.h: version-def.h.in GIT-VERSION-GEN GIT-VERSION-FILE GIT-USER-AGENT\n          $(QUIET_GEN)GIT_USER_AGENT=\"$(GIT_USER_AGENT)\" $(SHELL_PATH) ./GIT-VERSION-GEN \"$(shell pwd)\" $< $@+\n          @if cmp $@+ $@ >/dev/null 2>&1; then $(RM) $@+; else mv $@+ $@; fi\n\nShould we do the same thing for GIT_VERSION? And GIT_DATE, etc? If we're\ngoing to do many of these, it might also be easier to just add \"export\nGIT_VERSION\", etc, in the Makefile.\n\n-Peff\n\nPS I don't know if meson.build would need something similar. It does not\n   even pass through GIT_USER_AGENT now.\n"},{"id":"509364","messageId":"Z2UlpaDFjvl--zau@pks.im","threadId":"62670","inReplyTo":"20241220073437.GA2389154@coredump.intra.peff.net","subject":"Re: [PATCH 1/2] GIT-VERSION-GEN: fix overriding version via environment","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2024-12-20T08:45:36Z","receivedAt":"2024-12-20T08:45:57Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"On Fri, Dec 20, 2024 at 02:34:37AM -0500, Jeff King wrote:\n> On Thu, Dec 19, 2024 at 04:53:36PM +0100, Patrick Steinhardt wrote:\n> > diff --git a/GIT-VERSION-GEN b/GIT-VERSION-GEN\n> > index de0e63bdfbac263884e2ea328cc2ef11ace7a238..787c6cfd04f0a43d0c1c8a6690185d26ccf2fc2f 100755\n> > --- a/GIT-VERSION-GEN\n> > +++ b/GIT-VERSION-GEN\n> > @@ -29,7 +29,10 @@ export GIT_CEILING_DIRECTORIES\n> >  \n> >  # First see if there is a version file (included in release tarballs),\n> >  # then try git-describe, then default.\n> > -if test -f \"$SOURCE_DIR\"/version\n> > +if test -n \"$GIT_VERSION\"\n> > +then\n> > +    VN=\"$GIT_VERSION\"\n> > +elif test -f \"$SOURCE_DIR\"/version\n> >  then\n> >  \tVN=$(cat \"$SOURCE_DIR\"/version) || VN=\"$DEF_VER\"\n> >  elif {\n> \n> Later we process $VN into $GIT_VERSION, but after removing the leading\n> \"v\" (which would usually be there from the tag name):\n> \n>   GIT_VERSION=$(expr \"$VN\" : v*'\\(.*\\)')\n> \n> So if I do:\n> \n>   make GIT_VERSION=very-special\n> \n> with v2.47 I'd end up with the version \"very-special\". But after your\n> patch, it is \"ery-special\".\n> \n> I'd guess it's unlikely to come up in practice, but if we are trying to\n> restore the old behavior, that's one difference.\n\nAh, indeed, will fix.\n\n> The second is that the value is read from the environment, but make will\n> not always put its variables into the environment. Ones given on the\n> command line are, so:\n> \n>   make GIT_VERSION=foo\n> \n> works as before. But:\n> \n>   echo 'GIT_VERSION = foo' >config.mak\n>   make\n> \n> will not, as the variable isn't set in the environment. The invocation\n> of GIT-VERSION-GEN already passes along GIT_USER_AGENT explicitly:\n> \n>   version-def.h: version-def.h.in GIT-VERSION-GEN GIT-VERSION-FILE GIT-USER-AGENT\n>           $(QUIET_GEN)GIT_USER_AGENT=\"$(GIT_USER_AGENT)\" $(SHELL_PATH) ./GIT-VERSION-GEN \"$(shell pwd)\" $< $@+\n>           @if cmp $@+ $@ >/dev/null 2>&1; then $(RM) $@+; else mv $@+ $@; fi\n> \n> Should we do the same thing for GIT_VERSION? And GIT_DATE, etc? If we're\n> going to do many of these, it might also be easier to just add \"export\n> GIT_VERSION\", etc, in the Makefile.\n\nI guess. It'll become quite painful to do this at every callsite, so\nI'll add another commit on top to introduce a call template that does\nall of this for us.\n\n> PS I don't know if meson.build would need something similar. It does not\n>    even pass through GIT_USER_AGENT now.\n\nIt's easy enough to do, so why not?\n\nPatrick\n"},{"id":"509367","messageId":"20241220085626.GB133148@coredump.intra.peff.net","threadId":"62670","inReplyTo":"Z2UlpaDFjvl--zau@pks.im","subject":"Re: [PATCH 1/2] GIT-VERSION-GEN: fix overriding version via environment","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2024-12-20T08:56:26Z","receivedAt":"2024-12-20T08:56:27Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Fri, Dec 20, 2024 at 09:45:36AM +0100, Patrick Steinhardt wrote:\n\n> >   version-def.h: version-def.h.in GIT-VERSION-GEN GIT-VERSION-FILE GIT-USER-AGENT\n> >           $(QUIET_GEN)GIT_USER_AGENT=\"$(GIT_USER_AGENT)\" $(SHELL_PATH) ./GIT-VERSION-GEN \"$(shell pwd)\" $< $@+\n> >           @if cmp $@+ $@ >/dev/null 2>&1; then $(RM) $@+; else mv $@+ $@; fi\n> > \n> > Should we do the same thing for GIT_VERSION? And GIT_DATE, etc? If we're\n> > going to do many of these, it might also be easier to just add \"export\n> > GIT_VERSION\", etc, in the Makefile.\n> \n> I guess. It'll become quite painful to do this at every callsite, so\n> I'll add another commit on top to introduce a call template that does\n> all of this for us.\n\nIs there any reason not to just do:\n\n  export GIT_VERSION\n  export GIT_DATE\n  export GIT_BUILT_FROM_COMMIT\n  export GIT_USER_AGENT\n\nin shared.mak? Then you only have to do it once, and no need for\ntemplates.\n\n-Peff\n"},{"id":"509371","messageId":"Z2U5cslf10hs_-Az@pks.im","threadId":"62670","inReplyTo":"20241220085626.GB133148@coredump.intra.peff.net","subject":"Re: [PATCH 1/2] GIT-VERSION-GEN: fix overriding version via environment","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2024-12-20T09:31:30Z","receivedAt":"2024-12-20T09:31:49Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"On Fri, Dec 20, 2024 at 03:56:26AM -0500, Jeff King wrote:\n> On Fri, Dec 20, 2024 at 09:45:36AM +0100, Patrick Steinhardt wrote:\n> \n> > >   version-def.h: version-def.h.in GIT-VERSION-GEN GIT-VERSION-FILE GIT-USER-AGENT\n> > >           $(QUIET_GEN)GIT_USER_AGENT=\"$(GIT_USER_AGENT)\" $(SHELL_PATH) ./GIT-VERSION-GEN \"$(shell pwd)\" $< $@+\n> > >           @if cmp $@+ $@ >/dev/null 2>&1; then $(RM) $@+; else mv $@+ $@; fi\n> > > \n> > > Should we do the same thing for GIT_VERSION? And GIT_DATE, etc? If we're\n> > > going to do many of these, it might also be easier to just add \"export\n> > > GIT_VERSION\", etc, in the Makefile.\n> > \n> > I guess. It'll become quite painful to do this at every callsite, so\n> > I'll add another commit on top to introduce a call template that does\n> > all of this for us.\n> \n> Is there any reason not to just do:\n> \n>   export GIT_VERSION\n>   export GIT_DATE\n>   export GIT_BUILT_FROM_COMMIT\n>   export GIT_USER_AGENT\n> \n> in shared.mak? Then you only have to do it once, and no need for\n> templates.\n\nYou could do that, yeah, but the user needs to be aware that they can.\nI'm happy to not go down that path and live with the above solution.\nAlternatively, this would be what the call template would look like.\n\nPatrick\n\ndiff --git a/Documentation/Makefile b/Documentation/Makefile\nindex 3392e1ce7e..a7cb885b67 100644\n--- a/Documentation/Makefile\n+++ b/Documentation/Makefile\n@@ -211,12 +211,10 @@ XMLTO_EXTRA += --skip-validation\n XMLTO_EXTRA += -x manpage.xsl\n \n asciidoctor-extensions.rb: asciidoctor-extensions.rb.in FORCE\n-\t$(QUIET_GEN)GIT_USER_AGENT=\"$(GIT_USER_AGENT)\" $(SHELL_PATH) ../GIT-VERSION-GEN \"$(shell pwd)/..\" $< $@+\n-\t@if cmp $@+ $@ >/dev/null 2>&1; then $(RM) $@+; else mv $@+ $@; fi\n+\t$(QUIET_GEN)$(call version_gen,$(shell pwd)/..,$<,$@)\n else\n asciidoc.conf: asciidoc.conf.in FORCE\n-\t$(QUIET_GEN)GIT_USER_AGENT=\"$(GIT_USER_AGENT)\" $(SHELL_PATH) ../GIT-VERSION-GEN \"$(shell pwd)/..\" $< $@+\n-\t@if cmp $@+ $@ >/dev/null 2>&1; then $(RM) $@+; else mv $@+ $@; fi\n+\t$(QUIET_GEN)$(call version_gen,$(shell pwd)/..,$<,$@)\n endif\n \n ASCIIDOC_DEPS += docinfo.html\ndiff --git a/Makefile b/Makefile\nindex 79739a13d2..9cfe3d0aa9 100644\n--- a/Makefile\n+++ b/Makefile\n@@ -593,7 +593,7 @@ include shared.mak\n \n GIT-VERSION-FILE: FORCE\n \t@OLD=$$(cat $@ 2>/dev/null || :) && \\\n-\t$(SHELL_PATH) ./GIT-VERSION-GEN \"$(shell pwd)\" GIT-VERSION-FILE.in $@ && \\\n+\t$(call version_gen,\"$(shell pwd)\",GIT-VERSION-FILE.in,$@) && \\\n \tNEW=$$(cat $@ 2>/dev/null || :) && \\\n \tif test \"$$OLD\" != \"$$NEW\"; then echo \"$$NEW\" >&2; fi\n -include GIT-VERSION-FILE\n@@ -2512,8 +2512,7 @@ pager.sp pager.s pager.o: EXTRA_CPPFLAGS = \\\n \t-DPAGER_ENV='$(PAGER_ENV_CQ_SQ)'\n \n version-def.h: version-def.h.in GIT-VERSION-GEN GIT-VERSION-FILE GIT-USER-AGENT\n-\t$(QUIET_GEN)GIT_USER_AGENT=\"$(GIT_USER_AGENT)\" $(SHELL_PATH) ./GIT-VERSION-GEN \"$(shell pwd)\" $< $@+\n-\t@if cmp $@+ $@ >/dev/null 2>&1; then $(RM) $@+; else mv $@+ $@; fi\n+\t$(QUIET_GEN)$(call version_gen,\"$(shell pwd)\",$<,$@)\n \n version.sp version.s version.o: version-def.h\n \n@@ -2554,8 +2553,7 @@ $(SCRIPT_SH_GEN) $(SCRIPT_LIB) : % : %.sh generate-script.sh GIT-BUILD-OPTIONS G\n \tmv $@+ $@\n \n git.rc: git.rc.in GIT-VERSION-GEN GIT-VERSION-FILE\n-\t$(QUIET_GEN)$(SHELL_PATH) ./GIT-VERSION-GEN \"$(shell pwd)\" $< $@+\n-\t@if cmp $@+ $@ >/dev/null 2>&1; then $(RM) $@+; else mv $@+ $@; fi\n+\t$(QUIET_GEN)$(call version_gen,\"$(shell pwd)\",$<,$@)\n \n git.res: git.rc GIT-PREFIX\n \t$(QUIET_RC)$(RC) -i $< -o $@\ndiff --git a/shared.mak b/shared.mak\nindex 29bebd30d8..8e0a19691f 100644\n--- a/shared.mak\n+++ b/shared.mak\n@@ -116,3 +116,14 @@ endef\n define libpath_template\n -L$(1) $(if $(filter-out -L,$(CC_LD_DYNPATH)),$(CC_LD_DYNPATH)$(1))\n endef\n+\n+# Populate build information into a file via GIT-VERSION-GEN. Requires the\n+# absolute path to the root source directory as well as input and output files\n+# as arguments, in that order.\n+define version_gen\n+GIT_BUILT_FROM_COMMIT=\"$(GIT_BUILT_FROM_COMMIT)\" \\\n+GIT_DATE=\"$(GIT_DATE)\" \\\n+GIT_USER_AGENT=\"$(GIT_USER_AGENT)\" \\\n+GIT_VERSION=\"$(GIT_VERSION)\" \\\n+$(SHELL_PATH) \"$(1)/GIT-VERSION-GEN\" \"$(1)\" \"$(2)\" \"$(3)\"\n+endef\n"},{"id":"509379","messageId":"20241220111716.GA140368@coredump.intra.peff.net","threadId":"62670","inReplyTo":"Z2U5cslf10hs_-Az@pks.im","subject":"Re: [PATCH 1/2] GIT-VERSION-GEN: fix overriding version via environment","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2024-12-20T11:17:16Z","receivedAt":"2024-12-20T11:17:17Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Fri, Dec 20, 2024 at 10:31:30AM +0100, Patrick Steinhardt wrote:\n\n> > > I guess. It'll become quite painful to do this at every callsite, so\n> > > I'll add another commit on top to introduce a call template that does\n> > > all of this for us.\n> > \n> > Is there any reason not to just do:\n> > \n> >   export GIT_VERSION\n> >   export GIT_DATE\n> >   export GIT_BUILT_FROM_COMMIT\n> >   export GIT_USER_AGENT\n> > \n> > in shared.mak? Then you only have to do it once, and no need for\n> > templates.\n> \n> You could do that, yeah, but the user needs to be aware that they can.\n> I'm happy to not go down that path and live with the above solution.\n> Alternatively, this would be what the call template would look like.\n\nI meant that _we_ would mark those variables for export ourselves (in\nshared.mak, which is used by all of our Makefiles, not the user's\nconfig.mak).\n\nI.e., this:\n\ndiff --git a/shared.mak b/shared.mak\nindex 29bebd30d8..4aa7dbf5e0 100644\n--- a/shared.mak\n+++ b/shared.mak\n@@ -116,3 +116,5 @@ endef\n define libpath_template\n -L$(1) $(if $(filter-out -L,$(CC_LD_DYNPATH)),$(CC_LD_DYNPATH)$(1))\n endef\n+\n+export GIT_VERSION\n\nwhich makes:\n\n  echo 'GIT_VERSION = foo' >config.mak\n  make\n\nbehave as it used to (when coupled with the fix you already sent to\nrespect the variable within the version-gen script).\n\n-Peff\n"},{"id":"509382","messageId":"Z2VhknVFuhl0xbhA@pks.im","threadId":"62670","inReplyTo":"20241220111716.GA140368@coredump.intra.peff.net","subject":"Re: [PATCH 1/2] GIT-VERSION-GEN: fix overriding version via environment","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2024-12-20T12:22:42Z","receivedAt":"2024-12-20T12:23:03Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"On Fri, Dec 20, 2024 at 06:17:16AM -0500, Jeff King wrote:\n> On Fri, Dec 20, 2024 at 10:31:30AM +0100, Patrick Steinhardt wrote:\n> \n> > > > I guess. It'll become quite painful to do this at every callsite, so\n> > > > I'll add another commit on top to introduce a call template that does\n> > > > all of this for us.\n> > > \n> > > Is there any reason not to just do:\n> > > \n> > >   export GIT_VERSION\n> > >   export GIT_DATE\n> > >   export GIT_BUILT_FROM_COMMIT\n> > >   export GIT_USER_AGENT\n> > > \n> > > in shared.mak? Then you only have to do it once, and no need for\n> > > templates.\n> > \n> > You could do that, yeah, but the user needs to be aware that they can.\n> > I'm happy to not go down that path and live with the above solution.\n> > Alternatively, this would be what the call template would look like.\n> \n> I meant that _we_ would mark those variables for export ourselves (in\n> shared.mak, which is used by all of our Makefiles, not the user's\n> config.mak).\n> \n> I.e., this:\n> \n> diff --git a/shared.mak b/shared.mak\n> index 29bebd30d8..4aa7dbf5e0 100644\n> --- a/shared.mak\n> +++ b/shared.mak\n> @@ -116,3 +116,5 @@ endef\n>  define libpath_template\n>  -L$(1) $(if $(filter-out -L,$(CC_LD_DYNPATH)),$(CC_LD_DYNPATH)$(1))\n>  endef\n> +\n> +export GIT_VERSION\n> \n> which makes:\n> \n>   echo 'GIT_VERSION = foo' >config.mak\n>   make\n> \n> behave as it used to (when coupled with the fix you already sent to\n> respect the variable within the version-gen script).\n\nAh, I misread \"shared.mak\" and thought you meant \"config.mak\". That\nmakes more sense then, thanks!\n\nPatrick\n"},{"id":"509383","messageId":"20241220-b4-pks-git-version-via-environment-v2-0-f1457a5e8c38@pks.im","threadId":"62670","inReplyTo":"20241219-b4-pks-git-version-via-environment-v1-0-9393af058240@pks.im","subject":"[PATCH v2 0/5] GIT-VERSION-GEN: fix overriding values","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2024-12-20T12:22:44Z","receivedAt":"2024-12-20T12:23:07Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"Hi,\n\nPeff reported that overriding GIT_VERSION and GIT_DATE broke recently\ndue to the refactoring of GIT-VERSION-GEN. This small commit series\nfixes those cases, but also fixes the equivalent issue with\nGIT_BUILT_FROM_COMMIT.\n\nChanges in v2:\n\n  - Don't strip leading `v`s when `GIT_VERSION` was set explicitly.\n  - Allow setting build info via \"config.mak\" again.\n  - Wire up build info options for Meson.\n  - Link to v1: https://lore.kernel.org/r/20241219-b4-pks-git-version-via-environment-v1-0-9393af058240@pks.im\n\nThanks!\n\nPatrick\n\n---\nPatrick Steinhardt (5):\n      GIT-VERSION-GEN: fix overriding version via environment\n      GIT-VERSION-GEN: fix overriding GIT_BUILT_FROM_COMMIT and GIT_DATE\n      Makefile: drop unneeded indirection for GIT-VERSION-GEN outputs\n      Makefile: respect build info declared in \"config.mak\"\n      meson: add options to override build information\n\n Documentation/Makefile    |  6 ++----\n Documentation/meson.build |  1 +\n GIT-VERSION-GEN           | 25 +++++++++++++++++++++----\n Makefile                  |  6 ++----\n meson.build               | 13 +++++++++++++\n meson_options.txt         | 10 ++++++++++\n shared.mak                |  7 +++++++\n 7 files changed, 56 insertions(+), 12 deletions(-)\n\nRange-diff versus v1:\n\n1:  3560d0934f ! 1:  f9aabaa9b7 GIT-VERSION-GEN: fix overriding version via environment\n    @@ GIT-VERSION-GEN: export GIT_CEILING_DIRECTORIES\n      then\n      \tVN=$(cat \"$SOURCE_DIR\"/version) || VN=\"$DEF_VER\"\n      elif {\n    +@@ GIT-VERSION-GEN: else\n    + \tVN=\"$DEF_VER\"\n    + fi\n    + \n    +-GIT_VERSION=$(expr \"$VN\" : v*'\\(.*\\)')\n    ++# Only strip leading `v` in case we have derived VN manually. Otherwise we\n    ++# retain whatever the user has set in their environment.\n    ++if test -z \"$GIT_VERSION\"\n    ++then\n    ++    GIT_VERSION=$(expr \"$VN\" : v*'\\(.*\\)')\n    ++fi\n    ++\n    + GIT_BUILT_FROM_COMMIT=$(git -C \"$SOURCE_DIR\" rev-parse -q --verify HEAD 2>/dev/null)\n    + GIT_DATE=$(git -C \"$SOURCE_DIR\" show --quiet --format='%as' 2>/dev/null)\n    + if test -z \"$GIT_USER_AGENT\"\n2:  ec7477d14e ! 2:  2db637757e GIT-VERSION-GEN: fix overriding GIT_BUILT_FROM_COMMIT and GIT_DATE\n    @@ Commit message\n     \n      ## GIT-VERSION-GEN ##\n     @@ GIT-VERSION-GEN: then\n    - else\n    - \tVN=\"$DEF_VER\"\n    +     GIT_VERSION=$(expr \"$VN\" : v*'\\(.*\\)')\n      fi\n    --\n    - GIT_VERSION=$(expr \"$VN\" : v*'\\(.*\\)')\n    + \n     -GIT_BUILT_FROM_COMMIT=$(git -C \"$SOURCE_DIR\" rev-parse -q --verify HEAD 2>/dev/null)\n     -GIT_DATE=$(git -C \"$SOURCE_DIR\" show --quiet --format='%as' 2>/dev/null)\n    -+\n     +if test -z \"$GIT_BUILT_FROM_COMMIT\"\n     +then\n     +    GIT_BUILT_FROM_COMMIT=$(git -C \"$SOURCE_DIR\" rev-parse -q --verify HEAD 2>/dev/null)\n-:  ---------- > 3:  1024d10b46 Makefile: drop unneeded indirection for GIT-VERSION-GEN outputs\n-:  ---------- > 4:  14a2e5d7bb Makefile: respect build info declared in \"config.mak\"\n-:  ---------- > 5:  974ab29b35 meson: add options to override build information\n\n---\nbase-commit: d882f382b3d939d90cfa58d17b17802338f05d66\nchange-id: 20241219-b4-pks-git-version-via-environment-035490abec26\n\n"},{"id":"509384","messageId":"20241220-b4-pks-git-version-via-environment-v2-1-f1457a5e8c38@pks.im","threadId":"62670","inReplyTo":"20241220-b4-pks-git-version-via-environment-v2-0-f1457a5e8c38@pks.im","subject":"[PATCH v2 1/5] GIT-VERSION-GEN: fix overriding version via environment","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2024-12-20T12:22:45Z","receivedAt":"2024-12-20T12:23:08Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"GIT-VERSION-GEN tries to derive the version that Git is being built from\nvia multiple different sources in the following order:\n\n  1. A file called \"version\" in the source tree's root directory, if it\n     exists.\n\n  2. The current commit in case Git is built from a Git repository.\n\n  3. Otherwise, we use a fallback version stored in a variable which is\n     bumped whenever a new Git version is getting tagged.\n\nIt used to be possible to override the version by overriding the\n`GIT_VERSION` Makefile variable (e.g. `make GIT_VERSION=foo`). This\nworked somewhat by chance, only: `GIT-VERSION-GEN` would write the\nactual Git version into `GIT-VERSION-FILE`, not the overridden value,\nbut when including the file into our Makefile we would not override the\n`GIT_VERSION` variable because it has already been set by the user. And\nbecause our Makefile used the variable to propagate the version to our\nbuild tools instead of using `GIT-VERSION-FILE` the resulting build\nartifacts used the overridden version.\n\nBut that subtle mechanism broke with 4838deab65 (Makefile: refactor\nGIT-VERSION-GEN to be reusable, 2024-12-06) and subsequent commits\nbecause the version information is not propagated via the Makefile\nvariable anymore, but instead via the files that `GIT-VERSION-GEN`\nstarted to write. And as the script never knew about the `GIT_VERSION`\nenvironment variable in the first place it uses one of the values listed\nabove instead of the overridden value.\n\nFix this issue by making `GIT-VERSION-GEN` handle the case where\n`GIT_VERSION` has been set via the environment.\n\nSigned-off-by: Patrick Steinhardt <ps@pks.im>\n---\n GIT-VERSION-GEN | 13 +++++++++++--\n 1 file changed, 11 insertions(+), 2 deletions(-)\n\ndiff --git a/GIT-VERSION-GEN b/GIT-VERSION-GEN\nindex de0e63bdfbac263884e2ea328cc2ef11ace7a238..27f9d6a81f77248c652649ae21d0ec51b8f2d247 100755\n--- a/GIT-VERSION-GEN\n+++ b/GIT-VERSION-GEN\n@@ -29,7 +29,10 @@ export GIT_CEILING_DIRECTORIES\n \n # First see if there is a version file (included in release tarballs),\n # then try git-describe, then default.\n-if test -f \"$SOURCE_DIR\"/version\n+if test -n \"$GIT_VERSION\"\n+then\n+    VN=\"$GIT_VERSION\"\n+elif test -f \"$SOURCE_DIR\"/version\n then\n \tVN=$(cat \"$SOURCE_DIR\"/version) || VN=\"$DEF_VER\"\n elif {\n@@ -51,7 +54,13 @@ else\n \tVN=\"$DEF_VER\"\n fi\n \n-GIT_VERSION=$(expr \"$VN\" : v*'\\(.*\\)')\n+# Only strip leading `v` in case we have derived VN manually. Otherwise we\n+# retain whatever the user has set in their environment.\n+if test -z \"$GIT_VERSION\"\n+then\n+    GIT_VERSION=$(expr \"$VN\" : v*'\\(.*\\)')\n+fi\n+\n GIT_BUILT_FROM_COMMIT=$(git -C \"$SOURCE_DIR\" rev-parse -q --verify HEAD 2>/dev/null)\n GIT_DATE=$(git -C \"$SOURCE_DIR\" show --quiet --format='%as' 2>/dev/null)\n if test -z \"$GIT_USER_AGENT\"\n\n-- \n2.48.0.rc0.184.g0fc57dec57.dirty\n\n"},{"id":"509385","messageId":"20241220-b4-pks-git-version-via-environment-v2-2-f1457a5e8c38@pks.im","threadId":"62670","inReplyTo":"20241220-b4-pks-git-version-via-environment-v2-0-f1457a5e8c38@pks.im","subject":"[PATCH v2 2/5] GIT-VERSION-GEN: fix overriding GIT_BUILT_FROM_COMMIT and GIT_DATE","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2024-12-20T12:22:46Z","receivedAt":"2024-12-20T12:23:09Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"Same as with the preceding commit, neither GIT_BUILT_FROM_COMMIT nor\nGIT_DATE can be overridden via the environment. Especially the latter is\nof importance given that we set it in our own \"Documentation/doc-diff\"\nscript.\n\nMake the values of both variables overridable.\n\nSigned-off-by: Patrick Steinhardt <ps@pks.im>\n---\n GIT-VERSION-GEN | 12 ++++++++++--\n 1 file changed, 10 insertions(+), 2 deletions(-)\n\ndiff --git a/GIT-VERSION-GEN b/GIT-VERSION-GEN\nindex 27f9d6a81f77248c652649ae21d0ec51b8f2d247..4273d5284008e2787854928163e31802bf8a3d27 100755\n--- a/GIT-VERSION-GEN\n+++ b/GIT-VERSION-GEN\n@@ -61,8 +61,16 @@ then\n     GIT_VERSION=$(expr \"$VN\" : v*'\\(.*\\)')\n fi\n \n-GIT_BUILT_FROM_COMMIT=$(git -C \"$SOURCE_DIR\" rev-parse -q --verify HEAD 2>/dev/null)\n-GIT_DATE=$(git -C \"$SOURCE_DIR\" show --quiet --format='%as' 2>/dev/null)\n+if test -z \"$GIT_BUILT_FROM_COMMIT\"\n+then\n+    GIT_BUILT_FROM_COMMIT=$(git -C \"$SOURCE_DIR\" rev-parse -q --verify HEAD 2>/dev/null)\n+fi\n+\n+if test -z \"$GIT_DATE\"\n+then\n+    GIT_DATE=$(git -C \"$SOURCE_DIR\" show --quiet --format='%as' 2>/dev/null)\n+fi\n+\n if test -z \"$GIT_USER_AGENT\"\n then\n \tGIT_USER_AGENT=\"git/$GIT_VERSION\"\n\n-- \n2.48.0.rc0.184.g0fc57dec57.dirty\n\n"},{"id":"509386","messageId":"20241220-b4-pks-git-version-via-environment-v2-3-f1457a5e8c38@pks.im","threadId":"62670","inReplyTo":"20241220-b4-pks-git-version-via-environment-v2-0-f1457a5e8c38@pks.im","subject":"[PATCH v2 3/5] Makefile: drop unneeded indirection for GIT-VERSION-GEN outputs","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2024-12-20T12:22:47Z","receivedAt":"2024-12-20T12:23:11Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"Some of the callsites of GIT-VERSION-GEN generate the target file with a\n\"+\" suffix first and then move the file into place when the new contents\nare different compared to the old contents. This allows us to avoid a\nneedless rebuild by not updating timestamps of the target file when its\ncontents will remain unchanged anyway.\n\nIn fact though, this exact logic is already handled in GIT-VERSION-GEN,\nso doing this manually is pointless. This is a leftover from an earlier\nversion of 4838deab65 (Makefile: refactor GIT-VERSION-GEN to be\nreusable, 2024-12-06), where the script didn't handle that logic for us.\n\nDrop the needless indirection.\n\nSigned-off-by: Patrick Steinhardt <ps@pks.im>\n---\n Documentation/Makefile | 6 ++----\n Makefile               | 6 ++----\n 2 files changed, 4 insertions(+), 8 deletions(-)\n\ndiff --git a/Documentation/Makefile b/Documentation/Makefile\nindex 3392e1ce7ebc540784912476847380d9c1775ac8..cee88dbda66265141b87d5e5c16bf86df22fa4ef 100644\n--- a/Documentation/Makefile\n+++ b/Documentation/Makefile\n@@ -211,12 +211,10 @@ XMLTO_EXTRA += --skip-validation\n XMLTO_EXTRA += -x manpage.xsl\n \n asciidoctor-extensions.rb: asciidoctor-extensions.rb.in FORCE\n-\t$(QUIET_GEN)GIT_USER_AGENT=\"$(GIT_USER_AGENT)\" $(SHELL_PATH) ../GIT-VERSION-GEN \"$(shell pwd)/..\" $< $@+\n-\t@if cmp $@+ $@ >/dev/null 2>&1; then $(RM) $@+; else mv $@+ $@; fi\n+\t$(QUIET_GEN)GIT_USER_AGENT=\"$(GIT_USER_AGENT)\" $(SHELL_PATH) ../GIT-VERSION-GEN \"$(shell pwd)/..\" $< $@\n else\n asciidoc.conf: asciidoc.conf.in FORCE\n-\t$(QUIET_GEN)GIT_USER_AGENT=\"$(GIT_USER_AGENT)\" $(SHELL_PATH) ../GIT-VERSION-GEN \"$(shell pwd)/..\" $< $@+\n-\t@if cmp $@+ $@ >/dev/null 2>&1; then $(RM) $@+; else mv $@+ $@; fi\n+\t$(QUIET_GEN)GIT_USER_AGENT=\"$(GIT_USER_AGENT)\" $(SHELL_PATH) ../GIT-VERSION-GEN \"$(shell pwd)/..\" $< $@\n endif\n \n ASCIIDOC_DEPS += docinfo.html\ndiff --git a/Makefile b/Makefile\nindex 79739a13d2132204f56b1ef4ca879bd51c5164b4..695a9d9765daf864605002d572129bae7a8c4e40 100644\n--- a/Makefile\n+++ b/Makefile\n@@ -2512,8 +2512,7 @@ pager.sp pager.s pager.o: EXTRA_CPPFLAGS = \\\n \t-DPAGER_ENV='$(PAGER_ENV_CQ_SQ)'\n \n version-def.h: version-def.h.in GIT-VERSION-GEN GIT-VERSION-FILE GIT-USER-AGENT\n-\t$(QUIET_GEN)GIT_USER_AGENT=\"$(GIT_USER_AGENT)\" $(SHELL_PATH) ./GIT-VERSION-GEN \"$(shell pwd)\" $< $@+\n-\t@if cmp $@+ $@ >/dev/null 2>&1; then $(RM) $@+; else mv $@+ $@; fi\n+\t$(QUIET_GEN)GIT_USER_AGENT=\"$(GIT_USER_AGENT)\" $(SHELL_PATH) ./GIT-VERSION-GEN \"$(shell pwd)\" $< $@\n \n version.sp version.s version.o: version-def.h\n \n@@ -2554,8 +2553,7 @@ $(SCRIPT_SH_GEN) $(SCRIPT_LIB) : % : %.sh generate-script.sh GIT-BUILD-OPTIONS G\n \tmv $@+ $@\n \n git.rc: git.rc.in GIT-VERSION-GEN GIT-VERSION-FILE\n-\t$(QUIET_GEN)$(SHELL_PATH) ./GIT-VERSION-GEN \"$(shell pwd)\" $< $@+\n-\t@if cmp $@+ $@ >/dev/null 2>&1; then $(RM) $@+; else mv $@+ $@; fi\n+\t$(QUIET_GEN)$(SHELL_PATH) ./GIT-VERSION-GEN \"$(shell pwd)\" $< $@\n \n git.res: git.rc GIT-PREFIX\n \t$(QUIET_RC)$(RC) -i $< -o $@\n\n-- \n2.48.0.rc0.184.g0fc57dec57.dirty\n\n"},{"id":"509387","messageId":"20241220-b4-pks-git-version-via-environment-v2-4-f1457a5e8c38@pks.im","threadId":"62670","inReplyTo":"20241220-b4-pks-git-version-via-environment-v2-0-f1457a5e8c38@pks.im","subject":"[PATCH v2 4/5] Makefile: respect build info declared in \"config.mak\"","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2024-12-20T12:22:48Z","receivedAt":"2024-12-20T12:23:12Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"In preceding commits we fixed that build info set via e.g. `make\nGIT_VERSION=foo` didn't get propagated to GIT-VERSION-GEN. Similarly\nthough, setting build info via \"config.mak\" does not work anymore either\nbecause the variables are only declared as Makefile variables and thus\naren't accessible by the script.\n\nFix the issue by exporting those variables via \"shared.mak\". This also\nallows us to deduplicate the export of GIT_USER_AGENT.\n\nSuggested-by: Jeff King <peff@peff.net>\nSigned-off-by: Patrick Steinhardt <ps@pks.im>\n---\n Documentation/Makefile | 4 ++--\n Makefile               | 2 +-\n shared.mak             | 7 +++++++\n 3 files changed, 10 insertions(+), 3 deletions(-)\n\ndiff --git a/Documentation/Makefile b/Documentation/Makefile\nindex cee88dbda66265141b87d5e5c16bf86df22fa4ef..4c652dfa14f6af2c1374e2f83d3311b36dd297c4 100644\n--- a/Documentation/Makefile\n+++ b/Documentation/Makefile\n@@ -211,10 +211,10 @@ XMLTO_EXTRA += --skip-validation\n XMLTO_EXTRA += -x manpage.xsl\n \n asciidoctor-extensions.rb: asciidoctor-extensions.rb.in FORCE\n-\t$(QUIET_GEN)GIT_USER_AGENT=\"$(GIT_USER_AGENT)\" $(SHELL_PATH) ../GIT-VERSION-GEN \"$(shell pwd)/..\" $< $@\n+\t$(QUIET_GEN)$(SHELL_PATH) ../GIT-VERSION-GEN \"$(shell pwd)/..\" $< $@\n else\n asciidoc.conf: asciidoc.conf.in FORCE\n-\t$(QUIET_GEN)GIT_USER_AGENT=\"$(GIT_USER_AGENT)\" $(SHELL_PATH) ../GIT-VERSION-GEN \"$(shell pwd)/..\" $< $@\n+\t$(QUIET_GEN)$(SHELL_PATH) ../GIT-VERSION-GEN \"$(shell pwd)/..\" $< $@\n endif\n \n ASCIIDOC_DEPS += docinfo.html\ndiff --git a/Makefile b/Makefile\nindex 695a9d9765daf864605002d572129bae7a8c4e40..788f6ee1721850e75f19fe2181f578ad2e783043 100644\n--- a/Makefile\n+++ b/Makefile\n@@ -2512,7 +2512,7 @@ pager.sp pager.s pager.o: EXTRA_CPPFLAGS = \\\n \t-DPAGER_ENV='$(PAGER_ENV_CQ_SQ)'\n \n version-def.h: version-def.h.in GIT-VERSION-GEN GIT-VERSION-FILE GIT-USER-AGENT\n-\t$(QUIET_GEN)GIT_USER_AGENT=\"$(GIT_USER_AGENT)\" $(SHELL_PATH) ./GIT-VERSION-GEN \"$(shell pwd)\" $< $@\n+\t$(QUIET_GEN)$(SHELL_PATH) ./GIT-VERSION-GEN \"$(shell pwd)\" $< $@\n \n version.sp version.s version.o: version-def.h\n \ndiff --git a/shared.mak b/shared.mak\nindex 29bebd30d8acbce9f50661cef48ecdbae1e41f5a..8cd132b5a03b85d4eabc3819eb3d1f64d39afa47 100644\n--- a/shared.mak\n+++ b/shared.mak\n@@ -116,3 +116,10 @@ endef\n define libpath_template\n -L$(1) $(if $(filter-out -L,$(CC_LD_DYNPATH)),$(CC_LD_DYNPATH)$(1))\n endef\n+\n+# Export build information so that variables defined in config.mak can be read\n+# by GIT-VERSION-GEN.\n+export GIT_BUILT_FROM_COMMIT\n+export GIT_DATE\n+export GIT_USER_AGENT\n+export GIT_VERSION\n\n-- \n2.48.0.rc0.184.g0fc57dec57.dirty\n\n"},{"id":"509388","messageId":"20241220-b4-pks-git-version-via-environment-v2-5-f1457a5e8c38@pks.im","threadId":"62670","inReplyTo":"20241220-b4-pks-git-version-via-environment-v2-0-f1457a5e8c38@pks.im","subject":"[PATCH v2 5/5] meson: add options to override build information","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2024-12-20T12:22:49Z","receivedAt":"2024-12-20T12:23:12Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"We inject various different kinds of build information into build\nartifacts, like the version string or the commit from which Git was\nbuilt. Add options to let users explicitly override this information\nwith Meson.\n\nSigned-off-by: Patrick Steinhardt <ps@pks.im>\n---\n Documentation/meson.build |  1 +\n meson.build               | 13 +++++++++++++\n meson_options.txt         | 10 ++++++++++\n 3 files changed, 24 insertions(+)\n\ndiff --git a/Documentation/meson.build b/Documentation/meson.build\nindex f2426ccaa30c29bd60b850eb0a9a4ab77c66a629..fca3eab1f1360a5fdeda89c1766ab8cdb3267b89 100644\n--- a/Documentation/meson.build\n+++ b/Documentation/meson.build\n@@ -219,6 +219,7 @@ asciidoc_conf = custom_target(\n   input: meson.current_source_dir() / 'asciidoc.conf.in',\n   output: 'asciidoc.conf',\n   depends: [git_version_file],\n+  env: version_gen_environment,\n )\n \n asciidoc_common_options = [\ndiff --git a/meson.build b/meson.build\nindex 0dccebcdf16b07650d943e53643f0e09e2975cc9..be32d60e841a3055ed2bdf6fd449a48b66d94cd0 100644\n--- a/meson.build\n+++ b/meson.build\n@@ -201,6 +201,16 @@ if get_option('sane_tool_path') != ''\n   script_environment.prepend('PATH', get_option('sane_tool_path'))\n endif\n \n+# The environment used by GIT-VERSION-GEN. Note that we explicitly override\n+# environment variables that might be set by the user. This is by design so\n+# that we always use whatever Meson has configured instead of what is present\n+# in the environment.\n+version_gen_environment = script_environment\n+version_gen_environment.set('GIT_BUILT_FROM_COMMIT', get_option('built_from_commit'))\n+version_gen_environment.set('GIT_DATE', get_option('build_date'))\n+version_gen_environment.set('GIT_USER_AGENT', get_option('user_agent'))\n+version_gen_environment.set('GIT_VERSION', get_option('version'))\n+\n compiler = meson.get_compiler('c')\n \n libgit_sources = [\n@@ -1485,6 +1495,7 @@ git_version_file = custom_target(\n   ],\n   input: meson.current_source_dir() / 'GIT-VERSION-FILE.in',\n   output: 'GIT-VERSION-FILE',\n+  env: version_gen_environment,\n   build_always_stale: true,\n )\n \n@@ -1501,6 +1512,7 @@ version_def_h = custom_target(\n   # Depend on GIT-VERSION-FILE so that we don't always try to rebuild this\n   # target for the same commit.\n   depends: [git_version_file],\n+  env: version_gen_environment,\n )\n \n # Build a separate library for \"version.c\" so that we do not have to rebuild\n@@ -1544,6 +1556,7 @@ if host_machine.system() == 'windows'\n     input: meson.current_source_dir() / 'git.rc.in',\n     output: 'git.rc',\n     depends: [git_version_file],\n+    env: version_gen_environment,\n   )\n \n   common_main_sources += import('windows').compile_resources(git_rc,\ndiff --git a/meson_options.txt b/meson_options.txt\nindex 32a72139bae870745d9131cc9086a4594826be91..8ead1349550807420bdb95e55298ea4f3f2ea9d0 100644\n--- a/meson_options.txt\n+++ b/meson_options.txt\n@@ -16,6 +16,16 @@ option('runtime_prefix', type: 'boolean', value: false,\n option('sane_tool_path', type: 'string', value: '',\n   description: 'A colon-separated list of paths to prepend to PATH if your tools in /usr/bin are broken.')\n \n+# Build information compiled into Git and other parts like documentation.\n+option('build_date', type: 'string', value: '',\n+  description: 'Build date reported by our documentation.')\n+option('built_from_commit', type: 'string', value: '',\n+  description: 'Commit that Git was built from reported by git-version(1).')\n+option('user_agent', type: 'string', value: '',\n+  description: 'User agent reported to remote servers.')\n+option('version', type: 'string', value: '',\n+  description: 'Version string reported by git-version(1) and other tools.')\n+\n # Features supported by Git.\n option('curl', type: 'feature', value: 'enabled',\n   description: 'Build helpers used to access remotes with the HTTP transport.')\n\n-- \n2.48.0.rc0.184.g0fc57dec57.dirty\n\n"},{"id":"509394","messageId":"xmqqwmfufl9s.fsf@gitster.g","threadId":"62670","inReplyTo":"20241220073720.GB2389154@coredump.intra.peff.net","subject":"Re: [PATCH 2/2] GIT-VERSION-GEN: fix overriding GIT_BUILT_FROM_COMMIT and GIT_DATE","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2024-12-20T15:04:47Z","receivedAt":"2024-12-20T15:04:51Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King <peff@peff.net> writes:\n\n> On Thu, Dec 19, 2024 at 04:53:37PM +0100, Patrick Steinhardt wrote:\n>\n>>  GIT_VERSION=$(expr \"$VN\" : v*'\\(.*\\)')\n>> -GIT_BUILT_FROM_COMMIT=$(git -C \"$SOURCE_DIR\" rev-parse -q --verify HEAD 2>/dev/null)\n>> -GIT_DATE=$(git -C \"$SOURCE_DIR\" show --quiet --format='%as' 2>/dev/null)\n>> +\n>> +if test -z \"$GIT_BUILT_FROM_COMMIT\"\n>> +then\n>> +    GIT_BUILT_FROM_COMMIT=$(git -C \"$SOURCE_DIR\" rev-parse -q --verify HEAD 2>/dev/null)\n>> +fi\n>> +\n>> +if test -z \"$GIT_DATE\"\n>> +then\n>> +    GIT_DATE=$(git -C \"$SOURCE_DIR\" show --quiet --format='%as' 2>/dev/null)\n>> +fi\n>\n> Looks good. I doubt anybody would want to override BUILT_FROM_COMMIT\n> (and it was never possible to do so, even before your recent patches),\n> but it's reasonable to include it as well.\n\nThanks, both.\n"},{"id":"509400","messageId":"20241220155223.GA152570@coredump.intra.peff.net","threadId":"62670","inReplyTo":"20241220-b4-pks-git-version-via-environment-v2-1-f1457a5e8c38@pks.im","subject":"Re: [PATCH v2 1/5] GIT-VERSION-GEN: fix overriding version via environment","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2024-12-20T15:52:23Z","receivedAt":"2024-12-20T15:52:24Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Fri, Dec 20, 2024 at 01:22:45PM +0100, Patrick Steinhardt wrote:\n\n> diff --git a/GIT-VERSION-GEN b/GIT-VERSION-GEN\n> index de0e63bdfbac263884e2ea328cc2ef11ace7a238..27f9d6a81f77248c652649ae21d0ec51b8f2d247 100755\n> --- a/GIT-VERSION-GEN\n> +++ b/GIT-VERSION-GEN\n> @@ -29,7 +29,10 @@ export GIT_CEILING_DIRECTORIES\n>  \n>  # First see if there is a version file (included in release tarballs),\n>  # then try git-describe, then default.\n> -if test -f \"$SOURCE_DIR\"/version\n> +if test -n \"$GIT_VERSION\"\n> +then\n> +    VN=\"$GIT_VERSION\"\n> +elif test -f \"$SOURCE_DIR\"/version\n\nHmm. If $GIT_VERSION is set, then we set $VN here...\n\n> -GIT_VERSION=$(expr \"$VN\" : v*'\\(.*\\)')\n> +# Only strip leading `v` in case we have derived VN manually. Otherwise we\n> +# retain whatever the user has set in their environment.\n> +if test -z \"$GIT_VERSION\"\n> +then\n> +    GIT_VERSION=$(expr \"$VN\" : v*'\\(.*\\)')\n> +fi\n\n...but later we ignore $VN completely.\n\nSo it would work equally well with the first hunk dropped completely.\nHowever, having an entry in the cascading if/else does mean that we\nshort-circuit the effort to run git-describe, etc.\n\nI don't think the old code ever did that (we'd generate the Makefile\nsnippet in GIT-VERSION-FILE, read it back, and then make would still\noverride the value from the snippet).\n\nSo I dunno. I like keeping things simple, but I also like skipping\nunnecessary code, too. Maybe if the top hunk were:\n\n  if test -n \"$GIT_VERSION\"\n  then\n    : do nothing, we will use this value verbatim\n  elif ...\n\nthat would make the intended flow more obvious.\n\nThere are probably other ways to structure it, too. The whole $VN thing\ncould be inside the:\n\n  if test -z \"$GIT_VERSION\"\n\nblock. Or alternatively, if each block of the if/else just ran expr and\nset $GIT_VERSION itself (perhaps with a one-liner helper function) then\nwe wouldn't need $VN at all.\n\nI don't know how much trouble it's worth to refactor all this. Mostly I\nwas just surprised to see the first hunk at all in this version.\n\n-Peff\n"},{"id":"509402","messageId":"20241220155342.GB152570@coredump.intra.peff.net","threadId":"62670","inReplyTo":"20241220-b4-pks-git-version-via-environment-v2-3-f1457a5e8c38@pks.im","subject":"Re: [PATCH v2 3/5] Makefile: drop unneeded indirection for GIT-VERSION-GEN outputs","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2024-12-20T15:53:42Z","receivedAt":"2024-12-20T15:53:44Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Fri, Dec 20, 2024 at 01:22:47PM +0100, Patrick Steinhardt wrote:\n\n> Some of the callsites of GIT-VERSION-GEN generate the target file with a\n> \"+\" suffix first and then move the file into place when the new contents\n> are different compared to the old contents. This allows us to avoid a\n> needless rebuild by not updating timestamps of the target file when its\n> contents will remain unchanged anyway.\n> \n> In fact though, this exact logic is already handled in GIT-VERSION-GEN,\n> so doing this manually is pointless. This is a leftover from an earlier\n> version of 4838deab65 (Makefile: refactor GIT-VERSION-GEN to be\n> reusable, 2024-12-06), where the script didn't handle that logic for us.\n> \n> Drop the needless indirection.\n\nNice. I think when we can do stuff like this in an actual script instead\nof in a Makefile, the result is more readable.\n\n-Peff\n"},{"id":"509403","messageId":"20241220155433.GC152570@coredump.intra.peff.net","threadId":"62670","inReplyTo":"20241220-b4-pks-git-version-via-environment-v2-4-f1457a5e8c38@pks.im","subject":"Re: [PATCH v2 4/5] Makefile: respect build info declared in \"config.mak\"","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2024-12-20T15:54:33Z","receivedAt":"2024-12-20T15:54:35Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Fri, Dec 20, 2024 at 01:22:48PM +0100, Patrick Steinhardt wrote:\n\n> In preceding commits we fixed that build info set via e.g. `make\n> GIT_VERSION=foo` didn't get propagated to GIT-VERSION-GEN. Similarly\n> though, setting build info via \"config.mak\" does not work anymore either\n> because the variables are only declared as Makefile variables and thus\n> aren't accessible by the script.\n> \n> Fix the issue by exporting those variables via \"shared.mak\". This also\n> allows us to deduplicate the export of GIT_USER_AGENT.\n\nThis looks good. It fixes the issue, and I am happy that:\n\n>  asciidoctor-extensions.rb: asciidoctor-extensions.rb.in FORCE\n> -\t$(QUIET_GEN)GIT_USER_AGENT=\"$(GIT_USER_AGENT)\" $(SHELL_PATH) ../GIT-VERSION-GEN \"$(shell pwd)/..\" $< $@\n> +\t$(QUIET_GEN)$(SHELL_PATH) ../GIT-VERSION-GEN \"$(shell pwd)/..\" $< $@\n\n...these spots get even simpler.\n\n-Peff\n"},{"id":"509404","messageId":"20241220155610.GD152570@coredump.intra.peff.net","threadId":"62670","inReplyTo":"20241220-b4-pks-git-version-via-environment-v2-0-f1457a5e8c38@pks.im","subject":"Re: [PATCH v2 0/5] GIT-VERSION-GEN: fix overriding values","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2024-12-20T15:56:10Z","receivedAt":"2024-12-20T15:56:12Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Fri, Dec 20, 2024 at 01:22:44PM +0100, Patrick Steinhardt wrote:\n\n> Changes in v2:\n> \n>   - Don't strip leading `v`s when `GIT_VERSION` was set explicitly.\n>   - Allow setting build info via \"config.mak\" again.\n>   - Wire up build info options for Meson.\n>   - Link to v1: https://lore.kernel.org/r/20241219-b4-pks-git-version-via-environment-v1-0-9393af058240@pks.im\n\nThanks, I confirmed that this fixes the doc-diff issue, and setting\nvalues in config.mak works.\n\nI left some small comments on patch 1. Patches 2-4 look good to me, and\nI'm not qualified to comment on meson patches. ;)\n\n-Peff\n"},{"id":"509407","messageId":"xmqqpllme3cl.fsf@gitster.g","threadId":"62670","inReplyTo":"20241220155223.GA152570@coredump.intra.peff.net","subject":"Re: [PATCH v2 1/5] GIT-VERSION-GEN: fix overriding version via environment","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2024-12-20T16:17:14Z","receivedAt":"2024-12-20T16:17:17Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King <peff@peff.net> writes:\n\n> So I dunno. I like keeping things simple, but I also like skipping\n> unnecessary code, too. Maybe if the top hunk were:\n>\n>   if test -n \"$GIT_VERSION\"\n>   then\n>     : do nothing, we will use this value verbatim\n>   elif ...\n>\n> that would make the intended flow more obvious.\n\nTrue.\n\n> There are probably other ways to structure it, too. The whole $VN thing\n> could be inside the:\n>\n>   if test -z \"$GIT_VERSION\"\n>\n> block. Or alternatively, if each block of the if/else just ran expr and\n> set $GIT_VERSION itself (perhaps with a one-liner helper function) then\n> we wouldn't need $VN at all.\n\nTrue again.  It has been quite a while since I wrote the original\nbefore the meson topic came up and the script hasn't changed for a\nlong time (other than DEF_VER line for obvious reasons), but I think\nin that ancient version, $VN _was_ the variable to be looked at and\nGIT_VERSION did not even exist as a shell variable at all.\n\nIf $GIT_VERSION is serving the same role as old $VN in the\nmesonified version, perhaps we should get rid of the $VN variable to\nclarify the new world order.\n\n> I don't know how much trouble it's worth to refactor all this. Mostly I\n> was just surprised to see the first hunk at all in this version.\n>\n> -Peff\n"},{"id":"509408","messageId":"Z2WYajwW-Isxmzwz@pks.im","threadId":"62670","inReplyTo":"20241220155223.GA152570@coredump.intra.peff.net","subject":"Re: [PATCH v2 1/5] GIT-VERSION-GEN: fix overriding version via environment","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2024-12-20T16:16:42Z","receivedAt":"2024-12-20T16:18:24Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"On Fri, Dec 20, 2024 at 10:52:23AM -0500, Jeff King wrote:\n> On Fri, Dec 20, 2024 at 01:22:45PM +0100, Patrick Steinhardt wrote:\n> \n> > diff --git a/GIT-VERSION-GEN b/GIT-VERSION-GEN\n> > index de0e63bdfbac263884e2ea328cc2ef11ace7a238..27f9d6a81f77248c652649ae21d0ec51b8f2d247 100755\n> > --- a/GIT-VERSION-GEN\n> > +++ b/GIT-VERSION-GEN\n> > @@ -29,7 +29,10 @@ export GIT_CEILING_DIRECTORIES\n> >  \n> >  # First see if there is a version file (included in release tarballs),\n> >  # then try git-describe, then default.\n> > -if test -f \"$SOURCE_DIR\"/version\n> > +if test -n \"$GIT_VERSION\"\n> > +then\n> > +    VN=\"$GIT_VERSION\"\n> > +elif test -f \"$SOURCE_DIR\"/version\n> \n> Hmm. If $GIT_VERSION is set, then we set $VN here...\n> \n> > -GIT_VERSION=$(expr \"$VN\" : v*'\\(.*\\)')\n> > +# Only strip leading `v` in case we have derived VN manually. Otherwise we\n> > +# retain whatever the user has set in their environment.\n> > +if test -z \"$GIT_VERSION\"\n> > +then\n> > +    GIT_VERSION=$(expr \"$VN\" : v*'\\(.*\\)')\n> > +fi\n> \n> ...but later we ignore $VN completely.\n> \n> So it would work equally well with the first hunk dropped completely.\n> However, having an entry in the cascading if/else does mean that we\n> short-circuit the effort to run git-describe, etc.\n> \n> I don't think the old code ever did that (we'd generate the Makefile\n> snippet in GIT-VERSION-FILE, read it back, and then make would still\n> override the value from the snippet).\n> \n> So I dunno. I like keeping things simple, but I also like skipping\n> unnecessary code, too. Maybe if the top hunk were:\n> \n>   if test -n \"$GIT_VERSION\"\n>   then\n>     : do nothing, we will use this value verbatim\n>   elif ...\n> \n> that would make the intended flow more obvious.\n> \n> There are probably other ways to structure it, too. The whole $VN thing\n> could be inside the:\n> \n>   if test -z \"$GIT_VERSION\"\n> \n> block. Or alternatively, if each block of the if/else just ran expr and\n> set $GIT_VERSION itself (perhaps with a one-liner helper function) then\n> we wouldn't need $VN at all.\n> \n> I don't know how much trouble it's worth to refactor all this. Mostly I\n> was just surprised to see the first hunk at all in this version.\n\nI think wrapping it all in `if test -z \"$GIT_VERSION\"` would be best. I\nwas thinking about whether to do that refactoring, but I shied away from\nit due to the required reindentation. But I agree that the end result is\na bit on the awkward side.\n\nYou know, let me send another iteration where I just do it, now that\nwe're two having the same thought.\n\nPatrick\n"},{"id":"509417","messageId":"Z2WZ_6auUm0mVIqr@pks.im","threadId":"62670","inReplyTo":"xmqqpllme3cl.fsf@gitster.g","subject":"Re: [PATCH v2 1/5] GIT-VERSION-GEN: fix overriding version via environment","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2024-12-20T16:23:27Z","receivedAt":"2024-12-20T16:25:10Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"On Fri, Dec 20, 2024 at 08:17:14AM -0800, Junio C Hamano wrote:\n> Jeff King <peff@peff.net> writes:\n> \n> > So I dunno. I like keeping things simple, but I also like skipping\n> > unnecessary code, too. Maybe if the top hunk were:\n> >\n> >   if test -n \"$GIT_VERSION\"\n> >   then\n> >     : do nothing, we will use this value verbatim\n> >   elif ...\n> >\n> > that would make the intended flow more obvious.\n> \n> True.\n> \n> > There are probably other ways to structure it, too. The whole $VN thing\n> > could be inside the:\n> >\n> >   if test -z \"$GIT_VERSION\"\n> >\n> > block. Or alternatively, if each block of the if/else just ran expr and\n> > set $GIT_VERSION itself (perhaps with a one-liner helper function) then\n> > we wouldn't need $VN at all.\n> \n> True again.  It has been quite a while since I wrote the original\n> before the meson topic came up and the script hasn't changed for a\n> long time (other than DEF_VER line for obvious reasons), but I think\n> in that ancient version, $VN _was_ the variable to be looked at and\n> GIT_VERSION did not even exist as a shell variable at all.\n> \n> If $GIT_VERSION is serving the same role as old $VN in the\n> mesonified version, perhaps we should get rid of the $VN variable to\n> clarify the new world order.\n\nI think for now I'll keep $VN, if even just to discern the intermediate\nvalue without the leading \"v\" stripped from the final version that we\nhave in \"$GIT_VERSION\". But I'll send a revised version where I make the\ncontrol flow a bit more obvious.\n\nThanks!\n\nPatrick\n"},{"id":"509425","messageId":"Z2WfirfrpYYFgYdw@pks.im","threadId":"62670","inReplyTo":"20241220155433.GC152570@coredump.intra.peff.net","subject":"Re: [PATCH v2 4/5] Makefile: respect build info declared in \"config.mak\"","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2024-12-20T16:47:14Z","receivedAt":"2024-12-20T16:56:56Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"On Fri, Dec 20, 2024 at 10:54:33AM -0500, Jeff King wrote:\n> On Fri, Dec 20, 2024 at 01:22:48PM +0100, Patrick Steinhardt wrote:\n> \n> > In preceding commits we fixed that build info set via e.g. `make\n> > GIT_VERSION=foo` didn't get propagated to GIT-VERSION-GEN. Similarly\n> > though, setting build info via \"config.mak\" does not work anymore either\n> > because the variables are only declared as Makefile variables and thus\n> > aren't accessible by the script.\n> > \n> > Fix the issue by exporting those variables via \"shared.mak\". This also\n> > allows us to deduplicate the export of GIT_USER_AGENT.\n> \n> This looks good. It fixes the issue, and I am happy that:\n> \n> >  asciidoctor-extensions.rb: asciidoctor-extensions.rb.in FORCE\n> > -\t$(QUIET_GEN)GIT_USER_AGENT=\"$(GIT_USER_AGENT)\" $(SHELL_PATH) ../GIT-VERSION-GEN \"$(shell pwd)/..\" $< $@\n> > +\t$(QUIET_GEN)$(SHELL_PATH) ../GIT-VERSION-GEN \"$(shell pwd)/..\" $< $@\n> \n> ...these spots get even simpler.\n\nMeh. I just noticed that this doesn't work: we include GIT-VERSION-FILE\nand export its value, and consequently any subsequent invocation of\nGIT-VERSION-GEN will continue to use the value that we have in\nGIT-VERSION-FILE. So it's effectively only computed the first time.\n\nThis would all be much simpler if we didn't include the file in the\nfirst place. In Documentation/Makefile we don't indeed use it anymore.\nBut in the top-level Makefile we do use it to generate the name of a\ncouple of archives. I'll have a look there.\n\nPatrick\n"},{"id":"509436","messageId":"20241220175136.GA203033@coredump.intra.peff.net","threadId":"62670","inReplyTo":"Z2WfirfrpYYFgYdw@pks.im","subject":"Re: [PATCH v2 4/5] Makefile: respect build info declared in \"config.mak\"","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2024-12-20T17:51:36Z","receivedAt":"2024-12-20T17:51:39Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Fri, Dec 20, 2024 at 05:47:14PM +0100, Patrick Steinhardt wrote:\n\n> > This looks good. It fixes the issue, and I am happy that:\n> > \n> > >  asciidoctor-extensions.rb: asciidoctor-extensions.rb.in FORCE\n> > > -\t$(QUIET_GEN)GIT_USER_AGENT=\"$(GIT_USER_AGENT)\" $(SHELL_PATH) ../GIT-VERSION-GEN \"$(shell pwd)/..\" $< $@\n> > > +\t$(QUIET_GEN)$(SHELL_PATH) ../GIT-VERSION-GEN \"$(shell pwd)/..\" $< $@\n> > \n> > ...these spots get even simpler.\n> \n> Meh. I just noticed that this doesn't work: we include GIT-VERSION-FILE\n> and export its value, and consequently any subsequent invocation of\n> GIT-VERSION-GEN will continue to use the value that we have in\n> GIT-VERSION-FILE. So it's effectively only computed the first time.\n\nI'm not sure what you mean.\n\nI wondered earlier if we might into a chicken-and-egg problem like that,\nbut I tested and it seemed to work fine. The rule for GIT-VERSION-FILE\nmeans we'll build it before make reads it, so that first run of it will\nget the updated value. And:\n\n  make GIT_VERSION=foo && bin-wrappers/git version\n  make GIT_VERSION=bar && bin-wrappers/git version\n\ndoes what you'd expect. And the docs work the same way:\n\n  cd Documentation\n  make GIT_VERSION=foo git.1 && man -l git.1\n  make GIT_VERSION=bar git.1 && man -l git.1\n\nIs there a case you found that doesn't work?\n\n-Peff\n"},{"id":"509438","messageId":"Z2WxIRcV0LOvx6OX@pks.im","threadId":"62670","inReplyTo":"20241220175136.GA203033@coredump.intra.peff.net","subject":"Re: [PATCH v2 4/5] Makefile: respect build info declared in \"config.mak\"","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2024-12-20T18:02:09Z","receivedAt":"2024-12-20T18:02:32Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"On Fri, Dec 20, 2024 at 12:51:36PM -0500, Jeff King wrote:\n> On Fri, Dec 20, 2024 at 05:47:14PM +0100, Patrick Steinhardt wrote:\n> \n> > > This looks good. It fixes the issue, and I am happy that:\n> > > \n> > > >  asciidoctor-extensions.rb: asciidoctor-extensions.rb.in FORCE\n> > > > -\t$(QUIET_GEN)GIT_USER_AGENT=\"$(GIT_USER_AGENT)\" $(SHELL_PATH) ../GIT-VERSION-GEN \"$(shell pwd)/..\" $< $@\n> > > > +\t$(QUIET_GEN)$(SHELL_PATH) ../GIT-VERSION-GEN \"$(shell pwd)/..\" $< $@\n> > > \n> > > ...these spots get even simpler.\n> > \n> > Meh. I just noticed that this doesn't work: we include GIT-VERSION-FILE\n> > and export its value, and consequently any subsequent invocation of\n> > GIT-VERSION-GEN will continue to use the value that we have in\n> > GIT-VERSION-FILE. So it's effectively only computed the first time.\n> \n> I'm not sure what you mean.\n> \n> I wondered earlier if we might into a chicken-and-egg problem like that,\n> but I tested and it seemed to work fine. The rule for GIT-VERSION-FILE\n> means we'll build it before make reads it, so that first run of it will\n> get the updated value. And:\n> \n>   make GIT_VERSION=foo && bin-wrappers/git version\n>   make GIT_VERSION=bar && bin-wrappers/git version\n> \n> does what you'd expect. And the docs work the same way:\n> \n>   cd Documentation\n>   make GIT_VERSION=foo git.1 && man -l git.1\n>   make GIT_VERSION=bar git.1 && man -l git.1\n> \n> Is there a case you found that doesn't work?\n\nYes:\n\n    $ make GIT-VERSION-FILE GIT_VERSION=foo\n    GIT_VERSION=foo\n    make: 'GIT-VERSION-FILE' is up to date.\n    $ cat GIT-VERSION-FILE\n    GIT_VERSION=foo\n\n    # And now run without GIT_VERSION set.\n    make: 'GIT-VERSION-FILE' is up to date.\n    GIT_VERSION=foo\n\nSo the value remains \"sticky\" in this case. And that is true whenever\nyou don't set GIT_VERSION at all, we always stick with what is currently\nin that file.\n\nI've got a version now that works for all cases, but I had to use\nMakefile templates and some shuffling to make it work.\n\nPatrick\n"},{"id":"509439","messageId":"Z2W06zRsQei5E34D@pks.im","threadId":"62670","inReplyTo":"Z2WxIRcV0LOvx6OX@pks.im","subject":"Re: [PATCH v2 4/5] Makefile: respect build info declared in \"config.mak\"","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2024-12-20T18:18:19Z","receivedAt":"2024-12-20T18:18:40Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"On Fri, Dec 20, 2024 at 07:02:10PM +0100, Patrick Steinhardt wrote:\n> On Fri, Dec 20, 2024 at 12:51:36PM -0500, Jeff King wrote:\n> > On Fri, Dec 20, 2024 at 05:47:14PM +0100, Patrick Steinhardt wrote:\n> > \n> > > > This looks good. It fixes the issue, and I am happy that:\n> > > > \n> > > > >  asciidoctor-extensions.rb: asciidoctor-extensions.rb.in FORCE\n> > > > > -\t$(QUIET_GEN)GIT_USER_AGENT=\"$(GIT_USER_AGENT)\" $(SHELL_PATH) ../GIT-VERSION-GEN \"$(shell pwd)/..\" $< $@\n> > > > > +\t$(QUIET_GEN)$(SHELL_PATH) ../GIT-VERSION-GEN \"$(shell pwd)/..\" $< $@\n> > > > \n> > > > ...these spots get even simpler.\n> > > \n> > > Meh. I just noticed that this doesn't work: we include GIT-VERSION-FILE\n> > > and export its value, and consequently any subsequent invocation of\n> > > GIT-VERSION-GEN will continue to use the value that we have in\n> > > GIT-VERSION-FILE. So it's effectively only computed the first time.\n> > \n> > I'm not sure what you mean.\n> > \n> > I wondered earlier if we might into a chicken-and-egg problem like that,\n> > but I tested and it seemed to work fine. The rule for GIT-VERSION-FILE\n> > means we'll build it before make reads it, so that first run of it will\n> > get the updated value. And:\n> > \n> >   make GIT_VERSION=foo && bin-wrappers/git version\n> >   make GIT_VERSION=bar && bin-wrappers/git version\n> > \n> > does what you'd expect. And the docs work the same way:\n> > \n> >   cd Documentation\n> >   make GIT_VERSION=foo git.1 && man -l git.1\n> >   make GIT_VERSION=bar git.1 && man -l git.1\n> > \n> > Is there a case you found that doesn't work?\n> \n> Yes:\n> \n>     $ make GIT-VERSION-FILE GIT_VERSION=foo\n>     GIT_VERSION=foo\n>     make: 'GIT-VERSION-FILE' is up to date.\n>     $ cat GIT-VERSION-FILE\n>     GIT_VERSION=foo\n> \n>     # And now run without GIT_VERSION set.\n>     make: 'GIT-VERSION-FILE' is up to date.\n>     GIT_VERSION=foo\n> \n> So the value remains \"sticky\" in this case. And that is true whenever\n> you don't set GIT_VERSION at all, we always stick with what is currently\n> in that file.\n> \n> I've got a version now that works for all cases, but I had to use\n> Makefile templates and some shuffling to make it work.\n\nThe root of the problem is this [1]:\n\n    To this end, after reading in all makefiles make will consider each\n    as a goal target, in the order in which they were processed, and\n    attempt to update it. If parallel builds (see Parallel Execution)\n    are enabled then makefiles will be rebuilt in parallel as well.\n\nSo the Makefile will _first_ read in all includes before deciding\nwhether or not it needs to regenerate any of them. So we already export\nthe current GIT_VERSION in case \"GIT-VERSION-FILE\" exists at the time\nwhere we run \"GIT-VERSION-GEN\".\n\nThat behaviour is quite surprising to me, but it seems to work as\ndesigned.\n\nPatrick\n\n[1]: https://www.gnu.org/software/make/manual/html_node/Remaking-Makefiles.html\n"},{"id":"509440","messageId":"20241220182427.GA213015@coredump.intra.peff.net","threadId":"62670","inReplyTo":"Z2WxIRcV0LOvx6OX@pks.im","subject":"Re: [PATCH v2 4/5] Makefile: respect build info declared in \"config.mak\"","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2024-12-20T18:24:27Z","receivedAt":"2024-12-20T18:24:28Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Fri, Dec 20, 2024 at 07:02:09PM +0100, Patrick Steinhardt wrote:\n\n> > Is there a case you found that doesn't work?\n> \n> Yes:\n> \n>     $ make GIT-VERSION-FILE GIT_VERSION=foo\n>     GIT_VERSION=foo\n>     make: 'GIT-VERSION-FILE' is up to date.\n>     $ cat GIT-VERSION-FILE\n>     GIT_VERSION=foo\n> \n>     # And now run without GIT_VERSION set.\n>     make: 'GIT-VERSION-FILE' is up to date.\n>     GIT_VERSION=foo\n> \n> So the value remains \"sticky\" in this case. And that is true whenever\n> you don't set GIT_VERSION at all, we always stick with what is currently\n> in that file.\n\nAh, right. Even though we have a recipe to build it, and make knows it\nmust be built (because it depends on FORCE), make will read it (and all\nincludes) first before executing any rules.\n\nSomething like this seems to work:\n\ndiff --git a/Makefile b/Makefile\nindex 788f6ee172..0eb08d98f4 100644\n--- a/Makefile\n+++ b/Makefile\n@@ -596,7 +596,12 @@ GIT-VERSION-FILE: FORCE\n \t$(SHELL_PATH) ./GIT-VERSION-GEN \"$(shell pwd)\" GIT-VERSION-FILE.in $@ && \\\n \tNEW=$$(cat $@ 2>/dev/null || :) && \\\n \tif test \"$$OLD\" != \"$$NEW\"; then echo \"$$NEW\" >&2; fi\n+# Never include it on the first read-through, only after make has tried to\n+# refresh includes. We do not want the old values to pollute our new run of the\n+# rule above.\n+ifdef MAKE_RESTARTS\n -include GIT-VERSION-FILE\n+endif\n \n # Set our default configuration.\n #\n\n\nBut I don't know if there are any gotchas (I did not even know about\nMAKE_RESTARTS until digging in the docs looking for a solution here).\nIf we can stop including it as a Makefile snippet entirely, I think that\nis easier to reason about.\n\n-Peff\n"},{"id":"509441","messageId":"Z2W56ux3mLnfJ43Q@pks.im","threadId":"62670","inReplyTo":"20241220182427.GA213015@coredump.intra.peff.net","subject":"Re: [PATCH v2 4/5] Makefile: respect build info declared in \"config.mak\"","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2024-12-20T18:39:38Z","receivedAt":"2024-12-20T18:39:58Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"On Fri, Dec 20, 2024 at 01:24:27PM -0500, Jeff King wrote:\n> On Fri, Dec 20, 2024 at 07:02:09PM +0100, Patrick Steinhardt wrote:\n> \n> > > Is there a case you found that doesn't work?\n> > \n> > Yes:\n> > \n> >     $ make GIT-VERSION-FILE GIT_VERSION=foo\n> >     GIT_VERSION=foo\n> >     make: 'GIT-VERSION-FILE' is up to date.\n> >     $ cat GIT-VERSION-FILE\n> >     GIT_VERSION=foo\n> > \n> >     # And now run without GIT_VERSION set.\n> >     make: 'GIT-VERSION-FILE' is up to date.\n> >     GIT_VERSION=foo\n> > \n> > So the value remains \"sticky\" in this case. And that is true whenever\n> > you don't set GIT_VERSION at all, we always stick with what is currently\n> > in that file.\n> \n> Ah, right. Even though we have a recipe to build it, and make knows it\n> must be built (because it depends on FORCE), make will read it (and all\n> includes) first before executing any rules.\n> \n> Something like this seems to work:\n> \n> diff --git a/Makefile b/Makefile\n> index 788f6ee172..0eb08d98f4 100644\n> --- a/Makefile\n> +++ b/Makefile\n> @@ -596,7 +596,12 @@ GIT-VERSION-FILE: FORCE\n>  \t$(SHELL_PATH) ./GIT-VERSION-GEN \"$(shell pwd)\" GIT-VERSION-FILE.in $@ && \\\n>  \tNEW=$$(cat $@ 2>/dev/null || :) && \\\n>  \tif test \"$$OLD\" != \"$$NEW\"; then echo \"$$NEW\" >&2; fi\n> +# Never include it on the first read-through, only after make has tried to\n> +# refresh includes. We do not want the old values to pollute our new run of the\n> +# rule above.\n> +ifdef MAKE_RESTARTS\n>  -include GIT-VERSION-FILE\n> +endif\n>  \n>  # Set our default configuration.\n>  #\n\nOh, nifty! Playing around with it indeed seems to make things work, and\nit's simpler than what I have.\n\n> But I don't know if there are any gotchas (I did not even know about\n> MAKE_RESTARTS until digging in the docs looking for a solution here).\n\nGood question indeed. I was wondering whether Make restarts at all in\ncase where none of the included Makefiles change. But it very much seems\nlike it does.\n\nThe next question is since when the option has been available, as it's\nquite, and the answer is that it has been introduced via 978819e1 (Add a\nnew variable: MAKE_RESTARTS, to count how many times make has re-exec'd.\nWhen rebuilding makefiles, unset -B if MAKE_RESTARTS is >0.,\n2005-06-25), which is Make v3.81. Even macOS has that to the best of my\nknowledge.\n\nIt still does feel somewhat hacky in the end.\n\n> If we can stop including it as a Makefile snippet entirely, I think that\n> is easier to reason about.\n\nI very much agree, but it's a non-trivial change. I'll leave that for a\nfuture iteration.\n\nI'm a bit torn now. I have a solution locally that feels less hacky, but\nit requires a bit more shuffling. If the eventual goal would be to get\nrid of the include in the first place it feels somewhat pointless to do\nthese changes.\n\nPatrick\n"},{"id":"509442","messageId":"Z2XAiDC4pJ9OjTpC@pks.im","threadId":"62670","inReplyTo":"Z2W56ux3mLnfJ43Q@pks.im","subject":"Re: [PATCH v2 4/5] Makefile: respect build info declared in \"config.mak\"","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2024-12-20T19:07:52Z","receivedAt":"2024-12-20T19:08:13Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"On Fri, Dec 20, 2024 at 07:39:38PM +0100, Patrick Steinhardt wrote:\n> On Fri, Dec 20, 2024 at 01:24:27PM -0500, Jeff King wrote:\n> > On Fri, Dec 20, 2024 at 07:02:09PM +0100, Patrick Steinhardt wrote:\n> > \n> > > > Is there a case you found that doesn't work?\n> > > \n> > > Yes:\n> > > \n> > >     $ make GIT-VERSION-FILE GIT_VERSION=foo\n> > >     GIT_VERSION=foo\n> > >     make: 'GIT-VERSION-FILE' is up to date.\n> > >     $ cat GIT-VERSION-FILE\n> > >     GIT_VERSION=foo\n> > > \n> > >     # And now run without GIT_VERSION set.\n> > >     make: 'GIT-VERSION-FILE' is up to date.\n> > >     GIT_VERSION=foo\n> > > \n> > > So the value remains \"sticky\" in this case. And that is true whenever\n> > > you don't set GIT_VERSION at all, we always stick with what is currently\n> > > in that file.\n> > \n> > Ah, right. Even though we have a recipe to build it, and make knows it\n> > must be built (because it depends on FORCE), make will read it (and all\n> > includes) first before executing any rules.\n> > \n> > Something like this seems to work:\n> > \n> > diff --git a/Makefile b/Makefile\n> > index 788f6ee172..0eb08d98f4 100644\n> > --- a/Makefile\n> > +++ b/Makefile\n> > @@ -596,7 +596,12 @@ GIT-VERSION-FILE: FORCE\n> >  \t$(SHELL_PATH) ./GIT-VERSION-GEN \"$(shell pwd)\" GIT-VERSION-FILE.in $@ && \\\n> >  \tNEW=$$(cat $@ 2>/dev/null || :) && \\\n> >  \tif test \"$$OLD\" != \"$$NEW\"; then echo \"$$NEW\" >&2; fi\n> > +# Never include it on the first read-through, only after make has tried to\n> > +# refresh includes. We do not want the old values to pollute our new run of the\n> > +# rule above.\n> > +ifdef MAKE_RESTARTS\n> >  -include GIT-VERSION-FILE\n> > +endif\n> >  \n> >  # Set our default configuration.\n> >  #\n> \n> Oh, nifty! Playing around with it indeed seems to make things work, and\n> it's simpler than what I have.\n> \n> > But I don't know if there are any gotchas (I did not even know about\n> > MAKE_RESTARTS until digging in the docs looking for a solution here).\n> \n> Good question indeed. I was wondering whether Make restarts at all in\n> case where none of the included Makefiles change. But it very much seems\n> like it does.\n> \n> The next question is since when the option has been available, as it's\n> quite, and the answer is that it has been introduced via 978819e1 (Add a\n> new variable: MAKE_RESTARTS, to count how many times make has re-exec'd.\n> When rebuilding makefiles, unset -B if MAKE_RESTARTS is >0.,\n> 2005-06-25), which is Make v3.81. Even macOS has that to the best of my\n> knowledge.\n> \n> It still does feel somewhat hacky in the end.\n> \n> > If we can stop including it as a Makefile snippet entirely, I think that\n> > is easier to reason about.\n> \n> I very much agree, but it's a non-trivial change. I'll leave that for a\n> future iteration.\n> \n> I'm a bit torn now. I have a solution locally that feels less hacky, but\n> it requires a bit more shuffling. If the eventual goal would be to get\n> rid of the include in the first place it feels somewhat pointless to do\n> these changes.\n\nOkay, I did find an issue where it does not work:\n\n    $ git clean -dfx\n    $ make GIT-USER-AGENT\n    $ cat GIT-USER-AGENT\n    git/\n    $ cat GIT-VERSION-FILE\n    cat: GIT-VERSION-FILE: No such file or directory\n\nIt does not generate the version file at all anymore when it's not an\nexplicit dependency. While I could of course add the missing dependency\nI don't know whether there are any other implicit dependencies that\nwould be broken, as well. My gut feeling says \"probably\".\n\nI'll go with my version instead.\n\nPatrick\n"},{"id":"509445","messageId":"20241220-b4-pks-git-version-via-environment-v3-0-1fd79b52a5fb@pks.im","threadId":"62670","inReplyTo":"20241219-b4-pks-git-version-via-environment-v1-0-9393af058240@pks.im","subject":"[PATCH v3 0/6] GIT-VERSION-GEN: fix overriding values","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2024-12-20T19:44:20Z","receivedAt":"2024-12-20T19:44:46Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"Hi,\n\nPeff reported that overriding GIT_VERSION and GIT_DATE broke recently\ndue to the refactoring of GIT-VERSION-GEN. This small commit series\nfixes those cases, but also fixes the equivalent issue with\nGIT_BUILT_FROM_COMMIT.\n\nChanges in v2:\n\n  - Don't strip leading `v`s when `GIT_VERSION` was set explicitly.\n  - Allow setting build info via \"config.mak\" again.\n  - Wire up build info options for Meson.\n  - Link to v1: https://lore.kernel.org/r/20241219-b4-pks-git-version-via-environment-v1-0-9393af058240@pks.im\n\nChanges in v3:\n  - Redo things such that we use Makefile templates instead and a\n    separate GIT_VERSION_OVERRIDE variable. This requires a bit of\n    shuffling, but fixes a problem where the version becomes \"stuck\" due\n    to how Makefiles handle includes.\n  - Add another patch to stop including GIT-VERSION-FILE in\n    Documentation/Makefile.\n  - Improve code flow in GIT-VERSION-GEN.\n  - Link to v2: https://lore.kernel.org/r/20241220-b4-pks-git-version-via-environment-v2-0-f1457a5e8c38@pks.im\n\nThanks!\n\nPatrick\n\n---\nPatrick Steinhardt (6):\n      Makefile: stop including \"GIT-VERSION-FILE\" in docs\n      Makefile: drop unneeded indirection for GIT-VERSION-GEN outputs\n      Makefile: introduce template for GIT-VERSION-GEN\n      GIT-VERSION-GEN: fix overriding GIT_VERSION\n      GIT-VERSION-GEN: fix overriding GIT_BUILT_FROM_COMMIT and GIT_DATE\n      meson: add options to override build information\n\n Documentation/Makefile    | 17 +++++---------\n Documentation/meson.build |  1 +\n GIT-VERSION-GEN           | 58 ++++++++++++++++++++++++++++-------------------\n Makefile                  | 25 +++++++++++---------\n meson.build               | 13 +++++++++++\n meson_options.txt         | 10 ++++++++\n shared.mak                | 11 +++++++++\n 7 files changed, 90 insertions(+), 45 deletions(-)\n\nRange-diff versus v2:\n\n1:  16c647d30b < -:  ---------- GIT-VERSION-GEN: fix overriding version via environment\n2:  819154dc77 < -:  ---------- GIT-VERSION-GEN: fix overriding GIT_BUILT_FROM_COMMIT and GIT_DATE\n-:  ---------- > 1:  dadeded025 Makefile: stop including \"GIT-VERSION-FILE\" in docs\n3:  b658a5a42f = 2:  e5a4fff080 Makefile: drop unneeded indirection for GIT-VERSION-GEN outputs\n4:  79b407d6a2 < -:  ---------- Makefile: respect build info declared in \"config.mak\"\n-:  ---------- > 3:  c271f33aeb Makefile: introduce template for GIT-VERSION-GEN\n-:  ---------- > 4:  c4f2c8aa17 GIT-VERSION-GEN: fix overriding GIT_VERSION\n-:  ---------- > 5:  5d231080e0 GIT-VERSION-GEN: fix overriding GIT_BUILT_FROM_COMMIT and GIT_DATE\n5:  26feb8bc81 = 6:  53bcb815c4 meson: add options to override build information\n\n---\nbase-commit: d882f382b3d939d90cfa58d17b17802338f05d66\nchange-id: 20241219-b4-pks-git-version-via-environment-035490abec26\n\n"},{"id":"509446","messageId":"20241220-b4-pks-git-version-via-environment-v3-1-1fd79b52a5fb@pks.im","threadId":"62670","inReplyTo":"20241220-b4-pks-git-version-via-environment-v3-0-1fd79b52a5fb@pks.im","subject":"[PATCH v3 1/6] Makefile: stop including \"GIT-VERSION-FILE\" in docs","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2024-12-20T19:44:21Z","receivedAt":"2024-12-20T19:44:47Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"We include \"GIT-VERSION-FILE\" in our docs Makefile, but don't actually\nuse the \"GIT_VERSION\" variable that it provides. This is a leftover from\nthe conversion to make \"GIT-VERSION-GEN\" generate version information\nin-place by substituting placeholders in 4838deab65 (Makefile: refactor\nGIT-VERSION-GEN to be reusable, 2024-12-06) and subsequent commits,\nwhere all usages of the variable were removed.\n\nStop including the file.\n\nSigned-off-by: Patrick Steinhardt <ps@pks.im>\n---\n Documentation/Makefile | 7 -------\n 1 file changed, 7 deletions(-)\n\ndiff --git a/Documentation/Makefile b/Documentation/Makefile\nindex 3392e1ce7ebc540784912476847380d9c1775ac8..44c9e9369a11a6a5091079b7221a085b2f08e6cd 100644\n--- a/Documentation/Makefile\n+++ b/Documentation/Makefile\n@@ -276,13 +276,6 @@ install-pdf: pdf\n install-html: html\n \t'$(SHELL_PATH_SQ)' ./install-webdoc.sh $(DESTDIR)$(htmldir)\n \n-../GIT-VERSION-FILE: FORCE\n-\t$(QUIET_SUBDIR0)../ $(QUIET_SUBDIR1) GIT-VERSION-FILE\n-\n-ifneq ($(filter-out lint-docs clean,$(MAKECMDGOALS)),)\n--include ../GIT-VERSION-FILE\n-endif\n-\n mergetools_txt = mergetools-diff.txt mergetools-merge.txt\n \n #\n\n-- \n2.48.0.rc0.184.g0fc57dec57.dirty\n\n"},{"id":"509447","messageId":"20241220-b4-pks-git-version-via-environment-v3-2-1fd79b52a5fb@pks.im","threadId":"62670","inReplyTo":"20241220-b4-pks-git-version-via-environment-v3-0-1fd79b52a5fb@pks.im","subject":"[PATCH v3 2/6] Makefile: drop unneeded indirection for GIT-VERSION-GEN outputs","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2024-12-20T19:44:22Z","receivedAt":"2024-12-20T19:44:48Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"Some of the callsites of GIT-VERSION-GEN generate the target file with a\n\"+\" suffix first and then move the file into place when the new contents\nare different compared to the old contents. This allows us to avoid a\nneedless rebuild by not updating timestamps of the target file when its\ncontents will remain unchanged anyway.\n\nIn fact though, this exact logic is already handled in GIT-VERSION-GEN,\nso doing this manually is pointless. This is a leftover from an earlier\nversion of 4838deab65 (Makefile: refactor GIT-VERSION-GEN to be\nreusable, 2024-12-06), where the script didn't handle that logic for us.\n\nDrop the needless indirection.\n\nSigned-off-by: Patrick Steinhardt <ps@pks.im>\n---\n Documentation/Makefile | 6 ++----\n Makefile               | 6 ++----\n 2 files changed, 4 insertions(+), 8 deletions(-)\n\ndiff --git a/Documentation/Makefile b/Documentation/Makefile\nindex 44c9e9369a11a6a5091079b7221a085b2f08e6cd..1a398d0dc359671d461fceb7a1636268a51411da 100644\n--- a/Documentation/Makefile\n+++ b/Documentation/Makefile\n@@ -211,12 +211,10 @@ XMLTO_EXTRA += --skip-validation\n XMLTO_EXTRA += -x manpage.xsl\n \n asciidoctor-extensions.rb: asciidoctor-extensions.rb.in FORCE\n-\t$(QUIET_GEN)GIT_USER_AGENT=\"$(GIT_USER_AGENT)\" $(SHELL_PATH) ../GIT-VERSION-GEN \"$(shell pwd)/..\" $< $@+\n-\t@if cmp $@+ $@ >/dev/null 2>&1; then $(RM) $@+; else mv $@+ $@; fi\n+\t$(QUIET_GEN)GIT_USER_AGENT=\"$(GIT_USER_AGENT)\" $(SHELL_PATH) ../GIT-VERSION-GEN \"$(shell pwd)/..\" $< $@\n else\n asciidoc.conf: asciidoc.conf.in FORCE\n-\t$(QUIET_GEN)GIT_USER_AGENT=\"$(GIT_USER_AGENT)\" $(SHELL_PATH) ../GIT-VERSION-GEN \"$(shell pwd)/..\" $< $@+\n-\t@if cmp $@+ $@ >/dev/null 2>&1; then $(RM) $@+; else mv $@+ $@; fi\n+\t$(QUIET_GEN)GIT_USER_AGENT=\"$(GIT_USER_AGENT)\" $(SHELL_PATH) ../GIT-VERSION-GEN \"$(shell pwd)/..\" $< $@\n endif\n \n ASCIIDOC_DEPS += docinfo.html\ndiff --git a/Makefile b/Makefile\nindex 79739a13d2132204f56b1ef4ca879bd51c5164b4..695a9d9765daf864605002d572129bae7a8c4e40 100644\n--- a/Makefile\n+++ b/Makefile\n@@ -2512,8 +2512,7 @@ pager.sp pager.s pager.o: EXTRA_CPPFLAGS = \\\n \t-DPAGER_ENV='$(PAGER_ENV_CQ_SQ)'\n \n version-def.h: version-def.h.in GIT-VERSION-GEN GIT-VERSION-FILE GIT-USER-AGENT\n-\t$(QUIET_GEN)GIT_USER_AGENT=\"$(GIT_USER_AGENT)\" $(SHELL_PATH) ./GIT-VERSION-GEN \"$(shell pwd)\" $< $@+\n-\t@if cmp $@+ $@ >/dev/null 2>&1; then $(RM) $@+; else mv $@+ $@; fi\n+\t$(QUIET_GEN)GIT_USER_AGENT=\"$(GIT_USER_AGENT)\" $(SHELL_PATH) ./GIT-VERSION-GEN \"$(shell pwd)\" $< $@\n \n version.sp version.s version.o: version-def.h\n \n@@ -2554,8 +2553,7 @@ $(SCRIPT_SH_GEN) $(SCRIPT_LIB) : % : %.sh generate-script.sh GIT-BUILD-OPTIONS G\n \tmv $@+ $@\n \n git.rc: git.rc.in GIT-VERSION-GEN GIT-VERSION-FILE\n-\t$(QUIET_GEN)$(SHELL_PATH) ./GIT-VERSION-GEN \"$(shell pwd)\" $< $@+\n-\t@if cmp $@+ $@ >/dev/null 2>&1; then $(RM) $@+; else mv $@+ $@; fi\n+\t$(QUIET_GEN)$(SHELL_PATH) ./GIT-VERSION-GEN \"$(shell pwd)\" $< $@\n \n git.res: git.rc GIT-PREFIX\n \t$(QUIET_RC)$(RC) -i $< -o $@\n\n-- \n2.48.0.rc0.184.g0fc57dec57.dirty\n\n"},{"id":"509448","messageId":"20241220-b4-pks-git-version-via-environment-v3-3-1fd79b52a5fb@pks.im","threadId":"62670","inReplyTo":"20241220-b4-pks-git-version-via-environment-v3-0-1fd79b52a5fb@pks.im","subject":"[PATCH v3 3/6] Makefile: introduce template for GIT-VERSION-GEN","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2024-12-20T19:44:23Z","receivedAt":"2024-12-20T19:44:49Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"Introduce a new template to call GIT-VERSION-GEN. This will allow us to\niterate on how exactly the script is called in subsequent commits\nwithout having to adapt all call sites every time.\n\nSigned-off-by: Patrick Steinhardt <ps@pks.im>\n---\n Documentation/Makefile | 4 ++--\n Makefile               | 6 +++---\n shared.mak             | 8 ++++++++\n 3 files changed, 13 insertions(+), 5 deletions(-)\n\ndiff --git a/Documentation/Makefile b/Documentation/Makefile\nindex 1a398d0dc359671d461fceb7a1636268a51411da..ff30ab6c4295525757f6a150ec4ff0c72487f440 100644\n--- a/Documentation/Makefile\n+++ b/Documentation/Makefile\n@@ -211,10 +211,10 @@ XMLTO_EXTRA += --skip-validation\n XMLTO_EXTRA += -x manpage.xsl\n \n asciidoctor-extensions.rb: asciidoctor-extensions.rb.in FORCE\n-\t$(QUIET_GEN)GIT_USER_AGENT=\"$(GIT_USER_AGENT)\" $(SHELL_PATH) ../GIT-VERSION-GEN \"$(shell pwd)/..\" $< $@\n+\t$(QUIET_GEN)$(call version_gen,\"$(shell pwd)/..\",$<,$@)\n else\n asciidoc.conf: asciidoc.conf.in FORCE\n-\t$(QUIET_GEN)GIT_USER_AGENT=\"$(GIT_USER_AGENT)\" $(SHELL_PATH) ../GIT-VERSION-GEN \"$(shell pwd)/..\" $< $@\n+\t$(QUIET_GEN)$(call version_gen,\"$(shell pwd)/..\",$<,$@)\n endif\n \n ASCIIDOC_DEPS += docinfo.html\ndiff --git a/Makefile b/Makefile\nindex 695a9d9765daf864605002d572129bae7a8c4e40..9cfe3d0aa968eff10379d22edff6cc6f4518c2ff 100644\n--- a/Makefile\n+++ b/Makefile\n@@ -593,7 +593,7 @@ include shared.mak\n \n GIT-VERSION-FILE: FORCE\n \t@OLD=$$(cat $@ 2>/dev/null || :) && \\\n-\t$(SHELL_PATH) ./GIT-VERSION-GEN \"$(shell pwd)\" GIT-VERSION-FILE.in $@ && \\\n+\t$(call version_gen,\"$(shell pwd)\",GIT-VERSION-FILE.in,$@) && \\\n \tNEW=$$(cat $@ 2>/dev/null || :) && \\\n \tif test \"$$OLD\" != \"$$NEW\"; then echo \"$$NEW\" >&2; fi\n -include GIT-VERSION-FILE\n@@ -2512,7 +2512,7 @@ pager.sp pager.s pager.o: EXTRA_CPPFLAGS = \\\n \t-DPAGER_ENV='$(PAGER_ENV_CQ_SQ)'\n \n version-def.h: version-def.h.in GIT-VERSION-GEN GIT-VERSION-FILE GIT-USER-AGENT\n-\t$(QUIET_GEN)GIT_USER_AGENT=\"$(GIT_USER_AGENT)\" $(SHELL_PATH) ./GIT-VERSION-GEN \"$(shell pwd)\" $< $@\n+\t$(QUIET_GEN)$(call version_gen,\"$(shell pwd)\",$<,$@)\n \n version.sp version.s version.o: version-def.h\n \n@@ -2553,7 +2553,7 @@ $(SCRIPT_SH_GEN) $(SCRIPT_LIB) : % : %.sh generate-script.sh GIT-BUILD-OPTIONS G\n \tmv $@+ $@\n \n git.rc: git.rc.in GIT-VERSION-GEN GIT-VERSION-FILE\n-\t$(QUIET_GEN)$(SHELL_PATH) ./GIT-VERSION-GEN \"$(shell pwd)\" $< $@\n+\t$(QUIET_GEN)$(call version_gen,\"$(shell pwd)\",$<,$@)\n \n git.res: git.rc GIT-PREFIX\n \t$(QUIET_RC)$(RC) -i $< -o $@\ndiff --git a/shared.mak b/shared.mak\nindex 29bebd30d8acbce9f50661cef48ecdbae1e41f5a..b23c5505c9692b032cd0b18d3e4ede288614d937 100644\n--- a/shared.mak\n+++ b/shared.mak\n@@ -116,3 +116,11 @@ endef\n define libpath_template\n -L$(1) $(if $(filter-out -L,$(CC_LD_DYNPATH)),$(CC_LD_DYNPATH)$(1))\n endef\n+\n+# Populate build information into a file via GIT-VERSION-GEN. Requires the\n+# absolute path to the root source directory as well as input and output files\n+# as arguments, in that order.\n+define version_gen\n+GIT_USER_AGENT=\"$(GIT_USER_AGENT)\" \\\n+$(SHELL_PATH) \"$(1)/GIT-VERSION-GEN\" \"$(1)\" \"$(2)\" \"$(3)\"\n+endef\n\n-- \n2.48.0.rc0.184.g0fc57dec57.dirty\n\n"},{"id":"509449","messageId":"20241220-b4-pks-git-version-via-environment-v3-4-1fd79b52a5fb@pks.im","threadId":"62670","inReplyTo":"20241220-b4-pks-git-version-via-environment-v3-0-1fd79b52a5fb@pks.im","subject":"[PATCH v3 4/6] GIT-VERSION-GEN: fix overriding GIT_VERSION","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2024-12-20T19:44:24Z","receivedAt":"2024-12-20T19:44:49Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"GIT-VERSION-GEN tries to derive the version that Git is being built from\nvia multiple different sources in the following order:\n\n  1. A file called \"version\" in the source tree's root directory, if it\n     exists.\n\n  2. The current commit in case Git is built from a Git repository.\n\n  3. Otherwise, we use a fallback version stored in a variable which is\n     bumped whenever a new Git version is getting tagged.\n\nIt used to be possible to override the version by overriding the\n`GIT_VERSION` Makefile variable (e.g. `make GIT_VERSION=foo`). This\nworked somewhat by chance, only: `GIT-VERSION-GEN` would write the\nactual Git version into `GIT-VERSION-FILE`, not the overridden value,\nbut when including the file into our Makefile we would not override the\n`GIT_VERSION` variable because it has already been set by the user. And\nbecause our Makefile used the variable to propagate the version to our\nbuild tools instead of using `GIT-VERSION-FILE` the resulting build\nartifacts used the overridden version.\n\nBut that subtle mechanism broke with 4838deab65 (Makefile: refactor\nGIT-VERSION-GEN to be reusable, 2024-12-06) and subsequent commits\nbecause the version information is not propagated via the Makefile\nvariable anymore, but instead via the files that `GIT-VERSION-GEN`\nstarted to write. And as the script never knew about the `GIT_VERSION`\nenvironment variable in the first place it uses one of the values listed\nabove instead of the overridden value.\n\nFix this issue by making `GIT-VERSION-GEN` handle the case where\n`GIT_VERSION` has been set via the environment.\n\nNote that this requires us to introduce a new GIT_VERSION_OVERRIDE\nvariable that stores a potential user-provided value, either via the\nenvironment or via \"config.mak\". Ideally we wouldn't need it and could\njust continue to use GIT_VERSION for this. But unfortunately, Makefiles\nwill first include all sub-Makefiles before figuring out whether it\nneeds to re-make any of them [1]. Consequently, if there already is a\nGIT-VERSION-FILE, we would have slurped in its value of GIT_VERSION\nbefore we call GIT-VERSION-GEN, and because GIT-VERSION-GEN now uses\nthat value as an override it would mean that the first generated value\nfor GIT_VERSION will remain unchanged.\n\nFurthermore we have to move the include for \"GIT-VERSION-FILE\" after the\nincludes for \"config.mak\" and related so that GIT_VERSION_OVERRIDE can\nbe set to the value provided by \"config.mak\".\n\n[1]: https://www.gnu.org/software/make/manual/html_node/Remaking-Makefiles.html\n\nSigned-off-by: Patrick Steinhardt <ps@pks.im>\n---\n Documentation/Makefile |  4 ++++\n GIT-VERSION-GEN        | 48 ++++++++++++++++++++++++++----------------------\n Makefile               | 19 ++++++++++++-------\n shared.mak             |  1 +\n 4 files changed, 43 insertions(+), 29 deletions(-)\n\ndiff --git a/Documentation/Makefile b/Documentation/Makefile\nindex ff30ab6c4295525757f6a150ec4ff0c72487f440..a89823e1d1ee5042367bdcca6ed426196d49ce89 100644\n--- a/Documentation/Makefile\n+++ b/Documentation/Makefile\n@@ -181,6 +181,10 @@ endif\n -include ../config.mak.autogen\n -include ../config.mak\n \n+# Set GIT_VERSION_OVERRIDE such that version_gen knows to substitute\n+# GIT_VERSION in case it was set by the user.\n+GIT_VERSION_OVERRIDE := $(GIT_VERSION)\n+\n ifndef NO_MAN_BOLD_LITERAL\n XMLTO_EXTRA += -m manpage-bold-literal.xsl\n endif\ndiff --git a/GIT-VERSION-GEN b/GIT-VERSION-GEN\nindex de0e63bdfbac263884e2ea328cc2ef11ace7a238..9b785da226eff2d7952d3306f45fd2933fdafaca 100755\n--- a/GIT-VERSION-GEN\n+++ b/GIT-VERSION-GEN\n@@ -27,31 +27,35 @@ fi\n GIT_CEILING_DIRECTORIES=\"$SOURCE_DIR/..\"\n export GIT_CEILING_DIRECTORIES\n \n-# First see if there is a version file (included in release tarballs),\n-# then try git-describe, then default.\n-if test -f \"$SOURCE_DIR\"/version\n+if test -z \"$GIT_VERSION\"\n then\n-\tVN=$(cat \"$SOURCE_DIR\"/version) || VN=\"$DEF_VER\"\n-elif {\n-\t\ttest -d \"$SOURCE_DIR/.git\" ||\n-\t\ttest -d \"${GIT_DIR:-.git}\" ||\n-\t\ttest -f \"$SOURCE_DIR\"/.git;\n-\t} &&\n-\tVN=$(git -C \"$SOURCE_DIR\" describe --match \"v[0-9]*\" HEAD 2>/dev/null) &&\n-\tcase \"$VN\" in\n-\t*$LF*) (exit 1) ;;\n-\tv[0-9]*)\n-\t\tgit -C \"$SOURCE_DIR\" update-index -q --refresh\n-\t\ttest -z \"$(git -C \"$SOURCE_DIR\" diff-index --name-only HEAD --)\" ||\n-\t\tVN=\"$VN-dirty\" ;;\n-\tesac\n-then\n-\tVN=$(echo \"$VN\" | sed -e 's/-/./g');\n-else\n-\tVN=\"$DEF_VER\"\n+\t# First see if there is a version file (included in release tarballs),\n+\t# then try git-describe, then default.\n+\tif test -f \"$SOURCE_DIR\"/version\n+\tthen\n+\t\tVN=$(cat \"$SOURCE_DIR\"/version) || VN=\"$DEF_VER\"\n+\telif {\n+\t\t\ttest -d \"$SOURCE_DIR/.git\" ||\n+\t\t\ttest -d \"${GIT_DIR:-.git}\" ||\n+\t\t\ttest -f \"$SOURCE_DIR\"/.git;\n+\t\t} &&\n+\t\tVN=$(git -C \"$SOURCE_DIR\" describe --match \"v[0-9]*\" HEAD 2>/dev/null) &&\n+\t\tcase \"$VN\" in\n+\t\t*$LF*) (exit 1) ;;\n+\t\tv[0-9]*)\n+\t\t\tgit -C \"$SOURCE_DIR\" update-index -q --refresh\n+\t\t\ttest -z \"$(git -C \"$SOURCE_DIR\" diff-index --name-only HEAD --)\" ||\n+\t\t\tVN=\"$VN-dirty\" ;;\n+\t\tesac\n+\tthen\n+\t\tVN=$(echo \"$VN\" | sed -e 's/-/./g');\n+\telse\n+\t\tVN=\"$DEF_VER\"\n+\tfi\n+\n+\tGIT_VERSION=$(expr \"$VN\" : v*'\\(.*\\)')\n fi\n \n-GIT_VERSION=$(expr \"$VN\" : v*'\\(.*\\)')\n GIT_BUILT_FROM_COMMIT=$(git -C \"$SOURCE_DIR\" rev-parse -q --verify HEAD 2>/dev/null)\n GIT_DATE=$(git -C \"$SOURCE_DIR\" show --quiet --format='%as' 2>/dev/null)\n if test -z \"$GIT_USER_AGENT\"\ndiff --git a/Makefile b/Makefile\nindex 9cfe3d0aa968eff10379d22edff6cc6f4518c2ff..3cdd2278fdaa40feb6139992aa275ad26aaae4a6 100644\n--- a/Makefile\n+++ b/Makefile\n@@ -591,13 +591,6 @@ include shared.mak\n #\n #        Disable -pedantic compilation.\n \n-GIT-VERSION-FILE: FORCE\n-\t@OLD=$$(cat $@ 2>/dev/null || :) && \\\n-\t$(call version_gen,\"$(shell pwd)\",GIT-VERSION-FILE.in,$@) && \\\n-\tNEW=$$(cat $@ 2>/dev/null || :) && \\\n-\tif test \"$$OLD\" != \"$$NEW\"; then echo \"$$NEW\" >&2; fi\n--include GIT-VERSION-FILE\n-\n # Set our default configuration.\n #\n # Among the variables below, these:\n@@ -1465,6 +1458,18 @@ ifdef DEVELOPER\n include config.mak.dev\n endif\n \n+GIT-VERSION-FILE: FORCE\n+\t@OLD=$$(cat $@ 2>/dev/null || :) && \\\n+\t$(call version_gen,\"$(shell pwd)\",GIT-VERSION-FILE.in,$@) && \\\n+\tNEW=$$(cat $@ 2>/dev/null || :) && \\\n+\tif test \"$$OLD\" != \"$$NEW\"; then echo \"$$NEW\" >&2; fi\n+\n+# We need to set GIT_VERSION_OVERRIDE before including the version file as\n+# otherwise any user-provided value for GIT_VERSION would have been overridden\n+# already.\n+GIT_VERSION_OVERRIDE := $(GIT_VERSION)\n+-include GIT-VERSION-FILE\n+\n # what 'all' will build and 'install' will install in gitexecdir,\n # excluding programs for built-in commands\n ALL_PROGRAMS = $(PROGRAMS) $(SCRIPTS)\ndiff --git a/shared.mak b/shared.mak\nindex b23c5505c9692b032cd0b18d3e4ede288614d937..a66f46969e301b35d88650f2c6abc6c0ba1b0f3d 100644\n--- a/shared.mak\n+++ b/shared.mak\n@@ -122,5 +122,6 @@ endef\n # as arguments, in that order.\n define version_gen\n GIT_USER_AGENT=\"$(GIT_USER_AGENT)\" \\\n+GIT_VERSION=\"$(GIT_VERSION_OVERRIDE)\" \\\n $(SHELL_PATH) \"$(1)/GIT-VERSION-GEN\" \"$(1)\" \"$(2)\" \"$(3)\"\n endef\n\n-- \n2.48.0.rc0.184.g0fc57dec57.dirty\n\n"},{"id":"509450","messageId":"20241220-b4-pks-git-version-via-environment-v3-6-1fd79b52a5fb@pks.im","threadId":"62670","inReplyTo":"20241220-b4-pks-git-version-via-environment-v3-0-1fd79b52a5fb@pks.im","subject":"[PATCH v3 6/6] meson: add options to override build information","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2024-12-20T19:44:26Z","receivedAt":"2024-12-20T19:44:51Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"We inject various different kinds of build information into build\nartifacts, like the version string or the commit from which Git was\nbuilt. Add options to let users explicitly override this information\nwith Meson.\n\nSigned-off-by: Patrick Steinhardt <ps@pks.im>\n---\n Documentation/meson.build |  1 +\n meson.build               | 13 +++++++++++++\n meson_options.txt         | 10 ++++++++++\n 3 files changed, 24 insertions(+)\n\ndiff --git a/Documentation/meson.build b/Documentation/meson.build\nindex f2426ccaa30c29bd60b850eb0a9a4ab77c66a629..fca3eab1f1360a5fdeda89c1766ab8cdb3267b89 100644\n--- a/Documentation/meson.build\n+++ b/Documentation/meson.build\n@@ -219,6 +219,7 @@ asciidoc_conf = custom_target(\n   input: meson.current_source_dir() / 'asciidoc.conf.in',\n   output: 'asciidoc.conf',\n   depends: [git_version_file],\n+  env: version_gen_environment,\n )\n \n asciidoc_common_options = [\ndiff --git a/meson.build b/meson.build\nindex 0dccebcdf16b07650d943e53643f0e09e2975cc9..be32d60e841a3055ed2bdf6fd449a48b66d94cd0 100644\n--- a/meson.build\n+++ b/meson.build\n@@ -201,6 +201,16 @@ if get_option('sane_tool_path') != ''\n   script_environment.prepend('PATH', get_option('sane_tool_path'))\n endif\n \n+# The environment used by GIT-VERSION-GEN. Note that we explicitly override\n+# environment variables that might be set by the user. This is by design so\n+# that we always use whatever Meson has configured instead of what is present\n+# in the environment.\n+version_gen_environment = script_environment\n+version_gen_environment.set('GIT_BUILT_FROM_COMMIT', get_option('built_from_commit'))\n+version_gen_environment.set('GIT_DATE', get_option('build_date'))\n+version_gen_environment.set('GIT_USER_AGENT', get_option('user_agent'))\n+version_gen_environment.set('GIT_VERSION', get_option('version'))\n+\n compiler = meson.get_compiler('c')\n \n libgit_sources = [\n@@ -1485,6 +1495,7 @@ git_version_file = custom_target(\n   ],\n   input: meson.current_source_dir() / 'GIT-VERSION-FILE.in',\n   output: 'GIT-VERSION-FILE',\n+  env: version_gen_environment,\n   build_always_stale: true,\n )\n \n@@ -1501,6 +1512,7 @@ version_def_h = custom_target(\n   # Depend on GIT-VERSION-FILE so that we don't always try to rebuild this\n   # target for the same commit.\n   depends: [git_version_file],\n+  env: version_gen_environment,\n )\n \n # Build a separate library for \"version.c\" so that we do not have to rebuild\n@@ -1544,6 +1556,7 @@ if host_machine.system() == 'windows'\n     input: meson.current_source_dir() / 'git.rc.in',\n     output: 'git.rc',\n     depends: [git_version_file],\n+    env: version_gen_environment,\n   )\n \n   common_main_sources += import('windows').compile_resources(git_rc,\ndiff --git a/meson_options.txt b/meson_options.txt\nindex 32a72139bae870745d9131cc9086a4594826be91..8ead1349550807420bdb95e55298ea4f3f2ea9d0 100644\n--- a/meson_options.txt\n+++ b/meson_options.txt\n@@ -16,6 +16,16 @@ option('runtime_prefix', type: 'boolean', value: false,\n option('sane_tool_path', type: 'string', value: '',\n   description: 'A colon-separated list of paths to prepend to PATH if your tools in /usr/bin are broken.')\n \n+# Build information compiled into Git and other parts like documentation.\n+option('build_date', type: 'string', value: '',\n+  description: 'Build date reported by our documentation.')\n+option('built_from_commit', type: 'string', value: '',\n+  description: 'Commit that Git was built from reported by git-version(1).')\n+option('user_agent', type: 'string', value: '',\n+  description: 'User agent reported to remote servers.')\n+option('version', type: 'string', value: '',\n+  description: 'Version string reported by git-version(1) and other tools.')\n+\n # Features supported by Git.\n option('curl', type: 'feature', value: 'enabled',\n   description: 'Build helpers used to access remotes with the HTTP transport.')\n\n-- \n2.48.0.rc0.184.g0fc57dec57.dirty\n\n"},{"id":"509451","messageId":"20241220-b4-pks-git-version-via-environment-v3-5-1fd79b52a5fb@pks.im","threadId":"62670","inReplyTo":"20241220-b4-pks-git-version-via-environment-v3-0-1fd79b52a5fb@pks.im","subject":"[PATCH v3 5/6] GIT-VERSION-GEN: fix overriding GIT_BUILT_FROM_COMMIT and GIT_DATE","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2024-12-20T19:44:25Z","receivedAt":"2024-12-20T19:44:51Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"Same as with the preceding commit, neither GIT_BUILT_FROM_COMMIT nor\nGIT_DATE can be overridden via the environment. Especially the latter is\nof importance given that we set it in our own \"Documentation/doc-diff\"\nscript.\n\nMake the values of both variables overridable. Luckily we don't pull in\nthese values via any included Makefiles, so the fix is trivial compared\nto the fix for GIT_VERSON.\n\nSigned-off-by: Patrick Steinhardt <ps@pks.im>\n---\n GIT-VERSION-GEN | 12 ++++++++++--\n shared.mak      |  2 ++\n 2 files changed, 12 insertions(+), 2 deletions(-)\n\ndiff --git a/GIT-VERSION-GEN b/GIT-VERSION-GEN\nindex 9b785da226eff2d7952d3306f45fd2933fdafaca..497b4e48d20f9d1d20f2db2a3aae2a92316b7ca9 100755\n--- a/GIT-VERSION-GEN\n+++ b/GIT-VERSION-GEN\n@@ -56,8 +56,16 @@ then\n \tGIT_VERSION=$(expr \"$VN\" : v*'\\(.*\\)')\n fi\n \n-GIT_BUILT_FROM_COMMIT=$(git -C \"$SOURCE_DIR\" rev-parse -q --verify HEAD 2>/dev/null)\n-GIT_DATE=$(git -C \"$SOURCE_DIR\" show --quiet --format='%as' 2>/dev/null)\n+if test -z \"$GIT_BUILT_FROM_COMMIT\"\n+then\n+\tGIT_BUILT_FROM_COMMIT=$(git -C \"$SOURCE_DIR\" rev-parse -q --verify HEAD 2>/dev/null)\n+fi\n+\n+if test -z \"$GIT_DATE\"\n+then\n+\tGIT_DATE=$(git -C \"$SOURCE_DIR\" show --quiet --format='%as' 2>/dev/null)\n+fi\n+\n if test -z \"$GIT_USER_AGENT\"\n then\n \tGIT_USER_AGENT=\"git/$GIT_VERSION\"\ndiff --git a/shared.mak b/shared.mak\nindex a66f46969e301b35d88650f2c6abc6c0ba1b0f3d..1a99848a95174c5e386c59321655937e7b7d8a28 100644\n--- a/shared.mak\n+++ b/shared.mak\n@@ -121,6 +121,8 @@ endef\n # absolute path to the root source directory as well as input and output files\n # as arguments, in that order.\n define version_gen\n+GIT_BUILT_FROM_COMMIT=\"$(GIT_BUILT_FROM_COMMIT)\" \\\n+GIT_DATE=\"$(GIT_DATE)\" \\\n GIT_USER_AGENT=\"$(GIT_USER_AGENT)\" \\\n GIT_VERSION=\"$(GIT_VERSION_OVERRIDE)\" \\\n $(SHELL_PATH) \"$(1)/GIT-VERSION-GEN\" \"$(1)\" \"$(2)\" \"$(3)\"\n\n-- \n2.48.0.rc0.184.g0fc57dec57.dirty\n\n"},{"id":"509454","messageId":"xmqq4j2ydnxa.fsf@gitster.g","threadId":"62670","inReplyTo":"20241220-b4-pks-git-version-via-environment-v3-4-1fd79b52a5fb@pks.im","subject":"Re: [PATCH v3 4/6] GIT-VERSION-GEN: fix overriding GIT_VERSION","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2024-12-20T21:50:25Z","receivedAt":"2024-12-20T21:50:28Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Patrick Steinhardt <ps@pks.im> writes:\n\n> diff --git a/Documentation/Makefile b/Documentation/Makefile\n> index ff30ab6c4295525757f6a150ec4ff0c72487f440..a89823e1d1ee5042367bdcca6ed426196d49ce89 100644\n> --- a/Documentation/Makefile\n> +++ b/Documentation/Makefile\n> @@ -181,6 +181,10 @@ endif\n>  -include ../config.mak.autogen\n>  -include ../config.mak\n>  \n> +# Set GIT_VERSION_OVERRIDE such that version_gen knows to substitute\n> +# GIT_VERSION in case it was set by the user.\n> +GIT_VERSION_OVERRIDE := $(GIT_VERSION)\n> +\n>  ifndef NO_MAN_BOLD_LITERAL\n>  XMLTO_EXTRA += -m manpage-bold-literal.xsl\n>  endif\n\nSo the idea is that those targets and scripts may have their own\nGIT_VERION value when they run GIT-VERSION-GEN to cause GIT_VERSION\nto computed, and in such a case, they should pass the GIT_VERSION\nthey have in GIT_VERSION_OVERRIDE, and thanks to the version_gen\nthing, this value in GIT_VERSION_OVERRIDE is passed in the\nenvironment as GIT_VERSION when GIT-VERSION-GEN is run, and the\nvalue in turn is passed intact.  Somehow this makes my head spin, as\nit looks quite convoluted, but the overall flow should yield the\ndesired value.\n\nQueued.  Thanks.\n"},{"id":"509462","messageId":"Z2aYx608clVG_sAF@pks.im","threadId":"62670","inReplyTo":"xmqq4j2ydnxa.fsf@gitster.g","subject":"Re: [PATCH v3 4/6] GIT-VERSION-GEN: fix overriding GIT_VERSION","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2024-12-21T10:30:39Z","receivedAt":"2024-12-21T10:31:00Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"On Fri, Dec 20, 2024 at 01:50:25PM -0800, Junio C Hamano wrote:\n> Patrick Steinhardt <ps@pks.im> writes:\n> \n> > diff --git a/Documentation/Makefile b/Documentation/Makefile\n> > index ff30ab6c4295525757f6a150ec4ff0c72487f440..a89823e1d1ee5042367bdcca6ed426196d49ce89 100644\n> > --- a/Documentation/Makefile\n> > +++ b/Documentation/Makefile\n> > @@ -181,6 +181,10 @@ endif\n> >  -include ../config.mak.autogen\n> >  -include ../config.mak\n> >  \n> > +# Set GIT_VERSION_OVERRIDE such that version_gen knows to substitute\n> > +# GIT_VERSION in case it was set by the user.\n> > +GIT_VERSION_OVERRIDE := $(GIT_VERSION)\n> > +\n> >  ifndef NO_MAN_BOLD_LITERAL\n> >  XMLTO_EXTRA += -m manpage-bold-literal.xsl\n> >  endif\n> \n> So the idea is that those targets and scripts may have their own\n> GIT_VERION value when they run GIT-VERSION-GEN to cause GIT_VERSION\n> to computed, and in such a case, they should pass the GIT_VERSION\n> they have in GIT_VERSION_OVERRIDE, and thanks to the version_gen\n> thing, this value in GIT_VERSION_OVERRIDE is passed in the\n> environment as GIT_VERSION when GIT-VERSION-GEN is run, and the\n> value in turn is passed intact.  Somehow this makes my head spin, as\n> it looks quite convoluted, but the overall flow should yield the\n> desired value.\n\nAgreed, it makes mine spin, as well. Next release cycle I'll have a look\nat whether I can get rid of the include of \"GIT-VERSION-FILE\" altogether\nto make the logic simpler.\n\nPatrick\n"},{"id":"509698","messageId":"20241228194345.GA1535629@coredump.intra.peff.net","threadId":"62670","inReplyTo":"Z2XAiDC4pJ9OjTpC@pks.im","subject":"Re: [PATCH v2 4/5] Makefile: respect build info declared in \"config.mak\"","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2024-12-28T19:43:45Z","receivedAt":"2024-12-28T19:43:47Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Fri, Dec 20, 2024 at 08:07:52PM +0100, Patrick Steinhardt wrote:\n\n> > > +# Never include it on the first read-through, only after make has tried to\n> > > +# refresh includes. We do not want the old values to pollute our new run of the\n> > > +# rule above.\n> > > +ifdef MAKE_RESTARTS\n> > >  -include GIT-VERSION-FILE\n> > > +endif\n> [...]\n> Okay, I did find an issue where it does not work:\n> \n>     $ git clean -dfx\n>     $ make GIT-USER-AGENT\n>     $ cat GIT-USER-AGENT\n>     git/\n>     $ cat GIT-VERSION-FILE\n>     cat: GIT-VERSION-FILE: No such file or directory\n> \n> It does not generate the version file at all anymore when it's not an\n> explicit dependency. While I could of course add the missing dependency\n> I don't know whether there are any other implicit dependencies that\n> would be broken, as well. My gut feeling says \"probably\".\n\nDoh, of course. We really want to say \"do include this and consider it a\ndependency, but don't read it yet\". But I don't think there's a way to\ntell make to do that.\n\nI looked over your alternative approach with the OVERRIDE variable. I\ncan't think of any downsides, aside from the general head-spinning\ncomplexity. ;) So that seems like a good approach for now (and I see\nit's already in master. Yay).\n\n-Peff\n"}]}