Skip to content

Commit

Permalink
Preserve fragments when applying verbatim redirects (#4038)
Browse files Browse the repository at this point in the history
## Summary

`echo
"git+https://github.com/pypa/sample-namespace-packages.git#subdirectory=pkg_resources/pkg_a"
| cargo run pip compile -` now resolves to a compliant URL.

Closes #4037.
  • Loading branch information
charliermarsh authored Jun 5, 2024
1 parent 27e5bfe commit 6d148da
Show file tree
Hide file tree
Showing 2 changed files with 30 additions and 3 deletions.
31 changes: 29 additions & 2 deletions crates/uv-resolver/src/redirect.rs
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,9 @@ fn apply_redirect(url: &VerbatimUrl, redirect: Url) -> VerbatimUrl {
// The redirect should be the "same" URL, but with a specific commit hash added after the `@`.
// We take advantage of this to preserve as much of the verbatim representation as possible.
if let Some(given) = url.given() {
let (given, fragment) = given
.split_once('#')
.map_or((given, None), |(prefix, suffix)| (prefix, Some(suffix)));
if let Some(precise_suffix) = redirect
.raw()
.path()
Expand All @@ -56,13 +59,25 @@ fn apply_redirect(url: &VerbatimUrl, redirect: Url) -> VerbatimUrl {
// And the portion after the `@` is stable between the parsed and given representations...
if given_suffix == parsed_suffix {
// Preserve everything that precedes the `@` in the precise representation.
return redirect.with_given(format!("{given_prefix}@{precise_suffix}"));
let given = format!("{given_prefix}@{precise_suffix}");
let given = if let Some(fragment) = fragment {
format!("{given}#{fragment}")
} else {
given
};
return redirect.with_given(given);
}
}
} else {
// If there was no `@` in the original representation, we can just append the
// precise suffix to the given representation.
return redirect.with_given(format!("{given}@{precise_suffix}"));
let given = format!("{given}@{precise_suffix}");
let given = if let Some(fragment) = fragment {
format!("{given}#{fragment}")
} else {
given
};
return redirect.with_given(given);
}
}
}
Expand Down Expand Up @@ -118,6 +133,18 @@ mod tests {
)?;
assert_eq!(apply_redirect(&verbatim, redirect), expected);

// We should preserve subdirectory fragments.
let verbatim = VerbatimUrl::parse_url("https://github.com/flask.git#subdirectory=src")?
.with_given("git+https://github.com/flask.git#subdirectory=src");
let redirect =
Url::parse("https://github.com/flask.git@b90a4f1f4a370e92054b9cc9db0efcb864f87ebe#subdirectory=src")?;

let expected = VerbatimUrl::parse_url(
"https://github.com/flask.git@b90a4f1f4a370e92054b9cc9db0efcb864f87ebe#subdirectory=src",
)?.with_given("git+https://github.com/flask.git@b90a4f1f4a370e92054b9cc9db0efcb864f87ebe#subdirectory=src");

assert_eq!(apply_redirect(&verbatim, redirect), expected);

Ok(())
}
}
2 changes: 1 addition & 1 deletion crates/uv/tests/pip_compile.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1565,7 +1565,7 @@ fn compile_git_subdirectory_static_metadata() -> Result<()> {
----- stdout -----
# This file was autogenerated by uv via the following command:
# uv pip compile --cache-dir [CACHE_DIR] --exclude-newer 2024-03-25T00:00:00Z requirements.in
uv-public-pypackage @ git+https://github.com/astral-test/uv-workspace-pypackage#subdirectory=uv-public-pypackage@b8c4e192456d736c27f2c84c61175c896dba8373
uv-public-pypackage @ git+https://github.com/astral-test/uv-workspace-pypackage@b8c4e192456d736c27f2c84c61175c896dba8373#subdirectory=uv-public-pypackage
# via -r requirements.in
----- stderr -----
Expand Down

0 comments on commit 6d148da

Please sign in to comment.