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 3.x: fixes to support Conan 2.0 #16658

Merged
merged 18 commits into from
Mar 23, 2023

Conversation

jcar87
Copy link
Contributor

@jcar87 jcar87 commented Mar 22, 2023

Specify library name and version: openssl/3.x

Summary

  • Changes to support Conan 2.0
  • Clean-up dead / unused code
  • Raise invalid configuration when trying to build shared libraries for iOS (not supported by the underlying build system)
  • Add version 3.1.0

Details

  • Use rm_safe instead of deletions, to avoid errors in Conan 2.0 with double deletions

  • Use newer built-in self.win_bash instead of custom recipe attribute

  • Modernize conditional to require msys2/cci.latest - this PR syncs the logic with what we have for OpenSSL 1.x

  • Move the logic to generate the custom target file to the generate() method, instantiating an AutotoolsToolchain() to retrieve information like compiler/linker flags

  • Add a Conan layout (modern practice)

  • Stop passing cc/cxx/ld/ranlib - having tested this targetting Android and iOS, the underlying build system honours those environment variables which AutotoolsToolchain (and therefore, the conanbuild environment) may have defined - so there is no need to query these to pass them. This is reported as such in the build summary that we are printing. Have also tested using the new tool configuration to define compiler, as well as having CC etc defined in [buildenv] in the profile. It always seems to be picking up exactly what was intended, so this seems superfluous.

  • Stop passing generator-provided include directories - the logic to retrieve this from the cpp_info of the zlib dependency, via --with-zlib flags, was already doing the right thing and was unnecessary to do it twice.

  • Use new self.dependencies accessor instead of deps_cpp_info

  • Use --prefix=/ when calling configure, and DESTDIR= (with self.package_folder) when calling install from the package() method. This requires patching the generated Makefile on Windows to escape a backslash. This is needed because the final install location in the conan cache is not known at configure time in the build() method.

  • Get path to strawberryperl on WIndows from the user.strawberryperl:perl conf, the same way it is currently done in the OpenSSL 1.x recipe - this may be a tad unnecessary, it should already find perl in the path in the build environment.

  • Simplify _make_program (similar to OpenSSL 1.x recipe) - we may consider getting this from a conf

  • Ensure the OPENSSL_CRYPTO_LIBRARY variables (and similar), which come from the legacy OpenSSL module, are defined - note that it is strongly discouraged to use this - users should rely on the modern CMake targets (this comes from [openssl 3.x.x] fix "official" OpenSSL vars for CMakeDeps gen #14426 - added PR author as co-author of this one)

  • Add call to fix_apple_shared_install_name() so that install names of shared libraries are prepended with @rpath/

  • Add recently released version 3.1.0

  • No longer a need to query xcrun to derive the value of the -isysroot flag when building for Apple platforms, this is handled by AutotoolsToolchain and passed as part of the other flags.

  • Test package: unify in a single executable, to work around ability to call self.dependencies() in the test() method (does not work in Conan 1.x).

  • Test package: remove references to zlib, the OpenSSL headers and API do not impose a transitive requirement on zlib headers (or at least, it's not needed by what is actually tested).

  • Test v1 package is kept exactly as it was before this PR.

  • Several removals:

    • export_sources() for patches, as there are currently no patches (and they were not being applied anyway)
    • _patch_makefile_org() and _patch_configure() - they were not being called, thus it was dead code, presumably leftover from an earlier version of OpenSSL.
    • _tool() - see comment above regarding environment variables for cc/cxx/ld/ranlib
    • _cc() method - the OpenSSL build scripts honour the CC variable just fine (see comments above)

@conan-center-bot

This comment has been minimized.

@github-actions
Copy link
Contributor

🤖 Beep Boop! This pull request is making changes to 'recipes/openssl//'.

👋 @Hopobcn @Croydon you might be interested. 😉

@ghost
Copy link

ghost commented Mar 22, 2023

I detected other pull requests that are modifying openssl/3.x.x recipe:

This message is automatically generated by https://github.com/ericLemanissier/conan-center-conflicting-prs so don't hesitate to report issues/improvements there.

@conan-center-bot

This comment has been minimized.

@conan-center-bot

This comment has been minimized.

@jcar87 jcar87 marked this pull request as ready for review March 23, 2023 12:41
Copy link
Member

@uilianries uilianries left a comment

Choose a reason for hiding this comment

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

Tons of simplification and a well described PR, life is good.

if(DEFINED ${_custom_var})
message(STATUS "${_custom_var}: ${${_custom_var}}")
else()
message(FATAL_ERROR "${_custom_var} not defined")
Copy link
Member

Choose a reason for hiding this comment

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

nice way to validate it!

@conan-center-bot
Copy link
Collaborator

Conan v1 pipeline ✔️

All green in build 5 (684931a5d5bff02877a6e0385462dd1cedd80e98):

  • openssl/3.0.8@:
    All packages built successfully! (All logs)

  • openssl/3.1.0@:
    All packages built successfully! (All logs)

  • openssl/3.0.7@:
    All packages built successfully! (All logs)

  • openssl/3.0.5@:
    All packages built successfully! (All logs)


Conan v2 pipeline (informative, not required for merge) ✔️

Note: Conan v2 builds are informative and they are not required for the PR to be merged.

All green in build 5 (684931a5d5bff02877a6e0385462dd1cedd80e98):

  • openssl/3.0.8@:
    All packages built successfully! (All logs)

  • openssl/3.0.5@:
    All packages built successfully! (All logs)

  • openssl/3.1.0@:
    All packages built successfully! (All logs)

  • openssl/3.0.7@:
    All packages built successfully! (All logs)

@conan-center-bot conan-center-bot merged commit a21b276 into conan-io:master Mar 23, 2023
@jcar87 jcar87 deleted the lcc/maintenance/openssl3c2 branch March 23, 2023 20:44
0xFireWolf pushed a commit to 0xFireWolf/conan-center-index that referenced this pull request Apr 2, 2023
* openssl3: add a layout

* openssl3: remove obsolete recipe methods

* openssl3: use rm_safe instead of deletions

* openssl3: remove exporting non-existant patches

* openssl: generate target file during generate() method, wip

* openssl3: more changes

* openssl3: windows fixes

* openssl3: add CMake variables to match CMake's find module

Co-authored-by: Jan Niklas Grieb <jan.grieb@rohde-schwarz.com>

* openssl3: test package, WIP

* openssl3: windows fixes

* openssl3: fix apple install names and add new version

* openssl3: remove unused function

* openssl3: remove unused method

* openssl3: remove dead code

* openssl3: fixes

* fix typo

* fix indentation

* openssl3: unify test package in a single executable

---------

Co-authored-by: Jan Niklas Grieb <jan.grieb@rohde-schwarz.com>
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