From 8962bcb0282ff01cbafad249a8167a2f5918f794 Mon Sep 17 00:00:00 2001 From: Charlie Marsh Date: Thu, 3 Oct 2024 14:15:07 +0100 Subject: [PATCH] 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. --- crates/uv-resolver/src/lock/mod.rs | 9 +- crates/uv/src/commands/project/lock.rs | 52 +++++++----- crates/uv/tests/lock.rs | 111 +++++++++++++++++++++++++ 3 files changed, 152 insertions(+), 20 deletions(-) diff --git a/crates/uv-resolver/src/lock/mod.rs b/crates/uv-resolver/src/lock/mod.rs index f21e01016101..ce1be4cd0def 100644 --- a/crates/uv-resolver/src/lock/mod.rs +++ b/crates/uv-resolver/src/lock/mod.rs @@ -524,10 +524,17 @@ impl Lock { pub fn simplified_supported_environments(&self) -> Vec { 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] { diff --git a/crates/uv/src/commands/project/lock.rs b/crates/uv/src/commands/project/lock.rs index e6920d6b3bac..1e4dcc480fe7 100644 --- a/crates/uv/src/commands/project/lock.rs +++ b/crates/uv/src/commands/project/lock.rs @@ -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 => { @@ -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::>(); + 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. diff --git a/crates/uv/tests/lock.rs b/crates/uv/tests/lock.rs index a110b0f0635f..fe5090feb35f 100644 --- a/crates/uv/tests/lock.rs +++ b/crates/uv/tests/lock.rs @@ -12881,6 +12881,117 @@ fn lock_python_upper_bound() -> Result<()> { Ok(()) } +/// See: +#[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");