1. Patchew
  2. QEMU
  3. View series

[Qemu-devel] [PATCH] ui: Correct icon install path

Colin Xu posted 1 patch 6 years, 7 months ago
Test docker-clang@ubuntu passed
Test s390x passed
Test asan passed
Test docker-mingw@fedora passed
Test FreeBSD passed
Test checkpatch passed
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/20190625032142.13854-1-colin.xu@intel.com
Makefile | 16 ++++++++--------
1 file changed, 8 insertions(+), 8 deletions(-)
[Qemu-devel] [PATCH] ui: Correct icon install path
Posted by Colin Xu 6 years, 7 months ago 
The double slash in path will fail the installation on MINGW/MSYS.

Fixes: a8260d387638 (ui: install logo icons to $prefix/share/icons)

Signed-off-by: Colin Xu <colin.xu@intel.com>
---
 Makefile | 16 ++++++++--------
 1 file changed, 8 insertions(+), 8 deletions(-)

diff --git a/Makefile b/Makefile
index cfb18f152544..562205be290c 100644
--- a/Makefile
+++ b/Makefile
@@ -875,19 +875,19 @@ ifneq ($(DESCS),)
 	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.22.0
Re: [Qemu-devel] [PATCH] ui: Correct icon install path
Posted by Philippe Mathieu-Daudé 6 years, 7 months ago 
Hi Colin,

On 6/25/19 5:21 AM, Colin Xu wrote:
> The double slash in path will fail the installation on MINGW/MSYS.
> 
> Fixes: a8260d387638 (ui: install logo icons to $prefix/share/icons)
> 
> Signed-off-by: Colin Xu <colin.xu@intel.com>
> ---
>  Makefile | 16 ++++++++--------
>  1 file changed, 8 insertions(+), 8 deletions(-)
> 
> diff --git a/Makefile b/Makefile
> index cfb18f152544..562205be290c 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -875,19 +875,19 @@ ifneq ($(DESCS),)
>  	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
> 

I'm not sure about this. Did you test it on a POSIX system?

Maybe we should escape an eventual trailing '/' in datadir and DESTDIR?
Re: [Qemu-devel] [PATCH] ui: Correct icon install path
Posted by Colin Xu 6 years, 7 months ago 
On 2019-06-25 18:43, Philippe Mathieu-Daudé wrote:
> Hi Colin,
>
> On 6/25/19 5:21 AM, Colin Xu wrote:
>> The double slash in path will fail the installation on MINGW/MSYS.
>>
>> Fixes: a8260d387638 (ui: install logo icons to $prefix/share/icons)
>>
>> Signed-off-by: Colin Xu <colin.xu@intel.com>
>> ---
>>   Makefile | 16 ++++++++--------
>>   1 file changed, 8 insertions(+), 8 deletions(-)
>>
>> diff --git a/Makefile b/Makefile
>> index cfb18f152544..562205be290c 100644
>> --- a/Makefile
>> +++ b/Makefile
>> @@ -875,19 +875,19 @@ ifneq ($(DESCS),)
>>   	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
>>
> I'm not sure about this. Did you test it on a POSIX system?
>
> Maybe we should escape an eventual trailing '/' in datadir and DESTDIR?

Hi Philippe,
Yes, tested. Actually the other DIR referened in the Makefile only use $(DESTDIR)$(bindir)
, $(DESTDIR)$(qemu_docdir), etc. So I guess the inconsistencie should be fixed.

Best Regards,
Colin Xu
Re: [Qemu-devel] [PATCH] ui: Correct icon install path
Posted by Daniel P. Berrangé 6 years, 7 months ago 
On Tue, Jun 25, 2019 at 11:21:42AM +0800, Colin Xu wrote:
> The double slash in path will fail the installation on MINGW/MSYS.
> 
> Fixes: a8260d387638 (ui: install logo icons to $prefix/share/icons)
> 
> Signed-off-by: Colin Xu <colin.xu@intel.com>
> ---
>  Makefile | 16 ++++++++--------
>  1 file changed, 8 insertions(+), 8 deletions(-)

Hmmm I swear this exact fix has been posted before but I can't find
/ remember where and obviously it didnt get merged.

> 
> diff --git a/Makefile b/Makefile
> index cfb18f152544..562205be290c 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -875,19 +875,19 @@ ifneq ($(DESCS),)
>  	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

Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>


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 :|
Re: [Qemu-devel] [PATCH] ui: Correct icon install path
Posted by Eric Blake 6 years, 7 months ago 
On 6/25/19 5:42 AM, Daniel P. Berrangé wrote:
> On Tue, Jun 25, 2019 at 11:21:42AM +0800, Colin Xu wrote:
>> The double slash in path will fail the installation on MINGW/MSYS.
>>
>> Fixes: a8260d387638 (ui: install logo icons to $prefix/share/icons)
>>
>> Signed-off-by: Colin Xu <colin.xu@intel.com>
>> ---
>>  Makefile | 16 ++++++++--------
>>  1 file changed, 8 insertions(+), 8 deletions(-)
> 
> Hmmm I swear this exact fix has been posted before but I can't find
> / remember where and obviously it didnt get merged.

https://lists.gnu.org/archive/html/qemu-devel/2019-05/msg04885.html

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3226
Virtualization:  qemu.org | libvirt.org
Re: [Qemu-devel] [PATCH] ui: Correct icon install path
Posted by Colin Xu 6 years, 7 months ago 
On 2019-06-25 21:40, Eric Blake wrote:
> On 6/25/19 5:42 AM, Daniel P. Berrangé wrote:
>> On Tue, Jun 25, 2019 at 11:21:42AM +0800, Colin Xu wrote:
>>> The double slash in path will fail the installation on MINGW/MSYS.
>>>
>>> Fixes: a8260d387638 (ui: install logo icons to $prefix/share/icons)
>>>
>>> Signed-off-by: Colin Xu <colin.xu@intel.com>
>>> ---
>>>   Makefile | 16 ++++++++--------
>>>   1 file changed, 8 insertions(+), 8 deletions(-)
>> Hmmm I swear this exact fix has been posted before but I can't find
>> / remember where and obviously it didnt get merged.
> https://lists.gnu.org/archive/html/qemu-devel/2019-05/msg04885.html
>
Aha, you are right Daniel. This has been posted before.
-- 
Best Regards,
Colin Xu

Patchew 2.1-devel-b67e4c2

© 2016 - 2026 Red Hat, Inc.