Preserve Git username for SSH dependencies #6335
Merged
Conversation
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
Member
Author
|
Adding tests... |
charliermarsh
force-pushed
the
charlie/git-at
branch
from
c7d0bc4 to
594e865
Compare
zanieb
reviewed
Aug 21, 2024
crates/pypi-types/src/requirement.rs
Outdated
Comment on lines
766
to
773
| if url.username() == "git" && url.password().is_none() { | ||
| return; | ||
| } | ||
| let _ = url.set_password(None); | ||
| let _ = url.set_username(""); |
Member
There was a problem hiding this comment.
This seems weird, shouldn't we always drop the password and conditionally drop the username? Or is this just for SSH credentials? A comment would be helpful if so.
Member
Author
There was a problem hiding this comment.
The password is required to be None in the return case though.
Member
There was a problem hiding this comment.
Yes.. I expect:
let _ = url.set_password(None);
if url.username() != "git" {
let _ = url.set_username("");
}
or for the documentation to properly declare that we only allow git if it doesn't have an associated password. Otherwise.. git:[email protected] would redact both git and foo.
Member
Author
There was a problem hiding this comment.
Added a comment + ssh guard.
charliermarsh
force-pushed
the
charlie/git-at
branch
2 times, most recently
from
47db04c to
6f684fb
Compare
zanieb
reviewed
Aug 21, 2024
charliermarsh
force-pushed
the
charlie/git-at
branch
from
6f684fb to
1e03fc7
Compare
charliermarsh
force-pushed
the
charlie/git-at
branch
from
1e03fc7 to
def3084
Compare
zanieb
approved these changes
Aug 21, 2024
tmeijn
pushed a commit
to tmeijn/dotfiles
that referenced
this pull request
Aug 22, 2024
This MR contains the following updates: | Package | Update | Change | |---|---|---| | [astral-sh/uv](https://github.com/astral-sh/uv) | patch | `0.3.0` -> `0.3.1` | MR created with the help of [el-capitano/tools/renovate-bot](https://gitlab.com/el-capitano/tools/renovate-bot). **Proposed changes to behavior should be submitted there as MRs.** --- ### Release Notes <details> <summary>astral-sh/uv (astral-sh/uv)</summary> ### [`v0.3.1`](https://github.com/astral-sh/uv/blob/HEAD/CHANGELOG.md#031) [Compare Source](astral-sh/uv@0.3.0...0.3.1) ##### Enhancements - Add `--with-editable` support to `uv run` ([#​6262](astral-sh/uv#6262)) - Respect `.python-version` files and `pyproject.toml` in `uv python find` ([#​6369](astral-sh/uv#6369)) - Allow manylinux compatibility override via `_manylinux` module ([#​6039](astral-sh/uv#6039)) ##### CLI - Avoid treating `uv add -r` as `--raw-sources` ([#​6287](astral-sh/uv#6287)) ##### Bug fixes - Always invoke found interpreter when `uv run python` is used ([#​6363](astral-sh/uv#6363)) - Avoid adding extra newline for script with non-empty prelude ([#​6366](astral-sh/uv#6366)) - Fix metadata cache instability for lockfile ([#​6332](astral-sh/uv#6332)) - Handle Ctrl-C properly in `uvx` invocations ([#​6346](astral-sh/uv#6346)) - Ignore workspace discovery errors with `--no-workspace` ([#​6328](astral-sh/uv#6328)) - Invalidate `uv.lock` when virtual `dev-dependencies` change ([#​6291](astral-sh/uv#6291)) - Make cache robust to removed archives ([#​6284](astral-sh/uv#6284)) - Preserve Git username for SSH dependencies ([#​6335](astral-sh/uv#6335)) - Respect `--no-build-isolation` in `uv add` ([#​6368](astral-sh/uv#6368)) - Respect `.python-version` files in `uv run` outside projects ([#​6361](astral-sh/uv#6361)) - Use `sys_executable` for `uv run` invocations ([#​6354](astral-sh/uv#6354)) - Use atomic write for `pip compile` output ([#​6274](astral-sh/uv#6274)) - Use consistent logic for deserializing short revisions ([#​6341](astral-sh/uv#6341)) ##### Documentation - Remove the preview default value of `python-preference` ([#​6301](astral-sh/uv#6301)) - Update env vars doc about `XDG_*` variables on macOS ([#​6337](astral-sh/uv#6337)) </details> --- ### Configuration 📅 **Schedule**: Branch creation - At any time (no schedule defined), Automerge - At any time (no schedule defined). 🚦 **Automerge**: Disabled by config. Please merge this manually once you are satisfied. ♻ **Rebasing**: Whenever MR becomes conflicted, or you tick the rebase/retry checkbox. 🔕 **Ignore**: Close this MR and you won't be reminded about this update again. --- - [ ] <!-- rebase-check -->If you want to rebase/retry this MR, check this box --- This MR has been generated by [Renovate Bot](https://github.com/renovatebot/renovate). <!--renovate-debug:eyJjcmVhdGVkSW5WZXIiOiIzNy40NDAuNyIsInVwZGF0ZWRJblZlciI6IjM3LjQ0MC43IiwidGFyZ2V0QnJhbmNoIjoibWFpbiIsImxhYmVscyI6WyJSZW5vdmF0ZSBCb3QiXX0=-->
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.
Summary
We're gonna work on a more comprehensive review of whether we should preserve the username here, but for now,
git@is effectively a convention for GitHub and GitLab etc.Closes #6305.
Test Plan
I guess we don't have infrastructure for testing SSH private keys right now, but...