Simplify supported environments when comparing to lockfile (#7894)

## Summary

If a supported environment includes a Python marker, we don't simplify
it out, despite _storing_ the simplified markers. This PR modifies the
validation code to compare simplified to simplified markers.

Closes https://github.com/astral-sh/uv/issues/7876.
This commit is contained in:
Charlie Marsh 2024-10-03 14:15:07 +01:00 committed by GitHub
parent 005bb235f0
commit 8962bcb028
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
3 changed files with 152 additions and 20 deletions

View File

@ -524,10 +524,17 @@ impl Lock {
pub fn simplified_supported_environments(&self) -> Vec<MarkerTree> { pub fn simplified_supported_environments(&self) -> Vec<MarkerTree> {
self.supported_environments() self.supported_environments()
.iter() .iter()
.map(|marker| self.requires_python.simplify_markers(marker.clone())) .cloned()
.map(|marker| self.simplify_environment(marker))
.collect() .collect()
} }
/// Simplify the given marker environment with respect to the lockfile's
/// `requires-python` setting.
pub fn simplify_environment(&self, marker: MarkerTree) -> MarkerTree {
self.requires_python.simplify_markers(marker)
}
/// If this lockfile was built from a forking resolution with non-identical forks, return the /// If this lockfile was built from a forking resolution with non-identical forks, return the
/// markers of those forks, otherwise `None`. /// markers of those forks, otherwise `None`.
pub fn fork_markers(&self) -> &[MarkerTree] { pub fn fork_markers(&self) -> &[MarkerTree] {

View File

@ -687,25 +687,6 @@ impl ValidatedLock {
} }
} }
// If the set of supported environments has changed, we have to perform a clean resolution.
if lock.simplified_supported_environments()
!= environments
.map(SupportedEnvironments::as_markers)
.unwrap_or_default()
{
return Ok(Self::Versions(lock));
}
// If the Requires-Python bound has changed, we have to perform a clean resolution, since
// the set of `resolution-markers` may no longer cover the entire supported Python range.
if lock.requires_python().range() != requires_python.range() {
return if lock.fork_markers().is_empty() {
Ok(Self::Preferable(lock))
} else {
Ok(Self::Versions(lock))
};
}
match upgrade { match upgrade {
Upgrade::None => {} Upgrade::None => {}
Upgrade::All => { Upgrade::All => {
@ -716,10 +697,43 @@ impl ValidatedLock {
Upgrade::Packages(_) => { Upgrade::Packages(_) => {
// If the user specified `--upgrade-package`, then at best we can prefer some of // If the user specified `--upgrade-package`, then at best we can prefer some of
// the existing versions. // the existing versions.
debug!("Ignoring existing lockfile due to `--upgrade-package`");
return Ok(Self::Preferable(lock)); return Ok(Self::Preferable(lock));
} }
} }
// If the Requires-Python bound has changed, we have to perform a clean resolution, since
// the set of `resolution-markers` may no longer cover the entire supported Python range.
if lock.requires_python().range() != requires_python.range() {
debug!(
"Ignoring existing lockfile due to change in Python requirement: `{}` vs. `{}`",
lock.requires_python(),
requires_python,
);
return if lock.fork_markers().is_empty() {
Ok(Self::Preferable(lock))
} else {
Ok(Self::Versions(lock))
};
}
// If the set of supported environments has changed, we have to perform a clean resolution.
let expected = lock.simplified_supported_environments();
let actual = environments
.map(SupportedEnvironments::as_markers)
.unwrap_or_default()
.iter()
.cloned()
.map(|marker| lock.simplify_environment(marker))
.collect::<Vec<_>>();
if expected != actual {
debug!(
"Ignoring existing lockfile due to change in supported environments: `{:?}` vs. `{:?}`",
expected, actual
);
return Ok(Self::Versions(lock));
}
// If the user provided at least one index URL (from the command line, or from a configuration // If the user provided at least one index URL (from the command line, or from a configuration
// file), don't use the existing lockfile if it references any registries that are no longer // file), don't use the existing lockfile if it references any registries that are no longer
// included in the current configuration. // included in the current configuration.

View File

@ -12881,6 +12881,117 @@ fn lock_python_upper_bound() -> Result<()> {
Ok(()) Ok(())
} }
/// See: <https://github.com/astral-sh/uv/issues/7876>
#[test]
fn lock_simplified_environments() -> Result<()> {
let context = TestContext::new("3.11");
let pyproject_toml = context.temp_dir.child("pyproject.toml");
pyproject_toml.write_str(
r#"
[project]
name = "project"
version = "0.1.0"
requires-python = ">=3.11,<3.12"
dependencies = ["iniconfig"]
[tool.uv]
environments = [
"sys_platform == 'darwin' and python_version >= '3.11' and python_version < '3.12'",
"sys_platform != 'darwin' and python_version >= '3.11' and python_version < '3.12'",
]
"#,
)?;
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
requires-python = "==3.11.*"
resolution-markers = [
"sys_platform == 'darwin'",
"sys_platform != 'darwin'",
]
supported-markers = [
"sys_platform == 'darwin'",
"sys_platform != 'darwin'",
]
[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" }]
"###
);
});
// Re-run with `--locked`.
uv_snapshot!(context.filters(), context.lock().arg("--locked"), @r###"
success: true
exit_code: 0
----- stdout -----
----- stderr -----
Resolved 2 packages in [TIME]
"###);
// Re-run with `--offline`. We shouldn't need a network connection to validate an
// already-correct lockfile with immutable metadata.
uv_snapshot!(context.filters(), context.lock().arg("--locked").arg("--offline").arg("--no-cache"), @r###"
success: true
exit_code: 0
----- stdout -----
----- stderr -----
Resolved 2 packages in [TIME]
"###);
// Install from the lockfile.
uv_snapshot!(context.filters(), context.sync().arg("--frozen"), @r###"
success: true
exit_code: 0
----- stdout -----
----- stderr -----
Prepared 1 package in [TIME]
Installed 1 package in [TIME]
+ iniconfig==2.0.0
"###);
Ok(())
}
#[test] #[test]
fn lock_dependency_metadata() -> Result<()> { fn lock_dependency_metadata() -> Result<()> {
let context = TestContext::new("3.12"); let context = TestContext::new("3.12");