diff mbox series

ui: Correct icon install path

Message ID 20190625032142.13854-1-colin.xu@intel.com
State New
Headers show
Series ui: Correct icon install path | expand

Commit Message

Colin Xu June 25, 2019, 3:21 a.m. UTC
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(-)

Comments

Daniel P. Berrangé June 25, 2019, 10:42 a.m. UTC | #1
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
Philippe Mathieu-Daudé June 25, 2019, 10:43 a.m. UTC | #2
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?
Eric Blake June 25, 2019, 1:40 p.m. UTC | #3
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
Colin Xu June 26, 2019, 12:48 a.m. UTC | #4
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
Colin Xu June 26, 2019, 12:52 a.m. UTC | #5
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.
diff mbox series

Patch

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