[PATCH] meson: fix qxl module build

[PATCH] meson: fix qxl module build

[Gerd Hoffmann]

Drop qxl object from softmmu source set, it is built as module.

Also drop CONFIG_QXL condition from qxl_ss.add.  First because it is
pointless, the whole thing is wrapped into "has_key('CONFIG_QXL')".
Second because it doesn't work for some reason.  Looks like the source
files are not added to the set for some reason and we end up with an
empty hw-display-qxl.so.

Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 hw/display/meson.build | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/hw/display/meson.build b/hw/display/meson.build
index 78adaf9db463..becbedd24c23 100644
--- a/hw/display/meson.build
+++ b/hw/display/meson.build
@@ -41,12 +41,10 @@ specific_ss.add(when: 'CONFIG_VGA', if_true: files('vga.c'))
 
 if config_all_devices.has_key('CONFIG_QXL')
   qxl_ss = ss.source_set()
-  qxl_ss.add(when: 'CONFIG_QXL', if_true: files('qxl.c', 'qxl-logger.c', 'qxl-render.c'))
+  qxl_ss.add(files('qxl.c', 'qxl-logger.c', 'qxl-render.c'))
   hw_display_modules += {'qxl': qxl_ss}
 endif
 
-softmmu_ss.add(when: 'CONFIG_QXL', if_true: files('qxl.c', 'qxl-logger.c', 'qxl-render.c'))
-
 softmmu_ss.add(when: 'CONFIG_DPCD', if_true: files('dpcd.c'))
 softmmu_ss.add(when: 'CONFIG_XLNX_ZYNQMP_ARM', if_true: files('xlnx_dp.c'))
 
-- 
2.27.0

Re: [PATCH] meson: fix qxl module build

[Marc-André Lureau]

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

On Thu, Aug 27, 2020 at 10:47 AM Gerd Hoffmann <kraxel@redhat.com> wrote:

> Drop qxl object from softmmu source set, it is built as module.
>
> Also drop CONFIG_QXL condition from qxl_ss.add.  First because it is
> pointless, the whole thing is wrapped into "has_key('CONFIG_QXL')".
> Second because it doesn't work for some reason.  Looks like the source
> files are not added to the set for some reason and we end up with an
> empty hw-display-qxl.so.
>
> Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
>

Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>


> ---
>  hw/display/meson.build | 4 +---
>  1 file changed, 1 insertion(+), 3 deletions(-)
>
> diff --git a/hw/display/meson.build b/hw/display/meson.build
> index 78adaf9db463..becbedd24c23 100644
> --- a/hw/display/meson.build
> +++ b/hw/display/meson.build
> @@ -41,12 +41,10 @@ specific_ss.add(when: 'CONFIG_VGA', if_true:
> files('vga.c'))
>
>  if config_all_devices.has_key('CONFIG_QXL')
>    qxl_ss = ss.source_set()
> -  qxl_ss.add(when: 'CONFIG_QXL', if_true: files('qxl.c', 'qxl-logger.c',
> 'qxl-render.c'))
> +  qxl_ss.add(files('qxl.c', 'qxl-logger.c', 'qxl-render.c'))
>    hw_display_modules += {'qxl': qxl_ss}
>  endif
>
> -softmmu_ss.add(when: 'CONFIG_QXL', if_true: files('qxl.c',
> 'qxl-logger.c', 'qxl-render.c'))
> -
>  softmmu_ss.add(when: 'CONFIG_DPCD', if_true: files('dpcd.c'))
>  softmmu_ss.add(when: 'CONFIG_XLNX_ZYNQMP_ARM', if_true:
> files('xlnx_dp.c'))
>
> --
> 2.27.0
>
>
>

-- 
Marc-André Lureau

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

Re: [PATCH] meson: fix qxl module build

[Gerd Hoffmann]

  Hi,

>  if config_all_devices.has_key('CONFIG_QXL')
>    qxl_ss = ss.source_set()
> -  qxl_ss.add(when: 'CONFIG_QXL', if_true: files('qxl.c', 'qxl-logger.c', 'qxl-render.c'))
> +  qxl_ss.add(files('qxl.c', 'qxl-logger.c', 'qxl-render.c'))
>    hw_display_modules += {'qxl': qxl_ss}
>  endif
>  
> -softmmu_ss.add(when: 'CONFIG_QXL', if_true: files('qxl.c', 'qxl-logger.c', 'qxl-render.c'))

Damn.  Turned out to not be that easy.  Modular builds work fine, but
with non-modular builds I have the problem that qxl_ss is merged into
softmmu_ss *unconditionally*.  So when building two targets, one with
qxl enabled (i386 for example) and one without pci support (avr for
example) I get missing symbols for pci+vga due to the attempt to link
qxl into avr-softmmu.

Any hints how to solve that one?

thanks,
  Gerd

Re: [PATCH] meson: fix qxl module build

[Paolo Bonzini]

On 04/09/20 10:12, Gerd Hoffmann wrote:
>   Hi,
> 
>>  if config_all_devices.has_key('CONFIG_QXL')
>>    qxl_ss = ss.source_set()
>> -  qxl_ss.add(when: 'CONFIG_QXL', if_true: files('qxl.c', 'qxl-logger.c', 'qxl-render.c'))
>> +  qxl_ss.add(files('qxl.c', 'qxl-logger.c', 'qxl-render.c'))
>>    hw_display_modules += {'qxl': qxl_ss}
>>  endif
>>  
>> -softmmu_ss.add(when: 'CONFIG_QXL', if_true: files('qxl.c', 'qxl-logger.c', 'qxl-render.c'))
> 
> Damn.  Turned out to not be that easy.  Modular builds work fine, but
> with non-modular builds I have the problem that qxl_ss is merged into
> softmmu_ss *unconditionally*.  So when building two targets, one with
> qxl enabled (i386 for example) and one without pci support (avr for
> example) I get missing symbols for pci+vga due to the attempt to link
> qxl into avr-softmmu.

You have found why it's got that "when:". :)

> Any hints how to solve that one?

I think this should be enough---fixing it in the common code, not with a
qxl-specific change:

diff --git a/meson.build b/meson.build
index 6ed3c37f46..88f2254f3b 100644
--- a/meson.build
+++ b/meson.build
@@ -923,7 +923,7 @@ softmmu_mods = []
 foreach d, list : modules
   foreach m, module_ss : list
     if enable_modules and targetos != 'windows'
-      module_ss = module_ss.apply(config_host, strict: false)
+      module_ss = module_ss.apply(config_all, strict: false)
       sl = static_library(d + '-' + m, [genh, module_ss.sources()],
                           dependencies: [modulecommon, module_ss.dependencies()], pic: true)
       if d == 'block'

:-O

If you want to get rid of the "if config_all_devices.has_key(...)", you perhaps
could try doing something like

  module_srcs = module_ss.sources()
  if module_srcs.length() == 0
    continue
  endif
  sl = static_library(d + '-' + m, [genh, module_srcs],
                      dependencies: [modulecommon, module_ss.dependencies()], pic: true)

etc.  This would work also for the CONFIG_VIRTIO_GPU changes.

Paolo

Re: [PATCH] meson: fix qxl module build

[Gerd Hoffmann]

  Hi,

> >> -  qxl_ss.add(when: 'CONFIG_QXL', if_true: files('qxl.c', 'qxl-logger.c', 'qxl-render.c'))
> >> +  qxl_ss.add(files('qxl.c', 'qxl-logger.c', 'qxl-render.c'))

> You have found why it's got that "when:". :)

> -      module_ss = module_ss.apply(config_host, strict: false)
> +      module_ss = module_ss.apply(config_all, strict: false)

Ah.  That nicely explains why the "when:" didn't work in the
first place.

> etc.  This would work also for the CONFIG_VIRTIO_GPU changes.

Yes, virtio-gpu modules are next on the list ;)

thanks,
  Gerd

Re: [PATCH] meson: fix qxl module build

[Paolo Bonzini]

On 04/09/20 14:14, Gerd Hoffmann wrote:
>   Hi,
> 
>>>> -  qxl_ss.add(when: 'CONFIG_QXL', if_true: files('qxl.c', 'qxl-logger.c', 'qxl-render.c'))
>>>> +  qxl_ss.add(files('qxl.c', 'qxl-logger.c', 'qxl-render.c'))
> 
>> You have found why it's got that "when:". :)
> 
>> -      module_ss = module_ss.apply(config_host, strict: false)
>> +      module_ss = module_ss.apply(config_all, strict: false)
> 
> Ah.  That nicely explains why the "when:" didn't work in the
> first place.
> 
>> etc.  This would work also for the CONFIG_VIRTIO_GPU changes.
> 
> Yes, virtio-gpu modules are next on the list ;)

Great, feel free to pick the patch up, no need for attribution or anything.

Paolo