Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 configure   | 14 --------------
 meson.build |  7 ++++---
 2 files changed, 4 insertions(+), 17 deletions(-)

diff --git a/configure b/configure
index f9e2f3de65..6f2599f99f 100755
--- a/configure
+++ b/configure
@@ -908,7 +908,6 @@ Linux)
   linux_user="yes"
   kvm="yes"
   QEMU_INCLUDES="-isystem ${source_path}/linux-headers -Ilinux-headers $QEMU_INCLUDES"
-  libudev="yes"
 ;;
 esac
 
@@ -6287,15 +6286,6 @@ if test "$libnfs" != "no" ; then
 fi
 
 ##########################################
-# Do we have libudev
-if test "$libudev" != "no" ; then
-  if $pkg_config libudev && test "$static" != "yes"; then
-    libudev="yes"
-    libudev_libs=$($pkg_config --libs libudev)
-  else
-    libudev="no"
-  fi
-fi
 
 # Exclude --warn-common with TSan to suppress warnings from the TSan libraries.
 if test "$solaris" = "no" && test "$tsan" = "no"; then
@@ -7456,10 +7446,6 @@ if test "$gcov" = "yes" ; then
   echo "CONFIG_GCOV=y" >> $config_host_mak
 fi
 
-if test "$libudev" != "no"; then
-    echo "CONFIG_LIBUDEV=y" >> $config_host_mak
-    echo "LIBUDEV_LIBS=$libudev_libs" >> $config_host_mak
-fi
 if test "$fuzzing" != "no"; then
     echo "CONFIG_FUZZ=y" >> $config_host_mak
 fi
diff --git a/meson.build b/meson.build
index 975a187e2c..cd3d1ee260 100644
--- a/meson.build
+++ b/meson.build
@@ -257,8 +257,8 @@ if 'CONFIG_CURL' in config_host
                             link_args: config_host['CURL_LIBS'].split())
 endif
 libudev = not_found
-if 'CONFIG_LIBUDEV' in config_host
-  libudev = declare_dependency(link_args: config_host['LIBUDEV_LIBS'].split())
+if targetos == 'linux' and (have_system or have_tools)
+  libudev = dependency('libudev', static: enable_static)
 endif
 brlapi = not_found
 if 'CONFIG_BRLAPI' in config_host
@@ -440,6 +440,7 @@ has_gettid = cc.has_function('gettid')
 
 # Create config-host.h
 
+config_host_data.set('CONFIG_LIBUDEV', libudev.found())
 config_host_data.set('CONFIG_SDL', sdl.found())
 config_host_data.set('CONFIG_SDL_IMAGE', sdl_image.found())
 config_host_data.set('CONFIG_VNC', vnc.found())
@@ -1450,7 +1451,7 @@ summary_info += {'sheepdog support':  config_host.has_key('CONFIG_SHEEPDOG')}
 summary_info += {'capstone':          config_host.has_key('CONFIG_CAPSTONE')}
 summary_info += {'libpmem support':   config_host.has_key('CONFIG_LIBPMEM')}
 summary_info += {'libdaxctl support': config_host.has_key('CONFIG_LIBDAXCTL')}
-summary_info += {'libudev':           config_host.has_key('CONFIG_LIBUDEV')}
+summary_info += {'libudev':           libudev.found()}
 summary_info += {'default devices':   config_host['CONFIG_MINIKCONF_MODE'] == '--defconfig'}
 summary_info += {'plugin support':    config_host.has_key('CONFIG_PLUGIN')}
 summary_info += {'fuzzing support':   config_host.has_key('CONFIG_FUZZ')}
-- 
2.26.2

On Wed, 30 Sep 2020 at 21:01, Paolo Bonzini <pbonzini@redhat.com> wrote:
>
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
>  configure   | 14 --------------
>  meson.build |  7 ++++---
>  2 files changed, 4 insertions(+), 17 deletions(-)

Hi; this commit seems to have broken my static build.
Previously configure did not include libudev in the link
for a static build (there is no libudev.a, at least on my
system). Now it does, and then the link fails with
  /usr/bin/ld: cannot find -ludev

>  ##########################################
> -# Do we have libudev
> -if test "$libudev" != "no" ; then
> -  if $pkg_config libudev && test "$static" != "yes"; then
> -    libudev="yes"
> -    libudev_libs=$($pkg_config --libs libudev)
> -  else
> -    libudev="no"
> -  fi
> -fi

This is the old code, which doesn't enable libudev for
static builds.

> --- a/meson.build
> +++ b/meson.build
> @@ -257,8 +257,8 @@ if 'CONFIG_CURL' in config_host
>                              link_args: config_host['CURL_LIBS'].split())
>  endif
>  libudev = not_found
> -if 'CONFIG_LIBUDEV' in config_host
> -  libudev = declare_dependency(link_args: config_host['LIBUDEV_LIBS'].split())
> +if targetos == 'linux' and (have_system or have_tools)
> +  libudev = dependency('libudev', static: enable_static)
>  endif

I'm not very confident about reading meson.build logic, but it
looks like this trusts meson/pkg-config to tell it about
whether it can do a static link against this library,
which doesn't work on my system, at least. (Ubuntu 18.04.4).

>  brlapi = not_found
>  if 'CONFIG_BRLAPI' in config_host

thanks
-- PMM

