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.

Sign up for GitHub

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

Jump to bottom

cmake/3.x.x: bump OpenSSL #20438

Merged
merged 3 commits into from
Jun 12, 2024
Merged

cmake/3.x.x: bump OpenSSL #20438

Changes from 2 commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
e9d5fd8
cmake/3.x.x: bump OpenSSL
mayeut Oct 8, 2023
850a672
with_openssl option is only used on Linux
mayeut Oct 8, 2023
88250c2
cmake/3.x.x: use version range for OpenSSL
jcar87 Jun 12, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 4 additions & 4 deletions recipes/cmake/3.x.x/conanfile.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,12 +33,12 @@ class CMakeConan(ConanFile):
}

def config_options(self):
if self.settings.os == "Windows":
self.options.with_openssl = False
if self.settings.os != "Linux":
del self.options.with_openssl

def requirements(self):
if self.options.with_openssl:
self.requires("openssl/1.1.1t")
if self.options.get_safe("with_openssl", default=False):
self.requires("openssl/1.1.1w")
Copy link
Contributor

Choose a reason for hiding this comment

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

Why don't you use version range like all other recipes?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

because versions of cmake go far back and the support for 3.x is not clear. Given those are not being built by cci, I'd rather go the safe way at first. It can still be made a version range later with the correct pivot.

Copy link
Contributor

Choose a reason for hiding this comment

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

Could you test if it pass with openssl 3.x? It would avoid all these PR bumping versions of dependencies.

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 can't test this on macOS. The build does not use OpenSSL.

Copy link
Contributor

Choose a reason for hiding this comment

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

So why openssl is dragged irrespective of os? 🤔

Copy link
Contributor

Choose a reason for hiding this comment

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

Do a test build in CI with self.requires("openssl/[>=1.1 <4]") and update the recipe accordingly, perhaps?

Copy link
Contributor

Choose a reason for hiding this comment

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

Do a test build in CI with self.requires("openssl/[>=1.1 <4]") and update the recipe accordingly, perhaps?

Alternatively, just use the version range for the newest CMake versions and leave it to 1.1 for older.


def validate_build(self):
if self.settings.os == "Windows" and self.options.bootstrap:
Expand Down