diff mbox series

[1/1] package/libwebsockets: fix static build with openssl

Message ID 20191016204128.27360-1-fontaine.fabrice@gmail.com
State Accepted
Headers show
Series [1/1] package/libwebsockets: fix static build with openssl | expand

Commit Message

Fabrice Fontaine Oct. 16, 2019, 8:41 p.m. UTC 
Fixes:
 - http://autobuild.buildroot.org/results/65d0528b208c0a470264f7e2433be89425971dd7

Signed-off-by: Fabrice Fontaine <fontaine.fabrice@gmail.com>
---
 ...ts.txt-fix-static-build-with-openssl.patch | 36 +++++++++++++++++++
 1 file changed, 36 insertions(+)
 create mode 100644 package/libwebsockets/0002-CMakeLists.txt-fix-static-build-with-openssl.patch

Comments

Thomas Petazzoni Oct. 16, 2019, 8:48 p.m. UTC | #1 
On Wed, 16 Oct 2019 22:41:28 +0200
Fabrice Fontaine <fontaine.fabrice@gmail.com> wrote:

> + 		if (NOT LWS_WITH_ESP32)
> ++			find_package(PkgConfig QUIET)
> ++			pkg_check_modules(PC_OPENSSL openssl QUIET)
> + 			find_package(OpenSSL REQUIRED)

Isn't the find_package() then redundant with pkg_check_modules() ?

Thomas
Fabrice Fontaine Oct. 16, 2019, 9:08 p.m. UTC | #2 
Le mer. 16 oct. 2019 à 22:48, Thomas Petazzoni
<thomas.petazzoni@bootlin.com> a écrit :
>
> On Wed, 16 Oct 2019 22:41:28 +0200
> Fabrice Fontaine <fontaine.fabrice@gmail.com> wrote:
>
> > +             if (NOT LWS_WITH_ESP32)
> > ++                    find_package(PkgConfig QUIET)
> > ++                    pkg_check_modules(PC_OPENSSL openssl QUIET)
> > +                     find_package(OpenSSL REQUIRED)
>
> Isn't the find_package() then redundant with pkg_check_modules() ?
I don't think upstream will accept to add a mandatory dependency on
pkg-config so I'll have to keep find_package as fallback.
Moreover, CMakeLists.txt sets OPENSSL_INCLUDE_DIRS to
OPENSSL_INCLUDE_DIR so I'll also have to update this line as
OPENSSL_INCLUDE_DIR won't be set by pkg_check_modules.
I think we should wait for the upstream's review.
>
> Thomas
> --
> Thomas Petazzoni, CTO, Bootlin
> Embedded Linux and Kernel engineering
> https://bootlin.com
Best Regards,

Fabrice
Thomas Petazzoni Oct. 16, 2019, 9:10 p.m. UTC | #3 
On Wed, 16 Oct 2019 23:08:34 +0200
Fabrice Fontaine <fontaine.fabrice@gmail.com> wrote:

> > > +             if (NOT LWS_WITH_ESP32)
> > > ++                    find_package(PkgConfig QUIET)
> > > ++                    pkg_check_modules(PC_OPENSSL openssl QUIET)
> > > +                     find_package(OpenSSL REQUIRED)  
> >
> > Isn't the find_package() then redundant with pkg_check_modules() ?  
> I don't think upstream will accept to add a mandatory dependency on
> pkg-config so I'll have to keep find_package as fallback.
> Moreover, CMakeLists.txt sets OPENSSL_INCLUDE_DIRS to
> OPENSSL_INCLUDE_DIR so I'll also have to update this line as
> OPENSSL_INCLUDE_DIR won't be set by pkg_check_modules.
> I think we should wait for the upstream's review.

Then:

	find_package(PkgConfig QUIET)
	if(whatever is the right way to check if pkg-config was found)
          pkg_check_modules(PC_OPENSSL openssl QUIET)
          ... adjust LIBS/CFLAGS ...
        else()
          find_package(OpenSSL REQUIRED)
        endif()

Best regards,

