diff mbox

[RFC,1/1] qemu-ga: add missing libpcre to MSI build

Message ID 20170601124746.32140-1-t.lamprecht@proxmox.com (mailing list archive)
State New, archived
Headers show

Commit Message

Thomas Lamprecht June 1, 2017, 12:47 p.m. UTC
glib depends on libpcre which was not shipped with the MSI, thus
starting of the qemu-ga.exe failed with the respective error message.

Tell WIXL to ship this library with the MSI to avoid this problem.

Signed-off-by: Thomas Lamprecht <t.lamprecht@proxmox.com>
CC: Stefan Weil <sw@weilnetz.de>
CC: Michael Roth <mdroth@linux.vnet.ibm.com>
---

I haven't done much with the qga or WIXL, so I send this as a RFC.
I hope that I guessed the right people to get CC'ed from MAINTAINERS.

This fixes a current qemu-ga MSI build, I tested it successfully with Windows 7
and Windows 10 as guest OS.

I cross built from a Fedora 25 LXC container.

The Guid for the libpcre was generated by https://www.guidgen.com/ as suggested
by:
http://wixtoolset.org/documentation/manual/v3/howtos/general/generate_guids.html

 qga/installer/qemu-ga.wxs | 4 ++++
 1 file changed, 4 insertions(+)

Comments

Marc-André Lureau June 2, 2017, 11:42 a.m. UTC | #1
Hi

On Thu, Jun 1, 2017 at 5:08 PM Thomas Lamprecht <t.lamprecht@proxmox.com>
wrote:

> glib depends on libpcre which was not shipped with the MSI, thus
> starting of the qemu-ga.exe failed with the respective error message.
>
> Tell WIXL to ship this library with the MSI to avoid this problem.
>
> Signed-off-by: Thomas Lamprecht <t.lamprecht@proxmox.com>
> CC: Stefan Weil <sw@weilnetz.de>
> CC: Michael Roth <mdroth@linux.vnet.ibm.com>
>


It depends on your glib build, but since Fedora is one of the most
maintained cross mingw- distrib, it make sense to fix the build there.

Other solutions would involve either using wixl-specific require
preprocessor directive (which comes with a bunch of unused files since
those are mostly generated from mingw*- packages), or coming up with some
kind of dynamic dependency resolution (approach similar to Richard W.M.
Jones nsiswrapper). However this last approach is quite limited, since it
doesn't reach to data files etc.

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


> I haven't done much with the qga or WIXL, so I send this as a RFC.
> I hope that I guessed the right people to get CC'ed from MAINTAINERS.
>
> This fixes a current qemu-ga MSI build, I tested it successfully with
> Windows 7
> and Windows 10 as guest OS.
>
> I cross built from a Fedora 25 LXC container.
>
> The Guid for the libpcre was generated by https://www.guidgen.com/ as
> suggested
> by:
>
> http://wixtoolset.org/documentation/manual/v3/howtos/general/generate_guids.html
>
>  qga/installer/qemu-ga.wxs | 4 ++++
>  1 file changed, 4 insertions(+)
>
> diff --git a/qga/installer/qemu-ga.wxs b/qga/installer/qemu-ga.wxs
> index fa2260cafa..5af11627f8 100644
> --- a/qga/installer/qemu-ga.wxs
> +++ b/qga/installer/qemu-ga.wxs
> @@ -125,6 +125,9 @@
>            <Component Id="libwinpthread"
> Guid="{6C117C78-0F47-4B07-8F34-6BEE11643829}">
>              <File Id="libwinpthread_1.dll" Name="libwinpthread-1.dll"
> Source="$(var.Mingw_bin)/libwinpthread-1.dll" KeyPath="yes" DiskId="1"/>
>            </Component>
> +          <Component Id="libpcre"
> Guid="{7A86B45E-A009-489A-A849-CE3BACF03CD0}">
> +            <File Id="libpcre_1.dll" Name="libpcre-1.dll"
> Source="$(var.Mingw_bin)/libpcre-1.dll" KeyPath="yes" DiskId="1"/>
> +          </Component>
>            <Component Id="registry_entries"
> Guid="{D075D109-51CA-11E3-9F8B-000C29858960}">
>              <RegistryKey Root="HKLM"
>
> Key="Software\$(env.QEMU_GA_MANUFACTURER)\$(env.QEMU_GA_DISTRO)\Tools\QemuGA">
> @@ -173,6 +176,7 @@
>        <ComponentRef Id="libssp" />
>        <ComponentRef Id="libwinpthread" />
>        <ComponentRef Id="registry_entries" />
> +      <ComponentRef Id="libpcre" />
>      </Feature>
>
>      <InstallExecuteSequence>
> --
> 2.11.0
>
>
>
> --
Marc-André Lureau
Thomas Lamprecht July 7, 2017, 6:40 a.m. UTC | #2
Hi,

