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

Emit rustc-check-cfg for nightly #2235

Merged
merged 1 commit into from
May 5, 2024
Merged

Emit rustc-check-cfg for nightly #2235

merged 1 commit into from
May 5, 2024

Conversation

alex
Copy link
Collaborator

@alex alex commented May 5, 2024

Copy link
Owner

@sfackler sfackler left a comment

Choose a reason for hiding this comment

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

Seems like there's some refactoring to do to make this more maintainable long-term but this looks fine for now.

@@ -104,6 +140,9 @@ fn main() {
if version >= 0x1_01_01_00_0 {
println!("cargo:rustc-cfg=ossl111");
}
if version >= 0x1_01_01_04_0 {
println!("cargo:rustc-cfg=ossl111d");
}
Copy link
Owner

Choose a reason for hiding this comment

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

🙃

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

yuuuup.

@alex
Copy link
Collaborator Author

alex commented May 5, 2024

TBH this feels motivating to find some way to deprecate and remove support for super old OpenSSL's. Most of this list is for stuff that there's no earthly reason to use.

Not sure how to square that with semver + the need to have a single openssl-sys package though.

@alex alex merged commit a7c7bf0 into sfackler:master May 5, 2024
61 checks passed
@alex alex deleted the nightly-cfgs branch May 5, 2024 14:46
@sfackler
Copy link
Owner

sfackler commented May 5, 2024

Yeah. openssl-sys breaks are very rough ecosystem-wise, so I might say that an "if a tree falls in the forest" approach is best there.

In the main openssl crate we can (and should, there are a ton of pent up breaks to make) do a bump. We should definitely drop 1.0.1 support. CentOS 7 EOLs in a month or two and I feel like that's probably a decent time to drop 1.0.2 support as well.

@alex
Copy link
Collaborator Author

alex commented May 5, 2024

In terms of openssl-sys, my best idea is:

  1. Do a major version bump there, but have openssl support both the old and new major version for an extended period of time.
  2. Migrate the rest of the ecosystem to support both
  3. Drop support in openssl for the old -sys after the ecosystem has substantially migrated

End users still need a cutover point, but for most of the ecosystem they should be fine.

@sfackler
Copy link
Owner

sfackler commented May 5, 2024

There are a ton of crates that directly depend on openssl-sys that would need updating unfortunately: https://crates.io/crates/openssl-sys/reverse_dependencies

@alex
Copy link
Collaborator Author

alex commented May 5, 2024 via email

@sfackler
Copy link
Owner

sfackler commented May 5, 2024

This may have been fixed, but at least for a while Cargo's version resolution didn't handle constraints like that very well - it would select the newest version even if that would cause two versions to be included in the crate graph.

Probably worth some experimenting to see how things behave now.

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.

2 participants