-
Notifications
You must be signed in to change notification settings - Fork 891
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
Add reqwest download backend, remove hyper backends #1222
Conversation
Ah, doesn't work with file URLs. One moment. |
Fallout from updates: |
deprecrated/net2-rs#64 |
☔ The latest upstream changes (presumably #1229) made this pull request unmergeable. Please resolve the merge conflicts. |
Rebased. |
9203b91
to
4878ed1
Compare
Found a less-hacky way to fix the backtrace-sys build: just use env variables. Now to wait for travis. |
Hoping the rest of the build works* , but we're going to hit kornelski/rust-security-framework#43 now. * confused why it's not running, the opensource linux backlog is zero right now. |
6695d0f
to
769ff15
Compare
Same issue with mio as with net2. |
I think that was a spurious failure. |
0d72edc
to
4aa724d
Compare
☔ The latest upstream changes (presumably #1230) made this pull request unmergeable. Please resolve the merge conflicts. |
Thanks for this, and sorry for the churn! If you'd like to rebase one last time I think we should be ready to go in terms of review and looking this over. |
993a765
to
9efdf10
Compare
No worries, @alexcrichton! Going to try without the objcopy workaround first, since the build image changes may have fixed that issue. |
src/download/src/lib.rs
Outdated
#[cfg(not(feature = "rustls-backend"))] | ||
pub mod rustls { | ||
#[cfg(not(feature = "reqwest-backend"))] | ||
pub mod reqwest { |
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.
Should this be reqwest_be
or should the above be reqwest
?
src/rustup-utils/src/utils.rs
Outdated
@@ -221,12 221,9 @@ fn download_file_(url: &Url, | |||
}; | |||
|
|||
// Download the file | |||
let use_hyper_backend = env::var_os("RUSTUP_USE_HYPER").is_some(); |
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.
Mind leaving this around for a bit as an alias for using reqwest?
@bors: r Looks great, thanks! |
📌 Commit 9c1ae01 has been approved by |
☔ The latest upstream changes (presumably #1207) made this pull request unmergeable. Please resolve the merge conflicts. |
|
@bors: r |
📌 Commit 2ead23b has been approved by |
Add reqwest download backend, remove hyper backends Fixes #1183 Closes #1181 (hyper backend will no longer exist) About the lockfile changes: I had to update native_tls from 1.0.0 to 1.0.4 for reqwest. `cargo update -p native-tls` complained about not having a valid version of security-framework-sys. `cargo update -p security-framework-sys` complained about not having a valid version of native-tls. So I updated everything 🤷♂️ .
☀️ Test successful - status-appveyor, status-travis |
Use Reqwest backend for Appveyor, not Hyper which is deprecated Since rust-lang/rustup#1222 Appveyor builds have been complaining, so should stop asking for the Hyper backend
Fixes #1183
Closes #1181 (hyper backend will no longer exist)
About the lockfile changes:
I had to update native_tls from 1.0.0 to 1.0.4 for reqwest.
cargo update -p native-tls
complained about not having a valid version of security-framework-sys.cargo update -p security-framework-sys
complained about not having a valid version of native-tls.So I updated everything 🤷♂️ .