package/openjpeg: fix static build

PATCH New Samuel Martin 2 participants 6 messages Nov 6, 2016 – Nov 13, 2016

lore

Share & tools

lore

Samuel Martin

author

2016-11-06 16:35 UTC #0 lore raw

PATCH 1/1 package/openjpeg: fix static build

This change adds a patch to openjpeg fixing the tiff indirect dependencies in case of static build.

A similar patch for upstream master has been submitted [1].

Fixes:
http://autobuild.buildroot.net/results/d0d/d0d22727311d6300e0e400728126170407bfd699/
and many others...

[1] https://github.com/uclouvain/openjpeg/pull/866

Signed-off-by Samuel Martin <[email protected]>

Cc Olivier Schonken <[email protected]>

---
 ...tiff-append-flags-found-by-pkg-config-if-.patch | 72 ++++++++++++++++++++++
 1 file changed, 72 insertions(+)
 create mode 100644 package/openjpeg/0001-thirdparty-tiff-append-flags-found-by-pkg-config-if-.patch

package/openjpeg/0001-thirdparty-tiff-append-flags-found-by-pkg-config-if-.patch new file +72 -0

new file mode 100644
index 0000000..be50902
@@ -0,0 +1,72 @@
+From 38f50c7d9ad3ba06b64583045665203afb53cbd9 Mon Sep 17 00:00:00 2001
+From: Samuel Martin <[email protected]>
+Date: Sun, 6 Nov 2016 16:29:08 +0100
+Subject: [PATCH] thirdparty: tiff: append flags found by pkg-config if
+ available
+
+This change allows to get all required CFLAGS/LDFLAGS in case of static only
+build.
+
+This build issue [1] was triggered by the Buildroot farms.
+
+[1] http://autobuild.buildroot.net/results/d0d/d0d22727311d6300e0e400728126170407bfd699/build-end.log
+
+Signed-off-by: Samuel Martin <[email protected]>
+---
+ thirdparty/CMakeLists.txt | 23 +++++++++++++++++++++--
+ 1 file changed, 21 insertions(+), 2 deletions(-)
+
+diff --git a/thirdparty/CMakeLists.txt b/thirdparty/CMakeLists.txt
+index d1e68ca..225f51d 100644
+--- a/thirdparty/CMakeLists.txt
++++ b/thirdparty/CMakeLists.txt
+@@ -1,5 +1,9 @@
+ # 3rd party libs
+ 
++IF(NOT BUILD_THIRDPARTY)
++  include(FindPkgConfig)
++ENDIF(NOT BUILD_THIRDPARTY)
++
+ #------------
+ # Try to find lib Z
+ IF(BUILD_THIRDPARTY)
+@@ -35,6 +39,9 @@ IF(BUILD_THIRDPARTY)
+   SET(PNG_INCLUDE_DIRNAME ${OPENJPEG_SOURCE_DIR}/thirdparty/libpng PARENT_SCOPE)
+ ELSE (BUILD_THIRDPARTY)
+   IF (ZLIB_FOUND)
++    # Static only build:
++    #   it is not necessary to invoke pkg_check_module on libpng, because libpng
++    #   only depends on zlib, which is already checked.
+     FIND_PACKAGE(PNG)
+     IF(PNG_FOUND)
+       message(STATUS "Your system seems to have a PNG lib available, we will use it")
+@@ -66,12 +73,24 @@ IF(BUILD_THIRDPARTY)
+   SET(OPJ_HAVE_LIBTIFF 1 PARENT_SCOPE)
+ ELSE (BUILD_THIRDPARTY)
+   FIND_PACKAGE(TIFF)
++  # Static only build:
++  #   it is necessary to invoke pkg_check_module on libtiff since it may have
++  #   several other dependencies not declared by its cmake module, but they are
++  #   in the its pkgconfig module.
++  IF(PKG_CONFIG_FOUND)
++    FOREACH(pc_tiff_module tiff tiff3 tiff4 tiff-3 tiff-4 libtiff libtiff3 libtiff4 libtiff-3 libtiff-4)
++      pkg_check_modules(PC_TIFF QUIET ${pc_tiff_module})
++      IF(PC_TIFF_FOUND)
++        break()
++      ENDIF(PC_TIFF_FOUND)
++    ENDFOREACH()
++  ENDIF(PKG_CONFIG_FOUND)
+   IF(TIFF_FOUND)
+     message(STATUS "Your system seems to have a TIFF lib available, we will use it")
+     SET(OPJ_HAVE_TIFF_H 1 PARENT_SCOPE)
+     SET(OPJ_HAVE_LIBTIFF 1 PARENT_SCOPE)
+-    SET(TIFF_LIBNAME ${TIFF_LIBRARIES} PARENT_SCOPE)
+-    SET(TIFF_INCLUDE_DIRNAME ${TIFF_INCLUDE_DIR} PARENT_SCOPE)
++    SET(TIFF_LIBNAME ${TIFF_LIBRARIES} ${PC_TIFF_STATIC_LIBRARIES} PARENT_SCOPE)
++    SET(TIFF_INCLUDE_DIRNAME ${TIFF_INCLUDE_DIR} ${PC_TIFF_STATIC_INCLUDE_DIRS} PARENT_SCOPE)
+   ELSE (TIFF_FOUND) # not found
+     SET(OPJ_HAVE_TIFF_H 0 PARENT_SCOPE)
+     SET(OPJ_HAVE_LIBTIFF 0 PARENT_SCOPE)
+-- 
+2.10.2
+

