On Thu, Nov 5, 2015 at 3:45 PM, Christophe Fergeau <cfergeau@xxxxxxxxxx> 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@xxxxxxxxxx> >> + >> +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 _______________________________________________ Spice-devel mailing list Spice-devel@xxxxxxxxxxxxxxxxxxxxx http://lists.freedesktop.org/mailman/listinfo/spice-devel