Target-dependent include path, why?

Target-dependent include path, why?

[Markus Armbruster]

I stumbled over this:

    ../include/ui/qemu-pixman.h:12:10: fatal error: pixman.h: No such file or directory
       12 | #include <pixman.h>
          |          ^~~~~~~~~~

Works when included into target-dependent code.

Running make -V=1 shows we're passing a number of -I only when compiling
target-dependent code, i.e. together with -DNEED_CPU_H:

    -I/usr/include/pixman-1 -I/usr/include/capstone -I/usr/include/spice-server -I/usr/include/spice-1

    -I/usr/include/cacard -I/usr/include/nss3 -I/usr/include/nspr4 -I/usr/include/PCSC

    -isystem../linux-headers -isystemlinux-headers

Why?

Re: Target-dependent include path, why?

[Richard Henderson]

On 12/8/22 23:12, Markus Armbruster wrote:
> I stumbled over this:
> 
>      ../include/ui/qemu-pixman.h:12:10: fatal error: pixman.h: No such file or directory
>         12 | #include <pixman.h>
>            |          ^~~~~~~~~~
> 
> Works when included into target-dependent code.
> 
> Running make -V=1 shows we're passing a number of -I only when compiling
> target-dependent code, i.e. together with -DNEED_CPU_H:
> 
>      -I/usr/include/pixman-1 -I/usr/include/capstone -I/usr/include/spice-server -I/usr/include/spice-1
> 
>      -I/usr/include/cacard -I/usr/include/nss3 -I/usr/include/nspr4 -I/usr/include/PCSC
> 
>      -isystem../linux-headers -isystemlinux-headers
> 
> Why?

Because of where [pixman] is added as a dependency in meson.build.
If you want to use it somewhere new, you've got to move the dependency.


r~

Re: Target-dependent include path, why?

[Philippe Mathieu-Daudé]

On 9/12/22 06:24, Richard Henderson wrote:
> On 12/8/22 23:12, Markus Armbruster wrote:
>> I stumbled over this:
>>
>>      ../include/ui/qemu-pixman.h:12:10: fatal error: pixman.h: No such 
>> file or directory
>>         12 | #include <pixman.h>
>>            |          ^~~~~~~~~~
>>
>> Works when included into target-dependent code.
>>
>> Running make -V=1 shows we're passing a number of -I only when compiling
>> target-dependent code, i.e. together with -DNEED_CPU_H:
>>
>>      -I/usr/include/pixman-1 -I/usr/include/capstone 
>> -I/usr/include/spice-server -I/usr/include/spice-1
>>
>>      -I/usr/include/cacard -I/usr/include/nss3 -I/usr/include/nspr4 
>> -I/usr/include/PCSC
>>
>>      -isystem../linux-headers -isystemlinux-headers
>>
>> Why?
> 
> Because of where [pixman] is added as a dependency in meson.build.
> If you want to use it somewhere new, you've got to move the dependency.

Code involving virtio becomes target-specific.

Re: Target-dependent include path, why?

[Markus Armbruster]

Richard Henderson <richard.henderson@linaro.org> writes:

> On 12/8/22 23:12, Markus Armbruster wrote:
>> I stumbled over this:
>>      ../include/ui/qemu-pixman.h:12:10: fatal error: pixman.h: No such file or directory
>>         12 | #include <pixman.h>
>>            |          ^~~~~~~~~~
>> Works when included into target-dependent code.
>> Running make -V=1 shows we're passing a number of -I only when compiling
>> target-dependent code, i.e. together with -DNEED_CPU_H:
>>      -I/usr/include/pixman-1 -I/usr/include/capstone -I/usr/include/spice-server -I/usr/include/spice-1
>>      -I/usr/include/cacard -I/usr/include/nss3 -I/usr/include/nspr4 -I/usr/include/PCSC
>>      -isystem../linux-headers -isystemlinux-headers
>> Why?
>
> Because of where [pixman] is added as a dependency in meson.build.

Is it added where it is for a reason, or is it accidental?

> If you want to use it somewhere new, you've got to move the dependency.

Re: Target-dependent include path, why?

[Richard Henderson]

On 12/9/22 02:51, Markus Armbruster wrote:
> Richard Henderson <richard.henderson@linaro.org> writes:
> 
>> On 12/8/22 23:12, Markus Armbruster wrote:
>>> I stumbled over this:
>>>       ../include/ui/qemu-pixman.h:12:10: fatal error: pixman.h: No such file or directory
>>>          12 | #include <pixman.h>
>>>             |          ^~~~~~~~~~
>>> Works when included into target-dependent code.
>>> Running make -V=1 shows we're passing a number of -I only when compiling
>>> target-dependent code, i.e. together with -DNEED_CPU_H:
>>>       -I/usr/include/pixman-1 -I/usr/include/capstone -I/usr/include/spice-server -I/usr/include/spice-1
>>>       -I/usr/include/cacard -I/usr/include/nss3 -I/usr/include/nspr4 -I/usr/include/PCSC
>>>       -isystem../linux-headers -isystemlinux-headers
>>> Why?
>>
>> Because of where [pixman] is added as a dependency in meson.build.
> 
> Is it added where it is for a reason, or is it accidental?

I don't know for sure, but I doubt it's accidental.
I'm sure it is where it is based on existing usage.


r~

Re: Target-dependent include path, why?

[Philippe Mathieu-Daudé]

On 9/12/22 16:02, Richard Henderson wrote:
> On 12/9/22 02:51, Markus Armbruster wrote:
>> Richard Henderson <richard.henderson@linaro.org> writes:
>>
>>> On 12/8/22 23:12, Markus Armbruster wrote:
>>>> I stumbled over this:
>>>>       ../include/ui/qemu-pixman.h:12:10: fatal error: pixman.h: No 
>>>> such file or directory
>>>>          12 | #include <pixman.h>
>>>>             |          ^~~~~~~~~~
>>>> Works when included into target-dependent code.
>>>> Running make -V=1 shows we're passing a number of -I only when 
>>>> compiling
>>>> target-dependent code, i.e. together with -DNEED_CPU_H:
>>>>       -I/usr/include/pixman-1 -I/usr/include/capstone 
>>>> -I/usr/include/spice-server -I/usr/include/spice-1
>>>>       -I/usr/include/cacard -I/usr/include/nss3 -I/usr/include/nspr4 
>>>> -I/usr/include/PCSC
>>>>       -isystem../linux-headers -isystemlinux-headers
>>>> Why?
>>>
>>> Because of where [pixman] is added as a dependency in meson.build.
>>
>> Is it added where it is for a reason, or is it accidental?
> 
> I don't know for sure, but I doubt it's accidental.
> I'm sure it is where it is based on existing usage.

I hit a similar problem while converting units to be target-independent,
moving them out of specific_ss[] I got:

  In file included from ../monitor/misc.c:38:
  In file included from include/ui/console.h:4:
  include/ui/qemu-pixman.h:12:10: fatal error: 'pixman.h' file not found
  #include <pixman.h>
           ^~~~~~~~~~

The monitor/ dependency is odd because there is no pixman use in the
sources, only pulled by the headers:

   $ git grep pixman monitor/
   monitor/meson.build:9:specific_ss.add(when: 'CONFIG_SOFTMMU', 
if_true: [files('misc.c'), spice, pixman])

We require pixman for system emulation. The dependency is added in 
ui/meson.build:

   softmmu_ss.add(pixman)
   specific_ss.add(when: ['CONFIG_SOFTMMU'], if_true: pixman)   # for 
the include path

Due to the second line, I was expecting all files in specific_ss[] to 
have pixman path in their CPPFLAGS. The monitor/meson.build has:

   specific_ss.add(when: 'CONFIG_SOFTMMU', if_true: [files('misc.c'), 
spice])

Paolo, is it due to the [] that monitor/misc.c doesn't get the pixman
pkg-config flags?

Re: Target-dependent include path, why?

[Paolo Bonzini]

On 12/9/22 09:51, Markus Armbruster wrote:
>> Because of where [pixman] is added as a dependency in meson.build.
>
> Is it added where it is for a reason, or is it accidental?

Dependencies are usually added near the .c files that use them.  That's 
a bit messy of course if you have an "#include <>" in a heavily-included 
QEMU header.  You can consider it a way to discourage heavily-included 
headers.

If you have a dependency in multiple unrelated .c files, specifying them 
in multiple foo_ss.add() invocations doesn't hurt.  In fact it is both 
clearer and more compact, because it removes the need for "if"s.

