mirrors/uv
1
0
Fork 0
mirror of https://github.com/astral-sh/uv.git synced 2025-08-03 02:22:19 +00:00
Code Issues Projects Releases Packages Wiki Activity Actions 2

Avoid panic when re-locking with precise commit (#5863)

Browse source 
## Summary

Very subtle bug. The scenario is as follows:

- We resolve: `elmer-circuitbuilder = { git =
"https://github.com/ElmerCSC/elmer_circuitbuilder.git" }`

- The user then changes the request to: `elmer-circuitbuilder = { git =
"https://github.com/ElmerCSC/elmer_circuitbuilder.git", rev =
"44d2f4b19d6837ea990c16f494bdf7543d57483d" }`

- When we go to re-lock, we note two facts:

1. The "default branch" resolves to
`44d2f4b19d6837ea990c16f494bdf7543d57483d`.
2. The metadata for `44d2f4b19d6837ea990c16f494bdf7543d57483d` is
(whatever we grab from the lockfile).

- In the resolver, we then ask for the metadata for
`44d2f4b19d6837ea990c16f494bdf7543d57483d`. It's already in the cache,
so we return it; thus, we never add the
`44d2f4b19d6837ea990c16f494bdf7543d57483d` ->
`44d2f4b19d6837ea990c16f494bdf7543d57483d` mapping to the Git resolver,
because we never have to resolve it.

This would apply for any case in which a requested tag or branch was
replaced by its precise SHA. Replacing with a different commit is fine.

It only applied to `tool.uv.sources`, and not PEP 508 URLs, because the
underlying issue is that we aren't consistent about "automatically"
extracting the precise commit from a Git reference.

Closes https://github.com/astral-sh/uv/issues/5860.
This commit is contained in:
Charlie Marsh 2024-08-07 10:56:15 -04:00 • committed by GitHub
parent 3ae75a21aa
commit e4ec6e4025
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
9 changed files with 445 additions and 43 deletions
Download patch file Download diff file Expand all files Collapse all files

9
crates/pypi-types/src/parsed_url.rs
View file

@ -246,10 +246,11 @@ impl ParsedGitUrl {
precise: Option<GitSha>,
subdirectory: Option<PathBuf>,
) -> Self {
let mut url = GitUrl::new(repository, reference);
if let Some(precise) = precise {
url = url.with_precise(precise);
}
let url = if let Some(precise) = precise {
GitUrl::from_commit(repository, reference, precise)
} else {
GitUrl::from_reference(repository, reference)
};
Self { url, subdirectory }
}
}

4
crates/pypi-types/src/requirement.rs
View file

