Message ID | fb9c4633-27af-a01d-3cca-79ed20519727@zapateado.de |
---|---|
State | New |
Headers | show |
Series | qga/vss-win32: enable qga-vss.tlb generation with widl | expand |
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 > >
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 >
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
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 >
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 >> >
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
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)
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(-) endif