Update download dependencies #1523
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Contributor
pickfire
commented
Oct 8, 2018
•
pickfire
commented
- reqwest 0.9, fix Can't compile: Unable to detect OpenSSL version #1518
- hyper 0.12
- reqwest 0.9, fix rust-lang#1518 - hyper 0.12
meithecatte
reviewed
Oct 28, 2018
| callback(Event::DownloadContentLengthReceived(len.0 + resume_from))?; | ||
| if let Some(len) = res.headers().get(header::CONTENT_LENGTH) { | ||
| // TODO possible issues during unwrap? | ||
| let len = len.to_str().unwrap().parse::<u64>().unwrap() + resume_from; |
There was a problem hiding this comment.
What is the purpose of converting the header to a string and back?
There was a problem hiding this comment.
len is &reqwest::header::HeaderValue (not u64) there IIRC so I need to convert it to str and then parse it as u64.
Even though HeaderValue have HttpTryFrom but TryFrom is nigthly so not usable.
I removing the extra to_str but HeaderValue does not implement parse.
Maybe I might have missed something?
|
tl;dr: ignore this. Just hitting rust-lang/rust#55376 Changing just this line in file src/download/Cargo.toml (with a checked out rustup.rs (latest) commit being at 3abd097) -reqwest = { version = "0.8.4", optional = true }
+reqwest = { version = "0.9", optional = true }causes an ICE Ah, it's already reported: rust-lang/rust#55376 |
|
I've just tested this PR and it fixes #1518 for me. some unrelated warnings though: |
|
@xftroxgpx Yup, I am already planning to do those after this commit is merge. Still waiting for the pull request to be merged. And maybe reformat the project and reorganize some tests as well. |
|
Can we get this merged? Without it, rustup doesn't build on modern systems. |
|
Thanks for the PR, sorry for the delay in merging. Please note though that Rustup reviewers do not have much available bandwidth right not, so if anyone is keen on implementing more, please check that someone will be able to review it. (More info at https://internals.rust-lang.org/t/cargo-and-rustup-development) |
|
@nrc I understand that you guys don't have a lot of time right now but could you make a release with this fix? It's quite important because more and more distros will start offering OpenSSL 1.1 by default and then it just won't build. |
I'm planning to make a Rustup release in a couple of weeks time. I hope that is soon enough (because we don't have release trains, rustup releases are a fair bit of work and are somewhat risky). |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.