diff mbox series

[v6,09/28] package/quazip: Convert to qmake infra

Message ID 20200217212350.29750-10-anaumann@ultratronik.de
State Changes Requested
Headers show
Series Qt5 qmake infra and per-package compatibility | expand

Commit Message

Andreas Naumann Feb. 17, 2020, 9:23 p.m. UTC
We need to use correct staging path as prefix after switching to qmake infra,
because the qmake infra no longer overwrites the built in install pathes.

Otherwise the build breaks when the install steps are trying to copy to /usr.

Signed-off-by: Andreas Naumann <anaumann@ultratronik.de>
---
 package/quazip/quazip.mk | 18 ++----------------
 1 file changed, 2 insertions(+), 16 deletions(-)

Comments

Thomas Petazzoni Feb. 17, 2020, 9:57 p.m. UTC | #1
On Mon, 17 Feb 2020 22:23:31 +0100
Andreas Naumann <anaumann@ultratronik.de> wrote:

> We need to use correct staging path as prefix after switching to qmake infra,
> because the qmake infra no longer overwrites the built in install pathes.
> 
> Otherwise the build breaks when the install steps are trying to copy to /usr.
> 
> Signed-off-by: Andreas Naumann <anaumann@ultratronik.de>

A prefix of $(STAGING_DIR)/usr looks wrong, as prefix is normally where
the things will be once installed in the target. Do we really need to
do this change?

Thanks,

Thomas
Andreas Naumann Feb. 18, 2020, 10 a.m. UTC | #2
Hi,

Am 17.02.20 um 22:57 schrieb Thomas Petazzoni:
> On Mon, 17 Feb 2020 22:23:31 +0100
> Andreas Naumann <anaumann@ultratronik.de> wrote:
> 
>> We need to use correct staging path as prefix after switching to qmake infra,
>> because the qmake infra no longer overwrites the built in install pathes.
>>
>> Otherwise the build breaks when the install steps are trying to copy to /usr.
>>
>> Signed-off-by: Andreas Naumann <anaumann@ultratronik.de>
> 
> A prefix of $(STAGING_DIR)/usr looks wrong, as prefix is normally where
> the things will be once installed in the target. Do we really need to
> do this change?

I agree it's probably not how PREFIX is intended to be used. But after 
all the project doesnt seem to support cross-compile/install properly as 
you can see by the need to explicitly set INSTALL_ROOT to a different 
path for staging/target.

I had an earlier solution where I made a patch to the quazip .pro file, 
but I wasnt entirely sure if it's correct. So I figured with this 
solution I dont need to do any upstream work ;-), which I'm not 100% 
sure about.


Andreas

> 
> Thanks,
> 
> Thomas
>
Thomas Petazzoni March 11, 2020, 9:42 p.m. UTC | #3
On Tue, 18 Feb 2020 11:00:07 +0100
Andreas Naumann <dev@andin.de> wrote:

> > A prefix of $(STAGING_DIR)/usr looks wrong, as prefix is normally where
> > the things will be once installed in the target. Do we really need to
> > do this change?  
> 
> I agree it's probably not how PREFIX is intended to be used. But after 
> all the project doesnt seem to support cross-compile/install properly as 
> you can see by the need to explicitly set INSTALL_ROOT to a different 
> path for staging/target.

Well, in fact the current install procedure for quazip is perfect, as
you don't have to do the horrible rsync dance that we have to do for
other Qt packages. The fact that INSTALL_ROOT is different for staging
and target is *perfect*, it is exactly how INSTALL_ROOT should be have
in the first place: like DESTDIR.

So I'd really like to see more research done on this package, to
understand why we need to bogus PREFIX= variable, and make sure it
behaves like other packages.

It would in fact be ideal if all other Qt packages could properly
support INSTALL_ROOT like quazip does.

Best regards,

Thomas
Andreas Naumann March 14, 2020, 9:52 p.m. UTC | #4
Hi Thomas,

On 11.03.20 22:42, Thomas Petazzoni wrote:
> On Tue, 18 Feb 2020 11:00:07 +0100
> Andreas Naumann <dev@andin.de> wrote:
> 
>>> A prefix of $(STAGING_DIR)/usr looks wrong, as prefix is normally where
>>> the things will be once installed in the target. Do we really need to
>>> do this change?
>>
>> I agree it's probably not how PREFIX is intended to be used. But after
>> all the project doesnt seem to support cross-compile/install properly as
>> you can see by the need to explicitly set INSTALL_ROOT to a different
>> path for staging/target.
> 
> Well, in fact the current install procedure for quazip is perfect, as
> you don't have to do the horrible rsync dance that we have to do for
> other Qt packages. The fact that INSTALL_ROOT is different for staging
> and target is *perfect*, it is exactly how INSTALL_ROOT should be have
> in the first place: like DESTDIR.

I couldn't agree more, if INSTALL_ROOT would not touch the destination 
for host-files the whole thing would be much easier. But, it isnt that 
quazip works better than other packages. It's just simpler because it 
doesnt have any files for host.

> 
> So I'd really like to see more research done on this package, to
> understand why we need to bogus PREFIX= variable, and make sure it
> behaves like other packages.

Again, the quazip.pro uses PREFIX directly as install destination, so 
it's not aware of the sysroot/staging pathes for cross-compiles. This 
could probably be changed by patching it to use QT_INSTALL_PREFIX (which 
is prefixed with the staging path, if set). I dont know if that would 
break other things or require more changes in the project file.

> 
> It would in fact be ideal if all other Qt packages could properly
> support INSTALL_ROOT like quazip does.

See above. It's not the packages fault but how qmake uses INSTALL_ROOT.


best regards,
Andreas


> 
> Best regards,
> 
> Thomas
>
diff mbox series

Patch

diff --git a/package/quazip/quazip.mk b/package/quazip/quazip.mk
index 53042f6c4c..35cf8adafd 100644
--- a/package/quazip/quazip.mk
+++ b/package/quazip/quazip.mk
@@ -13,20 +13,6 @@  QUAZIP_DEPENDENCIES = \
 QUAZIP_LICENSE = LGPL-2.1
 QUAZIP_LICENSE_FILES = COPYING
 
-define QUAZIP_CONFIGURE_CMDS
-	(cd $(@D); $(TARGET_MAKE_ENV) $(QT5_QMAKE) PREFIX=/usr)
-endef
+QUAZIP_CONF_OPTS = PREFIX=$(STAGING_DIR)/usr
 
-define QUAZIP_BUILD_CMDS
-	$(TARGET_MAKE_ENV) $(MAKE) -C $(@D)
-endef
-
-define QUAZIP_INSTALL_STAGING_CMDS
-	$(TARGET_MAKE_ENV) $(MAKE) -C $(@D) install INSTALL_ROOT=$(STAGING_DIR)
-endef
-
-define QUAZIP_INSTALL_TARGET_CMDS
-	$(TARGET_MAKE_ENV) $(MAKE) -C $(@D) install INSTALL_ROOT=$(TARGET_DIR)
-endef
-
-$(eval $(generic-package))
+$(eval $(qmake-package))