On 01/10/20 18:19, Peter Maydell wrote:
> Hi; this commit seems to have broken my static build.
> Previously configure did not include libudev in the link
> for a static build (there is no libudev.a, at least on my
> system). Now it does, and then the link fails with
>   /usr/bin/ld: cannot find -ludev
> 
>>  ##########################################
>> -# Do we have libudev
>> -if test "$libudev" != "no" ; then
>> -  if $pkg_config libudev && test "$static" != "yes"; then
>> -    libudev="yes"
>> -    libudev_libs=$($pkg_config --libs libudev)
>> -  else
>> -    libudev="no"
>> -  fi
>> -fi
>
> This is the old code, which doesn't enable libudev for
> static builds. [...]
> I'm not very confident about reading meson.build logic, but it
> looks like this trusts meson/pkg-config to tell it about
> whether it can do a static link against this library,
> which doesn't work on my system, at least. (Ubuntu 18.04.4).

Yes, and the same was of course true of pkg-config without meson.  You
probably got the same warning that you reported on v7.

In fact, my guess is that the "test $static != yes" was added in reply
to a similar complaint; the commit that introduced it has a note that
the test was added by the maintainer:

    commit 3efac6ebb88e4d099f07fef65178ebaa595ae770
    Author: Tomáš Golembiovský <tgolembi@redhat.com>
    Date:   Tue Oct 23 13:23:10 2018 +0200

    configure: add test for libudev

    Signed-off-by: Tomáš Golembiovský <tgolembi@redhat.com>
    Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>
    *make libudev optional to avoid breaking existing build/test
    environments
    *disable libudev for --static builds
    Signed-off-by: Michael Roth <mdroth@linux.vnet.ibm.com>

But I don't think --static should affect the build this way, especially
since libudev was the only library that had this test in the configure
script (checked in 5.1).  Debian doesn't package "libgtk-3.a" either and
yet GTK+ it is not special cased in the configure script, so I'm not
sure what it is that makes libudev special.

Do you actually build just with "--static"?  Or do you have a list of
"--disable" options so you can add one more?

Paolo

On Thu, 1 Oct 2020 at 17:55, Paolo Bonzini <pbonzini@redhat.com> wrote:
> On 01/10/20 18:19, Peter Maydell wrote:
> > I'm not very confident about reading meson.build logic, but it
> > looks like this trusts meson/pkg-config to tell it about
> > whether it can do a static link against this library,
> > which doesn't work on my system, at least. (Ubuntu 18.04.4).
>
> Yes, and the same was of course true of pkg-config without meson.  You
> probably got the same warning that you reported on v7.
>
> In fact, my guess is that the "test $static != yes" was added in reply
> to a similar complaint; the commit that introduced it has a note that
> the test was added by the maintainer:
>
>     commit 3efac6ebb88e4d099f07fef65178ebaa595ae770
>     Author: Tomáš Golembiovský <tgolembi@redhat.com>
>     Date:   Tue Oct 23 13:23:10 2018 +0200
>
>     configure: add test for libudev
>
>     Signed-off-by: Tomáš Golembiovský <tgolembi@redhat.com>
>     Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>
>     *make libudev optional to avoid breaking existing build/test
>     environments
>     *disable libudev for --static builds
>     Signed-off-by: Michael Roth <mdroth@linux.vnet.ibm.com>
>
> But I don't think --static should affect the build this way, especially
> since libudev was the only library that had this test in the configure
> script (checked in 5.1).  Debian doesn't package "libgtk-3.a" either and
> yet GTK+ it is not special cased in the configure script, so I'm not
> sure what it is that makes libudev special.
>
> Do you actually build just with "--static"?  Or do you have a list of
> "--disable" options so you can add one more?

Yes, I have a lot of --disable-foo options. Ideally I wouldn't
need any, because our configure/build system would identify "there
isn't actually a static version of this dependency present"
rather than blindly trusting pkg-config when it lies to us.
(Is it possible to get Meson to just always do a "test that
a trivial program with these cflags and libs will build" as
part of whatever magic it does as part of dependency() ?
Or to have a qemu_dependency() wrapper that does ?)

The useful thing about libgtk is that it's pretty obvious
that the --disable option to use to stop QEMU linking to
it is --disable-gtk. It's much less obvious what would be
the --disable option to use to stop us linking against
libgio, or libudev, because those dependencies aren't closely
and obviously tied to QEMU features. So for that kind of
library dependency it's much more useful to have configure
do enough checks that it doesn't try to link against something
that doesn't exist.

thanks
-- PMM

On 01/10/20 20:51, Peter Maydell wrote:
> Yes, I have a lot of --disable-foo options. Ideally I wouldn't
> need any, because our configure/build system would identify "there
> isn't actually a static version of this dependency present"
> rather than blindly trusting pkg-config when it lies to us.
> (Is it possible to get Meson to just always do a "test that
> a trivial program with these cflags and libs will build" as
> part of whatever magic it does as part of dependency() ?

Unfortunately there are many special cases, including libraries that
require symbols in the executable, so its not possible to do that.  For
this reason the "static library not found for dependency" is just a warning.

We can add such a compile test ourselves.  Of course it might add to the
compile time if we do it for every dependency, but perhaps we can do one
with all the dependencies and if it fails loop on single dependencies to
give a better error.

Paolo