On 06/02/2017 01:42 PM, Marc-André Lureau wrote:
> Hi
>
> On Thu, Jun 1, 2017 at 5:08 PM Thomas Lamprecht<t.lamprecht@proxmox.com>
> wrote:
>
>> glib depends on libpcre which was not shipped with the MSI, thus
>> starting of the qemu-ga.exe failed with the respective error message.
>>
>> Tell WIXL to ship this library with the MSI to avoid this problem.
>>
>> Signed-off-by: Thomas Lamprecht<t.lamprecht@proxmox.com>
>> CC: Stefan Weil<sw@weilnetz.de>
>> CC: Michael Roth<mdroth@linux.vnet.ibm.com>
>>
> It depends on your glib build, but since Fedora is one of the most
> maintained cross mingw- distrib, it make sense to fix the build there.

But even if it isn't best to ship an unnecessary library it shouldn't
harm either. I'd like to make it nicer but after looking at the complexity
of possibilities to do so I rather want to avoid it, especially with my
almost non-existing knowledge of windows builds.

> Other solutions would involve either using wixl-specific require
> preprocessor directive (which comes with a bunch of unused files since
> those are mostly generated from mingw*- packages), or coming up with some
> kind of dynamic dependency resolution (approach similar to Richard W.M.
> Jones nsiswrapper). However this last approach is quite limited, since it
> doesn't reach to data files etc.
> In the meantime:
>   Reviewed-by: Marc-André Lureau<marcandre.lureau@redhat.com>
>

Thank you for the review!Has this any chance to still get into qemu 2.10?
Would be nice.

cheers,
Thomas

>> I haven't done much with the qga or WIXL, so I send this as a RFC.
>> I hope that I guessed the right people to get CC'ed from MAINTAINERS.
>>
>> This fixes a current qemu-ga MSI build, I tested it successfully with
>> Windows 7
>> and Windows 10 as guest OS.
>>
>> I cross built from a Fedora 25 LXC container.
>>
>> The Guid for the libpcre was generated byhttps://www.guidgen.com/  as
>> suggested
>> by:
>>
>> http://wixtoolset.org/documentation/manual/v3/howtos/general/generate_guids.html
>>
>>   qga/installer/qemu-ga.wxs | 4 ++++
>>   1 file changed, 4 insertions(+)
>>
>> diff --git a/qga/installer/qemu-ga.wxs b/qga/installer/qemu-ga.wxs
>> index fa2260cafa..5af11627f8 100644
>> --- a/qga/installer/qemu-ga.wxs
>> +++ b/qga/installer/qemu-ga.wxs
>> @@ -125,6 +125,9 @@
>>             <Component Id="libwinpthread"
>> Guid="{6C117C78-0F47-4B07-8F34-6BEE11643829}">
>>               <File Id="libwinpthread_1.dll" Name="libwinpthread-1.dll"
>> Source="$(var.Mingw_bin)/libwinpthread-1.dll" KeyPath="yes" DiskId="1"/>
>>             </Component>
>> +          <Component Id="libpcre"
>> Guid="{7A86B45E-A009-489A-A849-CE3BACF03CD0}">
>> +            <File Id="libpcre_1.dll" Name="libpcre-1.dll"
>> Source="$(var.Mingw_bin)/libpcre-1.dll" KeyPath="yes" DiskId="1"/>
>> +          </Component>
>>             <Component Id="registry_entries"
>> Guid="{D075D109-51CA-11E3-9F8B-000C29858960}">
>>               <RegistryKey Root="HKLM"
>>
>> Key="Software\$(env.QEMU_GA_MANUFACTURER)\$(env.QEMU_GA_DISTRO)\Tools\QemuGA">
>> @@ -173,6 +176,7 @@
>>         <ComponentRef Id="libssp" />
>>         <ComponentRef Id="libwinpthread" />
>>         <ComponentRef Id="registry_entries" />
>> +      <ComponentRef Id="libpcre" />
>>       </Feature>
>>
>>       <InstallExecuteSequence>
>> --
>> 2.11.0
>>
>>
>>
>> --
> Marc-André Lureau
>
Sameeh Jubran Nov. 21, 2018, 8:57 a.m. UTC | #3
Hi thomas,

Can you please share more info on why libpcre is needed in the first
place? I have tried to build qemu without it and I see no issues of
qemu-ga failing.
On Fri, Jul 7, 2017 at 9:47 AM Thomas Lamprecht <t.lamprecht@proxmox.com> wrote:
>
> Hi,
>
> On 06/02/2017 01:42 PM, Marc-André Lureau wrote:
> > Hi
> >
> > On Thu, Jun 1, 2017 at 5:08 PM Thomas Lamprecht<t.lamprecht@proxmox.com>
> > wrote:
> >
> >> glib depends on libpcre which was not shipped with the MSI, thus
> >> starting of the qemu-ga.exe failed with the respective error message.
> >>
> >> Tell WIXL to ship this library with the MSI to avoid this problem.
> >>
> >> Signed-off-by: Thomas Lamprecht<t.lamprecht@proxmox.com>
> >> CC: Stefan Weil<sw@weilnetz.de>
> >> CC: Michael Roth<mdroth@linux.vnet.ibm.com>
> >>
> > It depends on your glib build, but since Fedora is one of the most
> > maintained cross mingw- distrib, it make sense to fix the build there.
>
> But even if it isn't best to ship an unnecessary library it shouldn't
> harm either. I'd like to make it nicer but after looking at the complexity
> of possibilities to do so I rather want to avoid it, especially with my
> almost non-existing knowledge of windows builds.
>
> > Other solutions would involve either using wixl-specific require
> > preprocessor directive (which comes with a bunch of unused files since
> > those are mostly generated from mingw*- packages), or coming up with some
> > kind of dynamic dependency resolution (approach similar to Richard W.M.
> > Jones nsiswrapper). However this last approach is quite limited, since it
> > doesn't reach to data files etc.
> > In the meantime:
> >   Reviewed-by: Marc-André Lureau<marcandre.lureau@redhat.com>
> >
>
> Thank you for the review!Has this any chance to still get into qemu 2.10?
> Would be nice.
>
> cheers,
> Thomas
>
> >> I haven't done much with the qga or WIXL, so I send this as a RFC.
> >> I hope that I guessed the right people to get CC'ed from MAINTAINERS.
> >>
> >> This fixes a current qemu-ga MSI build, I tested it successfully with
> >> Windows 7
> >> and Windows 10 as guest OS.
> >>
> >> I cross built from a Fedora 25 LXC container.
> >>
> >> The Guid for the libpcre was generated byhttps://www.guidgen.com/  as
> >> suggested
> >> by:
> >>
> >> http://wixtoolset.org/documentation/manual/v3/howtos/general/generate_guids.html
> >>
> >>   qga/installer/qemu-ga.wxs | 4 ++++
> >>   1 file changed, 4 insertions(+)
> >>
> >> diff --git a/qga/installer/qemu-ga.wxs b/qga/installer/qemu-ga.wxs
> >> index fa2260cafa..5af11627f8 100644
> >> --- a/qga/installer/qemu-ga.wxs
> >> +++ b/qga/installer/qemu-ga.wxs
> >> @@ -125,6 +125,9 @@
> >>             <Component Id="libwinpthread"
> >> Guid="{6C117C78-0F47-4B07-8F34-6BEE11643829}">
> >>               <File Id="libwinpthread_1.dll" Name="libwinpthread-1.dll"
> >> Source="$(var.Mingw_bin)/libwinpthread-1.dll" KeyPath="yes" DiskId="1"/>
> >>             </Component>
> >> +          <Component Id="libpcre"
> >> Guid="{7A86B45E-A009-489A-A849-CE3BACF03CD0}">
> >> +            <File Id="libpcre_1.dll" Name="libpcre-1.dll"
> >> Source="$(var.Mingw_bin)/libpcre-1.dll" KeyPath="yes" DiskId="1"/>
> >> +          </Component>
> >>             <Component Id="registry_entries"
> >> Guid="{D075D109-51CA-11E3-9F8B-000C29858960}">
> >>               <RegistryKey Root="HKLM"
> >>
> >> Key="Software\$(env.QEMU_GA_MANUFACTURER)\$(env.QEMU_GA_DISTRO)\Tools\QemuGA">
> >> @@ -173,6 +176,7 @@
> >>         <ComponentRef Id="libssp" />
> >>         <ComponentRef Id="libwinpthread" />
> >>         <ComponentRef Id="registry_entries" />
> >> +      <ComponentRef Id="libpcre" />
> >>       </Feature>
> >>
> >>       <InstallExecuteSequence>
> >> --
> >> 2.11.0
> >>
> >>
> >>
> >> --
> > Marc-André Lureau
> >
>
>
>
>
diff mbox

Patch

diff --git a/qga/installer/qemu-ga.wxs b/qga/installer/qemu-ga.wxs
index fa2260cafa..5af11627f8 100644
--- a/qga/installer/qemu-ga.wxs
+++ b/qga/installer/qemu-ga.wxs
@@ -125,6 +125,9 @@ 
           <Component Id="libwinpthread" Guid="{6C117C78-0F47-4B07-8F34-6BEE11643829}">
             <File Id="libwinpthread_1.dll" Name="libwinpthread-1.dll" Source="$(var.Mingw_bin)/libwinpthread-1.dll" KeyPath="yes" DiskId="1"/>
           </Component>
+          <Component Id="libpcre" Guid="{7A86B45E-A009-489A-A849-CE3BACF03CD0}">
+            <File Id="libpcre_1.dll" Name="libpcre-1.dll" Source="$(var.Mingw_bin)/libpcre-1.dll" KeyPath="yes" DiskId="1"/>
+          </Component>
           <Component Id="registry_entries" Guid="{D075D109-51CA-11E3-9F8B-000C29858960}">
             <RegistryKey Root="HKLM"
                          Key="Software\$(env.QEMU_GA_MANUFACTURER)\$(env.QEMU_GA_DISTRO)\Tools\QemuGA">
@@ -173,6 +176,7 @@ 
       <ComponentRef Id="libssp" />
       <ComponentRef Id="libwinpthread" />
       <ComponentRef Id="registry_entries" />
+      <ComponentRef Id="libpcre" />
     </Feature>
 
     <InstallExecuteSequence>