[Buildroot] [PATCH 0/2] Fix linking gerbera with taglib

[Buildroot] [PATCH 0/2] Fix linking gerbera with taglib

[Jörg Krause]

This series fixes linking gerbera with taglib.

The first commit adds an upstream patch to tagblib, which fixes `taglib-config`
to output the correct paths when cross-compiling.

The second commit add an upstream patch to gerbera, which fixes the
behaviour of `find_program(taglib-config)` when cross-compiling. By
default, `find_program()` only searches on the host for binaries.
However, in the case of `taglib-config` it should only search in the
sysroot path for `taglib-config`.

Note, that the issue has been detected by the autobuilder for version 1.3.0 of
gerbera (current version is 1.3.1), but it is still present.

Fixes:
http://autobuild.buildroot.org/results/e95/e9548f8d43abe3a2f2aab4acaa19289af2b9737d
http://autobuild.buildroot.org/results/efa/efa203e07c2dd5aed09c0ed23e3bd56fa2dadd2a

J?rg Krause (2):
  package/taglib: add upstream patch to fix taglib-config
  package/gerbera: add upstream patch to fix find_program with
    taglib-config

 ...m-taglib-config-when-cross-compiling.patch | 46 +++++++++++++
 ...-config-file-for-cross-compiling-906.patch | 66 +++++++++++++++++++
 2 files changed, 112 insertions(+)
 create mode 100644 package/gerbera/0002-Fix-find_program-taglib-config-when-cross-compiling.patch
 create mode 100644 package/taglib/0001-fix-taglib-config-file-for-cross-compiling-906.patch

-- 
2.22.0

[Buildroot] [PATCH 1/2] package/taglib: add upstream patch to fix taglib-config

[Jörg Krause]

The current taglib-config program does not work when cross-compiling as it only
returns paths to the host, which breaks building programs which link against
taglib.

For example gerbera uses `taglib-config` and it fails with:

```
[..]
-- Found TagLib: -L/usr/lib -ltag
[..]
arm-linux-gnueabihf-g++: ERROR: unsafe header/library path used in cross-compilation: '-L/usr/lib'
```

Before the patch the output of `taglib-config` is:
```
$ ./output/host/arm-buildroot-linux-gnueabihf/sysroot/usr/bin/taglib-config --libs
-L/usr/lib -ltag
```

Add a patch from upstream which fixes taglib-config.

After applying the fix, the pkg-config file is correct:
```
$ ./output/host/arm-buildroot-linux-gnueabihf/sysroot/usr/bin/taglib-config --libs
-L/home/joerg/Development/git/buildroot/output/host/arm-buildroot-linux-gnueabihf/sysroot/usr/lib -ltag
```

Signed-off-by: J?rg Krause <joerg.krause@embedded.rocks>
---
 ...-config-file-for-cross-compiling-906.patch | 66 +++++++++++++++++++
 1 file changed, 66 insertions(+)
 create mode 100644 package/taglib/0001-fix-taglib-config-file-for-cross-compiling-906.patch

