-
Notifications
You must be signed in to change notification settings - Fork 243
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
Statically link to Rustls instead of using OpenSSL #1214
Conversation
dbdf10c
to
372b8a1
Compare
Would this mean that you wouldn't need openssl to install volta? |
@alvarlagerlof Yes, exactly! That's actually one of the main motivations for this suggested change (see the linked RFC): Dealing with all of the different OpenSSL versions—trying to detect the appropriate one, fetch the correct binary, etc.—is a major pain point in our Linux installer. This would simplify all of that by statically linking to a Rust-based TLS/SSL implementation and avoiding all of the headaches that come with dynamically-linked OpenSSL. |
@charlespierce I guess this PR would also allow #1049 to move forward, right? |
@ZauberNerd Yes, I believe this would allow that change to move forward as well, since it would remove the OpenSSL issues that ran into. The shift to Scientific Linux CERN 6 may also help alleviate the toolchain issues. |
@charlespierce that's awesome! Is there something I could do to help get this PR out of the draft state? |
@ZauberNerd I don't think there's anything left on this PR directly (other than resolving the merge conflicts). I'm leaving this in draft for now mostly to make sure that we don't accidentally merge it 😄 There's some infrastructure work we'll need to do before merging to make sure we don't break existing installs while we work towards the actual release of this change. |
372b8a1
to
7bc099b
Compare
7bc099b
to
ec008fe
Compare
@charlespierce what (if anything) can I do to unblock this over this coming week? Would love to help get it landed! |
@chriskrycho I think the only thing remaining is landing this PR (and then some testing when we cut the release). My initial thoughts were being a little wary of version churn with the in-progress pnpm implementation, but I think there's enough significant changes between this and Yarn 3 support, plus there are some follow-ups needed for pnpm, so I'd lean towards getting this merged (along with any smaller outstanding PRs), and then cutting it all as version 1.1. I'll update this to resolve the merge conflicts and also put out a PR to clean up some additional warnings we seem to be getting. |
ec008fe
to
a820cf2
Compare
a820cf2
to
a341789
Compare
That seems reasonable to me! Let's do it! |
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.
Other than the one question, this looks good to me.
itshappening.gif |
Wow this is huge! Thanks to everyone involved who helped to solve those openSSL linking issues with this PR. Is it possible to get an "official" |
That's precisely the plan—this and Yarn 3 support will be the headliner features! (With, hopefully, a 1.2 with pnpm support landing relatively soon after that. 🤞🏼) |
Info
glibc
version.glibc
support that we've maintained) are too old to support buildingring
(a dependency ofrustls
).ring
Changes
attohttpc
to enable the use ofrustls
instead ofnative-tls
(OpenSSL on Linux)Dockerfile
that builds off of Scientific Linux CERN 6, installing updated build toolsrelease
CI workflow to build a single Linux version using the above Docker container.Tested
Notes
linux
build artifact to bevolta-{version}-linux.tar.gz
, without theopenssl-X
suffix. This means we probably will need to roll it out in stages:get.volta.sh
to a commit hash, so that it isn't affected when this is merged. (Done in Pin get.volta.sh installer to current commit hash get#7) (Completed)get.volta.sh
redirect so that it points at the updated script.