-
Notifications
You must be signed in to change notification settings - Fork 12.9k
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
Use https for curl when building for linux #64786
Conversation
I'm impressed by the curl sites reliability that we haven't yet replaced it with our own aws tarballs... @bors r+ rollup |
📌 Commit 9bd201c has been approved by |
Use https for curl when building for linux I noticed that the dist-x86_64-linux builder uses http to fetch curl and doesn't do any signature verification. It should probably use https. r? @alexcrichton @Mark-Simulacrum
Failed in #64791 (comment), @bors r- |
Very interesting. I now seem to recall something like this. We can probably just mirror it on one our own bucket, I'll look into doing that today. |
@@ -5,7 +5,7 @@ source shared.sh | |||
|
|||
VERSION=7.51.0 | |||
|
|||
curl http://cool.haxx.se/download/curl-$VERSION.tar.bz2 | tar xjf - | |||
curl https://cool.haxx.se/download/curl-$VERSION.tar.bz2 | tar xjf - |
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.
curl https://cool.haxx.se/download/curl-$VERSION.tar.bz2 | tar xjf - | |
curl https://rust-lang-ci-mirrors.s3-us-west-1.amazonaws.com/rustc/curl-7.66.0.tar.xz | tar xJf - |
Should probably update the version to match as well and use that (I assume it's used at some later point in the build).
I don't believe this will work because we need an http client that supports sni, and basically all modern servers require sni, including s3 most likely. I don't believe the pre installed curl on centos supports sni |
Ah, I see. That's a bit unfortunate. So I guess that's probably why the https wasn't working either... I suppose we could get the curl in a ubuntu docker image and then switch to centos, but that seems really complicated. |
That being said, looks like AWS does not require SNI. We download OpenSSL through our s3 bucket, so seems like downloading curl through the bucket is worth a try! |
@bors try |
⌛ Trying commit ff43ab7d5d1d52bc912582e04da065cd093380e4 with merge 18c62604adbfd5cb003feb46a424712a4af6e98c... |
Your PR failed (pretty log, raw log). Through arcane magic we have determined that the following fragments from the build log may contain information about the problem. Click to expand the log.
I'm a bot! I can only do what humans tell me to, so if this was not helpful or you have suggestions for improvements, please ping or otherwise contact |
💔 Test failed - checks-azure |
Unfortunately I can't test this locally (centos 5 and 6 images use outdated vsyscall which is unsupported on my kernel), so I'm flying blind... |
💥 Test timed out |
@bors try |
Use https for curl when building for linux I noticed that the dist-x86_64-linux builder uses http to fetch curl and doesn't do any signature verification. It should probably use https. r? @alexcrichton @Mark-Simulacrum
💥 Test timed out |
@bors: r+ Looks like it worked, just timed out! |
📌 Commit 2185710 has been approved by |
Use https for curl when building for linux I noticed that the dist-x86_64-linux builder uses http to fetch curl and doesn't do any signature verification. It should probably use https. r? @alexcrichton @Mark-Simulacrum
Use https for curl when building for linux I noticed that the dist-x86_64-linux builder uses http to fetch curl and doesn't do any signature verification. It should probably use https. r? @alexcrichton @Mark-Simulacrum
Rollup of 9 pull requests Successful merges: - #64377 (Add long error explanation for E0493) - #64786 (Use https for curl when building for linux) - #64828 (Graphviz debug output for generic dataflow analysis) - #64838 (Add long error explanation for E0550) - #64891 (Fix `vec![x; n]` with null raw fat pointer zeroing the pointer metadata) - #64893 (Zero-initialize `vec![None; n]` for `Option<&T>`, `Option<&mut T>` and `Option<Box<T>>`) - #64911 (Fixed a misleading documentation issue #64844) - #64921 (Add test for issue-64662) - #64923 (Add missing links for mem::needs_drop) Failed merges: - #64918 (Add long error explanation for E0551) r? @ghost
[BETA] Backport changes - update rtpSpawn's parameters type(It's prototype has been updated in libc) rust-lang#64818 - Fix div_duration() marked as stable by mistake rust-lang#64815 - Use https for curl when building for linux rust-lang#64786 - remove rtp.rs, and move rtpSpawn and RTP_ID_ERROR to libc rust-lang#64720 r? @ghost
I noticed that the dist-x86_64-linux builder uses http to fetch curl and doesn't do any signature verification. It should probably use https.
r? @alexcrichton @Mark-Simulacrum