Re: Meson can't recover from deletion of generated QAPI file(s)

[Paolo Bonzini]

On 09/09/20 14:37, Markus Armbruster wrote:
> Watch this:
> 
>     $ rm qapi/qapi-types-error.h 
>     $ make
>     Generating qemu-version.h with a meson_exe.py custom command
>     Compiling C object qom/libqom.fa.p/object_interfaces.c.o
>     In file included from ../qom/object_interfaces.c:4:
>     /work/armbru/qemu/include/qapi/error.h:275:10: fatal error: 
> qapi/qapi-types-error.h: No such file or directory
>       275 | #include "qapi/qapi-types-error.h"
>           |          ^~~~~~~~~~~~~~~~~~~~~~~~~
>     compilation terminated.
>     make: *** [Makefile.ninja:348: qom/libqom.fa.p/object_interfaces.c.o] 
> Error 1
> 
> To recover, I have to run qapi-gen.py manually and exactly right, or
> blow away the build tree and start over.
> 
> The old build system did what a build system should: it remade the files
> that are missing or out of date.
> 
> I'm still too clueless about Meson to debug this, but I hope I can learn
> from watching you fix it.

Unfortunately you won't learn much about Meson, you would learn that 
Make is messy but you don't need that.  You can also learn a little bit 
about the new design of the QEMU build system though, so I'll explain 
and not just send a patch.

The bad news this tells you about the build system is that, when 
debugging an issue, you have to figure out if it's a bug in Meson,
in the meson.build files, or in ninja2make.  Of course the second
is the common case, but you never know especially now that there are
more people using ninja2make in anger.

Generating Makefile.ninja gets finicky because ninja (while it has other
things I don't like) is a little more expressive than Make as far as
simple build rules are concerned, therefore it doesn't need the stamp
file trick.  So while we there may be one or two more bugs like this one
down the road, ninja2make should not be an issue as soon as its teething
problems are solved.

(As an aside: the GNU Make 4.3 "grouped targets" feature can sometimes
eliminate stamp files, but it would not help here.  The stamp file has
another feature, namely the custom command can decide not to touch
its outputs if they won't change.  This avoid more rebuilds.  Grouped
targets don't have a way to do with that).

The good news is that there's an easy(ish) way to do this.  The build 
system is quite "linear" in how it works, so the first step should be to 
look at build.ninja and see what the rules are like.  Here you'd see
something like:

  build long list of files: CUSTOM_COMMAND actual prerequisites
   COMMAND = ...
   description = Generating$ shared$ QAPI$ source$ files

Your twenty-plus-years-of-writing-Makefiles spidey sense will tingle, as 
you can figure out that this is not going to be trivial to convert to 
Makefiles.  If you open Makefile.ninja you see the familiar stamp file
trick:

  long list of files: CUSTOM_COMMAND@57579de3eef.stamp; @:
  CUSTOM_COMMAND@57579de3eef.stamp: actual prerequisites
        $(ninja-command-restat)

and that's where the bug is.  If you delete one of the output files, Make
only runs ":" and does not rebuild it.  One solution is to add:

  ifneq (long list of files, $(wildcard long list of files))
  .PHONY: CUSTOM_COMMAND@57579de3eef.stamp
  endif

This way, if any of the prerequites is missing (not just older than the 
stamp file), the rule for CUSTOM_COMMAND@57579de3eef.stamp will always 
be executed.

This is fairly simple to do:

diff --git a/scripts/ninjatool.py b/scripts/ninjatool.py
index 627a1cab45..6f0e35c727 100755
--- a/scripts/ninjatool.py
+++ b/scripts/ninjatool.py
@@ -908,6 +908,9 @@ class Ninja2Make(NinjaParserEventsWithVars):
             else:
                 stamp = '%s@%s.stamp' % (rule, sha1_text(targets)[0:11])
             self.print('%s: %s; @:' % (targets, stamp))
+            self.print('ifneq (%s, $(wildcard %s))' % (targets, targets))
+            self.print('.PHONY: %s' % (stamp, ))
+            self.print('endif')
             self.print('%s: %s | %s; ${ninja-command-restat}' % (stamp, 
inputs, orderonly))
             self.rule_targets[rule].append(stamp)
             self.stamp_targets[rule].append(stamp)

To avoid this whole class of issues we could just use ninja to build QEMU
(Make would launch it, so there would still be no user-facing changes).
ninja2make's main strength was that it supported incremental conversion,
but right now all of the binaries are built by Meson therefore it's not
really *necessary* anymore.  Dropping ninja2make removes a relatively
expensive part of the build as well as a nontrivial amount of code.

Another advantage would be that ninja tracks command lines and automatically
rebuilds things if the command line has changed.  This is quite hard and
expensive to do with Make so ninja2make does not even try, but it has bitten
Philippe.

Of course, the main disadvantage is that it adds another dependency.

I will send the above patch formally soonish, but I wouldn't mind if somebody
else helped merging it.

Paolo