[Qemu-devel] [PATCH 0/5] misc fixes to deal with icon location changes

[Qemu-devel] [PATCH 0/5] misc fixes to deal with icon location changes

[Daniel P. Berrangé]

This is a few misc fixes identified after the icon location changes
were merged. Most importantly it deals with the nsis installer file
manifest.

Daniel P. Berrangé (5):
  nsis: don't install files into /tmp
  make: don't insert a '/' after $(DESTDIR)
  nsis: ensure we always pass -W64 flag to nsis installer script
  nsis: fix location of icons to be bundled in the installer
  configure: stop trying to link non-existant bmp file from bios dir

 Makefile  | 23 +++++++++++++----------
 configure |  1 -
 qemu.nsi  |  4 ++--
 3 files changed, 15 insertions(+), 13 deletions(-)

-- 
2.20.1

[Qemu-devel] [PATCH 1/5] nsis: don't install files into /tmp

[Daniel P. Berrangé]

The nsis installer target has to run 'make install' to populate a
directory tree with content for the package. Replace the current
usage of '/tmp/qemu-nsis' with a location underneath the build
directory. This ensures that when a developer cleans their build
directory, any left over files from the installer build process
are also purged.

Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
---
 Makefile | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/Makefile b/Makefile
index de898eab62..362a98d275 100644
--- a/Makefile
+++ b/Makefile
@@ -907,7 +907,7 @@ endif
 .PHONY: installer
 installer: $(INSTALLER)
 
-INSTDIR=/tmp/qemu-nsis
+INSTDIR=$(BUILD_DIR)/qemu-nsis-vroot
 
 $(INSTALLER): $(SRC_PATH)/qemu.nsi
 	$(MAKE) install prefix=${INSTDIR}
-- 
2.20.1

Re: [Qemu-devel] [PATCH 1/5] nsis: don't install files into /tmp

[Eric Blake]

