Coder Social home page Coder Social logo

Comments (12)

lws-team avatar lws-team commented on June 2, 2024

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?

from libwebsockets.

m00n5hyn3 avatar m00n5hyn3 commented on June 2, 2024

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.

from libwebsockets.

lws-team avatar lws-team commented on June 2, 2024

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/[email protected]/include

from libwebsockets.

m00n5hyn3 avatar m00n5hyn3 commented on June 2, 2024

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.

from libwebsockets.

lws-team avatar lws-team commented on June 2, 2024

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/[email protected]/include, where the versioned part is a homebrew thing.

from libwebsockets.

zzblydia avatar zzblydia commented on June 2, 2024

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})' ?

from libwebsockets.

lws-team avatar lws-team commented on June 2, 2024

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)

from libwebsockets.

lws-team avatar lws-team commented on June 2, 2024

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.

from libwebsockets.

zzblydia avatar zzblydia commented on June 2, 2024

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.

from libwebsockets.

lws-team avatar lws-team commented on June 2, 2024

Following what's discussed, I added a patch that might help on main.

from libwebsockets.

m00n5hyn3 avatar m00n5hyn3 commented on June 2, 2024

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.

from libwebsockets.

zzblydia avatar zzblydia commented on June 2, 2024

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.

from libwebsockets.

Related Issues (20)

Recommend Projects

  • React photo React

    A declarative, efficient, and flexible JavaScript library for building user interfaces.

  • Vue.js photo Vue.js

    🖖 Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.

  • Typescript photo Typescript

    TypeScript is a superset of JavaScript that compiles to clean JavaScript output.

  • TensorFlow photo TensorFlow

    An Open Source Machine Learning Framework for Everyone

  • Django photo Django

    The Web framework for perfectionists with deadlines.

  • D3 photo D3

    Bring data to life with SVG, Canvas and HTML. 📊📈🎉

Recommend Topics

  • javascript

    JavaScript (JS) is a lightweight interpreted programming language with first-class functions.

  • web

    Some thing interesting about web. New door for the world.

  • server

    A server is a program made to process requests and deliver data to clients.

  • Machine learning

    Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.

  • Game

    Some thing interesting about game, make everyone happy.

Recommend Org

  • Facebook photo Facebook

    We are working to build community through open source technology. NB: members must have two-factor auth.

  • Microsoft photo Microsoft

    Open source projects and samples from Microsoft.

  • Google photo Google

    Google ❤️ Open Source for everyone.

  • D3 photo D3

    Data-Driven Documents codes.