-
Notifications
You must be signed in to change notification settings - Fork 1.3k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Error on lockfiles with incoherent wheel versions #12235
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -2871,6 +2871,19 @@ impl PackageWire { | |
requires_python: &RequiresPython, | ||
unambiguous_package_ids: &FxHashMap<PackageName, PackageId>, | ||
) -> Result<Package, LockError> { | ||
// Consistency check | ||
if let Some(version) = &self.id.version { | ||
for wheel in &self.wheels { | ||
if version != &wheel.filename.version { | ||
return Err(LockError::from(LockErrorKind::InconsistentVersions { | ||
name: self.id.name, | ||
})); | ||
} | ||
} | ||
// We can't check the source dist version since it does not need to contain the version | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We should always have at least one wheel enabling us to catch the inconsistency? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Not necessarily, for example: dependencies = [
"sniffio",
]
[tool.uv.sources]
sniffio = { path = "sniffio.tar.gz" }
[[package]]
name = "sniffio"
version = "1.3.1"
source = { path = "sniffio.tar.gz" }
sdist = { hash = "sha256:f4324edc670a0f49750a81b895f35c3adb843cca46f0530f79fc1babb23789dc" } In this case, it's not possible to determine from the lockfile alone whether there is an inconsistency. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I've added another commit that catches all mismatches between locked source dist version and the built wheel versions at build time. This would enforce stricter guarantees than we require right now, CC @charliermarsh There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. As an example: [[package]]
name = "sniffio"
version = "2.3.4"
source = { url = "https://files.pythonhosted.org/packages/a2/87/a6771e1546d97e7e041b6ae58d80074f81b7d5121207425c964ddf5cfdbd/sniffio-1.3.1.tar.gz" }
sdist = { hash = "sha256:f4324edc670a0f49750a81b895f35c3adb843cca46f0530f79fc1babb23789dc" } With this last commit fails with:
The potential clash could be with git dependencies that use a version-from-git integration. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Moved this to a separate PR: #12237 |
||
// in the filename. | ||
} | ||
|
||
let unwire_deps = |deps: Vec<DependencyWire>| -> Result<Vec<Dependency>, LockError> { | ||
deps.into_iter() | ||
.map(|dep| dep.unwire(requires_python, unambiguous_package_ids)) | ||
|
@@ -5156,6 +5169,13 @@ enum LockErrorKind { | |
#[source] | ||
err: uv_distribution::Error, | ||
}, | ||
/// A package has inconsistent versions in a single entry | ||
// Using name instead of id since the version in the id is part of the conflict. | ||
#[error("Locked package and file versions are inconsistent for `{name}`", name = name.cyan())] | ||
InconsistentVersions { | ||
/// The name of the package with the inconsistent entry. | ||
name: PackageName, | ||
}, | ||
#[error( | ||
"Found conflicting extras `{package1}[{extra1}]` \ | ||
and `{package2}[{extra2}]` enabled simultaneously" | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -8473,3 +8473,97 @@ fn prune_cache_url_subdirectory() -> Result<()> { | |
|
||
Ok(()) | ||
} | ||
|
||
/// Test that incoherence in the versions in a package entry of the lockfile versions is caught. | ||
/// | ||
/// See <https://github.com/astral-sh/uv/issues/12164> | ||
#[test] | ||
fn locked_version_coherence() -> 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 = ["iniconfig"] | ||
"#, | ||
)?; | ||
|
||
uv_snapshot!(context.filters(), context.lock(), @r" | ||
success: true | ||
exit_code: 0 | ||
----- stdout ----- | ||
|
||
----- stderr ----- | ||
Resolved 2 packages in [TIME] | ||
"); | ||
|
||
let lock = context.read("uv.lock"); | ||
|
||
insta::with_settings!({ | ||
filters => context.filters(), | ||
}, { | ||
assert_snapshot!( | ||
lock, @r#" | ||
version = 1 | ||
revision = 1 | ||
requires-python = ">=3.12" | ||
|
||
[options] | ||
exclude-newer = "2024-03-25T00:00:00Z" | ||
|
||
[[package]] | ||
name = "iniconfig" | ||
version = "2.0.0" | ||
source = { registry = "https://pypi.org/simple" } | ||
sdist = { url = "https://files.pythonhosted.org/packages/d7/4b/cbd8e699e64a6f16ca3a8220661b5f83792b3017d0f79807cb8708d33913/iniconfig-2.0.0.tar.gz", hash = "sha256:2d91e135bf72d31a410b17c16da610a82cb55f6b0477d1a902134b24a455b8b3", size = 4646 } | ||
wheels = [ | ||
{ url = "https://files.pythonhosted.org/packages/ef/a6/62565a6e1cf69e10f5727360368e451d4b7f58beeac6173dc9db836a5b46/iniconfig-2.0.0-py3-none-any.whl", hash = "sha256:b6a85871a79d2e3b22d2d1b94ac2824226a63c6b741c88f7ae975f18b6778374", size = 5892 }, | ||
] | ||
|
||
[[package]] | ||
name = "project" | ||
version = "0.1.0" | ||
source = { virtual = "." } | ||
dependencies = [ | ||
{ name = "iniconfig" }, | ||
] | ||
|
||
[package.metadata] | ||
requires-dist = [{ name = "iniconfig" }] | ||
"#); | ||
}); | ||
|
||
// Write an inconsistent iniconfig entry | ||
context | ||
.temp_dir | ||
.child("uv.lock") | ||
.write_str(&lock.replace(r#"version = "2.0.0""#, r#"version = "1.0.0""#))?; | ||
|
||
// An inconsistent lockfile should fail with `--locked` | ||
uv_snapshot!(context.filters(), context.sync().arg("--locked"), @r" | ||
success: false | ||
exit_code: 2 | ||
----- stdout ----- | ||
|
||
----- stderr ----- | ||
error: Failed to parse `uv.lock` | ||
Caused by: Locked package and file versions are inconsistent for `iniconfig` | ||
"); | ||
|
||
// Without `--locked`, we could fail or recreate the lockfile, currently, we fail. | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Would a user find this surprising? I think I'd assume uv will create a correct lockfile when running I guess it partially depends on what "If a lockfile is present, its contents will be used as preferences for the resolution" from the docs for There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Currently, we fail on all kinds of parsing errors with lockfile. For example, an empty
As does a lockfile where I removed the source line:
We can try to be better about recovering from a broken lockfile, but that's outside of the scope of the PR (uv would never write such a lockfile, and with this change, uv will refuse to sync a broken dependabot PR, failing CI). For comparison, in cases where the lockfile is not invalid but doesn't fully match in a non-version anymore, e.g., the environments changed, we for example only reuse the versions as preference but not the full lockfile ( |
||
uv_snapshot!(context.filters(), context.lock(), @r" | ||
success: false | ||
exit_code: 2 | ||
----- stdout ----- | ||
|
||
----- stderr ----- | ||
error: Failed to parse `uv.lock` | ||
Caused by: Locked package and file versions are inconsistent for `iniconfig` | ||
"); | ||
|
||
Ok(()) | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we show the versions in the error here? Like "packaged version {} does not match {} from wheel {}"?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
#12249