more bogus meson warnings

more bogus meson warnings

[Peter Maydell]

I tried my static-system build config today; meson bogus
complaints include:

WARNING: Static library 'pulse' not found for dependency 'libpulse',
may not be statically linked
WARNING: Static library 'pulsecommon-11.1' not found for dependency
'libpulse', may not be statically linked
WARNING: Static library 'asound' not found for dependency 'alsa', may
not be statically linked
WARNING: Static library 'spice-server' not found for dependency
'spice-server', may not be statically linked
WARNING: Static library 'cacard' not found for dependency 'libcacard',
may not be statically linked

We asked for static linking, this should result in
"OK, no static library present, ignore optional feature",
not "decide we found the library and emit a warning message".

Has header "snappy-c.h" : YES
Library snappy found: YES
../../meson.build:1141: WARNING: could not link libsnappy, disabling
Has header "lzo/lzo1x.h" : YES
Library lzo2 found: YES
../../meson.build:1158: WARNING: could not link liblzo2, disabling

meson should just decide it doesn't have snappy and lzo2,
without emitting the warning.

thanks
-- PMM

Re: more bogus meson warnings

[Peter Maydell]

On Tue, 2 Nov 2021 at 13:38, Peter Maydell <peter.maydell@linaro.org> wrote:
>
> I tried my static-system build config today; meson bogus
> complaints include:
>
> WARNING: Static library 'pulse' not found for dependency 'libpulse',
> may not be statically linked
> WARNING: Static library 'pulsecommon-11.1' not found for dependency
> 'libpulse', may not be statically linked
> WARNING: Static library 'asound' not found for dependency 'alsa', may
> not be statically linked

...also this configure passed "--audio-drv-list=" so we
shouldn't be probing for pulse or asound at all.

The resulting build fails at link time because meson is
incorrectly trying to link dynamic libraries into a static
executable.

-- PMM

Re: more bogus meson warnings

[Paolo Bonzini]

On Tue, Nov 2, 2021 at 2:38 PM Peter Maydell <peter.maydell@linaro.org> wrote:
> I tried my static-system build config today; meson bogus
> complaints include:
>
> WARNING: Static library 'pulse' not found for dependency 'libpulse',
> may not be statically linked
> WARNING: Static library 'pulsecommon-11.1' not found for dependency
> 'libpulse', may not be statically linked
> WARNING: Static library 'asound' not found for dependency 'alsa', may
> not be statically linked
> WARNING: Static library 'spice-server' not found for dependency
> 'spice-server', may not be statically linked
> WARNING: Static library 'cacard' not found for dependency 'libcacard',
> may not be statically linked
>
> We asked for static linking, this should result in
> "OK, no static library present, ignore optional feature",
> not "decide we found the library and emit a warning message".

I think I explained this already, but what happens is that:

1) Meson retrieves the flags from pkg-config. It sanity checks the -l
flags, and cannot find the library. It prints a warning because maybe
it's looking in the wrong places, the path to the library might be
provided by --extra-ldflags or another place that is not in gcc
--print-search-dirs.

2) Therefore the root cause is that a .pc file is present but not
suitable for static linking. With the test in configure, the problem
would still be there, except it would not be detected and would just
break at the time of the final link.

So, in general, you _already_ have an improvement over what was there before.

For the specific case of Spice, you didn't need a separate
--disable-spice argument because Spice did have a "compile a test
program" step in configure, unlike basically every other $pkg_config
invocation. Marc-André didn't bring it over to Meson, and that was
fine with me because (IMO) this kind of slight difference between one
dependency and all the others is just a maintenance hassle.

> Has header "snappy-c.h" : YES
> Library snappy found: YES
> ../../meson.build:1141: WARNING: could not link libsnappy, disabling
> Has header "lzo/lzo1x.h" : YES
> Library lzo2 found: YES
> ../../meson.build:1158: WARNING: could not link liblzo2, disabling
>
> meson should just decide it doesn't have snappy and lzo2,
> without emitting the warning.

These are two different tests:

- the "header present" and "library present" tests are done with cc.find_library

- the "does a basic program" link is done with cc.links.

The warning is emitted in the case where the files are present but the
test program fails. Of course, Meson doesn't have a crystal ball for
the cc.links test when it says that the static lzo2 library is there
(apparently it is).

I have tried "../configure --without-default-features --enable-snappy
--static" and it correctly fails with "../meson.build:1110:2: ERROR: C
static library 'snappy' not found". Likewise, "../configure --static"
prints dozen of warnings of the previous kind for missing static
libraries in pkg-config files; for snappy, however, it correctly
prints

Has header "snappy-c.h" : YES
Library snappy found: NO

without any warning. If you send me your config-host.mak and
meson-logs/meson-log.txt I can check what's going on here. If you
would like a different wording such as "could not link liblzo2 test
program, disabling", that can be done too, of course.

Paolo

Re: more bogus meson warnings

[Peter Maydell]

On Wed, 3 Nov 2021 at 08:01, Paolo Bonzini <pbonzini@redhat.com> wrote:
>
> On Tue, Nov 2, 2021 at 2:38 PM Peter Maydell <peter.maydell@linaro.org> wrote:
> > I tried my static-system build config today; meson bogus
> > complaints include:
> >
> > WARNING: Static library 'pulse' not found for dependency 'libpulse',
> > may not be statically linked
> > WARNING: Static library 'pulsecommon-11.1' not found for dependency
> > 'libpulse', may not be statically linked
> > WARNING: Static library 'asound' not found for dependency 'alsa', may
> > not be statically linked
> > WARNING: Static library 'spice-server' not found for dependency
> > 'spice-server', may not be statically linked
> > WARNING: Static library 'cacard' not found for dependency 'libcacard',
> > may not be statically linked
> >
> > We asked for static linking, this should result in
> > "OK, no static library present, ignore optional feature",
> > not "decide we found the library and emit a warning message".
>
> I think I explained this already, but what happens is that:
>
> 1) Meson retrieves the flags from pkg-config. It sanity checks the -l
> flags, and cannot find the library. It prints a warning because maybe
> it's looking in the wrong places, the path to the library might be
> provided by --extra-ldflags or another place that is not in gcc
> --print-search-dirs.
>
> 2) Therefore the root cause is that a .pc file is present but not
> suitable for static linking. With the test in configure, the problem
> would still be there, except it would not be detected and would just
> break at the time of the final link.

This is not my experience. I find that:
 * test in configure: configure doesn't enable the thing
 * test in meson: meson produces a WARNING, but goes ahead anyway,
   and then the final link fails

> So, in general, you _already_ have an improvement over what was there before.

Well, in practice what happened was that before the recent changes
configure correctly didn't put dynamic libraries into the link line,
and so my configure options resulted in a successful build.
After these recent changes to move stuff from configure to meson,
meson is putting dynamic libraries into the link line, and the
build fails until I add extra --disable-foo to the configure arguments.
This is not what I would consider an improvement.

> For the specific case of Spice, you didn't need a separate
> --disable-spice argument because Spice did have a "compile a test
> program" step in configure, unlike basically every other $pkg_config
> invocation. Marc-André didn't bring it over to Meson, and that was
> fine with me because (IMO) this kind of slight difference between one
> dependency and all the others is just a maintenance hassle.
>
> > Has header "snappy-c.h" : YES
> > Library snappy found: YES
> > ../../meson.build:1141: WARNING: could not link libsnappy, disabling
> > Has header "lzo/lzo1x.h" : YES
> > Library lzo2 found: YES
> > ../../meson.build:1158: WARNING: could not link liblzo2, disabling
> >
> > meson should just decide it doesn't have snappy and lzo2,
> > without emitting the warning.
>
> These are two different tests:
>
> - the "header present" and "library present" tests are done with cc.find_library
>
> - the "does a basic program" link is done with cc.links.
>
> The warning is emitted in the case where the files are present but the
> test program fails. Of course, Meson doesn't have a crystal ball for
> the cc.links test when it says that the static lzo2 library is there
> (apparently it is).

They're two tests under the hood, but they both need to pass
for us to be able to use the feature. If they don't both pass,
then meson should just quietly say "OK, we don't have this thing"
(ie it could print a "Something or other: NO" line, but it should
not be printing a "WARNING" line).

> I have tried "../configure --without-default-features --enable-snappy
> --static" and it correctly fails with "../meson.build:1110:2: ERROR: C
> static library 'snappy' not found". Likewise, "../configure --static"
> prints dozen of warnings of the previous kind for missing static
> libraries in pkg-config files; for snappy, however, it correctly
> prints
>
> Has header "snappy-c.h" : YES
> Library snappy found: NO
>
> without any warning. If you send me your config-host.mak and
> meson-logs/meson-log.txt I can check what's going on here. If you
> would like a different wording such as "could not link liblzo2 test
> program, disabling", that can be done too, of course.

I just want meson to follow the convention that we have had for
years, which is:
 * if I say --enable-foo, then failing to find foo should be an
   error
 * if I say --disable-foo, then don't probe for foo at all
 * if I say nothing, then probe for the various things we need to
   enable the foo feature, and if they're not presentor not usable
   for some reason then just quietly don't enable the foo option

I specifically do *not* want meson to print anything saying
"WARNING" for case 3, because this should be a fairly normal
state of affairs.

thanks
-- PMM

Re: more bogus meson warnings

[Paolo Bonzini]

On 11/3/21 13:19, Peter Maydell wrote:
> This is not my experience. I find that:
>   * test in configure: configure doesn't enable the thing
>   * test in meson: meson produces a WARNING, but goes ahead anyway,
>     and then the final link fails
> 
>> So, in general, you _already_ have an improvement over what was there before.
> 
> Well, in practice what happened was that before the recent changes
> configure correctly didn't put dynamic libraries into the link line,
> and so my configure options resulted in a successful build.

That depends on the individual test.  There are three different cases:

- pkg-config without compile test: in that case you were already 
disabling everything.  You didn't see the improvement (WARN at 
configure/meson time vs. failure at build time) just because you already 
had the link failure years ago

- pkg-config with compile test: this was the case for a handful of 
libraries (spice, epoxy, virglrenderer, gnutls/nettle).  Here indeed 
it's not an improvement in your experience.  On the other hand, I don't 
see any reason to do this for 3-4 libraries out of the dozens that we test.

- library + compile test: the warning is likely not bogus

>> The warning is emitted in the case where the files are present but the
>> test program fails. Of course, Meson doesn't have a crystal ball for
>> the cc.links test when it says that the static lzo2 library is there
>> (apparently it is).
> 
> They're two tests under the hood, but they both need to pass
> for us to be able to use the feature. If they don't both pass,
> then meson should just quietly say "OK, we don't have this thing"
> (ie it could print a "Something or other: NO" line, but it should
> not be printing a "WARNING" line).

Of course it's possible to downgrade the warning line, but I'd rather 
first be sure that the warning is bogus.  If you _do_ have a header and 
static library, but somehow it cannot be used to link the test program, 
it would be correct to warn.  Unlike the configure script, Meson does 
have code to distinguish static vs shared libraries, so the compile test 
should be unnecessary; I would like to understand what causes it to 
fail, so that your system says "Library snappy found: YES" (and warns), 
whereas mine says "no".  I can check that using config-host.mak and 
meson-logs/meson-log.txt.

Paolo

> I just want meson to follow the convention that we have had for
> years, which is:
>   * if I say --enable-foo, then failing to find foo should be an
>     error
>   * if I say --disable-foo, then don't probe for foo at all
>   * if I say nothing, then probe for the various things we need to
>     enable the foo feature, and if they're not presentor not usable
>     for some reason then just quietly don't enable the foo option
> 
> I specifically do *not* want meson to print anything saying
> "WARNING" for case 3, because this should be a fairly normal
> state of affairs.

Re: more bogus meson warnings

[Peter Maydell]