diff --git a/package/taglib/0001-fix-taglib-config-file-for-cross-compiling-906.patch b/package/taglib/0001-fix-taglib-config-file-for-cross-compiling-906.patch
new file mode 100644
index 0000000000..2c6ebd74db
--- /dev/null
+++ b/package/taglib/0001-fix-taglib-config-file-for-cross-compiling-906.patch
@@ -0,0 +1,66 @@
+From 7470f92a67375d00e53b3785a88fa7b26ad6f1da Mon Sep 17 00:00:00 2001
+From: =?UTF-8?q?J=C3=B6rg=20Krause?= <joerg.krause@embedded.rocks>
+Date: Fri, 17 May 2019 13:13:35 +0200
+Subject: [PATCH] fix taglib-config file for cross compiling (#906)
+MIME-Version: 1.0
+Content-Type: text/plain; charset=UTF-8
+Content-Transfer-Encoding: 8bit
+
+The current taglib-config program does not work for cross-compiling as it only
+returns the paths to the host, which breaks building programs which uses
+`taglib-config` to link against taglib.
+
+Fix this by passing sysroot to the `prefix` and `exec_prefix` fields.
+
+Backported from: 7470f92a67375d00e53b3785a88fa7b26ad6f1da
+
+Signed-off-by: J?rg Krause <joerg.krause@embedded.rocks>
+---
+ CMakeLists.txt      |  2 +-
+ taglib-config.cmake | 10 +++++-----
+ 2 files changed, 6 insertions(+), 6 deletions(-)
+
+diff --git a/CMakeLists.txt b/CMakeLists.txt
+index 2de06324..1a0302c4 100644
+--- a/CMakeLists.txt
++++ b/CMakeLists.txt
+@@ -105,7 +105,7 @@ math(EXPR TAGLIB_SOVERSION_PATCH "${TAGLIB_SOVERSION_REVISION}")
+ include(ConfigureChecks.cmake)
+ 
+ if(NOT WIN32)
+-  configure_file("${CMAKE_CURRENT_SOURCE_DIR}/taglib-config.cmake" "${CMAKE_CURRENT_BINARY_DIR}/taglib-config")
++  configure_file("${CMAKE_CURRENT_SOURCE_DIR}/taglib-config.cmake" "${CMAKE_CURRENT_BINARY_DIR}/taglib-config" @ONLY)
+   install(PROGRAMS "${CMAKE_CURRENT_BINARY_DIR}/taglib-config" DESTINATION "${BIN_INSTALL_DIR}")
+ endif()
+ 
+diff --git a/taglib-config.cmake b/taglib-config.cmake
+index 2bc2811a..96ef6883 100644
+--- a/taglib-config.cmake
++++ b/taglib-config.cmake
+@@ -14,10 +14,10 @@ EOH
+ 	exit 1;
+ }
+ 
+-prefix=${CMAKE_INSTALL_PREFIX}
+-exec_prefix=${CMAKE_INSTALL_PREFIX}
+-libdir=${LIB_INSTALL_DIR}
+-includedir=${INCLUDE_INSTALL_DIR}
++prefix=@CMAKE_SYSROOT@@CMAKE_INSTALL_PREFIX@
++exec_prefix=@CMAKE_SYSROOT@@CMAKE_INSTALL_PREFIX@
++libdir=${exec_prefix}/lib
++includedir=${prefix}/include
+ 
+ flags=""
+ 
+@@ -35,7 +35,7 @@ do
+ 	  flags="$flags -I$includedir/taglib"
+ 	  ;;
+     --version)
+-	  echo ${TAGLIB_LIB_VERSION_STRING}
++	  echo @TAGLIB_LIB_VERSION_STRING@
+ 	  ;;
+     --prefix)
+ 	  echo $prefix
+-- 
+2.22.0
+
-- 
2.22.0

[Buildroot] [PATCH 2/2] package/gerbera: add upstream patch to fix find_program with taglib-config

[Jörg Krause]

Add an from from upstream to fix linking Gerbera with taglib. The patch
fixes the following issue:

When cross-compiling CMakes `find_program()` will search for binaries on the
host. This is typically correct, e.g. when search for compilers or shells.

When cross-compiling, searching for `taglib-config` using `find_program` should
not find the binary on the host, instead it should find the binary in the sysroot
directory, as the host `taglib-config` will output the wrong host paths
and libs, whereas the sysroot `taglib-config` will output the correct sysroot
paths and libs.

Therefore, use the `CMAKE_FIND_ROOT_PATH_MODE_PROGRAM` variable when
cross-compiling. This variable controls whether the `CMAKE_FIND_ROOT_PATH`
and `CMAKE_SYSROOT` are used by find_program().

Signed-off-by: J?rg Krause <joerg.krause@embedded.rocks>
---
 ...m-taglib-config-when-cross-compiling.patch | 46 +++++++++++++++++++
 1 file changed, 46 insertions(+)
 create mode 100644 package/gerbera/0002-Fix-find_program-taglib-config-when-cross-compiling.patch

