Open m00n5hyn3 opened 7 months ago
Well, thanks for looking at it... I take it we mean this kind of thing
diff --git a/lib/tls/CMakeLists.txt b/lib/tls/CMakeLists.txt
index 230b0d29..92534fbc 100644
--- a/lib/tls/CMakeLists.txt
+++ b/lib/tls/CMakeLists.txt
@@ -268,10 +268,7 @@ if (LWS_WITH_SSL)
if (NOT OPENSSL_FOUND AND NOT LWS_WITH_BORINGSSL)
# TODO: Add support for STATIC also.
if (NOT LWS_PLAT_FREERTOS)
- find_package(PkgConfig QUIET)
- pkg_check_modules(PC_OPENSSL openssl QUIET)
find_package(OpenSSL REQUIRED)
- list(APPEND OPENSSL_LIBRARIES ${PC_OPENSSL_LINK_LIBRARIES})
set(OPENSSL_LIBRARIES ${OPENSSL_LIBRARIES} PARENT_SCOPE)
endif()
set(OPENSSL_INCLUDE_DIRS "${OPENSSL_INCLUDE_DIR}")
I'm not sure what will happen on other platforms with the change... the last patch there was from @zzblydia, is it possible they could try the change on their platform?
No problem. I also have a conan layer in my build environment and I needed to know if that one wasn't causing the problem.
I looked into the FindOpenSSL.cmake module in the cmake repository and that also references the pkgconfig package for Unix. I would expect it to still work on other platforms too. The only platform I am worried about is the Mac one. It would be nice if someone could verify it there.
The patch doesn't seem to make the mac situation any worse (with openssl from homebrew) but with or without the patch, you have to give it this kind of help
$ cmake .. -DLWS_OPENSSL_LIBRARIES="/usr/local/opt/openssl/lib/libssl.dylib;/usr/local/opt/openssl/lib/libcrypto.dylib" -DLWS_OPENSSL_INCLUDE_DIRS=/usr/local/opt/openssl@1.1/include
Setting the LWS_OPENSSL_LIBRARIES and LWS_OPENSSL_INCLUDE_DIRS bypasses the library detection from the build scripts altogether. It just overwrites the items detected by cmake. It is the workaround I implemented to make my cross compilation work for now.
I think the difficulty is on OSX, there's no simple "good bet" to find OpenSSL installed pieces.
Eg, the headers as installed by homebrew are at /usr/local/opt/openssl@1.1/include
, where the versioned part is a homebrew thing.
This issue is similar to the one I encountered last time, where the openssl library found through pkgconfig did not have an absolute path, resulting in a linking error. I modified PC_OPENSSL_LIBRARIES
to PC_OPENSSL_LINK_LIBRARIES
in last patch.
I'm confused about the output of PC_OPENSSL_LINK_LIBRARIES in this issue because the cmake(your cmake version?) documentation states that when using pkg_check_modules, <XXX>_LINK_LIBRARIES
(PC_OPENSSL_LINK_LIBRARIES) should return the library and its absolute path.
I also notice that pkgconfig is just for unix in the FindOpenSSL.cmake. If pkg_check_modules(PC_OPENSSL openssl QUIET) was removed, I'm concerned that it might not be able to find the openssl library correctly on certain platforms(e.g. windows which configure mingw OpenSSL libraries through pkgconfig).
Is it better to check OPENSSL_FOUND before 'list(APPEND OPENSSL_LIBRARIES ${PC_OPENSSL_LINK_LIBRARIES})' ?
Thanks for looking at it. I think maybe other things happening here related to:
When cross-compiling libwebsockets for an embedded device it fails to correctly detect OpenSSL when the host system also contains OpenSSL development files.
The pc code is already contingent on OPENSSL_FOUND not being true, assuming this test is the right kind
if (NOT OPENSSL_FOUND AND NOT LWS_WITH_BORINGSSL)
...
is at the top of the existing stanza.
when the host system also contains OpenSSL development files.
What does the toolchain file for your 926ejs build actually say? You should be defeating visibility of whatever is in the build host by having these in it
# Search headers and libraries in the target environment only.
set(CMAKE_FIND_ROOT_PATH_MODE_LIBRARY ONLY)
set(CMAKE_FIND_ROOT_PATH_MODE_INCLUDE ONLY)
Oh... we run find_package(OpenSSL ...
inside the stanza... did you mean this kind of thing?
diff --git a/lib/tls/CMakeLists.txt b/lib/tls/CMakeLists.txt
index 230b0d29..b341fea1 100644
--- a/lib/tls/CMakeLists.txt
+++ b/lib/tls/CMakeLists.txt
@@ -271,7 +271,9 @@ if (LWS_WITH_SSL)
find_package(PkgConfig QUIET)
pkg_check_modules(PC_OPENSSL openssl QUIET)
find_package(OpenSSL REQUIRED)
- list(APPEND OPENSSL_LIBRARIES ${PC_OPENSSL_LINK_LIBRARIES})
+ if (NOT OPENSSL_FOUND)
+ list(APPEND OPENSSL_LIBRARIES ${PC_OPENSSL_LINK_LIBRARIES})
+ endif()
set(OPENSSL_LIBRARIES ${OPENSSL_LIBRARIES} PARENT_SCOPE)
endif()
set(OPENSSL_INCLUDE_DIRS "${OPENSSL_INCLUDE_DIR}")
if so it might be worth a try.
Oh... we run
find_package(OpenSSL ...
inside the stanza... did you mean this kind of thing?diff --git a/lib/tls/CMakeLists.txt b/lib/tls/CMakeLists.txt index 230b0d29..b341fea1 100644 --- a/lib/tls/CMakeLists.txt +++ b/lib/tls/CMakeLists.txt @@ -271,7 +271,9 @@ if (LWS_WITH_SSL) find_package(PkgConfig QUIET) pkg_check_modules(PC_OPENSSL openssl QUIET) find_package(OpenSSL REQUIRED) - list(APPEND OPENSSL_LIBRARIES ${PC_OPENSSL_LINK_LIBRARIES}) + if (NOT OPENSSL_FOUND) + list(APPEND OPENSSL_LIBRARIES ${PC_OPENSSL_LINK_LIBRARIES}) + endif() set(OPENSSL_LIBRARIES ${OPENSSL_LIBRARIES} PARENT_SCOPE) endif() set(OPENSSL_INCLUDE_DIRS "${OPENSSL_INCLUDE_DIR}")
if so it might be worth a try.
Yes, that's what I'm trying to say. or if (NOT OPENSSL_FOUND AND PC_OPENSSL_FOUND). NOTE: find_package(OpenSSL REQUIRED) The REQUIRED option stops processing with an error message if the package cannot be found.
Following what's discussed, I added a patch that might help on main.
I agree that the not found check is the better approach and will keep it working for other platforms. You only need to do the pkgconfig check if you haven't found OpenSSL yet. I tested the change on my cross-compile build and it works fine. I do have one recommendation though as a minor (performance) improvement:
find_package(OpenSSL REQUIRED)
if (NOT OPENSSL_FOUND)
find_package(PkgConfig QUIET)
pkg_check_modules(PC_OPENSSL openssl QUIET)
list(APPEND OPENSSL_LIBRARIES ${PC_OPENSSL_LINK_LIBRARIES})
endif()
set(OPENSSL_LIBRARIES ${OPENSSL_LIBRARIES} PARENT_SCOPE)
I did some more testing on my Debian 11 host system regarding the OpenSSL and PkgConfig packages for cmake 3.26.3:
find_package(OpenSSL REQUIRED)
OPENSSL_INCLUDE_DIR=/usr/include
OPENSSL_LIB_DIR=/usr/lib/x86_64-linux-gnu/libssl.so;/usr/lib/x86_64-linux-gnu/libcrypto.so
find_package(PkgConfig QUIET)
pkg_check_modules(PC_OPENSSL openssl QUIET)
PC_OPENSSL_INCLUDE_DIR=
PC_OPENSSL_LIB_DIR=ssl;crypto
Clearly the first find already found the correct libraries for my host system. There is no need to do the second one in that case. On my host system pkg-config just returns the two libraries. The compiler environment is capable of finding the libraries and headers through the default paths.
When I am cross-compiling and use a specific platform build of OpenSSL, I specify OPENSSL_ROOT_DIR in the build process to point to the correct OpenSSL. CMake correctly finds these with the absolute paths as expected. The problem is that the pkgconfig statement also find the OpenSSL files on my host system, which are not compatible with my arm build. Even if it would find absolute paths to the OpenSSL libraries the build would still fail as it would link x86_64 libraries against my arm build.
On a side note: I don't think it is the responsibility of the lws build files to include a PkgConfig check to find OpenSSL correctly. As you make use of cmake find logic using find_package(OpenSSL REQUIRED) you defer the responsibility to cmake to find your files. If this doesn't work on some platforms, it should be fixed/improved in the FindOpenSSL.cmake. The if not found is basically a workaround for something that is missing in cmake.
To some extent, I agree with the above viewpoint(that On a side note);
Also i test below content in CMakeLists.txt on debian11.8 with cmake 3.26.3 and openssl 1.1.1w.
cmake_minimum_required(VERSION 3.25)
include(CheckFunctionExists)
project(find_openssl_test C)
set(CMAKE_C_STANDARD 11)
# test pkg_check_modules
find_package(PkgConfig QUIET)
message("PKGCONFIG_FOUND: ${PKGCONFIG_FOUND}")
pkg_check_modules(PC_OPENSSL openssl QUIET)
if (PC_OPENSSL_FOUND)
message("PC_OPENSSL_VERSION: ${PC_OPENSSL_VERSION}")
message("PC_OPENSSL_INCLUDE_DIRS: ${PC_OPENSSL_INCLUDE_DIRS}")
message("PC_OPENSSL_LIBRARIES: ${PC_OPENSSL_LIBRARIES}")
message("PC_OPENSSL_LINK_LIBRARIES: ${PC_OPENSSL_LINK_LIBRARIES}")
endif ()
it has output:
PKGCONFIG_FOUND: TRUE
PC_OPENSSL_VERSION: 1.1.1w
PC_OPENSSL_INCLUDE_DIRS:
PC_OPENSSL_LIBRARIES: ssl;crypto
PC_OPENSSL_LINK_LIBRARIES: /usr/lib/x86_64-linux-gnu/libssl.so;/usr/lib/x86_64-linux-gnu/libcrypto.so
-- Configuring done (0.0s)
-- Generating done (0.0s)
In fact there are also openssl 3.0.12 and openssl 3.2.0 on my environment(both are installed from source) for testing, but the finding output of pkg-config depends on openssl.pc/libssl.pc/libcrypto.pc in /usr/lib/x86_64-linux-gnu/pkgconfig folder.
so i think it should use LWS_OPENSSL_INCLUDE_DIRS
and LWS_OPENSSL_LIBRARIES
to indicate what we want to use when there are openssl libraries (version diff or for different architectures) rather than prevent pkg-config from finding it.
When cross-compiling libwebsockets for an embedded device it fails to correctly detect OpenSSL when the host system also contains OpenSSL development files.
NOTE: In the explanation of the problem I stripped some long folders from the build to make it more readable.
The build fails with a common error that has been seen in other cross-compilation issues:
Looking at the CMakeConfigureLog.yaml I found the following problem:
If I have set the OPENSSL_ROOT_DIR to 'openssl/1.1.1f' and cmake is perfectly capable to find the needed openssl libraries.
In the cross-compiling scenario the ssl and crypto library can't be found on the search path of the compiler. That shouldn't be a problem as cmake includes the absolute path to the libraries to be used during linking. When looking at the compilation line I noticed that the libraries are added twice:
I did a separate test cross compiling a program that depends on cmake and that build without problem. What I noticed there was that the definitions for OpenSSL found by cmake were different:
The last part ;ssl;crypto was not part of that build. The next question was why does the libwebsocket build add these two libraries to the list? I found the anwser in the 'lib/tls/CMakeLists.txt' file at lines 271,272,274.
The build file uses pkgconfig too to find openssl libraries and this added the extra ;ssl;crypto to the libraries. In my case I also have OpenSSL development files on my host system and pkgconfig will find these and add them as extra libraries to the list. When I removed the pkgconfig lines form the lib/tls/CMakeLists.txt file the cross-compilation build works.
I believe this can be removed from the build file. The responsibility to find the OpenSSL package lies with cmake and it seems very capable to find the correct paths and libraries during host builds as well as cross-compilation builds.