The only dependency that you don't need to specify is glib, partly for 
historical pre-Meson reasons partly because it would be everywhere.  For 
the others, if it makes sense to add them to multiple source sets you're 
welcome to do so.

Paolo

Re: Target-dependent include path, why?

[Peter Maydell]

On Fri, 9 Dec 2022 at 16:49, Paolo Bonzini <pbonzini@redhat.com> wrote:
>
> On 12/9/22 09:51, Markus Armbruster wrote:
> >> Because of where [pixman] is added as a dependency in meson.build.
> >
> > Is it added where it is for a reason, or is it accidental?
>
> Dependencies are usually added near the .c files that use them.  That's
> a bit messy of course if you have an "#include <>" in a heavily-included
> QEMU header.  You can consider it a way to discourage heavily-included
> headers.

This has always seemed to me to be a recipe for bugs that only
show up in the uncommon case of "some dependent library's
header files have been installed somewhere other than in
a system include directory". Is it possible to get meson to do
things the more standard way, where if a binary has a dependency
declared then the CFLAGS for that dependency get used for all
objects that go into it?

thanks
-- PMM

Re: Target-dependent include path, why?

[Paolo Bonzini]

On 12/9/22 17:52, Peter Maydell wrote:
>> Dependencies are usually added near the .c files that use them.  That's
>> a bit messy of course if you have an "#include <>" in a heavily-included
>> QEMU header.  You can consider it a way to discourage heavily-included
>> headers.
> This has always seemed to me to be a recipe for bugs that only
> show up in the uncommon case of "some dependent library's
> header files have been installed somewhere other than in
> a system include directory". Is it possible to get meson to do
> things the more standard way, where if a binary has a dependency
> declared then the CFLAGS for that dependency get used for all
> objects that go into it?

This *is* what Meson does, it's QEMU that has always done non-standard 
things in order to share the .o files for target-independent sources. 
Back in the day is was -Wl,--whole-archive, then it became foo-obj-y. 
Now it's foo_ss but it's the same thing as foo-obj-y in the end.

Once the relation between objects and binaries becomes many-to-many, you 
can't really apply "the CFLAGS for the binaries' dependencies" to all 
the objects.  Pre-Meson, there were three ways to declare dependencies:

- placing pkg-config output directly in $(QEMU_CFLAGS) and $(LIBS). 
This caused binaries to have unnecessary dependencies at times.

- mentioning dependencies in $(obj)/foo.o_{CFLAGS,LIBS} or something 
like that, declaring dependencies in objects and applying them to 
binaries.  The Makefile implementation was very buggy.

- a mix of the two, with the include path added to QEMU_CFLAGS and a 
target variable definition "foo$(EXESUF): LIBS += ..." that avoided the 
unnecessary dependencies.

The sourceset thing was added to Meson specifically for QEMU, inspired 
by the second option.  Without the bugs[1], everything could become 
fine-grained.  Only glib stayed in QEMU_CFLAGS (the third option); 
anything else was unnecessary because everything includes glib.h through 
osdep.h anyway.

The closest thing to what you're suggesting is to keep LIBS fine-grained 
while making CFLAGS coarse-grained, i.e. the third option above.  That 
is what the patches I sent today do when moving the glib tests to Meson, 
so it is not hard to expand it to other dependencies; but while it might 
avoid the gnutls issues, it will probably cause other issues---think of 
SDL messing with "#define main".  Overall, I'm not sure it's a win.

Thanks,

Paolo

[1] there is the gnutls issue on macOS, but IIRC the dependency CFLAGS 
are just the manifestation of a bug somewhere else that I forgot the 
details about (the Meson issue tracker and wiki page has the info, I'll 
get back to it some day).  This bug only affects gnutls because we do 
tend to "#include <>" in .c files only, and did so even before the 
switch to Meson.

Re: Target-dependent include path, why?

[Peter Maydell]