diff --git a/package/gerbera/0002-Fix-find_program-taglib-config-when-cross-compiling.patch b/package/gerbera/0002-Fix-find_program-taglib-config-when-cross-compiling.patch
new file mode 100644
index 0000000000..d5cc908e22
--- /dev/null
+++ b/package/gerbera/0002-Fix-find_program-taglib-config-when-cross-compiling.patch
@@ -0,0 +1,46 @@
+From 0f78b883f02651bd15a96a7294e01f94b28c20f8 Mon Sep 17 00:00:00 2001
+From: =?UTF-8?q?J=C3=B6rg=20Krause?= <joerg.krause@embedded.rocks>
+Date: Mon, 13 May 2019 13:42:14 +0200
+Subject: [PATCH] Fix find_program taglib-config when cross-compiling
+MIME-Version: 1.0
+Content-Type: text/plain; charset=UTF-8
+Content-Transfer-Encoding: 8bit
+
+When cross-compiling Gerbera CMakes `find_program()` will search for
+binaries on the host. This is typically correct, e.g. when search for
+compilers or shells.
+
+When cross-compiling searching for `taglib-config` should not find the
+binary on the host, instead it should find the binary in the sysroot
+directory, as the host `taglib-config` will output the wrong host paths
+and libs, whereas the sysroot `taglib-config` will output the correct sysroot
+paths and libs.
+
+Therefore, use the `CMAKE_FIND_ROOT_PATH_MODE_PROGRAM` variable when
+cross-compiling. This variable controls whether the `CMAKE_FIND_ROOT_PATH`
+and `CMAKE_SYSROOT` are used by find_program().
+
+Backported from: 0f78b883f02651bd15a96a7294e01f94b28c20f8
+
+Signed-off-by: J?rg Krause <joerg.krause@embedded.rocks>
+---
+ cmake/FindTaglib.cmake | 3 +++
+ 1 file changed, 3 insertions(+)
+
+diff --git a/cmake/FindTaglib.cmake b/cmake/FindTaglib.cmake
+index 46a20ff2..7135d766 100644
+--- a/cmake/FindTaglib.cmake
++++ b/cmake/FindTaglib.cmake
+@@ -19,6 +19,9 @@ ELSE()
+ 	endif(NOT TAGLIB_MIN_VERSION)
+ 
+ 	if(NOT WIN32)
++            if (CMAKE_CROSSCOMPILING)
++              set(CMAKE_FIND_ROOT_PATH_MODE_PROGRAM ONLY)
++            endif(CMAKE_CROSSCOMPILING)
+             find_program(TAGLIBCONFIG_EXECUTABLE NAMES taglib-config PATHS
+ 		   ${BIN_INSTALL_DIR}
+             )
+-- 
+2.22.0
+
-- 
2.22.0

[Buildroot] [PATCH 0/2] Fix linking gerbera with taglib

[Arnout Vandecappelle]

On 10/06/2019 23:20, J?rg Krause wrote:
> This series fixes linking gerbera with taglib.
> 
> The first commit adds an upstream patch to tagblib, which fixes `taglib-config`
> to output the correct paths when cross-compiling.
> 
> The second commit add an upstream patch to gerbera, which fixes the
> behaviour of `find_program(taglib-config)` when cross-compiling. By
> default, `find_program()` only searches on the host for binaries.
> However, in the case of `taglib-config` it should only search in the
> sysroot path for `taglib-config`.

 I think both patches are wrong/incomplete. Still, because they are upstream,
and they fix the issue, I've pushed to master, thanks.

 I'll comment in the individual patches why I think they are wrong.

 Regards,
 Arnout

> 
> Note, that the issue has been detected by the autobuilder for version 1.3.0 of
> gerbera (current version is 1.3.1), but it is still present.
> 
> Fixes:
> http://autobuild.buildroot.org/results/e95/e9548f8d43abe3a2f2aab4acaa19289af2b9737d
> http://autobuild.buildroot.org/results/efa/efa203e07c2dd5aed09c0ed23e3bd56fa2dadd2a
> 
> J?rg Krause (2):
>   package/taglib: add upstream patch to fix taglib-config
>   package/gerbera: add upstream patch to fix find_program with
>     taglib-config
> 
>  ...m-taglib-config-when-cross-compiling.patch | 46 +++++++++++++
>  ...-config-file-for-cross-compiling-906.patch | 66 +++++++++++++++++++
>  2 files changed, 112 insertions(+)
>  create mode 100644 package/gerbera/0002-Fix-find_program-taglib-config-when-cross-compiling.patch
>  create mode 100644 package/taglib/0001-fix-taglib-config-file-for-cross-compiling-906.patch
> 

[Buildroot] [PATCH 1/2] package/taglib: add upstream patch to fix taglib-config

[Arnout Vandecappelle]

On 10/06/2019 23:20, J?rg Krause wrote:
> +-prefix=${CMAKE_INSTALL_PREFIX}
> +-exec_prefix=${CMAKE_INSTALL_PREFIX}
> +-libdir=${LIB_INSTALL_DIR}
> +-includedir=${INCLUDE_INSTALL_DIR}
> ++prefix=@CMAKE_SYSROOT@@CMAKE_INSTALL_PREFIX@
> ++exec_prefix=@CMAKE_SYSROOT@@CMAKE_INSTALL_PREFIX@

 I believe it is wrong to include CMAKE_SYSROOT here. In general, this is not
