Skip to content

Commit

Permalink
Simplify supported environments when comparing to lockfile (#7894)
Browse files Browse the repository at this point in the history
## 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 #7876.
  • Loading branch information
charliermarsh authored Oct 3, 2024
1 parent 005bb23 commit 8962bcb
Show file tree
Hide file tree
Showing 3 changed files with 152 additions and 20 deletions.
9 changes: 8 additions & 1 deletion crates/uv-resolver/src/lock/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -524,10 +524,17 @@ impl Lock {
pub fn simplified_supported_environments(&self) -> Vec<MarkerTree> {
self.supported_environments()
.iter()
.map(|marker| self.requires_python.simplify_markers(marker.clone()))
.cloned()
.map(|marker| self.simplify_environment(marker))
.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
/// markers of those forks, otherwise `None`.
pub fn fork_markers(&self) -> &[MarkerTree] {
Expand Down
52 changes: 33 additions & 19 deletions crates/uv/src/commands/project/lock.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Upgrade::None => {}
Upgrade::All => {
Expand All @@ -716,10 +697,43 @@ impl ValidatedLock {
Upgrade::Packages(_) => {
// If the user specified `--upgrade-package`, then at best we can prefer some of
// the existing versions.
debug!("Ignoring existing lockfile due to `--upgrade-package`");
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
// file), don't use the existing lockfile if it references any registries that are no longer
// included in the current configuration.
Expand Down
111 changes: 111 additions & 0 deletions crates/uv/tests/lock.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12881,6 +12881,117 @@ fn lock_python_upper_bound() -> Result<()> {
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]
fn lock_dependency_metadata() -> Result<()> {
let context = TestContext::new("3.12");
Expand Down

0 comments on commit 8962bcb

Please sign in to comment.