[openssl] incorrect CMake config (patch)

[invy]

Is your feature request related to a problem? Please describe.

OpenSSL version 3.3 provides it's own CMake Config. vcpkg patches this config, one part of the patch corrects paths (vcpkg_cmake_fixup won't work here). But the other part of the patch does pretty strange thing. It calls CMake's own FindOpenSSL module, which confuses CMake and causes it not be able to find the openssl at all.

Proposed solution

Remove this part: https://github.com/microsoft/vcpkg/blob/f0c8848cde589649bca3ab3649668540403786ce/ports/openssl/cmake-config.patch#L27-L52 Instead add something like below, because original OpenSSL config fails to distinguish debug/release modes and that's what hat do be done IMHO (this was my quick and dirty fix)

diff --git a/exporters/cmake/OpenSSLConfig.cmake.in b/exporters/cmake/OpenSSLConfig.cmake.in
index 2d2321931d..859a6341ec 100644
--- a/exporters/cmake/OpenSSLConfig.cmake.in
+++ b/exporters/cmake/OpenSSLConfig.cmake.in
@@ -127,10 +127,15 @@ set(OPENSSL_FOUND YES)

 # Directories and names
 set(OPENSSL_INCLUDE_DIR "${_ossl_prefix}/{- unixify($OpenSSL::safe::installdata::INCLUDEDIR_REL, 1); -}")
-set(OPENSSL_LIBRARY_DIR "${_ossl_prefix}/{- unixify($OpenSSL::safe::installdata::LIBDIR_REL, 1); -}")
+if (CMAKE_BUILD_TYPE STREQUAL "Debug")
+  set(OPENSSL_LIBRARY_DIR "${_ossl_prefix}/debug/{- unixify($OpenSSL::safe::installdata::LIBDIR_REL, 1); -}")
+  set(OPENSSL_RUNTIME_DIR "${_ossl_prefix}/debug/{- unixify($OpenSSL::safe::installdata::BINDIR_REL, 1); -}")
+else()
+  set(OPENSSL_LIBRARY_DIR "${_ossl_prefix}/{- unixify($OpenSSL::safe::installdata::LIBDIR_REL, 1); -}")
+  set(OPENSSL_RUNTIME_DIR "${_ossl_prefix}/{- unixify($OpenSSL::safe::installdata::BINDIR_REL, 1); -}")
+endif()
 set(OPENSSL_ENGINES_DIR "${_ossl_prefix}/{- unixify($OpenSSL::safe::installdata::ENGINESDIR_REL, 1); -}")
 set(OPENSSL_MODULES_DIR "${_ossl_prefix}/{- unixify($OpenSSL::safe::installdata::MODULESDIR_REL, 1); -}")
-set(OPENSSL_RUNTIME_DIR "${_ossl_prefix}/{- unixify($OpenSSL::safe::installdata::BINDIR_REL, 1); -}")
 {- output_off() if $disabled{uplink}; "" -}
 set(OPENSSL_APPLINK_SOURCE "${_ossl_prefix}/{- unixify($OpenSSL::safe::installdata::APPLINKDIR_REL, 1); -}/applink.c")
 {- output_on() if $disabled{uplink}; "" -}
-- 
2.43.0

Better Solution

Better solution would be to modify OpenSSL Config in the way, that it sets target properties for debug/release builds, so that CMake is able to point to the proper libraries.

[dg0yt]

Can you please explain how to reproduce the problem?

[invy]

Can you please explain how to reproduce the problem?

in my use case find_package(OpenSSL CONFIG REQUIRED) failed with something like

[cmake] CMake Error at /usr/share/cmake-3.28/Modules/FindPackageHandleStandardArgs.cmake:230 (message):
[cmake]   Could NOT find OpenSSL, try to set the path to OpenSSL root folder in the
[cmake]   system variable OPENSSL_ROOT_DIR (missing: OPENSSL_INCLUDE_DIR) (found
[cmake]   version "3.3.1")
[cmake] Call Stack (most recent call first):
[cmake]   /usr/share/cmake-3.28/Modules/FindPackageHandleStandardArgs.cmake:600 (_FPHSA_FAILURE_MESSAGE)
[cmake]   /usr/share/cmake-3.28/Modules/FindOpenSSL.cmake:668 (find_package_handle_standard_args)
[cmake]   ..../vcpkg/installed/x64-linux/share/openssl/OpenSSLConfig.cmake:... (find_package)
[cmake]   ..../mylib/CMakeLists.txt:5 (find_package)

