From 4aad89cf06ad2853ea31ef1ef77ef14f90a54044 Mon Sep 17 00:00:00 2001 From: konsti Date: Sat, 14 Sep 2024 22:03:47 +0200 Subject: [PATCH] Hint at missing `project.name` (#6803) We got user reports where users were confused about why they can't use `[project.urls]` in `pyproject.toml` (i think that's from poetry?). This PR adds a hint that (according to PEP 621), you need to set `project.name` when using any `project` fields. (PEP 621 also requires `project.version` xor `dynamic = ["version"]`, but we check that later.) The intermediate parsing layer to tell apart syntax errors from schema errors doesn't incur a performance penalty according to epage (https://github.com/toml-rs/toml/issues/778#issuecomment-2310369253). Closes #6419 Closes #6760 --- Cargo.lock | 3 +- Cargo.toml | 2 +- crates/pypi-types/Cargo.toml | 1 + crates/pypi-types/src/metadata.rs | 28 ++++++++++++++++-- crates/uv-build/Cargo.toml | 2 +- crates/uv-build/src/error.rs | 4 ++- crates/uv-build/src/lib.rs | 8 ++++-- crates/uv-metadata/src/lib.rs | 9 ++++-- crates/uv-tool/Cargo.toml | 2 +- crates/uv/tests/lock.rs | 47 +++++++++++++++++++++++++++++++ crates/uv/tests/run.rs | 39 +++++++++++++++++++++++++ 11 files changed, 132 insertions(+), 13 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index d84c5e66b..9e02fc56a 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2817,6 +2817,7 @@ dependencies = [ "serde", "thiserror", "toml", + "toml_edit", "tracing", "url", "uv-fs", @@ -4573,7 +4574,7 @@ dependencies = [ "tempfile", "thiserror", "tokio", - "toml", + "toml_edit", "tracing", "uv-configuration", "uv-fs", diff --git a/Cargo.toml b/Cargo.toml index 70d70a3ba..199cc05c4 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -148,7 +148,7 @@ tokio = { version = "1.35.1", features = ["fs", "io-util", "macros", "process", tokio-stream = { version = "0.1.14" } tokio-util = { version = "0.7.10", features = ["compat"] } toml = { version = "0.8.12" } -toml_edit = { version = "0.22.13" } +toml_edit = { version = "0.22.13", features = ["serde"] } tracing = { version = "0.1.40" } tracing-durations-export = { version = "0.3.0", features = ["plot"] } tracing-subscriber = { version = "0.3.18", features = ["env-filter", "json", "registry"] } diff --git a/crates/pypi-types/Cargo.toml b/crates/pypi-types/Cargo.toml index 5e3b42fbc..3f27358d2 100644 --- a/crates/pypi-types/Cargo.toml +++ b/crates/pypi-types/Cargo.toml @@ -29,6 +29,7 @@ rkyv = { workspace = true } serde = { workspace = true } thiserror = { workspace = true } toml = { workspace = true } +toml_edit = { workspace = true } tracing = { workspace = true } url = { workspace = true } diff --git a/crates/pypi-types/src/metadata.rs b/crates/pypi-types/src/metadata.rs index f1fb9c4a6..c162a3d4c 100644 --- a/crates/pypi-types/src/metadata.rs +++ b/crates/pypi-types/src/metadata.rs @@ -6,6 +6,7 @@ use std::str::FromStr; use indexmap::IndexMap; use itertools::Itertools; use mailparse::{MailHeaderMap, MailParseError}; +use serde::de::IntoDeserializer; use serde::{Deserialize, Serialize}; use thiserror::Error; use tracing::warn; @@ -44,8 +45,12 @@ pub struct Metadata23 { pub enum MetadataError { #[error(transparent)] MailParse(#[from] MailParseError), + #[error("Invalid `pyproject.toml`")] + InvalidPyprojectTomlSyntax(#[source] toml_edit::TomlError), + #[error("`pyproject.toml` is using the `[project]` table, but the required `project.name` is not set.")] + InvalidPyprojectTomlMissingName(#[source] toml_edit::de::Error), #[error(transparent)] - Toml(#[from] toml::de::Error), + InvalidPyprojectTomlSchema(toml_edit::de::Error), #[error("metadata field {0} not found")] FieldNotFound(&'static str), #[error("invalid version: {0}")] @@ -196,7 +201,7 @@ impl Metadata23 { /// Extract the metadata from a `pyproject.toml` file, as specified in PEP 621. pub fn parse_pyproject_toml(contents: &str) -> Result { - let pyproject_toml: PyProjectToml = toml::from_str(contents)?; + let pyproject_toml = PyProjectToml::from_toml(contents)?; let project = pyproject_toml .project @@ -279,6 +284,23 @@ struct PyProjectToml { tool: Option, } +impl PyProjectToml { + 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()) + .map_err(|err| { + // TODO(konsti): A typed error would be nicer, this can break on toml upgrades. + if err.message().contains("missing field `name`") { + MetadataError::InvalidPyprojectTomlMissingName(err) + } else { + MetadataError::InvalidPyprojectTomlSchema(err) + } + })?; + Ok(pyproject_toml) + } +} + /// PEP 621 project metadata. /// /// This is a subset of the full metadata specification, and only includes the fields that are @@ -435,7 +457,7 @@ pub struct RequiresDist { impl RequiresDist { /// Extract the [`RequiresDist`] from a `pyproject.toml` file, as specified in PEP 621. pub fn parse_pyproject_toml(contents: &str) -> Result { - let pyproject_toml: PyProjectToml = toml::from_str(contents)?; + let pyproject_toml = PyProjectToml::from_toml(contents)?; let project = pyproject_toml .project diff --git a/crates/uv-build/Cargo.toml b/crates/uv-build/Cargo.toml index db445b2ab..514e84848 100644 --- a/crates/uv-build/Cargo.toml +++ b/crates/uv-build/Cargo.toml @@ -35,7 +35,7 @@ serde_json = { workspace = true } tempfile = { workspace = true } thiserror = { workspace = true } tokio = { workspace = true } -toml = { workspace = true } +toml_edit = { workspace = true } tracing = { workspace = true } rustc-hash = { workspace = true } diff --git a/crates/uv-build/src/error.rs b/crates/uv-build/src/error.rs index 643eb0af9..b13a4246f 100644 --- a/crates/uv-build/src/error.rs +++ b/crates/uv-build/src/error.rs @@ -58,7 +58,9 @@ pub enum Error { #[error("{} does not appear to be a Python project, as neither `pyproject.toml` nor `setup.py` are present in the directory", _0.simplified_display())] InvalidSourceDist(PathBuf), #[error("Invalid `pyproject.toml`")] - InvalidPyprojectToml(#[from] toml::de::Error), + InvalidPyprojectTomlSyntax(#[from] toml_edit::TomlError), + #[error("`pyproject.toml` does not match the required schema. When the `[project]` table is present, `project.name` must be present and non-empty.")] + InvalidPyprojectTomlSchema(#[from] toml_edit::de::Error), #[error("Editable installs with setup.py legacy builds are unsupported, please specify a build backend in pyproject.toml")] EditableSetupPy, #[error("Failed to install requirements from {0}")] diff --git a/crates/uv-build/src/lib.rs b/crates/uv-build/src/lib.rs index cbe01c192..801d2fd3f 100644 --- a/crates/uv-build/src/lib.rs +++ b/crates/uv-build/src/lib.rs @@ -8,7 +8,7 @@ use fs_err as fs; use indoc::formatdoc; use itertools::Itertools; use rustc_hash::FxHashMap; -use serde::de::{value, SeqAccess, Visitor}; +use serde::de::{value, IntoDeserializer, SeqAccess, Visitor}; use serde::{de, Deserialize, Deserializer}; use std::ffi::OsString; use std::fmt::Formatter; @@ -430,8 +430,12 @@ impl SourceBuild { ) -> Result<(Pep517Backend, Option), Box> { match fs::read_to_string(source_tree.join("pyproject.toml")) { Ok(toml) => { + let pyproject_toml: toml_edit::ImDocument<_> = + toml_edit::ImDocument::from_str(&toml) + .map_err(Error::InvalidPyprojectTomlSyntax)?; let pyproject_toml: PyProjectToml = - toml::from_str(&toml).map_err(Error::InvalidPyprojectToml)?; + PyProjectToml::deserialize(pyproject_toml.into_deserializer()) + .map_err(Error::InvalidPyprojectTomlSchema)?; let backend = if let Some(build_system) = pyproject_toml.build_system { Pep517Backend { // If `build-backend` is missing, inject the legacy setuptools backend, but diff --git a/crates/uv-metadata/src/lib.rs b/crates/uv-metadata/src/lib.rs index 62db5407f..997737ff0 100644 --- a/crates/uv-metadata/src/lib.rs +++ b/crates/uv-metadata/src/lib.rs @@ -37,7 +37,7 @@ pub enum Error { #[error("The .dist-info directory name contains invalid characters")] InvalidName(#[from] InvalidNameError), #[error("The metadata at {0} is invalid")] - InvalidMetadata(String, pypi_types::MetadataError), + InvalidMetadata(String, Box), #[error("Failed to read from zip file")] Zip(#[from] zip::result::ZipError), #[error("Failed to read from zip file")] @@ -285,7 +285,7 @@ pub async fn read_metadata_async_stream( reader.read_to_end(&mut contents).await.unwrap(); let metadata = Metadata23::parse_metadata(&contents) - .map_err(|err| Error::InvalidMetadata(debug_path.to_string(), err))?; + .map_err(|err| Error::InvalidMetadata(debug_path.to_string(), Box::new(err)))?; return Ok(metadata); } @@ -305,7 +305,10 @@ pub fn read_flat_wheel_metadata( let dist_info_prefix = find_flat_dist_info(filename, &wheel)?; let metadata = read_dist_info_metadata(&dist_info_prefix, &wheel)?; Metadata23::parse_metadata(&metadata).map_err(|err| { - Error::InvalidMetadata(format!("{dist_info_prefix}.dist-info/METADATA"), err) + Error::InvalidMetadata( + format!("{dist_info_prefix}.dist-info/METADATA"), + Box::new(err), + ) }) } diff --git a/crates/uv-tool/Cargo.toml b/crates/uv-tool/Cargo.toml index 94507fd8f..6e0986f5b 100644 --- a/crates/uv-tool/Cargo.toml +++ b/crates/uv-tool/Cargo.toml @@ -31,5 +31,5 @@ pathdiff = { workspace = true } serde = { workspace = true } thiserror = { workspace = true } toml = { workspace = true } -toml_edit = { workspace = true, features = ["serde"] } +toml_edit = { workspace = true } tracing = { workspace = true } diff --git a/crates/uv/tests/lock.rs b/crates/uv/tests/lock.rs index d4de1bd4d..69b20c7d3 100644 --- a/crates/uv/tests/lock.rs +++ b/crates/uv/tests/lock.rs @@ -12693,3 +12693,50 @@ fn lock_duplicate_sources() -> Result<()> { Ok(()) } + +#[test] +fn lock_invalid_project_table() -> Result<()> { + let context = TestContext::new("3.12"); + + let pyproject_toml = context.temp_dir.child("a/pyproject.toml"); + pyproject_toml.write_str( + r#" + [project] + name = "a" + version = "0.1.0" + requires-python = ">=3.12" + dependencies = ["b"] + + [tool.uv.sources] + b = { path = "../b" } + "#, + )?; + + let pyproject_toml = context.temp_dir.child("b/pyproject.toml"); + pyproject_toml.write_str( + r" + [project.urls] + repository = 'https://github.com/octocat/octocat-python' + ", + )?; + + uv_snapshot!(context.filters(), context.lock().current_dir(context.temp_dir.join("a")), @r###" + success: false + exit_code: 2 + ----- stdout ----- + + ----- stderr ----- + Using Python 3.12.[X] interpreter at: [PYTHON-3.12] + error: Failed to build: `b @ file://[TEMP_DIR]/b` + Caused by: Failed to extract static metadata from `pyproject.toml` + Caused by: `pyproject.toml` is using the `[project]` table, but the required `project.name` is not set. + Caused by: TOML parse error at line 2, column 10 + | + 2 | [project.urls] + | ^^^^^^^ + missing field `name` + + "###); + + Ok(()) +} diff --git a/crates/uv/tests/run.rs b/crates/uv/tests/run.rs index 9d8a3078c..064f8377b 100644 --- a/crates/uv/tests/run.rs +++ b/crates/uv/tests/run.rs @@ -1910,3 +1910,42 @@ fn run_exit_code() -> Result<()> { Ok(()) } + +#[test] +fn run_lock_invalid_project_table() -> Result<()> { + let context = TestContext::new_with_versions(&["3.12", "3.11", "3.8"]); + + let pyproject_toml = context.temp_dir.child("pyproject.toml"); + pyproject_toml.write_str(indoc! { r#" + [project.urls] + repository = 'https://github.com/octocat/octocat-python' + + [build-system] + requires = ["setuptools>=42"] + build-backend = "setuptools.build_meta" + "# + })?; + + let test_script = context.temp_dir.child("main.py"); + test_script.write_str(indoc! { r#" + print("Hello, world!") + "# + })?; + + uv_snapshot!(context.filters(), context.run().arg("main.py"), @r###" + success: false + exit_code: 2 + ----- stdout ----- + + ----- stderr ----- + error: Failed to parse: `pyproject.toml` + Caused by: TOML parse error at line 1, column 2 + | + 1 | [project.urls] + | ^^^^^^^ + missing field `name` + + "###); + + Ok(()) +}