@merlimat @Demogorgon314 @RobertIndie Now all tests passed, PTAL.

Currently I'm working on fixing the Windows build when LINK_STATIC is ON, which might be based on this PR. Because when LINK_STATIC is ON, no Windows dependencies could be found because the suffix is not .a. This PR accidentally fixes some issues of finding dependencies. Regarding other issues to build statically linked Windows library, will push another PR later.

@BewareMyPower CMake is now failing to find OpenSSL on Mac (installed with brew):

CMake Error at /opt/homebrew/Cellar/cmake/3.23.0/share/cmake/Modules/FindPackageHandleStandardArgs.cmake:230 (message):
  Could NOT find OpenSSL, try to set the path to OpenSSL root folder in the
  system variable OPENSSL_ROOT_DIR (missing: OPENSSL_CRYPTO_LIBRARY
  OPENSSL_INCLUDE_DIR)
Call Stack (most recent call first):
  /opt/homebrew/Cellar/cmake/3.23.0/share/cmake/Modules/FindPackageHandleStandardArgs.cmake:594 (_FPHSA_FAILURE_MESSAGE)
  /opt/homebrew/Cellar/cmake/3.23.0/share/cmake/Modules/FindOpenSSL.cmake:578 (find_package_handle_standard_args)
  CMakeLists.txt:127 (find_package)

@merlimat I just tested again in my local env.

Here is my command (just test the default LINK_STATIC option because it does not affect OpenSSL):

cmake -Wno-dev \
    -DCMAKE_PREFIX_PATH="/usr/local" \
    -DBUILD_TESTS=OFF -DBUILD_PERF_TOOLS=ON \
    -B build

Here are my outputs after this PR:

-- Found OpenSSL: /opt/local/lib/libcrypto.dylib (found version "3.0.4")  
-- Found OpenSSL: /opt/local/lib/libcrypto.dylib (found version "3.0.4")  
RECORD_OPENSSL_SSL_LIBRARY: /opt/local/lib/libssl.dylib
RECORD_OPENSSL_CRYPTO_LIBRARY: /opt/local/lib/libcrypto.dylib
-- Found OpenSSL: /usr/local/opt/openssl/lib/libcrypto.dylib (found version "3.0.4")  
OPENSSL_INCLUDE_DIR: /usr/local/opt/openssl/include//opt/homebrew/opt/openssl/include
OPENSSL_SSL_LIBRARY: /usr/local/opt/openssl/lib/libssl.dylib
OPENSSL_CRYPTO_LIBRARY: /usr/local/opt/openssl/lib/libcrypto.dylib

Here are my outputs before this PR (just added the same logs):

-- Found OpenSSL: /usr/local/opt/openssl/lib/libcrypto.dylib   
RECORD_OPENSSL_SSL_LIBRARY: /usr/local/opt/openssl/lib/libssl.dylib
RECORD_OPENSSL_CRYPTO_LIBRARY: /usr/local/opt/openssl/lib/libcrypto.dylib
OPENSSL_INCLUDE_DIR: /usr/local/opt/openssl/include//opt/homebrew/opt/openssl/include
OPENSSL_SSL_LIBRARY: /usr/local/opt/openssl/lib/libssl.dylib
OPENSSL_CRYPTO_LIBRARY: /usr/local/opt/openssl/lib/libcrypto.dylib

The patch to add logs:

diff --git a/CMakeLists.txt b/CMakeLists.txt
index d3876f8..5952576 100644
--- a/CMakeLists.txt
+++ b/CMakeLists.txt
@@ -135,6 +135,8 @@ set(OPENSSL_USE_STATIC_LIBS FALSE)
 find_package(OpenSSL REQUIRED)
 set(RECORD_OPENSSL_SSL_LIBRARY ${OPENSSL_SSL_LIBRARY})
 set(RECORD_OPENSSL_CRYPTO_LIBRARY ${OPENSSL_CRYPTO_LIBRARY})
+message("RECORD_OPENSSL_SSL_LIBRARY: " ${RECORD_OPENSSL_SSL_LIBRARY})
+message("RECORD_OPENSSL_CRYPTO_LIBRARY: " ${RECORD_OPENSSL_CRYPTO_LIBRARY})
 
 unset(OPENSSL_FOUND CACHE)
 unset(OPENSSL_INCLUDE_DIR CACHE)
@@ -270,6 +272,9 @@ endif()
 find_package(Boost REQUIRED COMPONENTS ${BOOST_COMPONENTS})
 
 find_package(OpenSSL REQUIRED)
+message("OPENSSL_INCLUDE_DIR: " ${OPENSSL_INCLUDE_DIR})
+message("OPENSSL_SSL_LIBRARY: " ${OPENSSL_SSL_LIBRARY})
+message("OPENSSL_CRYPTO_LIBRARY: " ${OPENSSL_CRYPTO_LIBRARY})
 
 if (BUILD_TESTS)
     find_path(GTEST_INCLUDE_PATH gtest/gtest.h)

It looks like this PR brings some unexpected changes, I will investigate and fix it soon.

I think there’s one extra find_package for OpenSSL at the beginning (3 calls in total). It should be removed.

Additionally, we should still use static lib for OpenSSL too

Additionally, we should still use static lib for OpenSSL too

It is exactly what I am confused. From https://pulsar.apache.org/docs/client-libraries-cpp#install-dependencies

The libpulsarwithdeps.a does not include library openssl related libraries libssl and libcrypto, because these two libraries are related to security. It is more reasonable and easier to use the versions provided by the local system to handle security issues and upgrade libraries.

I also found the original PR (apache/pulsar#6458) from @jiazhai.

Because if there is a security problem in libssl, all the machines can just update their own libssl library without rebuilding libpulsar.a.

Should we revert these complicated logic to find and link OpenSSL dependency?

I think that if we don't do that, there can be linking problems between different OpenSSL version (eg: 1.1.1 vs 3.0.4)

if we don't do that, there can be linking problems between different OpenSSL version (eg: 1.1.1 vs 3.0.4)

Yeah, I agree. The ABI compatibility changed a lot, see https://abi-laboratory.pro/?view=timeline&l=openssl

In addition,

libpulsar.so is a shared library, containing statically linked boost and openssl.

libpulsarnossl.so is a shared library, similar to libpulsar.so except that the libraries openssl and crypto are dynamically linked.

I think we should remove libpulsarnossl.so and link statically to openssl in libpulsarwithdeps.a, right? @merlimat