You just shouldn't mix CONFIG and MODULE modes for package lookup. I think CMake's own module just notice some inconsistancies (i.e. previously found package), because some variables are already set.

[dg0yt]

The call stack shows that you are not using the vcpkg.cmake toolchain file. This is unsupported.

[invy]

@dg0yt I'm not using it, however this is not the problem.

vcpkg.cmake doesn't have any specific handling for OpenSSL (it doesn't set VCPKG_OPENSSL_USE_SINGLE_CONFIG variable in any way). vcpkg.cmake own find_package override function does do anything special like it would do with Boost or ICU.

I still think, that the Patch provided by vcpkg is incorrect.

If upstream version of package provides CMake package configuration, it should be used and just modified to handle vcpkg installation layout, so that debug/release libraries won't get mixedup.

[dg0yt]

I'm not using it, however this is not the problem.

Please show the error with the vcpkg toolchain.

vcpkg.cmake doesn't have any specific handling for OpenSSL (it doesn't set VCPKG_OPENSSL_USE_SINGLE_CONFIG variable in any way). vcpkg.cmake own find_package override function does do anything special like it would do with Boost or ICU.

I still think, that the Patch provided by vcpkg is incorrect.

Port openssl has a vcpkg cmake wrapper which is invoked via vcpkg.cmake's find_package and makes the Find module give the right results. (vcpkg had that before OpenSSL added CMake config, and IIRC they modeled their config as a drop-in replacement. That's why I trust the module approach in vcpkg.)

If upstream version of package provides CMake package configuration, it should be used and just modified to handle vcpkg installation layout, so that debug/release libraries won't get mixedup.

This upstream uses a unique build system which pessimizes Windows in favor of more exotic systems. Their CMake config doesn't support multi-config. I'm still waiting for other changes to be integrated.

That being said, I'm not opposed to a better patch. But we need proper multi-config support.

[invy]

@dg0yt Oh sorry, I've forgot about vcpkg-cmake wrapper. This could of course work. But I think when OpenSSL already provides their CMake Config, the wrapper could be removed?

This upstream uses a unique build system which pessimizes Windows in favor of more exotic systems.

Does Windows Build produces CMake Config? If so I think it could be patched more or less reasonable.

Their CMake config doesn't support multi-config.

This is what I meant by saying about better solution. Adding "set_target_properties" to imported targets should actually make it multi-config capable, because cmake will handle configurations for you.

Here's example from what cmake usually generates with 'proj' library as an example:

# Import target "PROJ::proj" for configuration "Debug"
set_property(TARGET PROJ::proj APPEND PROPERTY IMPORTED_CONFIGURATIONS DEBUG)
set_target_properties(PROJ::proj PROPERTIES
  IMPORTED_LOCATION_DEBUG "${_IMPORT_PREFIX}/debug/lib/libproj.so.25.9.4.1"
  IMPORTED_SONAME_DEBUG "libproj.so.25"
  )

and release

# Import target "PROJ::proj" for configuration "Release"
set_property(TARGET PROJ::proj APPEND PROPERTY IMPORTED_CONFIGURATIONS RELEASE)
set_target_properties(PROJ::proj PROPERTIES
  IMPORTED_LOCATION_RELEASE "${_IMPORT_PREFIX}/lib/libproj.so.25.9.4.1"
  IMPORTED_SONAME_RELEASE "libproj.so.25"
  )

So the patch should kinda add similar set_property on corresponding targets and that would be it.

[dg0yt]

Does Windows Build produces CMake Config? If so I think it could be patched more or less reasonable.

"Windows Build"? For Windows, OpenSSL's Configure creates nmake Makefiles. Configure also creates the CMake config, all from Perl and templates.

This is what I meant by saying about better solution. Adding "set_target_properties" to imported targets should actually make it multi-config capable, because cmake will handle configurations for you.

Here's example from what cmake usually generates with 'proj' library as an example:

No added value. I'm familiar with PROJ, and I'm familiar with config generated by CMake.

The point is not just to set per-config properties, but to have separate files for general CMake code and for per-config stuff. vcpkg_cmake_config_fixup is tuned for this pattern which is used by CMake.

And note that the build type (as encoded in properties) is fixed at OpenSSL build time (i.e. when generating the config). Your initial post suggests codes which looks up CMAKE_BUILD_TYPE when the config is used. CMAKE_BUILD_TYPE has no meaning for multi-config generators.

