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
Update vendored version to openssl 3 #1925
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -115,6 +115,16 @@ fn main() { | |
println!("cargo:rustc-link-lib={}={}", kind, lib); | ||
} | ||
|
||
// https://github.com/openssl/openssl/pull/15086 | ||
if version == Version::Openssl3xx | ||
&& kind == "static" | ||
&& (env::var("CARGO_CFG_TARGET_OS").unwrap() == "linux" | ||
|| env::var("CARGO_CFG_TARGET_OS").unwrap() == "android") | ||
&& env::var("CARGO_CFG_TARGET_POINTER_WIDTH").unwrap() == "32" | ||
{ | ||
println!("cargo:rustc-link-lib=dylib=atomic"); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. When upgrading to 3.1, vendored tests on arm-unknown-linux-gnueabihf fail with There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Hmm, I guess I'll need to poke around a bit to see if the commit that fixed this was reverted or something. This is also something that may actually be best off moved to openssl-src. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Ah, I think we just need to update the logic in openssl-src to include There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The libs to link don't seem to come from openssl-src (only the library path is used) but are redefined in openssl-sys. We could try to take lib list from openssl-src but it would still require defining them for non-vendored cases so I'm not sure it would make sense. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @sfackler We're new ~ 1 week from 1.1 EOL, is the current PR acceptable or do you want additional changes? As stated in the previous comment it does not seem practical to include the atomic lib on the openssl-src side. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I Know this is merged already, but i wanted to provide my feedback. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The issue about the resulting regression is here: #2043 |
||
} | ||
|
||
if kind == "static" && target.contains("windows") { | ||
println!("cargo:rustc-link-lib=dylib=gdi32"); | ||
println!("cargo:rustc-link-lib=dylib=user32"); | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Legacy feature is required for the tests at least.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It does seem like we realistically probably want to enable this - thoughts @alex?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We enable it by default in pyca/cryptography, its required for basically any encrypted key serialization (since they all use garbage algorithms).