Signature · 1 line

2.10.2

Thomas Petazzoni

2016-11-06 21:50 UTC #1 lore raw

PATCH 1/1 package/openjpeg: fix static build

Hello,

package/openjpeg/0001-thirdparty-tiff-append-flags-found-by-pkg-config-if-.patch line 39

Quoted 9 lines · Samuel Martin

+ # Try to find lib Z
+ IF(BUILD_THIRDPARTY)
+@@ -35,6 +39,9 @@ IF(BUILD_THIRDPARTY)
package/openjpeg/0001-thirdparty-tiff-append-flags-found-by-pkg-config-if-.patch new file +6 -0
+   SET(PNG_INCLUDE_DIRNAME ${OPENJPEG_SOURCE_DIR}/thirdparty/libpng PARENT_SCOPE)
+ ELSE (BUILD_THIRDPARTY)
+   IF (ZLIB_FOUND)
++    # Static only build:
++    #   it is not necessary to invoke pkg_check_module on libpng, because libpng
++    #   only depends on zlib, which is already checked.

This is correct today, but in the future? The whole point of pkg-config is to not make this sort of assumption.

package/openjpeg/0001-thirdparty-tiff-append-flags-found-by-pkg-config-if-.patch line 52

Quoted 13 lines

package/openjpeg/0001-thirdparty-tiff-append-flags-found-by-pkg-config-if-.patch new file +13 -0

+     FIND_PACKAGE(PNG)
+     IF(PNG_FOUND)
+       message(STATUS "Your system seems to have a PNG lib available, we will use it")
+@@ -66,12 +73,24 @@ IF(BUILD_THIRDPARTY)
+   SET(OPJ_HAVE_LIBTIFF 1 PARENT_SCOPE)
+ ELSE (BUILD_THIRDPARTY)
+   FIND_PACKAGE(TIFF)
++  # Static only build:
++  #   it is necessary to invoke pkg_check_module on libtiff since it may have
++  #   several other dependencies not declared by its cmake module, but they are
++  #   in the its pkgconfig module.
++  IF(PKG_CONFIG_FOUND)
++    FOREACH(pc_tiff_module tiff tiff3 tiff4 tiff-3 tiff-4 libtiff libtiff3 libtiff4 libtiff-3 libtiff-4)

I fail to understand why one needs to call both FIND_PACKAGE(TIFF) and then check again with PKG_CHECK_MODULES(). Isn't the latter sufficient?

Best regards,

Thomas

Signature · 3 lines

Thomas Petazzoni, CTO, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com

Samuel Martin

author

2016-11-07 05:17 UTC #2 lore raw

PATCH 1/1 package/openjpeg: fix static build

Hi Thomas, all,

Quoted 15 lines · Thomas Petazzoni

Hello,
package/openjpeg/0001-thirdparty-tiff-append-flags-found-by-pkg-config-if-.patch line 39
Quoted 9 lines · Samuel Martin
+ # Try to find lib Z
+ IF(BUILD_THIRDPARTY)
+@@ -35,6 +39,9 @@ IF(BUILD_THIRDPARTY)
package/openjpeg/0001-thirdparty-tiff-append-flags-found-by-pkg-config-if-.patch new file +6 -0
+   SET(PNG_INCLUDE_DIRNAME ${OPENJPEG_SOURCE_DIR}/thirdparty/libpng PARENT_SCOPE)
+ ELSE (BUILD_THIRDPARTY)
+   IF (ZLIB_FOUND)
++    # Static only build:
++    #   it is not necessary to invoke pkg_check_module on libpng, because libpng
++    #   only depends on zlib, which is already checked.

Quoted 2 lines

This is correct today, but in the future? The whole point of pkg-config is to not make this sort of assumption.

Some cmake modules are so poorly written, they only set the flags relative to the package itself (i.e. setting LDFLAGS="-L/usr/lib -lfoo" for package foo), and completly ignore/miss/forget the indirect dependencies that are required in the case of static build. However, cmake can leverage pkgconfig to set all these flags, so the proper fix is indeed using it in the module. But that's another story for another patch... ;-)