necessarily the place where the package will be installed. We happen to do that
now, but for example both OpenWrt and OpenEmbedded install with a different
DESTDIR, and dependent packages see a different sysroot.

 We have the TAGLIB_CONFIG_SCRIPT infra to handle this situation. It will add
the appropriate sysroot to the _prefix variables.

 Regards,
 Arnout

> ++libdir=${exec_prefix}/lib
> ++includedir=${prefix}/include

[Buildroot] [PATCH 2/2] package/gerbera: add upstream patch to fix find_program with taglib-config

[Arnout Vandecappelle]

On 10/06/2019 23:20, J?rg Krause wrote:
[snip]
> ++            if (CMAKE_CROSSCOMPILING)
> ++              set(CMAKE_FIND_ROOT_PATH_MODE_PROGRAM ONLY)

 Again, this is wrong IMO. The executables in the sysroot are not supposed to be
executed on the host.

 What we really should do is to copy those config scripts to some other
location, and add that to the PATH.

 Besides, I think gerbera should be using the .pc file rather than the
taglib-config script.

 Moreover, I think taglib should install a TaglibConfig.cmake file so CMake
packages like gerbera can pick that up directly rather than having to write a
FindTaglib file.

 Regards,
 Arnout

> ++            endif(CMAKE_CROSSCOMPILING)
> +             find_program(TAGLIBCONFIG_EXECUTABLE NAMES taglib-config PATHS
> + 		   ${BIN_INSTALL_DIR}
> +             )
> +-- 
> +2.22.0
> +
> 

[Buildroot] [PATCH 2/2] package/gerbera: add upstream patch to fix find_program with taglib-config

[Thomas Petazzoni]

On Tue, 11 Jun 2019 00:33:31 +0200
Arnout Vandecappelle <arnout@mind.be> wrote:

> > ++            if (CMAKE_CROSSCOMPILING)
> > ++              set(CMAKE_FIND_ROOT_PATH_MODE_PROGRAM ONLY)  
> 
>  Again, this is wrong IMO. The executables in the sysroot are not supposed to be
> executed on the host.
> 
>  What we really should do is to copy those config scripts to some other
> location, and add that to the PATH.

Not so simple: what do you do to differentiate between foo-config
describing foo for the host and foo-config describing foo for the
target ?

Yes, you could install the foo-config in a different directory than
$(HOST_DIR)/bin, but what if some package needs both foo-config for the
host and foo-config for the target, when building its code for the
target ?

Thomas
-- 
Thomas Petazzoni, CTO, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

[Buildroot] [PATCH 1/2] package/taglib: add upstream patch to fix taglib-config

[Jörg Krause]

Hi Arnout,

thanks for your review!

On Tue, 2019-06-11 at 00:25 +0200, Arnout Vandecappelle wrote:
> 
> On 10/06/2019 23:20, J?rg Krause wrote:
> > +-prefix=${CMAKE_INSTALL_PREFIX}
> > +-exec_prefix=${CMAKE_INSTALL_PREFIX}
> > +-libdir=${LIB_INSTALL_DIR}
> > +-includedir=${INCLUDE_INSTALL_DIR}
> > ++prefix=@CMAKE_SYSROOT@@CMAKE_INSTALL_PREFIX@
> > ++exec_prefix=@CMAKE_SYSROOT@@CMAKE_INSTALL_PREFIX@
> 
>  I believe it is wrong to include CMAKE_SYSROOT here. In general, this is not
> necessarily the place where the package will be installed. We happen to do that
> now, but for example both OpenWrt and OpenEmbedded install with a different
> DESTDIR, and dependent packages see a different sysroot.

I see! The upstream patch was sent by me, so I will find a better
solution.

>  We have the TAGLIB_CONFIG_SCRIPT infra to handle this situation. It will add
> the appropriate sysroot to the _prefix variables.

I've checked and used TAGLIB_CONFIG_SCRIPTS instead of applying the
patch. The current sed commands to fix the package configuration file
does not work for taglib-config:

```
# ./output/host/arm-buildroot-linux-gnueabihf/sysroot/usr/bin/taglib-config --libs
-L/usr/lib -ltag
```

To get taglib-config work with our CONFIG_SCRIPTS, it needs to be fixed
the way the patch does, just without using CMAKE_SYSROOT.

Best regards
J?rg Krause

[Buildroot] [PATCH 1/2] package/taglib: add upstream patch to fix taglib-config

[Arnout Vandecappelle]

