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

[Markus Armbruster]

Paolo Bonzini <pbonzini@redhat.com> writes:

> 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.

Appreciated!

> 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.

A Make replacement must address its issues to be credible.

Make's traditional inability to directly express "rule updates multiple
files" and "rule may or may not actually change its target(s)" has
always been an issue.  Countless Make users have had to learn the stamp
file work-around, and how to cope with its drawbacks.  I distinctly
remember grappling with it back in the 90s.

> (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.

This is *essential* when generating headers.  Without it, touching any
QAPI module or any part of the QAPI generator recompiles pretty much
everything.

>                                                               Grouped
> targets don't have a way to do with that).

Yes.  Also, TIL grouped targets :)

> 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.

Neat trick!

Now I remember the old Makefiles actually didn't recover all by
themselves either, because they didn't use this trick.  Manual recovery
was "easy", though: remove the stamp file.

I ran into the "can't recover from deletion of generated QAPI file(s)"
issue when I updated a silly script of mine that helps me diff them.  To
force regeneration, the script removes them (relying on ccache to keep
compilation time in check).  Pre-Meson, it removed the stamp files.  I
didn't see any in meson.build, so I dropped their removal, nothing
worked, and I got quite confused.

> 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.

Makes sense to me.

> 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.

Yes.  Drop in the bucket?

We already depend on ninja-the-language, we just feed it to ninja2make
instead of ninja.

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

Thank you very much for taking the time to explain!