A future-proof fix is fixing the FindTIFF.cmake module from the cmake package itself, making it using pkgconfig to get all the libs in the LDFLAGS instead of only -ltiff. This is not easily possible since we conditionally build the host-cmake package (patching host-cmake would mean no-op-ing commit c2d80a8c5d8b97cdc84c297a3d2d6896fff6560b).

Quoted 15 lines

Quoted 4 lines
package/openjpeg/0001-thirdparty-tiff-append-flags-found-by-pkg-config-if-.patch new file +3 -0
+     FIND_PACKAGE(PNG)
+     IF(PNG_FOUND)
+       message(STATUS "Your system seems to have a PNG lib available, we will use it")
package/openjpeg/0001-thirdparty-tiff-append-flags-found-by-pkg-config-if-.patch line 52
Quoted 10 lines
package/openjpeg/0001-thirdparty-tiff-append-flags-found-by-pkg-config-if-.patch new file +10 -0
+@@ -66,12 +73,24 @@ IF(BUILD_THIRDPARTY)
+   SET(OPJ_HAVE_LIBTIFF 1 PARENT_SCOPE)
+ ELSE (BUILD_THIRDPARTY)
+   FIND_PACKAGE(TIFF)
++  # Static only build:
++  #   it is necessary to invoke pkg_check_module on libtiff since it may have
++  #   several other dependencies not declared by its cmake module, but they are
++  #   in the its pkgconfig module.
++  IF(PKG_CONFIG_FOUND)
++    FOREACH(pc_tiff_module tiff tiff3 tiff4 tiff-3 tiff-4 libtiff libtiff3 libtiff4 libtiff-3 libtiff-4)

Quoted 2 lines

I fail to understand why one needs to call both FIND_PACKAGE(TIFF) and then check again with PKG_CHECK_MODULES(). Isn't the latter sufficient?

