Conversation
d260643 to
304ffdf
Compare
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
ac1fa25 to
e868ce8
Compare
e868ce8 to
8aa27b6
Compare
d176fa6 to
ee4f164
Compare
zanieb
commented
Feb 20, 2024
Comment on lines
110
to
118
| // Clone to avoid borrow checker issues | ||
| let immutable_url = url.clone(); | ||
|
|
||
| // If a Git URL ends in a reference (like a branch, tag, or commit), remove it. | ||
| if url.scheme().starts_with("git+") { | ||
| if let Some((prefix, _)) = url.as_str().rsplit_once('@') { | ||
| url = prefix.parse().unwrap(); | ||
| if let Some((prefix, _)) = immutable_url.path().rsplit_once('@') { | ||
| url.set_path(prefix); | ||
| } | ||
| } |
Member
Author
There was a problem hiding this comment.
This is awkward, is there a better pattern?
Member
There was a problem hiding this comment.
I'll play with it, I'm not sure...
Member
There was a problem hiding this comment.
One option:
if url.scheme().starts_with("git+") {
if let Some(prefix) = url.path().rsplit_once('@').map(|(prefix, _suffix)| prefix.to_string()) {
url.set_path(&prefix);
}
}
Member
Author
There was a problem hiding this comment.
Still kind of strange but it seems better.
Member
There was a problem hiding this comment.
Like... mildly better because it doesn't clone the suffix, I guess, and it's contained without another binding.
Member
Author
There was a problem hiding this comment.
In the other case we need to clone the suffix too lol
Member
Author
There was a problem hiding this comment.
The cloned original URL bit is weird though I prefer this
17b1f1a to
03a1e07
Compare
03a1e07 to
2a5bebe
Compare
charliermarsh
approved these changes
Feb 20, 2024
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.
charliermarsh
pushed a commit
that referenced
this pull request
Feb 22, 2024
[Rendered](https://github.com/astral-sh/uv/blob/zb/auth-docs/README.md#git-authentication) Adds docs for - #1781 - #1717
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
2 participants
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.
Fixes handling of GitHub PATs in HTTPS URLs, which were otherwise dropped. We now supporting the following authentication schemes:
On Windows, the username is required. We can consider adding a special-case for this in the future, but this just matches libgit2's behavior.
I tested with fine-grained tokens, OAuth tokens, and "classic" tokens. There's test coverage for fine-grained tokens in CI where we use a real private repository and PAT. Yes, the PAT is committed to make this test usable by anyone. It has read-only permissions to the single repository, expires Feb 1 2025, and is in an isolated organization and GitHub account.
Does not yet address SSH authentication (see #1781)
Related: