Re: [PATCH] configure: Support modules for Windows

["Daniel P. Berrangé" <berrange@redhat.com>]

On Fri, Nov 05, 2021 at 11:20:02AM +0800, LIU Zhiwei wrote:

Generally we expect a commit message describing what is being solved
and any reasons for the approach taken.

> Signed-off-by: LIU Zhiwei <zhiwei_liu@c-sky.com>
> ---
>  configure   |  7 +------
>  meson.build | 34 ++++++++++++++++++++++++----------
>  2 files changed, 25 insertions(+), 16 deletions(-)
> 
> diff --git a/configure b/configure
> index 33682cb971..6280e412a9 100755
> --- a/configure
> +++ b/configure
> @@ -1422,7 +1422,7 @@ cat << EOF
>    bsd-user        all BSD usermode emulation targets
>    guest-agent     build the QEMU Guest Agent
>    pie             Position Independent Executables
> -  modules         modules support (non-Windows)
> +  modules         modules support
>    module-upgrades try to load modules from alternate paths for upgrades
>    debug-tcg       TCG debugging (default is disabled)
>    debug-info      debugging information
> @@ -1698,11 +1698,6 @@ else
>    QEMU_CFLAGS="$QEMU_CFLAGS -Wno-missing-braces"
>  fi
>  
> -# Our module code doesn't support Windows
> -if test "$modules" = "yes" && test "$mingw32" = "yes" ; then
> -  error_exit "Modules are not available for Windows"
> -fi
> -
>  # module_upgrades is only reasonable if modules are enabled
>  if test "$modules" = "no" && test "$module_upgrades" = "yes" ; then
>    error_exit "Can't enable module-upgrades as Modules are not enabled"
> diff --git a/meson.build b/meson.build
> index 47df10afc2..26195cb24f 100644
> --- a/meson.build
> +++ b/meson.build
> @@ -2625,7 +2625,7 @@ block_mods = []
>  softmmu_mods = []
>  foreach d, list : modules
>    foreach m, module_ss : list
> -    if enable_modules and targetos != 'windows'
> +    if enable_modules
>        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)
> @@ -2658,7 +2658,7 @@ endforeach
>  
>  foreach d, list : target_modules
>    foreach m, module_ss : list
> -    if enable_modules and targetos != 'windows'
> +    if enable_modules
>        foreach target : target_dirs
>          if target.endswith('-softmmu')
>            config_target = config_target_mak[target]
> @@ -2800,14 +2800,15 @@ common_ss.add(hwcore)
>  ###########
>  # Targets #
>  ###########
> -
> -foreach m : block_mods + softmmu_mods
> -  shared_module(m.name(),
> -                name_prefix: '',
> -                link_whole: m,
> -                install: true,
> -                install_dir: qemu_moddir)
> -endforeach
> +if targetos != 'windows'
> +  foreach m : block_mods + softmmu_mods
> +    shared_module(m.name(),
> +                  name_prefix: '',
> +                  link_whole: m,
> +                  install: true,
> +                  install_dir: qemu_moddir)
> +  endforeach
> +endif
>  
>  softmmu_ss.add(authz, blockdev, chardev, crypto, io, qmp)
>  common_ss.add(qom, qemuutil)
> @@ -2826,6 +2827,7 @@ common_all = static_library('common',
>  feature_to_c = find_program('scripts/feature_to_c.sh')
>  
>  emulators = {}
> +emulator = 0
>  foreach target : target_dirs
>    config_target = config_target_mak[target]
>    target_name = config_target['TARGET_NAME']
> @@ -2966,6 +2968,7 @@ foreach target : target_dirs
>                 link_language: link_language,
>                 link_depends: [block_syms, qemu_syms] + exe.get('link_depends', []),
>                 link_args: link_args,
> +               implib: true,
>                 win_subsystem: exe['win_subsystem'])
>  
>      if targetos == 'darwin'
> @@ -3023,6 +3026,17 @@ foreach target : target_dirs
>      endif
>    endforeach
>  endforeach
> +if (targetos == 'windows')
> +  foreach m : block_mods + softmmu_mods
> +    shared_module(m.name(),
> +	    name_prefix: '',
> +	    link_whole: m,
> +	    install: true,
> +	    dependencies: glib,
> +	    link_with: emulator,

AFAIK from the docs meson wants "link_with" to be a list of
libraries. This is passing a literal '0' which doesn't
make sense. Is this really needed ?

What is the reason for the "dependencies" addition ?

Can we not just use the existing shared_module() rules
we already have. It might be ok to just add dependencies
to the existing rule even for non-windows.

> +	    install_dir: qemu_moddir)
> +  endforeach
> +endif



Regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|