On Fri, 9 Dec 2022 at 17:42, Paolo Bonzini <pbonzini@redhat.com> wrote:
>
> On 12/9/22 17:52, Peter Maydell wrote:
> >> Dependencies are usually added near the .c files that use them.  That's
> >> a bit messy of course if you have an "#include <>" in a heavily-included
> >> QEMU header.  You can consider it a way to discourage heavily-included
> >> headers.
> > This has always seemed to me to be a recipe for bugs that only
> > show up in the uncommon case of "some dependent library's
> > header files have been installed somewhere other than in
> > a system include directory". Is it possible to get meson to do
> > things the more standard way, where if a binary has a dependency
> > declared then the CFLAGS for that dependency get used for all
> > objects that go into it?
>
> This *is* what Meson does, it's QEMU that has always done non-standard
> things in order to share the .o files for target-independent sources.
> Back in the day is was -Wl,--whole-archive, then it became foo-obj-y.
> Now it's foo_ss but it's the same thing as foo-obj-y in the end.
>
> Once the relation between objects and binaries becomes many-to-many, you
> can't really apply "the CFLAGS for the binaries' dependencies" to all
> the objects.  Pre-Meson, there were three ways to declare dependencies:
>
> - placing pkg-config output directly in $(QEMU_CFLAGS) and $(LIBS).
> This caused binaries to have unnecessary dependencies at times.

Yeah, this is what I think of as "the standard thing".

> - mentioning dependencies in $(obj)/foo.o_{CFLAGS,LIBS} or something
> like that, declaring dependencies in objects and applying them to
> binaries.  The Makefile implementation was very buggy.
>
> - a mix of the two, with the include path added to QEMU_CFLAGS and a
> target variable definition "foo$(EXESUF): LIBS += ..." that avoided the
> unnecessary dependencies.
>
> The sourceset thing was added to Meson specifically for QEMU, inspired
> by the second option.  Without the bugs[1], everything could become
> fine-grained.  Only glib stayed in QEMU_CFLAGS (the third option);
> anything else was unnecessary because everything includes glib.h through
> osdep.h anyway.
>
> The closest thing to what you're suggesting is to keep LIBS fine-grained
> while making CFLAGS coarse-grained, i.e. the third option above.  That
> is what the patches I sent today do when moving the glib tests to Meson,
> so it is not hard to expand it to other dependencies; but while it might
> avoid the gnutls issues, it will probably cause other issues---think of
> SDL messing with "#define main".  Overall, I'm not sure it's a win.

The thing I find counterintuitive about what we have currently
is that I can add a #include of a QEMU-internal header to a
source file, and now the build can be broken on some host
system configurations.

I'd be happier with either:
 (1) it's always safe to #include QEMU's own headers in its
     source files
 (2) sometimes a new QEMU header #include requires you to add a
     dependency to a meson.build file, but if you forget to do
     this then the build reliably fails on *all* host systems

thanks
-- PMM

Re: Target-dependent include path, why?

[Paolo Bonzini]

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

Il ven 9 dic 2022, 20:54 Peter Maydell <peter.maydell@linaro.org> ha
scritto:

> > - placing pkg-config output directly in $(QEMU_CFLAGS) and $(LIBS).
> > This caused binaries to have unnecessary dependencies at times.
>
> Yeah, this is what I think of as "the standard thing".
>

Got it, and it wasn't exactly what QEMU was doing. There was at least
libs_softmmu, libs_qga. So in practice it would be more similar to the
other one:

> - a mix of the two, with the include path added to QEMU_CFLAGS and a
> > target variable definition "foo$(EXESUF): LIBS += ..." that avoided the
> > unnecessary dependencies.
>

which meson does support.

However the issue you mention below is indeed the gnutls bug, and it can be
fixed. If I recall correctly it was meant to be a temporary workaround for
the actual bug.

I need to check again but I recall I had two fixes in mind, one was a more
risky change in Meson, the other was a new declare_dependency(..., objects:
...) argument to be used instead of link_whole. link_whole was used before
but it didn't work for some reason, maybe something to do with static
linking.

I was hoping to stop the Meson upgrades at 0.63, but I agree that it is
messy and it would be a good reason for another bump in the future.

Paolo

The thing I find counterintuitive about what we have currently
> is that I can add a #include of a QEMU-internal header to a
> source file, and now the build can be broken on some host
> system configurations.
>



Paolo


> I'd be happier with either:
>  (1) it's always safe to #include QEMU's own headers in its
>      source files
>  (2) sometimes a new QEMU header #include requires you to add a
>      dependency to a meson.build file, but if you forget to do
>      this then the build reliably fails on *all* host systems
>
> thanks
> -- PMM
>
>

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