Re: [PATCH] Makefile: separate meson rerun from the rest of the ninja invocation

["Håvard Skinnemoen" <hskinnemoen@gmail.com>]

[-- Attachment #1: Type: text/plain, Size: 5689 bytes --]

On Fri, Oct 23, 2020 at 8:05 AM Paolo Bonzini <pbonzini@redhat.com> wrote:

> The rules to build Makefile.mtest are suffering from the "tunnel vision"
> problem that is common with recursive makefiles.  Makefile.mtest depends
> on build.ninja, but Make does not know when build.ninja needs to be
> rebuilt before creating Makefile.mtest.
>
> To fix this, separate the ninja invocation into the "regenerate build
> files" phase and the QEMU build phase.  Sentinel files such as
> meson-private/coredata.dat or build.ninja are used to figure out the
> phases that haven't run yet; however, because those files' timestamps
> are not guaranteed to be touched, the usual makefile stamp-file trick
> is used on top.
>

Thanks for the quick response. Unfortunately, it doesn't seem to work quite
right for me.

I think it should be possible to reproduce on unmodified upstream sources
like this:

$ cd bin/opt/arm
$ ../../../configure --target-list=arm-softmmu
$ make check-qtest  # Feel free to interrupt after verifying that
npcm7xx_timer-test is run
$ cd -
$ git revert 19d50149c857e50ccb1ee35dd4277f9d4954877f
Removing tests/qtest/npcm7xx_timer-test.c
[meson-test 3ce2b719aa] Revert "tests/qtest: Add npcm7xx timer test"
 2 files changed, 563 deletions(-)
 delete mode 100644 tests/qtest/npcm7xx_timer-test.c
$ cd -
$ make check-qtest

After the revert, check-qtest still runs npcm7xx_timer-test. Note that it
doesn't fail because the patch I reverted only adds a new test, it doesn't
touch the device being tested.

If you run 'make check-qtest' again, npcm7xx_timer-test does not run.
Interestingly, it looks like Makefile.mtest gets regenerated here -- I
didn't notice that happening in the first run, nor does it happen if I run
make check-qtest a third time.

I pasted the output from the last two runs of make check-qtest here:
https://gist.github.com/hskinnemoen/1773edafeb190773661076e00fdc14de

Reported-by: Havard Skinnemoen <hskinnemoen@gmail.com>
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
>  Makefile | 42 +++++++++++++++++++++++++++++++-----------
>  1 file changed, 31 insertions(+), 11 deletions(-)
>
> diff --git a/Makefile b/Makefile
> index 18f026eac3..007ad4d863 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -92,6 +92,8 @@ endif
>  ifeq ($(NINJA),)
>  .PHONY: config-host.mak
>  x := $(shell rm -rf meson-private meson-info meson-logs)
> +else
> +export NINJA
>  endif
>  ifeq ($(wildcard build.ninja),)
>  .PHONY: config-host.mak
> @@ -100,31 +102,46 @@ endif
>
>  # 1. ensure config-host.mak is up-to-date
>  config-host.mak: $(SRC_PATH)/configure $(SRC_PATH)/pc-bios
> $(SRC_PATH)/VERSION
> -       @echo $@ is out-of-date, running configure
> +       @echo config-host.mak is out-of-date, running configure
>         @if test -f meson-private/coredata.dat; then \
>           ./config.status --skip-meson; \
>         else \
> -         ./config.status; \
> +         ./config.status && touch build.ninja.stamp; \
>         fi
>
> -# 2. ensure generated build files are up-to-date
> +# 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
>
> -ifneq ($(NINJA),)
>  # A separate rule is needed for Makefile dependencies to avoid -n
> -export NINJA
> +ifneq ($(MESON),)
> +build.ninja: build.ninja.stamp
> +build.ninja.stamp: meson.stamp
> +       $(NINJA) $(if $V,-v,) reconfigure && touch $@
> +endif
> +
> +ifneq ($(NINJA),)
>  Makefile.ninja: build.ninja
> -       $(quiet-@){ echo 'ninja-targets = \'; $(NINJA) -t targets all |
> sed 's/:.*//; $$!s/$$/ \\/'; } > $@
> +       $(quiet-@){ \
> +         echo 'ninja-targets = \'; \
> +         $(NINJA) -t targets all | sed 's/:.*//; $$!s/$$/ \\/'; \
> +         echo 'build-files = \'; \
> +         $(NINJA) -t query build.ninja | sed -n '1,/^  input:/d; /^
> outputs:/q; s/$$/ \\/p'; \
> +       } > $@.tmp && mv $@.tmp $@
>  -include Makefile.ninja
>  endif
>
>  ifneq ($(MESON),)
> -# The dependency on config-host.mak ensures that meson has run
> -Makefile.mtest: build.ninja scripts/mtest2make.py config-host.mak
> +Makefile.mtest: build.ninja scripts/mtest2make.py
>         $(MESON) introspect --targets --tests --benchmarks | $(PYTHON)
> scripts/mtest2make.py > $@
>  -include Makefile.mtest
>  endif
>
> -# 3. Rules to bridge to other makefiles
> +# 4. Rules to bridge to other makefiles
>
>  ifneq ($(NINJA),)
>  NINJAFLAGS = $(if $V,-v,) \
> @@ -135,7 +152,10 @@ ninja-cmd-goals = $(or $(MAKECMDGOALS), all)
>  ninja-cmd-goals += $(foreach t, $(.tests), $(.test.deps.$t))
>
>  makefile-targets := build.ninja ctags TAGS cscope dist clean uninstall
> -ninja-targets := $(filter-out $(makefile-targets), $(ninja-targets))
> +# "ninja -t targets" also lists all prerequisites.  If build system
> +# files are marked as PHONY, however, Make will always try to execute
> +# "ninja reconfigure" to rebuild build.ninja.
> +ninja-targets := $(filter-out $(build-files) $(makefile-targets),
> $(ninja-targets))
>  .PHONY: $(ninja-targets) run-ninja
>  $(ninja-targets): run-ninja
>
> @@ -214,7 +234,7 @@ distclean: clean
>         rm -f qemu-plugins-ld.symbols qemu-plugins-ld64.symbols
>         rm -f *-config-target.h *-config-devices.mak *-config-devices.h
>         rm -rf meson-private meson-logs meson-info compile_commands.json
> -       rm -f Makefile.ninja Makefile.mtest
> +       rm -f Makefile.ninja Makefile.mtest build.ninja.stamp meson.stamp
>         rm -f config.log
>         rm -f linux-headers/asm
>         rm -Rf .sdk
> --
> 2.26.2
>
>

[-- Attachment #2: Type: text/html, Size: 7172 bytes --]