Use libgit2 first then fallback to the git command on failure#1786
Closed
Use libgit2 first then fallback to the git command on failure#1786
libgit2 first then fallback to the git command on failure#1786Conversation
zanieb
commented
Feb 20, 2024
zanieb
commented
Feb 20, 2024
17e924d to
c3710ac
Compare
afafb66 to
188fc0a
Compare
c3710ac to
e28245e
Compare
zanieb
commented
Feb 21, 2024
Member
Author
|
Currently a big problem here is that libgit2 will retry several times before we try git. |
0e99e40 to
5bb19d5
Compare
zanieb
added a commit
that referenced
this pull request
Feb 21, 2024
…st SSH support (#1781) Closes #1775 Closes #1452 Closes #1514 Follows #1717 libgit2 does not support host names with extra identifiers during SSH lookup (e.g. [`github.com-some_identifier`]( https://docs.github.com/en/authentication/connecting-to-github-with-ssh/managing-deploy-keys#using-multiple-repositories-on-one-server)) so we use the `git` command instead for fetching. This is required for `pip` parity. See the [Cargo documentation](https://doc.rust-lang.org/nightly/cargo/reference/config.html#netgit-fetch-with-cli) for more details on using the `git` CLI instead of libgit2. We may want to try to use libgit2 first in the future, as it is more performant (#1786). We now support authentication with: ``` git+ssh://git@<hostname>/... git+ssh://git@<hostname>-<identifier>/... ``` Tested with a deploy key e.g. ``` cargo run -- \ pip install uv-private-pypackage@git+ssh://git@github.com-test-uv-private-pypackage/astral-test/uv-private-pypackage.git \ --reinstall --no-cache -v ``` and ``` cargo run -- \ pip install uv-private-pypackage@git+ssh://git@github.com/astral-test/uv-private-pypackage.git \ --reinstall --no-cache -v ``` with a ssh config like ``` Host github.com Hostname github.com IdentityFile=/Users/mz/.ssh/id_ed25519 Host github.com-test-uv-private-pypackage Hostname github.com IdentityFile=/Users/mz/.ssh/id_ed25519 ``` It seems quite hard to add test coverage for this to the test suite, as we'd need to add the SSH key and I don't know how to isolate that from affecting other developer's machines.
This avoids redoing the fast-path and such and only actually retries the fetch itself # Conflicts: # crates/uv-git/src/git.rs # Conflicts: # crates/uv-git/src/git.rs
# Conflicts: # crates/uv/tests/pip_install.rs
acc4ff3 to
34f8999
Compare
Member
Author
|
To be explored another day.. |
This file contains hidden or 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
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.
Some necessary context in #1781.
This should be faster in the happy path but robust to less common authentication schemes. Cargo uses an environment variable to toggle instead of this "fallback" approach. I'm hesitant to require that from our users.
The downside of this is that we will always retry with the
gitcommand even if the failure is not related to authentication. This could result in confusing logs and is, of course, slower. In practice, I think the error messages from thegitcommand are better as they give you something reproducible to test with and have more context, thelibgit2errors are very opaque.I've also added a rough way to reuse the discovered correct strategy for fetching (df4382f) we could extend this to avoid unnecessary calls to
libgit2in the future.We can definitely say this is overkill and just drop fetching with libgit2 entirely in favor of the git CLI. We could also only use the
gitcommand if we detect authentication via some heuristic?