On Wed, 3 Nov 2021 at 13:38, Paolo Bonzini <pbonzini@redhat.com> wrote:
> On 11/3/21 13:19, Peter Maydell wrote:
> > They're two tests under the hood, but they both need to pass
> > for us to be able to use the feature. If they don't both pass,
> > then meson should just quietly say "OK, we don't have this thing"
> > (ie it could print a "Something or other: NO" line, but it should
> > not be printing a "WARNING" line).
>
> Of course it's possible to downgrade the warning line, but I'd rather
> first be sure that the warning is bogus.  If you _do_ have a header and
> static library, but somehow it cannot be used to link the test program,
> it would be correct to warn.

I don't really agree. There are lots and lots of things that
might put the user's system somewhere between "there is no
evidence of libfoo here at all" and "there is a working libfoo
we can use". We don't in general try to diagnose any of those,
we just say "no libfoo, move on".

> Unlike the configure script, Meson does
> have code to distinguish static vs shared libraries, so the compile test
> should be unnecessary; I would like to understand what causes it to
> fail, so that your system says "Library snappy found: YES" (and warns),
> whereas mine says "no".  I can check that using config-host.mak and
> meson-logs/meson-log.txt.

https://people.linaro.org/~peter.maydell/meson-log.txt
https://people.linaro.org/~peter.maydell/config-host.mak

The link of the test program against libsnappy.a fails because
it is being linked with cc and nothing is putting the C++ stdlib
that libsnappy needs on the link line, so you get errors like:

 /usr/lib/gcc/x86_64-linux-gnu/7/../../../x86_64-linux-gnu/libsnappy.a(snappy.cc.o):
In function `snappy::internal::WorkingMemory::GetHashTable(unsigned
long, int*)':
(.text+0x15b): undefined reference to `operator new[](unsigned long)'

This is (I suspect) another example of "distro does ship a
static .a file, but the shipped pkg-config file is only good for
dynamic linking, doesn't work for static linking". There's generally
nothing much the user can do about this, beyond file a bug report with
the distro if they're feeling enthusiastic.
("pkg-config --static --libs snappy" just outputs "-lsnappy".)

Note that Debian/Ubuntu pretty much always ships the .a
file in the libfoo-dev package, so "there's a .a file on the
disk but the pkg-config might or might not work" is pretty
common.

-- PMM

Re: more bogus meson warnings

[Paolo Bonzini]

On 11/4/21 12:02, Peter Maydell wrote:
> The link of the test program against libsnappy.a fails because
> it is being linked with cc and nothing is putting the C++ stdlib
> that libsnappy needs on the link line, so you get errors like:
> 
>   /usr/lib/gcc/x86_64-linux-gnu/7/../../../x86_64-linux-gnu/libsnappy.a(snappy.cc.o):
> In function `snappy::internal::WorkingMemory::GetHashTable(unsigned
> long, int*)':
> (.text+0x15b): undefined reference to `operator new[](unsigned long)'

That's interesting.  So there was also a bug before moving the test
to meson; statically-linked binaries would not use snappy even if
linking would be done by a C++ compiler.  This should fix it:

diff --git a/meson.build b/meson.build
index e330438270..cbc5c7a868 100644
--- a/meson.build
+++ b/meson.build
@@ -197,6 +197,9 @@ add_project_arguments('-iquote', '.',
  link_language = meson.get_external_property('link_language', 'cpp')
  if link_language == 'cpp'
    add_languages('cpp', required: true, native: false)
+  linker = cxx = meson.get_compiler('cpp')
+else
+  linker = cc
  endif
  if host_machine.system() == 'darwin'
    add_languages('objc', required: false, native: false)
@@ -1111,7 +1114,7 @@ if not get_option('snappy').auto() or have_system
                             required: get_option('snappy'),
                             kwargs: static_kwargs)
  endif
-if snappy.found() and not cc.links('''
+if snappy.found() and not linker.links('''
     #include <snappy-c.h>
     int main(void) { snappy_max_compressed_length(4096); return 0; }''', dependencies: snappy)
    snappy = not_found


> There's generally
> nothing much the user can do about this, beyond file a bug report with
> the distro if they're feeling enthusiastic.

It could be filed in snappy as well, but yeah I'm not feeling
enthusiastic about that.  Besides in this case we're not even using the
.pc file, but rather looking for -lsnappy by hand.

Paolo