On 16/06/2019 15:24, J?rg Krause wrote:
> Hi Arnout,
> 
> thanks for your review!
> 
> On Tue, 2019-06-11 at 00:25 +0200, Arnout Vandecappelle wrote:
>>
>> On 10/06/2019 23:20, J?rg Krause wrote:
>>> +-prefix=${CMAKE_INSTALL_PREFIX}
>>> +-exec_prefix=${CMAKE_INSTALL_PREFIX}
>>> +-libdir=${LIB_INSTALL_DIR}
>>> +-includedir=${INCLUDE_INSTALL_DIR}
>>> ++prefix=@CMAKE_SYSROOT@@CMAKE_INSTALL_PREFIX@
>>> ++exec_prefix=@CMAKE_SYSROOT@@CMAKE_INSTALL_PREFIX@
>>
>>  I believe it is wrong to include CMAKE_SYSROOT here. In general, this is not
>> necessarily the place where the package will be installed. We happen to do that
>> now, but for example both OpenWrt and OpenEmbedded install with a different
>> DESTDIR, and dependent packages see a different sysroot.
> 
> I see! The upstream patch was sent by me, so I will find a better
> solution.
> 
>>  We have the TAGLIB_CONFIG_SCRIPT infra to handle this situation. It will add
>> the appropriate sysroot to the _prefix variables.
> 
> I've checked and used TAGLIB_CONFIG_SCRIPTS instead of applying the
> patch. The current sed commands to fix the package configuration file
> does not work for taglib-config:
> 
> ```
> # ./output/host/arm-buildroot-linux-gnueabihf/sysroot/usr/bin/taglib-config --libs
> -L/usr/lib -ltag
> ```
> 
> To get taglib-config work with our CONFIG_SCRIPTS, it needs to be fixed
> the way the patch does, just without using CMAKE_SYSROOT.

 Yes, that was actually what I intended to say. Sorry for not being clearer.

 Regards,
 Arnout

[Buildroot] [PATCH 2/2] package/gerbera: add upstream patch to fix find_program with taglib-config

[Peter Korsgaard]

>>>>> "J?rg" == J?rg Krause <joerg.krause@embedded.rocks> writes:

 > Add an from from upstream to fix linking Gerbera with taglib. The patch
 > fixes the following issue:

 > When cross-compiling CMakes `find_program()` will search for binaries on the
 > host. This is typically correct, e.g. when search for compilers or shells.

 > When cross-compiling, searching for `taglib-config` using `find_program` should
 > not find the binary on the host, instead it should find the binary in the sysroot
 > directory, as the host `taglib-config` will output the wrong host paths
 > and libs, whereas the sysroot `taglib-config` will output the correct sysroot
 > paths and libs.

 > Therefore, use the `CMAKE_FIND_ROOT_PATH_MODE_PROGRAM` variable when
 > cross-compiling. This variable controls whether the `CMAKE_FIND_ROOT_PATH`
 > and `CMAKE_SYSROOT` are used by find_program().

 > Signed-off-by: J?rg Krause <joerg.krause@embedded.rocks>

Committed to 2019.02.x and 2019.05.x, thanks.

-- 
Bye, Peter Korsgaard

[Buildroot] [PATCH 1/2] package/taglib: add upstream patch to fix taglib-config

[Peter Korsgaard]

>>>>> "J?rg" == J?rg Krause <joerg.krause@embedded.rocks> writes:

 > The current taglib-config program does not work when cross-compiling as it only
 > returns paths to the host, which breaks building programs which link against
 > taglib.

 > For example gerbera uses `taglib-config` and it fails with:

 > ```
 > [..]
 > -- Found TagLib: -L/usr/lib -ltag
 > [..]
 > arm-linux-gnueabihf-g++: ERROR: unsafe header/library path used in cross-compilation: '-L/usr/lib'
 > ```

 > Before the patch the output of `taglib-config` is:
 > ```
 > $ ./output/host/arm-buildroot-linux-gnueabihf/sysroot/usr/bin/taglib-config --libs
 > -L/usr/lib -ltag
 > ```

 > Add a patch from upstream which fixes taglib-config.

 > After applying the fix, the pkg-config file is correct:
 > ```
 > $ ./output/host/arm-buildroot-linux-gnueabihf/sysroot/usr/bin/taglib-config --libs
 > -L/home/joerg/Development/git/buildroot/output/host/arm-buildroot-linux-gnueabihf/sysroot/usr/lib -ltag
 > ```

 > Signed-off-by: J?rg Krause <joerg.krause@embedded.rocks>

Committed to 2019.02.x and 2019.05.x, thanks.

-- 
Bye, Peter Korsgaard