From efc8d94e65253ff2cb004555368ebe71b9a7ae73 Mon Sep 17 00:00:00 2001 From: Charlie Marsh Date: Fri, 21 Feb 2025 18:51:04 -1000 Subject: [PATCH] Treat lockfile as outdated if (empty) extras are added (#11702) ## Summary Now that we track extras in the lockfile, we should validate them in `--locked`. --- crates/uv-resolver/src/lock/mod.rs | 108 +++++++--- .../uv/src/commands/project/install_target.rs | 3 +- crates/uv/src/commands/project/lock.rs | 14 ++ crates/uv/tests/it/lock.rs | 189 ++++++++++++++++++ 4 files changed, 289 insertions(+), 25 deletions(-) diff --git a/crates/uv-resolver/src/lock/mod.rs b/crates/uv-resolver/src/lock/mod.rs index be9acd766..37c3f9836 100644 --- a/crates/uv-resolver/src/lock/mod.rs +++ b/crates/uv-resolver/src/lock/mod.rs @@ -20,7 +20,7 @@ use url::Url; use uv_cache_key::RepositoryUrl; use uv_configuration::BuildOptions; -use uv_distribution::{DistributionDatabase, FlatRequiresDist, RequiresDist}; +use uv_distribution::{DistributionDatabase, FlatRequiresDist}; use uv_distribution_filename::{ BuildTag, DistExtension, ExtensionError, SourceDistExtension, WheelFilename, }; @@ -580,6 +580,12 @@ impl Lock { self } + /// Returns `true` if this [`Lock`] includes `provides-extra` metadata. + pub fn supports_provides_extra(&self) -> bool { + // `provides-extra` was added in Version 1 Revision 1. + (self.version(), self.revision()) >= (1, 1) + } + /// Returns the lockfile version. pub fn version(&self) -> u32 { self.version @@ -1020,30 +1026,55 @@ impl Lock { dist } - /// Return a [`SatisfiesResult`] if the given [`RequiresDist`] does not match the [`Package`]. + /// Return a [`SatisfiesResult`] if the given extras do not match the [`Package`] metadata. + fn satisfies_provides_extra<'lock>( + &self, + provides_extra: Vec, + package: &'lock Package, + ) -> SatisfiesResult<'lock> { + if !self.supports_provides_extra() { + return SatisfiesResult::Satisfied; + } + + let expected: BTreeSet<_> = provides_extra.iter().collect(); + let actual: BTreeSet<_> = package.metadata.provides_extras.iter().collect(); + + if expected != actual { + let expected = provides_extra.into_iter().collect(); + return SatisfiesResult::MismatchedPackageProvidesExtra( + &package.id.name, + package.id.version.as_ref(), + expected, + actual, + ); + } + + SatisfiesResult::Satisfied + } + + /// Return a [`SatisfiesResult`] if the given requirements do not match the [`Package`] metadata. + #[allow(clippy::unused_self)] fn satisfies_requires_dist<'lock>( - metadata: RequiresDist, + &self, + requires_dist: Vec, + dependency_groups: BTreeMap>, package: &'lock Package, root: &Path, ) -> Result, LockError> { // Special-case: if the version is dynamic, compare the flattened requirements. let flattened = if package.is_dynamic() { Some( - FlatRequiresDist::from_requirements( - metadata.requires_dist.clone(), - &package.id.name, - ) - .into_iter() - .map(|requirement| normalize_requirement(requirement, root)) - .collect::, _>>()?, + FlatRequiresDist::from_requirements(requires_dist.clone(), &package.id.name) + .into_iter() + .map(|requirement| normalize_requirement(requirement, root)) + .collect::, _>>()?, ) } else { None }; // Validate the `requires-dist` metadata. - let expected: BTreeSet<_> = metadata - .requires_dist + let expected: BTreeSet<_> = requires_dist .into_iter() .map(|requirement| normalize_requirement(requirement, root)) .collect::>()?; @@ -1065,8 +1096,7 @@ impl Lock { } // Validate the `dependency-groups` metadata. - let expected: BTreeMap> = metadata - .dependency_groups + let expected: BTreeMap> = dependency_groups .into_iter() .filter(|(_, requirements)| !requirements.is_empty()) .map(|(group, requirements)| { @@ -1402,8 +1432,19 @@ impl Lock { )); } + // Validate the `provides-extras` metadata. + match self.satisfies_provides_extra(metadata.provides_extras, package) { + SatisfiesResult::Satisfied => {} + result => return Ok(result), + } + // Validate that the requirements are unchanged. - match Self::satisfies_requires_dist(RequiresDist::from(metadata), package, root)? { + match self.satisfies_requires_dist( + metadata.requires_dist, + metadata.dependency_groups, + package, + root, + )? { SatisfiesResult::Satisfied => {} result => return Ok(result), } @@ -1432,21 +1473,30 @@ impl Lock { return false; } + // Validate that the extras are unchanged. + if let SatisfiesResult::Satisfied = self.satisfies_provides_extra(metadata.provides_extras, package, ) { + debug!("Static `provides-extra` for `{}` is up-to-date", package.id); + } else { + debug!("Static `provides-extra` for `{}` is out-of-date; falling back to distribution database", package.id); + return false; + } + // Validate that the requirements are unchanged. - match Self::satisfies_requires_dist(metadata, package, root) { + match self.satisfies_requires_dist(metadata.requires_dist, metadata.dependency_groups, package, root) { Ok(SatisfiesResult::Satisfied) => { debug!("Static `requires-dist` for `{}` is up-to-date", package.id); - true }, Ok(..) => { debug!("Static `requires-dist` for `{}` is out-of-date; falling back to distribution database", package.id); - false + return false; }, Err(..) => { debug!("Static `requires-dist` for `{}` is invalid; falling back to distribution database", package.id); - false + return false; }, } + + true }); // If the `requires-dist` metadata matches the requirements, we're done; otherwise, @@ -1504,9 +1554,16 @@ impl Lock { return Ok(SatisfiesResult::MismatchedDynamic(&package.id.name, true)); } + // Validate that the extras are unchanged. + match self.satisfies_provides_extra(metadata.provides_extras, package) { + SatisfiesResult::Satisfied => {} + result => return Ok(result), + } + // Validate that the requirements are unchanged. - match Self::satisfies_requires_dist( - RequiresDist::from(metadata), + match self.satisfies_requires_dist( + metadata.requires_dist, + metadata.dependency_groups, package, root, )? { @@ -1519,8 +1576,6 @@ impl Lock { } // Recurse. - // TODO(charlie): Do we care about extras here, or any other fields on the `Dependency`? - // Should we instead recurse on `requires_dist`? for dep in &package.dependencies { if seen.insert(&dep.package_id) { let dep_dist = self.find_by_id(&dep.package_id); @@ -1608,6 +1663,13 @@ pub enum SatisfiesResult<'lock> { BTreeSet, BTreeSet, ), + /// A package in the lockfile contains different `provides-extra` metadata than expected. + MismatchedPackageProvidesExtra( + &'lock PackageName, + Option<&'lock Version>, + BTreeSet, + BTreeSet<&'lock ExtraName>, + ), /// A package in the lockfile contains different `dependency-groups` metadata than expected. MismatchedPackageDependencyGroups( &'lock PackageName, diff --git a/crates/uv/src/commands/project/install_target.rs b/crates/uv/src/commands/project/install_target.rs index 185de6651..205b7d59c 100644 --- a/crates/uv/src/commands/project/install_target.rs +++ b/crates/uv/src/commands/project/install_target.rs @@ -259,8 +259,7 @@ impl<'lock> InstallTarget<'lock> { Self::Project { lock, .. } | Self::Workspace { lock, .. } | Self::NonProjectWorkspace { lock, .. } => { - // `provides-extra` was added in Version 1 Revision 1. - if (lock.version(), lock.revision()) < (1, 1) { + if !lock.supports_provides_extra() { return Ok(()); } diff --git a/crates/uv/src/commands/project/lock.rs b/crates/uv/src/commands/project/lock.rs index 701a7fc40..6949d8bbc 100644 --- a/crates/uv/src/commands/project/lock.rs +++ b/crates/uv/src/commands/project/lock.rs @@ -1123,6 +1123,20 @@ impl ValidatedLock { } Ok(Self::Preferable(lock)) } + SatisfiesResult::MismatchedPackageProvidesExtra(name, version, expected, actual) => { + if let Some(version) = version { + debug!( + "Ignoring existing lockfile due to mismatched extras for: `{name}=={version}`\n Requested: {:?}\n Existing: {:?}", + expected, actual + ); + } else { + debug!( + "Ignoring existing lockfile due to mismatched extras for: `{name}`\n Requested: {:?}\n Existing: {:?}", + expected, actual + ); + } + Ok(Self::Preferable(lock)) + } SatisfiesResult::MissingVersion(name) => { debug!("Ignoring existing lockfile due to missing version: `{name}`"); Ok(Self::Preferable(lock)) diff --git a/crates/uv/tests/it/lock.rs b/crates/uv/tests/it/lock.rs index 9fcc60b24..c8600e720 100644 --- a/crates/uv/tests/it/lock.rs +++ b/crates/uv/tests/it/lock.rs @@ -25630,3 +25630,192 @@ fn windows_arm() -> Result<()> { Ok(()) } + +#[test] +fn lock_empty_extra() -> 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" }] + "### + ); + }); + + // Add a non-empty extra. + pyproject_toml.write_str( + r#" + [project] + name = "project" + version = "0.1.0" + requires-python = ">=3.12" + dependencies = ["iniconfig"] + + [project.optional-dependencies] + foo = ["typing-extensions"] + "#, + )?; + + // Re-run with `--locked`. We expect this to fail, since we've added an extra. + uv_snapshot!(context.filters(), context.lock().arg("--locked"), @r###" + success: false + exit_code: 2 + ----- stdout ----- + + ----- stderr ----- + Resolved 3 packages in [TIME] + error: The lockfile at `uv.lock` needs to be updated, but `--locked` was provided. To update the lockfile, run `uv lock`. + "###); + + uv_snapshot!(context.filters(), context.lock(), @r###" + success: true + exit_code: 0 + ----- stdout ----- + + ----- stderr ----- + Resolved 3 packages in [TIME] + Added typing-extensions v4.10.0 + "###); + + // Add an empty extra. + pyproject_toml.write_str( + r#" + [project] + name = "project" + version = "0.1.0" + requires-python = ">=3.12" + dependencies = ["iniconfig"] + + [project.optional-dependencies] + foo = ["typing-extensions"] + bar = [] + "#, + )?; + + // Re-run with `--locked`. We expect this to fail, since we've added an extra. + uv_snapshot!(context.filters(), context.lock().arg("--locked"), @r###" + success: false + exit_code: 2 + ----- stdout ----- + + ----- stderr ----- + Resolved 3 packages in [TIME] + error: The lockfile at `uv.lock` needs to be updated, but `--locked` was provided. To update the lockfile, run `uv lock`. + "###); + + uv_snapshot!(context.filters(), context.lock(), @r###" + success: true + exit_code: 0 + ----- stdout ----- + + ----- stderr ----- + Resolved 3 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.optional-dependencies] + foo = [ + { name = "typing-extensions" }, + ] + + [package.metadata] + requires-dist = [ + { name = "iniconfig" }, + { name = "typing-extensions", marker = "extra == 'foo'" }, + ] + provides-extras = ["foo", "bar"] + + [[package]] + name = "typing-extensions" + version = "4.10.0" + source = { registry = "https://pypi.org/simple" } + sdist = { url = "https://files.pythonhosted.org/packages/16/3a/0d26ce356c7465a19c9ea8814b960f8a36c3b0d07c323176620b7b483e44/typing_extensions-4.10.0.tar.gz", hash = "sha256:b0abd7c89e8fb96f98db18d86106ff1d90ab692004eb746cf6eda2682f91b3cb", size = 77558 } + wheels = [ + { url = "https://files.pythonhosted.org/packages/f9/de/dc04a3ea60b22624b51c703a84bbe0184abcd1d0b9bc8074b5d6b7ab90bb/typing_extensions-4.10.0-py3-none-any.whl", hash = "sha256:69b1a937c3a517342112fb4c6df7e72fc39a38e7891a5730ed4985b5214b5475", size = 33926 }, + ] + "### + ); + }); + + Ok(()) +}