Do not modify OPENSSL_ROOT_DIR in the CMakeLists.txt

[mrexodia]

This is problematic because it prevents you from using a different (system) OpenSSL distribution.

Not exactly sure what the original intention was, but effectively this makes it so you can only use OpenSSL installed in the location you install xeus to (which doesn't make much sense). And besides you don't necessarily set CMAKE_INSTALL_PREFIX since you can override it with cmake --install <dir> --prefix at install-time. Perhaps the idea was:

if (NOT DEFINED OPENSSL_ROOT_DIR)
    set(OPENSSL_ROOT_DIR ${CMAKE_INSTALL_PREFIX})
endif()

But this is also wrong, you should use -DCMAKE_PREFIX_PATH=<openssl prefix> or -DOPENSSL_ROOT_DIR=<openssl prefix> on the CMake command line, not modify it in the project's CMake...

[JohanMabille]

Sorry for totally missing this! I agree that we should not set OPENSSL_ROOT_DIR in the CMakeLists.txt. However, we have situation where we build all the dependencies together with xeus-zmq, therefore OpenSSL should be searched only if OPENSSL_LIBRARY has not been defined yet.

[mrexodia]

Sorry for totally missing this! I agree that we should not set OPENSSL_ROOT_DIR in the CMakeLists.txt. However, we have situation where we build all the dependencies together with xeus-zmq, therefore OpenSSL should be searched only if OPENSSL_LIBRARY has not been defined yet.

Wouldn't find_package just find the same OpenSSL again? The modules usually check for targets they are supposed to create and/or they use an include guard to make sure they are only executed once.

[JohanMabille]

When we build everything together, we download the sources with ExternalProject_Add, so I think the targets are added directly to the main cmake, there is no call to the FindOpenSSL module. IIRC correctly, calling find_package in that scenario would find any OpenSSL instralled on the system and used it instead of the one we downloaded and build from source.

[mrexodia]

The target created by ExternalProject_Add is only used to build the project, it doesn't create targets for OpenSSL. I have to check our your project, but a quick glance makes it seem quite magical and I don't see where any targets for OpenSSL are defined. Regardless you should just set the OPENSSL_ROOT_DIR or CMAKE_MODULE_PATH (which you seem to set) in your superbuild project to change what version of OpenSSL find_package finds.

[mrexodia]

I would say that applying this PR + adding set(OPENSSL_ROOT_DIR "${CMAKE_INSTALL_PREFIX})" before https://github.com/jupyter-xeus/xeus-python-wheel/blob/main/CMakeLists.txt#L207-L219 should do the trick

[JohanMabille]

Indeed, I was totally wrong regarding the wheel; now I remember why we did that, it was added on purpose (see commit https://github.com/jupyter-xeus/xeus/commit/2a5fe692ae64b5d6c033654ee57ef332667266a9) because some people use xeus and its dependencies as submodules of their project. Therefore, you need to check if the targets have already be defined (because you have run add_subdirectory) before calling find_package, otherwise you got the target already defined error message when cmake finds the package.

[mrexodia]

I guess this makes sense for everything but OpenSSL. The other projects are CMake projects, so people can add them using add_subdirectory. There is no CMake project for OpenSSL, only the CMake-provided FindOpenSSL.cmake module. For OpenSSL the find_package call will not fail if it was already done in the parent project.

Even if checking OPENSSL_LIBRARY is necessary you still should not do set(OPENSSL_ROOT_DIR ${CMAKE_INSTALL_PREFIX}), because it prevents people from building the project (since find_package(OpenSSL) will fail).