From 6ad976912d614cb738ce8bb51d62cbd689cadda3 Mon Sep 17 00:00:00 2001 From: Ed Page Date: Tue, 19 Mar 2024 12:40:53 -0500 Subject: [PATCH 01/34] refactor(toml): Rely on resolved version --- crates/cargo-util-schemas/src/manifest/mod.rs | 16 ++++++++++++++++ src/cargo/util/toml/mod.rs | 12 ++++++------ 2 files changed, 22 insertions(+), 6 deletions(-) diff --git a/crates/cargo-util-schemas/src/manifest/mod.rs b/crates/cargo-util-schemas/src/manifest/mod.rs index db7c4a9cd..426000231 100644 --- a/crates/cargo-util-schemas/src/manifest/mod.rs +++ b/crates/cargo-util-schemas/src/manifest/mod.rs @@ -194,6 +194,12 @@ pub struct TomlPackage { pub _invalid_cargo_features: Option, } +impl TomlPackage { + pub fn resolved_version(&self) -> Result, UnresolvedError> { + self.version.as_ref().map(|v| v.resolved()).transpose() + } +} + /// An enum that allows for inheriting keys from a workspace in a Cargo.toml. #[derive(Serialize, Copy, Clone, Debug)] #[serde(untagged)] @@ -205,6 +211,10 @@ pub enum InheritableField { } impl InheritableField { + pub fn resolved(&self) -> Result<&T, UnresolvedError> { + self.as_value().ok_or(UnresolvedError) + } + pub fn as_value(&self) -> Option<&T> { match self { InheritableField::Inherit(_) => None, @@ -1512,3 +1522,9 @@ impl<'de> de::Deserialize<'de> for PathValue { Ok(PathValue(String::deserialize(deserializer)?.into())) } } + +/// Error validating names in Cargo. +#[derive(Debug, thiserror::Error)] +#[error("manifest field was not resolved")] +#[non_exhaustive] +pub struct UnresolvedError; diff --git a/src/cargo/util/toml/mod.rs b/src/cargo/util/toml/mod.rs index cdd92a478..272341c9d 100644 --- a/src/cargo/util/toml/mod.rs +++ b/src/cargo/util/toml/mod.rs @@ -544,13 +544,12 @@ pub fn to_real_manifest( features.require(Feature::open_namespaces())?; } - let version = package + package.version = package .version .clone() - .map(|version| field_inherit_with(version, "version", || inherit()?.version())) - .transpose()?; - - package.version = version.clone().map(manifest::InheritableField::Value); + .map(|value| field_inherit_with(value, "version", || inherit()?.version())) + .transpose()? + .map(manifest::InheritableField::Value); let rust_version = if let Some(rust_version) = &package.rust_version { let rust_version = field_inherit_with(rust_version.clone(), "rust_version", || { @@ -990,6 +989,7 @@ pub fn to_real_manifest( .clone() .map(|p| manifest::InheritableField::Value(p)); + let version = package.resolved_version().expect("previously resolved"); let publish = match publish { Some(manifest::VecStringOrBool::VecString(ref vecstring)) => Some(vecstring.clone()), Some(manifest::VecStringOrBool::Bool(false)) => Some(vec![]), @@ -1004,7 +1004,7 @@ pub fn to_real_manifest( let pkgid = PackageId::new( package.name.as_str().into(), version - .clone() + .cloned() .unwrap_or_else(|| semver::Version::new(0, 0, 0)), source_id, ); From b3183596cce59137d996f3701583ff193b1e1f2c Mon Sep 17 00:00:00 2001 From: Ed Page Date: Tue, 19 Mar 2024 12:51:40 -0500 Subject: [PATCH 02/34] refactor(toml): Rely on resolved rust-version This also removes duplicated inheritance and one of them specifying the wrong field. --- crates/cargo-util-schemas/src/manifest/mod.rs | 4 +++ src/cargo/util/toml/mod.rs | 26 ++++++++----------- 2 files changed, 15 insertions(+), 15 deletions(-) diff --git a/crates/cargo-util-schemas/src/manifest/mod.rs b/crates/cargo-util-schemas/src/manifest/mod.rs index 426000231..3b3795640 100644 --- a/crates/cargo-util-schemas/src/manifest/mod.rs +++ b/crates/cargo-util-schemas/src/manifest/mod.rs @@ -195,6 +195,10 @@ pub struct TomlPackage { } impl TomlPackage { + pub fn resolved_rust_version(&self) -> Result, UnresolvedError> { + self.rust_version.as_ref().map(|v| v.resolved()).transpose() + } + pub fn resolved_version(&self) -> Result, UnresolvedError> { self.version.as_ref().map(|v| v.resolved()).transpose() } diff --git a/src/cargo/util/toml/mod.rs b/src/cargo/util/toml/mod.rs index 272341c9d..51042f6f3 100644 --- a/src/cargo/util/toml/mod.rs +++ b/src/cargo/util/toml/mod.rs @@ -544,6 +544,12 @@ pub fn to_real_manifest( features.require(Feature::open_namespaces())?; } + package.rust_version = package + .rust_version + .clone() + .map(|value| field_inherit_with(value, "rust-version", || inherit()?.rust_version())) + .transpose()? + .map(manifest::InheritableField::Value); package.version = package .version .clone() @@ -551,14 +557,10 @@ pub fn to_real_manifest( .transpose()? .map(manifest::InheritableField::Value); - let rust_version = if let Some(rust_version) = &package.rust_version { - let rust_version = field_inherit_with(rust_version.clone(), "rust_version", || { - inherit()?.rust_version() - })?; - Some(rust_version) - } else { - None - }; + let rust_version = package + .resolved_rust_version() + .expect("previously resolved") + .cloned(); let edition = if let Some(edition) = package.edition.clone() { let edition: Edition = field_inherit_with(edition, "edition", || inherit()?.edition())? @@ -918,10 +920,7 @@ pub fn to_real_manifest( .transpose()? .unwrap_or_default(), links: package.links.clone(), - rust_version: package - .rust_version - .map(|mw| field_inherit_with(mw, "rust-version", || inherit()?.rust_version())) - .transpose()?, + rust_version: rust_version.clone(), }; package.description = metadata .description @@ -963,9 +962,6 @@ pub fn to_real_manifest( .categories .as_ref() .map(|_| manifest::InheritableField::Value(metadata.categories.clone())); - package.rust_version = rust_version - .clone() - .map(|rv| manifest::InheritableField::Value(rv)); package.exclude = package .exclude .as_ref() From 102b5890bef36bf136280a1c86bb6e9d438e48cc Mon Sep 17 00:00:00 2001 From: Ed Page Date: Tue, 19 Mar 2024 13:22:28 -0500 Subject: [PATCH 03/34] refactor(toml): Rely on resolved edition Returning a `&String` is unusual but this keeps things easier on both sides. --- crates/cargo-util-schemas/src/manifest/mod.rs | 4 ++++ src/cargo/util/toml/mod.rs | 11 ++++++++--- 2 files changed, 12 insertions(+), 3 deletions(-) diff --git a/crates/cargo-util-schemas/src/manifest/mod.rs b/crates/cargo-util-schemas/src/manifest/mod.rs index 3b3795640..1ada20b09 100644 --- a/crates/cargo-util-schemas/src/manifest/mod.rs +++ b/crates/cargo-util-schemas/src/manifest/mod.rs @@ -195,6 +195,10 @@ pub struct TomlPackage { } impl TomlPackage { + pub fn resolved_edition(&self) -> Result, UnresolvedError> { + self.edition.as_ref().map(|v| v.resolved()).transpose() + } + pub fn resolved_rust_version(&self) -> Result, UnresolvedError> { self.rust_version.as_ref().map(|v| v.resolved()).transpose() } diff --git a/src/cargo/util/toml/mod.rs b/src/cargo/util/toml/mod.rs index 51042f6f3..00b6b1430 100644 --- a/src/cargo/util/toml/mod.rs +++ b/src/cargo/util/toml/mod.rs @@ -544,6 +544,12 @@ pub fn to_real_manifest( features.require(Feature::open_namespaces())?; } + package.edition = package + .edition + .clone() + .map(|value| field_inherit_with(value, "edition", || inherit()?.edition())) + .transpose()? + .map(manifest::InheritableField::Value); package.rust_version = package .rust_version .clone() @@ -562,11 +568,10 @@ pub fn to_real_manifest( .expect("previously resolved") .cloned(); - let edition = if let Some(edition) = package.edition.clone() { - let edition: Edition = field_inherit_with(edition, "edition", || inherit()?.edition())? + let edition = if let Some(edition) = package.resolved_edition().expect("previously resolved") { + let edition: Edition = edition .parse() .with_context(|| "failed to parse the `edition` key")?; - package.edition = Some(manifest::InheritableField::Value(edition.to_string())); if let Some(pkg_msrv) = &rust_version { if let Some(edition_msrv) = edition.first_version() { let edition_msrv = RustVersion::try_from(edition_msrv).unwrap(); From f96638ea3b5688611d4ec89186979c70497a684c Mon Sep 17 00:00:00 2001 From: Ed Page Date: Tue, 19 Mar 2024 13:40:52 -0500 Subject: [PATCH 04/34] refactor(toml): Rely on resolved description --- crates/cargo-util-schemas/src/manifest/mod.rs | 4 ++++ src/cargo/util/toml/mod.rs | 17 +++++++++-------- 2 files changed, 13 insertions(+), 8 deletions(-) diff --git a/crates/cargo-util-schemas/src/manifest/mod.rs b/crates/cargo-util-schemas/src/manifest/mod.rs index 1ada20b09..d74d503df 100644 --- a/crates/cargo-util-schemas/src/manifest/mod.rs +++ b/crates/cargo-util-schemas/src/manifest/mod.rs @@ -206,6 +206,10 @@ impl TomlPackage { pub fn resolved_version(&self) -> Result, UnresolvedError> { self.version.as_ref().map(|v| v.resolved()).transpose() } + + pub fn resolved_description(&self) -> Result, UnresolvedError> { + self.description.as_ref().map(|v| v.resolved()).transpose() + } } /// An enum that allows for inheriting keys from a workspace in a Cargo.toml. diff --git a/src/cargo/util/toml/mod.rs b/src/cargo/util/toml/mod.rs index 00b6b1430..826717553 100644 --- a/src/cargo/util/toml/mod.rs +++ b/src/cargo/util/toml/mod.rs @@ -562,6 +562,12 @@ pub fn to_real_manifest( .map(|value| field_inherit_with(value, "version", || inherit()?.version())) .transpose()? .map(manifest::InheritableField::Value); + package.description = package + .description + .clone() + .map(|value| field_inherit_with(value, "description", || inherit()?.description())) + .transpose()? + .map(manifest::InheritableField::Value); let rust_version = package .resolved_rust_version() @@ -862,10 +868,9 @@ pub fn to_real_manifest( let metadata = ManifestMetadata { description: package - .description - .clone() - .map(|mw| field_inherit_with(mw, "description", || inherit()?.description())) - .transpose()?, + .resolved_description() + .expect("previously resolved") + .cloned(), homepage: package .homepage .clone() @@ -927,10 +932,6 @@ pub fn to_real_manifest( links: package.links.clone(), rust_version: rust_version.clone(), }; - package.description = metadata - .description - .clone() - .map(|description| manifest::InheritableField::Value(description)); package.homepage = metadata .homepage .clone() From 5b5f64460bf55d5eb99304a2081e1a259baf40a7 Mon Sep 17 00:00:00 2001 From: Ed Page Date: Tue, 19 Mar 2024 13:46:24 -0500 Subject: [PATCH 05/34] refactor(toml): Rely on resolved homepage --- crates/cargo-util-schemas/src/manifest/mod.rs | 4 ++++ src/cargo/util/toml/mod.rs | 17 +++++++++-------- 2 files changed, 13 insertions(+), 8 deletions(-) diff --git a/crates/cargo-util-schemas/src/manifest/mod.rs b/crates/cargo-util-schemas/src/manifest/mod.rs index d74d503df..cf749e0e9 100644 --- a/crates/cargo-util-schemas/src/manifest/mod.rs +++ b/crates/cargo-util-schemas/src/manifest/mod.rs @@ -210,6 +210,10 @@ impl TomlPackage { pub fn resolved_description(&self) -> Result, UnresolvedError> { self.description.as_ref().map(|v| v.resolved()).transpose() } + + pub fn resolved_homepage(&self) -> Result, UnresolvedError> { + self.homepage.as_ref().map(|v| v.resolved()).transpose() + } } /// An enum that allows for inheriting keys from a workspace in a Cargo.toml. diff --git a/src/cargo/util/toml/mod.rs b/src/cargo/util/toml/mod.rs index 826717553..7d4e0a011 100644 --- a/src/cargo/util/toml/mod.rs +++ b/src/cargo/util/toml/mod.rs @@ -568,6 +568,12 @@ pub fn to_real_manifest( .map(|value| field_inherit_with(value, "description", || inherit()?.description())) .transpose()? .map(manifest::InheritableField::Value); + package.homepage = package + .homepage + .clone() + .map(|value| field_inherit_with(value, "homepage", || inherit()?.homepage())) + .transpose()? + .map(manifest::InheritableField::Value); let rust_version = package .resolved_rust_version() @@ -872,10 +878,9 @@ pub fn to_real_manifest( .expect("previously resolved") .cloned(), homepage: package - .homepage - .clone() - .map(|mw| field_inherit_with(mw, "homepage", || inherit()?.homepage())) - .transpose()?, + .resolved_homepage() + .expect("previously resolved") + .cloned(), documentation: package .documentation .clone() @@ -932,10 +937,6 @@ pub fn to_real_manifest( links: package.links.clone(), rust_version: rust_version.clone(), }; - package.homepage = metadata - .homepage - .clone() - .map(|homepage| manifest::InheritableField::Value(homepage)); package.documentation = metadata .documentation .clone() From c62a559d827919714c5665cdfb760816d1e4e094 Mon Sep 17 00:00:00 2001 From: Ed Page Date: Tue, 19 Mar 2024 13:50:33 -0500 Subject: [PATCH 06/34] refactor(toml): Rely on resolved documentation --- crates/cargo-util-schemas/src/manifest/mod.rs | 7 +++++++ src/cargo/util/toml/mod.rs | 17 +++++++++-------- 2 files changed, 16 insertions(+), 8 deletions(-) diff --git a/crates/cargo-util-schemas/src/manifest/mod.rs b/crates/cargo-util-schemas/src/manifest/mod.rs index cf749e0e9..c26b4c130 100644 --- a/crates/cargo-util-schemas/src/manifest/mod.rs +++ b/crates/cargo-util-schemas/src/manifest/mod.rs @@ -214,6 +214,13 @@ impl TomlPackage { pub fn resolved_homepage(&self) -> Result, UnresolvedError> { self.homepage.as_ref().map(|v| v.resolved()).transpose() } + + pub fn resolved_documentation(&self) -> Result, UnresolvedError> { + self.documentation + .as_ref() + .map(|v| v.resolved()) + .transpose() + } } /// An enum that allows for inheriting keys from a workspace in a Cargo.toml. diff --git a/src/cargo/util/toml/mod.rs b/src/cargo/util/toml/mod.rs index 7d4e0a011..a443a5b39 100644 --- a/src/cargo/util/toml/mod.rs +++ b/src/cargo/util/toml/mod.rs @@ -574,6 +574,12 @@ pub fn to_real_manifest( .map(|value| field_inherit_with(value, "homepage", || inherit()?.homepage())) .transpose()? .map(manifest::InheritableField::Value); + package.documentation = package + .documentation + .clone() + .map(|value| field_inherit_with(value, "documentation", || inherit()?.documentation())) + .transpose()? + .map(manifest::InheritableField::Value); let rust_version = package .resolved_rust_version() @@ -882,10 +888,9 @@ pub fn to_real_manifest( .expect("previously resolved") .cloned(), documentation: package - .documentation - .clone() - .map(|mw| field_inherit_with(mw, "documentation", || inherit()?.documentation())) - .transpose()?, + .resolved_documentation() + .expect("previously resolved") + .cloned(), readme: readme_for_package( package_root, package @@ -937,10 +942,6 @@ pub fn to_real_manifest( links: package.links.clone(), rust_version: rust_version.clone(), }; - package.documentation = metadata - .documentation - .clone() - .map(|documentation| manifest::InheritableField::Value(documentation)); package.readme = metadata .readme .clone() From d435d0e72a2a91b105de1acab7c01c30cad2c15d Mon Sep 17 00:00:00 2001 From: Ed Page Date: Tue, 19 Mar 2024 13:58:06 -0500 Subject: [PATCH 07/34] refactor(toml): Rely on resolved readme --- crates/cargo-util-schemas/src/manifest/mod.rs | 12 ++++++++ src/cargo/util/toml/mod.rs | 29 ++++++++++--------- 2 files changed, 27 insertions(+), 14 deletions(-) diff --git a/crates/cargo-util-schemas/src/manifest/mod.rs b/crates/cargo-util-schemas/src/manifest/mod.rs index c26b4c130..121d30af7 100644 --- a/crates/cargo-util-schemas/src/manifest/mod.rs +++ b/crates/cargo-util-schemas/src/manifest/mod.rs @@ -221,6 +221,18 @@ impl TomlPackage { .map(|v| v.resolved()) .transpose() } + + pub fn resolved_readme(&self) -> Result, UnresolvedError> { + self.readme + .as_ref() + .map(|v| { + v.resolved().and_then(|sb| match sb { + StringOrBool::Bool(_) => Err(UnresolvedError), + StringOrBool::String(value) => Ok(value), + }) + }) + .transpose() + } } /// An enum that allows for inheriting keys from a workspace in a Cargo.toml. diff --git a/src/cargo/util/toml/mod.rs b/src/cargo/util/toml/mod.rs index a443a5b39..498835e17 100644 --- a/src/cargo/util/toml/mod.rs +++ b/src/cargo/util/toml/mod.rs @@ -9,8 +9,8 @@ use crate::AlreadyPrintedError; use anyhow::{anyhow, bail, Context as _}; use cargo_platform::Platform; use cargo_util::paths; -use cargo_util_schemas::manifest::RustVersion; use cargo_util_schemas::manifest::{self, TomlManifest}; +use cargo_util_schemas::manifest::{RustVersion, StringOrBool}; use itertools::Itertools; use lazycell::LazyCell; use pathdiff::diff_paths; @@ -580,6 +580,16 @@ pub fn to_real_manifest( .map(|value| field_inherit_with(value, "documentation", || inherit()?.documentation())) .transpose()? .map(manifest::InheritableField::Value); + package.readme = readme_for_package( + package_root, + package + .readme + .clone() + .map(|value| field_inherit_with(value, "readme", || inherit()?.readme(package_root))) + .transpose()? + .as_ref(), + ) + .map(|s| manifest::InheritableField::Value(StringOrBool::String(s))); let rust_version = package .resolved_rust_version() @@ -891,15 +901,10 @@ pub fn to_real_manifest( .resolved_documentation() .expect("previously resolved") .cloned(), - readme: readme_for_package( - package_root, - package - .readme - .clone() - .map(|mw| field_inherit_with(mw, "readme", || inherit()?.readme(package_root))) - .transpose()? - .as_ref(), - ), + readme: package + .resolved_readme() + .expect("previously resolved") + .cloned(), authors: package .authors .clone() @@ -942,10 +947,6 @@ pub fn to_real_manifest( links: package.links.clone(), rust_version: rust_version.clone(), }; - package.readme = metadata - .readme - .clone() - .map(|readme| manifest::InheritableField::Value(manifest::StringOrBool::String(readme))); package.authors = package .authors .as_ref() From 258d8447a9b12130c41ae968e0f334eefd5780e9 Mon Sep 17 00:00:00 2001 From: Ed Page Date: Tue, 19 Mar 2024 14:01:03 -0500 Subject: [PATCH 08/34] refactor(toml): Rely on resolved keywords --- crates/cargo-util-schemas/src/manifest/mod.rs | 4 ++++ src/cargo/util/toml/mod.rs | 17 +++++++++-------- 2 files changed, 13 insertions(+), 8 deletions(-) diff --git a/crates/cargo-util-schemas/src/manifest/mod.rs b/crates/cargo-util-schemas/src/manifest/mod.rs index 121d30af7..d63a2010d 100644 --- a/crates/cargo-util-schemas/src/manifest/mod.rs +++ b/crates/cargo-util-schemas/src/manifest/mod.rs @@ -233,6 +233,10 @@ impl TomlPackage { }) .transpose() } + + pub fn resolved_keywords(&self) -> Result>, UnresolvedError> { + self.keywords.as_ref().map(|v| v.resolved()).transpose() + } } /// An enum that allows for inheriting keys from a workspace in a Cargo.toml. diff --git a/src/cargo/util/toml/mod.rs b/src/cargo/util/toml/mod.rs index 498835e17..35c7a3aa9 100644 --- a/src/cargo/util/toml/mod.rs +++ b/src/cargo/util/toml/mod.rs @@ -590,6 +590,12 @@ pub fn to_real_manifest( .as_ref(), ) .map(|s| manifest::InheritableField::Value(StringOrBool::String(s))); + package.keywords = package + .keywords + .clone() + .map(|value| field_inherit_with(value, "keywords", || inherit()?.keywords())) + .transpose()? + .map(manifest::InheritableField::Value); let rust_version = package .resolved_rust_version() @@ -927,10 +933,9 @@ pub fn to_real_manifest( .map(|mw| field_inherit_with(mw, "repository", || inherit()?.repository())) .transpose()?, keywords: package - .keywords - .clone() - .map(|mw| field_inherit_with(mw, "keywords", || inherit()?.keywords())) - .transpose()? + .resolved_keywords() + .expect("previously resolved") + .cloned() .unwrap_or_default(), categories: package .categories @@ -963,10 +968,6 @@ pub fn to_real_manifest( .repository .clone() .map(|repository| manifest::InheritableField::Value(repository)); - package.keywords = package - .keywords - .as_ref() - .map(|_| manifest::InheritableField::Value(metadata.keywords.clone())); package.categories = package .categories .as_ref() From 047c1fe9d0da2af01d42f973ff9e300eac3f7ac9 Mon Sep 17 00:00:00 2001 From: Ed Page Date: Tue, 19 Mar 2024 14:04:58 -0500 Subject: [PATCH 09/34] refactor(toml): Rely on resolved categories --- crates/cargo-util-schemas/src/manifest/mod.rs | 4 ++++ src/cargo/util/toml/mod.rs | 17 +++++++++-------- 2 files changed, 13 insertions(+), 8 deletions(-) diff --git a/crates/cargo-util-schemas/src/manifest/mod.rs b/crates/cargo-util-schemas/src/manifest/mod.rs index d63a2010d..b58d65276 100644 --- a/crates/cargo-util-schemas/src/manifest/mod.rs +++ b/crates/cargo-util-schemas/src/manifest/mod.rs @@ -237,6 +237,10 @@ impl TomlPackage { pub fn resolved_keywords(&self) -> Result>, UnresolvedError> { self.keywords.as_ref().map(|v| v.resolved()).transpose() } + + pub fn resolved_categories(&self) -> Result>, UnresolvedError> { + self.categories.as_ref().map(|v| v.resolved()).transpose() + } } /// An enum that allows for inheriting keys from a workspace in a Cargo.toml. diff --git a/src/cargo/util/toml/mod.rs b/src/cargo/util/toml/mod.rs index 35c7a3aa9..e2aac30a1 100644 --- a/src/cargo/util/toml/mod.rs +++ b/src/cargo/util/toml/mod.rs @@ -596,6 +596,12 @@ pub fn to_real_manifest( .map(|value| field_inherit_with(value, "keywords", || inherit()?.keywords())) .transpose()? .map(manifest::InheritableField::Value); + package.categories = package + .categories + .clone() + .map(|value| field_inherit_with(value, "categories", || inherit()?.categories())) + .transpose()? + .map(manifest::InheritableField::Value); let rust_version = package .resolved_rust_version() @@ -938,10 +944,9 @@ pub fn to_real_manifest( .cloned() .unwrap_or_default(), categories: package - .categories - .clone() - .map(|mw| field_inherit_with(mw, "categories", || inherit()?.categories())) - .transpose()? + .resolved_categories() + .expect("previously resolved") + .cloned() .unwrap_or_default(), badges: original_toml .badges @@ -968,10 +973,6 @@ pub fn to_real_manifest( .repository .clone() .map(|repository| manifest::InheritableField::Value(repository)); - package.categories = package - .categories - .as_ref() - .map(|_| manifest::InheritableField::Value(metadata.categories.clone())); package.exclude = package .exclude .as_ref() From b942be5bc1dc4c140cb0f97498b915cbd6bbd1ff Mon Sep 17 00:00:00 2001 From: Ed Page Date: Tue, 19 Mar 2024 14:07:40 -0500 Subject: [PATCH 10/34] refactor(toml): Rely on resolved license --- crates/cargo-util-schemas/src/manifest/mod.rs | 4 ++++ src/cargo/util/toml/mod.rs | 17 +++++++++-------- 2 files changed, 13 insertions(+), 8 deletions(-) diff --git a/crates/cargo-util-schemas/src/manifest/mod.rs b/crates/cargo-util-schemas/src/manifest/mod.rs index b58d65276..13bab7b93 100644 --- a/crates/cargo-util-schemas/src/manifest/mod.rs +++ b/crates/cargo-util-schemas/src/manifest/mod.rs @@ -241,6 +241,10 @@ impl TomlPackage { pub fn resolved_categories(&self) -> Result>, UnresolvedError> { self.categories.as_ref().map(|v| v.resolved()).transpose() } + + pub fn resolved_license(&self) -> Result, UnresolvedError> { + self.license.as_ref().map(|v| v.resolved()).transpose() + } } /// An enum that allows for inheriting keys from a workspace in a Cargo.toml. diff --git a/src/cargo/util/toml/mod.rs b/src/cargo/util/toml/mod.rs index e2aac30a1..cd4204fc7 100644 --- a/src/cargo/util/toml/mod.rs +++ b/src/cargo/util/toml/mod.rs @@ -602,6 +602,12 @@ pub fn to_real_manifest( .map(|value| field_inherit_with(value, "categories", || inherit()?.categories())) .transpose()? .map(manifest::InheritableField::Value); + package.license = package + .license + .clone() + .map(|value| field_inherit_with(value, "license", || inherit()?.license())) + .transpose()? + .map(manifest::InheritableField::Value); let rust_version = package .resolved_rust_version() @@ -924,10 +930,9 @@ pub fn to_real_manifest( .transpose()? .unwrap_or_default(), license: package - .license - .clone() - .map(|mw| field_inherit_with(mw, "license", || inherit()?.license())) - .transpose()?, + .resolved_license() + .expect("previously resolved") + .cloned(), license_file: package .license_file .clone() @@ -961,10 +966,6 @@ pub fn to_real_manifest( .authors .as_ref() .map(|_| manifest::InheritableField::Value(metadata.authors.clone())); - package.license = metadata - .license - .clone() - .map(|license| manifest::InheritableField::Value(license)); package.license_file = metadata .license_file .clone() From 18550b251278827639fc89ef5e559b5867c8ac37 Mon Sep 17 00:00:00 2001 From: Ed Page Date: Tue, 19 Mar 2024 14:11:00 -0500 Subject: [PATCH 11/34] refactor(toml): Rely on resolved license-file --- crates/cargo-util-schemas/src/manifest/mod.rs | 4 ++++ src/cargo/util/toml/mod.rs | 21 ++++++++++++------- 2 files changed, 17 insertions(+), 8 deletions(-) diff --git a/crates/cargo-util-schemas/src/manifest/mod.rs b/crates/cargo-util-schemas/src/manifest/mod.rs index 13bab7b93..4c497bfa7 100644 --- a/crates/cargo-util-schemas/src/manifest/mod.rs +++ b/crates/cargo-util-schemas/src/manifest/mod.rs @@ -245,6 +245,10 @@ impl TomlPackage { pub fn resolved_license(&self) -> Result, UnresolvedError> { self.license.as_ref().map(|v| v.resolved()).transpose() } + + pub fn resolved_license_file(&self) -> Result, UnresolvedError> { + self.license_file.as_ref().map(|v| v.resolved()).transpose() + } } /// An enum that allows for inheriting keys from a workspace in a Cargo.toml. diff --git a/src/cargo/util/toml/mod.rs b/src/cargo/util/toml/mod.rs index cd4204fc7..ff3c80042 100644 --- a/src/cargo/util/toml/mod.rs +++ b/src/cargo/util/toml/mod.rs @@ -608,6 +608,16 @@ pub fn to_real_manifest( .map(|value| field_inherit_with(value, "license", || inherit()?.license())) .transpose()? .map(manifest::InheritableField::Value); + package.license_file = package + .license_file + .clone() + .map(|value| { + field_inherit_with(value, "license-file", || { + inherit()?.license_file(package_root) + }) + }) + .transpose()? + .map(manifest::InheritableField::Value); let rust_version = package .resolved_rust_version() @@ -934,10 +944,9 @@ pub fn to_real_manifest( .expect("previously resolved") .cloned(), license_file: package - .license_file - .clone() - .map(|mw| field_inherit_with(mw, "license", || inherit()?.license_file(package_root))) - .transpose()?, + .resolved_license_file() + .expect("previously resolved") + .cloned(), repository: package .repository .clone() @@ -966,10 +975,6 @@ pub fn to_real_manifest( .authors .as_ref() .map(|_| manifest::InheritableField::Value(metadata.authors.clone())); - package.license_file = metadata - .license_file - .clone() - .map(|license_file| manifest::InheritableField::Value(license_file)); package.repository = metadata .repository .clone() From 425a8ae47833388a4066c8dc2a72b0f0b5bede05 Mon Sep 17 00:00:00 2001 From: Ed Page Date: Tue, 19 Mar 2024 14:27:42 -0500 Subject: [PATCH 12/34] refactor(toml): Rely on resolved repository --- crates/cargo-util-schemas/src/manifest/mod.rs | 4 ++++ src/cargo/util/toml/mod.rs | 17 +++++++++-------- 2 files changed, 13 insertions(+), 8 deletions(-) diff --git a/crates/cargo-util-schemas/src/manifest/mod.rs b/crates/cargo-util-schemas/src/manifest/mod.rs index 4c497bfa7..92c3344c7 100644 --- a/crates/cargo-util-schemas/src/manifest/mod.rs +++ b/crates/cargo-util-schemas/src/manifest/mod.rs @@ -249,6 +249,10 @@ impl TomlPackage { pub fn resolved_license_file(&self) -> Result, UnresolvedError> { self.license_file.as_ref().map(|v| v.resolved()).transpose() } + + pub fn resolved_repository(&self) -> Result, UnresolvedError> { + self.repository.as_ref().map(|v| v.resolved()).transpose() + } } /// An enum that allows for inheriting keys from a workspace in a Cargo.toml. diff --git a/src/cargo/util/toml/mod.rs b/src/cargo/util/toml/mod.rs index ff3c80042..8272f6b7d 100644 --- a/src/cargo/util/toml/mod.rs +++ b/src/cargo/util/toml/mod.rs @@ -618,6 +618,12 @@ pub fn to_real_manifest( }) .transpose()? .map(manifest::InheritableField::Value); + package.repository = package + .repository + .clone() + .map(|value| field_inherit_with(value, "repository", || inherit()?.repository())) + .transpose()? + .map(manifest::InheritableField::Value); let rust_version = package .resolved_rust_version() @@ -948,10 +954,9 @@ pub fn to_real_manifest( .expect("previously resolved") .cloned(), repository: package - .repository - .clone() - .map(|mw| field_inherit_with(mw, "repository", || inherit()?.repository())) - .transpose()?, + .resolved_repository() + .expect("previously resolved") + .cloned(), keywords: package .resolved_keywords() .expect("previously resolved") @@ -975,10 +980,6 @@ pub fn to_real_manifest( .authors .as_ref() .map(|_| manifest::InheritableField::Value(metadata.authors.clone())); - package.repository = metadata - .repository - .clone() - .map(|repository| manifest::InheritableField::Value(repository)); package.exclude = package .exclude .as_ref() From 00ba5780e42eeb41f0ba1bae1538f31ef2a33b23 Mon Sep 17 00:00:00 2001 From: Ed Page Date: Tue, 19 Mar 2024 14:32:41 -0500 Subject: [PATCH 13/34] refactor(toml): Rely on resolved authors --- crates/cargo-util-schemas/src/manifest/mod.rs | 4 ++++ src/cargo/util/toml/mod.rs | 17 +++++++++-------- 2 files changed, 13 insertions(+), 8 deletions(-) diff --git a/crates/cargo-util-schemas/src/manifest/mod.rs b/crates/cargo-util-schemas/src/manifest/mod.rs index 92c3344c7..f8844df2e 100644 --- a/crates/cargo-util-schemas/src/manifest/mod.rs +++ b/crates/cargo-util-schemas/src/manifest/mod.rs @@ -207,6 +207,10 @@ impl TomlPackage { self.version.as_ref().map(|v| v.resolved()).transpose() } + pub fn resolved_authors(&self) -> Result>, UnresolvedError> { + self.authors.as_ref().map(|v| v.resolved()).transpose() + } + pub fn resolved_description(&self) -> Result, UnresolvedError> { self.description.as_ref().map(|v| v.resolved()).transpose() } diff --git a/src/cargo/util/toml/mod.rs b/src/cargo/util/toml/mod.rs index 8272f6b7d..eb6973576 100644 --- a/src/cargo/util/toml/mod.rs +++ b/src/cargo/util/toml/mod.rs @@ -562,6 +562,12 @@ pub fn to_real_manifest( .map(|value| field_inherit_with(value, "version", || inherit()?.version())) .transpose()? .map(manifest::InheritableField::Value); + package.authors = package + .authors + .clone() + .map(|value| field_inherit_with(value, "authors", || inherit()?.authors())) + .transpose()? + .map(manifest::InheritableField::Value); package.description = package .description .clone() @@ -940,10 +946,9 @@ pub fn to_real_manifest( .expect("previously resolved") .cloned(), authors: package - .authors - .clone() - .map(|mw| field_inherit_with(mw, "authors", || inherit()?.authors())) - .transpose()? + .resolved_authors() + .expect("previously resolved") + .cloned() .unwrap_or_default(), license: package .resolved_license() @@ -976,10 +981,6 @@ pub fn to_real_manifest( links: package.links.clone(), rust_version: rust_version.clone(), }; - package.authors = package - .authors - .as_ref() - .map(|_| manifest::InheritableField::Value(metadata.authors.clone())); package.exclude = package .exclude .as_ref() From 20302b34b762041fe88aef7e3479126c62481f07 Mon Sep 17 00:00:00 2001 From: Ed Page Date: Tue, 19 Mar 2024 14:56:07 -0500 Subject: [PATCH 14/34] refactor(toml): Rely on resolved include/exclude --- crates/cargo-util-schemas/src/manifest/mod.rs | 8 ++++ src/cargo/util/toml/mod.rs | 43 ++++++++++--------- 2 files changed, 30 insertions(+), 21 deletions(-) diff --git a/crates/cargo-util-schemas/src/manifest/mod.rs b/crates/cargo-util-schemas/src/manifest/mod.rs index f8844df2e..4e2322cd3 100644 --- a/crates/cargo-util-schemas/src/manifest/mod.rs +++ b/crates/cargo-util-schemas/src/manifest/mod.rs @@ -211,6 +211,14 @@ impl TomlPackage { self.authors.as_ref().map(|v| v.resolved()).transpose() } + pub fn resolved_exclude(&self) -> Result>, UnresolvedError> { + self.exclude.as_ref().map(|v| v.resolved()).transpose() + } + + pub fn resolved_include(&self) -> Result>, UnresolvedError> { + self.include.as_ref().map(|v| v.resolved()).transpose() + } + pub fn resolved_description(&self) -> Result, UnresolvedError> { self.description.as_ref().map(|v| v.resolved()).transpose() } diff --git a/src/cargo/util/toml/mod.rs b/src/cargo/util/toml/mod.rs index eb6973576..30b0b9446 100644 --- a/src/cargo/util/toml/mod.rs +++ b/src/cargo/util/toml/mod.rs @@ -568,6 +568,18 @@ pub fn to_real_manifest( .map(|value| field_inherit_with(value, "authors", || inherit()?.authors())) .transpose()? .map(manifest::InheritableField::Value); + package.exclude = package + .exclude + .clone() + .map(|value| field_inherit_with(value, "exclude", || inherit()?.exclude())) + .transpose()? + .map(manifest::InheritableField::Value); + package.include = package + .include + .clone() + .map(|value| field_inherit_with(value, "include", || inherit()?.include())) + .transpose()? + .map(manifest::InheritableField::Value); package.description = package .description .clone() @@ -915,19 +927,6 @@ pub fn to_real_manifest( } } - let exclude = package - .exclude - .clone() - .map(|mw| field_inherit_with(mw, "exclude", || inherit()?.exclude())) - .transpose()? - .unwrap_or_default(); - let include = package - .include - .clone() - .map(|mw| field_inherit_with(mw, "include", || inherit()?.include())) - .transpose()? - .unwrap_or_default(); - let metadata = ManifestMetadata { description: package .resolved_description() @@ -981,14 +980,6 @@ pub fn to_real_manifest( links: package.links.clone(), rust_version: rust_version.clone(), }; - package.exclude = package - .exclude - .as_ref() - .map(|_| manifest::InheritableField::Value(exclude.clone())); - package.include = package - .include - .as_ref() - .map(|_| manifest::InheritableField::Value(include.clone())); if let Some(profiles) = &original_toml.profile { let cli_unstable = gctx.cli_unstable(); @@ -1073,6 +1064,16 @@ pub fn to_real_manifest( .map(|t| CompileTarget::new(&*t)) .transpose()? .map(CompileKind::Target); + let include = package + .resolved_include() + .expect("previously resolved") + .cloned() + .unwrap_or_default(); + let exclude = package + .resolved_exclude() + .expect("previously resolved") + .cloned() + .unwrap_or_default(); let custom_metadata = package.metadata.clone(); let resolved_toml = manifest::TomlManifest { cargo_features: original_toml.cargo_features.clone(), From 2ea1ac6fac590def5c75ea053d2009ae98b12742 Mon Sep 17 00:00:00 2001 From: Ed Page Date: Tue, 19 Mar 2024 15:04:12 -0500 Subject: [PATCH 15/34] refactor(toml): Rely on resolved publish --- crates/cargo-util-schemas/src/manifest/mod.rs | 4 ++++ src/cargo/util/toml/mod.rs | 17 +++++++---------- 2 files changed, 11 insertions(+), 10 deletions(-) diff --git a/crates/cargo-util-schemas/src/manifest/mod.rs b/crates/cargo-util-schemas/src/manifest/mod.rs index 4e2322cd3..d61415f1f 100644 --- a/crates/cargo-util-schemas/src/manifest/mod.rs +++ b/crates/cargo-util-schemas/src/manifest/mod.rs @@ -219,6 +219,10 @@ impl TomlPackage { self.include.as_ref().map(|v| v.resolved()).transpose() } + pub fn resolved_publish(&self) -> Result, UnresolvedError> { + self.publish.as_ref().map(|v| v.resolved()).transpose() + } + pub fn resolved_description(&self) -> Result, UnresolvedError> { self.description.as_ref().map(|v| v.resolved()).transpose() } diff --git a/src/cargo/util/toml/mod.rs b/src/cargo/util/toml/mod.rs index 30b0b9446..863c35c42 100644 --- a/src/cargo/util/toml/mod.rs +++ b/src/cargo/util/toml/mod.rs @@ -580,6 +580,12 @@ pub fn to_real_manifest( .map(|value| field_inherit_with(value, "include", || inherit()?.include())) .transpose()? .map(manifest::InheritableField::Value); + package.publish = package + .publish + .clone() + .map(|value| field_inherit_with(value, "publish", || inherit()?.publish())) + .transpose()? + .map(manifest::InheritableField::Value); package.description = package .description .clone() @@ -986,17 +992,8 @@ pub fn to_real_manifest( validate_profiles(profiles, cli_unstable, &features, warnings)?; } - let publish = package - .publish - .clone() - .map(|publish| field_inherit_with(publish, "publish", || inherit()?.publish()).unwrap()); - - package.publish = publish - .clone() - .map(|p| manifest::InheritableField::Value(p)); - let version = package.resolved_version().expect("previously resolved"); - let publish = match publish { + let publish = match package.resolved_publish().expect("previously resolved") { Some(manifest::VecStringOrBool::VecString(ref vecstring)) => Some(vecstring.clone()), Some(manifest::VecStringOrBool::Bool(false)) => Some(vec![]), Some(manifest::VecStringOrBool::Bool(true)) => None, From a2033965a88b48591d8a159781ee700ae1d846e4 Mon Sep 17 00:00:00 2001 From: Ed Page Date: Tue, 19 Mar 2024 15:19:22 -0500 Subject: [PATCH 16/34] refactor(toml): Directly initialize TomlPackage This will make it easier to evaluate what needs to be resolved in the future. --- src/cargo/util/toml/mod.rs | 318 +++++++++++++++++++++---------------- 1 file changed, 181 insertions(+), 137 deletions(-) diff --git a/src/cargo/util/toml/mod.rs b/src/cargo/util/toml/mod.rs index 863c35c42..5880e0d5f 100644 --- a/src/cargo/util/toml/mod.rs +++ b/src/cargo/util/toml/mod.rs @@ -506,7 +506,7 @@ pub fn to_real_manifest( let cargo_features = original_toml.cargo_features.as_ref().unwrap_or(&empty); let features = Features::new(cargo_features, gctx, warnings, source_id.is_path())?; - let mut package = match (&original_toml.package, &original_toml.project) { + let original_package = match (&original_toml.package, &original_toml.project) { (Some(_), Some(project)) => { warnings.push(format!( "manifest at `{}` contains both `project` and `package`, \ @@ -539,122 +539,145 @@ pub fn to_real_manifest( .try_borrow_with(|| load_inheritable_fields(gctx, manifest_file, &workspace_config)) }; - let package_name = &package.name; + let package_name = &original_package.name; if package_name.contains(':') { features.require(Feature::open_namespaces())?; } - package.edition = package - .edition - .clone() - .map(|value| field_inherit_with(value, "edition", || inherit()?.edition())) - .transpose()? - .map(manifest::InheritableField::Value); - package.rust_version = package - .rust_version - .clone() - .map(|value| field_inherit_with(value, "rust-version", || inherit()?.rust_version())) - .transpose()? - .map(manifest::InheritableField::Value); - package.version = package - .version - .clone() - .map(|value| field_inherit_with(value, "version", || inherit()?.version())) - .transpose()? - .map(manifest::InheritableField::Value); - package.authors = package - .authors - .clone() - .map(|value| field_inherit_with(value, "authors", || inherit()?.authors())) - .transpose()? - .map(manifest::InheritableField::Value); - package.exclude = package - .exclude - .clone() - .map(|value| field_inherit_with(value, "exclude", || inherit()?.exclude())) - .transpose()? - .map(manifest::InheritableField::Value); - package.include = package - .include - .clone() - .map(|value| field_inherit_with(value, "include", || inherit()?.include())) - .transpose()? - .map(manifest::InheritableField::Value); - package.publish = package - .publish - .clone() - .map(|value| field_inherit_with(value, "publish", || inherit()?.publish())) - .transpose()? - .map(manifest::InheritableField::Value); - package.description = package - .description - .clone() - .map(|value| field_inherit_with(value, "description", || inherit()?.description())) - .transpose()? - .map(manifest::InheritableField::Value); - package.homepage = package - .homepage - .clone() - .map(|value| field_inherit_with(value, "homepage", || inherit()?.homepage())) - .transpose()? - .map(manifest::InheritableField::Value); - package.documentation = package - .documentation - .clone() - .map(|value| field_inherit_with(value, "documentation", || inherit()?.documentation())) - .transpose()? - .map(manifest::InheritableField::Value); - package.readme = readme_for_package( - package_root, - package - .readme + let resolved_package = manifest::TomlPackage { + edition: original_package + .edition .clone() - .map(|value| field_inherit_with(value, "readme", || inherit()?.readme(package_root))) + .map(|value| field_inherit_with(value, "edition", || inherit()?.edition())) .transpose()? - .as_ref(), - ) - .map(|s| manifest::InheritableField::Value(StringOrBool::String(s))); - package.keywords = package - .keywords - .clone() - .map(|value| field_inherit_with(value, "keywords", || inherit()?.keywords())) - .transpose()? - .map(manifest::InheritableField::Value); - package.categories = package - .categories - .clone() - .map(|value| field_inherit_with(value, "categories", || inherit()?.categories())) - .transpose()? - .map(manifest::InheritableField::Value); - package.license = package - .license - .clone() - .map(|value| field_inherit_with(value, "license", || inherit()?.license())) - .transpose()? - .map(manifest::InheritableField::Value); - package.license_file = package - .license_file - .clone() - .map(|value| { - field_inherit_with(value, "license-file", || { - inherit()?.license_file(package_root) + .map(manifest::InheritableField::Value), + rust_version: original_package + .rust_version + .clone() + .map(|value| field_inherit_with(value, "rust-version", || inherit()?.rust_version())) + .transpose()? + .map(manifest::InheritableField::Value), + name: original_package.name.clone(), + version: original_package + .version + .clone() + .map(|value| field_inherit_with(value, "version", || inherit()?.version())) + .transpose()? + .map(manifest::InheritableField::Value), + authors: original_package + .authors + .clone() + .map(|value| field_inherit_with(value, "authors", || inherit()?.authors())) + .transpose()? + .map(manifest::InheritableField::Value), + build: original_package.build.clone(), + metabuild: original_package.metabuild.clone(), + default_target: original_package.default_target.clone(), + forced_target: original_package.forced_target.clone(), + links: original_package.links.clone(), + exclude: original_package + .exclude + .clone() + .map(|value| field_inherit_with(value, "exclude", || inherit()?.exclude())) + .transpose()? + .map(manifest::InheritableField::Value), + include: original_package + .include + .clone() + .map(|value| field_inherit_with(value, "include", || inherit()?.include())) + .transpose()? + .map(manifest::InheritableField::Value), + publish: original_package + .publish + .clone() + .map(|value| field_inherit_with(value, "publish", || inherit()?.publish())) + .transpose()? + .map(manifest::InheritableField::Value), + workspace: original_package.workspace.clone(), + im_a_teapot: original_package.im_a_teapot.clone(), + autobins: original_package.autobins.clone(), + autoexamples: original_package.autoexamples.clone(), + autotests: original_package.autotests.clone(), + autobenches: original_package.autobenches.clone(), + default_run: original_package.default_run.clone(), + description: original_package + .description + .clone() + .map(|value| field_inherit_with(value, "description", || inherit()?.description())) + .transpose()? + .map(manifest::InheritableField::Value), + homepage: original_package + .homepage + .clone() + .map(|value| field_inherit_with(value, "homepage", || inherit()?.homepage())) + .transpose()? + .map(manifest::InheritableField::Value), + documentation: original_package + .documentation + .clone() + .map(|value| field_inherit_with(value, "documentation", || inherit()?.documentation())) + .transpose()? + .map(manifest::InheritableField::Value), + readme: readme_for_package( + package_root, + original_package + .readme + .clone() + .map(|value| { + field_inherit_with(value, "readme", || inherit()?.readme(package_root)) + }) + .transpose()? + .as_ref(), + ) + .map(|s| manifest::InheritableField::Value(StringOrBool::String(s))), + keywords: original_package + .keywords + .clone() + .map(|value| field_inherit_with(value, "keywords", || inherit()?.keywords())) + .transpose()? + .map(manifest::InheritableField::Value), + categories: original_package + .categories + .clone() + .map(|value| field_inherit_with(value, "categories", || inherit()?.categories())) + .transpose()? + .map(manifest::InheritableField::Value), + license: original_package + .license + .clone() + .map(|value| field_inherit_with(value, "license", || inherit()?.license())) + .transpose()? + .map(manifest::InheritableField::Value), + license_file: original_package + .license_file + .clone() + .map(|value| { + field_inherit_with(value, "license-file", || { + inherit()?.license_file(package_root) + }) }) - }) - .transpose()? - .map(manifest::InheritableField::Value); - package.repository = package - .repository - .clone() - .map(|value| field_inherit_with(value, "repository", || inherit()?.repository())) - .transpose()? - .map(manifest::InheritableField::Value); + .transpose()? + .map(manifest::InheritableField::Value), + repository: original_package + .repository + .clone() + .map(|value| field_inherit_with(value, "repository", || inherit()?.repository())) + .transpose()? + .map(manifest::InheritableField::Value), + resolver: original_package.resolver.clone(), + metadata: original_package.metadata.clone(), + _invalid_cargo_features: Default::default(), + }; - let rust_version = package + let rust_version = resolved_package .resolved_rust_version() .expect("previously resolved") .cloned(); - let edition = if let Some(edition) = package.resolved_edition().expect("previously resolved") { + let edition = if let Some(edition) = resolved_package + .resolved_edition() + .expect("previously resolved") + { let edition: Edition = edition .parse() .with_context(|| "failed to parse the `edition` key")?; @@ -726,12 +749,12 @@ pub fn to_real_manifest( ))); } - if package.metabuild.is_some() { + if resolved_package.metabuild.is_some() { features.require(Feature::metabuild())?; } let resolve_behavior = match ( - package.resolver.as_ref(), + resolved_package.resolver.as_ref(), original_toml .workspace .as_ref() @@ -753,8 +776,8 @@ pub fn to_real_manifest( package_name, package_root, edition, - &package.build, - &package.metabuild, + &resolved_package.build, + &resolved_package.metabuild, warnings, errors, )?; @@ -783,7 +806,7 @@ pub fn to_real_manifest( }) } - if let Some(links) = &package.links { + if let Some(links) = &resolved_package.links { if !targets.iter().any(|t| t.is_custom_build()) { bail!("package specifies that it links to `{links}` but does not have a custom build script") } @@ -934,45 +957,45 @@ pub fn to_real_manifest( } let metadata = ManifestMetadata { - description: package + description: resolved_package .resolved_description() .expect("previously resolved") .cloned(), - homepage: package + homepage: resolved_package .resolved_homepage() .expect("previously resolved") .cloned(), - documentation: package + documentation: resolved_package .resolved_documentation() .expect("previously resolved") .cloned(), - readme: package + readme: resolved_package .resolved_readme() .expect("previously resolved") .cloned(), - authors: package + authors: resolved_package .resolved_authors() .expect("previously resolved") .cloned() .unwrap_or_default(), - license: package + license: resolved_package .resolved_license() .expect("previously resolved") .cloned(), - license_file: package + license_file: resolved_package .resolved_license_file() .expect("previously resolved") .cloned(), - repository: package + repository: resolved_package .resolved_repository() .expect("previously resolved") .cloned(), - keywords: package + keywords: resolved_package .resolved_keywords() .expect("previously resolved") .cloned() .unwrap_or_default(), - categories: package + categories: resolved_package .resolved_categories() .expect("previously resolved") .cloned() @@ -983,7 +1006,7 @@ pub fn to_real_manifest( .map(|mw| field_inherit_with(mw, "badges", || inherit()?.badges())) .transpose()? .unwrap_or_default(), - links: package.links.clone(), + links: resolved_package.links.clone(), rust_version: rust_version.clone(), }; @@ -992,8 +1015,13 @@ pub fn to_real_manifest( validate_profiles(profiles, cli_unstable, &features, warnings)?; } - let version = package.resolved_version().expect("previously resolved"); - let publish = match package.resolved_publish().expect("previously resolved") { + let version = resolved_package + .resolved_version() + .expect("previously resolved"); + let publish = match resolved_package + .resolved_publish() + .expect("previously resolved") + { Some(manifest::VecStringOrBool::VecString(ref vecstring)) => Some(vecstring.clone()), Some(manifest::VecStringOrBool::Bool(false)) => Some(vec![]), Some(manifest::VecStringOrBool::Bool(true)) => None, @@ -1005,7 +1033,7 @@ pub fn to_real_manifest( } let pkgid = PackageId::new( - package.name.as_str().into(), + resolved_package.name.as_str().into(), version .cloned() .unwrap_or_else(|| semver::Version::new(0, 0, 0)), @@ -1026,7 +1054,7 @@ pub fn to_real_manifest( ) }) .collect(), - package.links.as_deref(), + resolved_package.links.as_deref(), rust_version.clone(), )?; if summary.features().contains_key("default-features") { @@ -1037,7 +1065,7 @@ pub fn to_real_manifest( ) } - if let Some(run) = &package.default_run { + if let Some(run) = &resolved_package.default_run { if !targets .iter() .filter(|t| t.is_bin()) @@ -1049,32 +1077,36 @@ pub fn to_real_manifest( } } - let default_kind = package + let default_kind = resolved_package .default_target .as_ref() .map(|t| CompileTarget::new(&*t)) .transpose()? .map(CompileKind::Target); - let forced_kind = package + let forced_kind = resolved_package .forced_target .as_ref() .map(|t| CompileTarget::new(&*t)) .transpose()? .map(CompileKind::Target); - let include = package + let include = resolved_package .resolved_include() .expect("previously resolved") .cloned() .unwrap_or_default(); - let exclude = package + let exclude = resolved_package .resolved_exclude() .expect("previously resolved") .cloned() .unwrap_or_default(); - let custom_metadata = package.metadata.clone(); + let links = resolved_package.links.clone(); + let custom_metadata = resolved_package.metadata.clone(); + let im_a_teapot = resolved_package.im_a_teapot; + let default_run = resolved_package.default_run.clone(); + let metabuild = resolved_package.metabuild.clone().map(|sov| sov.0); let resolved_toml = manifest::TomlManifest { cargo_features: original_toml.cargo_features.clone(), - package: Some(package.clone()), + package: Some(Box::new(resolved_package)), project: None, profile: original_toml.profile.clone(), lib: original_toml.lib.clone(), @@ -1113,7 +1145,7 @@ pub fn to_real_manifest( targets, exclude, include, - package.links.clone(), + links, metadata, custom_metadata, publish, @@ -1123,14 +1155,26 @@ pub fn to_real_manifest( features, edition, rust_version, - package.im_a_teapot, - package.default_run.clone(), - package.metabuild.clone().map(|sov| sov.0), + im_a_teapot, + default_run, + metabuild, resolve_behavior, rustflags, embedded, ); - if package.license_file.is_some() && package.license.is_some() { + if manifest + .resolved_toml() + .package() + .unwrap() + .license_file + .is_some() + && manifest + .resolved_toml() + .package() + .unwrap() + .license + .is_some() + { warnings.push( "only one of `license` or `license-file` is necessary\n\ `license` should be used if the package license can be expressed \ From 772539a03a11ff950c16e1b5858b3e8eba094666 Mon Sep 17 00:00:00 2001 From: Ed Page Date: Tue, 19 Mar 2024 15:28:20 -0500 Subject: [PATCH 17/34] refactor(toml): Group resolving of lints with package We can't have validation depend on `TomlManifest::resolved_lints` yet because we need to pull out the resolving of deps first. --- crates/cargo-util-schemas/src/manifest/mod.rs | 14 ++++++++++++++ src/cargo/util/toml/mod.rs | 16 ++++++++-------- 2 files changed, 22 insertions(+), 8 deletions(-) diff --git a/crates/cargo-util-schemas/src/manifest/mod.rs b/crates/cargo-util-schemas/src/manifest/mod.rs index d61415f1f..4494fad45 100644 --- a/crates/cargo-util-schemas/src/manifest/mod.rs +++ b/crates/cargo-util-schemas/src/manifest/mod.rs @@ -105,6 +105,10 @@ impl TomlManifest { pub fn features(&self) -> Option<&BTreeMap>> { self.features.as_ref() } + + pub fn resolved_lints(&self) -> Result, UnresolvedError> { + self.lints.as_ref().map(|l| l.resolved()).transpose() + } } #[derive(Debug, Deserialize, Serialize, Clone)] @@ -1378,6 +1382,16 @@ pub struct InheritableLints { pub lints: TomlLints, } +impl InheritableLints { + pub fn resolved(&self) -> Result<&TomlLints, UnresolvedError> { + if self.workspace { + Err(UnresolvedError) + } else { + Ok(&self.lints) + } + } +} + fn is_false(b: &bool) -> bool { !b } diff --git a/src/cargo/util/toml/mod.rs b/src/cargo/util/toml/mod.rs index 5880e0d5f..14007b28f 100644 --- a/src/cargo/util/toml/mod.rs +++ b/src/cargo/util/toml/mod.rs @@ -668,6 +668,11 @@ pub fn to_real_manifest( metadata: original_package.metadata.clone(), _invalid_cargo_features: Default::default(), }; + let resolved_lints = original_toml + .lints + .clone() + .map(|value| lints_inherit_with(value, || inherit()?.lints())) + .transpose()?; let rust_version = resolved_package .resolved_rust_version() @@ -865,14 +870,9 @@ pub fn to_real_manifest( &inherit_cell, )?; - let lints = original_toml - .lints - .clone() - .map(|mw| lints_inherit_with(mw, || inherit()?.lints())) - .transpose()?; - verify_lints(lints.as_ref(), gctx, manifest_ctx.warnings)?; + verify_lints(resolved_lints.as_ref(), gctx, manifest_ctx.warnings)?; let default = manifest::TomlLints::default(); - let rustflags = lints_to_rustflags(lints.as_ref().unwrap_or(&default)); + let rustflags = lints_to_rustflags(resolved_lints.as_ref().unwrap_or(&default)); let mut target: BTreeMap = BTreeMap::new(); for (name, platform) in original_toml.target.iter().flatten() { @@ -1128,7 +1128,7 @@ pub fn to_real_manifest( .badges .as_ref() .map(|_| manifest::InheritableField::Value(metadata.badges.clone())), - lints: lints.map(|lints| manifest::InheritableLints { + lints: resolved_lints.map(|lints| manifest::InheritableLints { workspace: false, lints, }), From b517320f1af53c945caa9d4eaab630d229b6bdd3 Mon Sep 17 00:00:00 2001 From: Ed Page Date: Tue, 19 Mar 2024 15:50:00 -0500 Subject: [PATCH 18/34] refactor(toml): Remove ManifestContext from default_feature_msg --- src/cargo/util/toml/mod.rs | 14 +++++--------- 1 file changed, 5 insertions(+), 9 deletions(-) diff --git a/src/cargo/util/toml/mod.rs b/src/cargo/util/toml/mod.rs index 14007b28f..5ad804721 100644 --- a/src/cargo/util/toml/mod.rs +++ b/src/cargo/util/toml/mod.rs @@ -1893,17 +1893,13 @@ fn inner_dependency_inherit_with<'a>( inheritable: impl FnOnce() -> CargoResult<&'a InheritableFields>, manifest_ctx: &mut ManifestContext<'_, '_>, ) -> CargoResult { - fn default_features_msg( - label: &str, - ws_def_feat: Option, - manifest_ctx: &mut ManifestContext<'_, '_>, - ) { + fn default_features_msg(label: &str, ws_def_feat: Option, warnings: &mut Vec) { let ws_def_feat = match ws_def_feat { Some(true) => "true", Some(false) => "false", None => "not specified", }; - manifest_ctx.warnings.push(format!( + warnings.push(format!( "`default-features` is ignored for {label}, since `default-features` was \ {ws_def_feat} for `workspace.dependencies.{label}`, \ this could become a hard error in the future" @@ -1923,7 +1919,7 @@ fn inner_dependency_inherit_with<'a>( match d { manifest::TomlDependency::Simple(s) => { if let Some(false) = dependency.default_features() { - default_features_msg(name, None, manifest_ctx); + default_features_msg(name, None, &mut manifest_ctx.warnings); } if dependency.optional.is_some() || dependency.features.is_some() @@ -1953,12 +1949,12 @@ fn inner_dependency_inherit_with<'a>( // workspace: default-features = true should ignore member // default-features (Some(false), Some(true)) => { - default_features_msg(name, Some(true), manifest_ctx); + default_features_msg(name, Some(true), &mut manifest_ctx.warnings); } // member: default-features = false and // workspace: dep = "1.0" should ignore member default-features (Some(false), None) => { - default_features_msg(name, None, manifest_ctx); + default_features_msg(name, None, &mut manifest_ctx.warnings); } _ => {} } From 21ca8ab6474fa22e9f9ae03288ec6ff4fbab8fae Mon Sep 17 00:00:00 2001 From: Ed Page Date: Tue, 19 Mar 2024 15:52:29 -0500 Subject: [PATCH 19/34] refactor(toml): Remove ManifestContext from dependency_inherit_with --- src/cargo/util/toml/mod.rs | 137 +++++++++++++++++++------------------ 1 file changed, 69 insertions(+), 68 deletions(-) diff --git a/src/cargo/util/toml/mod.rs b/src/cargo/util/toml/mod.rs index 5ad804721..1c7eb1687 100644 --- a/src/cargo/util/toml/mod.rs +++ b/src/cargo/util/toml/mod.rs @@ -1217,7 +1217,13 @@ fn resolve_and_validate_dependencies( let mut deps: BTreeMap = BTreeMap::new(); for (name_in_toml, v) in dependencies.iter() { - let resolved = dependency_inherit_with(v.clone(), name_in_toml, inheritable, manifest_ctx)?; + let resolved = dependency_inherit_with( + v.clone(), + name_in_toml, + inheritable, + &manifest_ctx.root, + &mut manifest_ctx.warnings, + )?; let kind_name = match kind { Some(k) => k.kind_table(), None => "dependencies", @@ -1873,12 +1879,13 @@ fn dependency_inherit_with<'a>( dependency: manifest::InheritableDependency, name: &str, inheritable: impl FnOnce() -> CargoResult<&'a InheritableFields>, - manifest_ctx: &mut ManifestContext<'_, '_>, + package_root: &Path, + warnings: &mut Vec, ) -> CargoResult { match dependency { manifest::InheritableDependency::Value(value) => Ok(value), manifest::InheritableDependency::Inherit(w) => { - inner_dependency_inherit_with(w, name, inheritable, manifest_ctx).with_context(|| { + inner_dependency_inherit_with(w, name, inheritable, package_root, warnings).with_context(|| { format!( "error inheriting `{name}` from workspace root manifest's `workspace.dependencies.{name}`", ) @@ -1891,7 +1898,8 @@ fn inner_dependency_inherit_with<'a>( dependency: manifest::TomlInheritedDependency, name: &str, inheritable: impl FnOnce() -> CargoResult<&'a InheritableFields>, - manifest_ctx: &mut ManifestContext<'_, '_>, + package_root: &Path, + warnings: &mut Vec, ) -> CargoResult { fn default_features_msg(label: &str, ws_def_feat: Option, warnings: &mut Vec) { let ws_def_feat = match ws_def_feat { @@ -1906,74 +1914,67 @@ fn inner_dependency_inherit_with<'a>( )) } if dependency.default_features.is_some() && dependency.default_features2.is_some() { - warn_on_deprecated( - "default-features", - name, - "dependency", - manifest_ctx.warnings, - ); + warn_on_deprecated("default-features", name, "dependency", warnings); } - inheritable()? - .get_dependency(name, manifest_ctx.root) - .map(|d| { - match d { - manifest::TomlDependency::Simple(s) => { - if let Some(false) = dependency.default_features() { - default_features_msg(name, None, &mut manifest_ctx.warnings); - } - if dependency.optional.is_some() - || dependency.features.is_some() - || dependency.public.is_some() - { - manifest::TomlDependency::Detailed(manifest::TomlDetailedDependency { - version: Some(s), - optional: dependency.optional, - features: dependency.features.clone(), - public: dependency.public, - ..Default::default() - }) - } else { - manifest::TomlDependency::Simple(s) - } + inheritable()?.get_dependency(name, package_root).map(|d| { + match d { + manifest::TomlDependency::Simple(s) => { + if let Some(false) = dependency.default_features() { + default_features_msg(name, None, warnings); } - manifest::TomlDependency::Detailed(d) => { - let mut d = d.clone(); - match (dependency.default_features(), d.default_features()) { - // member: default-features = true and - // workspace: default-features = false should turn on - // default-features - (Some(true), Some(false)) => { - d.default_features = Some(true); - } - // member: default-features = false and - // workspace: default-features = true should ignore member - // default-features - (Some(false), Some(true)) => { - default_features_msg(name, Some(true), &mut manifest_ctx.warnings); - } - // member: default-features = false and - // workspace: dep = "1.0" should ignore member default-features - (Some(false), None) => { - default_features_msg(name, None, &mut manifest_ctx.warnings); - } - _ => {} - } - d.features = match (d.features.clone(), dependency.features.clone()) { - (Some(dep_feat), Some(inherit_feat)) => Some( - dep_feat - .into_iter() - .chain(inherit_feat) - .collect::>(), - ), - (Some(dep_fet), None) => Some(dep_fet), - (None, Some(inherit_feat)) => Some(inherit_feat), - (None, None) => None, - }; - d.optional = dependency.optional; - manifest::TomlDependency::Detailed(d) + if dependency.optional.is_some() + || dependency.features.is_some() + || dependency.public.is_some() + { + manifest::TomlDependency::Detailed(manifest::TomlDetailedDependency { + version: Some(s), + optional: dependency.optional, + features: dependency.features.clone(), + public: dependency.public, + ..Default::default() + }) + } else { + manifest::TomlDependency::Simple(s) } } - }) + manifest::TomlDependency::Detailed(d) => { + let mut d = d.clone(); + match (dependency.default_features(), d.default_features()) { + // member: default-features = true and + // workspace: default-features = false should turn on + // default-features + (Some(true), Some(false)) => { + d.default_features = Some(true); + } + // member: default-features = false and + // workspace: default-features = true should ignore member + // default-features + (Some(false), Some(true)) => { + default_features_msg(name, Some(true), warnings); + } + // member: default-features = false and + // workspace: dep = "1.0" should ignore member default-features + (Some(false), None) => { + default_features_msg(name, None, warnings); + } + _ => {} + } + d.features = match (d.features.clone(), dependency.features.clone()) { + (Some(dep_feat), Some(inherit_feat)) => Some( + dep_feat + .into_iter() + .chain(inherit_feat) + .collect::>(), + ), + (Some(dep_fet), None) => Some(dep_fet), + (None, Some(inherit_feat)) => Some(inherit_feat), + (None, None) => None, + }; + d.optional = dependency.optional; + manifest::TomlDependency::Detailed(d) + } + } + }) } pub(crate) fn to_dependency( From 611b6889a6cc87b802eb3107fa2970bb1283f7ec Mon Sep 17 00:00:00 2001 From: Ed Page Date: Tue, 19 Mar 2024 16:21:08 -0500 Subject: [PATCH 20/34] refactor(toml): Separate resolve/validate dependencies --- crates/cargo-util-schemas/src/manifest/mod.rs | 7 + src/cargo/core/workspace.rs | 1 - src/cargo/util/toml/mod.rs | 334 +++++++++++------- 3 files changed, 223 insertions(+), 119 deletions(-) diff --git a/crates/cargo-util-schemas/src/manifest/mod.rs b/crates/cargo-util-schemas/src/manifest/mod.rs index 4494fad45..dc3948cf2 100644 --- a/crates/cargo-util-schemas/src/manifest/mod.rs +++ b/crates/cargo-util-schemas/src/manifest/mod.rs @@ -589,6 +589,13 @@ impl InheritableDependency { InheritableDependency::Inherit(w) => w._unused_keys.keys().cloned().collect(), } } + + pub fn resolved(&self) -> Result<&TomlDependency, UnresolvedError> { + match self { + InheritableDependency::Value(d) => Ok(d), + InheritableDependency::Inherit(_) => Err(UnresolvedError), + } + } } impl<'de> de::Deserialize<'de> for InheritableDependency { diff --git a/src/cargo/core/workspace.rs b/src/cargo/core/workspace.rs index d05b802cf..9cf4a100f 100644 --- a/src/cargo/core/workspace.rs +++ b/src/cargo/core/workspace.rs @@ -449,7 +449,6 @@ impl<'gctx> Workspace<'gctx> { // NOTE: Since we use ConfigRelativePath, this root isn't used as // any relative paths are resolved before they'd be joined with root. Path::new("unused-relative-path"), - self.unstable_features(), /* kind */ None, ) }) diff --git a/src/cargo/util/toml/mod.rs b/src/cargo/util/toml/mod.rs index 1c7eb1687..1e400080d 100644 --- a/src/cargo/util/toml/mod.rs +++ b/src/cargo/util/toml/mod.rs @@ -668,6 +668,86 @@ pub fn to_real_manifest( metadata: original_package.metadata.clone(), _invalid_cargo_features: Default::default(), }; + let resolved_dependencies = resolve_dependencies( + gctx, + &features, + manifest_file, + original_toml.dependencies.as_ref(), + None, + &workspace_config, + &inherit_cell, + package_root, + warnings, + )?; + let resolved_dev_dependencies = resolve_dependencies( + gctx, + &features, + manifest_file, + original_toml.dev_dependencies(), + Some(DepKind::Development), + &workspace_config, + &inherit_cell, + package_root, + warnings, + )?; + let resolved_build_dependencies = resolve_dependencies( + gctx, + &features, + manifest_file, + original_toml.build_dependencies(), + Some(DepKind::Build), + &workspace_config, + &inherit_cell, + package_root, + warnings, + )?; + let mut resolved_target = BTreeMap::new(); + for (name, platform) in original_toml.target.iter().flatten() { + let resolved_dependencies = resolve_dependencies( + gctx, + &features, + manifest_file, + platform.dependencies.as_ref(), + None, + &workspace_config, + &inherit_cell, + package_root, + warnings, + )?; + let resolved_dev_dependencies = resolve_dependencies( + gctx, + &features, + manifest_file, + platform.dev_dependencies(), + Some(DepKind::Development), + &workspace_config, + &inherit_cell, + package_root, + warnings, + )?; + let resolved_build_dependencies = resolve_dependencies( + gctx, + &features, + manifest_file, + platform.build_dependencies(), + Some(DepKind::Build), + &workspace_config, + &inherit_cell, + package_root, + warnings, + )?; + resolved_target.insert( + name.clone(), + manifest::TomlPlatform { + dependencies: resolved_dependencies, + build_dependencies: resolved_build_dependencies, + build_dependencies2: None, + dev_dependencies: resolved_dev_dependencies, + dev_dependencies2: None, + }, + ); + } + let resolved_target = (!resolved_target.is_empty()).then_some(resolved_target); let resolved_lints = original_toml .lints .clone() @@ -824,19 +904,18 @@ pub fn to_real_manifest( source_id, gctx, warnings, - features: &features, platform: None, root: package_root, }; // Collect the dependencies. - let dependencies = resolve_and_validate_dependencies( + validate_dependencies( &mut manifest_ctx, original_toml.dependencies.as_ref(), None, - &workspace_config, - &inherit_cell, + None, )?; + gather_dependencies(&mut manifest_ctx, resolved_dependencies.as_ref(), None)?; if original_toml.dev_dependencies.is_some() && original_toml.dev_dependencies2.is_some() { warn_on_deprecated( "dev-dependencies", @@ -845,13 +924,16 @@ pub fn to_real_manifest( manifest_ctx.warnings, ); } - let dev_deps = original_toml.dev_dependencies(); - let dev_deps = resolve_and_validate_dependencies( + validate_dependencies( &mut manifest_ctx, - dev_deps, + original_toml.dev_dependencies(), + None, + Some(DepKind::Development), + )?; + gather_dependencies( + &mut manifest_ctx, + resolved_dev_dependencies.as_ref(), Some(DepKind::Development), - &workspace_config, - &inherit_cell, )?; if original_toml.build_dependencies.is_some() && original_toml.build_dependencies2.is_some() { warn_on_deprecated( @@ -861,32 +943,31 @@ pub fn to_real_manifest( manifest_ctx.warnings, ); } - let build_deps = original_toml.build_dependencies(); - let build_deps = resolve_and_validate_dependencies( + validate_dependencies( &mut manifest_ctx, - build_deps, + original_toml.build_dependencies(), + None, + Some(DepKind::Build), + )?; + gather_dependencies( + &mut manifest_ctx, + resolved_build_dependencies.as_ref(), Some(DepKind::Build), - &workspace_config, - &inherit_cell, )?; verify_lints(resolved_lints.as_ref(), gctx, manifest_ctx.warnings)?; let default = manifest::TomlLints::default(); let rustflags = lints_to_rustflags(resolved_lints.as_ref().unwrap_or(&default)); - let mut target: BTreeMap = BTreeMap::new(); for (name, platform) in original_toml.target.iter().flatten() { - manifest_ctx.platform = { - let platform: Platform = name.parse()?; - platform.check_cfg_attributes(manifest_ctx.warnings); - Some(platform) - }; - let deps = resolve_and_validate_dependencies( + let platform_kind: Platform = name.parse()?; + platform_kind.check_cfg_attributes(manifest_ctx.warnings); + let platform_kind = Some(platform_kind); + validate_dependencies( &mut manifest_ctx, platform.dependencies.as_ref(), + platform_kind.as_ref(), None, - &workspace_config, - &inherit_cell, )?; if platform.build_dependencies.is_some() && platform.build_dependencies2.is_some() { warn_on_deprecated( @@ -896,13 +977,11 @@ pub fn to_real_manifest( manifest_ctx.warnings, ); } - let build_deps = platform.build_dependencies(); - let build_deps = resolve_and_validate_dependencies( + validate_dependencies( &mut manifest_ctx, - build_deps, + platform.build_dependencies(), + platform_kind.as_ref(), Some(DepKind::Build), - &workspace_config, - &inherit_cell, )?; if platform.dev_dependencies.is_some() && platform.dev_dependencies2.is_some() { warn_on_deprecated( @@ -912,31 +991,28 @@ pub fn to_real_manifest( manifest_ctx.warnings, ); } - let dev_deps = platform.dev_dependencies(); - let dev_deps = resolve_and_validate_dependencies( + validate_dependencies( &mut manifest_ctx, - dev_deps, + platform.dev_dependencies(), + platform_kind.as_ref(), + Some(DepKind::Development), + )?; + } + for (name, platform) in resolved_target.iter().flatten() { + manifest_ctx.platform = Some(name.parse()?); + gather_dependencies(&mut manifest_ctx, platform.dependencies.as_ref(), None)?; + gather_dependencies( + &mut manifest_ctx, + platform.build_dependencies(), + Some(DepKind::Build), + )?; + gather_dependencies( + &mut manifest_ctx, + platform.dev_dependencies(), Some(DepKind::Development), - &workspace_config, - &inherit_cell, )?; - target.insert( - name.clone(), - manifest::TomlPlatform { - dependencies: deps, - build_dependencies: build_deps, - build_dependencies2: None, - dev_dependencies: dev_deps, - dev_dependencies2: None, - }, - ); } - let target = if target.is_empty() { - None - } else { - Some(target) - }; let replace = replace(&original_toml, &mut manifest_ctx)?; let patch = patch(&original_toml, &mut manifest_ctx)?; @@ -1114,13 +1190,13 @@ pub fn to_real_manifest( example: original_toml.example.clone(), test: original_toml.test.clone(), bench: original_toml.bench.clone(), - dependencies, - dev_dependencies: dev_deps, + dependencies: resolved_dependencies, + dev_dependencies: resolved_dev_dependencies, dev_dependencies2: None, - build_dependencies: build_deps, + build_dependencies: resolved_build_dependencies, build_dependencies2: None, features: original_toml.features.clone(), - target, + target: resolved_target, replace: original_toml.replace.clone(), patch: original_toml.patch.clone(), workspace: original_toml.workspace.clone(), @@ -1192,43 +1268,90 @@ pub fn to_real_manifest( Ok(manifest) } -#[tracing::instrument(skip(manifest_ctx, new_deps, workspace_config, inherit_cell))] -fn resolve_and_validate_dependencies( - manifest_ctx: &mut ManifestContext<'_, '_>, - new_deps: Option<&BTreeMap>, +#[tracing::instrument(skip_all)] +fn resolve_dependencies( + gctx: &GlobalContext, + features: &Features, + manifest_file: &Path, + orig_deps: Option<&BTreeMap>, kind: Option, workspace_config: &WorkspaceConfig, inherit_cell: &LazyCell, + package_root: &Path, + warnings: &mut Vec, ) -> CargoResult>> { - let Some(dependencies) = new_deps else { + let Some(dependencies) = orig_deps else { return Ok(None); }; let inheritable = || { - inherit_cell.try_borrow_with(|| { - load_inheritable_fields( - manifest_ctx.gctx, - &manifest_ctx.root.join("Cargo.toml"), - &workspace_config, - ) - }) + inherit_cell + .try_borrow_with(|| load_inheritable_fields(gctx, manifest_file, &workspace_config)) + }; + + let mut deps = BTreeMap::new(); + for (name_in_toml, v) in dependencies.iter() { + let mut resolved = + dependency_inherit_with(v.clone(), name_in_toml, inheritable, package_root, warnings)?; + if let manifest::TomlDependency::Detailed(ref mut d) = resolved { + if d.public.is_some() { + let public_feature = features.require(Feature::public_dependency()); + let with_public_feature = public_feature.is_ok(); + let with_z_public = gctx.cli_unstable().public_dependency; + if !with_public_feature && (!with_z_public && !gctx.nightly_features_allowed) { + public_feature?; + } + if matches!(kind, None) { + if !with_public_feature && !with_z_public { + d.public = None; + warnings.push(format!( + "ignoring `public` on dependency {name_in_toml}, pass `-Zpublic-dependency` to enable support for it" + )) + } + } else { + let kind_name = match kind { + Some(k) => k.kind_table(), + None => "dependencies", + }; + let hint = format!( + "'public' specifier can only be used on regular dependencies, not {kind_name}", + ); + if with_public_feature || with_z_public { + bail!(hint) + } else { + // If public feature isn't enabled in nightly, we instead warn that. + warnings.push(hint); + d.public = None; + } + } + } + } + + deps.insert( + name_in_toml.clone(), + manifest::InheritableDependency::Value(resolved.clone()), + ); + } + Ok(Some(deps)) +} + +#[tracing::instrument(skip_all)] +fn validate_dependencies( + manifest_ctx: &mut ManifestContext<'_, '_>, + original_deps: Option<&BTreeMap>, + platform: Option<&Platform>, + kind: Option, +) -> CargoResult<()> { + let Some(dependencies) = original_deps else { + return Ok(()); }; - let mut deps: BTreeMap = - BTreeMap::new(); for (name_in_toml, v) in dependencies.iter() { - let resolved = dependency_inherit_with( - v.clone(), - name_in_toml, - inheritable, - &manifest_ctx.root, - &mut manifest_ctx.warnings, - )?; let kind_name = match kind { Some(k) => k.kind_table(), None => "dependencies", }; - let table_in_toml = if let Some(platform) = &manifest_ctx.platform { + let table_in_toml = if let Some(platform) = platform { format!("target.{}.{kind_name}", platform.to_string()) } else { kind_name.to_string() @@ -1239,47 +1362,26 @@ fn resolve_and_validate_dependencies( v.unused_keys(), manifest_ctx.warnings, ); - let mut resolved = resolved; - if let manifest::TomlDependency::Detailed(ref mut d) = resolved { - if d.public.is_some() { - let public_feature = manifest_ctx.features.require(Feature::public_dependency()); - let with_public_feature = public_feature.is_ok(); - let with_z_public = manifest_ctx.gctx.cli_unstable().public_dependency; - if !with_public_feature - && (!with_z_public && !manifest_ctx.gctx.nightly_features_allowed) - { - public_feature?; - } - if matches!(kind, None) { - if !with_public_feature && !with_z_public { - d.public = None; - manifest_ctx.warnings.push(format!( - "ignoring `public` on dependency {name_in_toml}, pass `-Zpublic-dependency` to enable support for it" - )) - } - } else { - let hint = format!( - "'public' specifier can only be used on regular dependencies, not {kind_name}", - ); - if with_public_feature || with_z_public { - bail!(hint) - } else { - // If public feature isn't enabled in nightly, we instead warn that. - manifest_ctx.warnings.push(hint); - d.public = None; - } - } - } - } - - let dep = dep_to_dependency(&resolved, name_in_toml, manifest_ctx, kind)?; - manifest_ctx.deps.push(dep); - deps.insert( - name_in_toml.clone(), - manifest::InheritableDependency::Value(resolved.clone()), - ); } - Ok(Some(deps)) + Ok(()) +} + +#[tracing::instrument(skip_all)] +fn gather_dependencies( + manifest_ctx: &mut ManifestContext<'_, '_>, + resolved_deps: Option<&BTreeMap>, + kind: Option, +) -> CargoResult<()> { + let Some(dependencies) = resolved_deps else { + return Ok(()); + }; + + for (n, v) in dependencies.iter() { + let resolved = v.resolved().expect("previously resolved"); + let dep = dep_to_dependency(&resolved, n, manifest_ctx, kind)?; + manifest_ctx.deps.push(dep); + } + Ok(()) } fn to_workspace_config( @@ -1375,7 +1477,6 @@ fn to_virtual_manifest( gctx, warnings, platform: None, - features: &features, root, }; ( @@ -1509,7 +1610,6 @@ struct ManifestContext<'a, 'b> { warnings: &'a mut Vec, platform: Option, root: &'a Path, - features: &'a Features, } fn verify_lints( @@ -1985,7 +2085,6 @@ pub(crate) fn to_dependency( warnings: &mut Vec, platform: Option, root: &Path, - features: &Features, kind: Option, ) -> CargoResult { dep_to_dependency( @@ -1998,7 +2097,6 @@ pub(crate) fn to_dependency( warnings, platform, root, - features, }, kind, ) From 3d1da63222e19cd8397f175cb626714e0dc0bdae Mon Sep 17 00:00:00 2001 From: Ed Page Date: Mon, 25 Mar 2024 20:16:00 -0500 Subject: [PATCH 21/34] refactor(toml): Rely on resolved badges --- src/cargo/util/toml/mod.rs | 17 +++++++---------- 1 file changed, 7 insertions(+), 10 deletions(-) diff --git a/src/cargo/util/toml/mod.rs b/src/cargo/util/toml/mod.rs index 1e400080d..95cb73497 100644 --- a/src/cargo/util/toml/mod.rs +++ b/src/cargo/util/toml/mod.rs @@ -1032,6 +1032,11 @@ pub fn to_real_manifest( } } + let resolved_badges = original_toml + .badges + .clone() + .map(|mw| field_inherit_with(mw, "badges", || inherit()?.badges())) + .transpose()?; let metadata = ManifestMetadata { description: resolved_package .resolved_description() @@ -1076,12 +1081,7 @@ pub fn to_real_manifest( .expect("previously resolved") .cloned() .unwrap_or_default(), - badges: original_toml - .badges - .clone() - .map(|mw| field_inherit_with(mw, "badges", || inherit()?.badges())) - .transpose()? - .unwrap_or_default(), + badges: resolved_badges.clone().unwrap_or_default(), links: resolved_package.links.clone(), rust_version: rust_version.clone(), }; @@ -1200,10 +1200,7 @@ pub fn to_real_manifest( replace: original_toml.replace.clone(), patch: original_toml.patch.clone(), workspace: original_toml.workspace.clone(), - badges: original_toml - .badges - .as_ref() - .map(|_| manifest::InheritableField::Value(metadata.badges.clone())), + badges: resolved_badges.map(manifest::InheritableField::Value), lints: resolved_lints.map(|lints| manifest::InheritableLints { workspace: false, lints, From 5d8bdf4f41732cf053e4f1ce97e3f861fd4c95d8 Mon Sep 17 00:00:00 2001 From: Ed Page Date: Tue, 26 Mar 2024 16:42:10 -0500 Subject: [PATCH 22/34] refactor(toml): Separate resolving from other in same fn --- crates/cargo-util-schemas/src/manifest/mod.rs | 6 ++ src/cargo/util/toml/mod.rs | 94 +++++++++++-------- 2 files changed, 61 insertions(+), 39 deletions(-) diff --git a/crates/cargo-util-schemas/src/manifest/mod.rs b/crates/cargo-util-schemas/src/manifest/mod.rs index dc3948cf2..57e97e964 100644 --- a/crates/cargo-util-schemas/src/manifest/mod.rs +++ b/crates/cargo-util-schemas/src/manifest/mod.rs @@ -106,6 +106,12 @@ impl TomlManifest { self.features.as_ref() } + pub fn resolved_badges( + &self, + ) -> Result>>, UnresolvedError> { + self.badges.as_ref().map(|l| l.resolved()).transpose() + } + pub fn resolved_lints(&self) -> Result, UnresolvedError> { self.lints.as_ref().map(|l| l.resolved()).transpose() } diff --git a/src/cargo/util/toml/mod.rs b/src/cargo/util/toml/mod.rs index 95cb73497..178a285b8 100644 --- a/src/cargo/util/toml/mod.rs +++ b/src/cargo/util/toml/mod.rs @@ -753,7 +753,42 @@ pub fn to_real_manifest( .clone() .map(|value| lints_inherit_with(value, || inherit()?.lints())) .transpose()?; + let resolved_badges = original_toml + .badges + .clone() + .map(|mw| field_inherit_with(mw, "badges", || inherit()?.badges())) + .transpose()?; + let resolved_toml = manifest::TomlManifest { + cargo_features: original_toml.cargo_features.clone(), + package: Some(Box::new(resolved_package)), + project: None, + profile: original_toml.profile.clone(), + lib: original_toml.lib.clone(), + bin: original_toml.bin.clone(), + example: original_toml.example.clone(), + test: original_toml.test.clone(), + bench: original_toml.bench.clone(), + dependencies: resolved_dependencies, + dev_dependencies: resolved_dev_dependencies, + dev_dependencies2: None, + build_dependencies: resolved_build_dependencies, + build_dependencies2: None, + features: original_toml.features.clone(), + target: resolved_target, + replace: original_toml.replace.clone(), + patch: original_toml.patch.clone(), + workspace: original_toml.workspace.clone(), + badges: resolved_badges.map(manifest::InheritableField::Value), + lints: resolved_lints.map(|lints| manifest::InheritableLints { + workspace: false, + lints, + }), + _unused_keys: Default::default(), + }; + let resolved_package = resolved_toml + .package() + .expect("previously verified to have a `[package]`"); let rust_version = resolved_package .resolved_rust_version() .expect("previously resolved") @@ -915,7 +950,7 @@ pub fn to_real_manifest( None, None, )?; - gather_dependencies(&mut manifest_ctx, resolved_dependencies.as_ref(), None)?; + gather_dependencies(&mut manifest_ctx, resolved_toml.dependencies.as_ref(), None)?; if original_toml.dev_dependencies.is_some() && original_toml.dev_dependencies2.is_some() { warn_on_deprecated( "dev-dependencies", @@ -932,7 +967,7 @@ pub fn to_real_manifest( )?; gather_dependencies( &mut manifest_ctx, - resolved_dev_dependencies.as_ref(), + resolved_toml.dev_dependencies(), Some(DepKind::Development), )?; if original_toml.build_dependencies.is_some() && original_toml.build_dependencies2.is_some() { @@ -951,13 +986,22 @@ pub fn to_real_manifest( )?; gather_dependencies( &mut manifest_ctx, - resolved_build_dependencies.as_ref(), + resolved_toml.build_dependencies(), Some(DepKind::Build), )?; - verify_lints(resolved_lints.as_ref(), gctx, manifest_ctx.warnings)?; + verify_lints( + resolved_toml.resolved_lints().expect("previously resolved"), + gctx, + manifest_ctx.warnings, + )?; let default = manifest::TomlLints::default(); - let rustflags = lints_to_rustflags(resolved_lints.as_ref().unwrap_or(&default)); + let rustflags = lints_to_rustflags( + resolved_toml + .resolved_lints() + .expect("previously resolved") + .unwrap_or(&default), + ); for (name, platform) in original_toml.target.iter().flatten() { let platform_kind: Platform = name.parse()?; @@ -998,7 +1042,7 @@ pub fn to_real_manifest( Some(DepKind::Development), )?; } - for (name, platform) in resolved_target.iter().flatten() { + for (name, platform) in resolved_toml.target.iter().flatten() { manifest_ctx.platform = Some(name.parse()?); gather_dependencies(&mut manifest_ctx, platform.dependencies.as_ref(), None)?; gather_dependencies( @@ -1032,11 +1076,6 @@ pub fn to_real_manifest( } } - let resolved_badges = original_toml - .badges - .clone() - .map(|mw| field_inherit_with(mw, "badges", || inherit()?.badges())) - .transpose()?; let metadata = ManifestMetadata { description: resolved_package .resolved_description() @@ -1081,7 +1120,11 @@ pub fn to_real_manifest( .expect("previously resolved") .cloned() .unwrap_or_default(), - badges: resolved_badges.clone().unwrap_or_default(), + badges: resolved_toml + .resolved_badges() + .expect("previously resolved") + .cloned() + .unwrap_or_default(), links: resolved_package.links.clone(), rust_version: rust_version.clone(), }; @@ -1180,33 +1223,6 @@ pub fn to_real_manifest( let im_a_teapot = resolved_package.im_a_teapot; let default_run = resolved_package.default_run.clone(); let metabuild = resolved_package.metabuild.clone().map(|sov| sov.0); - let resolved_toml = manifest::TomlManifest { - cargo_features: original_toml.cargo_features.clone(), - package: Some(Box::new(resolved_package)), - project: None, - profile: original_toml.profile.clone(), - lib: original_toml.lib.clone(), - bin: original_toml.bin.clone(), - example: original_toml.example.clone(), - test: original_toml.test.clone(), - bench: original_toml.bench.clone(), - dependencies: resolved_dependencies, - dev_dependencies: resolved_dev_dependencies, - dev_dependencies2: None, - build_dependencies: resolved_build_dependencies, - build_dependencies2: None, - features: original_toml.features.clone(), - target: resolved_target, - replace: original_toml.replace.clone(), - patch: original_toml.patch.clone(), - workspace: original_toml.workspace.clone(), - badges: resolved_badges.map(manifest::InheritableField::Value), - lints: resolved_lints.map(|lints| manifest::InheritableLints { - workspace: false, - lints, - }), - _unused_keys: Default::default(), - }; let manifest = Manifest::new( Rc::new(contents), Rc::new(document), From 0f5b562e620ee7ecaef724b2ab40bc0db84c3e5b Mon Sep 17 00:00:00 2001 From: Ed Page Date: Mon, 1 Apr 2024 16:56:52 -0500 Subject: [PATCH 23/34] refactor(toml): Centralize cargo-features parsing --- src/cargo/util/toml/mod.rs | 21 +++++++++++---------- 1 file changed, 11 insertions(+), 10 deletions(-) diff --git a/src/cargo/util/toml/mod.rs b/src/cargo/util/toml/mod.rs index 178a285b8..b1b0e4853 100644 --- a/src/cargo/util/toml/mod.rs +++ b/src/cargo/util/toml/mod.rs @@ -59,11 +59,16 @@ pub fn read_manifest( .map_err(|e| emit_diagnostic(e.into(), &contents, path, gctx))?; let mut manifest = (|| { + let empty = Vec::new(); + let cargo_features = original_toml.cargo_features.as_ref().unwrap_or(&empty); + let features = Features::new(cargo_features, gctx, &mut warnings, source_id.is_path())?; + if original_toml.package().is_some() { to_real_manifest( contents, document, original_toml, + features, source_id, path, gctx, @@ -76,6 +81,7 @@ pub fn read_manifest( contents, document, original_toml, + features, source_id, path, gctx, @@ -222,6 +228,7 @@ pub fn prepare_for_publish(me: &Package, ws: &Workspace<'_>) -> CargoResult) -> CargoResult, original_toml: manifest::TomlManifest, + features: Features, source_id: SourceId, manifest_file: &Path, gctx: &GlobalContext, @@ -501,11 +510,6 @@ pub fn to_real_manifest( ); }; - // Parse features first so they will be available when parsing other parts of the TOML. - let empty = Vec::new(); - let cargo_features = original_toml.cargo_features.as_ref().unwrap_or(&empty); - let features = Features::new(cargo_features, gctx, warnings, source_id.is_path())?; - let original_package = match (&original_toml.package, &original_toml.project) { (Some(_), Some(project)) => { warnings.push(format!( @@ -1462,6 +1466,7 @@ fn to_virtual_manifest( contents: String, document: toml_edit::ImDocument, original_toml: manifest::TomlManifest, + features: Features, source_id: SourceId, manifest_file: &Path, gctx: &GlobalContext, @@ -1476,13 +1481,9 @@ fn to_virtual_manifest( bail!("this virtual manifest specifies a `{field}` section, which is not allowed"); } - let mut deps = Vec::new(); - let empty = Vec::new(); - let cargo_features = original_toml.cargo_features.as_ref().unwrap_or(&empty); - let features = Features::new(cargo_features, gctx, warnings, source_id.is_path())?; - resolved_toml._unused_keys = Default::default(); + let mut deps = Vec::new(); let (replace, patch) = { let mut manifest_ctx = ManifestContext { deps: &mut deps, From c921f52b4f6016ce5ca03caddeb8eb84d2fde130 Mon Sep 17 00:00:00 2001 From: Ed Page Date: Mon, 1 Apr 2024 20:30:59 -0500 Subject: [PATCH 24/34] refactor(toml): Centralize creation of workspace_config --- src/cargo/util/toml/mod.rs | 33 ++++++++++++++++++--------------- 1 file changed, 18 insertions(+), 15 deletions(-) diff --git a/src/cargo/util/toml/mod.rs b/src/cargo/util/toml/mod.rs index b1b0e4853..d4e547983 100644 --- a/src/cargo/util/toml/mod.rs +++ b/src/cargo/util/toml/mod.rs @@ -62,6 +62,13 @@ pub fn read_manifest( let empty = Vec::new(); let cargo_features = original_toml.cargo_features.as_ref().unwrap_or(&empty); let features = Features::new(cargo_features, gctx, &mut warnings, source_id.is_path())?; + let workspace_config = to_workspace_config(&original_toml, path, gctx, &mut warnings)?; + if let WorkspaceConfig::Root(ws_root_config) = &workspace_config { + let package_root = path.parent().unwrap(); + gctx.ws_roots + .borrow_mut() + .insert(package_root.to_owned(), ws_root_config.clone()); + } if original_toml.package().is_some() { to_real_manifest( @@ -69,6 +76,7 @@ pub fn read_manifest( document, original_toml, features, + workspace_config, source_id, path, gctx, @@ -82,6 +90,7 @@ pub fn read_manifest( document, original_toml, features, + workspace_config, source_id, path, gctx, @@ -229,6 +238,7 @@ pub fn prepare_for_publish(me: &Package, ws: &Workspace<'_>) -> CargoResult) -> CargoResult, original_toml: manifest::TomlManifest, features: Features, + workspace_config: WorkspaceConfig, source_id: SourceId, manifest_file: &Path, gctx: &GlobalContext, @@ -531,12 +543,6 @@ pub fn to_real_manifest( (None, None) => bail!("no `package` section found"), }; - let workspace_config = to_workspace_config(&original_toml, package_root, gctx, warnings)?; - if let WorkspaceConfig::Root(ws_root_config) = &workspace_config { - gctx.ws_roots - .borrow_mut() - .insert(package_root.to_owned(), ws_root_config.clone()); - } let inherit_cell: LazyCell = LazyCell::new(); let inherit = || { inherit_cell @@ -1403,7 +1409,7 @@ fn gather_dependencies( fn to_workspace_config( original_toml: &manifest::TomlManifest, - package_root: &Path, + manifest_file: &Path, gctx: &GlobalContext, warnings: &mut Vec, ) -> CargoResult { @@ -1427,7 +1433,7 @@ fn to_workspace_config( unused_dep_keys(name, "workspace.dependencies", dep.unused_keys(), warnings); } } - let ws_root_config = to_workspace_root_config(toml_config, package_root); + let ws_root_config = to_workspace_root_config(toml_config, manifest_file); WorkspaceConfig::Root(ws_root_config) } (None, root) => WorkspaceConfig::Member { @@ -1443,8 +1449,9 @@ fn to_workspace_config( fn to_workspace_root_config( resolved_toml: &manifest::TomlWorkspace, - package_root: &Path, + manifest_file: &Path, ) -> WorkspaceRootConfig { + let package_root = manifest_file.parent().unwrap(); let inheritable = InheritableFields { package: resolved_toml.package.clone(), dependencies: resolved_toml.dependencies.clone(), @@ -1467,6 +1474,7 @@ fn to_virtual_manifest( document: toml_edit::ImDocument, original_toml: manifest::TomlManifest, features: Features, + workspace_config: WorkspaceConfig, source_id: SourceId, manifest_file: &Path, gctx: &GlobalContext, @@ -1507,12 +1515,7 @@ fn to_virtual_manifest( .and_then(|ws| ws.resolver.as_deref()) .map(|r| ResolveBehavior::from_manifest(r)) .transpose()?; - let workspace_config = to_workspace_config(&original_toml, root, gctx, warnings)?; - if let WorkspaceConfig::Root(ws_root_config) = &workspace_config { - gctx.ws_roots - .borrow_mut() - .insert(root.to_owned(), ws_root_config.clone()); - } else { + if let WorkspaceConfig::Member { .. } = &workspace_config { bail!("virtual manifests must be configured with [workspace]"); } let manifest = VirtualManifest::new( From 9cc5b9093251b7cfa551c43d8ed74d3690a7b78a Mon Sep 17 00:00:00 2001 From: Ed Page Date: Mon, 1 Apr 2024 21:10:28 -0500 Subject: [PATCH 25/34] refactor(toml): Extract resolve_toml --- src/cargo/util/toml/mod.rs | 371 ++++++++++++++++++++----------------- 1 file changed, 203 insertions(+), 168 deletions(-) diff --git a/src/cargo/util/toml/mod.rs b/src/cargo/util/toml/mod.rs index d4e547983..18a81c30c 100644 --- a/src/cargo/util/toml/mod.rs +++ b/src/cargo/util/toml/mod.rs @@ -69,12 +69,22 @@ pub fn read_manifest( .borrow_mut() .insert(package_root.to_owned(), ws_root_config.clone()); } + let resolved_toml = resolve_toml( + &original_toml, + &features, + &workspace_config, + path, + gctx, + &mut warnings, + &mut errors, + )?; - if original_toml.package().is_some() { + if resolved_toml.package().is_some() { to_real_manifest( contents, document, original_toml, + resolved_toml, features, workspace_config, source_id, @@ -89,6 +99,7 @@ pub fn read_manifest( contents, document, original_toml, + resolved_toml, features, workspace_config, source_id, @@ -236,7 +247,8 @@ fn warn_on_unused(unused: &BTreeSet, warnings: &mut Vec) { pub fn prepare_for_publish(me: &Package, ws: &Workspace<'_>) -> CargoResult { let contents = me.manifest().contents(); let document = me.manifest().document(); - let toml_manifest = prepare_toml_for_publish(me.manifest().resolved_toml(), ws, me.root())?; + let original_toml = prepare_toml_for_publish(me.manifest().resolved_toml(), ws, me.root())?; + let resolved_toml = original_toml.clone(); let features = me.manifest().unstable_features().clone(); let workspace_config = me.manifest().workspace_config().clone(); let source_id = me.package_id().source_id(); @@ -246,7 +258,8 @@ pub fn prepare_for_publish(me: &Package, ws: &Workspace<'_>) -> CargoResult, - original_toml: manifest::TomlManifest, - features: Features, - workspace_config: WorkspaceConfig, - source_id: SourceId, +fn resolve_toml( + original_toml: &manifest::TomlManifest, + features: &Features, + workspace_config: &WorkspaceConfig, manifest_file: &Path, gctx: &GlobalContext, warnings: &mut Vec, - errors: &mut Vec, -) -> CargoResult { - let embedded = is_embedded(manifest_file); + _errors: &mut Vec, +) -> CargoResult { let package_root = manifest_file.parent().unwrap(); - if !package_root.is_dir() { - bail!( - "package root '{}' is not a directory", - package_root.display() - ); - }; - - let original_package = match (&original_toml.package, &original_toml.project) { - (Some(_), Some(project)) => { - warnings.push(format!( - "manifest at `{}` contains both `project` and `package`, \ - this could become a hard error in the future", - package_root.display() - )); - project.clone() - } - (Some(package), None) => package.clone(), - (None, Some(project)) => { - warnings.push(format!( - "manifest at `{}` contains `[project]` instead of `[package]`, \ - this could become a hard error in the future", - package_root.display() - )); - project.clone() - } - (None, None) => bail!("no `package` section found"), - }; let inherit_cell: LazyCell = LazyCell::new(); let inherit = || { @@ -549,134 +531,138 @@ pub fn to_real_manifest( .try_borrow_with(|| load_inheritable_fields(gctx, manifest_file, &workspace_config)) }; - let package_name = &original_package.name; - if package_name.contains(':') { - features.require(Feature::open_namespaces())?; - } - - let resolved_package = manifest::TomlPackage { - edition: original_package - .edition - .clone() - .map(|value| field_inherit_with(value, "edition", || inherit()?.edition())) - .transpose()? - .map(manifest::InheritableField::Value), - rust_version: original_package - .rust_version - .clone() - .map(|value| field_inherit_with(value, "rust-version", || inherit()?.rust_version())) - .transpose()? - .map(manifest::InheritableField::Value), - name: original_package.name.clone(), - version: original_package - .version - .clone() - .map(|value| field_inherit_with(value, "version", || inherit()?.version())) - .transpose()? - .map(manifest::InheritableField::Value), - authors: original_package - .authors - .clone() - .map(|value| field_inherit_with(value, "authors", || inherit()?.authors())) - .transpose()? - .map(manifest::InheritableField::Value), - build: original_package.build.clone(), - metabuild: original_package.metabuild.clone(), - default_target: original_package.default_target.clone(), - forced_target: original_package.forced_target.clone(), - links: original_package.links.clone(), - exclude: original_package - .exclude - .clone() - .map(|value| field_inherit_with(value, "exclude", || inherit()?.exclude())) - .transpose()? - .map(manifest::InheritableField::Value), - include: original_package - .include - .clone() - .map(|value| field_inherit_with(value, "include", || inherit()?.include())) - .transpose()? - .map(manifest::InheritableField::Value), - publish: original_package - .publish - .clone() - .map(|value| field_inherit_with(value, "publish", || inherit()?.publish())) - .transpose()? - .map(manifest::InheritableField::Value), - workspace: original_package.workspace.clone(), - im_a_teapot: original_package.im_a_teapot.clone(), - autobins: original_package.autobins.clone(), - autoexamples: original_package.autoexamples.clone(), - autotests: original_package.autotests.clone(), - autobenches: original_package.autobenches.clone(), - default_run: original_package.default_run.clone(), - description: original_package - .description - .clone() - .map(|value| field_inherit_with(value, "description", || inherit()?.description())) - .transpose()? - .map(manifest::InheritableField::Value), - homepage: original_package - .homepage - .clone() - .map(|value| field_inherit_with(value, "homepage", || inherit()?.homepage())) - .transpose()? - .map(manifest::InheritableField::Value), - documentation: original_package - .documentation - .clone() - .map(|value| field_inherit_with(value, "documentation", || inherit()?.documentation())) - .transpose()? - .map(manifest::InheritableField::Value), - readme: readme_for_package( - package_root, - original_package - .readme + let resolved_package = if let Some(original_package) = original_toml.package() { + let resolved_package = manifest::TomlPackage { + edition: original_package + .edition + .clone() + .map(|value| field_inherit_with(value, "edition", || inherit()?.edition())) + .transpose()? + .map(manifest::InheritableField::Value), + rust_version: original_package + .rust_version .clone() .map(|value| { - field_inherit_with(value, "readme", || inherit()?.readme(package_root)) + field_inherit_with(value, "rust-version", || inherit()?.rust_version()) }) .transpose()? - .as_ref(), - ) - .map(|s| manifest::InheritableField::Value(StringOrBool::String(s))), - keywords: original_package - .keywords - .clone() - .map(|value| field_inherit_with(value, "keywords", || inherit()?.keywords())) - .transpose()? - .map(manifest::InheritableField::Value), - categories: original_package - .categories - .clone() - .map(|value| field_inherit_with(value, "categories", || inherit()?.categories())) - .transpose()? - .map(manifest::InheritableField::Value), - license: original_package - .license - .clone() - .map(|value| field_inherit_with(value, "license", || inherit()?.license())) - .transpose()? - .map(manifest::InheritableField::Value), - license_file: original_package - .license_file - .clone() - .map(|value| { - field_inherit_with(value, "license-file", || { - inherit()?.license_file(package_root) + .map(manifest::InheritableField::Value), + name: original_package.name.clone(), + version: original_package + .version + .clone() + .map(|value| field_inherit_with(value, "version", || inherit()?.version())) + .transpose()? + .map(manifest::InheritableField::Value), + authors: original_package + .authors + .clone() + .map(|value| field_inherit_with(value, "authors", || inherit()?.authors())) + .transpose()? + .map(manifest::InheritableField::Value), + build: original_package.build.clone(), + metabuild: original_package.metabuild.clone(), + default_target: original_package.default_target.clone(), + forced_target: original_package.forced_target.clone(), + links: original_package.links.clone(), + exclude: original_package + .exclude + .clone() + .map(|value| field_inherit_with(value, "exclude", || inherit()?.exclude())) + .transpose()? + .map(manifest::InheritableField::Value), + include: original_package + .include + .clone() + .map(|value| field_inherit_with(value, "include", || inherit()?.include())) + .transpose()? + .map(manifest::InheritableField::Value), + publish: original_package + .publish + .clone() + .map(|value| field_inherit_with(value, "publish", || inherit()?.publish())) + .transpose()? + .map(manifest::InheritableField::Value), + workspace: original_package.workspace.clone(), + im_a_teapot: original_package.im_a_teapot.clone(), + autobins: original_package.autobins.clone(), + autoexamples: original_package.autoexamples.clone(), + autotests: original_package.autotests.clone(), + autobenches: original_package.autobenches.clone(), + default_run: original_package.default_run.clone(), + description: original_package + .description + .clone() + .map(|value| field_inherit_with(value, "description", || inherit()?.description())) + .transpose()? + .map(manifest::InheritableField::Value), + homepage: original_package + .homepage + .clone() + .map(|value| field_inherit_with(value, "homepage", || inherit()?.homepage())) + .transpose()? + .map(manifest::InheritableField::Value), + documentation: original_package + .documentation + .clone() + .map(|value| { + field_inherit_with(value, "documentation", || inherit()?.documentation()) }) - }) - .transpose()? - .map(manifest::InheritableField::Value), - repository: original_package - .repository - .clone() - .map(|value| field_inherit_with(value, "repository", || inherit()?.repository())) - .transpose()? - .map(manifest::InheritableField::Value), - resolver: original_package.resolver.clone(), - metadata: original_package.metadata.clone(), - _invalid_cargo_features: Default::default(), + .transpose()? + .map(manifest::InheritableField::Value), + readme: readme_for_package( + package_root, + original_package + .readme + .clone() + .map(|value| { + field_inherit_with(value, "readme", || inherit()?.readme(package_root)) + }) + .transpose()? + .as_ref(), + ) + .map(|s| manifest::InheritableField::Value(StringOrBool::String(s))), + keywords: original_package + .keywords + .clone() + .map(|value| field_inherit_with(value, "keywords", || inherit()?.keywords())) + .transpose()? + .map(manifest::InheritableField::Value), + categories: original_package + .categories + .clone() + .map(|value| field_inherit_with(value, "categories", || inherit()?.categories())) + .transpose()? + .map(manifest::InheritableField::Value), + license: original_package + .license + .clone() + .map(|value| field_inherit_with(value, "license", || inherit()?.license())) + .transpose()? + .map(manifest::InheritableField::Value), + license_file: original_package + .license_file + .clone() + .map(|value| { + field_inherit_with(value, "license-file", || { + inherit()?.license_file(package_root) + }) + }) + .transpose()? + .map(manifest::InheritableField::Value), + repository: original_package + .repository + .clone() + .map(|value| field_inherit_with(value, "repository", || inherit()?.repository())) + .transpose()? + .map(manifest::InheritableField::Value), + resolver: original_package.resolver.clone(), + metadata: original_package.metadata.clone(), + _invalid_cargo_features: Default::default(), + }; + Some(Box::new(resolved_package)) + } else { + None }; let resolved_dependencies = resolve_dependencies( gctx, @@ -770,7 +756,7 @@ pub fn to_real_manifest( .transpose()?; let resolved_toml = manifest::TomlManifest { cargo_features: original_toml.cargo_features.clone(), - package: Some(Box::new(resolved_package)), + package: resolved_package, project: None, profile: original_toml.profile.clone(), lib: original_toml.lib.clone(), @@ -796,6 +782,58 @@ pub fn to_real_manifest( _unused_keys: Default::default(), }; + Ok(resolved_toml) +} + +#[tracing::instrument(skip_all)] +pub fn to_real_manifest( + contents: String, + document: toml_edit::ImDocument, + original_toml: manifest::TomlManifest, + resolved_toml: manifest::TomlManifest, + features: Features, + workspace_config: WorkspaceConfig, + source_id: SourceId, + manifest_file: &Path, + gctx: &GlobalContext, + warnings: &mut Vec, + errors: &mut Vec, +) -> CargoResult { + let embedded = is_embedded(manifest_file); + let package_root = manifest_file.parent().unwrap(); + if !package_root.is_dir() { + bail!( + "package root '{}' is not a directory", + package_root.display() + ); + }; + + let original_package = match (&original_toml.package, &original_toml.project) { + (Some(_), Some(project)) => { + warnings.push(format!( + "manifest at `{}` contains both `project` and `package`, \ + this could become a hard error in the future", + package_root.display() + )); + project.clone() + } + (Some(package), None) => package.clone(), + (None, Some(project)) => { + warnings.push(format!( + "manifest at `{}` contains `[project]` instead of `[package]`, \ + this could become a hard error in the future", + package_root.display() + )); + project.clone() + } + (None, None) => bail!("no `package` section found"), + }; + + let package_name = &original_package.name; + if package_name.contains(':') { + features.require(Feature::open_namespaces())?; + } + let resolved_package = resolved_toml .package() .expect("previously verified to have a `[package]`"); @@ -1473,6 +1511,7 @@ fn to_virtual_manifest( contents: String, document: toml_edit::ImDocument, original_toml: manifest::TomlManifest, + resolved_toml: manifest::TomlManifest, features: Features, workspace_config: WorkspaceConfig, source_id: SourceId, @@ -1483,14 +1522,10 @@ fn to_virtual_manifest( ) -> CargoResult { let root = manifest_file.parent().unwrap(); - let mut resolved_toml = original_toml.clone(); - for field in original_toml.requires_package() { bail!("this virtual manifest specifies a `{field}` section, which is not allowed"); } - resolved_toml._unused_keys = Default::default(); - let mut deps = Vec::new(); let (replace, patch) = { let mut manifest_ctx = ManifestContext { From e9d28d88139ec8f71420d603d97ef5c8ea6c263b Mon Sep 17 00:00:00 2001 From: Ed Page Date: Mon, 1 Apr 2024 21:25:06 -0500 Subject: [PATCH 26/34] refactor(toml): Simplify how we pass around inheritable data --- src/cargo/util/toml/mod.rs | 45 +++++++++++--------------------------- 1 file changed, 13 insertions(+), 32 deletions(-) diff --git a/src/cargo/util/toml/mod.rs b/src/cargo/util/toml/mod.rs index 18a81c30c..e8f094506 100644 --- a/src/cargo/util/toml/mod.rs +++ b/src/cargo/util/toml/mod.rs @@ -667,33 +667,27 @@ fn resolve_toml( let resolved_dependencies = resolve_dependencies( gctx, &features, - manifest_file, original_toml.dependencies.as_ref(), None, - &workspace_config, - &inherit_cell, + &inherit, package_root, warnings, )?; let resolved_dev_dependencies = resolve_dependencies( gctx, &features, - manifest_file, original_toml.dev_dependencies(), Some(DepKind::Development), - &workspace_config, - &inherit_cell, + &inherit, package_root, warnings, )?; let resolved_build_dependencies = resolve_dependencies( gctx, &features, - manifest_file, original_toml.build_dependencies(), Some(DepKind::Build), - &workspace_config, - &inherit_cell, + &inherit, package_root, warnings, )?; @@ -702,33 +696,27 @@ fn resolve_toml( let resolved_dependencies = resolve_dependencies( gctx, &features, - manifest_file, platform.dependencies.as_ref(), None, - &workspace_config, - &inherit_cell, + &inherit, package_root, warnings, )?; let resolved_dev_dependencies = resolve_dependencies( gctx, &features, - manifest_file, platform.dev_dependencies(), Some(DepKind::Development), - &workspace_config, - &inherit_cell, + &inherit, package_root, warnings, )?; let resolved_build_dependencies = resolve_dependencies( gctx, &features, - manifest_file, platform.build_dependencies(), Some(DepKind::Build), - &workspace_config, - &inherit_cell, + &inherit, package_root, warnings, )?; @@ -1330,14 +1318,12 @@ pub fn to_real_manifest( } #[tracing::instrument(skip_all)] -fn resolve_dependencies( +fn resolve_dependencies<'a>( gctx: &GlobalContext, features: &Features, - manifest_file: &Path, orig_deps: Option<&BTreeMap>, kind: Option, - workspace_config: &WorkspaceConfig, - inherit_cell: &LazyCell, + inherit: &dyn Fn() -> CargoResult<&'a InheritableFields>, package_root: &Path, warnings: &mut Vec, ) -> CargoResult>> { @@ -1345,15 +1331,10 @@ fn resolve_dependencies( return Ok(None); }; - let inheritable = || { - inherit_cell - .try_borrow_with(|| load_inheritable_fields(gctx, manifest_file, &workspace_config)) - }; - let mut deps = BTreeMap::new(); for (name_in_toml, v) in dependencies.iter() { let mut resolved = - dependency_inherit_with(v.clone(), name_in_toml, inheritable, package_root, warnings)?; + dependency_inherit_with(v.clone(), name_in_toml, inherit, package_root, warnings)?; if let manifest::TomlDependency::Detailed(ref mut d) = resolved { if d.public.is_some() { let public_feature = features.require(Feature::public_dependency()); @@ -2030,14 +2011,14 @@ fn lints_inherit_with( fn dependency_inherit_with<'a>( dependency: manifest::InheritableDependency, name: &str, - inheritable: impl FnOnce() -> CargoResult<&'a InheritableFields>, + inherit: &dyn Fn() -> CargoResult<&'a InheritableFields>, package_root: &Path, warnings: &mut Vec, ) -> CargoResult { match dependency { manifest::InheritableDependency::Value(value) => Ok(value), manifest::InheritableDependency::Inherit(w) => { - inner_dependency_inherit_with(w, name, inheritable, package_root, warnings).with_context(|| { + inner_dependency_inherit_with(w, name, inherit, package_root, warnings).with_context(|| { format!( "error inheriting `{name}` from workspace root manifest's `workspace.dependencies.{name}`", ) @@ -2049,7 +2030,7 @@ fn dependency_inherit_with<'a>( fn inner_dependency_inherit_with<'a>( dependency: manifest::TomlInheritedDependency, name: &str, - inheritable: impl FnOnce() -> CargoResult<&'a InheritableFields>, + inherit: &dyn Fn() -> CargoResult<&'a InheritableFields>, package_root: &Path, warnings: &mut Vec, ) -> CargoResult { @@ -2068,7 +2049,7 @@ fn inner_dependency_inherit_with<'a>( if dependency.default_features.is_some() && dependency.default_features2.is_some() { warn_on_deprecated("default-features", name, "dependency", warnings); } - inheritable()?.get_dependency(name, package_root).map(|d| { + inherit()?.get_dependency(name, package_root).map(|d| { match d { manifest::TomlDependency::Simple(s) => { if let Some(false) = dependency.default_features() { From 8a6fa8bcb4ef06f46ce41bfffd928abb8f27d518 Mon Sep 17 00:00:00 2001 From: Ed Page Date: Tue, 2 Apr 2024 11:55:07 -0500 Subject: [PATCH 27/34] refactor(toml): Extract package resolving --- src/cargo/util/toml/mod.rs | 264 +++++++++++++++++++------------------ 1 file changed, 135 insertions(+), 129 deletions(-) diff --git a/src/cargo/util/toml/mod.rs b/src/cargo/util/toml/mod.rs index e8f094506..84586f2d7 100644 --- a/src/cargo/util/toml/mod.rs +++ b/src/cargo/util/toml/mod.rs @@ -532,135 +532,8 @@ fn resolve_toml( }; let resolved_package = if let Some(original_package) = original_toml.package() { - let resolved_package = manifest::TomlPackage { - edition: original_package - .edition - .clone() - .map(|value| field_inherit_with(value, "edition", || inherit()?.edition())) - .transpose()? - .map(manifest::InheritableField::Value), - rust_version: original_package - .rust_version - .clone() - .map(|value| { - field_inherit_with(value, "rust-version", || inherit()?.rust_version()) - }) - .transpose()? - .map(manifest::InheritableField::Value), - name: original_package.name.clone(), - version: original_package - .version - .clone() - .map(|value| field_inherit_with(value, "version", || inherit()?.version())) - .transpose()? - .map(manifest::InheritableField::Value), - authors: original_package - .authors - .clone() - .map(|value| field_inherit_with(value, "authors", || inherit()?.authors())) - .transpose()? - .map(manifest::InheritableField::Value), - build: original_package.build.clone(), - metabuild: original_package.metabuild.clone(), - default_target: original_package.default_target.clone(), - forced_target: original_package.forced_target.clone(), - links: original_package.links.clone(), - exclude: original_package - .exclude - .clone() - .map(|value| field_inherit_with(value, "exclude", || inherit()?.exclude())) - .transpose()? - .map(manifest::InheritableField::Value), - include: original_package - .include - .clone() - .map(|value| field_inherit_with(value, "include", || inherit()?.include())) - .transpose()? - .map(manifest::InheritableField::Value), - publish: original_package - .publish - .clone() - .map(|value| field_inherit_with(value, "publish", || inherit()?.publish())) - .transpose()? - .map(manifest::InheritableField::Value), - workspace: original_package.workspace.clone(), - im_a_teapot: original_package.im_a_teapot.clone(), - autobins: original_package.autobins.clone(), - autoexamples: original_package.autoexamples.clone(), - autotests: original_package.autotests.clone(), - autobenches: original_package.autobenches.clone(), - default_run: original_package.default_run.clone(), - description: original_package - .description - .clone() - .map(|value| field_inherit_with(value, "description", || inherit()?.description())) - .transpose()? - .map(manifest::InheritableField::Value), - homepage: original_package - .homepage - .clone() - .map(|value| field_inherit_with(value, "homepage", || inherit()?.homepage())) - .transpose()? - .map(manifest::InheritableField::Value), - documentation: original_package - .documentation - .clone() - .map(|value| { - field_inherit_with(value, "documentation", || inherit()?.documentation()) - }) - .transpose()? - .map(manifest::InheritableField::Value), - readme: readme_for_package( - package_root, - original_package - .readme - .clone() - .map(|value| { - field_inherit_with(value, "readme", || inherit()?.readme(package_root)) - }) - .transpose()? - .as_ref(), - ) - .map(|s| manifest::InheritableField::Value(StringOrBool::String(s))), - keywords: original_package - .keywords - .clone() - .map(|value| field_inherit_with(value, "keywords", || inherit()?.keywords())) - .transpose()? - .map(manifest::InheritableField::Value), - categories: original_package - .categories - .clone() - .map(|value| field_inherit_with(value, "categories", || inherit()?.categories())) - .transpose()? - .map(manifest::InheritableField::Value), - license: original_package - .license - .clone() - .map(|value| field_inherit_with(value, "license", || inherit()?.license())) - .transpose()? - .map(manifest::InheritableField::Value), - license_file: original_package - .license_file - .clone() - .map(|value| { - field_inherit_with(value, "license-file", || { - inherit()?.license_file(package_root) - }) - }) - .transpose()? - .map(manifest::InheritableField::Value), - repository: original_package - .repository - .clone() - .map(|value| field_inherit_with(value, "repository", || inherit()?.repository())) - .transpose()? - .map(manifest::InheritableField::Value), - resolver: original_package.resolver.clone(), - metadata: original_package.metadata.clone(), - _invalid_cargo_features: Default::default(), - }; - Some(Box::new(resolved_package)) + let resolved_package = resolve_package_toml(original_package, package_root, &inherit)?; + Some(resolved_package) } else { None }; @@ -773,6 +646,139 @@ fn resolve_toml( Ok(resolved_toml) } +#[tracing::instrument(skip_all)] +fn resolve_package_toml<'a>( + original_package: &manifest::TomlPackage, + package_root: &Path, + inherit: &dyn Fn() -> CargoResult<&'a InheritableFields>, +) -> CargoResult> { + let resolved_package = manifest::TomlPackage { + edition: original_package + .edition + .clone() + .map(|value| field_inherit_with(value, "edition", || inherit()?.edition())) + .transpose()? + .map(manifest::InheritableField::Value), + rust_version: original_package + .rust_version + .clone() + .map(|value| field_inherit_with(value, "rust-version", || inherit()?.rust_version())) + .transpose()? + .map(manifest::InheritableField::Value), + name: original_package.name.clone(), + version: original_package + .version + .clone() + .map(|value| field_inherit_with(value, "version", || inherit()?.version())) + .transpose()? + .map(manifest::InheritableField::Value), + authors: original_package + .authors + .clone() + .map(|value| field_inherit_with(value, "authors", || inherit()?.authors())) + .transpose()? + .map(manifest::InheritableField::Value), + build: original_package.build.clone(), + metabuild: original_package.metabuild.clone(), + default_target: original_package.default_target.clone(), + forced_target: original_package.forced_target.clone(), + links: original_package.links.clone(), + exclude: original_package + .exclude + .clone() + .map(|value| field_inherit_with(value, "exclude", || inherit()?.exclude())) + .transpose()? + .map(manifest::InheritableField::Value), + include: original_package + .include + .clone() + .map(|value| field_inherit_with(value, "include", || inherit()?.include())) + .transpose()? + .map(manifest::InheritableField::Value), + publish: original_package + .publish + .clone() + .map(|value| field_inherit_with(value, "publish", || inherit()?.publish())) + .transpose()? + .map(manifest::InheritableField::Value), + workspace: original_package.workspace.clone(), + im_a_teapot: original_package.im_a_teapot.clone(), + autobins: original_package.autobins.clone(), + autoexamples: original_package.autoexamples.clone(), + autotests: original_package.autotests.clone(), + autobenches: original_package.autobenches.clone(), + default_run: original_package.default_run.clone(), + description: original_package + .description + .clone() + .map(|value| field_inherit_with(value, "description", || inherit()?.description())) + .transpose()? + .map(manifest::InheritableField::Value), + homepage: original_package + .homepage + .clone() + .map(|value| field_inherit_with(value, "homepage", || inherit()?.homepage())) + .transpose()? + .map(manifest::InheritableField::Value), + documentation: original_package + .documentation + .clone() + .map(|value| field_inherit_with(value, "documentation", || inherit()?.documentation())) + .transpose()? + .map(manifest::InheritableField::Value), + readme: readme_for_package( + package_root, + original_package + .readme + .clone() + .map(|value| { + field_inherit_with(value, "readme", || inherit()?.readme(package_root)) + }) + .transpose()? + .as_ref(), + ) + .map(|s| manifest::InheritableField::Value(StringOrBool::String(s))), + keywords: original_package + .keywords + .clone() + .map(|value| field_inherit_with(value, "keywords", || inherit()?.keywords())) + .transpose()? + .map(manifest::InheritableField::Value), + categories: original_package + .categories + .clone() + .map(|value| field_inherit_with(value, "categories", || inherit()?.categories())) + .transpose()? + .map(manifest::InheritableField::Value), + license: original_package + .license + .clone() + .map(|value| field_inherit_with(value, "license", || inherit()?.license())) + .transpose()? + .map(manifest::InheritableField::Value), + license_file: original_package + .license_file + .clone() + .map(|value| { + field_inherit_with(value, "license-file", || { + inherit()?.license_file(package_root) + }) + }) + .transpose()? + .map(manifest::InheritableField::Value), + repository: original_package + .repository + .clone() + .map(|value| field_inherit_with(value, "repository", || inherit()?.repository())) + .transpose()? + .map(manifest::InheritableField::Value), + resolver: original_package.resolver.clone(), + metadata: original_package.metadata.clone(), + _invalid_cargo_features: Default::default(), + }; + Ok(Box::new(resolved_package)) +} + #[tracing::instrument(skip_all)] pub fn to_real_manifest( contents: String, From 2811c15485411dc1a2effe97beac41a8807ab517 Mon Sep 17 00:00:00 2001 From: Ed Page Date: Tue, 2 Apr 2024 12:13:35 -0500 Subject: [PATCH 28/34] refactor(toml): Build up resolved manifest as we go Normally, I prefer directly constructing something but I feel this works better in this case so I can limit a lot of work that is coupled to a `package` being present. Since we still directly construct, just with `None`, I think this still works. --- src/cargo/util/toml/mod.rs | 75 ++++++++++++++++++++------------------ 1 file changed, 39 insertions(+), 36 deletions(-) diff --git a/src/cargo/util/toml/mod.rs b/src/cargo/util/toml/mod.rs index 84586f2d7..1c556142c 100644 --- a/src/cargo/util/toml/mod.rs +++ b/src/cargo/util/toml/mod.rs @@ -523,6 +523,31 @@ fn resolve_toml( warnings: &mut Vec, _errors: &mut Vec, ) -> CargoResult { + let mut resolved_toml = manifest::TomlManifest { + cargo_features: original_toml.cargo_features.clone(), + package: None, + project: None, + profile: original_toml.profile.clone(), + lib: original_toml.lib.clone(), + bin: original_toml.bin.clone(), + example: original_toml.example.clone(), + test: original_toml.test.clone(), + bench: original_toml.bench.clone(), + dependencies: None, + dev_dependencies: None, + dev_dependencies2: None, + build_dependencies: None, + build_dependencies2: None, + features: original_toml.features.clone(), + target: None, + replace: original_toml.replace.clone(), + patch: original_toml.patch.clone(), + workspace: original_toml.workspace.clone(), + badges: None, + lints: None, + _unused_keys: Default::default(), + }; + let package_root = manifest_file.parent().unwrap(); let inherit_cell: LazyCell = LazyCell::new(); @@ -531,13 +556,11 @@ fn resolve_toml( .try_borrow_with(|| load_inheritable_fields(gctx, manifest_file, &workspace_config)) }; - let resolved_package = if let Some(original_package) = original_toml.package() { + if let Some(original_package) = original_toml.package() { let resolved_package = resolve_package_toml(original_package, package_root, &inherit)?; - Some(resolved_package) - } else { - None - }; - let resolved_dependencies = resolve_dependencies( + resolved_toml.package = Some(resolved_package); + } + resolved_toml.dependencies = resolve_dependencies( gctx, &features, original_toml.dependencies.as_ref(), @@ -546,7 +569,7 @@ fn resolve_toml( package_root, warnings, )?; - let resolved_dev_dependencies = resolve_dependencies( + resolved_toml.dev_dependencies = resolve_dependencies( gctx, &features, original_toml.dev_dependencies(), @@ -555,7 +578,7 @@ fn resolve_toml( package_root, warnings, )?; - let resolved_build_dependencies = resolve_dependencies( + resolved_toml.build_dependencies = resolve_dependencies( gctx, &features, original_toml.build_dependencies(), @@ -604,44 +627,24 @@ fn resolve_toml( }, ); } - let resolved_target = (!resolved_target.is_empty()).then_some(resolved_target); + resolved_toml.target = (!resolved_target.is_empty()).then_some(resolved_target); + let resolved_lints = original_toml .lints .clone() .map(|value| lints_inherit_with(value, || inherit()?.lints())) .transpose()?; + resolved_toml.lints = resolved_lints.map(|lints| manifest::InheritableLints { + workspace: false, + lints, + }); + let resolved_badges = original_toml .badges .clone() .map(|mw| field_inherit_with(mw, "badges", || inherit()?.badges())) .transpose()?; - let resolved_toml = manifest::TomlManifest { - cargo_features: original_toml.cargo_features.clone(), - package: resolved_package, - project: None, - profile: original_toml.profile.clone(), - lib: original_toml.lib.clone(), - bin: original_toml.bin.clone(), - example: original_toml.example.clone(), - test: original_toml.test.clone(), - bench: original_toml.bench.clone(), - dependencies: resolved_dependencies, - dev_dependencies: resolved_dev_dependencies, - dev_dependencies2: None, - build_dependencies: resolved_build_dependencies, - build_dependencies2: None, - features: original_toml.features.clone(), - target: resolved_target, - replace: original_toml.replace.clone(), - patch: original_toml.patch.clone(), - workspace: original_toml.workspace.clone(), - badges: resolved_badges.map(manifest::InheritableField::Value), - lints: resolved_lints.map(|lints| manifest::InheritableLints { - workspace: false, - lints, - }), - _unused_keys: Default::default(), - }; + resolved_toml.badges = resolved_badges.map(manifest::InheritableField::Value); Ok(resolved_toml) } From 9eb7c0946307979010952adbf806ad7b9252bd22 Mon Sep 17 00:00:00 2001 From: Ed Page Date: Tue, 2 Apr 2024 12:17:37 -0500 Subject: [PATCH 29/34] refactor(toml): Scope package-related tables to package scope --- src/cargo/util/toml/mod.rs | 129 +++++++++++++++++++------------------ 1 file changed, 65 insertions(+), 64 deletions(-) diff --git a/src/cargo/util/toml/mod.rs b/src/cargo/util/toml/mod.rs index 1c556142c..e5a512f77 100644 --- a/src/cargo/util/toml/mod.rs +++ b/src/cargo/util/toml/mod.rs @@ -559,92 +559,93 @@ fn resolve_toml( if let Some(original_package) = original_toml.package() { let resolved_package = resolve_package_toml(original_package, package_root, &inherit)?; resolved_toml.package = Some(resolved_package); - } - resolved_toml.dependencies = resolve_dependencies( - gctx, - &features, - original_toml.dependencies.as_ref(), - None, - &inherit, - package_root, - warnings, - )?; - resolved_toml.dev_dependencies = resolve_dependencies( - gctx, - &features, - original_toml.dev_dependencies(), - Some(DepKind::Development), - &inherit, - package_root, - warnings, - )?; - resolved_toml.build_dependencies = resolve_dependencies( - gctx, - &features, - original_toml.build_dependencies(), - Some(DepKind::Build), - &inherit, - package_root, - warnings, - )?; - let mut resolved_target = BTreeMap::new(); - for (name, platform) in original_toml.target.iter().flatten() { - let resolved_dependencies = resolve_dependencies( + + resolved_toml.dependencies = resolve_dependencies( gctx, &features, - platform.dependencies.as_ref(), + original_toml.dependencies.as_ref(), None, &inherit, package_root, warnings, )?; - let resolved_dev_dependencies = resolve_dependencies( + resolved_toml.dev_dependencies = resolve_dependencies( gctx, &features, - platform.dev_dependencies(), + original_toml.dev_dependencies(), Some(DepKind::Development), &inherit, package_root, warnings, )?; - let resolved_build_dependencies = resolve_dependencies( + resolved_toml.build_dependencies = resolve_dependencies( gctx, &features, - platform.build_dependencies(), + original_toml.build_dependencies(), Some(DepKind::Build), &inherit, package_root, warnings, )?; - resolved_target.insert( - name.clone(), - manifest::TomlPlatform { - dependencies: resolved_dependencies, - build_dependencies: resolved_build_dependencies, - build_dependencies2: None, - dev_dependencies: resolved_dev_dependencies, - dev_dependencies2: None, - }, - ); + let mut resolved_target = BTreeMap::new(); + for (name, platform) in original_toml.target.iter().flatten() { + let resolved_dependencies = resolve_dependencies( + gctx, + &features, + platform.dependencies.as_ref(), + None, + &inherit, + package_root, + warnings, + )?; + let resolved_dev_dependencies = resolve_dependencies( + gctx, + &features, + platform.dev_dependencies(), + Some(DepKind::Development), + &inherit, + package_root, + warnings, + )?; + let resolved_build_dependencies = resolve_dependencies( + gctx, + &features, + platform.build_dependencies(), + Some(DepKind::Build), + &inherit, + package_root, + warnings, + )?; + resolved_target.insert( + name.clone(), + manifest::TomlPlatform { + dependencies: resolved_dependencies, + build_dependencies: resolved_build_dependencies, + build_dependencies2: None, + dev_dependencies: resolved_dev_dependencies, + dev_dependencies2: None, + }, + ); + } + resolved_toml.target = (!resolved_target.is_empty()).then_some(resolved_target); + + let resolved_lints = original_toml + .lints + .clone() + .map(|value| lints_inherit_with(value, || inherit()?.lints())) + .transpose()?; + resolved_toml.lints = resolved_lints.map(|lints| manifest::InheritableLints { + workspace: false, + lints, + }); + + let resolved_badges = original_toml + .badges + .clone() + .map(|mw| field_inherit_with(mw, "badges", || inherit()?.badges())) + .transpose()?; + resolved_toml.badges = resolved_badges.map(manifest::InheritableField::Value); } - resolved_toml.target = (!resolved_target.is_empty()).then_some(resolved_target); - - let resolved_lints = original_toml - .lints - .clone() - .map(|value| lints_inherit_with(value, || inherit()?.lints())) - .transpose()?; - resolved_toml.lints = resolved_lints.map(|lints| manifest::InheritableLints { - workspace: false, - lints, - }); - - let resolved_badges = original_toml - .badges - .clone() - .map(|mw| field_inherit_with(mw, "badges", || inherit()?.badges())) - .transpose()?; - resolved_toml.badges = resolved_badges.map(manifest::InheritableField::Value); Ok(resolved_toml) } From 7640081cda40040c5f56d3747fc558f8259454db Mon Sep 17 00:00:00 2001 From: Ed Page Date: Tue, 2 Apr 2024 12:25:34 -0500 Subject: [PATCH 30/34] Group logic for fields dependent on package I'm somewhat tempted to flatten the function call. The parallel between the package an virtual-manifest cases would help to keep them in sync. --- src/cargo/util/toml/mod.rs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/cargo/util/toml/mod.rs b/src/cargo/util/toml/mod.rs index e5a512f77..1e027c2ed 100644 --- a/src/cargo/util/toml/mod.rs +++ b/src/cargo/util/toml/mod.rs @@ -645,6 +645,10 @@ fn resolve_toml( .map(|mw| field_inherit_with(mw, "badges", || inherit()?.badges())) .transpose()?; resolved_toml.badges = resolved_badges.map(manifest::InheritableField::Value); + } else { + for field in original_toml.requires_package() { + bail!("this virtual manifest specifies a `{field}` section, which is not allowed"); + } } Ok(resolved_toml) @@ -1513,10 +1517,6 @@ fn to_virtual_manifest( ) -> CargoResult { let root = manifest_file.parent().unwrap(); - for field in original_toml.requires_package() { - bail!("this virtual manifest specifies a `{field}` section, which is not allowed"); - } - let mut deps = Vec::new(); let (replace, patch) = { let mut manifest_ctx = ManifestContext { From 70ad920e747d54df2072d80532b358caf3afe0f6 Mon Sep 17 00:00:00 2001 From: Ed Page Date: Tue, 2 Apr 2024 12:31:18 -0500 Subject: [PATCH 31/34] refactor(toml): Prefer making a Manifest from resolved_toml --- src/cargo/util/toml/mod.rs | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/src/cargo/util/toml/mod.rs b/src/cargo/util/toml/mod.rs index 1e027c2ed..4b7be49cc 100644 --- a/src/cargo/util/toml/mod.rs +++ b/src/cargo/util/toml/mod.rs @@ -925,7 +925,7 @@ pub fn to_real_manifest( let resolve_behavior = match ( resolved_package.resolver.as_ref(), - original_toml + resolved_toml .workspace .as_ref() .and_then(|ws| ws.resolver.as_ref()), @@ -942,7 +942,7 @@ pub fn to_real_manifest( // If we have a lib with no path, use the inferred lib or else the package name. let targets = targets( &features, - &original_toml, + &resolved_toml, package_name, package_root, edition, @@ -1107,8 +1107,8 @@ pub fn to_real_manifest( )?; } - let replace = replace(&original_toml, &mut manifest_ctx)?; - let patch = patch(&original_toml, &mut manifest_ctx)?; + let replace = replace(&resolved_toml, &mut manifest_ctx)?; + let patch = patch(&resolved_toml, &mut manifest_ctx)?; { let mut names_sources = BTreeMap::new(); @@ -1179,7 +1179,7 @@ pub fn to_real_manifest( rust_version: rust_version.clone(), }; - if let Some(profiles) = &original_toml.profile { + if let Some(profiles) = &resolved_toml.profile { let cli_unstable = gctx.cli_unstable(); validate_profiles(profiles, cli_unstable, &features, warnings)?; } @@ -1211,7 +1211,7 @@ pub fn to_real_manifest( let summary = Summary::new( pkgid, deps, - &original_toml + &resolved_toml .features .as_ref() .unwrap_or(&Default::default()) From ecf97cff51ad0c83a049de557631c712c932d24e Mon Sep 17 00:00:00 2001 From: Ed Page Date: Tue, 2 Apr 2024 12:55:25 -0500 Subject: [PATCH 32/34] refactor(toml): Simplify dependency validation --- src/cargo/util/toml/mod.rs | 21 ++++++++------------- 1 file changed, 8 insertions(+), 13 deletions(-) diff --git a/src/cargo/util/toml/mod.rs b/src/cargo/util/toml/mod.rs index 4b7be49cc..1c08a9ed7 100644 --- a/src/cargo/util/toml/mod.rs +++ b/src/cargo/util/toml/mod.rs @@ -995,10 +995,10 @@ pub fn to_real_manifest( // Collect the dependencies. validate_dependencies( - &mut manifest_ctx, original_toml.dependencies.as_ref(), None, None, + manifest_ctx.warnings, )?; gather_dependencies(&mut manifest_ctx, resolved_toml.dependencies.as_ref(), None)?; if original_toml.dev_dependencies.is_some() && original_toml.dev_dependencies2.is_some() { @@ -1010,10 +1010,10 @@ pub fn to_real_manifest( ); } validate_dependencies( - &mut manifest_ctx, original_toml.dev_dependencies(), None, Some(DepKind::Development), + manifest_ctx.warnings, )?; gather_dependencies( &mut manifest_ctx, @@ -1029,10 +1029,10 @@ pub fn to_real_manifest( ); } validate_dependencies( - &mut manifest_ctx, original_toml.build_dependencies(), None, Some(DepKind::Build), + manifest_ctx.warnings, )?; gather_dependencies( &mut manifest_ctx, @@ -1058,10 +1058,10 @@ pub fn to_real_manifest( platform_kind.check_cfg_attributes(manifest_ctx.warnings); let platform_kind = Some(platform_kind); validate_dependencies( - &mut manifest_ctx, platform.dependencies.as_ref(), platform_kind.as_ref(), None, + manifest_ctx.warnings, )?; if platform.build_dependencies.is_some() && platform.build_dependencies2.is_some() { warn_on_deprecated( @@ -1072,10 +1072,10 @@ pub fn to_real_manifest( ); } validate_dependencies( - &mut manifest_ctx, platform.build_dependencies(), platform_kind.as_ref(), Some(DepKind::Build), + manifest_ctx.warnings, )?; if platform.dev_dependencies.is_some() && platform.dev_dependencies2.is_some() { warn_on_deprecated( @@ -1086,10 +1086,10 @@ pub fn to_real_manifest( ); } validate_dependencies( - &mut manifest_ctx, platform.dev_dependencies(), platform_kind.as_ref(), Some(DepKind::Development), + manifest_ctx.warnings, )?; } for (name, platform) in resolved_toml.target.iter().flatten() { @@ -1393,10 +1393,10 @@ fn resolve_dependencies<'a>( #[tracing::instrument(skip_all)] fn validate_dependencies( - manifest_ctx: &mut ManifestContext<'_, '_>, original_deps: Option<&BTreeMap>, platform: Option<&Platform>, kind: Option, + warnings: &mut Vec, ) -> CargoResult<()> { let Some(dependencies) = original_deps else { return Ok(()); @@ -1412,12 +1412,7 @@ fn validate_dependencies( } else { kind_name.to_string() }; - unused_dep_keys( - name_in_toml, - &table_in_toml, - v.unused_keys(), - manifest_ctx.warnings, - ); + unused_dep_keys(name_in_toml, &table_in_toml, v.unused_keys(), warnings); } Ok(()) } From 1e761a15288060906c3f3dc872c6550bc8b84160 Mon Sep 17 00:00:00 2001 From: Ed Page Date: Tue, 2 Apr 2024 12:58:55 -0500 Subject: [PATCH 33/34] refactor(toml): Gather dependency gathering --- src/cargo/util/toml/mod.rs | 154 +++++++++++++++---------------------- 1 file changed, 63 insertions(+), 91 deletions(-) diff --git a/src/cargo/util/toml/mod.rs b/src/cargo/util/toml/mod.rs index 1c08a9ed7..72385396d 100644 --- a/src/cargo/util/toml/mod.rs +++ b/src/cargo/util/toml/mod.rs @@ -982,8 +982,57 @@ pub fn to_real_manifest( } } - let mut deps = Vec::new(); + validate_dependencies(original_toml.dependencies.as_ref(), None, None, warnings)?; + if original_toml.dev_dependencies.is_some() && original_toml.dev_dependencies2.is_some() { + warn_on_deprecated("dev-dependencies", package_name, "package", warnings); + } + validate_dependencies( + original_toml.dev_dependencies(), + None, + Some(DepKind::Development), + warnings, + )?; + if original_toml.build_dependencies.is_some() && original_toml.build_dependencies2.is_some() { + warn_on_deprecated("build-dependencies", package_name, "package", warnings); + } + validate_dependencies( + original_toml.build_dependencies(), + None, + Some(DepKind::Build), + warnings, + )?; + for (name, platform) in original_toml.target.iter().flatten() { + let platform_kind: Platform = name.parse()?; + platform_kind.check_cfg_attributes(warnings); + let platform_kind = Some(platform_kind); + validate_dependencies( + platform.dependencies.as_ref(), + platform_kind.as_ref(), + None, + warnings, + )?; + if platform.build_dependencies.is_some() && platform.build_dependencies2.is_some() { + warn_on_deprecated("build-dependencies", name, "platform target", warnings); + } + validate_dependencies( + platform.build_dependencies(), + platform_kind.as_ref(), + Some(DepKind::Build), + warnings, + )?; + if platform.dev_dependencies.is_some() && platform.dev_dependencies2.is_some() { + warn_on_deprecated("dev-dependencies", name, "platform target", warnings); + } + validate_dependencies( + platform.dev_dependencies(), + platform_kind.as_ref(), + Some(DepKind::Development), + warnings, + )?; + } + // Collect the dependencies. + let mut deps = Vec::new(); let mut manifest_ctx = ManifestContext { deps: &mut deps, source_id, @@ -992,106 +1041,17 @@ pub fn to_real_manifest( platform: None, root: package_root, }; - - // Collect the dependencies. - validate_dependencies( - original_toml.dependencies.as_ref(), - None, - None, - manifest_ctx.warnings, - )?; gather_dependencies(&mut manifest_ctx, resolved_toml.dependencies.as_ref(), None)?; - if original_toml.dev_dependencies.is_some() && original_toml.dev_dependencies2.is_some() { - warn_on_deprecated( - "dev-dependencies", - package_name, - "package", - manifest_ctx.warnings, - ); - } - validate_dependencies( - original_toml.dev_dependencies(), - None, - Some(DepKind::Development), - manifest_ctx.warnings, - )?; gather_dependencies( &mut manifest_ctx, resolved_toml.dev_dependencies(), Some(DepKind::Development), )?; - if original_toml.build_dependencies.is_some() && original_toml.build_dependencies2.is_some() { - warn_on_deprecated( - "build-dependencies", - package_name, - "package", - manifest_ctx.warnings, - ); - } - validate_dependencies( - original_toml.build_dependencies(), - None, - Some(DepKind::Build), - manifest_ctx.warnings, - )?; gather_dependencies( &mut manifest_ctx, resolved_toml.build_dependencies(), Some(DepKind::Build), )?; - - verify_lints( - resolved_toml.resolved_lints().expect("previously resolved"), - gctx, - manifest_ctx.warnings, - )?; - let default = manifest::TomlLints::default(); - let rustflags = lints_to_rustflags( - resolved_toml - .resolved_lints() - .expect("previously resolved") - .unwrap_or(&default), - ); - - for (name, platform) in original_toml.target.iter().flatten() { - let platform_kind: Platform = name.parse()?; - platform_kind.check_cfg_attributes(manifest_ctx.warnings); - let platform_kind = Some(platform_kind); - validate_dependencies( - platform.dependencies.as_ref(), - platform_kind.as_ref(), - None, - manifest_ctx.warnings, - )?; - if platform.build_dependencies.is_some() && platform.build_dependencies2.is_some() { - warn_on_deprecated( - "build-dependencies", - name, - "platform target", - manifest_ctx.warnings, - ); - } - validate_dependencies( - platform.build_dependencies(), - platform_kind.as_ref(), - Some(DepKind::Build), - manifest_ctx.warnings, - )?; - if platform.dev_dependencies.is_some() && platform.dev_dependencies2.is_some() { - warn_on_deprecated( - "dev-dependencies", - name, - "platform target", - manifest_ctx.warnings, - ); - } - validate_dependencies( - platform.dev_dependencies(), - platform_kind.as_ref(), - Some(DepKind::Development), - manifest_ctx.warnings, - )?; - } for (name, platform) in resolved_toml.target.iter().flatten() { manifest_ctx.platform = Some(name.parse()?); gather_dependencies(&mut manifest_ctx, platform.dependencies.as_ref(), None)?; @@ -1106,7 +1066,6 @@ pub fn to_real_manifest( Some(DepKind::Development), )?; } - let replace = replace(&resolved_toml, &mut manifest_ctx)?; let patch = patch(&resolved_toml, &mut manifest_ctx)?; @@ -1126,6 +1085,19 @@ pub fn to_real_manifest( } } + verify_lints( + resolved_toml.resolved_lints().expect("previously resolved"), + gctx, + warnings, + )?; + let default = manifest::TomlLints::default(); + let rustflags = lints_to_rustflags( + resolved_toml + .resolved_lints() + .expect("previously resolved") + .unwrap_or(&default), + ); + let metadata = ManifestMetadata { description: resolved_package .resolved_description() From 58b6501a48ab7d6ecb2d7c363e3efe30b9793b57 Mon Sep 17 00:00:00 2001 From: Ed Page Date: Tue, 2 Apr 2024 14:21:54 -0500 Subject: [PATCH 34/34] refactor(toml): Attempt to logically group everything --- src/cargo/core/features.rs | 4 +- src/cargo/util/toml/mod.rs | 1940 ++++++++++++++++++------------------ 2 files changed, 972 insertions(+), 972 deletions(-) diff --git a/src/cargo/core/features.rs b/src/cargo/core/features.rs index 1b6cba046..f72797dd9 100644 --- a/src/cargo/core/features.rs +++ b/src/cargo/core/features.rs @@ -151,7 +151,7 @@ pub type AllowFeatures = BTreeSet; /// - Update [`CLI_VALUES`] to include the new edition. /// - Set [`LATEST_UNSTABLE`] to Some with the new edition. /// - Add an unstable feature to the [`features!`] macro invocation below for the new edition. -/// - Gate on that new feature in [`toml::to_real_manifest`]. +/// - Gate on that new feature in [`toml`]. /// - Update the shell completion files. /// - Update any failing tests (hopefully there are very few). /// - Update unstable.md to add a new section for this new edition (see [this example]). @@ -178,7 +178,7 @@ pub type AllowFeatures = BTreeSet; /// [`LATEST_STABLE`]: Edition::LATEST_STABLE /// [this example]: https://github.com/rust-lang/cargo/blob/3ebb5f15a940810f250b68821149387af583a79e/src/doc/src/reference/unstable.md?plain=1#L1238-L1264 /// [`is_stable`]: Edition::is_stable -/// [`toml::to_real_manifest`]: crate::util::toml::to_real_manifest +/// [`toml`]: crate::util::toml /// [`features!`]: macro.features.html #[derive( Default, Clone, Copy, Debug, Hash, PartialOrd, Ord, Eq, PartialEq, Serialize, Deserialize, diff --git a/src/cargo/util/toml/mod.rs b/src/cargo/util/toml/mod.rs index 72385396d..954f594a0 100644 --- a/src/cargo/util/toml/mod.rs +++ b/src/cargo/util/toml/mod.rs @@ -34,6 +34,14 @@ mod targets; use self::targets::targets; +/// See also `bin/cargo/commands/run.rs`s `is_manifest_command` +pub fn is_embedded(path: &Path) -> bool { + let ext = path.extension(); + ext == Some(OsStr::new("rs")) || + // Provide better errors by not considering directories to be embedded manifests + (ext.is_none() && path.is_file()) +} + /// Loads a `Cargo.toml` from a file on disk. /// /// This could result in a real or virtual manifest being returned. @@ -160,47 +168,6 @@ fn deserialize_toml( Ok(document) } -/// See also `bin/cargo/commands/run.rs`s `is_manifest_command` -pub fn is_embedded(path: &Path) -> bool { - let ext = path.extension(); - ext == Some(OsStr::new("rs")) || - // Provide better errors by not considering directories to be embedded manifests - (ext.is_none() && path.is_file()) -} - -fn emit_diagnostic( - e: toml_edit::de::Error, - contents: &str, - manifest_file: &Path, - gctx: &GlobalContext, -) -> anyhow::Error { - let Some(span) = e.span() else { - return e.into(); - }; - - // Get the path to the manifest, relative to the cwd - let manifest_path = diff_paths(manifest_file, gctx.cwd()) - .unwrap_or_else(|| manifest_file.to_path_buf()) - .display() - .to_string(); - let message = Level::Error.title(e.message()).snippet( - Snippet::source(contents) - .origin(&manifest_path) - .fold(true) - .annotation(Level::Error.span(span)), - ); - let renderer = Renderer::styled().term_width( - gctx.shell() - .err_width() - .diagnostic_terminal_width() - .unwrap_or(annotate_snippets::renderer::DEFAULT_TERM_WIDTH), - ); - if let Err(err) = writeln!(gctx.shell().err(), "{}", renderer.render(message)) { - return err.into(); - } - return AlreadyPrintedError::new(e.into()).into(); -} - fn stringify(dst: &mut String, path: &serde_ignored::Path<'_>) { use serde_ignored::Path; @@ -226,291 +193,66 @@ fn stringify(dst: &mut String, path: &serde_ignored::Path<'_>) { } } -/// Warn about paths that have been deprecated and may conflict. -fn warn_on_deprecated(new_path: &str, name: &str, kind: &str, warnings: &mut Vec) { - let old_path = new_path.replace("-", "_"); - warnings.push(format!( - "conflicting between `{new_path}` and `{old_path}` in the `{name}` {kind}.\n - `{old_path}` is ignored and not recommended for use in the future" - )) -} - -fn warn_on_unused(unused: &BTreeSet, warnings: &mut Vec) { - for key in unused { - warnings.push(format!("unused manifest key: {}", key)); - if key == "profiles.debug" { - warnings.push("use `[profile.dev]` to configure debug builds".to_string()); - } - } -} - -pub fn prepare_for_publish(me: &Package, ws: &Workspace<'_>) -> CargoResult { - let contents = me.manifest().contents(); - let document = me.manifest().document(); - let original_toml = prepare_toml_for_publish(me.manifest().resolved_toml(), ws, me.root())?; - let resolved_toml = original_toml.clone(); - let features = me.manifest().unstable_features().clone(); - let workspace_config = me.manifest().workspace_config().clone(); - let source_id = me.package_id().source_id(); - let mut warnings = Default::default(); - let mut errors = Default::default(); - let gctx = ws.gctx(); - let manifest = to_real_manifest( - contents.to_owned(), - document.clone(), - original_toml, - resolved_toml, - features, - workspace_config, - source_id, - me.manifest_path(), - gctx, - &mut warnings, - &mut errors, - )?; - let new_pkg = Package::new(manifest, me.manifest_path()); - Ok(new_pkg) -} - -/// Prepares the manifest for publishing. -// - Path and git components of dependency specifications are removed. -// - License path is updated to point within the package. -fn prepare_toml_for_publish( - me: &manifest::TomlManifest, - ws: &Workspace<'_>, - package_root: &Path, -) -> CargoResult { - let gctx = ws.gctx(); - - if me - .cargo_features - .iter() - .flat_map(|f| f.iter()) - .any(|f| f == "open-namespaces") - { - anyhow::bail!("cannot publish with `open-namespaces`") - } - - let mut package = me.package().unwrap().clone(); - package.workspace = None; - let current_resolver = package - .resolver - .as_ref() - .map(|r| ResolveBehavior::from_manifest(r)) - .unwrap_or_else(|| { - package - .edition - .as_ref() - .and_then(|e| e.as_value()) - .map(|e| Edition::from_str(e)) - .unwrap_or(Ok(Edition::Edition2015)) - .map(|e| e.default_resolve_behavior()) - })?; - if ws.resolve_behavior() != current_resolver { - // This ensures the published crate if built as a root (e.g. `cargo install`) will - // use the same resolver behavior it was tested with in the workspace. - // To avoid forcing a higher MSRV we don't explicitly set this if it would implicitly - // result in the same thing. - package.resolver = Some(ws.resolve_behavior().to_manifest()); - } - if let Some(license_file) = &package.license_file { - let license_file = license_file - .as_value() - .context("license file should have been resolved before `prepare_for_publish()`")?; - let license_path = Path::new(&license_file); - let abs_license_path = paths::normalize_path(&package_root.join(license_path)); - if abs_license_path.strip_prefix(package_root).is_err() { - // This path points outside of the package root. `cargo package` - // will copy it into the root, so adjust the path to this location. - package.license_file = Some(manifest::InheritableField::Value( - license_path - .file_name() - .unwrap() - .to_str() - .unwrap() - .to_string(), - )); - } - } - - if let Some(readme) = &package.readme { - let readme = readme - .as_value() - .context("readme should have been resolved before `prepare_for_publish()`")?; - match readme { - manifest::StringOrBool::String(readme) => { - let readme_path = Path::new(&readme); - let abs_readme_path = paths::normalize_path(&package_root.join(readme_path)); - if abs_readme_path.strip_prefix(package_root).is_err() { - // This path points outside of the package root. `cargo package` - // will copy it into the root, so adjust the path to this location. - package.readme = Some(manifest::InheritableField::Value( - manifest::StringOrBool::String( - readme_path - .file_name() - .unwrap() - .to_str() - .unwrap() - .to_string(), - ), - )); - } - } - manifest::StringOrBool::Bool(_) => {} - } - } - let all = |_d: &manifest::TomlDependency| true; - let mut manifest = manifest::TomlManifest { - package: Some(package), - project: None, - profile: me.profile.clone(), - lib: me.lib.clone(), - bin: me.bin.clone(), - example: me.example.clone(), - test: me.test.clone(), - bench: me.bench.clone(), - dependencies: map_deps(gctx, me.dependencies.as_ref(), all)?, - dev_dependencies: map_deps( - gctx, - me.dev_dependencies(), - manifest::TomlDependency::is_version_specified, - )?, - dev_dependencies2: None, - build_dependencies: map_deps(gctx, me.build_dependencies(), all)?, - build_dependencies2: None, - features: me.features.clone(), - target: match me.target.as_ref().map(|target_map| { - target_map - .iter() - .map(|(k, v)| { - Ok(( - k.clone(), - manifest::TomlPlatform { - dependencies: map_deps(gctx, v.dependencies.as_ref(), all)?, - dev_dependencies: map_deps( - gctx, - v.dev_dependencies(), - manifest::TomlDependency::is_version_specified, - )?, - dev_dependencies2: None, - build_dependencies: map_deps(gctx, v.build_dependencies(), all)?, - build_dependencies2: None, - }, - )) - }) - .collect() - }) { - Some(Ok(v)) => Some(v), - Some(Err(e)) => return Err(e), - None => None, - }, - replace: None, - patch: None, - workspace: None, - badges: me.badges.clone(), - cargo_features: me.cargo_features.clone(), - lints: me.lints.clone(), - _unused_keys: Default::default(), - }; - strip_features(&mut manifest); - return Ok(manifest); - - fn strip_features(manifest: &mut TomlManifest) { - fn insert_dep_name( - dep_name_set: &mut BTreeSet, - deps: Option<&BTreeMap>, - ) { - let Some(deps) = deps else { - return; - }; - deps.iter().for_each(|(k, _v)| { - dep_name_set.insert(k.clone()); - }); - } - let mut dep_name_set = BTreeSet::new(); - insert_dep_name(&mut dep_name_set, manifest.dependencies.as_ref()); - insert_dep_name(&mut dep_name_set, manifest.dev_dependencies()); - insert_dep_name(&mut dep_name_set, manifest.build_dependencies()); - if let Some(target_map) = manifest.target.as_ref() { - target_map.iter().for_each(|(_k, v)| { - insert_dep_name(&mut dep_name_set, v.dependencies.as_ref()); - insert_dep_name(&mut dep_name_set, v.dev_dependencies()); - insert_dep_name(&mut dep_name_set, v.build_dependencies()); - }); - } - let features = manifest.features.as_mut(); - - let Some(features) = features else { - return; - }; - - features.values_mut().for_each(|feature_deps| { - feature_deps.retain(|feature_dep| { - let feature_value = FeatureValue::new(InternedString::new(feature_dep)); - match feature_value { - FeatureValue::Dep { dep_name } | FeatureValue::DepFeature { dep_name, .. } => { - let k = &manifest::PackageName::new(dep_name.to_string()).unwrap(); - dep_name_set.contains(k) +fn to_workspace_config( + original_toml: &manifest::TomlManifest, + manifest_file: &Path, + gctx: &GlobalContext, + warnings: &mut Vec, +) -> CargoResult { + let workspace_config = match ( + original_toml.workspace.as_ref(), + original_toml.package().and_then(|p| p.workspace.as_ref()), + ) { + (Some(toml_config), None) => { + verify_lints(toml_config.lints.as_ref(), gctx, warnings)?; + if let Some(ws_deps) = &toml_config.dependencies { + for (name, dep) in ws_deps { + if dep.is_optional() { + bail!("{name} is optional, but workspace dependencies cannot be optional",); + } + if dep.is_public() { + bail!("{name} is public, but workspace dependencies cannot be public",); } - _ => true, } - }); - }); - } - fn map_deps( - gctx: &GlobalContext, - deps: Option<&BTreeMap>, - filter: impl Fn(&manifest::TomlDependency) -> bool, - ) -> CargoResult>> { - let Some(deps) = deps else { - return Ok(None); - }; - let deps = deps - .iter() - .filter(|(_k, v)| { - if let manifest::InheritableDependency::Value(def) = v { - filter(def) - } else { - false + for (name, dep) in ws_deps { + unused_dep_keys(name, "workspace.dependencies", dep.unused_keys(), warnings); } - }) - .map(|(k, v)| Ok((k.clone(), map_dependency(gctx, v)?))) - .collect::>>()?; - Ok(Some(deps)) - } + } + let ws_root_config = to_workspace_root_config(toml_config, manifest_file); + WorkspaceConfig::Root(ws_root_config) + } + (None, root) => WorkspaceConfig::Member { + root: root.cloned(), + }, + (Some(..), Some(..)) => bail!( + "cannot configure both `package.workspace` and \ + `[workspace]`, only one can be specified" + ), + }; + Ok(workspace_config) +} - fn map_dependency( - gctx: &GlobalContext, - dep: &manifest::InheritableDependency, - ) -> CargoResult { - let dep = match dep { - manifest::InheritableDependency::Value(manifest::TomlDependency::Detailed(d)) => { - let mut d = d.clone(); - // Path dependencies become crates.io deps. - d.path.take(); - // Same with git dependencies. - d.git.take(); - d.branch.take(); - d.tag.take(); - d.rev.take(); - // registry specifications are elaborated to the index URL - if let Some(registry) = d.registry.take() { - d.registry_index = Some(gctx.get_registry_index(®istry)?.to_string()); - } - Ok(d) - } - manifest::InheritableDependency::Value(manifest::TomlDependency::Simple(s)) => { - Ok(manifest::TomlDetailedDependency { - version: Some(s.clone()), - ..Default::default() - }) - } - _ => unreachable!(), - }; - dep.map(manifest::TomlDependency::Detailed) - .map(manifest::InheritableDependency::Value) - } +fn to_workspace_root_config( + resolved_toml: &manifest::TomlWorkspace, + manifest_file: &Path, +) -> WorkspaceRootConfig { + let package_root = manifest_file.parent().unwrap(); + let inheritable = InheritableFields { + package: resolved_toml.package.clone(), + dependencies: resolved_toml.dependencies.clone(), + lints: resolved_toml.lints.clone(), + _ws_root: package_root.to_owned(), + }; + let ws_root_config = WorkspaceRootConfig::new( + package_root, + &resolved_toml.members, + &resolved_toml.default_members, + &resolved_toml.exclude, + &Some(inheritable), + &resolved_toml.metadata, + ); + ws_root_config } #[tracing::instrument(skip_all)] @@ -787,8 +529,391 @@ fn resolve_package_toml<'a>( Ok(Box::new(resolved_package)) } +/// Returns the name of the README file for a [`manifest::TomlPackage`]. +fn readme_for_package( + package_root: &Path, + readme: Option<&manifest::StringOrBool>, +) -> Option { + match &readme { + None => default_readme_from_package_root(package_root), + Some(value) => match value { + manifest::StringOrBool::Bool(false) => None, + manifest::StringOrBool::Bool(true) => Some("README.md".to_string()), + manifest::StringOrBool::String(v) => Some(v.clone()), + }, + } +} + +const DEFAULT_README_FILES: [&str; 3] = ["README.md", "README.txt", "README"]; + +/// Checks if a file with any of the default README file names exists in the package root. +/// If so, returns a `String` representing that name. +fn default_readme_from_package_root(package_root: &Path) -> Option { + for &readme_filename in DEFAULT_README_FILES.iter() { + if package_root.join(readme_filename).is_file() { + return Some(readme_filename.to_string()); + } + } + + None +} + #[tracing::instrument(skip_all)] -pub fn to_real_manifest( +fn resolve_dependencies<'a>( + gctx: &GlobalContext, + features: &Features, + orig_deps: Option<&BTreeMap>, + kind: Option, + inherit: &dyn Fn() -> CargoResult<&'a InheritableFields>, + package_root: &Path, + warnings: &mut Vec, +) -> CargoResult>> { + let Some(dependencies) = orig_deps else { + return Ok(None); + }; + + let mut deps = BTreeMap::new(); + for (name_in_toml, v) in dependencies.iter() { + let mut resolved = + dependency_inherit_with(v.clone(), name_in_toml, inherit, package_root, warnings)?; + if let manifest::TomlDependency::Detailed(ref mut d) = resolved { + if d.public.is_some() { + let public_feature = features.require(Feature::public_dependency()); + let with_public_feature = public_feature.is_ok(); + let with_z_public = gctx.cli_unstable().public_dependency; + if !with_public_feature && (!with_z_public && !gctx.nightly_features_allowed) { + public_feature?; + } + if matches!(kind, None) { + if !with_public_feature && !with_z_public { + d.public = None; + warnings.push(format!( + "ignoring `public` on dependency {name_in_toml}, pass `-Zpublic-dependency` to enable support for it" + )) + } + } else { + let kind_name = match kind { + Some(k) => k.kind_table(), + None => "dependencies", + }; + let hint = format!( + "'public' specifier can only be used on regular dependencies, not {kind_name}", + ); + if with_public_feature || with_z_public { + bail!(hint) + } else { + // If public feature isn't enabled in nightly, we instead warn that. + warnings.push(hint); + d.public = None; + } + } + } + } + + deps.insert( + name_in_toml.clone(), + manifest::InheritableDependency::Value(resolved.clone()), + ); + } + Ok(Some(deps)) +} + +fn load_inheritable_fields( + gctx: &GlobalContext, + resolved_path: &Path, + workspace_config: &WorkspaceConfig, +) -> CargoResult { + match workspace_config { + WorkspaceConfig::Root(root) => Ok(root.inheritable().clone()), + WorkspaceConfig::Member { + root: Some(ref path_to_root), + } => { + let path = resolved_path + .parent() + .unwrap() + .join(path_to_root) + .join("Cargo.toml"); + let root_path = paths::normalize_path(&path); + inheritable_from_path(gctx, root_path) + } + WorkspaceConfig::Member { root: None } => { + match find_workspace_root(&resolved_path, gctx)? { + Some(path_to_root) => inheritable_from_path(gctx, path_to_root), + None => Err(anyhow!("failed to find a workspace root")), + } + } + } +} + +fn inheritable_from_path( + gctx: &GlobalContext, + workspace_path: PathBuf, +) -> CargoResult { + // Workspace path should have Cargo.toml at the end + let workspace_path_root = workspace_path.parent().unwrap(); + + // Let the borrow exit scope so that it can be picked up if there is a need to + // read a manifest + if let Some(ws_root) = gctx.ws_roots.borrow().get(workspace_path_root) { + return Ok(ws_root.inheritable().clone()); + }; + + let source_id = SourceId::for_path(workspace_path_root)?; + let man = read_manifest(&workspace_path, source_id, gctx)?; + match man.workspace_config() { + WorkspaceConfig::Root(root) => { + gctx.ws_roots + .borrow_mut() + .insert(workspace_path, root.clone()); + Ok(root.inheritable().clone()) + } + _ => bail!( + "root of a workspace inferred but wasn't a root: {}", + workspace_path.display() + ), + } +} + +/// Defines simple getter methods for inheritable fields. +macro_rules! package_field_getter { + ( $(($key:literal, $field:ident -> $ret:ty),)* ) => ( + $( + #[doc = concat!("Gets the field `workspace.package", $key, "`.")] + fn $field(&self) -> CargoResult<$ret> { + let Some(val) = self.package.as_ref().and_then(|p| p.$field.as_ref()) else { + bail!("`workspace.package.{}` was not defined", $key); + }; + Ok(val.clone()) + } + )* + ) +} + +/// A group of fields that are inheritable by members of the workspace +#[derive(Clone, Debug, Default)] +pub struct InheritableFields { + package: Option, + dependencies: Option>, + lints: Option, + + // Bookkeeping to help when resolving values from above + _ws_root: PathBuf, +} + +impl InheritableFields { + package_field_getter! { + // Please keep this list lexicographically ordered. + ("authors", authors -> Vec), + ("badges", badges -> BTreeMap>), + ("categories", categories -> Vec), + ("description", description -> String), + ("documentation", documentation -> String), + ("edition", edition -> String), + ("exclude", exclude -> Vec), + ("homepage", homepage -> String), + ("include", include -> Vec), + ("keywords", keywords -> Vec), + ("license", license -> String), + ("publish", publish -> manifest::VecStringOrBool), + ("repository", repository -> String), + ("rust-version", rust_version -> RustVersion), + ("version", version -> semver::Version), + } + + /// Gets a workspace dependency with the `name`. + fn get_dependency( + &self, + name: &str, + package_root: &Path, + ) -> CargoResult { + let Some(deps) = &self.dependencies else { + bail!("`workspace.dependencies` was not defined"); + }; + let Some(dep) = deps.get(name) else { + bail!("`dependency.{name}` was not found in `workspace.dependencies`"); + }; + let mut dep = dep.clone(); + if let manifest::TomlDependency::Detailed(detailed) = &mut dep { + if let Some(rel_path) = &detailed.path { + detailed.path = Some(resolve_relative_path( + name, + self.ws_root(), + package_root, + rel_path, + )?); + } + } + Ok(dep) + } + + /// Gets the field `workspace.lint`. + fn lints(&self) -> CargoResult { + let Some(val) = &self.lints else { + bail!("`workspace.lints` was not defined"); + }; + Ok(val.clone()) + } + + /// Gets the field `workspace.package.license-file`. + fn license_file(&self, package_root: &Path) -> CargoResult { + let Some(license_file) = self.package.as_ref().and_then(|p| p.license_file.as_ref()) else { + bail!("`workspace.package.license-file` was not defined"); + }; + resolve_relative_path("license-file", &self._ws_root, package_root, license_file) + } + + /// Gets the field `workspace.package.readme`. + fn readme(&self, package_root: &Path) -> CargoResult { + let Some(readme) = readme_for_package( + self._ws_root.as_path(), + self.package.as_ref().and_then(|p| p.readme.as_ref()), + ) else { + bail!("`workspace.package.readme` was not defined"); + }; + resolve_relative_path("readme", &self._ws_root, package_root, &readme) + .map(manifest::StringOrBool::String) + } + + fn ws_root(&self) -> &PathBuf { + &self._ws_root + } +} + +fn field_inherit_with<'a, T>( + field: manifest::InheritableField, + label: &str, + get_ws_inheritable: impl FnOnce() -> CargoResult, +) -> CargoResult { + match field { + manifest::InheritableField::Value(value) => Ok(value), + manifest::InheritableField::Inherit(_) => get_ws_inheritable().with_context(|| { + format!( + "error inheriting `{label}` from workspace root manifest's `workspace.package.{label}`", + ) + }), + } +} + +fn lints_inherit_with( + lints: manifest::InheritableLints, + get_ws_inheritable: impl FnOnce() -> CargoResult, +) -> CargoResult { + if lints.workspace { + if !lints.lints.is_empty() { + anyhow::bail!("cannot override `workspace.lints` in `lints`, either remove the overrides or `lints.workspace = true` and manually specify the lints"); + } + get_ws_inheritable().with_context(|| { + "error inheriting `lints` from workspace root manifest's `workspace.lints`" + }) + } else { + Ok(lints.lints) + } +} + +fn dependency_inherit_with<'a>( + dependency: manifest::InheritableDependency, + name: &str, + inherit: &dyn Fn() -> CargoResult<&'a InheritableFields>, + package_root: &Path, + warnings: &mut Vec, +) -> CargoResult { + match dependency { + manifest::InheritableDependency::Value(value) => Ok(value), + manifest::InheritableDependency::Inherit(w) => { + inner_dependency_inherit_with(w, name, inherit, package_root, warnings).with_context(|| { + format!( + "error inheriting `{name}` from workspace root manifest's `workspace.dependencies.{name}`", + ) + }) + } + } +} + +fn inner_dependency_inherit_with<'a>( + dependency: manifest::TomlInheritedDependency, + name: &str, + inherit: &dyn Fn() -> CargoResult<&'a InheritableFields>, + package_root: &Path, + warnings: &mut Vec, +) -> CargoResult { + fn default_features_msg(label: &str, ws_def_feat: Option, warnings: &mut Vec) { + let ws_def_feat = match ws_def_feat { + Some(true) => "true", + Some(false) => "false", + None => "not specified", + }; + warnings.push(format!( + "`default-features` is ignored for {label}, since `default-features` was \ + {ws_def_feat} for `workspace.dependencies.{label}`, \ + this could become a hard error in the future" + )) + } + if dependency.default_features.is_some() && dependency.default_features2.is_some() { + warn_on_deprecated("default-features", name, "dependency", warnings); + } + inherit()?.get_dependency(name, package_root).map(|d| { + match d { + manifest::TomlDependency::Simple(s) => { + if let Some(false) = dependency.default_features() { + default_features_msg(name, None, warnings); + } + if dependency.optional.is_some() + || dependency.features.is_some() + || dependency.public.is_some() + { + manifest::TomlDependency::Detailed(manifest::TomlDetailedDependency { + version: Some(s), + optional: dependency.optional, + features: dependency.features.clone(), + public: dependency.public, + ..Default::default() + }) + } else { + manifest::TomlDependency::Simple(s) + } + } + manifest::TomlDependency::Detailed(d) => { + let mut d = d.clone(); + match (dependency.default_features(), d.default_features()) { + // member: default-features = true and + // workspace: default-features = false should turn on + // default-features + (Some(true), Some(false)) => { + d.default_features = Some(true); + } + // member: default-features = false and + // workspace: default-features = true should ignore member + // default-features + (Some(false), Some(true)) => { + default_features_msg(name, Some(true), warnings); + } + // member: default-features = false and + // workspace: dep = "1.0" should ignore member default-features + (Some(false), None) => { + default_features_msg(name, None, warnings); + } + _ => {} + } + d.features = match (d.features.clone(), dependency.features.clone()) { + (Some(dep_feat), Some(inherit_feat)) => Some( + dep_feat + .into_iter() + .chain(inherit_feat) + .collect::>(), + ), + (Some(dep_fet), None) => Some(dep_fet), + (None, Some(inherit_feat)) => Some(inherit_feat), + (None, None) => None, + }; + d.optional = dependency.optional; + manifest::TomlDependency::Detailed(d) + } + } + }) +} + +#[tracing::instrument(skip_all)] +fn to_real_manifest( contents: String, document: toml_edit::ImDocument, original_toml: manifest::TomlManifest, @@ -1303,172 +1428,6 @@ pub fn to_real_manifest( Ok(manifest) } -#[tracing::instrument(skip_all)] -fn resolve_dependencies<'a>( - gctx: &GlobalContext, - features: &Features, - orig_deps: Option<&BTreeMap>, - kind: Option, - inherit: &dyn Fn() -> CargoResult<&'a InheritableFields>, - package_root: &Path, - warnings: &mut Vec, -) -> CargoResult>> { - let Some(dependencies) = orig_deps else { - return Ok(None); - }; - - let mut deps = BTreeMap::new(); - for (name_in_toml, v) in dependencies.iter() { - let mut resolved = - dependency_inherit_with(v.clone(), name_in_toml, inherit, package_root, warnings)?; - if let manifest::TomlDependency::Detailed(ref mut d) = resolved { - if d.public.is_some() { - let public_feature = features.require(Feature::public_dependency()); - let with_public_feature = public_feature.is_ok(); - let with_z_public = gctx.cli_unstable().public_dependency; - if !with_public_feature && (!with_z_public && !gctx.nightly_features_allowed) { - public_feature?; - } - if matches!(kind, None) { - if !with_public_feature && !with_z_public { - d.public = None; - warnings.push(format!( - "ignoring `public` on dependency {name_in_toml}, pass `-Zpublic-dependency` to enable support for it" - )) - } - } else { - let kind_name = match kind { - Some(k) => k.kind_table(), - None => "dependencies", - }; - let hint = format!( - "'public' specifier can only be used on regular dependencies, not {kind_name}", - ); - if with_public_feature || with_z_public { - bail!(hint) - } else { - // If public feature isn't enabled in nightly, we instead warn that. - warnings.push(hint); - d.public = None; - } - } - } - } - - deps.insert( - name_in_toml.clone(), - manifest::InheritableDependency::Value(resolved.clone()), - ); - } - Ok(Some(deps)) -} - -#[tracing::instrument(skip_all)] -fn validate_dependencies( - original_deps: Option<&BTreeMap>, - platform: Option<&Platform>, - kind: Option, - warnings: &mut Vec, -) -> CargoResult<()> { - let Some(dependencies) = original_deps else { - return Ok(()); - }; - - for (name_in_toml, v) in dependencies.iter() { - let kind_name = match kind { - Some(k) => k.kind_table(), - None => "dependencies", - }; - let table_in_toml = if let Some(platform) = platform { - format!("target.{}.{kind_name}", platform.to_string()) - } else { - kind_name.to_string() - }; - unused_dep_keys(name_in_toml, &table_in_toml, v.unused_keys(), warnings); - } - Ok(()) -} - -#[tracing::instrument(skip_all)] -fn gather_dependencies( - manifest_ctx: &mut ManifestContext<'_, '_>, - resolved_deps: Option<&BTreeMap>, - kind: Option, -) -> CargoResult<()> { - let Some(dependencies) = resolved_deps else { - return Ok(()); - }; - - for (n, v) in dependencies.iter() { - let resolved = v.resolved().expect("previously resolved"); - let dep = dep_to_dependency(&resolved, n, manifest_ctx, kind)?; - manifest_ctx.deps.push(dep); - } - Ok(()) -} - -fn to_workspace_config( - original_toml: &manifest::TomlManifest, - manifest_file: &Path, - gctx: &GlobalContext, - warnings: &mut Vec, -) -> CargoResult { - let workspace_config = match ( - original_toml.workspace.as_ref(), - original_toml.package().and_then(|p| p.workspace.as_ref()), - ) { - (Some(toml_config), None) => { - verify_lints(toml_config.lints.as_ref(), gctx, warnings)?; - if let Some(ws_deps) = &toml_config.dependencies { - for (name, dep) in ws_deps { - if dep.is_optional() { - bail!("{name} is optional, but workspace dependencies cannot be optional",); - } - if dep.is_public() { - bail!("{name} is public, but workspace dependencies cannot be public",); - } - } - - for (name, dep) in ws_deps { - unused_dep_keys(name, "workspace.dependencies", dep.unused_keys(), warnings); - } - } - let ws_root_config = to_workspace_root_config(toml_config, manifest_file); - WorkspaceConfig::Root(ws_root_config) - } - (None, root) => WorkspaceConfig::Member { - root: root.cloned(), - }, - (Some(..), Some(..)) => bail!( - "cannot configure both `package.workspace` and \ - `[workspace]`, only one can be specified" - ), - }; - Ok(workspace_config) -} - -fn to_workspace_root_config( - resolved_toml: &manifest::TomlWorkspace, - manifest_file: &Path, -) -> WorkspaceRootConfig { - let package_root = manifest_file.parent().unwrap(); - let inheritable = InheritableFields { - package: resolved_toml.package.clone(), - dependencies: resolved_toml.dependencies.clone(), - lints: resolved_toml.lints.clone(), - _ws_root: package_root.to_owned(), - }; - let ws_root_config = WorkspaceRootConfig::new( - package_root, - &resolved_toml.members, - &resolved_toml.default_members, - &resolved_toml.exclude, - &Some(inheritable), - &resolved_toml.metadata, - ); - ws_root_config -} - fn to_virtual_manifest( contents: String, document: toml_edit::ImDocument, @@ -1528,6 +1487,59 @@ fn to_virtual_manifest( Ok(manifest) } +#[tracing::instrument(skip_all)] +fn validate_dependencies( + original_deps: Option<&BTreeMap>, + platform: Option<&Platform>, + kind: Option, + warnings: &mut Vec, +) -> CargoResult<()> { + let Some(dependencies) = original_deps else { + return Ok(()); + }; + + for (name_in_toml, v) in dependencies.iter() { + let kind_name = match kind { + Some(k) => k.kind_table(), + None => "dependencies", + }; + let table_in_toml = if let Some(platform) = platform { + format!("target.{}.{kind_name}", platform.to_string()) + } else { + kind_name.to_string() + }; + unused_dep_keys(name_in_toml, &table_in_toml, v.unused_keys(), warnings); + } + Ok(()) +} + +struct ManifestContext<'a, 'b> { + deps: &'a mut Vec, + source_id: SourceId, + gctx: &'b GlobalContext, + warnings: &'a mut Vec, + platform: Option, + root: &'a Path, +} + +#[tracing::instrument(skip_all)] +fn gather_dependencies( + manifest_ctx: &mut ManifestContext<'_, '_>, + resolved_deps: Option<&BTreeMap>, + kind: Option, +) -> CargoResult<()> { + let Some(dependencies) = resolved_deps else { + return Ok(()); + }; + + for (n, v) in dependencies.iter() { + let resolved = v.resolved().expect("previously resolved"); + let dep = dep_to_dependency(&resolved, n, manifest_ctx, kind)?; + manifest_ctx.deps.push(dep); + } + Ok(()) +} + fn replace( me: &manifest::TomlManifest, manifest_ctx: &mut ManifestContext<'_, '_>, @@ -1613,480 +1625,6 @@ fn patch( Ok(patch) } -struct ManifestContext<'a, 'b> { - deps: &'a mut Vec, - source_id: SourceId, - gctx: &'b GlobalContext, - warnings: &'a mut Vec, - platform: Option, - root: &'a Path, -} - -fn verify_lints( - lints: Option<&manifest::TomlLints>, - gctx: &GlobalContext, - warnings: &mut Vec, -) -> CargoResult<()> { - let Some(lints) = lints else { - return Ok(()); - }; - - for (tool, lints) in lints { - let supported = ["cargo", "clippy", "rust", "rustdoc"]; - if !supported.contains(&tool.as_str()) { - let supported = supported.join(", "); - anyhow::bail!("unsupported `{tool}` in `[lints]`, must be one of {supported}") - } - if tool == "cargo" && !gctx.cli_unstable().cargo_lints { - warn_for_cargo_lint_feature(gctx, warnings); - } - for name in lints.keys() { - if let Some((prefix, suffix)) = name.split_once("::") { - if tool == prefix { - anyhow::bail!( - "`lints.{tool}.{name}` is not valid lint name; try `lints.{prefix}.{suffix}`" - ) - } else if tool == "rust" && supported.contains(&prefix) { - anyhow::bail!( - "`lints.{tool}.{name}` is not valid lint name; try `lints.{prefix}.{suffix}`" - ) - } else { - anyhow::bail!("`lints.{tool}.{name}` is not a valid lint name") - } - } - } - } - - Ok(()) -} - -fn warn_for_cargo_lint_feature(gctx: &GlobalContext, warnings: &mut Vec) { - use std::fmt::Write as _; - - let key_name = "lints.cargo"; - let feature_name = "cargo-lints"; - - let mut message = String::new(); - - let _ = write!( - message, - "unused manifest key `{key_name}` (may be supported in a future version)" - ); - if gctx.nightly_features_allowed { - let _ = write!( - message, - " - -consider passing `-Z{feature_name}` to enable this feature." - ); - } else { - let _ = write!( - message, - " - -this Cargo does not support nightly features, but if you -switch to nightly channel you can pass -`-Z{feature_name}` to enable this feature.", - ); - } - warnings.push(message); -} - -fn lints_to_rustflags(lints: &manifest::TomlLints) -> Vec { - let mut rustflags = lints - .iter() - // We don't want to pass any of the `cargo` lints to `rustc` - .filter(|(tool, _)| tool != &"cargo") - .flat_map(|(tool, lints)| { - lints.iter().map(move |(name, config)| { - let flag = match config.level() { - manifest::TomlLintLevel::Forbid => "--forbid", - manifest::TomlLintLevel::Deny => "--deny", - manifest::TomlLintLevel::Warn => "--warn", - manifest::TomlLintLevel::Allow => "--allow", - }; - - let option = if tool == "rust" { - format!("{flag}={name}") - } else { - format!("{flag}={tool}::{name}") - }; - ( - config.priority(), - // Since the most common group will be `all`, put it last so people are more - // likely to notice that they need to use `priority`. - std::cmp::Reverse(name), - option, - ) - }) - }) - .collect::>(); - rustflags.sort(); - rustflags.into_iter().map(|(_, _, option)| option).collect() -} - -fn unused_dep_keys( - dep_name: &str, - kind: &str, - unused_keys: Vec, - warnings: &mut Vec, -) { - for unused in unused_keys { - let key = format!("unused manifest key: {kind}.{dep_name}.{unused}"); - warnings.push(key); - } -} - -fn load_inheritable_fields( - gctx: &GlobalContext, - resolved_path: &Path, - workspace_config: &WorkspaceConfig, -) -> CargoResult { - match workspace_config { - WorkspaceConfig::Root(root) => Ok(root.inheritable().clone()), - WorkspaceConfig::Member { - root: Some(ref path_to_root), - } => { - let path = resolved_path - .parent() - .unwrap() - .join(path_to_root) - .join("Cargo.toml"); - let root_path = paths::normalize_path(&path); - inheritable_from_path(gctx, root_path) - } - WorkspaceConfig::Member { root: None } => { - match find_workspace_root(&resolved_path, gctx)? { - Some(path_to_root) => inheritable_from_path(gctx, path_to_root), - None => Err(anyhow!("failed to find a workspace root")), - } - } - } -} - -fn inheritable_from_path( - gctx: &GlobalContext, - workspace_path: PathBuf, -) -> CargoResult { - // Workspace path should have Cargo.toml at the end - let workspace_path_root = workspace_path.parent().unwrap(); - - // Let the borrow exit scope so that it can be picked up if there is a need to - // read a manifest - if let Some(ws_root) = gctx.ws_roots.borrow().get(workspace_path_root) { - return Ok(ws_root.inheritable().clone()); - }; - - let source_id = SourceId::for_path(workspace_path_root)?; - let man = read_manifest(&workspace_path, source_id, gctx)?; - match man.workspace_config() { - WorkspaceConfig::Root(root) => { - gctx.ws_roots - .borrow_mut() - .insert(workspace_path, root.clone()); - Ok(root.inheritable().clone()) - } - _ => bail!( - "root of a workspace inferred but wasn't a root: {}", - workspace_path.display() - ), - } -} - -/// Returns the name of the README file for a [`manifest::TomlPackage`]. -fn readme_for_package( - package_root: &Path, - readme: Option<&manifest::StringOrBool>, -) -> Option { - match &readme { - None => default_readme_from_package_root(package_root), - Some(value) => match value { - manifest::StringOrBool::Bool(false) => None, - manifest::StringOrBool::Bool(true) => Some("README.md".to_string()), - manifest::StringOrBool::String(v) => Some(v.clone()), - }, - } -} - -const DEFAULT_README_FILES: [&str; 3] = ["README.md", "README.txt", "README"]; - -/// Checks if a file with any of the default README file names exists in the package root. -/// If so, returns a `String` representing that name. -fn default_readme_from_package_root(package_root: &Path) -> Option { - for &readme_filename in DEFAULT_README_FILES.iter() { - if package_root.join(readme_filename).is_file() { - return Some(readme_filename.to_string()); - } - } - - None -} - -/// Checks a list of build targets, and ensures the target names are unique within a vector. -/// If not, the name of the offending build target is returned. -#[tracing::instrument(skip_all)] -fn unique_build_targets( - targets: &[Target], - package_root: &Path, -) -> Result<(), HashMap>> { - let mut source_targets = HashMap::<_, Vec<_>>::new(); - for target in targets { - if let TargetSourcePath::Path(path) = target.src_path() { - let full = package_root.join(path); - source_targets.entry(full).or_default().push(target.clone()); - } - } - - let conflict_targets = source_targets - .into_iter() - .filter(|(_, targets)| targets.len() > 1) - .collect::>(); - - if !conflict_targets.is_empty() { - return Err(conflict_targets); - } - - Ok(()) -} - -/// Defines simple getter methods for inheritable fields. -macro_rules! package_field_getter { - ( $(($key:literal, $field:ident -> $ret:ty),)* ) => ( - $( - #[doc = concat!("Gets the field `workspace.package", $key, "`.")] - fn $field(&self) -> CargoResult<$ret> { - let Some(val) = self.package.as_ref().and_then(|p| p.$field.as_ref()) else { - bail!("`workspace.package.{}` was not defined", $key); - }; - Ok(val.clone()) - } - )* - ) -} - -/// A group of fields that are inheritable by members of the workspace -#[derive(Clone, Debug, Default)] -pub struct InheritableFields { - package: Option, - dependencies: Option>, - lints: Option, - - // Bookkeeping to help when resolving values from above - _ws_root: PathBuf, -} - -impl InheritableFields { - package_field_getter! { - // Please keep this list lexicographically ordered. - ("authors", authors -> Vec), - ("badges", badges -> BTreeMap>), - ("categories", categories -> Vec), - ("description", description -> String), - ("documentation", documentation -> String), - ("edition", edition -> String), - ("exclude", exclude -> Vec), - ("homepage", homepage -> String), - ("include", include -> Vec), - ("keywords", keywords -> Vec), - ("license", license -> String), - ("publish", publish -> manifest::VecStringOrBool), - ("repository", repository -> String), - ("rust-version", rust_version -> RustVersion), - ("version", version -> semver::Version), - } - - /// Gets a workspace dependency with the `name`. - fn get_dependency( - &self, - name: &str, - package_root: &Path, - ) -> CargoResult { - let Some(deps) = &self.dependencies else { - bail!("`workspace.dependencies` was not defined"); - }; - let Some(dep) = deps.get(name) else { - bail!("`dependency.{name}` was not found in `workspace.dependencies`"); - }; - let mut dep = dep.clone(); - if let manifest::TomlDependency::Detailed(detailed) = &mut dep { - if let Some(rel_path) = &detailed.path { - detailed.path = Some(resolve_relative_path( - name, - self.ws_root(), - package_root, - rel_path, - )?); - } - } - Ok(dep) - } - - /// Gets the field `workspace.lint`. - fn lints(&self) -> CargoResult { - let Some(val) = &self.lints else { - bail!("`workspace.lints` was not defined"); - }; - Ok(val.clone()) - } - - /// Gets the field `workspace.package.license-file`. - fn license_file(&self, package_root: &Path) -> CargoResult { - let Some(license_file) = self.package.as_ref().and_then(|p| p.license_file.as_ref()) else { - bail!("`workspace.package.license-file` was not defined"); - }; - resolve_relative_path("license-file", &self._ws_root, package_root, license_file) - } - - /// Gets the field `workspace.package.readme`. - fn readme(&self, package_root: &Path) -> CargoResult { - let Some(readme) = readme_for_package( - self._ws_root.as_path(), - self.package.as_ref().and_then(|p| p.readme.as_ref()), - ) else { - bail!("`workspace.package.readme` was not defined"); - }; - resolve_relative_path("readme", &self._ws_root, package_root, &readme) - .map(manifest::StringOrBool::String) - } - - fn ws_root(&self) -> &PathBuf { - &self._ws_root - } -} - -fn field_inherit_with<'a, T>( - field: manifest::InheritableField, - label: &str, - get_ws_inheritable: impl FnOnce() -> CargoResult, -) -> CargoResult { - match field { - manifest::InheritableField::Value(value) => Ok(value), - manifest::InheritableField::Inherit(_) => get_ws_inheritable().with_context(|| { - format!( - "error inheriting `{label}` from workspace root manifest's `workspace.package.{label}`", - ) - }), - } -} - -fn lints_inherit_with( - lints: manifest::InheritableLints, - get_ws_inheritable: impl FnOnce() -> CargoResult, -) -> CargoResult { - if lints.workspace { - if !lints.lints.is_empty() { - anyhow::bail!("cannot override `workspace.lints` in `lints`, either remove the overrides or `lints.workspace = true` and manually specify the lints"); - } - get_ws_inheritable().with_context(|| { - "error inheriting `lints` from workspace root manifest's `workspace.lints`" - }) - } else { - Ok(lints.lints) - } -} - -fn dependency_inherit_with<'a>( - dependency: manifest::InheritableDependency, - name: &str, - inherit: &dyn Fn() -> CargoResult<&'a InheritableFields>, - package_root: &Path, - warnings: &mut Vec, -) -> CargoResult { - match dependency { - manifest::InheritableDependency::Value(value) => Ok(value), - manifest::InheritableDependency::Inherit(w) => { - inner_dependency_inherit_with(w, name, inherit, package_root, warnings).with_context(|| { - format!( - "error inheriting `{name}` from workspace root manifest's `workspace.dependencies.{name}`", - ) - }) - } - } -} - -fn inner_dependency_inherit_with<'a>( - dependency: manifest::TomlInheritedDependency, - name: &str, - inherit: &dyn Fn() -> CargoResult<&'a InheritableFields>, - package_root: &Path, - warnings: &mut Vec, -) -> CargoResult { - fn default_features_msg(label: &str, ws_def_feat: Option, warnings: &mut Vec) { - let ws_def_feat = match ws_def_feat { - Some(true) => "true", - Some(false) => "false", - None => "not specified", - }; - warnings.push(format!( - "`default-features` is ignored for {label}, since `default-features` was \ - {ws_def_feat} for `workspace.dependencies.{label}`, \ - this could become a hard error in the future" - )) - } - if dependency.default_features.is_some() && dependency.default_features2.is_some() { - warn_on_deprecated("default-features", name, "dependency", warnings); - } - inherit()?.get_dependency(name, package_root).map(|d| { - match d { - manifest::TomlDependency::Simple(s) => { - if let Some(false) = dependency.default_features() { - default_features_msg(name, None, warnings); - } - if dependency.optional.is_some() - || dependency.features.is_some() - || dependency.public.is_some() - { - manifest::TomlDependency::Detailed(manifest::TomlDetailedDependency { - version: Some(s), - optional: dependency.optional, - features: dependency.features.clone(), - public: dependency.public, - ..Default::default() - }) - } else { - manifest::TomlDependency::Simple(s) - } - } - manifest::TomlDependency::Detailed(d) => { - let mut d = d.clone(); - match (dependency.default_features(), d.default_features()) { - // member: default-features = true and - // workspace: default-features = false should turn on - // default-features - (Some(true), Some(false)) => { - d.default_features = Some(true); - } - // member: default-features = false and - // workspace: default-features = true should ignore member - // default-features - (Some(false), Some(true)) => { - default_features_msg(name, Some(true), warnings); - } - // member: default-features = false and - // workspace: dep = "1.0" should ignore member default-features - (Some(false), None) => { - default_features_msg(name, None, warnings); - } - _ => {} - } - d.features = match (d.features.clone(), dependency.features.clone()) { - (Some(dep_feat), Some(inherit_feat)) => Some( - dep_feat - .into_iter() - .chain(inherit_feat) - .collect::>(), - ), - (Some(dep_fet), None) => Some(dep_fet), - (None, Some(inherit_feat)) => Some(inherit_feat), - (None, None) => None, - }; - d.optional = dependency.optional; - manifest::TomlDependency::Detailed(d) - } - } - }) -} - pub(crate) fn to_dependency( dep: &manifest::TomlDependency

