[Spice-devel] [NSIS 2/2] packaging: add Makefile, spec file, jenkins automation

[Yedidyah Bar David]

On Thu, Nov 5, 2015 at 3:45 PM, Christophe Fergeau <cfergeau at redhat.com> wrote:
> Hey,
>
> Some comments on the Makefile/.spec files
>
> On Mon, Oct 26, 2015 at 12:35:29PM +0200, Yedidyah Bar David wrote:
>> Allow 'make dist'.
>>
>> Allow building the spice installer and building/installing the ovirt
>> installer.
>>
>> Move VERSION maintenance from the nsis file to the Makefile.
>>
>> Allow passing DISPLAYED_VERSION to make.
>>
>> Allow passing MODE to make to choose between the two installers -
>> SPICE (default) or OVIRT.
>>
>> Some of the code in the Makefile was copied and adapted from
>> ovirt-wgt-installer.spec. The spec file itself was also copied here and
>> adapted, and will be removed [1] from its current location.
>
> (I still believe that this would be much better split in smaller
> chunks).

Done.

>
>> --- /dev/null
>> +++ b/Makefile
>> @@ -0,0 +1,101 @@
>> +NAME=spice-nsis
>> +VERSION=0.103
>> +DISPLAYED_VERSION=$(VERSION)
>> +ARCHIVE=$(NAME)-$(VERSION).tar.gz
>> +
>> +# set to OVIRT to build the ovirt guest tools installer
>> +MODE=SPICE
>> +
>> +# Note: If you want to change the UN/INSTALLER name, you
>> +# have to edit also the nsis source.
>> +ifeq ($(MODE),SPICE)
>> +INSTALLER=spice-guest-tools-$(VERSION)
>> +UNINSTALLER=Uninstall spice-guest-tools
>> +EXE_VERSION=-$(VERSION)
>> +INSTALL_NAME=spice-wgt-installer
>> +else
>> +ifeq ($(MODE),OVIRT)
>> +INSTALLER=ovirt-guest-tools-setup
>> +UNINSTALLER=Uninstall ovirt-guest-tools-setup
>> +EXE_VERSION=
>> +INSTALL_NAME=ovirt-wgt-installer
>> +else
>> +$(error Please set MODE to one of SPICE or OVIRT, not [$(MODE)])
>> +endif
>> +endif
>> +
>> +# common dependencies/sources
>
>> +MINGW32BIN=/usr/i686-w64-mingw32/sys-root/mingw/bin
>> +MINGW64BIN=/usr/x86_64-w64-mingw32/sys-root/mingw/bin
>
> I'd add a reference to the RPMs installing the agent in this location:
> + # From RPMs available at http://www.spice-space.org/download/windows/vdagent/vdagent-win-0.7.3/

Done

>
> Ideally, we'd get this integrated in the virtio-win ISO and would not
> need an additional RPM.

Added a TODO comment

>
>> +VIRTIOWINDRIVERS=/usr/share/virtio-win-drivers
>
> Ditto, reference to the virtio-win RPM:
> + # From virtio-win package available in
> + # https://fedoraproject.org/wiki/Windows_Virtio_Drivers#Yum.7CDnf_Repo
>
> Correct path is /usr/share/virtio-win/drivers, not virtio-win-drivers

(It was correct with our own wrapping...)

>
> (you mentioned on IRC an oVirt-specific virtio-win package using
> /usr/share/virtio-win-drivers - this package extracts the drivers from
> the upstream virtio-win ISO as its layout differs from the one in the
> RPM, and win-guest-tools.nsis expects the ISO layout, not the RPM layout
> - in my opinion, this needs further discussion with Cole, and at worse,
> we probably can adjust win-guest-tools.nsis to use the RPM layout)

Copied the code and added a TODO comment

>
>> +QXLDRIVER=/usr/share/spice-qxl
>
> I don't think this is useful anymore, virtio-win upstream ships the QXL
> driver

Done

