1. Patchew
  2. QEMU
  3. View series

[PATCH] qga/vss-win32: enable qga-vss.tlb generation with widl

Helge Konetzka posted 1 patch 2 years ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/fb9c4633-27af-a01d-3cca-79ed20519727@zapateado.de
Maintainers: Konstantin Kostiuk <kkostiuk@redhat.com>, Michael Roth <michael.roth@amd.com>
qga/vss-win32/meson.build | 11 +++++++++--
1 file changed, 9 insertions(+), 2 deletions(-)
[PATCH] qga/vss-win32: enable qga-vss.tlb generation with widl
Posted by Helge Konetzka 2 years ago 
Generation with widl needs to be triggered explicitly and requires
library and include directories containing referenced *.idl and *.tlb
as parameters.

Signed-off-by: Helge Konetzka <hk@zapateado.de>
---

For tested Msys2 build all referenced resources reside in /<usr>/include.
Msys2 provides its flavours in different /<usr> bases.

This patch derives the missing include directory path from widl path.
Assuming the given widl path is /<usr>/bin/widl, it determines /<usr>
as base and appends /<usr>/include as include and library directories
to widl command. This way the directory is correct for any Msys2
flavour.
It makes sure, only existing directories are appended as parameter.

---
  qga/vss-win32/meson.build | 11 +++++++++--
  1 file changed, 9 insertions(+), 2 deletions(-)

diff --git a/qga/vss-win32/meson.build b/qga/vss-win32/meson.build
index 71c50d0866..51539a582c 100644
--- a/qga/vss-win32/meson.build
+++ b/qga/vss-win32/meson.build
@@ -30,9 +30,16 @@ if midl.found()
                            input: 'qga-vss.idl',
                            output: 'qga-vss.tlb',
                            command: [midl, '@INPUT@', '/tlb', '@OUTPUT@'])
-else
+elif widl.found()
+  widl_cmd = [widl, '-t', '@INPUT@', '-o', '@OUTPUT@']
+  usr_include = fs.parent(fs.parent(widl.full_path()))/'include'
+  if fs.is_dir(usr_include)
+    widl_cmd += ['-L', usr_include]
+    widl_cmd += ['-I', usr_include]
+  endif
    gen_tlb = custom_target('gen-tlb',
                            input: 'qga-vss.idl',
                            output: 'qga-vss.tlb',
-                          command: [widl, '-t', '@INPUT@', '-o', 
'@OUTPUT@'])
+                          build_by_default: true,
+                          command: widl_cmd)
  endif
-- 
2.30.2
Re: [PATCH] qga/vss-win32: enable qga-vss.tlb generation with widl
Posted by Marc-André Lureau 2 years ago 
Hi Helge

On Sun, Apr 17, 2022 at 6:51 PM Helge Konetzka <hk@zapateado.de> wrote:

> Generation with widl needs to be triggered explicitly and requires
> library and include directories containing referenced *.idl and *.tlb
> as parameters.
>

Ok, that's different issues, it would help to split the patch.


>
> Signed-off-by: Helge Konetzka <hk@zapateado.de>
> ---
>
> For tested Msys2 build all referenced resources reside in /<usr>/include.
> Msys2 provides its flavours in different /<usr> bases.
>
> This patch derives the missing include directory path from widl path.
> Assuming the given widl path is /<usr>/bin/widl, it determines /<usr>
> as base and appends /<usr>/include as include and library directories
> to widl command. This way the directory is correct for any Msys2
> flavour.
> It makes sure, only existing directories are appended as parameter.
>

I would file a bug to msys2 instead for widl to use the default include
directory. Otherwise, every widl user out there needs to be adjusted.
(I think it would need a special --with-widl-includedir=DIR, given how
msys2 remaps directory)