@ -113,9 +113,9 @@ impl From<Requirement> for pep508_rs::Requirement<VerbatimParsedUrl> {
url,
} => {
let git_url = if let Some(precise) = precise {
GitUrl::new(repository, reference).with_precise(precise)
GitUrl::from_commit(repository, reference, precise)
} else {
GitUrl::new(repository, reference)
GitUrl::from_reference(repository, reference)
};
Some(VersionOrUrl::Url(VerbatimParsedUrl {
parsed_url: ParsedUrl::Git(ParsedGitUrl {

15
crates/uv-git/src/git.rs
View file

@ -3,8 +3,10 @@
//! Source: <https://github.com/rust-lang/cargo/blob/23eb492cf920ce051abfc56bbaf838514dc8365c/src/cargo/sources/git/utils.rs>
use std::fmt::Display;
use std::path::{Path, PathBuf};
use std::str::{self};
use std::str::{self, FromStr};
use crate::sha::GitOid;
use crate::GitSha;
use anyhow::{anyhow, Context, Result};
use cargo_util::{paths, ProcessBuilder};
use reqwest::StatusCode;
@ -13,8 +15,6 @@ use tracing::debug;
use url::Url;
use uv_fs::Simplified;
use crate::sha::GitOid;
/// A file indicates that if present, `git reset` has been done and a repo
/// checkout is ready to go. See [`GitCheckout::reset`] for why we need this.
const CHECKOUT_READY_LOCK: &str = ".ok";
@ -108,6 +108,15 @@ impl GitReference {
Self::DefaultBranch => "default branch",
}
}
/// Returns the precise [`GitSha`] of this reference, if it's a full commit.
pub(crate) fn as_sha(&self) -> Option<GitSha> {
if let Self::FullCommit(rev) = self {
Some(GitSha::from_str(rev).expect("Full commit should be exactly 40 characters"))
} else {
None
}
}
}
impl Display for GitReference {

28
crates/uv-git/src/lib.rs
View file

@ -1,4 +1,3 @@
use std::str::FromStr;
use url::Url;
pub use crate::git::GitReference;
@ -26,11 +25,22 @@ pub struct GitUrl {
}
impl GitUrl {
pub fn new(repository: Url, reference: GitReference) -> Self {
/// Create a new [`GitUrl`] from a repository URL and a reference.
pub fn from_reference(repository: Url, reference: GitReference) -> Self {
let precise = reference.as_sha();
Self {
repository,
reference,
precise: None,
precise,
}
}
/// Create a new [`GitUrl`] from a repository URL and a precise commit.
pub fn from_commit(repository: Url, reference: GitReference, precise: GitSha) -> Self {
Self {
repository,
reference,
precise: Some(precise),
}
}
@ -77,17 +87,7 @@ impl TryFrom<Url> for GitUrl {
url.set_path(&prefix);
}
let precise = if let GitReference::FullCommit(rev) = &reference {
Some(GitSha::from_str(rev)?)
} else {
None
};
Ok(Self {
repository: url,
reference,
precise,
})
Ok(Self::from_reference(url, reference))
}
}

8
crates/uv-git/src/resolver.rs
View file

@ -71,7 +71,7 @@ impl GitResolver {
Ok(fetch)
}
/// Given a remote source distribution, return a precise variant, if possible.
/// Given a remote source distribution, return a precise variant.
///
/// For example, given a Git dependency with a reference to a branch or tag, return a URL
/// with a precise reference to the current commit of that branch or tag.
@ -79,6 +79,9 @@ impl GitResolver {
/// This method takes into account various normalizations that are independent from the Git
/// layer. For example: removing `#subdirectory=pkg_dir`-like fragments, and removing `git+`
/// prefix kinds.
///
/// Returns `Ok(None)` if the URL already has a precise reference (i.e., it includes a full
/// commit hash in the URL itself, as opposed to, e.g., a branch name).
pub async fn resolve(
&self,
url: &GitUrl,
@ -121,7 +124,8 @@ impl GitResolver {
/// prefix kinds.
///
/// This method will only return precise URLs for URLs that have already been resolved via
/// [`resolve_precise`].
/// [`resolve_precise`], and will return `None` for URLs that have not been resolved _or_
/// already have a precise reference.
pub fn precise(&self, url: GitUrl) -> Option<GitUrl> {
let reference = RepositoryReference::from(&url);
let precise = self.get(&reference)?;

9
crates/uv-installer/src/plan.rs
View file

@ -241,10 +241,11 @@ impl<'a> Planner<'a> {
subdirectory,
url,
} => {
let mut git = GitUrl::new(repository.clone(), reference.clone());
if let Some(precise) = precise {
git = git.with_precise(*precise);
}
let git = if let Some(precise) = precise {
GitUrl::from_commit(repository.clone(), reference.clone(), *precise)
} else {
GitUrl::from_reference(repository.clone(), reference.clone())
};
let sdist = GitSourceDist {
name: requirement.name.clone(),
git: Box::new(git),

9
crates/uv-requirements/src/lookahead.rs
View file

@ -261,10 +261,11 @@ fn required_dist(requirement: &Requirement) -> Result<Option<Dist>, distribution
subdirectory,
url,
} => {
let mut git_url = GitUrl::new(repository.clone(), reference.clone());
if let Some(precise) = precise {
git_url = git_url.with_precise(*precise);
}
let git_url = if let Some(precise) = precise {
GitUrl::from_commit(repository.clone(), reference.clone(), *precise)
} else {
GitUrl::from_reference(repository.clone(), reference.clone())
};
Dist::Source(SourceDist::Git(GitSourceDist {
name: requirement.name.clone(),
git: Box::new(git_url),

20
crates/uv-resolver/src/lock.rs
View file

@ -863,9 +863,11 @@ impl Distribution {
}
Source::Git(url, git) => {
// Reconstruct the `GitUrl` from the `GitSource`.
let git_url =
uv_git::GitUrl::new(url.to_url(), GitReference::from(git.kind.clone()))
.with_precise(git.precise);
let git_url = uv_git::GitUrl::from_commit(
url.to_url(),
GitReference::from(git.kind.clone()),
git.precise,
);
// Reconstruct the PEP 508-compatible URL from the `GitSource`.
let url = Url::from(ParsedGitUrl {
@ -1488,7 +1490,9 @@ impl Source {
UrlString::from(locked_git_url(git_dist)),
GitSource {
kind: GitSourceKind::from(git_dist.git.reference().clone()),
precise: git_dist.git.precise().expect("precise commit"),
precise: git_dist.git.precise().unwrap_or_else(|| {
panic!("Git distribution is missing a precise hash: {git_dist}")
}),
subdirectory: git_dist
.subdirectory
.as_deref()
@ -2205,9 +2209,11 @@ impl Dependency {
index: None,
},
Source::Git(repository, git) => {
let git_url =
uv_git::GitUrl::new(repository.to_url(), GitReference::from(git.kind.clone()))
.with_precise(git.precise);
let git_url = uv_git::GitUrl::from_commit(
repository.to_url(),
GitReference::from(git.kind.clone()),
git.precise,
);
let parsed_url = ParsedUrl::Git(ParsedGitUrl {
url: git_url.clone(),

402
crates/uv/tests/lock.rs
View file

@ -202,7 +202,7 @@ fn lock_sdist_registry() -> Result<()> {
Ok(())
}
/// Lock a Git requirement.
/// Lock a Git requirement using `tool.uv.sources`.
#[test]
fn lock_sdist_git() -> Result<()> {
let context = TestContext::new("3.12");
@ -214,11 +214,252 @@ fn lock_sdist_git() -> Result<()> {
name = "project"
version = "0.1.0"
requires-python = ">=3.12"
dependencies = ["uv-public-pypackage @ git+https://github.com/astral-test/uv-public-pypackage@0.0.1"]
dependencies = ["uv-public-pypackage"]
[tool.uv.sources]
uv-public-pypackage = { git = "https://github.com/astral-test/uv-public-pypackage", tag = "0.0.1" }
"#,
)?;
deterministic! { context =>
uv_snapshot!(context.filters(), context.lock(), @r###"
success: true
exit_code: 0
----- stdout -----
----- stderr -----
warning: `uv lock` is experimental and may change without warning
warning: `uv.sources` is experimental and may change without warning
Resolved 2 packages in [TIME]
"###);
let lock = fs_err::read_to_string(context.temp_dir.join("uv.lock")).unwrap();
insta::with_settings!({
filters => context.filters(),
}, {
assert_snapshot!(
lock, @r###"
version = 1
requires-python = ">=3.12"
exclude-newer = "2024-03-25 00:00:00 UTC"
[[distribution]]
name = "project"
version = "0.1.0"
source = { editable = "." }
dependencies = [
{ name = "uv-public-pypackage" },
]
[[distribution]]
name = "uv-public-pypackage"
version = "0.1.0"
source = { git = "https://github.com/astral-test/uv-public-pypackage?tag=0.0.1#0dacfd662c64cb4ceb16e6cf65a157a8b715b979" }
"###
);
});
}
// Install from the lockfile.
uv_snapshot!(context.filters(), context.sync().arg("--frozen"), @r###"
success: true
exit_code: 0
----- stdout -----
----- stderr -----
warning: `uv sync` is experimental and may change without warning
Prepared 2 packages in [TIME]
Installed 2 packages in [TIME]
+ project==0.1.0 (from file://[TEMP_DIR]/)
+ uv-public-pypackage==0.1.0 (from git+https://github.com/astral-test/uv-public-pypackage@0dacfd662c64cb4ceb16e6cf65a157a8b715b979)
"###);
// Re-lock with a precise commit that maps to the same tag.
let pyproject_toml = context.temp_dir.child("pyproject.toml");
pyproject_toml.write_str(
r#"
[project]
name = "project"
version = "0.1.0"
requires-python = ">=3.12"
dependencies = ["uv-public-pypackage"]
[tool.uv.sources]
uv-public-pypackage = { git = "https://github.com/astral-test/uv-public-pypackage", rev = "0dacfd662c64cb4ceb16e6cf65a157a8b715b979" }
"#,
)?;
deterministic! { context =>
uv_snapshot!(context.filters(), context.lock(), @r###"
success: true
exit_code: 0
----- stdout -----
----- stderr -----
warning: `uv lock` is experimental and may change without warning
warning: `uv.sources` is experimental and may change without warning
Resolved 2 packages in [TIME]
"###);
let lock = fs_err::read_to_string(context.temp_dir.join("uv.lock")).unwrap();
insta::with_settings!({
filters => context.filters(),
}, {
assert_snapshot!(
lock, @r###"
version = 1
requires-python = ">=3.12"
exclude-newer = "2024-03-25 00:00:00 UTC"
[[distribution]]
name = "project"
version = "0.1.0"
source = { editable = "." }
dependencies = [
{ name = "uv-public-pypackage" },
]
[[distribution]]
name = "uv-public-pypackage"
version = "0.1.0"
source = { git = "https://github.com/astral-test/uv-public-pypackage?rev=0dacfd662c64cb4ceb16e6cf65a157a8b715b979#0dacfd662c64cb4ceb16e6cf65a157a8b715b979" }
"###
);
});
}
// Re-lock with a different commit.
let pyproject_toml = context.temp_dir.child("pyproject.toml");
pyproject_toml.write_str(
r#"
[project]
name = "project"
version = "0.1.0"
requires-python = ">=3.12"
dependencies = ["uv-public-pypackage"]
[tool.uv.sources]
uv-public-pypackage = { git = "https://github.com/astral-test/uv-public-pypackage", rev = "b270df1a2fb5d012294e9aaf05e7e0bab1e6a389" }
"#,
)?;
deterministic! { context =>
uv_snapshot!(context.filters(), context.lock(), @r###"
success: true
exit_code: 0
----- stdout -----
----- stderr -----
warning: `uv lock` is experimental and may change without warning
warning: `uv.sources` is experimental and may change without warning
Resolved 2 packages in [TIME]
"###);
let lock = fs_err::read_to_string(context.temp_dir.join("uv.lock")).unwrap();
insta::with_settings!({
filters => context.filters(),
}, {
assert_snapshot!(
lock, @r###"
version = 1
requires-python = ">=3.12"
exclude-newer = "2024-03-25 00:00:00 UTC"
[[distribution]]
name = "project"
version = "0.1.0"
source = { editable = "." }
dependencies = [
{ name = "uv-public-pypackage" },
]
[[distribution]]
name = "uv-public-pypackage"
version = "0.1.0"
source = { git = "https://github.com/astral-test/uv-public-pypackage?rev=b270df1a2fb5d012294e9aaf05e7e0bab1e6a389#b270df1a2fb5d012294e9aaf05e7e0bab1e6a389" }
"###
);
});
}
// Re-lock with a different tag (which matches the new commit).
let pyproject_toml = context.temp_dir.child("pyproject.toml");
pyproject_toml.write_str(
r#"
[project]
name = "project"
version = "0.1.0"
requires-python = ">=3.12"
dependencies = ["uv-public-pypackage"]
[tool.uv.sources]
uv-public-pypackage = { git = "https://github.com/astral-test/uv-public-pypackage", tag = "0.0.2" }
"#,
)?;
deterministic! { context =>
uv_snapshot!(context.filters(), context.lock(), @r###"
success: true
exit_code: 0
----- stdout -----
----- stderr -----
warning: `uv lock` is experimental and may change without warning
warning: `uv.sources` is experimental and may change without warning
Resolved 2 packages in [TIME]
"###);
let lock = fs_err::read_to_string(context.temp_dir.join("uv.lock")).unwrap();
insta::with_settings!({
filters => context.filters(),
}, {
assert_snapshot!(
lock, @r###"
version = 1
requires-python = ">=3.12"
exclude-newer = "2024-03-25 00:00:00 UTC"
[[distribution]]
name = "project"
version = "0.1.0"
source = { editable = "." }
dependencies = [
{ name = "uv-public-pypackage" },
]
[[distribution]]
name = "uv-public-pypackage"
version = "0.1.0"
source = { git = "https://github.com/astral-test/uv-public-pypackage?tag=0.0.2#b270df1a2fb5d012294e9aaf05e7e0bab1e6a389" }
"###
);
});
}
Ok(())
}
/// Lock a Git requirement using PEP 508.
#[test]
fn lock_sdist_git_pep508() -> Result<()> {
let context = TestContext::new("3.12");
let pyproject_toml = context.temp_dir.child("pyproject.toml");
pyproject_toml.write_str(
r#"
[project]
name = "project"
version = "0.1.0"
requires-python = ">=3.12"
dependencies = ["uv-public-pypackage @ git+https://github.com/astral-test/uv-public-pypackage.git@0.0.1"]
"#,
)?;
// deterministic! { context =>
uv_snapshot!(context.filters(), context.lock(), @r###"
success: true
exit_code: 0
@ -251,26 +492,165 @@ fn lock_sdist_git() -> Result<()> {
[[distribution]]
name = "uv-public-pypackage"
version = "0.1.0"
source = { git = "https://github.com/astral-test/uv-public-pypackage?rev=0.0.1#0dacfd662c64cb4ceb16e6cf65a157a8b715b979" }
source = { git = "https://github.com/astral-test/uv-public-pypackage.git?rev=0.0.1#0dacfd662c64cb4ceb16e6cf65a157a8b715b979" }
"###
);
});
}
// }
// Install from the lockfile.
uv_snapshot!(context.filters(), context.sync().arg("--frozen"), @r###"
// Re-lock with a precise commit that maps to the same tag.
let pyproject_toml = context.temp_dir.child("pyproject.toml");
pyproject_toml.write_str(
r#"
[project]
name = "project"
version = "0.1.0"
requires-python = ">=3.12"
dependencies = ["uv-public-pypackage @ git+https://github.com/astral-test/uv-public-pypackage.git@0dacfd662c64cb4ceb16e6cf65a157a8b715b979"]
"#,
)?;
// deterministic! { context =>
uv_snapshot!(context.filters(), context.lock(), @r###"
success: true
exit_code: 0
----- stdout -----
----- stderr -----
warning: `uv sync` is experimental and may change without warning
Prepared 2 packages in [TIME]
Installed 2 packages in [TIME]
+ project==0.1.0 (from file://[TEMP_DIR]/)
+ uv-public-pypackage==0.1.0 (from git+https://github.com/astral-test/uv-public-pypackage@0dacfd662c64cb4ceb16e6cf65a157a8b715b979)
warning: `uv lock` is experimental and may change without warning
Resolved 2 packages in [TIME]
"###);
let lock = fs_err::read_to_string(context.temp_dir.join("uv.lock")).unwrap();
insta::with_settings!({
filters => context.filters(),
}, {
assert_snapshot!(
lock, @r###"
version = 1
requires-python = ">=3.12"
exclude-newer = "2024-03-25 00:00:00 UTC"
[[distribution]]
name = "project"
version = "0.1.0"
source = { editable = "." }
dependencies = [
{ name = "uv-public-pypackage" },
]
[[distribution]]
name = "uv-public-pypackage"
version = "0.1.0"
source = { git = "https://github.com/astral-test/uv-public-pypackage.git?rev=0dacfd662c64cb4ceb16e6cf65a157a8b715b979#0dacfd662c64cb4ceb16e6cf65a157a8b715b979" }
"###
);
});
// }
// Re-lock with a different commit.
let pyproject_toml = context.temp_dir.child("pyproject.toml");
pyproject_toml.write_str(
r#"
[project]
name = "project"
version = "0.1.0"
requires-python = ">=3.12"
dependencies = ["uv-public-pypackage @ git+https://github.com/astral-test/uv-public-pypackage.git@b270df1a2fb5d012294e9aaf05e7e0bab1e6a389"]
"#,
)?;
// deterministic! { context =>
uv_snapshot!(context.filters(), context.lock(), @r###"
success: true
exit_code: 0
----- stdout -----
----- stderr -----
warning: `uv lock` is experimental and may change without warning
Resolved 2 packages in [TIME]
"###);
let lock = fs_err::read_to_string(context.temp_dir.join("uv.lock")).unwrap();
insta::with_settings!({
filters => context.filters(),
}, {
assert_snapshot!(
lock, @r###"
version = 1
requires-python = ">=3.12"
exclude-newer = "2024-03-25 00:00:00 UTC"
[[distribution]]
name = "project"
version = "0.1.0"
source = { editable = "." }
dependencies = [
{ name = "uv-public-pypackage" },
]
[[distribution]]
name = "uv-public-pypackage"
version = "0.1.0"
source = { git = "https://github.com/astral-test/uv-public-pypackage.git?rev=b270df1a2fb5d012294e9aaf05e7e0bab1e6a389#b270df1a2fb5d012294e9aaf05e7e0bab1e6a389" }
"###
);
});
// }
// Re-lock with a different tag (which matches the new commit).
let pyproject_toml = context.temp_dir.child("pyproject.toml");
pyproject_toml.write_str(
r#"
[project]
name = "project"
version = "0.1.0"
requires-python = ">=3.12"
dependencies = ["uv-public-pypackage @ git+https://github.com/astral-test/uv-public-pypackage.git@0.0.2"]
"#,
)?;
// deterministic! { context =>
uv_snapshot!(context.filters(), context.lock(), @r###"
success: true
exit_code: 0
----- stdout -----
----- stderr -----
warning: `uv lock` is experimental and may change without warning
Resolved 2 packages in [TIME]
"###);
let lock = fs_err::read_to_string(context.temp_dir.join("uv.lock")).unwrap();
insta::with_settings!({
filters => context.filters(),
}, {
assert_snapshot!(
lock, @r###"
version = 1
requires-python = ">=3.12"
exclude-newer = "2024-03-25 00:00:00 UTC"
[[distribution]]
name = "project"
version = "0.1.0"
source = { editable = "." }
dependencies = [
{ name = "uv-public-pypackage" },
]
[[distribution]]
name = "uv-public-pypackage"
version = "0.1.0"
source = { git = "https://github.com/astral-test/uv-public-pypackage.git?rev=0.0.2#b270df1a2fb5d012294e9aaf05e7e0bab1e6a389" }
"###
);
});
// }
Ok(())
}