Skip to content

Support builds of OpenSSL from vendored source #684

New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Closed
wants to merge 3 commits into from
Closed

Conversation

sfackler
Copy link
Owner

Closes #580.

@sfackler sfackler force-pushed the vendored-src branch 3 times, most recently from 310439d to 157e8d3 Compare August 22, 2017 04:11
@sfackler
Copy link
Owner Author

cc @alexcrichton

@@ -33,6 +34,8 @@ case "${TARGET}" in
;;
esac

apt-get install -y --no-install-recommends curl
Copy link
Collaborator

Choose a reason for hiding this comment

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

Shouldn't this logic (and most of the logic in this script) end up getting removed in favor of the src crate?

Copy link
Owner Author

Choose a reason for hiding this comment

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

The src crate can only build 1.1.0.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Bah :(

@alexcrichton
Copy link
Collaborator

Some things I'd personally do before merging:

  • Add documentation in the README for the feature
  • Make sure there's a system in place for updating openssl-src -- I'd specifically like to talk to the infra team in Rust about this.

@sfackler
Copy link
Owner Author

One other thing we should figure out is the upgrade story. 1.1.0 EOLs in just over a year: https://www.openssl.org/policies/releasestrat.html. We could simply say that we track the newest release?

@alexcrichton
Copy link
Collaborator

Yes I'd imagine that the crate provides no guarantee really about what version it provides, other than:

  • The version is compatible with the openssl crate
  • The version is supported by the OpenSSL project at the time of publication
  • The most recent version of the crate uses a version of OpenSSL with no known security vulnerabilities.

That is, I'd imagine 1.0.2 and 1.1.0 would both be fine to use today in openssl-src.

@sfackler
Copy link
Owner Author

It is a bit nontrivial due to version-specific features. For example, if we went with 1.0.2, people would presumably want to use set_ecdh_auto, but that's been removed in newer versions. Even for things that aren't removed, you'd need to enable the v111 or v120 or whatever feature to retain access.

We could disable those features when vendored is enabled? Or just not worry about it since it'll only happen once every couple of years.

@alexcrichton
Copy link
Collaborator

Hm that's a good point. I wouldn't be too too worried about this yeah but it seems like enabling vendoring probably shouldn't opt you in to a specific version, you'd have to still select it manually? We could then, if necessary, give a nicer error when you require vendoring and features but there's a version mismatch.

@sfackler
Copy link
Owner Author

Yeah, you'd still have to manually opt-in. It seems fine to just note our upgrade policy in the documentation. If people want to opt into version specific features they'll just need to be careful around upgrade times.

@sfackler sfackler force-pushed the vendored-src branch 2 times, most recently from 1121463 to 1e577a9 Compare August 29, 2017 04:46
@sfackler
Copy link
Owner Author

sfackler commented Sep 8, 2017

One other thing to decide here is how the vendored build is activated. It's currently done via a Cargo feature, but it's not a super great use of that. Only the root crate is going to be the thing that cares, and it normally isn't going to be directly depending on openssl-sys. The other option is to control via an environment variable, but we then have to unconditionally depend on the source package. Seems like not the worst thing though?

@alexcrichton
Copy link
Collaborator

Perhaps we could take a leaf out of rayon's book and require RUSTFLAGS='--cfg openssl_sys_unstable'?

alexcrichton added a commit to alexcrichton/rust-openssl that referenced this pull request Jul 27, 2018
This is a revival of sfackler#684 to see if I can help push it across the finish line!

Closes sfackler#580
@sfackler sfackler closed this Jul 28, 2018
alexcrichton added a commit to alexcrichton/rust-openssl that referenced this pull request Jul 30, 2018
This is a revival of sfackler#684 to see if I can help push it across the finish line!

Closes sfackler#580
@sfackler sfackler deleted the vendored-src branch August 4, 2018 18:03
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants