[Buildroot] [PATCH] bctoolbox: fix typos and logic error

[Buildroot] [PATCH] bctoolbox: fix typos and logic error

[Waldemar Brodkorb]

To get the directory path from the library name you need to
use a single filename. Fix typo in variable name.

This fixes the ortp autobuild errors which uses the broken pkgconfig file
from bctoolbox.

Fixes:
  http://autobuild.buildroot.net/results/37d5625df4be11ccdc063871e9f6e13d5f59fb52
  http://autobuild.buildroot.net/results/1999c841fae41f860f00747a362327cb2857e687

Signed-off-by: Waldemar Brodkorb <wbx@openadk.org>
---
 package/bctoolbox/0001-fix-typo.patch | 18 ++++++++++++++++++
 1 file changed, 18 insertions(+)
 create mode 100644 package/bctoolbox/0001-fix-typo.patch

diff --git a/package/bctoolbox/0001-fix-typo.patch b/package/bctoolbox/0001-fix-typo.patch
new file mode 100644
index 0000000..b94daee
--- /dev/null
+++ b/package/bctoolbox/0001-fix-typo.patch
@@ -0,0 +1,18 @@
+Fix a typo in the variable name. Only check path for a single library name.
+
+Signed-off-by: Waldemar Brodkorb <wbx@openadk.org> 
+
+diff -Nur bctoolbox-0.4.0.orig/CMakeLists.txt bctoolbox-0.4.0/CMakeLists.txt
+--- bctoolbox-0.4.0.orig/CMakeLists.txt	2016-10-06 17:30:41.000000000 +0200
++++ bctoolbox-0.4.0/CMakeLists.txt	2017-02-13 23:30:38.641288032 +0100
+@@ -103,8 +103,8 @@
+ endif()
+ 
+ if(MBEDTLS_FOUND)
+-	get_filename_component(mbedtls_library_path "${MBEDTLS_LIBRARIES}" PATH)
+-	set(LIBS_PRIVATE "${LIBS_PRIVATE} -L${mbedlts_library_path}")
++	get_filename_component(mbedtls_library_path "${MBEDTLS_LIBRARY}" PATH)
++	set(LIBS_PRIVATE "${LIBS_PRIVATE} -L${mbedtls_library_path}")
+ endif()
+ if(POLARSSL_FOUND)
+ 	get_filename_component(polarssl_library_path "${POLARSSL_LIBRARIES}" PATH)
-- 
2.1.4

[Buildroot] [PATCH] bctoolbox: fix typos and logic error

[Peter Korsgaard]

>>>>> "Waldemar" == Waldemar Brodkorb <wbx@openadk.org> writes:

 > To get the directory path from the library name you need to
 > use a single filename. Fix typo in variable name.

 > This fixes the ortp autobuild errors which uses the broken pkgconfig file
 > from bctoolbox.

 > Fixes:
 >   http://autobuild.buildroot.net/results/37d5625df4be11ccdc063871e9f6e13d5f59fb52
 >   http://autobuild.buildroot.net/results/1999c841fae41f860f00747a362327cb2857e687

 > Signed-off-by: Waldemar Brodkorb <wbx@openadk.org>

Committed, thanks. Don't forget to send the patch upstream.

-- 
Bye, Peter Korsgaard

[Buildroot] [PATCH] bctoolbox: fix typos and logic error

[Thomas Petazzoni]

Hello,

Thanks a lot for looking into this build issue and fixing it. Just one
comment below.

On Tue, 14 Feb 2017 10:01:32 +0100, Waldemar Brodkorb wrote:

> diff --git a/package/bctoolbox/0001-fix-typo.patch b/package/bctoolbox/0001-fix-typo.patch
> new file mode 100644
> index 0000000..b94daee
> --- /dev/null
> +++ b/package/bctoolbox/0001-fix-typo.patch
> @@ -0,0 +1,18 @@
> +Fix a typo in the variable name. Only check path for a single library name.
> +
> +Signed-off-by: Waldemar Brodkorb <wbx@openadk.org> 

When the upstream project uses Git, I really, really prefer when the
patches are Git formatted. This way, they can easily be git am'ed by
other people who need to create/rework patches for the same package.

Thanks!

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

[Buildroot] [PATCH] bctoolbox: fix typos and logic error

[Jörg Krause]

Hi Waldemar,

On Tue, 2017-02-14 at 10:01 +0100, Waldemar Brodkorb wrote:
> To get the directory path from the library name you need to
> use a single filename. Fix typo in variable name.
> 
> This fixes the ortp autobuild errors which uses the broken pkgconfig
> file
> from bctoolbox.
> 
> Fixes:
> ? http://autobuild.buildroot.net/results/37d5625df4be11ccdc063871e9f6
> e13d5f59fb52
> ? http://autobuild.buildroot.net/results/1999c841fae41f860f00747a3623
> 27cb2857e687
> 
> Signed-off-by: Waldemar Brodkorb <wbx@openadk.org>
> ---
> ?package/bctoolbox/0001-fix-typo.patch | 18 ++++++++++++++++++
> ?1 file changed, 18 insertions(+)
> ?create mode 100644 package/bctoolbox/0001-fix-typo.patch
> 
> diff --git a/package/bctoolbox/0001-fix-typo.patch
> b/package/bctoolbox/0001-fix-typo.patch
> new file mode 100644
> index 0000000..b94daee
> --- /dev/null
> +++ b/package/bctoolbox/0001-fix-typo.patch
> @@ -0,0 +1,18 @@
> +Fix a typo in the variable name. Only check path for a single
> library name.
> +
> +Signed-off-by: Waldemar Brodkorb <wbx@openadk.org>?
> +
> +diff -Nur bctoolbox-0.4.0.orig/CMakeLists.txt bctoolbox-
> 0.4.0/CMakeLists.txt
> +--- bctoolbox-0.4.0.orig/CMakeLists.txt	2016-10-06
> 17:30:41.000000000 +0200
> ++++ bctoolbox-0.4.0/CMakeLists.txt	2017-02-13
> 23:30:38.641288032 +0100
> +@@ -103,8 +103,8 @@
> + endif()
> +?
> + if(MBEDTLS_FOUND)
> +-	get_filename_component(mbedtls_library_path
> "${MBEDTLS_LIBRARIES}" PATH)
> +-	set(LIBS_PRIVATE "${LIBS_PRIVATE}
> -L${mbedlts_library_path}")
> ++	get_filename_component(mbedtls_library_path
> "${MBEDTLS_LIBRARY}" PATH)
> ++	set(LIBS_PRIVATE "${LIBS_PRIVATE}
> -L${mbedtls_library_path}")
> + endif()
> + if(POLARSSL_FOUND)
> +?	get_filename_component(polarssl_library_path
> "${POLARSSL_LIBRARIES}" PATH)

With this fix applied the bctoolbox pkg-config 'Libs.private:' value is
set to:

"""
Libs.private:??-L/mnt/data/git/buildroot/output/host/usr/arm-buildroot-
linux-uclibcgnueabi/sysroot/usr/lib32
"""

No mbedtls libraries are included here.

And the flag 'BCTOOLBOX_LIBS' in ortp is set to:

"""
-L/mnt/data/git/buildroot/output/host/usr/arm-buildroot-linux-
uclibcgnueabi/sysroot/usr/lib -lbctoolbox
-L/mnt/data/git/buildroot/output/host/usr/ar"\
"m-buildroot-linux-uclibcgnueabi/sysroot/usr/lib32
"""

From my understanding, all the three mbedtls libraries libmbedcrypto,
libmbedx509, and libmbedtls should be added to the linker flags and
therefore to 'Libs.private' in the pkg-config file, right?

J?rg

[Buildroot] [PATCH] bctoolbox: fix typos and logic error

[Waldemar Brodkorb]

Hi,
J?rg Krause wrote,

> Hi Waldemar,
> 
> On Tue, 2017-02-14 at 10:01 +0100, Waldemar Brodkorb wrote:
> > To get the directory path from the library name you need to
> > use a single filename. Fix typo in variable name.
> > 
> > This fixes the ortp autobuild errors which uses the broken pkgconfig
> > file
> > from bctoolbox.
> > 
> > Fixes:
> > ? http://autobuild.buildroot.net/results/37d5625df4be11ccdc063871e9f6
> > e13d5f59fb52
> > ? http://autobuild.buildroot.net/results/1999c841fae41f860f00747a3623
> > 27cb2857e687
> > 
> > Signed-off-by: Waldemar Brodkorb <wbx@openadk.org>
> > ---
> > ?package/bctoolbox/0001-fix-typo.patch | 18 ++++++++++++++++++
> > ?1 file changed, 18 insertions(+)
> > ?create mode 100644 package/bctoolbox/0001-fix-typo.patch
> > 
> > diff --git a/package/bctoolbox/0001-fix-typo.patch
> > b/package/bctoolbox/0001-fix-typo.patch
> > new file mode 100644
> > index 0000000..b94daee
> > --- /dev/null
> > +++ b/package/bctoolbox/0001-fix-typo.patch
> > @@ -0,0 +1,18 @@
> > +Fix a typo in the variable name. Only check path for a single
> > library name.
> > +
> > +Signed-off-by: Waldemar Brodkorb <wbx@openadk.org>?
> > +
> > +diff -Nur bctoolbox-0.4.0.orig/CMakeLists.txt bctoolbox-
> > 0.4.0/CMakeLists.txt
> > +--- bctoolbox-0.4.0.orig/CMakeLists.txt	2016-10-06
> > 17:30:41.000000000 +0200
> > ++++ bctoolbox-0.4.0/CMakeLists.txt	2017-02-13
> > 23:30:38.641288032 +0100
> > +@@ -103,8 +103,8 @@
> > + endif()
> > +?
> > + if(MBEDTLS_FOUND)
> > +-	get_filename_component(mbedtls_library_path
> > "${MBEDTLS_LIBRARIES}" PATH)
> > +-	set(LIBS_PRIVATE "${LIBS_PRIVATE}
> > -L${mbedlts_library_path}")
> > ++	get_filename_component(mbedtls_library_path
> > "${MBEDTLS_LIBRARY}" PATH)
> > ++	set(LIBS_PRIVATE "${LIBS_PRIVATE}
> > -L${mbedtls_library_path}")
> > + endif()
> > + if(POLARSSL_FOUND)
> > +?	get_filename_component(polarssl_library_path
> > "${POLARSSL_LIBRARIES}" PATH)
> 
> With this fix applied the bctoolbox pkg-config 'Libs.private:' value is
> set to:
> 
> """
> Libs.private:??-L/mnt/data/git/buildroot/output/host/usr/arm-buildroot-
> linux-uclibcgnueabi/sysroot/usr/lib32
> """
> 
> No mbedtls libraries are included here.
> 
> And the flag 'BCTOOLBOX_LIBS' in ortp is set to:
> 
> """
> -L/mnt/data/git/buildroot/output/host/usr/arm-buildroot-linux-
> uclibcgnueabi/sysroot/usr/lib -lbctoolbox
> -L/mnt/data/git/buildroot/output/host/usr/ar"\
> "m-buildroot-linux-uclibcgnueabi/sysroot/usr/lib32
> """
> 
> From my understanding, all the three mbedtls libraries libmbedcrypto,
> libmbedx509, and libmbedtls should be added to the linker flags and
> therefore to 'Libs.private' in the pkg-config file, right?

I was only looking into the ortp build errors with complete empty -L . But you are right
actually we might need to add -lmbedx509 -lmbedtls -lmbedcrypto
to fix any static linking issues.

I think upstream does add some -l for the polarssl case.
I am not really using this packages, just want to minimize my
architecture related build errors :)

Do you have a chance to suggest a better fix and test with static
linking?

best regards
 Waldemar

[Buildroot] [PATCH] bctoolbox: fix typos and logic error

[Jörg Krause]

On Tue, 2017-02-14 at 16:39 +0100, Waldemar Brodkorb wrote:
> Hi,
> J?rg Krause wrote,
> 
> > Hi Waldemar,
> > 
> > On Tue, 2017-02-14 at 10:01 +0100, Waldemar Brodkorb wrote:
> > > To get the directory path from the library name you need to
> > > use a single filename. Fix typo in variable name.
> > > 
> > > This fixes the ortp autobuild errors which uses the broken
> > > pkgconfig
> > > file
> > > from bctoolbox.
> > > 
> > > Fixes:
> > > ? http://autobuild.buildroot.net/results/37d5625df4be11ccdc063871
> > > e9f6
> > > e13d5f59fb52
> > > ? http://autobuild.buildroot.net/results/1999c841fae41f860f00747a
> > > 3623
> > > 27cb2857e687
> > > 
> > > Signed-off-by: Waldemar Brodkorb <wbx@openadk.org>
> > > ---
> > > ?package/bctoolbox/0001-fix-typo.patch | 18 ++++++++++++++++++
> > > ?1 file changed, 18 insertions(+)
> > > ?create mode 100644 package/bctoolbox/0001-fix-typo.patch
> > > 
> > > diff --git a/package/bctoolbox/0001-fix-typo.patch
> > > b/package/bctoolbox/0001-fix-typo.patch
> > > new file mode 100644
> > > index 0000000..b94daee
> > > --- /dev/null
> > > +++ b/package/bctoolbox/0001-fix-typo.patch
> > > @@ -0,0 +1,18 @@
> > > +Fix a typo in the variable name. Only check path for a single
> > > library name.
> > > +
> > > +Signed-off-by: Waldemar Brodkorb <wbx@openadk.org>?
> > > +
> > > +diff -Nur bctoolbox-0.4.0.orig/CMakeLists.txt bctoolbox-
> > > 0.4.0/CMakeLists.txt
> > > +--- bctoolbox-0.4.0.orig/CMakeLists.txt	2016-10-06
> > > 17:30:41.000000000 +0200
> > > ++++ bctoolbox-0.4.0/CMakeLists.txt	2017-02-13
> > > 23:30:38.641288032 +0100
> > > +@@ -103,8 +103,8 @@
> > > + endif()
> > > +?
> > > + if(MBEDTLS_FOUND)
> > > +-	get_filename_component(mbedtls_library_path
> > > "${MBEDTLS_LIBRARIES}" PATH)
> > > +-	set(LIBS_PRIVATE "${LIBS_PRIVATE}
> > > -L${mbedlts_library_path}")
> > > ++	get_filename_component(mbedtls_library_path
> > > "${MBEDTLS_LIBRARY}" PATH)
> > > ++	set(LIBS_PRIVATE "${LIBS_PRIVATE}
> > > -L${mbedtls_library_path}")
> > > + endif()
> > > + if(POLARSSL_FOUND)
> > > +?	get_filename_component(polarssl_library_path
> > > "${POLARSSL_LIBRARIES}" PATH)
> > 
> > With this fix applied the bctoolbox pkg-config 'Libs.private:'
> > value is
> > set to:
> > 
> > """
> > Libs.private:??-L/mnt/data/git/buildroot/output/host/usr/arm-
> > buildroot-
> > linux-uclibcgnueabi/sysroot/usr/lib32
> > """
> > 
> > No mbedtls libraries are included here.
> > 
> > And the flag 'BCTOOLBOX_LIBS' in ortp is set to:
> > 
> > """
> > -L/mnt/data/git/buildroot/output/host/usr/arm-buildroot-linux-
> > uclibcgnueabi/sysroot/usr/lib -lbctoolbox
> > -L/mnt/data/git/buildroot/output/host/usr/ar"\
> > "m-buildroot-linux-uclibcgnueabi/sysroot/usr/lib32
> > """
> > 
> > From my understanding, all the three mbedtls libraries
> > libmbedcrypto,
> > libmbedx509, and libmbedtls should be added to the linker flags and
> > therefore to 'Libs.private' in the pkg-config file, right?
> 
> I was only looking into the ortp build errors with complete empty -L
> . But you are right
> actually we might need to add -lmbedx509 -lmbedtls -lmbedcrypto
> to fix any static linking issues.
> 
> I think upstream does add some -l for the polarssl case.
> I am not really using this packages, just want to minimize my
> architecture related build errors :)
> 
> Do you have a chance to suggest a better fix and test with static
> linking?

From my understanding of pkg-config, 'Libs.private' should be:

"""
Libs.private:??-lmbedtls -lmbedcrypto -lmbedx509
"""

And how to set it in CMakeLists.txt:

"""
if(MBEDTLS_FOUND)
	set(LIBS_PRIVATE "${LIBS_PRIVATE} -lmbedtls -lmbedcrypto
-lmbedx509")
endif()
if(POLARSSL_FOUND)
	set(LIBS_PRIVATE "${LIBS_PRIVATE} -lpolarssl")
endif()
"""

This way?pkg-config (used in ortp) sets 'BCTOOLBOX_LIBS' to:

"""
-L/mnt/data/git/buildroot/output/host/usr/arm-buildroot-linux-
uclibcgnueabi/sysroot/usr/lib -lbctoolbox -lmbedtls -lmbedcrypto
-lmbedx509
"""

.. which should be correct.

J?rg

[Buildroot] [PATCH] bctoolbox: fix typos and logic error

[Peter Korsgaard]

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

Hi,

 >> Do you have a chance to suggest a better fix and test with static
 >> linking?

 > From my understanding of pkg-config, 'Libs.private' should be:

 > """
 > Libs.private:??-lmbedtls -lmbedcrypto -lmbedx509

And possibly a -L to where these libraries are located (which is the
original issue).

 > """
 > -L/mnt/data/git/buildroot/output/host/usr/arm-buildroot-linux-
 > uclibcgnueabi/sysroot/usr/lib -lbctoolbox -lmbedtls -lmbedcrypto
 > -lmbedx509
 > """

 > .. which should be correct.

WIll you send a patch to do this?

-- 
Bye, Peter Korsgaard

[Buildroot] [PATCH] bctoolbox: fix typos and logic error

[Jörg Krause]

On Tue, 2017-02-14 at 20:25 +0100, Peter Korsgaard wrote:
> > > > > > "J?rg" == J?rg Krause <joerg.krause@embedded.rocks> writes:
> 
> Hi,
> 
> ?>> Do you have a chance to suggest a better fix and test with static
> ?>> linking?
> 
> ?> From my understanding of pkg-config, 'Libs.private' should be:
> 
> ?> """
> ?> Libs.private:??-lmbedtls -lmbedcrypto -lmbedx509
> 
> And possibly a -L to where these libraries are located (which is the
> original issue).

Is this necassary? I had a look at some pkg-config files and none of
them used '-L' in 'Libs.private', only in 'Libs:'

> ?> """
> ?> -L/mnt/data/git/buildroot/output/host/usr/arm-buildroot-linux-
> ?> uclibcgnueabi/sysroot/usr/lib -lbctoolbox -lmbedtls -lmbedcrypto
> ?> -lmbedx509
> ?> """
> 
> ?> .. which should be correct.
> 
> WIll you send a patch to do this?

Sure.

J?rg