Thomas
Fabrice Fontaine Oct. 17, 2019, 4:57 p.m. UTC | #4 
Le mer. 16 oct. 2019 à 23:10, Thomas Petazzoni
<thomas.petazzoni@bootlin.com> a écrit :
>
> On Wed, 16 Oct 2019 23:08:34 +0200
> Fabrice Fontaine <fontaine.fabrice@gmail.com> wrote:
>
> > > > +             if (NOT LWS_WITH_ESP32)
> > > > ++                    find_package(PkgConfig QUIET)
> > > > ++                    pkg_check_modules(PC_OPENSSL openssl QUIET)
> > > > +                     find_package(OpenSSL REQUIRED)
> > >
> > > Isn't the find_package() then redundant with pkg_check_modules() ?
> > I don't think upstream will accept to add a mandatory dependency on
> > pkg-config so I'll have to keep find_package as fallback.
> > Moreover, CMakeLists.txt sets OPENSSL_INCLUDE_DIRS to
> > OPENSSL_INCLUDE_DIR so I'll also have to update this line as
> > OPENSSL_INCLUDE_DIR won't be set by pkg_check_modules.
> > I think we should wait for the upstream's review.
>
> Then:
>
>         find_package(PkgConfig QUIET)
>         if(whatever is the right way to check if pkg-config was found)
>           pkg_check_modules(PC_OPENSSL openssl QUIET)
>           ... adjust LIBS/CFLAGS ...
>         else()
>           find_package(OpenSSL REQUIRED)
>         endif()
Upstream merged the patch as is, should I sent them a new PR with your updates?
>
> Best regards,
>
> Thomas
> --
> Thomas Petazzoni, CTO, Bootlin
> Embedded Linux and Kernel engineering
> https://bootlin.com
Best Regards,

Fabrice
Thomas Petazzoni Oct. 18, 2019, 6:54 a.m. UTC | #5 
On Thu, 17 Oct 2019 18:57:43 +0200
Fabrice Fontaine <fontaine.fabrice@gmail.com> wrote:

> > Then:
> >
> >         find_package(PkgConfig QUIET)
> >         if(whatever is the right way to check if pkg-config was found)
> >           pkg_check_modules(PC_OPENSSL openssl QUIET)
> >           ... adjust LIBS/CFLAGS ...
> >         else()
> >           find_package(OpenSSL REQUIRED)
> >         endif()  
> Upstream merged the patch as is, should I sent them a new PR with your updates?

If upstream was happy with your patch as-is, I'm fine with it :-)

Thanks!

Thomas
Thomas Petazzoni Oct. 21, 2019, 8:01 p.m. UTC | #6 
On Wed, 16 Oct 2019 22:41:28 +0200
Fabrice Fontaine <fontaine.fabrice@gmail.com> wrote:

> Fixes:
>  - http://autobuild.buildroot.org/results/65d0528b208c0a470264f7e2433be89425971dd7
> 
> Signed-off-by: Fabrice Fontaine <fontaine.fabrice@gmail.com>
> ---
>  ...ts.txt-fix-static-build-with-openssl.patch | 36 +++++++++++++++++++
>  1 file changed, 36 insertions(+)
>  create mode 100644 package/libwebsockets/0002-CMakeLists.txt-fix-static-build-with-openssl.patch

Applied to master, thanks.

Thomas
diff mbox series

Patch

diff --git a/package/libwebsockets/0002-CMakeLists.txt-fix-static-build-with-openssl.patch b/package/libwebsockets/0002-CMakeLists.txt-fix-static-build-with-openssl.patch
new file mode 100644
index 0000000000..c5aaafab85
--- /dev/null
+++ b/package/libwebsockets/0002-CMakeLists.txt-fix-static-build-with-openssl.patch
@@ -0,0 +1,36 @@ 
+From faf26eee75f5d58ff499cdc40ad1af58a33fb83f Mon Sep 17 00:00:00 2001
+From: Fabrice Fontaine <fontaine.fabrice@gmail.com>
+Date: Wed, 16 Oct 2019 20:09:13 +0200
+Subject: [PATCH] CMakeLists.txt: fix static build with openssl
+
+openssl can depends on -latomic so use pkg-config (if available) to
+retrieve these static dependencies otherwise build will fail because
+HMAC_CTX_new test will return a wrong result
+
+Fixes:
+ - http://autobuild.buildroot.org/results/65d0528b208c0a470264f7e2433be89425971dd7
+
+Signed-off-by: Fabrice Fontaine <fontaine.fabrice@gmail.com>
+[Upstream status: https://github.com/warmcat/libwebsockets/pull/1717]
+---
+ CMakeLists.txt | 3 +++
+ 1 file changed, 3 insertions(+)
+
+diff --git a/CMakeLists.txt b/CMakeLists.txt
+index 2693ac56..888f65e8 100644
+--- a/CMakeLists.txt
++++ b/CMakeLists.txt
+@@ -1803,7 +1803,10 @@ if (LWS_WITH_SSL)
+ 		if (NOT OPENSSL_FOUND AND NOT LWS_WITH_BORINGSSL)
+ 			# TODO: Add support for STATIC also.
+ 		if (NOT LWS_WITH_ESP32)
++			find_package(PkgConfig QUIET)
++			pkg_check_modules(PC_OPENSSL openssl QUIET)
+ 			find_package(OpenSSL REQUIRED)
++			list(APPEND OPENSSL_LIBRARIES ${PC_OPENSSL_LIBRARIES})
+ 		endif()
+ 			set(OPENSSL_INCLUDE_DIRS "${OPENSSL_INCLUDE_DIR}")
+ 		endif()
+-- 
+2.23.0
+