From a4bd73f9229eabce1ecc70cc605ab5872d40e27c Mon Sep 17 00:00:00 2001 From: Charlie Marsh Date: Wed, 12 Feb 2025 20:44:14 -0500 Subject: [PATCH] Omit lockfile version when additional fields are dynamic (#11468) ## Summary Just a logic issue... If we see a dynamic field that isn't `"version"`, we end up _not_ propagating the fact that `"version"` is also dynamic. Closes https://github.com/astral-sh/uv/issues/11460. --- crates/uv-distribution/src/source/mod.rs | 142 +++++++++++------- .../src/metadata/metadata_resolver.rs | 22 +-- crates/uv-pypi-types/src/metadata/mod.rs | 1 + .../src/metadata/pyproject_toml.rs | 20 +-- .../src/metadata/requires_dist.rs | 1 + crates/uv/tests/it/lock.rs | 110 +++++++++++++- 6 files changed, 225 insertions(+), 71 deletions(-) diff --git a/crates/uv-distribution/src/source/mod.rs b/crates/uv-distribution/src/source/mod.rs index 69a2d7d05..1d865f798 100644 --- a/crates/uv-distribution/src/source/mod.rs +++ b/crates/uv-distribution/src/source/mod.rs @@ -45,7 +45,9 @@ use uv_metadata::read_archive_metadata; use uv_normalize::PackageName; use uv_pep440::{release_specifiers_to_ranges, Version}; use uv_platform_tags::Tags; -use uv_pypi_types::{HashAlgorithm, HashDigest, Metadata12, RequiresTxt, ResolutionMetadata}; +use uv_pypi_types::{ + HashAlgorithm, HashDigest, Metadata12, PyProjectToml, RequiresTxt, ResolutionMetadata, +}; use uv_types::{BuildContext, BuildStack, SourceBuildTrait}; use uv_workspace::pyproject::ToolUvSources; use zip::ZipArchive; @@ -1890,21 +1892,35 @@ impl<'a, T: BuildContext> SourceDistributionBuilder<'a, T> { return Ok(None); }; - // Parse the metadata. - let metadata = match ResolutionMetadata::parse_pyproject_toml(&content, source.version()) { + // Parse the `pyproject.toml`. + let pyproject_toml = match PyProjectToml::from_toml(&content) { Ok(metadata) => metadata, Err( - uv_pypi_types::MetadataError::Pep508Error(_) - | uv_pypi_types::MetadataError::DynamicField(_) - | uv_pypi_types::MetadataError::FieldNotFound(_) - | uv_pypi_types::MetadataError::PoetrySyntax, + uv_pypi_types::MetadataError::InvalidPyprojectTomlSyntax(..) + | uv_pypi_types::MetadataError::InvalidPyprojectTomlSchema(..), ) => { - debug!("Failed to extract static metadata from GitHub API for: {url}"); + debug!("Failed to read `pyproject.toml` from GitHub API for: {url}"); return Ok(None); } Err(err) => return Err(err.into()), }; + // Parse the metadata. + let metadata = + match ResolutionMetadata::parse_pyproject_toml(pyproject_toml, source.version()) { + Ok(metadata) => metadata, + Err( + uv_pypi_types::MetadataError::Pep508Error(..) + | uv_pypi_types::MetadataError::DynamicField(..) + | uv_pypi_types::MetadataError::FieldNotFound(..) + | uv_pypi_types::MetadataError::PoetrySyntax, + ) => { + debug!("Failed to extract static metadata from GitHub API for: {url}"); + return Ok(None); + } + Err(err) => return Err(err.into()), + }; + // Determine whether the project has `tool.uv.sources`. If the project has sources, it must // be lowered, which requires access to the workspace. For example, it could have workspace // members that need to be translated to concrete paths on disk. @@ -2417,49 +2433,58 @@ impl StaticMetadata { source_root: &Path, subdirectory: Option<&Path>, ) -> Result { - // Attempt to read static metadata from the `pyproject.toml`. - match read_pyproject_toml(source_root, subdirectory, source.version()).await { - Ok(metadata) => { - debug!("Found static `pyproject.toml` for: {source}"); - - // Validate the metadata, but ignore it if the metadata doesn't match. - match validate_metadata(source, &metadata) { - Ok(()) => { - return Ok(Self::Some(metadata)); - } - Err(err) => { - debug!("Ignoring `pyproject.toml` for {source}: {err}"); - } - } - } - Err( - err @ Error::PyprojectToml(uv_pypi_types::MetadataError::DynamicField("version")), - ) if source.is_source_tree() => { - // In Metadata 2.2, `Dynamic` was introduced to Core Metadata to indicate that a - // given field was marked as dynamic in the originating source tree. However, we may - // be looking at a distribution with a build backend that doesn't support Metadata 2.2. In that case, - // we want to infer the `Dynamic` status from the `pyproject.toml` file, if available. - debug!("No static `pyproject.toml` available for: {source} ({err:?})"); - return Ok(Self::Dynamic); - } - Err( - err @ (Error::MissingPyprojectToml - | Error::PyprojectToml( - uv_pypi_types::MetadataError::Pep508Error(_) - | uv_pypi_types::MetadataError::DynamicField(_) - | uv_pypi_types::MetadataError::FieldNotFound(_) - | uv_pypi_types::MetadataError::PoetrySyntax, - )), - ) => { - debug!("No static `pyproject.toml` available for: {source} ({err:?})"); + // Attempt to read the `pyproject.toml`. + let pyproject_toml = match read_pyproject_toml(source_root, subdirectory).await { + Ok(pyproject_toml) => Some(pyproject_toml), + Err(Error::MissingPyprojectToml) => { + debug!("No `pyproject.toml` available for: {source}"); + None } Err(err) => return Err(err), + }; + + // Determine whether the version is static or dynamic. + let dynamic = pyproject_toml.as_ref().is_some_and(|pyproject_toml| { + pyproject_toml.project.as_ref().is_some_and(|project| { + project + .dynamic + .as_ref() + .is_some_and(|dynamic| dynamic.iter().any(|field| field == "version")) + }) + }); + + // Attempt to read static metadata from the `pyproject.toml`. + if let Some(pyproject_toml) = pyproject_toml { + match ResolutionMetadata::parse_pyproject_toml(pyproject_toml, source.version()) { + Ok(metadata) => { + debug!("Found static `pyproject.toml` for: {source}"); + + // Validate the metadata, but ignore it if the metadata doesn't match. + match validate_metadata(source, &metadata) { + Ok(()) => { + return Ok(Self::Some(metadata)); + } + Err(err) => { + debug!("Ignoring `pyproject.toml` for {source}: {err}"); + } + } + } + Err( + err @ (uv_pypi_types::MetadataError::Pep508Error(_) + | uv_pypi_types::MetadataError::DynamicField(_) + | uv_pypi_types::MetadataError::FieldNotFound(_) + | uv_pypi_types::MetadataError::PoetrySyntax), + ) => { + debug!("No static `pyproject.toml` available for: {source} ({err:?})"); + } + Err(err) => return Err(Error::PyprojectToml(err)), + } } // If the source distribution is a source tree, avoid reading `PKG-INFO` or `egg-info`, // since they could be out-of-date. if source.is_source_tree() { - return Ok(Self::None); + return Ok(if dynamic { Self::Dynamic } else { Self::None }); } // Attempt to read static metadata from the `PKG-INFO` file. @@ -2470,6 +2495,15 @@ impl StaticMetadata { // Validate the metadata, but ignore it if the metadata doesn't match. match validate_metadata(source, &metadata) { Ok(()) => { + // If necessary, mark the metadata as dynamic. + let metadata = if dynamic { + ResolutionMetadata { + dynamic: true, + ..metadata + } + } else { + metadata + }; return Ok(Self::Some(metadata)); } Err(err) => { @@ -2499,6 +2533,15 @@ impl StaticMetadata { // Validate the metadata, but ignore it if the metadata doesn't match. match validate_metadata(source, &metadata) { Ok(()) => { + // If necessary, mark the metadata as dynamic. + let metadata = if dynamic { + ResolutionMetadata { + dynamic: true, + ..metadata + } + } else { + metadata + }; return Ok(Self::Some(metadata)); } Err(err) => { @@ -2526,7 +2569,7 @@ impl StaticMetadata { Err(err) => return Err(err), } - Ok(Self::None) + Ok(if dynamic { Self::Dynamic } else { Self::None }) } } @@ -2845,8 +2888,7 @@ async fn read_pkg_info( async fn read_pyproject_toml( source_tree: &Path, subdirectory: Option<&Path>, - sdist_version: Option<&Version>, -) -> Result { +) -> Result { // Read the `pyproject.toml` file. let pyproject_toml = match subdirectory { Some(subdirectory) => source_tree.join(subdirectory).join("pyproject.toml"), @@ -2860,11 +2902,9 @@ async fn read_pyproject_toml( Err(err) => return Err(Error::CacheRead(err)), }; - // Parse the metadata. - let metadata = ResolutionMetadata::parse_pyproject_toml(&content, sdist_version) - .map_err(Error::PyprojectToml)?; + let pyproject_toml = PyProjectToml::from_toml(&content)?; - Ok(metadata) + Ok(pyproject_toml) } /// Return the [`pypi_types::RequiresDist`] from a `pyproject.toml`, if it can be statically extracted. diff --git a/crates/uv-pypi-types/src/metadata/metadata_resolver.rs b/crates/uv-pypi-types/src/metadata/metadata_resolver.rs index 2add8c1c2..c4dcebb41 100644 --- a/crates/uv-pypi-types/src/metadata/metadata_resolver.rs +++ b/crates/uv-pypi-types/src/metadata/metadata_resolver.rs @@ -165,11 +165,9 @@ impl ResolutionMetadata { /// If we're coming from a source distribution, we may already know the version (unlike for a /// source tree), so we can tolerate dynamic versions. pub fn parse_pyproject_toml( - content: &str, + pyproject_toml: PyProjectToml, sdist_version: Option<&Version>, ) -> Result { - let pyproject_toml = PyProjectToml::from_toml(content)?; - let project = pyproject_toml .project .ok_or(MetadataError::FieldNotFound("project"))?; @@ -333,7 +331,8 @@ mod tests { [project] name = "asdf" "#; - let meta = ResolutionMetadata::parse_pyproject_toml(s, None); + let pyproject = PyProjectToml::from_toml(s).unwrap(); + let meta = ResolutionMetadata::parse_pyproject_toml(pyproject, None); assert!(matches!(meta, Err(MetadataError::FieldNotFound("version")))); let s = r#" @@ -341,7 +340,8 @@ mod tests { name = "asdf" dynamic = ["version"] "#; - let meta = ResolutionMetadata::parse_pyproject_toml(s, None); + let pyproject = PyProjectToml::from_toml(s).unwrap(); + let meta = ResolutionMetadata::parse_pyproject_toml(pyproject, None); assert!(matches!(meta, Err(MetadataError::DynamicField("version")))); let s = r#" @@ -349,7 +349,8 @@ mod tests { name = "asdf" version = "1.0" "#; - let meta = ResolutionMetadata::parse_pyproject_toml(s, None).unwrap(); + let pyproject = PyProjectToml::from_toml(s).unwrap(); + let meta = ResolutionMetadata::parse_pyproject_toml(pyproject, None).unwrap(); assert_eq!(meta.name, PackageName::from_str("asdf").unwrap()); assert_eq!(meta.version, Version::new([1, 0])); assert!(meta.requires_python.is_none()); @@ -362,7 +363,8 @@ mod tests { version = "1.0" requires-python = ">=3.6" "#; - let meta = ResolutionMetadata::parse_pyproject_toml(s, None).unwrap(); + let pyproject = PyProjectToml::from_toml(s).unwrap(); + let meta = ResolutionMetadata::parse_pyproject_toml(pyproject, None).unwrap(); assert_eq!(meta.name, PackageName::from_str("asdf").unwrap()); assert_eq!(meta.version, Version::new([1, 0])); assert_eq!(meta.requires_python, Some(">=3.6".parse().unwrap())); @@ -376,7 +378,8 @@ mod tests { requires-python = ">=3.6" dependencies = ["foo"] "#; - let meta = ResolutionMetadata::parse_pyproject_toml(s, None).unwrap(); + let pyproject = PyProjectToml::from_toml(s).unwrap(); + let meta = ResolutionMetadata::parse_pyproject_toml(pyproject, None).unwrap(); assert_eq!(meta.name, PackageName::from_str("asdf").unwrap()); assert_eq!(meta.version, Version::new([1, 0])); assert_eq!(meta.requires_python, Some(">=3.6".parse().unwrap())); @@ -393,7 +396,8 @@ mod tests { [project.optional-dependencies] dotenv = ["bar"] "#; - let meta = ResolutionMetadata::parse_pyproject_toml(s, None).unwrap(); + let pyproject = PyProjectToml::from_toml(s).unwrap(); + let meta = ResolutionMetadata::parse_pyproject_toml(pyproject, None).unwrap(); assert_eq!(meta.name, PackageName::from_str("asdf").unwrap()); assert_eq!(meta.version, Version::new([1, 0])); assert_eq!(meta.requires_python, Some(">=3.6".parse().unwrap())); diff --git a/crates/uv-pypi-types/src/metadata/mod.rs b/crates/uv-pypi-types/src/metadata/mod.rs index 09493ca89..f6886793c 100644 --- a/crates/uv-pypi-types/src/metadata/mod.rs +++ b/crates/uv-pypi-types/src/metadata/mod.rs @@ -23,6 +23,7 @@ pub use metadata10::Metadata10; pub use metadata12::Metadata12; pub use metadata23::Metadata23; pub use metadata_resolver::ResolutionMetadata; +pub use pyproject_toml::PyProjectToml; pub use requires_dist::RequiresDist; pub use requires_txt::RequiresTxt; diff --git a/crates/uv-pypi-types/src/metadata/pyproject_toml.rs b/crates/uv-pypi-types/src/metadata/pyproject_toml.rs index 19fa91de2..e8d162f9a 100644 --- a/crates/uv-pypi-types/src/metadata/pyproject_toml.rs +++ b/crates/uv-pypi-types/src/metadata/pyproject_toml.rs @@ -12,13 +12,13 @@ use crate::MetadataError; /// A `pyproject.toml` as specified in PEP 517. #[derive(Deserialize, Debug)] #[serde(rename_all = "kebab-case")] -pub(super) struct PyProjectToml { - pub(super) project: Option, +pub struct PyProjectToml { + pub project: Option, pub(super) tool: Option, } impl PyProjectToml { - pub(super) fn from_toml(toml: &str) -> Result { + pub fn from_toml(toml: &str) -> Result { let pyproject_toml: toml_edit::ImDocument<_> = toml_edit::ImDocument::from_str(toml) .map_err(MetadataError::InvalidPyprojectTomlSyntax)?; let pyproject_toml: Self = PyProjectToml::deserialize(pyproject_toml.into_deserializer()) @@ -35,20 +35,20 @@ impl PyProjectToml { /// See . #[derive(Deserialize, Debug)] #[serde(try_from = "PyprojectTomlWire")] -pub(super) struct Project { +pub struct Project { /// The name of the project - pub(super) name: PackageName, + pub name: PackageName, /// The version of the project as supported by PEP 440 - pub(super) version: Option, + pub version: Option, /// The Python version requirements of the project - pub(super) requires_python: Option, + pub requires_python: Option, /// Project dependencies - pub(super) dependencies: Option>, + pub dependencies: Option>, /// Optional dependencies - pub(super) optional_dependencies: Option>>, + pub optional_dependencies: Option>>, /// Specifies which fields listed by PEP 621 were intentionally unspecified /// so another tool can/will provide such metadata dynamically. - pub(super) dynamic: Option>, + pub dynamic: Option>, } #[derive(Deserialize, Debug)] diff --git a/crates/uv-pypi-types/src/metadata/requires_dist.rs b/crates/uv-pypi-types/src/metadata/requires_dist.rs index fe80a8632..dd2e58d66 100644 --- a/crates/uv-pypi-types/src/metadata/requires_dist.rs +++ b/crates/uv-pypi-types/src/metadata/requires_dist.rs @@ -21,6 +21,7 @@ pub struct RequiresDist { pub name: PackageName, pub requires_dist: Vec>, pub provides_extras: Vec, + #[serde(default)] pub dynamic: bool, } diff --git a/crates/uv/tests/it/lock.rs b/crates/uv/tests/it/lock.rs index f8880d747..7bfa4c11b 100644 --- a/crates/uv/tests/it/lock.rs +++ b/crates/uv/tests/it/lock.rs @@ -17063,7 +17063,7 @@ fn lock_invalid_project_table() -> Result<()> { ----- stderr ----- Using CPython 3.12.[X] interpreter at: [PYTHON-3.12] × Failed to build `b @ file://[TEMP_DIR]/b` - ├─▶ Failed to extract static metadata from `pyproject.toml` + ├─▶ Failed to parse metadata from built wheel ╰─▶ TOML parse error at line 2, column 10 | 2 | [project.urls] @@ -19757,6 +19757,114 @@ fn lock_dynamic_version() -> Result<()> { Ok(()) } +/// Lock a package with a dynamic version and dynamic dependencies. +#[test] +fn lock_dynamic_version_dependencies() -> Result<()> { + let context = TestContext::new("3.12"); + + let pyproject_toml = context.temp_dir.child("pyproject.toml"); + pyproject_toml.write_str( + r#" + [project] + name = "project" + requires-python = ">=3.12" + dynamic = ["dependencies", "version"] + + [build-system] + requires = ["setuptools"] + build-backend = "setuptools.build_meta" + + [tool.uv] + cache-keys = [{ file = "pyproject.toml" }, { file = "src/project/__init__.py" }] + + [tool.setuptools.dynamic] + version = { attr = "project.__version__" } + + [tool.setuptools] + package-dir = { "" = "src" } + + [tool.setuptools.packages.find] + where = ["src"] + "#, + )?; + + context + .temp_dir + .child("src") + .child("project") + .child("__init__.py") + .write_str("__version__ = '0.1.0'")?; + + uv_snapshot!(context.filters(), context.lock(), @r###" + success: true + exit_code: 0 + ----- stdout ----- + + ----- stderr ----- + Resolved 1 package in [TIME] + "###); + + let lock = fs_err::read_to_string(context.temp_dir.join("uv.lock")).unwrap(); + + insta::with_settings!({ + filters => context.filters(), + }, { + assert_snapshot!( + lock, @r###" + version = 1 + requires-python = ">=3.12" + + [options] + exclude-newer = "2024-03-25T00:00:00Z" + + [[package]] + name = "project" + source = { editable = "." } + "### + ); + }); + + // Bump the version. + context + .temp_dir + .child("src") + .child("project") + .child("__init__.py") + .write_str("__version__ = '0.1.1'")?; + + // Re-run with `--locked`. We should accept the lockfile, since dynamic versions are omitted. + uv_snapshot!(context.filters(), context.lock().arg("--locked"), @r###" + success: true + exit_code: 0 + ----- stdout ----- + + ----- stderr ----- + Resolved 1 package in [TIME] + "###); + + let lock = fs_err::read_to_string(context.temp_dir.join("uv.lock")).unwrap(); + + insta::with_settings!({ + filters => context.filters(), + }, { + assert_snapshot!( + lock, @r###" + version = 1 + requires-python = ">=3.12" + + [options] + exclude-newer = "2024-03-25T00:00:00Z" + + [[package]] + name = "project" + source = { editable = "." } + "### + ); + }); + + Ok(()) +} + /// Validating a lockfile with a dynamic version (but static dependencies) shouldn't require /// building the package. #[test]