Re: [PATCH v2 01/18] build-sys: add a "pixman" feature

[Paolo Bonzini <pbonzini@redhat.com>]

On 9/18/23 15:51, marcandre.lureau@redhat.com wrote:
> From: Marc-André Lureau <marcandre.lureau@redhat.com>
> 
> For now, pixman is mandatory, but we set config_host.h and Kconfig.
> Once compilation is fixed, "pixman" will become actually optional.
> 
> Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> ---
>   meson.build                   | 10 ++++++++--
>   include/ui/qemu-pixman.h      |  2 ++
>   Kconfig.host                  |  3 +++
>   meson_options.txt             |  2 ++
>   scripts/meson-buildoptions.sh |  3 +++
>   5 files changed, 18 insertions(+), 2 deletions(-)
> 
> diff --git a/meson.build b/meson.build
> index 5150a74831..e870b039cc 100644
> --- a/meson.build
> +++ b/meson.build
> @@ -828,10 +828,14 @@ if 'ust' in get_option('trace_backends')
>                        method: 'pkg-config')
>   endif
>   pixman = not_found
> -if have_system or have_tools
> -  pixman = dependency('pixman-1', required: have_system, version:'>=0.21.8',
> +if not get_option('pixman').auto() or have_system or have_tools
> +  pixman = dependency('pixman-1', required: get_option('pixman'), version:'>=0.21.8',
>                         method: 'pkg-config')

I wonder if we should _anyway_ default to "enabled" if system emulators are
enabled, even after the series.  That would be

if not get_option('pixman').auto() or have_system or have_tools
   pixman = dependency('pixman-1',
       required: have_system or have_tools ? true : get_option('pixman'),
       version:'>=0.21.8',
       method: 'pkg-config')
endif

(There is also .enable_auto_if() but that requires Meson 1.1).

>   endif
> +if not pixman.found()
> +  error('FIXME: pixman is currently required')
> +endif

This would be subsumed by the above "required" argument.

Even if we don't want to require pixman if system emulators are enabled,
this should be "if not pixman.found() and (have_system or have_tools)"
otherwise compilation with --disable-system would require pixman until
the end of this series.

No need for a v3 to fix the above.

Paolo

> +
>   zlib = dependency('zlib', required: true)
>   
>   libaio = not_found
> @@ -2124,6 +2128,7 @@ config_host_data.set('CONFIG_SECCOMP', seccomp.found())
>   if seccomp.found()
>     config_host_data.set('CONFIG_SECCOMP_SYSRAWRC', seccomp_has_sysrawrc)
>   endif
> +config_host_data.set('CONFIG_PIXMAN', pixman.found())
>   config_host_data.set('CONFIG_SNAPPY', snappy.found())
>   config_host_data.set('CONFIG_SOLARIS', targetos == 'sunos')
>   if get_option('tcg').allowed()
> @@ -2843,6 +2848,7 @@ have_ivshmem = config_host_data.get('CONFIG_EVENTFD')
>   host_kconfig = \
>     (get_option('fuzzing') ? ['CONFIG_FUZZ=y'] : []) + \
>     (have_tpm ? ['CONFIG_TPM=y'] : []) + \
> +  (pixman.found() ? ['CONFIG_PIXMAN=y'] : []) + \
>     (spice.found() ? ['CONFIG_SPICE=y'] : []) + \
>     (have_ivshmem ? ['CONFIG_IVSHMEM=y'] : []) + \
>     (opengl.found() ? ['CONFIG_OPENGL=y'] : []) + \
> diff --git a/include/ui/qemu-pixman.h b/include/ui/qemu-pixman.h
> index 51f8709327..b3379f6625 100644
> --- a/include/ui/qemu-pixman.h
> +++ b/include/ui/qemu-pixman.h
> @@ -6,11 +6,13 @@
>   #ifndef QEMU_PIXMAN_H
>   #define QEMU_PIXMAN_H
>   
> +#ifdef CONFIG_PIXMAN
>   /* pixman-0.16.0 headers have a redundant declaration */
>   #pragma GCC diagnostic push
>   #pragma GCC diagnostic ignored "-Wredundant-decls"
>   #include <pixman.h>
>   #pragma GCC diagnostic pop
> +#endif
>   
>   /*
>    * pixman image formats are defined to be native endian,
> diff --git a/Kconfig.host b/Kconfig.host
> index d763d89269..b6ac2b9316 100644
> --- a/Kconfig.host
> +++ b/Kconfig.host
> @@ -11,6 +11,9 @@ config OPENGL
>   config X11
>       bool
>   
> +config PIXMAN
> +    bool
> +
>   config SPICE
>       bool
>   
> diff --git a/meson_options.txt b/meson_options.txt
> index f82d88b7c6..1aaec02d68 100644
> --- a/meson_options.txt
> +++ b/meson_options.txt
> @@ -218,6 +218,8 @@ option('l2tpv3', type : 'feature', value : 'auto',
>          description: 'l2tpv3 network backend support')
>   option('netmap', type : 'feature', value : 'auto',
>          description: 'netmap network backend support')
> +option('pixman', type : 'feature', value : 'auto',
> +       description: 'pixman support')
>   option('slirp', type: 'feature', value: 'auto',
>          description: 'libslirp user mode network backend support')
>   option('vde', type : 'feature', value : 'auto',
> diff --git a/scripts/meson-buildoptions.sh b/scripts/meson-buildoptions.sh
> index e1d178370c..d016caf819 100644
> --- a/scripts/meson-buildoptions.sh
> +++ b/scripts/meson-buildoptions.sh
> @@ -147,6 +147,7 @@ meson_options_help() {
>     printf "%s\n" '  pa              PulseAudio sound support'
>     printf "%s\n" '  parallels       parallels image format support'
>     printf "%s\n" '  pipewire        PipeWire sound support'
> +  printf "%s\n" '  pixman          pixman support'
>     printf "%s\n" '  png             PNG support with libpng'
>     printf "%s\n" '  pvrdma          Enable PVRDMA support'
>     printf "%s\n" '  qcow1           qcow1 image format support'
> @@ -398,6 +399,8 @@ _meson_option_parse() {
>       --disable-parallels) printf "%s" -Dparallels=disabled ;;
>       --enable-pipewire) printf "%s" -Dpipewire=enabled ;;
>       --disable-pipewire) printf "%s" -Dpipewire=disabled ;;
> +    --enable-pixman) printf "%s" -Dpixman=enabled ;;
> +    --disable-pixman) printf "%s" -Dpixman=disabled ;;
>       --with-pkgversion=*) quote_sh "-Dpkgversion=$2" ;;
>       --enable-plugins) printf "%s" -Dplugins=true ;;
>       --disable-plugins) printf "%s" -Dplugins=false ;;