[Buildroot] [PATCH 1/4] qextserialport: define QEXTSERIALPORT_CONFIGURE_CMDS only once

[Buildroot] [PATCH 1/4] qextserialport: define QEXTSERIALPORT_CONFIGURE_CMDS only once

[Arnout Vandecappelle]

Distinguish qt/qt5 by defining QEXTSERIALPORT_QMAKE, like is done e.g.
for quazip.

Signed-off-by: Arnout Vandecappelle (Essensium/Mind) <arnout@mind.be>
---
 package/qextserialport/qextserialport.mk | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/package/qextserialport/qextserialport.mk b/package/qextserialport/qextserialport.mk
index 39e0407..11b1cc8 100644
--- a/package/qextserialport/qextserialport.mk
+++ b/package/qextserialport/qextserialport.mk
@@ -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)
-- 
2.10.2

[Buildroot] [PATCH 2/4] qextserialport: remove useless () around configure command

[Arnout Vandecappelle]

Signed-off-by: Arnout Vandecappelle (Essensium/Mind) <arnout@mind.be>
---
 package/qextserialport/qextserialport.mk | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/package/qextserialport/qextserialport.mk b/package/qextserialport/qextserialport.mk
index 11b1cc8..f48f596 100644
--- a/package/qextserialport/qextserialport.mk
+++ b/package/qextserialport/qextserialport.mk
@@ -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
-- 
2.10.2

[Buildroot] [PATCH 3/4] qextserialport: use 'make install' to install to staging

[Arnout Vandecappelle]

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.

Signed-off-by: Arnout Vandecappelle (Essensium/Mind) <arnout@mind.be>
Cc: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
---
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(-)

diff --git a/package/qextserialport/0002-main-include.patch b/package/qextserialport/0002-main-include.patch
index 858f335..27f67f6 100644
--- a/package/qextserialport/0002-main-include.patch
+++ b/package/qextserialport/0002-main-include.patch
@@ -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 <thomas.petazzoni@free-electrons.com>
diff --git a/package/qextserialport/qextserialport.mk b/package/qextserialport/qextserialport.mk
index f48f596..15cb74c 100644
--- a/package/qextserialport/qextserialport.mk
+++ b/package/qextserialport/qextserialport.mk
@@ -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
 
-- 
2.10.2

[Buildroot] [PATCH 4/4] qextserialport: fix static build

[Arnout Vandecappelle]

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

Signed-off-by: Arnout Vandecappelle (Essensium/Mind) <arnout@mind.be>
Cc: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
---
 package/qextserialport/qextserialport.mk | 10 +++++++++-
 1 file changed, 9 insertions(+), 1 deletion(-)

diff --git a/package/qextserialport/qextserialport.mk b/package/qextserialport/qextserialport.mk
index 15cb74c..3641353 100644
--- a/package/qextserialport/qextserialport.mk
+++ b/package/qextserialport/qextserialport.mk
@@ -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))
-- 
2.10.2

[Buildroot] [PATCH 3/4] qextserialport: use 'make install' to install to staging

[Thomas Petazzoni]

Hello,

On Sun, 20 Nov 2016 23:13:28 +0100, Arnout Vandecappelle
(Essensium/Mind) wrote:

> 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.

> 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
-- 
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com

[Buildroot] [PATCH 4/4] qextserialport: fix static build

[Thomas Petazzoni]

Hello,

On Sun, 20 Nov 2016 23:13:29 +0100, Arnout Vandecappelle
(Essensium/Mind) wrote:
> 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
> 
> Signed-off-by: Arnout Vandecappelle (Essensium/Mind) <arnout@mind.be>
> Cc: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>

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
-- 
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com

[Buildroot] [PATCH 4/4] qextserialport: fix static build

[Arnout Vandecappelle]

On 21-11-16 21:41, Thomas Petazzoni wrote:
> Hello,
> 
> On Sun, 20 Nov 2016 23:13:29 +0100, Arnout Vandecappelle
> (Essensium/Mind) wrote:
>> 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
>>
>> Signed-off-by: Arnout Vandecappelle (Essensium/Mind) <arnout@mind.be>
>> Cc: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
> 
> 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

> 
> 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
> 

-- 
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

[Buildroot] [PATCH 4/4] qextserialport: fix static build

[Thomas Petazzoni]

Hello,

On Mon, 21 Nov 2016 21:53:51 +0100, Arnout Vandecappelle wrote:

> > 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
-- 
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com

[Buildroot] [PATCH 4/4] qextserialport: fix static build

[Arnout Vandecappelle]

On 21-11-16 22:00, Thomas Petazzoni wrote:
> Hello,
> 
> On Mon, 21 Nov 2016 21:53:51 +0100, Arnout Vandecappelle wrote:
> 
>>> 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.

 Okay. You'll fix up patch 3 to modify 0003-pkgconfig.patch as well?

 Regards,
 Arnout

-- 
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

[Buildroot] [PATCH 1/4] qextserialport: define QEXTSERIALPORT_CONFIGURE_CMDS only once

[Thomas Petazzoni]

Hello,

On Sun, 20 Nov 2016 23:13:26 +0100, Arnout Vandecappelle
(Essensium/Mind) wrote:
> Distinguish qt/qt5 by defining QEXTSERIALPORT_QMAKE, like is done e.g.
> for quazip.
> 
> Signed-off-by: Arnout Vandecappelle (Essensium/Mind) <arnout@mind.be>
> ---
>  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
-- 
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com