Skip to content
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

Merged
merged 2 commits into from
Aug 13, 2017

Conversation

mattico
Copy link
Contributor

@mattico mattico commented Aug 3, 2017

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 🤷‍♂️ .

@mattico
Copy link
Contributor Author

mattico commented Aug 3, 2017

Ah, doesn't work with file URLs. One moment.

@mattico
Copy link
Contributor Author

mattico commented Aug 4, 2017

Fallout from updates:
MIPS/PPC: backtrace build failed (need to install powerpc-unknown-linux-binutils ?)
Android: net2 build failed (deprecrated/net2-rs#63)

@mattico
Copy link
Contributor Author

mattico commented Aug 5, 2017

deprecrated/net2-rs#64
x86_64-linux-android now fails with the same error as the rest. I think the issue is with the docker image (https://github.com/rust-lang/rust-buildbot/tree/master/slaves/linux-cross). The configure script is picking up unprefixed objcopy rather than using the cross one.

@mattico mattico changed the title Add reqwest download backend, remove hyper backends [WIP] Add reqwest download backend, remove hyper backends Aug 9, 2017
@bors
Copy link
Contributor

bors commented Aug 11, 2017

☔ The latest upstream changes (presumably #1229) made this pull request unmergeable. Please resolve the merge conflicts.

@mattico
Copy link
Contributor Author

mattico commented Aug 11, 2017

Rebased.

@mattico mattico force-pushed the use-reqwest branch 2 times, most recently from 9203b91 to 4878ed1 Compare August 11, 2017 19:12
@mattico mattico changed the title [WIP] Add reqwest download backend, remove hyper backends Add reqwest download backend, remove hyper backends Aug 11, 2017
@mattico
Copy link
Contributor Author

mattico commented Aug 11, 2017

Found a less-hacky way to fix the backtrace-sys build: just use env variables. Now to wait for travis.

@mattico
Copy link
Contributor Author

mattico commented Aug 12, 2017

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.

@mattico mattico force-pushed the use-reqwest branch 2 times, most recently from 6695d0f to 769ff15 Compare August 12, 2017 01:24
@mattico
Copy link
Contributor Author

mattico commented Aug 12, 2017

Same issue with mio as with net2.

@mattico
Copy link
Contributor Author

mattico commented Aug 12, 2017

I think that was a spurious failure.

@mattico mattico force-pushed the use-reqwest branch 5 times, most recently from 0d72edc to 4aa724d Compare August 12, 2017 17:22
@bors
Copy link
Contributor

bors commented Aug 12, 2017

☔ The latest upstream changes (presumably #1230) made this pull request unmergeable. Please resolve the merge conflicts.

@alexcrichton
Copy link
Member

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.

@mattico mattico force-pushed the use-reqwest branch 2 times, most recently from 993a765 to 9efdf10 Compare August 12, 2017 18:41
@mattico
Copy link
Contributor Author

mattico commented Aug 12, 2017

No worries, @alexcrichton!

Going to try without the objcopy workaround first, since the build image changes may have fixed that issue.

#[cfg(not(feature = "rustls-backend"))]
pub mod rustls {
#[cfg(not(feature = "reqwest-backend"))]
pub mod reqwest {
Copy link
Member

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?

@@ -221,12 +221,9 @@ fn download_file_(url: &Url,
};

// Download the file
let use_hyper_backend = env::var_os("RUSTUP_USE_HYPER").is_some();
Copy link
Member

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?

@alexcrichton
Copy link
Member

@bors: r+

Looks great, thanks!

@bors
Copy link
Contributor

bors commented Aug 12, 2017

📌 Commit 9c1ae01 has been approved by alexcrichton

@bors
Copy link
Contributor

bors commented Aug 13, 2017

☔ The latest upstream changes (presumably #1207) made this pull request unmergeable. Please resolve the merge conflicts.

@mattico
Copy link
Contributor Author

mattico commented Aug 13, 2017

Cargo.lock is fun... rebased.

@alexcrichton
Copy link
Member

@bors: r+

@bors
Copy link
Contributor

bors commented Aug 13, 2017

📌 Commit 2ead23b has been approved by alexcrichton

@bors
Copy link
Contributor

bors commented Aug 13, 2017

⌛ Testing commit 2ead23b with merge d2c30db...

bors added a commit that referenced this pull request Aug 13, 2017
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 🤷‍♂️  .
@bors
Copy link
Contributor

bors commented Aug 13, 2017

☀️ Test successful - status-appveyor, status-travis
Approved by: alexcrichton
Pushing d2c30db to master...

@bors bors merged commit 2ead23b into rust-lang:master Aug 13, 2017
@mattico mattico deleted the use-reqwest branch August 13, 2017 04:46
bors added a commit to rust-lang/libc that referenced this pull request Nov 20, 2018
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
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants