libcurl: fix build with ssh2 and static mbedtls

PATCH New Fabrice Fontaine 3 participants 6 messages May 21, 2018 – Jun 17, 2018

Share & tools

Fabrice Fontaine

author

2018-05-21 11:41 UTC #0 lore raw

PATCH 1/1 libcurl: fix build with ssh2 and static mbedtls

The ssh2 pkg-config file could contain the following lines when build with a static version of mbedtls:
Libs: -L${libdir} -lssh2 /xxx/libmbedcrypto.a Libs.private: /xxx/libmbedcrypto.a

This static mbedtls library must be used to correctly detect ssh2 support and this library must be copied in libcurl.pc otherwise compilation of any application (such as upmpdcli) with libcurl will fail when trying to found mbedtls functions included in libssh2. So, replace pkg-config --libs-only-l by pkg-config --libs.

Fixes:
- http://autobuild.buildroot.net/results/43e24b22a77f616d6198c10435dcc23cc3b9088a

Signed-off-by Fabrice Fontaine <[email protected]>

---
 ...ink-with-ssh2-built-with-a-static-mbedtls.patch | 40 ++++++++++++++++++++++
 package/libcurl/libcurl.mk                         |  2 ++
 2 files changed, 42 insertions(+)
 create mode 100644 package/libcurl/0001-Fix-link-with-ssh2-built-with-a-static-mbedtls.patch

package/libcurl/0001-Fix-link-with-ssh2-built-with-a-static-mbedtls.patch new file +40 -0

new file mode 100644
index 0000000000..9107fa7c8c
@@ -0,0 +1,40 @@
+From b5fbc486e805805efb8400373ccec2a3dee1c81b Mon Sep 17 00:00:00 2001
+From: Fabrice Fontaine <[email protected]>
+Date: Mon, 21 May 2018 12:07:00 +0200
+Subject: [PATCH 1/1] Fix link with ssh2 built with a static mbedtls
+
+The ssh2 pkg-config file could contain the following lines when build
+with a static version of mbedtls:
+   Libs: -L${libdir} -lssh2 /xxx/libmbedcrypto.a
+   Libs.private: /xxx/libmbedcrypto.a
+
+This static mbedtls library must be used to correctly detect ssh2
+support and this library must be copied in libcurl.pc otherwise
+compilation of any application (such as upmpdcli) with libcurl will fail
+when trying to found mbedtls functions included in libssh2.
+So, replace pkg-config --libs-only-l by pkg-config --libs.
+
+Fixes:
+ - http://autobuild.buildroot.net/results/43e24b22a77f616d6198c10435dcc23cc3b9088a
+
+Signed-off-by: Fabrice Fontaine <[email protected]>
+---
+ configure.ac | 2 +-
+ 1 file changed, 1 insertion(+), 1 deletion(-)
+
+diff --git a/configure.ac b/configure.ac
+index 5569a26b4..9e2606885 100755
+--- a/configure.ac
++++ b/configure.ac
+@@ -2766,7 +2766,7 @@ if test X"$OPT_LIBSSH2" != Xno; then
+     CURL_CHECK_PKGCONFIG(libssh2)
+ 
+     if test "$PKGCONFIG" != "no" ; then
+-      LIB_SSH2=`$PKGCONFIG --libs-only-l libssh2`
++      LIB_SSH2=`$PKGCONFIG --libs libssh2`
+       LD_SSH2=`$PKGCONFIG --libs-only-L libssh2`
+       CPP_SSH2=`$PKGCONFIG --cflags-only-I libssh2`
+       version=`$PKGCONFIG --modversion libssh2`
+-- 
+2.14.1
+

package/libcurl/libcurl.mk +2 -0

index fbaeaa8975..25d74394c7 100644
@@ -14,6 +14,8 @@ LIBCURL_DEPENDENCIES = host-pkgconf \
 LIBCURL_LICENSE = curl
 LIBCURL_LICENSE_FILES = COPYING
 LIBCURL_INSTALL_STAGING = YES
+# We're patching configure.ac
+LIBCURL_AUTORECONF = YES
 
 # We disable NTLM support because it uses fork(), which doesn't work
 # on non-MMU platforms. Moreover, this authentication method is

Signature · 1 line

2.14.1

Thomas Petazzoni

2018-05-22 21:06 UTC #1 lore raw

PATCH 1/1 libcurl: fix build with ssh2 and static mbedtls

Hello,

Quoted 4 lines · Fabrice Fontaine

The ssh2 pkg-config file could contain the following lines when build with a static version of mbedtls:
   Libs: -L${libdir} -lssh2 /xxx/libmbedcrypto.a
   Libs.private: /xxx/libmbedcrypto.a

I find it weird to have the same library in both Libs and Libs.private.

Either it's a direct dependency of ssh2 and it should be in Libs. Or it's an indirect dependency of ssh2 and should only be used when static linking, in which case it should be only in Libs.private.

Section thing that looks weird is: why a full path to libmbedcrypto.a and not a -l flag ?

Quoted 5 lines

This static mbedtls library must be used to correctly detect ssh2 support and this library must be copied in libcurl.pc otherwise compilation of any application (such as upmpdcli) with libcurl will fail when trying to found mbedtls functions included in libssh2. So, replace pkg-config --libs-only-l by pkg-config --libs.

That being said, the libcurl change looks good to me. Did you submit it upstream ?

Thanks,

Thomas

Signature · 3 lines

Thomas Petazzoni, CTO, Bootlin (formerly Free Electrons)
Embedded Linux and Kernel engineering
https://bootlin.com

Fabrice Fontaine

author

2018-05-22 21:43 UTC #2 lore raw

PATCH 1/1 libcurl: fix build with ssh2 and static mbedtls

Dear Thomas,

2018-05-22 23:06 GMT+02:00 Thomas Petazzoni <[email protected]>:

Quoted 8 lines

Hello,
Quoted 4 lines · Fabrice Fontaine
The ssh2 pkg-config file could contain the following lines when build with a static version of mbedtls:
   Libs: -L${libdir} -lssh2 /xxx/libmbedcrypto.a
   Libs.private: /xxx/libmbedcrypto.a

Quoted 9 lines

I find it weird to have the same library in both Libs and Libs.private.
Either it's a direct dependency of ssh2 and it should be in Libs. Or it's an indirect dependency of ssh2 and should only be used when static linking, in which case it should be only in Libs.private.
Section thing that looks weird is: why a full path to libmbedcrypto.a and not a -l flag ?

Indeed, this seems pretty weird to me too.
This patch to libssh2 was added recently to buildroot (see
http://patchwork.ozlabs.org/patch/888395) by Baruch (now in cc). It seems that this full path is returned by the AC_LIB_HAVE_LINKFLAGS function, see https://github.com/libssh2/libssh2/blob/master/acinclude.m4.
So, perhaps we could find a better patch for libssh2 instead of patching libcurl.

Quoted 9 lines

Quoted 5 lines
This static mbedtls library must be used to correctly detect ssh2 support and this library must be copied in libcurl.pc otherwise compilation of any application (such as upmpdcli) with libcurl will fail when trying to found mbedtls functions included in libssh2. So, replace pkg-config --libs-only-l by pkg-config --libs.
That being said, the libcurl change looks good to me. Did you submit it upstream ?

No, I didn't send the patch upstream yet. I wanted to get your feedback first.

Quoted 9 lines

Thanks,
Thomas
--
Thomas Petazzoni, CTO, Bootlin (formerly Free Electrons) Embedded Linux and Kernel engineering
https://bootlin.com

Best Regards,

Fabrice
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.busybox.net/pipermail/buildroot/attachments/20180522/62d3c9c1/attachment.html>

Thomas Petazzoni

2018-05-23 06:02 UTC #3 lore raw

PATCH 1/1 libcurl: fix build with ssh2 and static mbedtls

Hello,

Quoted 7 lines · Fabrice Fontaine

Quoted 7 lines
Either it's a direct dependency of ssh2 and it should be in Libs. Or it's an indirect dependency of ssh2 and should only be used when static linking, in which case it should be only in Libs.private.
Section thing that looks weird is: why a full path to libmbedcrypto.a and not a -l flag ?

Quoted 7 lines

Indeed, this seems pretty weird to me too.
This patch to libssh2 was added recently to buildroot (see
http://patchwork.ozlabs.org/patch/888395) by Baruch (now in cc). It seems that this full path is returned by the AC_LIB_HAVE_LINKFLAGS function, see https://github.com/libssh2/libssh2/blob/master/acinclude.m4.
So, perhaps we could find a better patch for libssh2 instead of patching libcurl.

Indeed the resulting .pc file from libssh2 looks strange. But however, I think your change to libcurl remains valid. 'pkg-config --libs' is what should be used to get the linker flags. I don't see why libcurl should filter the options and only keep the -l options. Other linker options may be important.

Best regards,

Thomas

Signature · 3 lines

Thomas Petazzoni, CTO, Bootlin (formerly Free Electrons)
Embedded Linux and Kernel engineering
https://bootlin.com

Peter Korsgaard

2018-05-27 21:42 UTC #4 lore raw

PATCH 1/1 libcurl: fix build with ssh2 and static mbedtls

In reply to Fabrice Fontaine

Quoted 13 lines

Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
"Fabrice" == Fabrice Fontaine <[email protected]> writes:
The ssh2 pkg-config file could contain the following lines when build with a static version of mbedtls:
   Libs: -L${libdir} -lssh2 /xxx/libmbedcrypto.a
   Libs.private: /xxx/libmbedcrypto.a
This static mbedtls library must be used to correctly detect ssh2 support and this library must be copied in libcurl.pc otherwise compilation of any application (such as upmpdcli) with libcurl will fail when trying to found mbedtls functions included in libssh2. So, replace pkg-config --libs-only-l by pkg-config --libs.

Quoted 4 lines

Fixes:
- http://autobuild.buildroot.net/results/43e24b22a77f616d6198c10435dcc23cc3b9088a
Signed-off-by Fabrice Fontaine <[email protected]>

Committed, thanks. Please submit the patch upstream.

Signature · 1 line

Bye, Peter Korsgaard

Peter Korsgaard

2018-06-17 14:57 UTC #5 lore raw

PATCH 1/1 libcurl: fix build with ssh2 and static mbedtls

In reply to Fabrice Fontaine

Quoted 13 lines

Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
"Fabrice" == Fabrice Fontaine <[email protected]> writes:
The ssh2 pkg-config file could contain the following lines when build with a static version of mbedtls:
   Libs: -L${libdir} -lssh2 /xxx/libmbedcrypto.a
   Libs.private: /xxx/libmbedcrypto.a
This static mbedtls library must be used to correctly detect ssh2 support and this library must be copied in libcurl.pc otherwise compilation of any application (such as upmpdcli) with libcurl will fail when trying to found mbedtls functions included in libssh2. So, replace pkg-config --libs-only-l by pkg-config --libs.

Quoted 4 lines

Fixes:
- http://autobuild.buildroot.net/results/43e24b22a77f616d6198c10435dcc23cc3b9088a
Signed-off-by Fabrice Fontaine <[email protected]>

Committed to 2018.02.x, thanks.

Signature · 1 line

Bye, Peter Korsgaard