build: further refine build.ninja rules

[PATCH] build: further refine build.ninja rules

Posted by Paolo Bonzini 10 months, 3 weeks ago

In commit b0fcc6fc7fc1 ("build: rebuild build.ninja using
"meson setup --reconfigure"", 2023-05-19) I changed the build.ninja
rule in the Makefile to use "meson setup" so that the Makefile would
pick up a changed path to the meson binary.

However, there was a reason why build.ninja was rebuilt using $(NINJA)
itself.  Namely, ninja has its own cache of file modification times,
and if it does not know about the modification that was done outside
its control, it will *also* try to regenerate build.ninja.  This can be
simply by running "make" on a fresh tree immediately after "configure";
that will trigger an unnecessary meson run.

So, apply a refinement to the rule in order to cover both cases:

- track the meson binary that was used (and that is embedded in
  build.ninja's reconfigure rules); to do this, write build.ninja.stamp
  right after executing meson successfully

- if it changed, force usage of "$(MESON) setup --reconfigure" to
  update the path in the reconfigure rule

- if it didn't change, use "$(NINJA) build.ninja" just like before
  commit b0fcc6fc7fc1.

Reported-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 Makefile  | 17 +++++++++++++----
 configure |  1 +
 2 files changed, 14 insertions(+), 4 deletions(-)

diff --git a/Makefile b/Makefile
index b22bf6fba12..804a5681e0a 100644
--- a/Makefile
+++ b/Makefile
@@ -83,16 +83,17 @@ config-host.mak: $(SRC_PATH)/configure $(SRC_PATH)/scripts/meson-buildoptions.sh
 	@if test -f meson-private/coredata.dat; then \
 	  ./config.status --skip-meson; \
 	else \
-	  ./config.status && touch build.ninja.stamp; \
+	  ./config.status; \
 	fi
 
 # 2. meson.stamp exists if meson has run at least once (so ninja reconfigure
 # works), but otherwise never needs to be updated
+
 meson-private/coredata.dat: meson.stamp
 meson.stamp: config-host.mak
 	@touch meson.stamp
 
-# 3. ensure generated build files are up-to-date
+# 3. ensure meson-generated build files are up-to-date
 
 ifneq ($(NINJA),)
 Makefile.ninja: build.ninja
@@ -106,11 +107,19 @@ Makefile.ninja: build.ninja
 endif
 
 ifneq ($(MESON),)
-# A separate rule is needed for Makefile dependencies to avoid -n
+# The path to meson always points to pyvenv/bin/meson, but the absolute
+# paths could change.  In that case, force a regeneration of build.ninja.
+# Note that this invocation of $(NINJA), just like when Make rebuilds
+# Makefiles, does not include -n.
 build.ninja: build.ninja.stamp
 $(build-files):
 build.ninja.stamp: meson.stamp $(build-files)
-	$(MESON) setup --reconfigure $(SRC_PATH) && touch $@
+	@if test "$$(cat build.ninja.stamp)" = "$(MESON)" && test -n "$(NINJA)"; then \
+	  $(NINJA) build.ninja; \
+	else \
+	  echo "$(MESON) setup --reconfigure $(SRC_PATH)"; \
+	  $(MESON) setup --reconfigure $(SRC_PATH); \
+	fi && echo "$(MESON)" > $@
 
 Makefile.mtest: build.ninja scripts/mtest2make.py
 	$(MESON) introspect --targets --tests --benchmarks | $(PYTHON) scripts/mtest2make.py > $@
diff --git a/configure b/configure
index 8a638dd82ae..cbdb389dc95 100755
--- a/configure
+++ b/configure
@@ -1894,6 +1894,7 @@ if test "$skip_meson" = no; then
   if test "$?" -ne 0 ; then
       error_exit "meson setup failed"
   fi
+  echo "$meson" > build.ninja.stamp
 else
   if test -f meson-private/cmd_line.txt; then
     # Adjust old command line options that were removed
-- 
2.40.1

Re: [PATCH] build: further refine build.ninja rules

Posted by Philippe Mathieu-Daudé 10 months, 3 weeks ago

On 8/6/23 15:43, Paolo Bonzini wrote:
> In commit b0fcc6fc7fc1 ("build: rebuild build.ninja using
> "meson setup --reconfigure"", 2023-05-19) I changed the build.ninja
> rule in the Makefile to use "meson setup" so that the Makefile would
> pick up a changed path to the meson binary.
> 
> However, there was a reason why build.ninja was rebuilt using $(NINJA)
> itself.  Namely, ninja has its own cache of file modification times,
> and if it does not know about the modification that was done outside
> its control, it will *also* try to regenerate build.ninja.  This can be
> simply by running "make" on a fresh tree immediately after "configure";
> that will trigger an unnecessary meson run.
> 
> So, apply a refinement to the rule in order to cover both cases:
> 
> - track the meson binary that was used (and that is embedded in
>    build.ninja's reconfigure rules); to do this, write build.ninja.stamp
>    right after executing meson successfully
> 
> - if it changed, force usage of "$(MESON) setup --reconfigure" to
>    update the path in the reconfigure rule
> 
> - if it didn't change, use "$(NINJA) build.ninja" just like before
>    commit b0fcc6fc7fc1.
> 
> Reported-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
>   Makefile  | 17 +++++++++++++----
>   configure |  1 +
>   2 files changed, 14 insertions(+), 4 deletions(-)

Tested-by: Philippe Mathieu-Daudé <philmd@linaro.org>

Re: [PATCH] build: further refine build.ninja rules

Posted by Mark Cave-Ayland 10 months, 3 weeks ago

On 08/06/2023 14:43, Paolo Bonzini wrote:

> In commit b0fcc6fc7fc1 ("build: rebuild build.ninja using
> "meson setup --reconfigure"", 2023-05-19) I changed the build.ninja
> rule in the Makefile to use "meson setup" so that the Makefile would
> pick up a changed path to the meson binary.
> 
> However, there was a reason why build.ninja was rebuilt using $(NINJA)
> itself.  Namely, ninja has its own cache of file modification times,
> and if it does not know about the modification that was done outside
> its control, it will *also* try to regenerate build.ninja.  This can be
> simply by running "make" on a fresh tree immediately after "configure";
> that will trigger an unnecessary meson run.
> 
> So, apply a refinement to the rule in order to cover both cases:
> 
> - track the meson binary that was used (and that is embedded in
>    build.ninja's reconfigure rules); to do this, write build.ninja.stamp
>    right after executing meson successfully
> 
> - if it changed, force usage of "$(MESON) setup --reconfigure" to
>    update the path in the reconfigure rule
> 
> - if it didn't change, use "$(NINJA) build.ninja" just like before
>    commit b0fcc6fc7fc1.
> 
> Reported-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
>   Makefile  | 17 +++++++++++++----
>   configure |  1 +
>   2 files changed, 14 insertions(+), 4 deletions(-)
> 
> diff --git a/Makefile b/Makefile
> index b22bf6fba12..804a5681e0a 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -83,16 +83,17 @@ config-host.mak: $(SRC_PATH)/configure $(SRC_PATH)/scripts/meson-buildoptions.sh
>   	@if test -f meson-private/coredata.dat; then \
>   	  ./config.status --skip-meson; \
>   	else \
> -	  ./config.status && touch build.ninja.stamp; \
> +	  ./config.status; \
>   	fi
>   
>   # 2. meson.stamp exists if meson has run at least once (so ninja reconfigure
>   # works), but otherwise never needs to be updated
> +
>   meson-private/coredata.dat: meson.stamp
>   meson.stamp: config-host.mak
>   	@touch meson.stamp
>   
> -# 3. ensure generated build files are up-to-date
> +# 3. ensure meson-generated build files are up-to-date
>   
>   ifneq ($(NINJA),)
>   Makefile.ninja: build.ninja
> @@ -106,11 +107,19 @@ Makefile.ninja: build.ninja
>   endif
>   
>   ifneq ($(MESON),)
> -# A separate rule is needed for Makefile dependencies to avoid -n
> +# The path to meson always points to pyvenv/bin/meson, but the absolute
> +# paths could change.  In that case, force a regeneration of build.ninja.
> +# Note that this invocation of $(NINJA), just like when Make rebuilds
> +# Makefiles, does not include -n.
>   build.ninja: build.ninja.stamp
>   $(build-files):
>   build.ninja.stamp: meson.stamp $(build-files)
> -	$(MESON) setup --reconfigure $(SRC_PATH) && touch $@
> +	@if test "$$(cat build.ninja.stamp)" = "$(MESON)" && test -n "$(NINJA)"; then \
> +	  $(NINJA) build.ninja; \
> +	else \
> +	  echo "$(MESON) setup --reconfigure $(SRC_PATH)"; \
> +	  $(MESON) setup --reconfigure $(SRC_PATH); \
> +	fi && echo "$(MESON)" > $@
>   
>   Makefile.mtest: build.ninja scripts/mtest2make.py
>   	$(MESON) introspect --targets --tests --benchmarks | $(PYTHON) scripts/mtest2make.py > $@
> diff --git a/configure b/configure
> index 8a638dd82ae..cbdb389dc95 100755
> --- a/configure
> +++ b/configure
> @@ -1894,6 +1894,7 @@ if test "$skip_meson" = no; then
>     if test "$?" -ne 0 ; then
>         error_exit "meson setup failed"
>     fi
> +  echo "$meson" > build.ninja.stamp
>   else
>     if test -f meson-private/cmd_line.txt; then
>       # Adjust old command line options that were removed

Thanks Paolo!

I just tried a complete fresh build, make, a few changes followed by another make and 
everything built without running reconfigure so:

Tested-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>


ATB,

Mark.