-
Notifications
You must be signed in to change notification settings - Fork 892
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
Migrate to using libcurl for now instead of hyper #434
Conversation
I wouldn't merge this just yet as I'm still filling out some details on the Windows side of libcurl, but hopefully those should be addressed shortly. |
@@ -152,161 +154,84 @@ pub fn tee_file<W: io::Write>(path: &Path, mut w: &mut W) -> io::Result<()> { | |||
} | |||
} | |||
|
|||
pub fn download_file<P: AsRef<Path>>(url: hyper::Url, | |||
pub fn download_file<P: AsRef<Path>>(url: &str, |
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.
Can you instead switch to url::Url
instead of str
(it's already used instead of hyper::Url
everywhere els)? I hope to push Urls upward through the APIs to improve type safety.
Thanks @alexcrichton |
Updated to thread |
336d754
to
7e56481
Compare
I think this is good to go but I'd prefer to wait for Windows CI to come along and agree. That.. may take awhile. |
I killed the builds in front of it in appveyor. As soon as the first target is green I'll merge and start the build on master. |
This probably wants to wait at least until the MinGW target gets green, I still have yet to see a green build of curl on appveyor unfortunately. I think I've fixed it locally though. |
@alexcrichton Failed on gnu. |
7981839
to
65e0cbc
Compare
Unfortunately the move to rust-native-tls has had some hiccups on Windows and Hyper also currently doesn't support features like HTTP proxies (but support's coming soon!). For now this temporarily moves over to using libcurl to weed out these problems until we get to a point where we're hyper's features are more filled out. This also somewhat aligns with Cargo as well which is using libcurl currently. This does not use the `curl` crate on crates.io because it doesn't support streaming downloads. Instead I've got an in-progress rewrite which is just binding much more directly to libcurl's APIs (e.g. exposing the callback functions directly). This branch is in a git repo currently and I hope to upstream it to curl-rust soon as well.
Unfortunately the move to rust-native-tls has had some hiccups on Windows and
Hyper also currently doesn't support features like HTTP proxies (but support's
coming soon!). For now this temporarily moves over to using libcurl to weed out
these problems until we get to a point where we're hyper's features are more
filled out. This also somewhat aligns with Cargo as well which is using libcurl
currently.
This does not use the
curl
crate on crates.io because it doesn't supportstreaming downloads. Instead I've got an in-progress rewrite which is just
binding much more directly to libcurl's APIs (e.g. exposing the callback
functions directly). This branch is in a git repo currently and I hope to
upstream it to curl-rust soon as well.