>
> This goes with
>
> diff --git a/win-guest-tools.nsis b/win-guest-tools.nsis
> index 463a049..67516da 100644
> --- a/win-guest-tools.nsis
> +++ b/win-guest-tools.nsis
> @@ -123,7 +123,6 @@ Section "install"
>
>    SetOutPath "$INSTDIR\drivers"
>    File /r drivers/virtio/
> -  File /r drivers/qxl/

Done (in a separate patch)

>
>    Push "vioserial"
>    Push "vioser"
>
>> +
>> +# ovirt dependencies/sources
>
> + # Available from http://resources.ovirt.org/pub/ovirt-3.6/rpm/fc22

Done

>
>
>> diff --git a/ovirt-wgt-installer.spec b/ovirt-wgt-installer.spec
>> new file mode 100644
>> index 0000000..6012f54
>> --- /dev/null
>> +++ b/ovirt-wgt-installer.spec
>> @@ -0,0 +1,63 @@
>> +Name:                ovirt-wgt-installer
>> +Version:     3.6.0
>> +Release:     0.2_master%{?release_suffix}%{?dist}
>> +Summary:     oVirt Windows Guest Tools Installer
>> +License:     GPLv2 and GPLv2+ and ASL 2.0 and Zlib and MIT and Python and Platform SDK Redistributable EULA and Microsoft DDK Redistributable EULA
>> +Source:              http://resources.ovirt.org/pub/ovirt-3.6-snapshot/src/ovirt-wgt/spice-nsis-0.103.tar.gz
>> +URL:         http://www.ovirt.org/Features/oVirt_Windows_Guest_Tools
>> +BuildArch:   noarch
>> +Packager:    Lev Veyde <lveyde at redhat.com>
>> +
>> +BuildRequires:       mingw32-nsis >= 2.46
>> +BuildRequires:       mingw32-spice-vdagent >= 0.7.3
>> +BuildRequires:       mingw64-spice-vdagent >= 0.7.3
>> +BuildRequires:       ovirt-guest-agent-windows
>> +BuildRequires:       vcredist-x86
>> +BuildRequires:       virtio-win-drivers
>
> Name of the upstream package is virtio-win,not virtio-win-drivers

Done

>
>
>> +BuildRequires:       spice-qxl
>
> I believe this can be dropped now

Done

>
>> +BuildRequires:       nsis-simple-service-plugin
>
> Ah, this is interesting, you need this because win-guest-tools.nsis uses this
> plugin. Its README document that it's needed, but maybe we need a better
> solution? I assume this is no good fit for an official fedora package?

No idea.

>
>> +
>> +%description
>> +oVirt Windows Guest Tools installer.
>> +The installer includes VirtIO-Win drivers, Spice QXL drivers, as well as oVirt and Spice Guest Agents.
>> +
>> +%global make_common_opts \\\
>> +     PREFIX=%{_prefix} \\\
>> +     MODE=OVIRT \\\
>> +     DISPLAYED_VERSION='%{version}-%{release}'
>> +
>> +%prep
>> +%setup -n spice-nsis -q
>> +
>> +%build
>> +
>> +make %{make_common_opts}
>> +
>> +%install
>> +
>> +make %{make_common_opts} install DESTDIR="%{buildroot}"
>> +
>> +%files
>> +%{_datadir}/%{name}/ovirt-guest-tools-setup.exe
>> +
>> +%package iso
>> +Summary: RPM wrapper for %{name}
>> +
>> +%description iso
>> +A package wrapping %{name} to provide dependency features.
>> +
>> +%files iso
>> +%{_datadir}/%{name}-iso
>
> I have a hard time understanding this -iso package. It's just a directory
> containing the installer executable, and the unpacked agent/drivers.
> What is this meant for?

It's a directory containing the stuff intended to eventually be packaged
inside the iso. Thus separating the iso creation from its population.

As I said (in private), it should be treated as an internal package to be
used inside the build process. See [1] for (a start of) documentation for
this build process.

Anyway, now accepted your suggestion and dropped it, moving everything
to spice-nsis Makefile/spec. For our toolchain, patch is:

https://gerrit.ovirt.org/48425

Thanks for the review. It helped me decide it's about time to get rid of
the extra packages and have a single 'make' create the iso directly
(and a single 'rpmbuild' package it).
-- 
Didi