> ---
>   qga/vss-win32/meson.build | 11 +++++++++--
>   1 file changed, 9 insertions(+), 2 deletions(-)
>
> diff --git a/qga/vss-win32/meson.build b/qga/vss-win32/meson.build
> index 71c50d0866..51539a582c 100644
> --- a/qga/vss-win32/meson.build
> +++ b/qga/vss-win32/meson.build
> @@ -30,9 +30,16 @@ if midl.found()
>                             input: 'qga-vss.idl',
>                             output: 'qga-vss.tlb',
>                             command: [midl, '@INPUT@', '/tlb', '@OUTPUT@
> '])
> -else
> +elif widl.found()
> +  widl_cmd = [widl, '-t', '@INPUT@', '-o', '@OUTPUT@']
> +  usr_include = fs.parent(fs.parent(widl.full_path()))/'include'
> +  if fs.is_dir(usr_include)
> +    widl_cmd += ['-L', usr_include]
> +    widl_cmd += ['-I', usr_include]
> +  endif
>     gen_tlb = custom_target('gen-tlb',
>                             input: 'qga-vss.idl',
>                             output: 'qga-vss.tlb',
> -                          command: [widl, '-t', '@INPUT@', '-o',
> '@OUTPUT@'])
> +                          build_by_default: true,
>

I would make qga_vss depend on gen_tlb instead (so the tlb is always built
with the dll)

thanks

-- 
Marc-André Lureau
Re: [PATCH] qga/vss-win32: enable qga-vss.tlb generation with widl
Posted by Konstantin Kostiuk 2 years ago 
Hi Helge,

I checked what happened in MSYS2 and this looks like a bug in the widl tool.

I looked into the widl source code and think that it detects the default
include path incorrectly.

During build of widl tool the corresponding variable receive incorrect
value:
`BIN_TO_INCLUDEDIR = ../x86_64-w64-mingw32/include` but should be
`BIN_TO_INCLUDEDIR = ../include`. Looks like a package mismatch,
because the `/ming64/x86_64-w64-mingw32` directory exist
but contains only few libs and no any include files.

So I agreed with Marc-André. I think this bug should be fixed in MSYS2.
I think you can report this issue there
https://github.com/msys2/MINGW-packages/issues

When I checked the build using cross-compilation from Linux,
the widl tool uses proper BIN_TO_INCLUDEDIR.

We should add the rule that qga_vss depends on gen_tlb to get this error
more visible.

Marc-André, what do you think?

Best Regards,
Konstantin Kostiuk.


On Mon, Apr 18, 2022 at 11:15 AM Marc-André Lureau <
marcandre.lureau@gmail.com> wrote:

> Hi Helge
>
> On Sun, Apr 17, 2022 at 6:51 PM Helge Konetzka <hk@zapateado.de> wrote:
>
>> Generation with widl needs to be triggered explicitly and requires
>> library and include directories containing referenced *.idl and *.tlb
>> as parameters.
>>
>
> Ok, that's different issues, it would help to split the patch.
>
>
>>
>> Signed-off-by: Helge Konetzka <hk@zapateado.de>
>> ---
>>
>> For tested Msys2 build all referenced resources reside in /<usr>/include.
>> Msys2 provides its flavours in different /<usr> bases.
>>
>> This patch derives the missing include directory path from widl path.
>> Assuming the given widl path is /<usr>/bin/widl, it determines /<usr>
>> as base and appends /<usr>/include as include and library directories
>> to widl command. This way the directory is correct for any Msys2
>> flavour.
>> It makes sure, only existing directories are appended as parameter.
>>
>
> I would file a bug to msys2 instead for widl to use the default include
> directory. Otherwise, every widl user out there needs to be adjusted.
> (I think it would need a special --with-widl-includedir=DIR, given how
> msys2 remaps directory)
>
>
>> ---
>>   qga/vss-win32/meson.build | 11 +++++++++--
>>   1 file changed, 9 insertions(+), 2 deletions(-)
>>
>> diff --git a/qga/vss-win32/meson.build b/qga/vss-win32/meson.build
>> index 71c50d0866..51539a582c 100644
>> --- a/qga/vss-win32/meson.build
>> +++ b/qga/vss-win32/meson.build
>> @@ -30,9 +30,16 @@ if midl.found()
>>                             input: 'qga-vss.idl',
>>                             output: 'qga-vss.tlb',
>>                             command: [midl, '@INPUT@', '/tlb', '@OUTPUT@
>> '])
>> -else
>> +elif widl.found()
>> +  widl_cmd = [widl, '-t', '@INPUT@', '-o', '@OUTPUT@']
>> +  usr_include = fs.parent(fs.parent(widl.full_path()))/'include'
>> +  if fs.is_dir(usr_include)
>> +    widl_cmd += ['-L', usr_include]
>> +    widl_cmd += ['-I', usr_include]
>> +  endif
>>     gen_tlb = custom_target('gen-tlb',
>>                             input: 'qga-vss.idl',
>>                             output: 'qga-vss.tlb',
>> -                          command: [widl, '-t', '@INPUT@', '-o',
>> '@OUTPUT@'])
>> +                          build_by_default: true,
>>
>
> I would make qga_vss depend on gen_tlb instead (so the tlb is always built
> with the dll)
>
> thanks
>
> --
> Marc-André Lureau
>
Re: [PATCH] qga/vss-win32: enable qga-vss.tlb generation with widl
Posted by Marc-André Lureau 2 years ago 
Hi

On Wed, Apr 20, 2022 at 3:17 PM Konstantin Kostiuk <kkostiuk@redhat.com>
wrote:

> Hi Helge,
>
> I checked what happened in MSYS2 and this looks like a bug in the widl
> tool.
>
> I looked into the widl source code and think that it detects the default
> include path incorrectly.
>
> During build of widl tool the corresponding variable receive incorrect
> value:
> `BIN_TO_INCLUDEDIR = ../x86_64-w64-mingw32/include` but should be
> `BIN_TO_INCLUDEDIR = ../include`. Looks like a package mismatch,
> because the `/ming64/x86_64-w64-mingw32` directory exist
> but contains only few libs and no any include files.
>
> So I agreed with Marc-André. I think this bug should be fixed in MSYS2.
> I think you can report this issue there
> https://github.com/msys2/MINGW-packages/issues
>
>
Thanks for the investigation and your comment on the msys2 issue:
https://github.com/msys2/MINGW-packages/issues/11520

When I checked the build using cross-compilation from Linux,
> the widl tool uses proper BIN_TO_INCLUDEDIR.
>
> We should add the rule that qga_vss depends on gen_tlb to get this error
> more visible.
>
> Marc-André, what do you think?
>
>
yes, that's what I suggested earlier

thanks


> Best Regards,
> Konstantin Kostiuk.
>
>
> On Mon, Apr 18, 2022 at 11:15 AM Marc-André Lureau <
> marcandre.lureau@gmail.com> wrote:
>
>> Hi Helge
>>
>> On Sun, Apr 17, 2022 at 6:51 PM Helge Konetzka <hk@zapateado.de> wrote:
>>
>>> Generation with widl needs to be triggered explicitly and requires
>>> library and include directories containing referenced *.idl and *.tlb
>>> as parameters.
>>>
>>
>> Ok, that's different issues, it would help to split the patch.
>>
>>
>>>
>>> Signed-off-by: Helge Konetzka <hk@zapateado.de>
>>> ---
>>>
>>> For tested Msys2 build all referenced resources reside in /<usr>/include.
>>> Msys2 provides its flavours in different /<usr> bases.
>>>
>>> This patch derives the missing include directory path from widl path.
>>> Assuming the given widl path is /<usr>/bin/widl, it determines /<usr>
>>> as base and appends /<usr>/include as include and library directories
>>> to widl command. This way the directory is correct for any Msys2
>>> flavour.
>>> It makes sure, only existing directories are appended as parameter.
>>>
>>
>> I would file a bug to msys2 instead for widl to use the default include
>> directory. Otherwise, every widl user out there needs to be adjusted.
>> (I think it would need a special --with-widl-includedir=DIR, given how
>> msys2 remaps directory)
>>
>>
>>> ---
>>>   qga/vss-win32/meson.build | 11 +++++++++--
>>>   1 file changed, 9 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/qga/vss-win32/meson.build b/qga/vss-win32/meson.build
>>> index 71c50d0866..51539a582c 100644
>>> --- a/qga/vss-win32/meson.build
>>> +++ b/qga/vss-win32/meson.build
>>> @@ -30,9 +30,16 @@ if midl.found()
>>>                             input: 'qga-vss.idl',
>>>                             output: 'qga-vss.tlb',
>>>                             command: [midl, '@INPUT@', '/tlb', '@OUTPUT@
>>> '])
>>> -else
>>> +elif widl.found()
>>> +  widl_cmd = [widl, '-t', '@INPUT@', '-o', '@OUTPUT@']
>>> +  usr_include = fs.parent(fs.parent(widl.full_path()))/'include'
>>> +  if fs.is_dir(usr_include)
>>> +    widl_cmd += ['-L', usr_include]
>>> +    widl_cmd += ['-I', usr_include]
>>> +  endif
>>>     gen_tlb = custom_target('gen-tlb',
>>>                             input: 'qga-vss.idl',
>>>                             output: 'qga-vss.tlb',
>>> -                          command: [widl, '-t', '@INPUT@', '-o',
>>> '@OUTPUT@'])
>>> +                          build_by_default: true,
>>>
>>
>> I would make qga_vss depend on gen_tlb instead (so the tlb is always
>> built with the dll)
>>
>> thanks
>>
>> --
>> Marc-André Lureau
>>
>

-- 
Marc-André Lureau
Re: [PATCH] qga/vss-win32: enable qga-vss.tlb generation with widl
Posted by Helge Konetzka 2 years ago 
Hello,

a few moments ago I sent a PR to Msys2 to make widl work as expected.

I've prepared a rather simple patch to activate the generation of 
qga-vss.tlb by widl in targets all and qemu-ga, which I will post later on.

Thank you for your patience and your help!

Regards, Helge.


Am 20.04.22 um 14:27 schrieb Marc-André Lureau:
> Hi
> 
> On Wed, Apr 20, 2022 at 3:17 PM Konstantin Kostiuk <kkostiuk@redhat.com 
> <mailto:kkostiuk@redhat.com>> wrote:
> 
>     Hi Helge,
> 
>     I checked what happened in MSYS2 and this looks like a bug in the
>     widl tool.
> 
>     I looked into the widl source code and think that it detects the
>     default include path incorrectly.
> 
>     During build of widl tool the corresponding variable receive
>     incorrect value:
>     `BIN_TO_INCLUDEDIR = ../x86_64-w64-mingw32/include` but should be
>     `BIN_TO_INCLUDEDIR = ../include`. Looks like a package mismatch,
>     because the `/ming64/x86_64-w64-mingw32` directory exist
>     but contains only few libs and no any include files.
> 
>     So I agreed with Marc-André. I think this bug should be fixed in MSYS2.
>     I think you can report this issue there
>     https://github.com/msys2/MINGW-packages/issues
>     <https://github.com/msys2/MINGW-packages/issues>
> 
> 
> Thanks for the investigation and your comment on the msys2 issue: 
> https://github.com/msys2/MINGW-packages/issues/11520 
> <https://github.com/msys2/MINGW-packages/issues/11520>
> 
>     When I checked the build using cross-compilation from Linux,
>     the widl tool uses proper BIN_TO_INCLUDEDIR.
> 
>     We should add the rule that qga_vss depends on gen_tlb to get this
>     error more visible.
> 
>     Marc-André, what do you think?
> 
> 
> yes, that's what I suggested earlier
> 
> thanks
> 
>     Best Regards,
>     Konstantin Kostiuk.
> 
> 
>     On Mon, Apr 18, 2022 at 11:15 AM Marc-André Lureau
>     <marcandre.lureau@gmail.com <mailto:marcandre.lureau@gmail.com>> wrote:
> 
>         Hi Helge
> 
>         On Sun, Apr 17, 2022 at 6:51 PM Helge Konetzka <hk@zapateado.de
>         <mailto:hk@zapateado.de>> wrote:
> 
>             Generation with widl needs to be triggered explicitly and
>             requires
>             library and include directories containing referenced *.idl
>             and *.tlb
>             as parameters.
> 
> 
>         Ok, that's different issues, it would help to split the patch.
> 
> 
>             Signed-off-by: Helge Konetzka <hk@zapateado.de
>             <mailto:hk@zapateado.de>>
>             ---
> 
>             For tested Msys2 build all referenced resources reside in
>             /<usr>/include.
>             Msys2 provides its flavours in different /<usr> bases.
> 
>             This patch derives the missing include directory path from
>             widl path.
>             Assuming the given widl path is /<usr>/bin/widl, it
>             determines /<usr>
>             as base and appends /<usr>/include as include and library
>             directories
>             to widl command. This way the directory is correct for any Msys2
>             flavour.
>             It makes sure, only existing directories are appended as
>             parameter.
> 
> 
>         I would file a bug to msys2 instead for widl to use the default
>         include directory. Otherwise, every widl user out there needs to
>         be adjusted.
>         (I think it would need a special --with-widl-includedir=DIR,
>         given how msys2 remaps directory)
> 
> 
>             ---
>                qga/vss-win32/meson.build | 11 +++++++++--
>                1 file changed, 9 insertions(+), 2 deletions(-)
> 
>             diff --git a/qga/vss-win32/meson.build
>             b/qga/vss-win32/meson.build
>             index 71c50d0866..51539a582c 100644
>             --- a/qga/vss-win32/meson.build
>             +++ b/qga/vss-win32/meson.build
>             @@ -30,9 +30,16 @@ if midl.found()
>                                          input: 'qga-vss.idl',
>                                          output: 'qga-vss.tlb',
>                                          command: [midl, '@INPUT@',
>             '/tlb', '@OUTPUT@'])
>             -else
>             +elif widl.found()
>             +  widl_cmd = [widl, '-t', '@INPUT@', '-o', '@OUTPUT@']
>             +  usr_include =
>             fs.parent(fs.parent(widl.full_path()))/'include'
>             +  if fs.is_dir(usr_include)
>             +    widl_cmd += ['-L', usr_include]
>             +    widl_cmd += ['-I', usr_include]
>             +  endif
>                  gen_tlb = custom_target('gen-tlb',
>                                          input: 'qga-vss.idl',
>                                          output: 'qga-vss.tlb',
>             -                          command: [widl, '-t', '@INPUT@',
>             '-o',
>             '@OUTPUT@'])
>             +                          build_by_default: true,
> 
> 
>         I would make qga_vss depend on gen_tlb instead (so the tlb is
>         always built with the dll)
> 
>         thanks
> 
>         -- 
>         Marc-André Lureau
> 
> 
> 
> -- 
> Marc-André Lureau
Re: [PATCH] qga/vss-win32: enable qga-vss.tlb generation with widl
Posted by Konstantin Kostiuk 2 years ago 
Hi Helge,

In general, the patch looks good but I want to make sure
that we will not break other compilation environments.

What version of MSYS2 do you use?
In my case, I can compile GA without this patch.


Best Regards,
Konstantin Kostiuk.


On Sun, Apr 17, 2022 at 5:50 PM Helge Konetzka <hk@zapateado.de> wrote:

> Generation with widl needs to be triggered explicitly and requires
> library and include directories containing referenced *.idl and *.tlb
> as parameters.
>
> Signed-off-by: Helge Konetzka <hk@zapateado.de>
> ---
>
> For tested Msys2 build all referenced resources reside in /<usr>/include.
> Msys2 provides its flavours in different /<usr> bases.
>
> This patch derives the missing include directory path from widl path.
> Assuming the given widl path is /<usr>/bin/widl, it determines /<usr>
> as base and appends /<usr>/include as include and library directories
> to widl command. This way the directory is correct for any Msys2
> flavour.
> It makes sure, only existing directories are appended as parameter.
>
> ---
>   qga/vss-win32/meson.build | 11 +++++++++--
>   1 file changed, 9 insertions(+), 2 deletions(-)
>
> diff --git a/qga/vss-win32/meson.build b/qga/vss-win32/meson.build
> index 71c50d0866..51539a582c 100644
> --- a/qga/vss-win32/meson.build
> +++ b/qga/vss-win32/meson.build
> @@ -30,9 +30,16 @@ if midl.found()
>                             input: 'qga-vss.idl',
>                             output: 'qga-vss.tlb',
>                             command: [midl, '@INPUT@', '/tlb', '@OUTPUT@
> '])
> -else
> +elif widl.found()
> +  widl_cmd = [widl, '-t', '@INPUT@', '-o', '@OUTPUT@']
> +  usr_include = fs.parent(fs.parent(widl.full_path()))/'include'
> +  if fs.is_dir(usr_include)
> +    widl_cmd += ['-L', usr_include]
> +    widl_cmd += ['-I', usr_include]
> +  endif
>     gen_tlb = custom_target('gen-tlb',
>                             input: 'qga-vss.idl',
>                             output: 'qga-vss.tlb',
> -                          command: [widl, '-t', '@INPUT@', '-o',
> '@OUTPUT@'])
> +                          build_by_default: true,
> +                          command: widl_cmd)
>   endif
> --
> 2.30.2
>
>
Re: [PATCH] qga/vss-win32: enable qga-vss.tlb generation with widl
Posted by Helge Konetzka 2 years ago 
Hi Konstantin,

sorry not being clear about QEMU versions.

On building the RCs of QEMU 7.0.0:

qemu-ga.exe is produced
qga-vss.dll is produced
qga-vss.tlb is not created

In RCs of QEMU 7.0.0 qga-vss.tlb is no part of the tarball anymore.

The git history of qga/vss-win32/meson.build suggests that qga-vss.tlb 
was part of the tarball in 6.2.0.

Now, to create qga-vss.tlb either

MINGW_PREFIX=/mingw64 # or any of /mingw32 /ucrt64 /clang32 /clang64
cd qga/vss-win32
widl -I${MINGW_PREFIX}/include -L${MINGW_PREFIX}/include \
     -t "${srcdir}"/${_tarname}/qga/vss-win32/qga-vss.idl -o qga-vss.tlb

needs to be executed or the patch may be applied.

I am not sure what to answer about MSYS2 version. I am talking about 
https://msys2.org with packaging code hosted on 
https://github.com/msys2/MINGW-packages. MSYS2 is a rolling release 
distribution.

Happy Easter,
Helge Konetzka.

Am 17.04.22 um 17:40 schrieb Konstantin Kostiuk:
> Hi Helge,
> 
> In general, the patch looks good but I want to make sure
> that we will not break other compilation environments.
> 
> What version of MSYS2 do you use?
> In my case, I can compile GA without this patch.
> 
> 
> Best Regards,
> Konstantin Kostiuk.
> 
> 
> On Sun, Apr 17, 2022 at 5:50 PM Helge Konetzka <hk@zapateado.de 
> <mailto:hk@zapateado.de>> wrote:
> 
>     Generation with widl needs to be triggered explicitly and requires
>     library and include directories containing referenced *.idl and *.tlb
>     as parameters.
> 
>     Signed-off-by: Helge Konetzka <hk@zapateado.de <mailto:hk@zapateado.de>>
>     ---
> 
>     For tested Msys2 build all referenced resources reside in
>     /<usr>/include.
>     Msys2 provides its flavours in different /<usr> bases.
> 
>     This patch derives the missing include directory path from widl path.
>     Assuming the given widl path is /<usr>/bin/widl, it determines /<usr>
>     as base and appends /<usr>/include as include and library directories
>     to widl command. This way the directory is correct for any Msys2
>     flavour.
>     It makes sure, only existing directories are appended as parameter.
> 
>     ---
>        qga/vss-win32/meson.build | 11 +++++++++--
>        1 file changed, 9 insertions(+), 2 deletions(-)
> 
>     diff --git a/qga/vss-win32/meson.build b/qga/vss-win32/meson.build
>     index 71c50d0866..51539a582c 100644
>     --- a/qga/vss-win32/meson.build
>     +++ b/qga/vss-win32/meson.build
>     @@ -30,9 +30,16 @@ if midl.found()
>                                  input: 'qga-vss.idl',
>                                  output: 'qga-vss.tlb',
>                                  command: [midl, '@INPUT@', '/tlb',
>     '@OUTPUT@'])
>     -else
>     +elif widl.found()
>     +  widl_cmd = [widl, '-t', '@INPUT@', '-o', '@OUTPUT@']
>     +  usr_include = fs.parent(fs.parent(widl.full_path()))/'include'
>     +  if fs.is_dir(usr_include)
>     +    widl_cmd += ['-L', usr_include]
>     +    widl_cmd += ['-I', usr_include]
>     +  endif
>          gen_tlb = custom_target('gen-tlb',
>                                  input: 'qga-vss.idl',
>                                  output: 'qga-vss.tlb',
>     -                          command: [widl, '-t', '@INPUT@', '-o',
>     '@OUTPUT@'])
>     +                          build_by_default: true,
>     +                          command: widl_cmd)
>        endif
>     -- 
>     2.30.2
>

Patchew 2.1-devel-0870f84

© 2016 - 2024 Red Hat, Inc.