[v3,1/1] libcpprestsdk: fix building as a static library

Message ID 20180212172350.5697-1-aduskett@gmail.com
State Accepted
Headers show
Series
  • [v3,1/1] libcpprestsdk: fix building as a static library
Related show

Commit Message

Adam Duskett Feb. 12, 2018, 5:23 p.m.
Use pkg-config to find OpenSSL. This will automatically find any
dependent libraries and put them in the correct order for linking.

Upstream status: submitted
https://github.com/Microsoft/cpprestsdk/pull/688

Signed-off-by: Adam Duskett <aduskett@gmail.com>
---
Changes v1 -> v2:
  - Now using pkg-config instead of manually moving library search order
    (Thomas)

Changes v2 -> v3:
  - Removed spurious and unrelated change in patch. (Thomas)

 ...prestsdk-fix-building-as-a-static-library.patch | 38 ++++++++++++++++++++++
 1 file changed, 38 insertions(+)
 create mode 100644 package/libcpprestsdk/0004-libcpprestsdk-fix-building-as-a-static-library.patch

Comments

Adrian Perez de Castro Feb. 12, 2018, 7:27 p.m. | #1
Hi,

Looks good overall, just a small nit...

On Mon, 12 Feb 2018 12:23:50 -0500, Adam Duskett <aduskett@gmail.com> wrote:
> Use pkg-config to find OpenSSL. This will automatically find any
> dependent libraries and put them in the correct order for linking.
> 
> Upstream status: submitted
> https://github.com/Microsoft/cpprestsdk/pull/688
> 
> Signed-off-by: Adam Duskett <aduskett@gmail.com>
> ---
> Changes v1 -> v2:
>   - Now using pkg-config instead of manually moving library search order
>     (Thomas)
> 
> Changes v2 -> v3:
>   - Removed spurious and unrelated change in patch. (Thomas)
> 
>  ...prestsdk-fix-building-as-a-static-library.patch | 38 ++++++++++++++++++++++
>  1 file changed, 38 insertions(+)
>  create mode 100644 package/libcpprestsdk/0004-libcpprestsdk-fix-building-as-a-static-library.patch
> 
> diff --git a/package/libcpprestsdk/0004-libcpprestsdk-fix-building-as-a-static-library.patch b/package/libcpprestsdk/0004-libcpprestsdk-fix-building-as-a-static-library.patch
> new file mode 100644
> index 0000000000..de900178d5
> --- /dev/null
> +++ b/package/libcpprestsdk/0004-libcpprestsdk-fix-building-as-a-static-library.patch
> @@ -0,0 +1,38 @@
> +From 8a9c3db14a390c0a8788405e52e9b8737a430191 Mon Sep 17 00:00:00 2001
> +From: Adam Duskett <aduskett@gmail.com>
> +Date: Mon, 12 Feb 2018 07:49:34 -0500
> +Subject: [PATCH] libcpprestsdk: fix building as a static library
> +
> +Use pkg-config to find OpenSSL. This will automatically find any
> +dependent libraries and put them in the correct order for linking.
> +
> +Upstream status: submitted
> +https://github.com/Microsoft/cpprestsdk/pull/688
> +
> +Signed-off-by: Adam Duskett <aduskett@gmail.com>
> +---
> + Release/cmake/cpprest_find_openssl.cmake | 10 +++++++---
> + 1 file changed, 7 insertions(+), 3 deletions(-)
> +
> +diff --git a/Release/cmake/cpprest_find_openssl.cmake b/Release/cmake/cpprest_find_openssl.cmake
> +index 0b49a7e..2be8afb 100644
> +--- a/Release/cmake/cpprest_find_openssl.cmake
> ++++ b/Release/cmake/cpprest_find_openssl.cmake
> +@@ -41,8 +41,12 @@ function(cpprest_find_openssl)
> +       # This should prevent linking against the system provided 0.9.8y
> +       set(_OPENSSL_VERSION "")
> +     endif()
> +-    find_package(OpenSSL 1.0.0 REQUIRED)
> +-
> ++    if(UNIX)
> ++		find_package(PkgConfig REQUIRED)
> ++		pkg_search_module(OPENSSL openssl REQUIRED)
> ++    else()
> ++    	find_package(OpenSSL 1.0.0 REQUIRED)
> ++	endif()

These added lines have a mix of spaces and tabs for indentation. It would be
nicer to use only tabs or only spaces, but I guess this is not an important
enough reason to avoid merging the patch :-)

> +     INCLUDE(CheckCXXSourceCompiles)
> +     set(CMAKE_REQUIRED_INCLUDES "${OPENSSL_INCLUDE_DIR}")
> +     set(CMAKE_REQUIRED_LIBRARIES "${OPENSSL_LIBRARIES}")
> +-- 
> +2.14.3
> +
> -- 
> 2.14.3

