qextserialport: define QEXTSERIALPORT_CONFIGURE_CMDS only once
PATCH 1/4 qextserialport: define QEXTSERIALPORT_CONFIGURE_CMDS only once
Distinguish qt/qt5 by defining QEXTSERIALPORT_QMAKE, like is done e.g. for quazip.
--- package/qextserialport/qextserialport.mk | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-)
package/qextserialport/qextserialport.mk +5 -5
@@ -12,15 +12,15 @@ QEXTSERIALPORT_INSTALL_STAGING = YES ifeq ($(BR2_PACKAGE_QT),y) QEXTSERIALPORT_DEPENDENCIES += qt -define QEXTSERIALPORT_CONFIGURE_CMDS - (cd $(@D); $(TARGET_MAKE_ENV) $(QT_QMAKE)) -endef +QEXTSERIALPORT_QMAKE = $(QT_QMAKE) else ifeq ($(BR2_PACKAGE_QT5),y) QEXTSERIALPORT_DEPENDENCIES += qt5base +QEXTSERIALPORT_QMAKE = $(QT5_QMAKE) +endif + define QEXTSERIALPORT_CONFIGURE_CMDS - (cd $(@D); $(TARGET_MAKE_ENV) $(QT5_QMAKE)) + (cd $(@D); $(TARGET_MAKE_ENV) $(QEXTSERIALPORT_QMAKE)) endef -endif define QEXTSERIALPORT_BUILD_CMDS $(TARGET_MAKE_ENV) $(MAKE) -C $(@D)
Signature · 1 line
2.10.2
PATCH 2/4 qextserialport: remove useless () around configure command
--- package/qextserialport/qextserialport.mk | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
package/qextserialport/qextserialport.mk +1 -1
@@ -19,7 +19,7 @@ QEXTSERIALPORT_QMAKE = $(QT5_QMAKE) endif define QEXTSERIALPORT_CONFIGURE_CMDS - (cd $(@D); $(TARGET_MAKE_ENV) $(QEXTSERIALPORT_QMAKE)) + cd $(@D); $(TARGET_MAKE_ENV) $(QEXTSERIALPORT_QMAKE) endef define QEXTSERIALPORT_BUILD_CMDS
Signature · 1 line
2.10.2
PATCH 3/4 qextserialport: use 'make install' to install to staging
In reply to Arnout Vandecappelle (Essensium/Mind)qmake hardcodes the path to the sysroot in the install commands, so we can't use it for target. But it's perfectly usable for staging.
By using 'make install', we get:
- the extserialport.prf file is installed to the mkspecs directory and libqextserialport.prl in staging/usr/lib, so qmake can do its magic and add the necessary compiler options;
- it also works for static build, when *.so files don't exist.
The QExtSerialPort and qextserialport.pc files are created by Buildroot so they still have to be installed explicitly. Note that upstream installs in the QtExtSerialPort directory, not QExtSerialPort, so we follow that decision.
---
Why do we have this 0002-main-include.patch anyway? It's clearly a feature patch... And apparently no attempt has been made to upstream it?
--- package/qextserialport/0002-main-include.patch | 2 +- package/qextserialport/qextserialport.mk | 6 ++---- 2 files changed, 3 insertions(+), 5 deletions(-)
package/qextserialport/0002-main-include.patch +1 -1
@@ -1,7 +1,7 @@ Create a main include file QExtSerialPort This main include file will be installed in -<QExtSerialPort/QExtSerialPort> so that Qt applications can use this +<QtExtSerialPort/QExtSerialPort> so that Qt applications can use this library by including header files in a Qt-like style. Signed-off-by: Thomas Petazzoni <[email protected]>
package/qextserialport/qextserialport.mk +2 -4
@@ -27,10 +27,8 @@ define QEXTSERIALPORT_BUILD_CMDS endef define QEXTSERIALPORT_INSTALL_STAGING_CMDS - mkdir -p $(STAGING_DIR)/usr/include/QExtSerialPort - cp $(@D)/src/*.h $(STAGING_DIR)/usr/include/QExtSerialPort/ - cp $(@D)/src/QExtSerialPort $(STAGING_DIR)/usr/include/QExtSerialPort/ - cp -a $(@D)/*.so* $(STAGING_DIR)/usr/lib/ + $(TARGET_MAKE_ENV) $(MAKE) -C $(@D) install + cp $(@D)/src/QExtSerialPort $(STAGING_DIR)/usr/include/QtExtSerialPort/ cp $(@D)/qextserialport.pc $(STAGING_DIR)/usr/lib/pkgconfig/ endef
Signature · 1 line
2.10.2
PATCH 3/4 qextserialport: use 'make install' to install to staging
Hello,
Quoted 4 lines · Arnout Vandecappelle (Essensium/Mind)
The QExtSerialPort and qextserialport.pc files are created by Buildroot so they still have to be installed explicitly. Note that upstream installs in the QtExtSerialPort directory, not QExtSerialPort, so we follow that decision.
This breaks the pkg-config use case, because the installed qextserialport.pc (added by patch 0003) is not updated to reflect the QExtSerialPort -> QtExtSerialPort include directory changed.
Quoted 3 lines
Why do we have this 0002-main-include.patch anyway? It's clearly a feature patch... And apparently no attempt has been made to upstream it?
To be honest, I don't remember. I do remember doing this patch for some reason (at the time, I was using qextserialport for a specific project), but I can't remember what the reason was (just convenience, or a real issue). Since there is no package in Buildroot that depends on qextserialport, I guess we could try to drop this patch, and see if somebody complains? :-)
Best regards,
Thomas
Signature · 3 lines
Thomas Petazzoni, CTO, Free Electrons Embedded Linux and Kernel engineering http://free-electrons.com
PATCH 4/4 qextserialport: fix static build
In reply to Arnout Vandecappelle (Essensium/Mind)By default, qextserialport will attempt to build a shared library. qesp_static has to be set in CONFIG to build static. Note that static+shared is not supported, in that case we just build shared.
The install target commands also have to be gated in that case, because the *.so files don't exist. For completeness we both set QEXTSERIALPORT_INSTALL_STAGING to NO and don't define QEXTSERIALPORT_INSTALL_TARGET_CMDS for static builds, although one of them would be sufficient.
Fixes:
http://autobuild.buildroot.net/results/c9233ad71fd60d0e6a85731a8bd4e598bd84947a
--- package/qextserialport/qextserialport.mk | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-)
package/qextserialport/qextserialport.mk +9 -1
@@ -10,6 +10,10 @@ QEXTSERIALPORT_LICENSE = MIT QEXTSERIALPORT_LICENSE_FILES = LICENSE.md QEXTSERIALPORT_INSTALL_STAGING = YES +ifeq ($(BR2_STATIC_LIBS),y) +QEXTSERIALPORT_CONF_OPTS += CONFIG+=qesp_static +endif + ifeq ($(BR2_PACKAGE_QT),y) QEXTSERIALPORT_DEPENDENCIES += qt QEXTSERIALPORT_QMAKE = $(QT_QMAKE)
@@ -19,7 +23,7 @@ QEXTSERIALPORT_QMAKE = $(QT5_QMAKE) endif define QEXTSERIALPORT_CONFIGURE_CMDS - cd $(@D); $(TARGET_MAKE_ENV) $(QEXTSERIALPORT_QMAKE) + cd $(@D); $(TARGET_MAKE_ENV) $(QEXTSERIALPORT_QMAKE) $(QEXTSERIALPORT_CONF_OPTS) endef define QEXTSERIALPORT_BUILD_CMDS
@@ -32,8 +36,12 @@ define QEXTSERIALPORT_INSTALL_STAGING_CMDS cp $(@D)/qextserialport.pc $(STAGING_DIR)/usr/lib/pkgconfig/ endef +ifeq ($(BR2_STATIC_LIBS),y) +QEXTSERIALPORT_INSTALL_STAGING = NO +else define QEXTSERIALPORT_INSTALL_TARGET_CMDS cp -a $(@D)/*.so.* $(TARGET_DIR)/usr/lib endef +endif $(eval $(generic-package))
Signature · 1 line
2.10.2
PATCH 4/4 qextserialport: fix static build
Hello,
Quoted 10 lines · Arnout Vandecappelle (Essensium/Mind)
By default, qextserialport will attempt to build a shared library.
qesp_statichas to be set in CONFIG to build static. Note that static+shared is not supported, in that case we just build shared.The install target commands also have to be gated in that case, because the *.so files don't exist. For completeness we both set
QEXTSERIALPORT_INSTALL_STAGINGto NO and don't defineQEXTSERIALPORT_INSTALL_TARGET_CMDSfor static builds, although one of them would be sufficient.
Quoted 5 lines
Fixes:
http://autobuild.buildroot.net/results/c9233ad71fd60d0e6a85731a8bd4e598bd84947aCc Thomas Petazzoni <[email protected]>
It's a bit annoying that this comes late in the series. Some of the rework in the previous patches are not technically needed to get this build fix, and with the way your patch series is organized, we have to merge more-or-less unrelated cleanups in master to get the build fix.
Patches 1/4 to 3/4 don't look too complicated, so maybe I'll merge. I'll just wait and see if people agree or not.
Best regards,
Thomas
Signature · 3 lines
Thomas Petazzoni, CTO, Free Electrons Embedded Linux and Kernel engineering http://free-electrons.com
PATCH 4/4 qextserialport: fix static build
Quoted 17 lines · Thomas Petazzoni
Hello,
Quoted 13 lines · Arnout Vandecappelle (Essensium/Mind)
By default, qextserialport will attempt to build a shared library.
qesp_statichas to be set in CONFIG to build static. Note that static+shared is not supported, in that case we just build shared.The install target commands also have to be gated in that case, because the *.so files don't exist. For completeness we both set
QEXTSERIALPORT_INSTALL_STAGINGto NO and don't defineQEXTSERIALPORT_INSTALL_TARGET_CMDSfor static builds, although one of them would be sufficient.Fixes:
http://autobuild.buildroot.net/results/c9233ad71fd60d0e6a85731a8bd4e598bd84947a
Quoted 7 lines
Quoted 2 lines
Cc Thomas Petazzoni <[email protected]>It's a bit annoying that this comes late in the series. Some of the rework in the previous patches are not technically needed to get this build fix, and with the way your patch series is organized, we have to merge more-or-less unrelated cleanups in master to get the build fix.
Well, I considered patches 1 and 2 to be safe enough to be used even for
master. Just to give you the choice, I'll respin without patches 1 and 2 (but without marking this series as superseded).
Regards,
Arnout
Quoted 8 lines
Patches 1/4 to 3/4 don't look too complicated, so maybe I'll merge. I'll just wait and see if people agree or not.
Best regards,
Thomas
Signature · 6 lines
Arnout Vandecappelle arnout at mind be Senior Embedded Software Architect +32-16-286500 Essensium/Mind http://www.mind.be G.Geenslaan 9, 3001 Leuven, Belgium BE 872 984 063 RPR Leuven LinkedIn profile: http://www.linkedin.com/in/arnoutvandecappelle GPG fingerprint: 7493 020B C7E3 8618 8DEC 222C 82EB F404 F9AC 0DDF
PATCH 4/4 qextserialport: fix static build
Hello,
Quoted 8 lines · Arnout Vandecappelle
Quoted 4 lines
It's a bit annoying that this comes late in the series. Some of the rework in the previous patches are not technically needed to get this build fix, and with the way your patch series is organized, we have to merge more-or-less unrelated cleanups in master to get the build fix.
Well, I considered patches 1 and 2 to be safe enough to be used even for
master. Just to give you the choice, I'll respin without patches 1 and 2 (but without marking this series as superseded).
Yeah, it's true 1 and 2 are quite obvious, and 3 is really needed for the build fix. I guess I'll apply as-is, no need to respin.
Thomas
Signature · 3 lines
Thomas Petazzoni, CTO, Free Electrons Embedded Linux and Kernel engineering http://free-electrons.com
PATCH 4/4 qextserialport: fix static build
Quoted 9 lines · Thomas Petazzoni
Hello,
Quoted 5 lines · Arnout Vandecappelle
Quoted 4 lines
It's a bit annoying that this comes late in the series. Some of the rework in the previous patches are not technically needed to get this build fix, and with the way your patch series is organized, we have to merge more-or-less unrelated cleanups in master to get the build fix.
Quoted 6 lines
Quoted 3 lines
Well, I considered patches 1 and 2 to be safe enough to be used even for
master. Just to give you the choice, I'll respin without patches 1 and 2 (but without marking this series as superseded).Yeah, it's true 1 and 2 are quite obvious, and 3 is really needed for the build fix. I guess I'll apply as-is, no need to respin.
Okay. You'll fix up patch 3 to modify 0003-pkgconfig.patch as well?
Regards,
Arnout
Signature · 6 lines
Arnout Vandecappelle arnout at mind be Senior Embedded Software Architect +32-16-286500 Essensium/Mind http://www.mind.be G.Geenslaan 9, 3001 Leuven, Belgium BE 872 984 063 RPR Leuven LinkedIn profile: http://www.linkedin.com/in/arnoutvandecappelle GPG fingerprint: 7493 020B C7E3 8618 8DEC 222C 82EB F404 F9AC 0DDF
PATCH 1/4 qextserialport: define QEXTSERIALPORT_CONFIGURE_CMDS only once
In reply to Arnout Vandecappelle (Essensium/Mind)Hello,
Quoted 7 lines · Arnout Vandecappelle (Essensium/Mind)
Distinguish qt/qt5 by defining
QEXTSERIALPORT_QMAKE, like is done e.g. for quazip.--- package/qextserialport/qextserialport.mk | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-)
All four patches applied to master. On PATCH 3/4, I've fixed up 0003-pkgconfig.patch to take into account the modified header installation location.
Thanks!
Thomas
Signature · 3 lines
Thomas Petazzoni, CTO, Free Electrons Embedded Linux, Kernel and Android engineering http://free-electrons.com