Indeed FIND_PACKAGE(TIFF) is redundant with what PKG_CHECK_MODULES() does. But to give a chance to this patch to get upstream I have to take care platform missing pkgconfig (yeah! it exists :( ...), so FIND_PACKAGE() is always called, and PKG_CHECK_MODULES() is called only when available (mea culpa, the commit log does not reflect this).

Regards,

Signature · 1 line

Samuel

Thomas Petazzoni

2016-11-07 08:19 UTC #3 lore raw

PATCH 1/1 package/openjpeg: fix static build

Hello,

Quoted 21 lines · Samuel Martin

Quoted 10 lines
package/openjpeg/0001-thirdparty-tiff-append-flags-found-by-pkg-config-if-.patch line 39
Quoted 9 lines
+ # Try to find lib Z
+ IF(BUILD_THIRDPARTY)
+@@ -35,6 +39,9 @@ IF(BUILD_THIRDPARTY)
package/openjpeg/0001-thirdparty-tiff-append-flags-found-by-pkg-config-if-.patch new file +6 -0
+   SET(PNG_INCLUDE_DIRNAME ${OPENJPEG_SOURCE_DIR}/thirdparty/libpng PARENT_SCOPE)
+ ELSE (BUILD_THIRDPARTY)
+   IF (ZLIB_FOUND)
++    # Static only build:
++    #   it is not necessary to invoke pkg_check_module on libpng, because libpng
++    #   only depends on zlib, which is already checked.  
Quoted 2 lines
This is correct today, but in the future? The whole point of pkg-config is to not make this sort of assumption.
Some cmake modules are so poorly written, they only set the flags relative to the package itself (i.e. setting LDFLAGS="-L/usr/lib -lfoo" for package foo), and completly ignore/miss/forget the indirect dependencies that are required in the case of static build. However, cmake can leverage pkgconfig to set all these flags, so the proper fix is indeed using it in the module. But that's another story for another patch... ;-)

Quoted 5 lines

A future-proof fix is fixing the FindTIFF.cmake module from the cmake package itself, making it using pkgconfig to get all the libs in the LDFLAGS instead of only -ltiff. This is not easily possible since we conditionally build the host-cmake package (patching host-cmake would mean no-op-ing commit c2d80a8c5d8b97cdc84c297a3d2d6896fff6560b).

This I understand. What I don't understand is your reasoning of "for libpng, we don't need to use pkg-config, because the only indirect dependency is zlib, which is already checked". This is not good, as you make an assumption on which indirect libraries libpng might use.

Even though I agree libpng is unlikely to change and grow additional dependencies, you should also use pkg-config on libpng just like you're doing on libtiff.

Quoted 8 lines

Quoted 2 lines
I fail to understand why one needs to call both FIND_PACKAGE(TIFF) and then check again with PKG_CHECK_MODULES(). Isn't the latter sufficient?
Indeed FIND_PACKAGE(TIFF) is redundant with what PKG_CHECK_MODULES() does. But to give a chance to this patch to get upstream I have to take care platform missing pkgconfig (yeah! it exists :( ...), so FIND_PACKAGE() is always called, and PKG_CHECK_MODULES() is called only when available (mea culpa, the commit log does not reflect this).

OK, makes sense.

Thanks for the explanation,

Thomas

Signature · 3 lines

Thomas Petazzoni, CTO, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com

Samuel Martin

author

2016-11-07 08:42 UTC #4 lore raw

PATCH 1/1 package/openjpeg: fix static build

Quoted 31 lines · Thomas Petazzoni

Hello,
Quoted 21 lines · Samuel Martin
Quoted 10 lines
package/openjpeg/0001-thirdparty-tiff-append-flags-found-by-pkg-config-if-.patch line 39
Quoted 9 lines
+ # Try to find lib Z
+ IF(BUILD_THIRDPARTY)
+@@ -35,6 +39,9 @@ IF(BUILD_THIRDPARTY)
package/openjpeg/0001-thirdparty-tiff-append-flags-found-by-pkg-config-if-.patch new file +6 -0
+   SET(PNG_INCLUDE_DIRNAME ${OPENJPEG_SOURCE_DIR}/thirdparty/libpng PARENT_SCOPE)
+ ELSE (BUILD_THIRDPARTY)
+   IF (ZLIB_FOUND)
++    # Static only build:
++    #   it is not necessary to invoke pkg_check_module on libpng, because libpng
++    #   only depends on zlib, which is already checked.
Quoted 2 lines
This is correct today, but in the future? The whole point of pkg-config is to not make this sort of assumption.
Some cmake modules are so poorly written, they only set the flags relative to the package itself (i.e. setting LDFLAGS="-L/usr/lib -lfoo" for package foo), and completly ignore/miss/forget the indirect dependencies that are required in the case of static build. However, cmake can leverage pkgconfig to set all these flags, so the proper fix is indeed using it in the module. But that's another story for another patch... ;-)
Quoted 5 lines
A future-proof fix is fixing the FindTIFF.cmake module from the cmake package itself, making it using pkgconfig to get all the libs in the LDFLAGS instead of only -ltiff. This is not easily possible since we conditionally build the host-cmake package (patching host-cmake would mean no-op-ing commit c2d80a8c5d8b97cdc84c297a3d2d6896fff6560b).

Quoted 4 lines

This I understand. What I don't understand is your reasoning of "for libpng, we don't need to use pkg-config, because the only indirect dependency is zlib, which is already checked". This is not good, as you make an assumption on which indirect libraries libpng might use.

Ha! Sorry, I did not get you were talking about png here :s

Quoted 4 lines

Even though I agree libpng is unlikely to change and grow additional dependencies, you should also use pkg-config on libpng just like you're doing on libtiff.

As you said, I don't think it's gonna change, plus I may be sometime lazy! ;)

Quoted 19 lines

Quoted 9 lines
Quoted 3 lines
I fail to understand why one needs to call both FIND_PACKAGE(TIFF) and then check again with PKG_CHECK_MODULES(). Isn't the latter sufficient?
Indeed FIND_PACKAGE(TIFF) is redundant with what PKG_CHECK_MODULES() does. But to give a chance to this patch to get upstream I have to take care platform missing pkgconfig (yeah! it exists :( ...), so FIND_PACKAGE() is always called, and PKG_CHECK_MODULES() is called only when available (mea culpa, the commit log does not reflect this).
OK, makes sense.
Thanks for the explanation,
Thomas
--
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com

Signature · 1 line

Samuel

Thomas Petazzoni

Applied

2016-11-13 13:29 UTC #5 lore raw

PATCH 1/1 package/openjpeg: fix static build

In reply to Samuel Martin

Hello,

Quoted 9 lines · Samuel Martin

This change adds a patch to openjpeg fixing the tiff indirect dependencies in case of static build.
A similar patch for upstream master has been submitted [1].
Fixes:
http://autobuild.buildroot.net/results/d0d/d0d22727311d6300e0e400728126170407bfd699/
and many others...

Quoted 8 lines

[1] https://github.com/uclouvain/openjpeg/pull/866
Signed-off-by Samuel Martin <[email protected]>
Cc Olivier Schonken <[email protected]>
---
 ...tiff-append-flags-found-by-pkg-config-if-.patch | 72 ++++++++++++++++++++++
 1 file changed, 72 insertions(+)
 create mode 100644 package/openjpeg/0001-thirdparty-tiff-append-flags-found-by-pkg-config-if-.patch

Applied to master, thanks.

Thomas

Signature · 3 lines

Thomas Petazzoni, CTO, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com