[-- Attachment #1: Type: text/plain, Size: 1120 bytes --]

On 1/29/19 5:39 AM, Daniel P. Berrangé wrote:
> The nsis installer target has to run 'make install' to populate a
> directory tree with content for the package. Replace the current
> usage of '/tmp/qemu-nsis' with a location underneath the build
> directory. This ensures that when a developer cleans their build
> directory, any left over files from the installer build process
> are also purged.
> 
> Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
> ---
>  Makefile | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 

Should "qemu-nsis-vroot/" be added to .gitignore for the sake of in-tree
builds?

> diff --git a/Makefile b/Makefile
> index de898eab62..362a98d275 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -907,7 +907,7 @@ endif
>  .PHONY: installer
>  installer: $(INSTALLER)
>  
> -INSTDIR=/tmp/qemu-nsis
> +INSTDIR=$(BUILD_DIR)/qemu-nsis-vroot
>  
>  $(INSTALLER): $(SRC_PATH)/qemu.nsi
>  	$(MAKE) install prefix=${INSTDIR}
> 

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3226
Virtualization:  qemu.org | libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [Qemu-devel] [PATCH 1/5] nsis: don't install files into /tmp

[Stefan Weil]

[-- Attachment #1: Type: text/plain, Size: 1282 bytes --]

On 29.01.19 22:07, Eric Blake wrote:
> On 1/29/19 5:39 AM, Daniel P. Berrangé wrote:
>> The nsis installer target has to run 'make install' to populate a
>> directory tree with content for the package. Replace the current
>> usage of '/tmp/qemu-nsis' with a location underneath the build
>> directory. This ensures that when a developer cleans their build
>> directory, any left over files from the installer build process
>> are also purged.
>>
>> Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
>> ---
>>  Makefile | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
> 
> Should "qemu-nsis-vroot/" be added to .gitignore for the sake of in-tree
> builds?

Yes. I suggest to use INSTDIR=$(BUILD_DIR)/qemu-nsis-vroot.tmp. Then it
is already covered by the existing pattern *.tmp in .gitignore.

With that modification

Signed-off-by: Stefan Weil <sw@weilnetz.de>

> 
>> diff --git a/Makefile b/Makefile
>> index de898eab62..362a98d275 100644
>> --- a/Makefile
>> +++ b/Makefile
>> @@ -907,7 +907,7 @@ endif
>>  .PHONY: installer
>>  installer: $(INSTALLER)
>>  
>> -INSTDIR=/tmp/qemu-nsis
>> +INSTDIR=$(BUILD_DIR)/qemu-nsis-vroot
>>  
>>  $(INSTALLER): $(SRC_PATH)/qemu.nsi
>>  	$(MAKE) install prefix=${INSTDIR}
>>
> 



[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

[Qemu-devel] [PATCH 2/5] make: don't insert a '/' after $(DESTDIR)

[Daniel P. Berrangé]

This breaks when $(prefix) is a relative directory, as it turns it
into an absolute path.

Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
---
 Makefile | 16 ++++++++--------
 1 file changed, 8 insertions(+), 8 deletions(-)

diff --git a/Makefile b/Makefile
index 362a98d275..14a463564e 100644
--- a/Makefile
+++ b/Makefile
@@ -744,19 +744,19 @@ ifneq ($(BLOBS),)
 	done
 endif
 	for s in $(ICON_SIZES); do \
-		mkdir -p "$(DESTDIR)/$(qemu_icondir)/hicolor/$${s}/apps"; \
+		mkdir -p "$(DESTDIR)$(qemu_icondir)/hicolor/$${s}/apps"; \
 		$(INSTALL_DATA) $(SRC_PATH)/ui/icons/qemu_$${s}.png \
-			"$(DESTDIR)/$(qemu_icondir)/hicolor/$${s}/apps/qemu.png"; \
+			"$(DESTDIR)$(qemu_icondir)/hicolor/$${s}/apps/qemu.png"; \
 	done; \
-	mkdir -p "$(DESTDIR)/$(qemu_icondir)/hicolor/32x32/apps"; \
+	mkdir -p "$(DESTDIR)$(qemu_icondir)/hicolor/32x32/apps"; \
 	$(INSTALL_DATA) $(SRC_PATH)/ui/icons/qemu_32x32.bmp \
-		"$(DESTDIR)/$(qemu_icondir)/hicolor/32x32/apps/qemu.bmp"; \
-	mkdir -p "$(DESTDIR)/$(qemu_icondir)/hicolor/scalable/apps"; \
+		"$(DESTDIR)$(qemu_icondir)/hicolor/32x32/apps/qemu.bmp"; \
+	mkdir -p "$(DESTDIR)$(qemu_icondir)/hicolor/scalable/apps"; \
 	$(INSTALL_DATA) $(SRC_PATH)/ui/icons/qemu.svg \
-		"$(DESTDIR)/$(qemu_icondir)/hicolor/scalable/apps/qemu.svg"
-	mkdir -p "$(DESTDIR)/$(qemu_desktopdir)"
+		"$(DESTDIR)$(qemu_icondir)/hicolor/scalable/apps/qemu.svg"
+	mkdir -p "$(DESTDIR)$(qemu_desktopdir)"
 	$(INSTALL_DATA) $(SRC_PATH)/ui/qemu.desktop \
-		"$(DESTDIR)/$(qemu_desktopdir)/qemu.desktop"
+		"$(DESTDIR)$(qemu_desktopdir)/qemu.desktop"
 ifdef CONFIG_GTK
 	$(MAKE) -C po $@
 endif
-- 
2.20.1

Re: [Qemu-devel] [PATCH 2/5] make: don't insert a '/' after $(DESTDIR)

[Eric Blake]

[-- Attachment #1: Type: text/plain, Size: 720 bytes --]

On 1/29/19 5:39 AM, Daniel P. Berrangé wrote:
> This breaks when $(prefix) is a relative directory, as it turns it
> into an absolute path.
> 
> Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
> ---
>  Makefile | 16 ++++++++--------
>  1 file changed, 8 insertions(+), 8 deletions(-)

Yes, it is long-standing practice among Automake and others that
$(DESTDIR) is normally empty but should contain its own trailing slash
when defined, as it is NOT separated from the rest of the installation
lines with an explicit /.

Reviewed-by: Eric Blake <eblake@redhat.com>

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3226
Virtualization:  qemu.org | libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

[Qemu-devel] [PATCH 3/5] nsis: ensure we always pass -W64 flag to nsis installer script

[Daniel P. Berrangé]

The code to set the -W64 flag is inside a conditional block that only
executes when we are bundling DLLs with the installer. This results in
QEMU being installed in the wrong location on 64-bit hosts when DLLs
are not bundled.

Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
---
 Makefile | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/Makefile b/Makefile
index 14a463564e..97a6727358 100644
--- a/Makefile
+++ b/Makefile
@@ -893,11 +893,14 @@ INSTALLER = qemu-setup-$(VERSION)$(EXESUF)
 
 nsisflags = -V2 -NOCD
 
+ifeq ($(ARCH),x86_64)
+nsisflags += -DW64
+endif
+
 ifneq ($(wildcard $(SRC_PATH)/dll),)
 ifeq ($(ARCH),x86_64)
 # 64 bit executables
 DLL_PATH = $(SRC_PATH)/dll/w64
-nsisflags += -DW64
 else
 # 32 bit executables
 DLL_PATH = $(SRC_PATH)/dll/w32
-- 
2.20.1

Re: [Qemu-devel] [PATCH 3/5] nsis: ensure we always pass -W64 flag to nsis installer script

[Stefan Weil]

On 29.01.19 12:39, Daniel P. Berrangé wrote:
> The code to set the -W64 flag is inside a conditional block that only
> executes when we are bundling DLLs with the installer. This results in
> QEMU being installed in the wrong location on 64-bit hosts when DLLs
> are not bundled.
> 
> Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
> ---
>  Makefile | 5 ++++-
>  1 file changed, 4 insertions(+), 1 deletion(-)
> 
> diff --git a/Makefile b/Makefile
> index 14a463564e..97a6727358 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -893,11 +893,14 @@ INSTALLER = qemu-setup-$(VERSION)$(EXESUF)
>  
>  nsisflags = -V2 -NOCD
>  
> +ifeq ($(ARCH),x86_64)
> +nsisflags += -DW64
> +endif
> +
>  ifneq ($(wildcard $(SRC_PATH)/dll),)
>  ifeq ($(ARCH),x86_64)
>  # 64 bit executables
>  DLL_PATH = $(SRC_PATH)/dll/w64
> -nsisflags += -DW64
>  else
>  # 32 bit executables
>  DLL_PATH = $(SRC_PATH)/dll/w32
> 

Technically this is fine, therefore

Signed-off-by: Stefan Weil <sw@weilnetz.de>

But I wonder whether a Windows installer without bundled DLLs makes
sense. If it doesn't, we could as well fail if the DLL path is empty.

Regards
Stefan

[Qemu-devel] [PATCH 4/5] nsis: fix location of icons to be bundled in the installer

[Daniel P. Berrangé]

The bmp icon files no longer exist in the root of the installation
tree. Instead we must reference the new icons subdirectory location
introduced in

  commit a8260d3876389eb52ca5c62ed4d80cdb7e025c85
  Author: Daniel P. Berrangé <berrange@redhat.com>
  Date:   Thu Jan 10 12:00:45 2019 +0000

    ui: install logo icons to $prefix/share/icons

Reported-by: Adam Baxter <voltagex@voltagex.org>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
---
 qemu.nsi | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/qemu.nsi b/qemu.nsi
index 75f1608b9e..53fd974877 100644
--- a/qemu.nsi
+++ b/qemu.nsi
@@ -119,12 +119,12 @@ Section "${PRODUCT} (required)"
     File "${SRCDIR}\README"
     File "${SRCDIR}\VERSION"
 
-    File "${BINDIR}\*.bmp"
     File "${BINDIR}\*.bin"
     File "${BINDIR}\*.dtb"
     File "${BINDIR}\*.rom"
     File "${BINDIR}\openbios-*"
 
+    File /r "${BINDIR}\icons"
     File /r "${BINDIR}\keymaps"
 !ifdef CONFIG_GTK
     File /r "${BINDIR}\share"
@@ -206,7 +206,6 @@ Section "Uninstall"
     Delete "$INSTDIR\COPYING.LIB"
     Delete "$INSTDIR\README"
     Delete "$INSTDIR\VERSION"
-    Delete "$INSTDIR\*.bmp"
     Delete "$INSTDIR\*.bin"
     Delete "$INSTDIR\*.dll"
     Delete "$INSTDIR\*.dtb"
@@ -217,6 +216,7 @@ Section "Uninstall"
     Delete "$INSTDIR\qemu.exe"
     Delete "$INSTDIR\qemu-system-*.exe"
     Delete "$INSTDIR\qemu-doc.html"
+    RMDir /r "$INSTDIR\icons"
     RMDir /r "$INSTDIR\keymaps"
     RMDir /r "$INSTDIR\share"
     ; Remove generated files
-- 
2.20.1

[Qemu-devel] [PATCH 5/5] configure: stop trying to link non-existant bmp file from bios dir

[Daniel P. Berrangé]

The icons were all moved from pci-bios to ui/icons with:

    commit a8260d3876389eb52ca5c62ed4d80cdb7e025c85
    Author: Daniel P. Berrangé <berrange@redhat.com>
    Date:   Thu Jan 10 12:00:45 2019 +0000

      ui: install logo icons to $prefix/share/icons

Adding symlinks from the source directory into the build directory
for the new source location will not help QEMU to locate them when
running uninstalled binaries, as we no longer search QEMU bios
directory to locate icon files.

Inability to find icons during startup when not having run a
"make install" is harmless, as frontends just continue without
error. So it is not worth trying to add code hacks to find icons
in non-installed directories.

Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
---
 configure | 1 -
 1 file changed, 1 deletion(-)

diff --git a/configure b/configure
index b18281c61f..fb1f65958e 100755
--- a/configure
+++ b/configure
@@ -7617,7 +7617,6 @@ LINKS="$LINKS pc-bios/optionrom/Makefile pc-bios/keymaps"
 LINKS="$LINKS pc-bios/spapr-rtas/Makefile"
 LINKS="$LINKS pc-bios/s390-ccw/Makefile"
 LINKS="$LINKS roms/seabios/Makefile roms/vgabios/Makefile"
-LINKS="$LINKS pc-bios/qemu-icon.bmp"
 LINKS="$LINKS .gdbinit scripts" # scripts needed by relative path in .gdbinit
 LINKS="$LINKS tests/acceptance tests/data"
 LINKS="$LINKS tests/qemu-iotests/check"
-- 
2.20.1

Re: [Qemu-devel] [PATCH 5/5] configure: stop trying to link non-existant bmp file from bios dir

[Peter Maydell]

On Tue, 29 Jan 2019 at 11:39, Daniel P. Berrangé <berrange@redhat.com> wrote:
> Inability to find icons during startup when not having run a
> "make install" is harmless, as frontends just continue without
> error. So it is not worth trying to add code hacks to find icons
> in non-installed directories.

I'm not hugely happy about this -- I think running from the
build directory should work the same as running installed
(it is how QEMU is often run as a developer and how it runs
when running tests).

thanks
-- PMM

Re: [Qemu-devel] [PATCH 0/5] misc fixes to deal with icon location changes

[Stefan Weil]

On 29.01.19 12:39, Daniel P. Berrangé wrote:
> This is a few misc fixes identified after the icon location changes
> were merged. Most importantly it deals with the nsis installer file
> manifest.
> 
> Daniel P. Berrangé (5):
>   nsis: don't install files into /tmp
>   make: don't insert a '/' after $(DESTDIR)
>   nsis: ensure we always pass -W64 flag to nsis installer script
>   nsis: fix location of icons to be bundled in the installer
>   configure: stop trying to link non-existant bmp file from bios dir
> 
>  Makefile  | 23 +++++++++++++----------
>  configure |  1 -
>  qemu.nsi  |  4 ++--
>  3 files changed, 15 insertions(+), 13 deletions(-)
> 

There was also a file rename which needs to be handled,
otherwise NSIS still fails:

File: "/tmp/qemu-nsis\qemu_logo_no_text.svg" -> no files found.

The new qemu.svg still includes the old file name. That could be fixed, too.

Regards,
Stefan

Re: [Qemu-devel] [PATCH 0/5] misc fixes to deal with icon location changes

[Daniel P. Berrangé]

On Fri, Feb 01, 2019 at 08:52:25AM +0100, Stefan Weil wrote:
> On 29.01.19 12:39, Daniel P. Berrangé wrote:
> > This is a few misc fixes identified after the icon location changes
> > were merged. Most importantly it deals with the nsis installer file
> > manifest.
> > 
> > Daniel P. Berrangé (5):
> >   nsis: don't install files into /tmp
> >   make: don't insert a '/' after $(DESTDIR)
> >   nsis: ensure we always pass -W64 flag to nsis installer script
> >   nsis: fix location of icons to be bundled in the installer
> >   configure: stop trying to link non-existant bmp file from bios dir
> > 
> >  Makefile  | 23 +++++++++++++----------
> >  configure |  1 -
> >  qemu.nsi  |  4 ++--
> >  3 files changed, 15 insertions(+), 13 deletions(-)
> > 
> 
> There was also a file rename which needs to be handled,
> otherwise NSIS still fails:
> 
> File: "/tmp/qemu-nsis\qemu_logo_no_text.svg" -> no files found.

I don't see that prolem - NSIS builds successfully for me, and I can't
find any reference to .svg files in the qemu.nsi 

> The new qemu.svg still includes the old file name. That could be fixed, too.

Oh, in its metadata I see.

Regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|