Reviewed-by: Adrian Perez de Castro <aperez@igalia.com>


--
 Adrián 🎩
Thomas Petazzoni Feb. 13, 2018, 10:14 p.m. | #2
Hello,

On Mon, 12 Feb 2018 12:23:50 -0500, Adam Duskett wrote:
> Use pkg-config to find OpenSSL. This will automatically find any
> dependent libraries and put them in the correct order for linking.
> 
> Upstream status: submitted
> https://github.com/Microsoft/cpprestsdk/pull/688
> 
> Signed-off-by: Adam Duskett <aduskett@gmail.com>
> ---
> Changes v1 -> v2:
>   - Now using pkg-config instead of manually moving library search order
>     (Thomas)

Your commit log was missing a reference to the autobuilder failure
being fixed, so I've added one and applied to master.

Thanks,

Thomas
Peter Korsgaard Feb. 15, 2018, 10:10 a.m. | #3
>>>>> "Thomas" == Thomas Petazzoni <thomas.petazzoni@bootlin.com> writes:

 > Hello,
 > On Mon, 12 Feb 2018 12:23:50 -0500, Adam Duskett wrote:
 >> Use pkg-config to find OpenSSL. This will automatically find any
 >> dependent libraries and put them in the correct order for linking.
 >> 
 >> Upstream status: submitted
 >> https://github.com/Microsoft/cpprestsdk/pull/688
 >> 
 >> Signed-off-by: Adam Duskett <aduskett@gmail.com>
 >> ---
 >> Changes v1 -> v2:
 >> - Now using pkg-config instead of manually moving library search order
 >> (Thomas)

 > Your commit log was missing a reference to the autobuilder failure
 > being fixed, so I've added one and applied to master.

As it now uses pkg-config, libcpprestsdk should add host-pkgconf to
_DEPENDENCIES.

Care to send a patch?
Peter Korsgaard Feb. 15, 2018, 9:38 p.m. | #4
>>>>> "Peter" == Peter Korsgaard <peter@korsgaard.com> writes:

Hi,

 >> Your commit log was missing a reference to the autobuilder failure
 >> being fixed, so I've added one and applied to master.

 > As it now uses pkg-config, libcpprestsdk should add host-pkgconf to
 > _DEPENDENCIES.

 > Care to send a patch?

I've now fixed it in git.

Patch

diff --git a/package/libcpprestsdk/0004-libcpprestsdk-fix-building-as-a-static-library.patch b/package/libcpprestsdk/0004-libcpprestsdk-fix-building-as-a-static-library.patch
new file mode 100644
index 0000000000..de900178d5
--- /dev/null
+++ b/package/libcpprestsdk/0004-libcpprestsdk-fix-building-as-a-static-library.patch
@@ -0,0 +1,38 @@ 
+From 8a9c3db14a390c0a8788405e52e9b8737a430191 Mon Sep 17 00:00:00 2001
+From: Adam Duskett <aduskett@gmail.com>
+Date: Mon, 12 Feb 2018 07:49:34 -0500
+Subject: [PATCH] libcpprestsdk: fix building as a static library
+
+Use pkg-config to find OpenSSL. This will automatically find any
+dependent libraries and put them in the correct order for linking.
+
+Upstream status: submitted
+https://github.com/Microsoft/cpprestsdk/pull/688
+
+Signed-off-by: Adam Duskett <aduskett@gmail.com>
+---
+ Release/cmake/cpprest_find_openssl.cmake | 10 +++++++---
+ 1 file changed, 7 insertions(+), 3 deletions(-)
+
+diff --git a/Release/cmake/cpprest_find_openssl.cmake b/Release/cmake/cpprest_find_openssl.cmake
+index 0b49a7e..2be8afb 100644
+--- a/Release/cmake/cpprest_find_openssl.cmake
++++ b/Release/cmake/cpprest_find_openssl.cmake
+@@ -41,8 +41,12 @@ function(cpprest_find_openssl)
+       # This should prevent linking against the system provided 0.9.8y
+       set(_OPENSSL_VERSION "")
+     endif()
+-    find_package(OpenSSL 1.0.0 REQUIRED)
+-
++    if(UNIX)
++		find_package(PkgConfig REQUIRED)
++		pkg_search_module(OPENSSL openssl REQUIRED)
++    else()
++    	find_package(OpenSSL 1.0.0 REQUIRED)
++	endif()
+     INCLUDE(CheckCXXSourceCompiles)
+     set(CMAKE_REQUIRED_INCLUDES "${OPENSSL_INCLUDE_DIR}")
+     set(CMAKE_REQUIRED_LIBRARIES "${OPENSSL_LIBRARIES}")
+-- 
+2.14.3
+