, name: &str, @@ -2371,6 +1909,49 @@ fn detailed_dep_to_dependency( Ok(dep) } +pub trait ResolveToPath { + fn resolve(&self, gctx: &GlobalContext) -> PathBuf; +} + +impl ResolveToPath for String { + fn resolve(&self, _: &GlobalContext) -> PathBuf { + self.into() + } +} + +impl ResolveToPath for ConfigRelativePath { + fn resolve(&self, gctx: &GlobalContext) -> PathBuf { + self.resolve_path(gctx) + } +} + +/// Checks a list of build targets, and ensures the target names are unique within a vector. +/// If not, the name of the offending build target is returned. +#[tracing::instrument(skip_all)] +fn unique_build_targets( + targets: &[Target], + package_root: &Path, +) -> Result<(), HashMap>> { + let mut source_targets = HashMap::<_, Vec<_>>::new(); + for target in targets { + if let TargetSourcePath::Path(path) = target.src_path() { + let full = package_root.join(path); + source_targets.entry(full).or_default().push(target.clone()); + } + } + + let conflict_targets = source_targets + .into_iter() + .filter(|(_, targets)| targets.len() > 1) + .collect::>(); + + if !conflict_targets.is_empty() { + return Err(conflict_targets); + } + + Ok(()) +} + /// Checks syntax validity and unstable feature gate for each profile. /// /// It's a bit unfortunate both `-Z` flags and `cargo-features` are required, @@ -2540,18 +2121,437 @@ fn validate_profile_override(profile: &manifest::TomlProfile, which: &str) -> Ca Ok(()) } -pub trait ResolveToPath { - fn resolve(&self, gctx: &GlobalContext) -> PathBuf; +fn verify_lints( + lints: Option<&manifest::TomlLints>, + gctx: &GlobalContext, + warnings: &mut Vec, +) -> CargoResult<()> { + let Some(lints) = lints else { + return Ok(()); + }; + + for (tool, lints) in lints { + let supported = ["cargo", "clippy", "rust", "rustdoc"]; + if !supported.contains(&tool.as_str()) { + let supported = supported.join(", "); + anyhow::bail!("unsupported `{tool}` in `[lints]`, must be one of {supported}") + } + if tool == "cargo" && !gctx.cli_unstable().cargo_lints { + warn_for_cargo_lint_feature(gctx, warnings); + } + for name in lints.keys() { + if let Some((prefix, suffix)) = name.split_once("::") { + if tool == prefix { + anyhow::bail!( + "`lints.{tool}.{name}` is not valid lint name; try `lints.{prefix}.{suffix}`" + ) + } else if tool == "rust" && supported.contains(&prefix) { + anyhow::bail!( + "`lints.{tool}.{name}` is not valid lint name; try `lints.{prefix}.{suffix}`" + ) + } else { + anyhow::bail!("`lints.{tool}.{name}` is not a valid lint name") + } + } + } + } + + Ok(()) } -impl ResolveToPath for String { - fn resolve(&self, _: &GlobalContext) -> PathBuf { - self.into() +fn warn_for_cargo_lint_feature(gctx: &GlobalContext, warnings: &mut Vec) { + use std::fmt::Write as _; + + let key_name = "lints.cargo"; + let feature_name = "cargo-lints"; + + let mut message = String::new(); + + let _ = write!( + message, + "unused manifest key `{key_name}` (may be supported in a future version)" + ); + if gctx.nightly_features_allowed { + let _ = write!( + message, + " + +consider passing `-Z{feature_name}` to enable this feature." + ); + } else { + let _ = write!( + message, + " + +this Cargo does not support nightly features, but if you +switch to nightly channel you can pass +`-Z{feature_name}` to enable this feature.", + ); + } + warnings.push(message); +} + +fn lints_to_rustflags(lints: &manifest::TomlLints) -> Vec { + let mut rustflags = lints + .iter() + // We don't want to pass any of the `cargo` lints to `rustc` + .filter(|(tool, _)| tool != &"cargo") + .flat_map(|(tool, lints)| { + lints.iter().map(move |(name, config)| { + let flag = match config.level() { + manifest::TomlLintLevel::Forbid => "--forbid", + manifest::TomlLintLevel::Deny => "--deny", + manifest::TomlLintLevel::Warn => "--warn", + manifest::TomlLintLevel::Allow => "--allow", + }; + + let option = if tool == "rust" { + format!("{flag}={name}") + } else { + format!("{flag}={tool}::{name}") + }; + ( + config.priority(), + // Since the most common group will be `all`, put it last so people are more + // likely to notice that they need to use `priority`. + std::cmp::Reverse(name), + option, + ) + }) + }) + .collect::>(); + rustflags.sort(); + rustflags.into_iter().map(|(_, _, option)| option).collect() +} + +fn emit_diagnostic( + e: toml_edit::de::Error, + contents: &str, + manifest_file: &Path, + gctx: &GlobalContext, +) -> anyhow::Error { + let Some(span) = e.span() else { + return e.into(); + }; + + // Get the path to the manifest, relative to the cwd + let manifest_path = diff_paths(manifest_file, gctx.cwd()) + .unwrap_or_else(|| manifest_file.to_path_buf()) + .display() + .to_string(); + let message = Level::Error.title(e.message()).snippet( + Snippet::source(contents) + .origin(&manifest_path) + .fold(true) + .annotation(Level::Error.span(span)), + ); + let renderer = Renderer::styled().term_width( + gctx.shell() + .err_width() + .diagnostic_terminal_width() + .unwrap_or(annotate_snippets::renderer::DEFAULT_TERM_WIDTH), + ); + if let Err(err) = writeln!(gctx.shell().err(), "{}", renderer.render(message)) { + return err.into(); + } + return AlreadyPrintedError::new(e.into()).into(); +} + +/// Warn about paths that have been deprecated and may conflict. +fn warn_on_deprecated(new_path: &str, name: &str, kind: &str, warnings: &mut Vec) { + let old_path = new_path.replace("-", "_"); + warnings.push(format!( + "conflicting between `{new_path}` and `{old_path}` in the `{name}` {kind}.\n + `{old_path}` is ignored and not recommended for use in the future" + )) +} + +fn warn_on_unused(unused: &BTreeSet, warnings: &mut Vec) { + for key in unused { + warnings.push(format!("unused manifest key: {}", key)); + if key == "profiles.debug" { + warnings.push("use `[profile.dev]` to configure debug builds".to_string()); + } } } -impl ResolveToPath for ConfigRelativePath { - fn resolve(&self, gctx: &GlobalContext) -> PathBuf { - self.resolve_path(gctx) +fn unused_dep_keys( + dep_name: &str, + kind: &str, + unused_keys: Vec, + warnings: &mut Vec, +) { + for unused in unused_keys { + let key = format!("unused manifest key: {kind}.{dep_name}.{unused}"); + warnings.push(key); + } +} + +pub fn prepare_for_publish(me: &Package, ws: &Workspace<'_>) -> CargoResult { + let contents = me.manifest().contents(); + let document = me.manifest().document(); + let original_toml = prepare_toml_for_publish(me.manifest().resolved_toml(), ws, me.root())?; + let resolved_toml = original_toml.clone(); + let features = me.manifest().unstable_features().clone(); + let workspace_config = me.manifest().workspace_config().clone(); + let source_id = me.package_id().source_id(); + let mut warnings = Default::default(); + let mut errors = Default::default(); + let gctx = ws.gctx(); + let manifest = to_real_manifest( + contents.to_owned(), + document.clone(), + original_toml, + resolved_toml, + features, + workspace_config, + source_id, + me.manifest_path(), + gctx, + &mut warnings, + &mut errors, + )?; + let new_pkg = Package::new(manifest, me.manifest_path()); + Ok(new_pkg) +} + +/// Prepares the manifest for publishing. +// - Path and git components of dependency specifications are removed. +// - License path is updated to point within the package. +fn prepare_toml_for_publish( + me: &manifest::TomlManifest, + ws: &Workspace<'_>, + package_root: &Path, +) -> CargoResult { + let gctx = ws.gctx(); + + if me + .cargo_features + .iter() + .flat_map(|f| f.iter()) + .any(|f| f == "open-namespaces") + { + anyhow::bail!("cannot publish with `open-namespaces`") + } + + let mut package = me.package().unwrap().clone(); + package.workspace = None; + let current_resolver = package + .resolver + .as_ref() + .map(|r| ResolveBehavior::from_manifest(r)) + .unwrap_or_else(|| { + package + .edition + .as_ref() + .and_then(|e| e.as_value()) + .map(|e| Edition::from_str(e)) + .unwrap_or(Ok(Edition::Edition2015)) + .map(|e| e.default_resolve_behavior()) + })?; + if ws.resolve_behavior() != current_resolver { + // This ensures the published crate if built as a root (e.g. `cargo install`) will + // use the same resolver behavior it was tested with in the workspace. + // To avoid forcing a higher MSRV we don't explicitly set this if it would implicitly + // result in the same thing. + package.resolver = Some(ws.resolve_behavior().to_manifest()); + } + if let Some(license_file) = &package.license_file { + let license_file = license_file + .as_value() + .context("license file should have been resolved before `prepare_for_publish()`")?; + let license_path = Path::new(&license_file); + let abs_license_path = paths::normalize_path(&package_root.join(license_path)); + if abs_license_path.strip_prefix(package_root).is_err() { + // This path points outside of the package root. `cargo package` + // will copy it into the root, so adjust the path to this location. + package.license_file = Some(manifest::InheritableField::Value( + license_path + .file_name() + .unwrap() + .to_str() + .unwrap() + .to_string(), + )); + } + } + + if let Some(readme) = &package.readme { + let readme = readme + .as_value() + .context("readme should have been resolved before `prepare_for_publish()`")?; + match readme { + manifest::StringOrBool::String(readme) => { + let readme_path = Path::new(&readme); + let abs_readme_path = paths::normalize_path(&package_root.join(readme_path)); + if abs_readme_path.strip_prefix(package_root).is_err() { + // This path points outside of the package root. `cargo package` + // will copy it into the root, so adjust the path to this location. + package.readme = Some(manifest::InheritableField::Value( + manifest::StringOrBool::String( + readme_path + .file_name() + .unwrap() + .to_str() + .unwrap() + .to_string(), + ), + )); + } + } + manifest::StringOrBool::Bool(_) => {} + } + } + let all = |_d: &manifest::TomlDependency| true; + let mut manifest = manifest::TomlManifest { + package: Some(package), + project: None, + profile: me.profile.clone(), + lib: me.lib.clone(), + bin: me.bin.clone(), + example: me.example.clone(), + test: me.test.clone(), + bench: me.bench.clone(), + dependencies: map_deps(gctx, me.dependencies.as_ref(), all)?, + dev_dependencies: map_deps( + gctx, + me.dev_dependencies(), + manifest::TomlDependency::is_version_specified, + )?, + dev_dependencies2: None, + build_dependencies: map_deps(gctx, me.build_dependencies(), all)?, + build_dependencies2: None, + features: me.features.clone(), + target: match me.target.as_ref().map(|target_map| { + target_map + .iter() + .map(|(k, v)| { + Ok(( + k.clone(), + manifest::TomlPlatform { + dependencies: map_deps(gctx, v.dependencies.as_ref(), all)?, + dev_dependencies: map_deps( + gctx, + v.dev_dependencies(), + manifest::TomlDependency::is_version_specified, + )?, + dev_dependencies2: None, + build_dependencies: map_deps(gctx, v.build_dependencies(), all)?, + build_dependencies2: None, + }, + )) + }) + .collect() + }) { + Some(Ok(v)) => Some(v), + Some(Err(e)) => return Err(e), + None => None, + }, + replace: None, + patch: None, + workspace: None, + badges: me.badges.clone(), + cargo_features: me.cargo_features.clone(), + lints: me.lints.clone(), + _unused_keys: Default::default(), + }; + strip_features(&mut manifest); + return Ok(manifest); + + fn strip_features(manifest: &mut TomlManifest) { + fn insert_dep_name( + dep_name_set: &mut BTreeSet, + deps: Option<&BTreeMap>, + ) { + let Some(deps) = deps else { + return; + }; + deps.iter().for_each(|(k, _v)| { + dep_name_set.insert(k.clone()); + }); + } + let mut dep_name_set = BTreeSet::new(); + insert_dep_name(&mut dep_name_set, manifest.dependencies.as_ref()); + insert_dep_name(&mut dep_name_set, manifest.dev_dependencies()); + insert_dep_name(&mut dep_name_set, manifest.build_dependencies()); + if let Some(target_map) = manifest.target.as_ref() { + target_map.iter().for_each(|(_k, v)| { + insert_dep_name(&mut dep_name_set, v.dependencies.as_ref()); + insert_dep_name(&mut dep_name_set, v.dev_dependencies()); + insert_dep_name(&mut dep_name_set, v.build_dependencies()); + }); + } + let features = manifest.features.as_mut(); + + let Some(features) = features else { + return; + }; + + features.values_mut().for_each(|feature_deps| { + feature_deps.retain(|feature_dep| { + let feature_value = FeatureValue::new(InternedString::new(feature_dep)); + match feature_value { + FeatureValue::Dep { dep_name } | FeatureValue::DepFeature { dep_name, .. } => { + let k = &manifest::PackageName::new(dep_name.to_string()).unwrap(); + dep_name_set.contains(k) + } + _ => true, + } + }); + }); + } + + fn map_deps( + gctx: &GlobalContext, + deps: Option<&BTreeMap>, + filter: impl Fn(&manifest::TomlDependency) -> bool, + ) -> CargoResult>> { + let Some(deps) = deps else { + return Ok(None); + }; + let deps = deps + .iter() + .filter(|(_k, v)| { + if let manifest::InheritableDependency::Value(def) = v { + filter(def) + } else { + false + } + }) + .map(|(k, v)| Ok((k.clone(), map_dependency(gctx, v)?))) + .collect::>>()?; + Ok(Some(deps)) + } + + fn map_dependency( + gctx: &GlobalContext, + dep: &manifest::InheritableDependency, + ) -> CargoResult { + let dep = match dep { + manifest::InheritableDependency::Value(manifest::TomlDependency::Detailed(d)) => { + let mut d = d.clone(); + // Path dependencies become crates.io deps. + d.path.take(); + // Same with git dependencies. + d.git.take(); + d.branch.take(); + d.tag.take(); + d.rev.take(); + // registry specifications are elaborated to the index URL + if let Some(registry) = d.registry.take() { + d.registry_index = Some(gctx.get_registry_index(®istry)?.to_string()); + } + Ok(d) + } + manifest::InheritableDependency::Value(manifest::TomlDependency::Simple(s)) => { + Ok(manifest::TomlDetailedDependency { + version: Some(s.clone()), + ..Default::default() + }) + } + _ => unreachable!(), + }; + dep.map(manifest::TomlDependency::Detailed) + .map(manifest::InheritableDependency::Value) } }