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 git2_ureq transport #625

Closed
wants to merge 16 commits into from
Closed

Add git2_ureq transport #625

wants to merge 16 commits into from

Conversation

jsha
Copy link

@jsha jsha commented Sep 25, 2020

This is an alternate approach to #624 that uses an actual HTTP client (ureq) on top of rustls. In the end this came out significantly cleaner, and by using default_features = false I was able to keep the dependencies very minimal.

Note that this branch is built on top of the branch for #624 for convenience. If we decide to go this route I'll excise the git2_rustls files.

@jsha
Copy link
Author

jsha commented Sep 25, 2020

Also note this actually won't compile with ureq 1.4.1. I needed to make some local changes to add Send + Sync to the return value of into_reader, which I'll upstream now.

@alexcrichton
Copy link
Member

Nice! This seems pretty reasonable to me to add! I haven't looked at git2-curl in a very long time though so some things could probably be improved there, but this seems like a great start at least!

@jsha
Copy link
Author

jsha commented Sep 29, 2020

Updated to refer to a released version of ureq with the needed Send marker. I also refactored significantly to better reflect my understanding of how things work. Definitely interested in feedback on code structure and style.

Also, call it version 1.0.0.
@jsha
Copy link
Author

jsha commented Oct 7, 2020

I'm actually going to close this for now. When I went deeper on integrating this with cargo, I realized the other fetches in cargo (the crate downloads) rely on HTTP/2 for a significant speedup. Since ureq doesn't currently support HTTP/2, it wouldn't be suitable as a general replacement for curl in cargo. I'm going to take a look at whether it's feasible to implement a lightweight subset of HTTP/2 in ureq for concurrent / pipelined fetches. But I may wind up coming back around to this with a different HTTP client.

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.

2 participants