Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

openssl: let openssl create FindOpenSSL.cmake script equivalent to official #486

Closed
wants to merge 3 commits into from

Conversation

madebr
Copy link
Contributor

@madebr madebr commented Dec 13, 2019

Specify library name and version: openssl/all

  • I've read the guidelines for contributing.
  • I've followed the PEP8 style guides for Python code in the recipes.
  • I've used the latest Conan client version.
  • I've tried at least one configuration locally with the
    conan-center hook activated.

This is related to #419
When using penssl packages generated from this recipe,
the generated FindOpenSSL.cmake can be used interchangeably with the offical FindOpenSSL.cmake module..

@conan-center-bot
Copy link
Collaborator

All green! 😊

@conan-center-bot
Copy link
Collaborator

All green! 😊

@madebr madebr mentioned this pull request Dec 13, 2019
4 tasks
@conan-center-bot
Copy link
Collaborator

An unexpected error happened and has been reported. Help is on its way! 🏇

find_library(CONAN_CRYPTO_ABSOLUTE_LIBRARY NAME ${CONAN_CRYPTO_LIBRARY} PATHS ${OpenSSL_LIB_DIRS} NO_DEFAULT_PATH NO_CMAKE_FIND_ROOT_PATH)
find_library(CONAN_SSL_ABSOLUTE_LIBRARY NAME ${CONAN_SSL_LIBRARY} PATHS ${OpenSSL_LIB_DIRS} NO_DEFAULT_PATH NO_CMAKE_FIND_ROOT_PATH)

# https://cmake.org/cmake/help/latest/module/FindOpenSSL.html

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It is probably a good idea to call find_package_handle_standard_args (https://cmake.org/cmake/help/latest/module/FindPackageHandleStandardArgs.html) to report the error if the files could not be found and set some of the variables that are set in the next few lines.

There is more documentation on regarding writing finders in https://gitlab.kitware.com/cmake/community/wikis/doc/tutorials/How-To-Find-Libraries.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good idea about find_package_handle_standard_args.

set(OPENSSL_SSL_LIBRARY ${CONAN_CRYPTO_ABSOLUTE_LIBRARY})
set(OPENSSL_SSL_LIBRARIES ${CONAN_CRYPTO_ABSOLUTE_LIBRARY} ${OpenSSL_SYSTEM_LIBS})

if(NOT CMAKE_VERSION VERSION_LESS 3.0)

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are conan recipes targeting cmake versions older than 3.0 (cmake 3 was released in 2014 https://cmake.org/pipermail/cmake/2014-June/057793.html)? Maybe people using conan are using a reasonably recent version of cmake? Is that a valid assumption?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've just copied the same logic as the FindXXX.cmake files generated by conan.

But more generally, I believe conan should add a feature for cmake_find_package and cmake_find_package_multi. As this pull request won't co-operate well with the multi-generator.

conan would need a feature that allows this:

def package_info(self):
    self.cpp_info.name = "OpenSSL"
    self.cpp_info.libs = [*self._ssl_crypto_library]
    system_libs = []
    if self.settings.os == "Windows":
        system_libs.extend(["crypt32", "msi", "ws2_32", "advapi32", "user32", "gdi32"])
    elif self.settings.os == "Linux":
        system_libs.extend(["dl", "pthread"])
    self.cpp_info.system_libs.extend(system_libs)

    # By default: the generated cmake targets should inherit the dependencies of this recipe
    ssl_system_libs = ["crypt32", "msi", "ws2_32", "advapi32", "user32", "gdi32"]
    crypto_system_libs = ["crypt32", "msi", "advapi32", "user32", "gdi32"]
    for generator in ("cmake_find_package", "cmake_find_package_multi"):
        ssl_library, crypto_library = self._ssl_crypto_library

        # Set OPENSSL_FOUND unconditionally, is a string by default
        self.cpp_info.generators[generator].add_variable("OPENSSL_FOUND", "ON")
        self.cpp_info.generators[generator].add_variable("OPENSSL_VERSION", self.version, type=CMakeVariableType.String)

        # Add OPENSSL_SSL_LIBRARIES variable: it will be set by find_library calls + is genex ==> will be a generator expression when generator == cmake_find_package_multi
        sslLibraryVariable = self.cpp_info.generators[generator].add_variable("OPENSSL_SSL_LIBRARY", self._ssl_library, type=CMakeVariableType.Library, genex=True)

        sslCMakeTarget = self.cpp_info.generators[generator].createCMakeTarget("OpenSSL::SSL")
        # - override libraries
        sslCMakeTarget.libs = [ssl_library]
        # - override system libraries (SSL requires different system_libs then crypto)
        sslCMakeTarget.system_libs = ssl_system_libs
        # - inherit include directories
        # sslCMakeTarget.includedirs = ["include"]
        # - inherit package dependencies (so in CMake, link to all dependencies (CONAN_PKG::dep1, CONAN_PKG_dep2)
        # sslCMakeTarget.deps = ["dep1", "dep2"]
        # - inherit defines
        # sslCMakeTarget.defines = []
        # - inherit link flags
        # sslCMakeTarget.linkflags = []

        # Add OPENSSL_SSL_LIBRARIES variable: it will be set by find_library calls + is genex ==> will be a generator expression when generator == cmake_find_package_multi
        cryptoLibraryVariable = self.cpp_info.generators[generator].add_variable("OPENSSL_CRYPTO_LIBRARY", self._crypto_library, type=CMakeVariableType.Library, genex=True)

        cryptoCMakeTarget = self.cpp_info.generators[generator].createCMakeTarget("OpenSSL::Crypto")
        cryptoCMakeTarget.libs = [crypto_library]
        cryptoCMakeTarget.system_libs = crypto_system_libs
        # inherit other variables ....

        # Add OPENSSL_LIBRARIES: it will be a list of these variables (which can be cmake variables themselves) + is gen_ex
        self.cpp_info.generators[generator].add_variable("OPENSSL_LIBRARIES", [sslLibraryVariable, cryptoLibraryVariable] + system_libs, type=CMakeVariableType.List, genex=True)

        # Add OPENSSL_INCLUDE_DIR variable: is a path + generator expression
        self.cpp_info.generators[generator].add_variable("OPENSSL_INCLUDE_DIR", os.path.join(self.package_folder, "include"), type=CMakeVariableType.Path, genex=True)

The setting of variables might need some dependency tracking (optionally), but I think this interface can serve a lot of use cases.

@conan-center-bot
Copy link
Collaborator

All green! 😊

@SSE4
Copy link
Contributor

SSE4 commented Dec 25, 2019

@madebr why this is needed? as I understand, our openssl package is fully compatible with FindOpenSSL.cmake (and we test that), which is always available (installed with CMake itself). is there a problem?

@conan-center-bot
Copy link
Collaborator

All green in build 5 (165152a136596613a80b8d4e6f92d00d164c55af)! 😊

@SSE4
Copy link
Contributor

SSE4 commented Jan 28, 2020

@madebr friendly ping, pls answer :) also, conflicts

@madebr
Copy link
Contributor Author

madebr commented Jan 29, 2020

@SSE4

  • CMake's FindOpenSSL.cmake has no support such as cmake_multi_package.
  • When packaging projects, they will use OpenSSL::Crypto which cmake_multi_package does not support.
    This is just [question] Mimicking CMake Find Modules #419 and needs support from Conan.
    Otherwise, we will need to endlessly monkey patch CMakeLists.txt scripts.

I will close this pr as support by conan is required.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants