Re: [Qemu-devel] [PATCH 4/8] convert libqemuutil to meson

[Paolo Bonzini <pbonzini@redhat.com>]

On 29/07/19 09:09, Markus Armbruster wrote:
> Peter Maydell <peter.maydell@linaro.org> writes:
> 
>> On Sat, 27 Jul 2019 at 13:24, Paolo Bonzini <pbonzini@redhat.com> wrote:
>>>
>>> On 27/07/19 09:16, Markus Armbruster wrote:
>>>> We started with a single trace-events.  That wasn't good, so we split it
>>>> up into one per directory.  That isn't good, so what about splitting it
>>>> up into one per source file?  Pass -DTRACE_HEADER='"trace-DIR-FOO.h"
>>>> instead of -DTRACE_HEADER='"trace-DIR.h"' when compiling DIR/FOO.c.
>>>
>>> For Make this would all work great, however not for Meson because it
>>> doesn't allow per-file compile flags.
>>
>> Apologies for randomly parachuting into this email thread, but if
>> Meson doesn't support per-file compile flags then what's the plan
>> for handling the cases where we currently need per-file compile flags ?
>> (This is one of the things that I thought was quite a nice move
>> forward in our make infrastructure -- we now have clean syntax
>> for saying "these files need to be built with these warnings disabled
>> or these extra include paths or whatever" and also "these files
>> imply we're going to need to link against library X".)
> 
> I vaguely remember from my review that Meson lets us express "these
> files imply we're going to need to link against library X" even more
> clearly.  I can't point you to an example, though.

Yes, you can do just

   common_ss.add(when: curl, if_true: files('curl.c'))

as a replacement for

   common-obj-$(CONFIG_CURL) += curl.c
   curl.c-cflags = $(CURL_CFLAGS)
   curl.c-libs = $(CURL_LIBS)


If conditional compilation is handled inside the file, i.e the same but
with common-obj-y, you can instead do

   common_ss.add(files('vl.c'), sdl)


In both cases, the cflags from the dependency are applied to the whole
target, rather than just to repectively curl.c and vl.c.  So you do lose
the possibility of specifying cflags only for a file.

There is no case where we're using per-.o file CFLAGS for anything other
than dependencies.  I was using per-file -f... options in the
(not-yet-applied) CET series though.  I would have to check whether
pragmas work in that case (if they do, I feel they are superior to
specifying CFLAGS in the Makefile).

>>> Meson maintainers suggest building a static library for each special set
>>> of compile flags; we could do that automatically per-directory(*) but it
>>> would be harder to scale that to per-file.
> 
> This is clearly not "minimal fuss", not even per directory, let alone
> per file.
> 
> It's pretty lame even for the large sets we have (per target,
> target-independent): I guess we'd either throw away the .a unused, or
> link with --wholearchive.

> For me, forwarding headers are just fine for a PoC, quite tolerable
> while a conversion is in progress, and perhaps even tolerable as a
> permanent work-around.  My *actual* question is how we could do per-file
> rather than per-directory trace.h with Meson.  Quoting myself:
> 
>     We have trace-events with hundreds of tracepoints for tens of source
>     files.  The generated trace.h clock in at hundreds of KiB for me.
>     Messing with tracepoints in one source file recompiles most of the
>     directory.  This is so much better than it used to be, but clearly not
>     as good as it could be.
> 
>     The worst of the lot is trace-root.h, which gets included for >1300 .o
>     in my "build everything" tree, mostly because it contains tracepoints
>     for static inline functions in widely included headers.  See also
>     "[PATCH 07/28] trace: Do not include qom/cpu.h into generated trace.h",
>     Message-Id: <20190726120542.9894-8-armbru@redhat.com>.
> 
>     We started with a single trace-events.  That wasn't good, so we split it
>     up into one per directory.  That isn't good, so what about splitting it
>     up into one per source file?
> 
> Any ideas?

Per-file (really per-device) would really be easier than per-directory,
I think, because with fine-grained trace-events there is a point in
including a specific header like

   #include "trace-mptsas.h"

or even

   #include "trace/trace-mptsas.h"

(possibly generated from trace-events-mptsas).  The only constraint
would be that the subsystem names would have to be global across all of
QEMU.  If it's per-directory as it is now, instead, you'd have something
like

   #include "trace/trace-hw_scsi.h"

which is really ugly and then forwarding headers look better.

Paolo