Message ID | 20200826070243.11969-1-pbonzini@redhat.com |
---|---|
State | New |
Headers | show |
Series | meson: move pixman detection to meson | expand |
On 26/08/2020 09.02, Paolo Bonzini wrote: > When pixman is not installed (or too old), but virglrenderer is available > and "configure" has been run with "--disable-system", the build currently > aborts when trying to compile vhost-user-gpu (since it requires pixman). > > Let's skip the build of vhost-user-gpu when pixman is not installed or > too old. Instead of adding CONFIG_PIXMAN, it is simpler to move the > detection to pixman. > > Based on a patch by Thomas Huth. <thuth@redhat.com> > > Fixes: 9b52b17ba5 ("configure: Allow to build tools without pixman") > Reported-by: Rafael Kitover <rkitover@gmail.com> > Reported-by: Philippe Mathieu-Daudé <philmd@redhat.com> > Signed-off-by: Paolo Bonzini <pbonzini@redhat.com> > --- > configure | 21 ++------------------- > contrib/vhost-user-gpu/meson.build | 3 ++- > meson.build | 12 +++++++----- > 3 files changed, 11 insertions(+), 25 deletions(-) > > diff --git a/configure b/configure > index a5fa472c64..51b6164f69 100755 > --- a/configure > +++ b/configure > @@ -3923,20 +3923,6 @@ if test "$modules" = yes; then > fi > fi > > -########################################## > -# pixman support probe > - > -if test "$softmmu" = "no"; then > - pixman_cflags= > - pixman_libs= > -elif $pkg_config --atleast-version=0.21.8 pixman-1 > /dev/null 2>&1; then > - pixman_cflags=$($pkg_config --cflags pixman-1) > - pixman_libs=$($pkg_config --libs pixman-1) > -else > - error_exit "pixman >= 0.21.8 not present." \ > - "Please install the pixman devel package." > -fi The "else" part now got completely lost, didn't it? pixman is required for building the softmmu targets, so we should error out if configure is run with a softmmu target on systems where pixman is not installed. Maybe you can simply replace the above check with: if test "$softmmu" = "yes" && ! $pkg_config --atleast-version=0.21.8 pixman-1 > /dev/null 2>&1; then error_exit "pixman >= 0.21.8 not present." \ "Please install the pixman devel package." fi ? Thomas > ########################################## > # libmpathpersist probe > > @@ -6649,8 +6635,8 @@ echo_version() { > fi > } > > -# prepend pixman and ftd flags after all config tests are done > -QEMU_CFLAGS="$pixman_cflags $fdt_cflags $QEMU_CFLAGS" > +# prepend ftd flags after all config tests are done > +QEMU_CFLAGS="$fdt_cflags $QEMU_CFLAGS" > QEMU_LDFLAGS="$fdt_ldflags $QEMU_LDFLAGS" > > config_host_mak="config-host.mak" > @@ -8053,9 +8039,6 @@ fi > > done # for target in $targets > > -echo "PIXMAN_CFLAGS=$pixman_cflags" >> $config_host_mak > -echo "PIXMAN_LIBS=$pixman_libs" >> $config_host_mak > - > if [ "$fdt" = "git" ]; then > subdirs="$subdirs dtc" > fi > diff --git a/contrib/vhost-user-gpu/meson.build b/contrib/vhost-user-gpu/meson.build > index 8df4c13bc5..7d9b29da8b 100644 > --- a/contrib/vhost-user-gpu/meson.build > +++ b/contrib/vhost-user-gpu/meson.build > @@ -1,5 +1,6 @@ > if 'CONFIG_TOOLS' in config_host and 'CONFIG_VIRGL' in config_host \ > - and 'CONFIG_GBM' in config_host and 'CONFIG_LINUX' in config_host > + and 'CONFIG_GBM' in config_host and 'CONFIG_LINUX' in config_host \ > + and pixman.found() > executable('vhost-user-gpu', files('vhost-user-gpu.c', 'virgl.c', 'vugbm.c'), > link_with: libvhost_user, > dependencies: [qemuutil, pixman, gbm, virgl], > diff --git a/meson.build b/meson.build > index bcd39b39da..57c2fe2b65 100644 > --- a/meson.build > +++ b/meson.build > @@ -114,8 +114,11 @@ if 'CONFIG_GNUTLS' in config_host > gnutls = declare_dependency(compile_args: config_host['GNUTLS_CFLAGS'].split(), > link_args: config_host['GNUTLS_LIBS'].split()) > endif > -pixman = declare_dependency(compile_args: config_host['PIXMAN_CFLAGS'].split(), > - link_args: config_host['PIXMAN_LIBS'].split()) > +pixman = not_found > +if have_system or have_tools > + pixman = dependency('pixman', required: have_system, version:'>=0.21.8', > + static: enable_static) > +endif > pam = not_found > if 'CONFIG_AUTH_PAM' in config_host > pam = cc.find_library('pam') > @@ -1094,9 +1097,7 @@ if have_tools > if 'CONFIG_VHOST_USER' in config_host > subdir('contrib/libvhost-user') > subdir('contrib/vhost-user-blk') > - if 'CONFIG_LINUX' in config_host > - subdir('contrib/vhost-user-gpu') > - endif > + subdir('contrib/vhost-user-gpu') > subdir('contrib/vhost-user-input') > subdir('contrib/vhost-user-scsi') > endif > @@ -1303,6 +1304,7 @@ summary_info += {'SDL image support': sdl_image.found()} > # TODO: add back version > summary_info += {'GTK support': config_host.has_key('CONFIG_GTK')} > summary_info += {'GTK GL support': config_host.has_key('CONFIG_GTK_GL')} > +summary_info += {'pixman': pixman.found()} > # TODO: add back version > summary_info += {'VTE support': config_host.has_key('CONFIG_VTE')} > summary_info += {'TLS priority': config_host['CONFIG_TLS_PRIORITY']} >
Il gio 27 ago 2020, 06:53 Thomas Huth <thuth@redhat.com> ha scritto: > > -else > > - error_exit "pixman >= 0.21.8 not present." \ > > - "Please install the pixman devel package." > > -fi > > The "else" part now got completely lost, didn't it? It's replaced by "required: have_system" in meson.build. We can add a custom "not found" message, but if the current one is not enough we should improve Meson so that it figures out a good one from the version requirement.
On 27/08/2020 08.14, Paolo Bonzini wrote: > > > Il gio 27 ago 2020, 06:53 Thomas Huth <thuth@redhat.com > <mailto:thuth@redhat.com>> ha scritto: > > > -else > > - error_exit "pixman >= 0.21.8 not present." \ > > - "Please install the pixman devel package." > > -fi > > The "else" part now got completely lost, didn't it? > > > It's replaced by "required: have_system" in meson.build. We can add a > custom "not found" message, but if the current one is not enough we > should improve Meson so that it figures out a good one from the version > requirement. Ah, now it makes sense, thanks for the explanation! But I've tested the patch now, and it does not work: .../qemu/meson.build:117:3: ERROR: Unknown variable "have_system". Should the pixman test maybe be moved to a later spot in the file? It currently shows up before the have_system variable is initialized... Thomas
Il gio 27 ago 2020, 09:28 Thomas Huth <thuth@redhat.com> ha scritto: > But I've tested the patch now, and it does not work: > > .../qemu/meson.build:117:3: ERROR: Unknown variable "have_system". > > Should the pixman test maybe be moved to a later spot in the file? It > currently shows up before the have_system variable is initialized... > There are some dependencies with other patches that were posted (I should have added Based-on), you can test the tags/for-upstream tag in my gitlab repository. Paolo > Thomas > >
On 26/08/2020 09.02, Paolo Bonzini wrote: > When pixman is not installed (or too old), but virglrenderer is available > and "configure" has been run with "--disable-system", the build currently > aborts when trying to compile vhost-user-gpu (since it requires pixman). > > Let's skip the build of vhost-user-gpu when pixman is not installed or > too old. Instead of adding CONFIG_PIXMAN, it is simpler to move the > detection to pixman. > > Based on a patch by Thomas Huth. <thuth@redhat.com> > > Fixes: 9b52b17ba5 ("configure: Allow to build tools without pixman") > Reported-by: Rafael Kitover <rkitover@gmail.com> > Reported-by: Philippe Mathieu-Daudé <philmd@redhat.com> > Signed-off-by: Paolo Bonzini <pbonzini@redhat.com> > --- > configure | 21 ++------------------- > contrib/vhost-user-gpu/meson.build | 3 ++- > meson.build | 12 +++++++----- > 3 files changed, 11 insertions(+), 25 deletions(-) > > diff --git a/configure b/configure > index a5fa472c64..51b6164f69 100755 > --- a/configure > +++ b/configure > @@ -3923,20 +3923,6 @@ if test "$modules" = yes; then > fi > fi > > -########################################## > -# pixman support probe > - > -if test "$softmmu" = "no"; then > - pixman_cflags= > - pixman_libs= > -elif $pkg_config --atleast-version=0.21.8 pixman-1 > /dev/null 2>&1; then > - pixman_cflags=$($pkg_config --cflags pixman-1) > - pixman_libs=$($pkg_config --libs pixman-1) > -else > - error_exit "pixman >= 0.21.8 not present." \ > - "Please install the pixman devel package." > -fi > - > ########################################## > # libmpathpersist probe > > @@ -6649,8 +6635,8 @@ echo_version() { > fi > } > > -# prepend pixman and ftd flags after all config tests are done > -QEMU_CFLAGS="$pixman_cflags $fdt_cflags $QEMU_CFLAGS" > +# prepend ftd flags after all config tests are done > +QEMU_CFLAGS="$fdt_cflags $QEMU_CFLAGS" > QEMU_LDFLAGS="$fdt_ldflags $QEMU_LDFLAGS" > > config_host_mak="config-host.mak" > @@ -8053,9 +8039,6 @@ fi > > done # for target in $targets > > -echo "PIXMAN_CFLAGS=$pixman_cflags" >> $config_host_mak > -echo "PIXMAN_LIBS=$pixman_libs" >> $config_host_mak > - > if [ "$fdt" = "git" ]; then > subdirs="$subdirs dtc" > fi > diff --git a/contrib/vhost-user-gpu/meson.build b/contrib/vhost-user-gpu/meson.build > index 8df4c13bc5..7d9b29da8b 100644 > --- a/contrib/vhost-user-gpu/meson.build > +++ b/contrib/vhost-user-gpu/meson.build > @@ -1,5 +1,6 @@ > if 'CONFIG_TOOLS' in config_host and 'CONFIG_VIRGL' in config_host \ > - and 'CONFIG_GBM' in config_host and 'CONFIG_LINUX' in config_host > + and 'CONFIG_GBM' in config_host and 'CONFIG_LINUX' in config_host \ > + and pixman.found() > executable('vhost-user-gpu', files('vhost-user-gpu.c', 'virgl.c', 'vugbm.c'), > link_with: libvhost_user, > dependencies: [qemuutil, pixman, gbm, virgl], > diff --git a/meson.build b/meson.build > index bcd39b39da..57c2fe2b65 100644 > --- a/meson.build > +++ b/meson.build > @@ -114,8 +114,11 @@ if 'CONFIG_GNUTLS' in config_host > gnutls = declare_dependency(compile_args: config_host['GNUTLS_CFLAGS'].split(), > link_args: config_host['GNUTLS_LIBS'].split()) > endif > -pixman = declare_dependency(compile_args: config_host['PIXMAN_CFLAGS'].split(), > - link_args: config_host['PIXMAN_LIBS'].split()) > +pixman = not_found > +if have_system or have_tools > + pixman = dependency('pixman', required: have_system, version:'>=0.21.8', > + static: enable_static) > +endif With the dependency from another patch that have_system and have_tools are initialized earlier in this file: Reviewed-by: Thomas Huth <thuth@redhat.com>
Am 26.08.20 um 09:02 schrieb Paolo Bonzini: > When pixman is not installed (or too old), but virglrenderer is available > and "configure" has been run with "--disable-system", the build currently > aborts when trying to compile vhost-user-gpu (since it requires pixman). > > Let's skip the build of vhost-user-gpu when pixman is not installed or > too old. Instead of adding CONFIG_PIXMAN, it is simpler to move the > detection to pixman. > > Based on a patch by Thomas Huth. <thuth@redhat.com> I don't know whether this is related to these patches, but I observe a failing build with pixman. A build which was configured with --enable-debug and --enable-modules currently terminates with this error message: Compiling C object libui-curses.a.p/ui_curses.c.o In file included from /home/stefan/src/qemu/qemu.org/qemu/include/ui/console.h:4, from ../ui/curses.c:38: /home/stefan/src/qemu/qemu.org/qemu/include/ui/qemu-pixman.h:12:10: fatal error: pixman.h: No such file or directory 12 | #include <pixman.h> | ^~~~~~~~~~ compilation terminated. The same error also occurs for two more compilations when I run make -k: Compiling C object libui-spice-app.a.p/ui_spice-app.c.o [...] Compiling C object libchardev-baum.a.p/chardev_baum.c.o [...] Obviously the compiler flags which are required for pixman-1 (-I/usr/include/pixman-1) are not passed to the compiler. A build without --enable-modules does not show these errors. Are there already CI tests with --enable-modules? Regards, Stefan
Il dom 30 ago 2020, 15:31 Stefan Weil <sw@weilnetz.de> ha scritto: > Obviously the compiler flags which are required for pixman-1 > (-I/usr/include/pixman-1) are not passed to the compiler. > Yes, they must be added to the modules' sourcesets. I will fix this. A build without --enable-modules does not show these errors. Are there > already CI tests with --enable-modules? > Clearly there are none on Gitlab or among Peter's tests. Paolo >
diff --git a/configure b/configure index a5fa472c64..51b6164f69 100755 --- a/configure +++ b/configure @@ -3923,20 +3923,6 @@ if test "$modules" = yes; then fi fi -########################################## -# pixman support probe - -if test "$softmmu" = "no"; then - pixman_cflags= - pixman_libs= -elif $pkg_config --atleast-version=0.21.8 pixman-1 > /dev/null 2>&1; then - pixman_cflags=$($pkg_config --cflags pixman-1) - pixman_libs=$($pkg_config --libs pixman-1) -else - error_exit "pixman >= 0.21.8 not present." \ - "Please install the pixman devel package." -fi - ########################################## # libmpathpersist probe @@ -6649,8 +6635,8 @@ echo_version() { fi } -# prepend pixman and ftd flags after all config tests are done -QEMU_CFLAGS="$pixman_cflags $fdt_cflags $QEMU_CFLAGS" +# prepend ftd flags after all config tests are done +QEMU_CFLAGS="$fdt_cflags $QEMU_CFLAGS" QEMU_LDFLAGS="$fdt_ldflags $QEMU_LDFLAGS" config_host_mak="config-host.mak" @@ -8053,9 +8039,6 @@ fi done # for target in $targets -echo "PIXMAN_CFLAGS=$pixman_cflags" >> $config_host_mak -echo "PIXMAN_LIBS=$pixman_libs" >> $config_host_mak - if [ "$fdt" = "git" ]; then subdirs="$subdirs dtc" fi diff --git a/contrib/vhost-user-gpu/meson.build b/contrib/vhost-user-gpu/meson.build index 8df4c13bc5..7d9b29da8b 100644 --- a/contrib/vhost-user-gpu/meson.build +++ b/contrib/vhost-user-gpu/meson.build @@ -1,5 +1,6 @@ if 'CONFIG_TOOLS' in config_host and 'CONFIG_VIRGL' in config_host \ - and 'CONFIG_GBM' in config_host and 'CONFIG_LINUX' in config_host + and 'CONFIG_GBM' in config_host and 'CONFIG_LINUX' in config_host \ + and pixman.found() executable('vhost-user-gpu', files('vhost-user-gpu.c', 'virgl.c', 'vugbm.c'), link_with: libvhost_user, dependencies: [qemuutil, pixman, gbm, virgl], diff --git a/meson.build b/meson.build index bcd39b39da..57c2fe2b65 100644 --- a/meson.build +++ b/meson.build @@ -114,8 +114,11 @@ if 'CONFIG_GNUTLS' in config_host gnutls = declare_dependency(compile_args: config_host['GNUTLS_CFLAGS'].split(), link_args: config_host['GNUTLS_LIBS'].split()) endif -pixman = declare_dependency(compile_args: config_host['PIXMAN_CFLAGS'].split(), - link_args: config_host['PIXMAN_LIBS'].split()) +pixman = not_found +if have_system or have_tools + pixman = dependency('pixman', required: have_system, version:'>=0.21.8', + static: enable_static) +endif pam = not_found if 'CONFIG_AUTH_PAM' in config_host pam = cc.find_library('pam') @@ -1094,9 +1097,7 @@ if have_tools if 'CONFIG_VHOST_USER' in config_host subdir('contrib/libvhost-user') subdir('contrib/vhost-user-blk') - if 'CONFIG_LINUX' in config_host - subdir('contrib/vhost-user-gpu') - endif + subdir('contrib/vhost-user-gpu') subdir('contrib/vhost-user-input') subdir('contrib/vhost-user-scsi') endif @@ -1303,6 +1304,7 @@ summary_info += {'SDL image support': sdl_image.found()} # TODO: add back version summary_info += {'GTK support': config_host.has_key('CONFIG_GTK')} summary_info += {'GTK GL support': config_host.has_key('CONFIG_GTK_GL')} +summary_info += {'pixman': pixman.found()} # TODO: add back version summary_info += {'VTE support': config_host.has_key('CONFIG_VTE')} summary_info += {'TLS priority': config_host['CONFIG_TLS_PRIORITY']}
When pixman is not installed (or too old), but virglrenderer is available and "configure" has been run with "--disable-system", the build currently aborts when trying to compile vhost-user-gpu (since it requires pixman). Let's skip the build of vhost-user-gpu when pixman is not installed or too old. Instead of adding CONFIG_PIXMAN, it is simpler to move the detection to pixman. Based on a patch by Thomas Huth. <thuth@redhat.com> Fixes: 9b52b17ba5 ("configure: Allow to build tools without pixman") Reported-by: Rafael Kitover <rkitover@gmail.com> Reported-by: Philippe Mathieu-Daudé <philmd@redhat.com> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com> --- configure | 21 ++------------------- contrib/vhost-user-gpu/meson.build | 3 ++- meson.build | 12 +++++++----- 3 files changed, 11 insertions(+), 25 deletions(-)