[v6,15/28] package/qt-webkit-kiosk: Convert to qmake infra
diff mbox series

Message ID 20200217212350.29750-16-anaumann@ultratronik.de
State Accepted
Headers show
Series
  • Qt5 qmake infra and per-package compatibility
Related show

Commit Message

Andreas Naumann Feb. 17, 2020, 9:23 p.m. UTC
Convert but keep the slightly complicated custom install step for now.

Signed-off-by: Andreas Naumann <anaumann@ultratronik.de>
---
 package/qt-webkit-kiosk/qt-webkit-kiosk.mk | 11 +++--------
 1 file changed, 3 insertions(+), 8 deletions(-)

Comments

Thomas Petazzoni March 11, 2020, 10:04 p.m. UTC | #1
Hello Andreas,

On Mon, 17 Feb 2020 22:23:37 +0100
Andreas Naumann <anaumann@ultratronik.de> wrote:

> Convert but keep the slightly complicated custom install step for now.
> 
> Signed-off-by: Andreas Naumann <anaumann@ultratronik.de>
> ---
>  package/qt-webkit-kiosk/qt-webkit-kiosk.mk | 11 +++--------
>  1 file changed, 3 insertions(+), 8 deletions(-)

I've applied, but I have some comments below.

> 
> diff --git a/package/qt-webkit-kiosk/qt-webkit-kiosk.mk b/package/qt-webkit-kiosk/qt-webkit-kiosk.mk
> index a714fca9c9..22cbf3cb87 100644
> --- a/package/qt-webkit-kiosk/qt-webkit-kiosk.mk
> +++ b/package/qt-webkit-kiosk/qt-webkit-kiosk.mk
> @@ -11,13 +11,8 @@ QT_WEBKIT_KIOSK_DEPENDENCIES = qt5webkit qt5multimedia
>  QT_WEBKIT_KIOSK_LICENSE = LGPL-3.0
>  QT_WEBKIT_KIOSK_LICENSE_FILES = doc/lgpl.html
>  
> -define QT_WEBKIT_KIOSK_CONFIGURE_CMDS
> -	(cd $(@D); $(TARGET_MAKE_ENV) $(QT5_QMAKE) PREFIX=/usr)
> -endef
> -
> -define QT_WEBKIT_KIOSK_BUILD_CMDS
> -	$(TARGET_MAKE_ENV) $(MAKE) -C $(@D)
> -endef
> +QT_WEBKIT_KIOSK_INSTALL_STAGING = NO

This is not needed, as this is the default, and check-package complains
about this.

> +QT_WEBKIT_KIOSK_CONF_OPTS = PREFIX=/usr

Do you know why specifically for this package we need to pass
PREFIX=/usr, and not for any other qmake-based package ?

>  define QT_WEBKIT_KIOSK_INSTALL_TARGET_CMDS
>  	$(TARGET_MAKE_ENV) $(MAKE) -C $(@D)/src -f Makefile.qt-webkit-kiosk \
> @@ -29,4 +24,4 @@ define QT_WEBKIT_KIOSK_INSTALL_TARGET_CMDS
>  		$(if $(BR2_PACKAGE_QT_WEBKIT_KIOSK_SOUNDS),install_sound)
>  endef

Any reason for these custom installation commands, rather than using
the package-provided installation logic ?

Thanks!

Thomas
Andreas Naumann March 16, 2020, 9:53 p.m. UTC | #2
Hi Thomas,

On 11.03.20 23:04, Thomas Petazzoni wrote:
> Hello Andreas,
> 
> On Mon, 17 Feb 2020 22:23:37 +0100
> Andreas Naumann <anaumann@ultratronik.de> wrote:
> 
>> Convert but keep the slightly complicated custom install step for now.
>>
>> Signed-off-by: Andreas Naumann <anaumann@ultratronik.de>
>> ---
>>   package/qt-webkit-kiosk/qt-webkit-kiosk.mk | 11 +++--------
>>   1 file changed, 3 insertions(+), 8 deletions(-)
> 
> I've applied, but I have some comments below.
> 
>>
>> diff --git a/package/qt-webkit-kiosk/qt-webkit-kiosk.mk b/package/qt-webkit-kiosk/qt-webkit-kiosk.mk
>> index a714fca9c9..22cbf3cb87 100644
>> --- a/package/qt-webkit-kiosk/qt-webkit-kiosk.mk
>> +++ b/package/qt-webkit-kiosk/qt-webkit-kiosk.mk
>> @@ -11,13 +11,8 @@ QT_WEBKIT_KIOSK_DEPENDENCIES = qt5webkit qt5multimedia
>>   QT_WEBKIT_KIOSK_LICENSE = LGPL-3.0
>>   QT_WEBKIT_KIOSK_LICENSE_FILES = doc/lgpl.html
>>   
>> -define QT_WEBKIT_KIOSK_CONFIGURE_CMDS
>> -	(cd $(@D); $(TARGET_MAKE_ENV) $(QT5_QMAKE) PREFIX=/usr)
>> -endef
>> -
>> -define QT_WEBKIT_KIOSK_BUILD_CMDS
>> -	$(TARGET_MAKE_ENV) $(MAKE) -C $(@D)
>> -endef
>> +QT_WEBKIT_KIOSK_INSTALL_STAGING = NO
> 
> This is not needed, as this is the default, and check-package complains
> about this.
> 

Ok.

>> +QT_WEBKIT_KIOSK_CONF_OPTS = PREFIX=/usr
> 
> Do you know why specifically for this package we need to pass
> PREFIX=/usr, and not for any other qmake-based package ?

Well, it's the same for quazip and other application projects I've seen: 
They dont use the default install paths of the corresponding qmake 
(QT_INSTALL_PREFIX|DATA|DOCS|LIBS|BINS|...), but define their own. This 
is fair enough but usually results in ignoring the knowledge about 
sysroot/staging destination which qmake does have. This information then 
has to be put into the INSTALL_ROOT variable in case of cross-compile.

> 
>>   define QT_WEBKIT_KIOSK_INSTALL_TARGET_CMDS
>>   	$(TARGET_MAKE_ENV) $(MAKE) -C $(@D)/src -f Makefile.qt-webkit-kiosk \
>> @@ -29,4 +24,4 @@ define QT_WEBKIT_KIOSK_INSTALL_TARGET_CMDS
>>   		$(if $(BR2_PACKAGE_QT_WEBKIT_KIOSK_SOUNDS),install_sound)
>>   endef
> 
> Any reason for these custom installation commands, rather than using
> the package-provided installation logic ?

Uh, I dont remember. Probably the default didnt work or me being lazy to 
not having to test even more.

regards,
Andreas

> 
> Thanks!
> 
> Thomas
>

Patch
diff mbox series

diff --git a/package/qt-webkit-kiosk/qt-webkit-kiosk.mk b/package/qt-webkit-kiosk/qt-webkit-kiosk.mk
index a714fca9c9..22cbf3cb87 100644
--- a/package/qt-webkit-kiosk/qt-webkit-kiosk.mk
+++ b/package/qt-webkit-kiosk/qt-webkit-kiosk.mk
@@ -11,13 +11,8 @@  QT_WEBKIT_KIOSK_DEPENDENCIES = qt5webkit qt5multimedia
 QT_WEBKIT_KIOSK_LICENSE = LGPL-3.0
 QT_WEBKIT_KIOSK_LICENSE_FILES = doc/lgpl.html
 
-define QT_WEBKIT_KIOSK_CONFIGURE_CMDS
-	(cd $(@D); $(TARGET_MAKE_ENV) $(QT5_QMAKE) PREFIX=/usr)
-endef
-
-define QT_WEBKIT_KIOSK_BUILD_CMDS
-	$(TARGET_MAKE_ENV) $(MAKE) -C $(@D)
-endef
+QT_WEBKIT_KIOSK_INSTALL_STAGING = NO
+QT_WEBKIT_KIOSK_CONF_OPTS = PREFIX=/usr
 
 define QT_WEBKIT_KIOSK_INSTALL_TARGET_CMDS
 	$(TARGET_MAKE_ENV) $(MAKE) -C $(@D)/src -f Makefile.qt-webkit-kiosk \
@@ -29,4 +24,4 @@  define QT_WEBKIT_KIOSK_INSTALL_TARGET_CMDS
 		$(if $(BR2_PACKAGE_QT_WEBKIT_KIOSK_SOUNDS),install_sound)
 endef
 
-$(eval $(generic-package))
+$(eval $(qmake-package))