Re: [PATCH v2 4/5] Makefile: respect build info declared in "config.mak"

From: Jeff King <peff@peff.net>
Date: Dec 20, 2024, 18:24 UTC
Message-ID: <20241220182427.GA213015@coredump.intra.peff.net>
In-Reply-To: <Z2WxIRcV0LOvx6OX@pks.im>

On Fri, Dec 20, 2024 at 07:02:09PM +0100, Patrick Steinhardt wrote:

Show 17 quoted lines

> > Is there a case you found that doesn't work?
> 
> Yes:
> 
>     $ make GIT-VERSION-FILE GIT_VERSION=foo
>     GIT_VERSION=foo
>     make: 'GIT-VERSION-FILE' is up to date.
>     $ cat GIT-VERSION-FILE
>     GIT_VERSION=foo
> 
>     # And now run without GIT_VERSION set.
>     make: 'GIT-VERSION-FILE' is up to date.
>     GIT_VERSION=foo
> 
> So the value remains "sticky" in this case. And that is true whenever
> you don't set GIT_VERSION at all, we always stick with what is currently
> in that file.

Ah, right. Even though we have a recipe to build it, and make knows it must be built (because it depends on FORCE), make will read it (and all includes) first before executing any rules.

Something like this seems to work:

diff --git a/Makefile b/Makefile
index 788f6ee172..0eb08d98f4 100644
--- a/Makefile
+++ b/Makefile
@@ -596,7 +596,12 @@ GIT-VERSION-FILE: FORCE
 	$(SHELL_PATH) ./GIT-VERSION-GEN "$(shell pwd)" GIT-VERSION-FILE.in $@ && \
 	NEW=$$(cat $@ 2>/dev/null || :) && \
 	if test "$$OLD" != "$$NEW"; then echo "$$NEW" >&2; fi
+# Never include it on the first read-through, only after make has tried to
+# refresh includes. We do not want the old values to pollute our new run of the
+# rule above.
+ifdef MAKE_RESTARTS
 -include GIT-VERSION-FILE
+endif
 
 # Set our default configuration.
 #


But I don't know if there are any gotchas (I did not even know about
MAKE_RESTARTS until digging in the docs looking for a solution here).
If we can stop including it as a Makefile snippet entirely, I think that
is easier to reason about.

-Peff