cpprest_find_openssl.cmake broken - fix #1521

mouse07410 · 2020-10-23T21:44:51Z

Current cpprest_find_openssl.cmake is broken on MacOS for anything but Homebrew (which is the least secure way to install packages).

So, while it correctly locates the installed OpenSSL, it still fails to correctly link with it, failing to pass the -Lxxxxx flag to the linker.

This patch fixes that problem:

diff --git a/Release/cmake/cpprest_find_openssl.cmake b/Release/cmake/cpprest_find_openssl.cmake
index 93336636..2fcb6b6c 100644
--- a/Release/cmake/cpprest_find_openssl.cmake
+++ b/Release/cmake/cpprest_find_openssl.cmake
@@ -33,15 +33,29 @@ function(cpprest_find_openssl)
   else()
     if(APPLE)
       if(NOT DEFINED OPENSSL_ROOT_DIR)
-        # Prefer a homebrew version of OpenSSL over the one in /usr/lib
+        # Prefer a homebrew or Macports version of OpenSSL over the one in /usr/lib
         file(GLOB OPENSSL_ROOT_DIR /usr/local/Cellar/openssl*/*)
-        # Prefer the latest (make the latest one first)
-        list(REVERSE OPENSSL_ROOT_DIR)
-        list(GET OPENSSL_ROOT_DIR 0 OPENSSL_ROOT_DIR)
-      endif()
+        if(OPENSSL_ROOT_DIR STREQUAL "")
+          # No homebrew-installed OpenSSL, try Macports-installed OpenSSL
+          find_package(PkgConfig) # Macports provides pkg-config for OpenSSL it installs
+          pkg_search_module(OPENSSL openssl)
+          if(OPENSSL_FOUND)
+            target_link_libraries(cpprest PRIVATE ${OPENSSL_LDFLAGS})
+          else(OPENSSL_FOUND)
+            find_package(OpenSSL 1.1.1 REQUIRED)
+          endif(OPENSSL_FOUND)
+          set(OPENSSL_ROOT_DIR "${OPENSSL_LIBDIR}/../") # to silence VSC IntelliSence error messahe
+        else(OPENSSL_ROOT_DIR STREQUAL "")
+          # Prefer the latest (make the latest one first)
+          list(REVERSE OPENSSL_ROOT_DIR)
+          list(GET OPENSSL_ROOT_DIR 0 OPENSSL_ROOT_DIR)
+        endif(OPENSSL_ROOT_DIR STREQUAL "")
+      endif(NOT DEFINED OPENSSL_ROOT_DIR)
       # This should prevent linking against the system provided 0.9.8y
       message(STATUS "OPENSSL_ROOT_DIR = ${OPENSSL_ROOT_DIR}")
-      set(_OPENSSL_VERSION "")
+      if(NOT OPENSSL_FOUND) # If there is neither homebrew- nor Macports-installed OpenSSL
+        set(_OPENSSL_VERSION "")
+      endif(NOT OPENSSL_FOUND)
     endif()
     if(UNIX)
       find_package(PkgConfig)
@@ -67,9 +81,9 @@ function(cpprest_find_openssl)
 
   add_library(cpprestsdk_openssl_internal INTERFACE)
   if(TARGET OpenSSL::SSL)
-    target_link_libraries(cpprestsdk_openssl_internal INTERFACE OpenSSL::SSL)
+    target_link_libraries(cpprestsdk_openssl_internal INTERFACE "${OPENSSL_LDFLAGS}")
   else()
-    target_link_libraries(cpprestsdk_openssl_internal INTERFACE "$<BUILD_INTERFACE:${OPENSSL_LIBRARIES}>")
+    target_link_libraries(cpprestsdk_openssl_internal INTERFACE "$<BUILD_INTERFACE:${OPENSSL_LDFLAGS}>")
     target_include_directories(cpprestsdk_openssl_internal INTERFACE "$<BUILD_INTERFACE:${OPENSSL_INCLUDE_DIR}>")
   endif()

Coincidentally, MacOS (thankfully!) does not include OpenSSL-0.9.8 anymore. Unfortunately, it includes LibreSSL, whose libraries one cannot link against:

$ /usr/bin/openssl version
LibreSSL 2.8.3

Thus, there's no risk that pkg-config would find system-provided OpenSSL on MacOS - it's not even listed in /usr/lib/pkgconfig/.

This issue relates to - aka provides the fix for - #1518.

Update

I suspect that the critical change is this:

@@ -67,9 +81,9 @@ function(cpprest_find_openssl)
 
   add_library(cpprestsdk_openssl_internal INTERFACE)
   if(TARGET OpenSSL::SSL)
-    target_link_libraries(cpprestsdk_openssl_internal INTERFACE OpenSSL::SSL)
+    target_link_libraries(cpprestsdk_openssl_internal INTERFACE "${OPENSSL_LDFLAGS}")
   else()
-    target_link_libraries(cpprestsdk_openssl_internal INTERFACE "$<BUILD_INTERFACE:${OPENSSL_LIBRARIES}>")
+    target_link_libraries(cpprestsdk_openssl_internal INTERFACE "$<BUILD_INTERFACE:${OPENSSL_LDFLAGS}>")
     target_include_directories(cpprestsdk_openssl_internal INTERFACE "$<BUILD_INTERFACE:${OPENSSL_INCLUDE_DIR}>")
   endif()

but I haven't tested it with only this change.

mouse07410 · 2020-10-27T15:44:50Z

@barcharcraz ping?

microsoft / cpprestsdk

cpprest_find_openssl.cmake broken - fix #1521

cpprest_find_openssl.cmake broken - fix #1521

mouse07410 commented Oct 23, 2020 •

edited

mouse07410 commented Oct 27, 2020

microsoft / cpprestsdk

Join GitHub today

GitHub is where the world builds software

cpprest_find_openssl.cmake broken - fix #1521

cpprest_find_openssl.cmake broken - fix #1521

Comments

mouse07410 commented Oct 23, 2020 • edited

Update

mouse07410 commented Oct 27, 2020

Essential cookies

Always active

Analytics cookies

mouse07410 commented Oct 23, 2020 •

edited