search

alexcrichton / openssl-src-rs

Source code and logic to build OpenSSL from source
Apache License 2.0
68 stars 113 forks source link

Use cc for ar/ranlib detection #173

Closed jonhoo closed 1 year ago

jonhoo commented 1 year ago

Note that Command::get_program and Command::get_args both stabilized in Rust 1.57.0, and so implicitly bump this crate's MSRV.

Depends on rust-lang/cc-rs#763.

Replaces #164.

jonhoo commented 1 year ago

Marked this as a draft until the cc-rs PR lands.

jonhoo commented 1 year ago

cargo package fails because it doesn't respect the [patch.crates-io].

jonhoo commented 1 year ago

@alexcrichton The features we needed from cc have now landed, so I've marked the PR as ready for review. The biggest discussion point here I think is that this bumps MSRV for the crate since it uses CommandBuilder::get_args which requires 1.57.0. We could extend cc so that it also provided a mechanism for getting the command and arguments separately, but it feels unnecessary given that we now have the "right" way to do this via std.

jonhoo commented 1 year ago

Do you have a rough sense for when this may land in a release? I have a couple of builds in semi-weird environments that are currently failing and would be fixed by this :)

alexcrichton commented 1 year ago

I believe an OpenSSL security release is happening next week, so next week.

jonhoo commented 1 year ago

Interesting — that makes me wonder if we might want to release this now to get a heads up in case it breaks anyone's builds. I worry about getting into a position where a security fix goes out at the same time as a change that may break people's builds, as it may mean some people end up not being able to take the security fix.

jonhoo commented 1 year ago

Ah, I just realized this only affects the 300 major version. I assume it'd also be okay to backport to 111? I'm happy to do that assuming it's acceptable.

alexcrichton commented 1 year ago

Yes that's fine, I can make a release after a PR to the 111 branch

jonhoo commented 1 year ago

Backport to 111 in https://github.com/alexcrichton/openssl-src-rs/pull/180

Skepfyr commented 1 year ago

Note this does appear to have broken rustup builds: https://github.com/rust-lang/rustup/issues/3263 https://github.com/sfackler/rust-openssl/issues/1839

alexcrichton commented 1 year ago

I don't know how best to fix this myself, unfortunately.

jonhoo commented 1 year ago

As per this comment I don't think this is a bug in openssl or openssl-src — it's really just that cc's logic for detecting ar doesn't (always) work for illumos, and its logic needs updating. Once that happens, openssl-* will also build fine again.