So we also need a mapping of OpenSSL config parameters to the config type to be exported. And ideally forming a patch which is upstream might accept after some years.

[invy]

Are you sure, you need separate files per configuration? CMake libname-targets.cmake just includes just all libname-targets-*.cmake files. It doesn't matter whether you have one or multiple, because all will be added and set. Again, the key point here is set_properties(TARGET target ... DEBUG/RELEASE). This will allow cmake to correctly generate build-system specific code. The Config Files or Modules just set target properties more or less.

[github-actions[bot]]

This is an automated message. Per our repo policy, stale issues get closed if there has been no activity in the past 28 days. The issue will be automatically closed in 14 days. If you wish to keep this issue open, please add a new comment.

[smartnet-club]

The problem still exists

There are 2 configs in vcpkg_installed/x64-linux/share/openssl

1. OpenSSLConfig.cmake
2. vcpkg-cmake-wrapper.cmake

The contents of OpenSSLConfig.cmake are incorrect:

get_filename_component(_ossl_prefix "${CMAKE_CURRENT_LIST_FILE}" PATH)
get_filename_component(_ossl_prefix "${_ossl_prefix}" PATH)
get_filename_component(_ossl_prefix "${_ossl_prefix}" PATH)
get_filename_component(_ossl_prefix "${_ossl_prefix}" PATH)
get_filename_component(_ossl_prefix "${_ossl_prefix}" PATH)

If you call _findpackage(OpenSSL), it will use the wrong OpenSSLConfig.cmake And target OpenSSL::Crypto has INTERFACE_INCLUDE_DIRECTORIES: cmake-build-dir/include instead of correct cmake-build-dir/vcpkg_installed/x64-linux

It affects building of opentelemetry-cpp port with otlp_grpc enabled The error: Imported target "gRPC::grpc++" includes non-existent path

[dg0yt]

If you call _findpackage(OpenSSL), it will use the wrong OpenSSLConfig.cmake

This call does not use CMake config unless you ask for it with extra options or variables. The wrapper does the right thing for multi-config usage via the traditional Find module.

[smartnet-club]

This call does not use CMake config unless you ask for it with extra options or variables. The wrapper does the right thing for multi-config usage via the traditional Find module.

If you call find_package(OpenSSL) from another port build, the wrong OpenSSLConfig.cmake will be used That's why gRPC::grpc++ has OpenSSL as a dependency with the wrong INTERFACE_INCLUDE_DIRECTORIES

[dg0yt]

Virtually all ports use OpenSSL successfully. Maybe your own project is telling CMake to prefer CONFIG? (I would have preferred to not install the openssl CONFIG at all, until it is ready for use.)

[smartnet-club]

The problem can be reproduced by building the opentelemetry-cpp port with otlp_grpc enabled

No special settings in my project. The error appears after updating vcpkg to the latest version.

And yes, it can be fixed by patching OpenSSL's portfile.cmake:

file(REMOVE "${CURRENT_PACKAGES_DIR}/share/${PORT}/OpenSSLConfig.cmake")
file(REMOVE "${CURRENT_PACKAGES_DIR}/share/${PORT}/OpenSSLConfigVersion.cmake")

[dg0yt]

The prefix/include dir patching was probably broken with 3.3.2 in https://github.com/openssl/openssl/commit/1c437b5704c9ee5f667bc2b11e5fdf176dfb714f. But I see the issue was opened for 3.3.1.

I added a correction to the 3.4.0 update PR now.

It still doesn't give the right libdirs etc. for the debug config. Everybody invited to contribute.

[dg0yt]

The problem can be reproduced by building the opentelemetry-cpp port with otlp_grpc enabled

So this port's CMakeLists.txt has set(CMAKE_FIND_PACKAGE_PREFER_CONFIG TRUE). This is the variable which changes search behavior. This may cause problems. We have seen it with ZLIB on Fedora.

[dg0yt]

The problem can be reproduced by building the opentelemetry-cpp port with otlp_grpc enabled

So this port's CMakeLists.txt has set(CMAKE_FIND_PACKAGE_PREFER_CONFIG TRUE). This is the variable which changes search behavior. This may cause problems. We have seen it with ZLIB on Fedora.

Fixing in #41742.

[dg0yt]

The fix for opentelemetry-cpp was merged.

openssl still waits for contributions.