From ccb321a633fe2ca4f25814e52efcc26de15d7c80 Mon Sep 17 00:00:00 2001 From: scott Date: Mon, 21 Mar 2022 21:27:49 -0500 Subject: [PATCH 1/3] -- Part 1 of RFC2906 --- src/cargo/core/compiler/standard_lib.rs | 1 + src/cargo/core/features.rs | 3 + src/cargo/core/mod.rs | 4 +- src/cargo/core/workspace.rs | 135 +++- src/cargo/util/toml/mod.rs | 833 +++++++++++++++++++---- src/doc/src/reference/unstable.md | 63 ++ tests/testsuite/deduplicate_workspace.rs | 817 ++++++++++++++++++++++ tests/testsuite/main.rs | 1 + 8 files changed, 1704 insertions(+), 153 deletions(-) create mode 100644 tests/testsuite/deduplicate_workspace.rs diff --git a/src/cargo/core/compiler/standard_lib.rs b/src/cargo/core/compiler/standard_lib.rs index e556fe9d566..2e253291888 100644 --- a/src/cargo/core/compiler/standard_lib.rs +++ b/src/cargo/core/compiler/standard_lib.rs @@ -65,6 +65,7 @@ pub fn resolve_std<'cfg>( &Some(members), /*default_members*/ &None, /*exclude*/ &None, + /*inheritable*/ &None, /*custom_metadata*/ &None, )); let virtual_manifest = crate::core::VirtualManifest::new( diff --git a/src/cargo/core/features.rs b/src/cargo/core/features.rs index cc8d0a3cdbe..98653a19a39 100644 --- a/src/cargo/core/features.rs +++ b/src/cargo/core/features.rs @@ -412,6 +412,9 @@ features! { // Allow specifying rustflags directly in a profile (unstable, profile_rustflags, "", "reference/unstable.html#profile-rustflags-option"), + + // Allow specifying rustflags directly in a profile + (unstable, workspace_inheritance, "", "reference/unstable.html#workspace-inheritance"), } pub struct Feature { diff --git a/src/cargo/core/mod.rs b/src/cargo/core/mod.rs index aec49b143bd..b52fdef63de 100644 --- a/src/cargo/core/mod.rs +++ b/src/cargo/core/mod.rs @@ -10,7 +10,9 @@ pub use self::resolver::{Resolve, ResolveVersion}; pub use self::shell::{Shell, Verbosity}; pub use self::source::{GitReference, Source, SourceId, SourceMap}; pub use self::summary::{FeatureMap, FeatureValue, Summary}; -pub use self::workspace::{MaybePackage, Workspace, WorkspaceConfig, WorkspaceRootConfig}; +pub use self::workspace::{ + InheritableFields, MaybePackage, Workspace, WorkspaceConfig, WorkspaceRootConfig, +}; pub mod compiler; pub mod dependency; diff --git a/src/cargo/core/workspace.rs b/src/cargo/core/workspace.rs index 189971dab30..34e8150c3d1 100644 --- a/src/cargo/core/workspace.rs +++ b/src/cargo/core/workspace.rs @@ -22,7 +22,9 @@ use crate::sources::{PathSource, CRATES_IO_INDEX, CRATES_IO_REGISTRY}; use crate::util::errors::{CargoResult, ManifestError}; use crate::util::interning::InternedString; use crate::util::lev_distance; -use crate::util::toml::{read_manifest, TomlDependency, TomlProfiles}; +use crate::util::toml::{ + read_manifest, StringOrBool, TomlDependency, TomlProfiles, VecStringOrBool, +}; use crate::util::{config::ConfigRelativePath, Config, Filesystem, IntoUrl}; use cargo_util::paths; @@ -123,6 +125,15 @@ pub enum WorkspaceConfig { Member { root: Option }, } +impl WorkspaceConfig { + pub fn inheritable(&self) -> Option<&InheritableFields> { + match self { + WorkspaceConfig::Root(root) => Some(&root.inheritable_fields), + WorkspaceConfig::Member { .. } => None, + } + } +} + /// Intermediate configuration of a workspace root in a manifest. /// /// Knows the Workspace Root path, as well as `members` and `exclude` lists of path patterns, which @@ -133,6 +144,7 @@ pub struct WorkspaceRootConfig { members: Option>, default_members: Option>, exclude: Vec, + inheritable_fields: InheritableFields, custom_metadata: Option, } @@ -1567,6 +1579,7 @@ impl WorkspaceRootConfig { members: &Option>, default_members: &Option>, exclude: &Option>, + inheritable: &Option, custom_metadata: &Option, ) -> WorkspaceRootConfig { WorkspaceRootConfig { @@ -1574,10 +1587,10 @@ impl WorkspaceRootConfig { members: members.clone(), default_members: default_members.clone(), exclude: exclude.clone().unwrap_or_default(), + inheritable_fields: inheritable.clone().unwrap_or_default(), custom_metadata: custom_metadata.clone(), } } - /// Checks the path against the `excluded` list. /// /// This method does **not** consider the `members` list. @@ -1641,3 +1654,121 @@ impl WorkspaceRootConfig { Ok(res) } } + +/// A group of fields that are inheritable by members of the workspace +#[derive(Clone, Debug, Default)] +pub struct InheritableFields { + dependencies: Option>, + version: Option, + authors: Option>, + description: Option, + homepage: Option, + documentation: Option, + readme: Option, + keywords: Option>, + categories: Option>, + license: Option, + license_file: Option, + repository: Option, + publish: Option, + edition: Option, + badges: Option>>, +} + +impl InheritableFields { + pub fn new( + dependencies: Option>, + version: Option, + authors: Option>, + description: Option, + homepage: Option, + documentation: Option, + readme: Option, + keywords: Option>, + categories: Option>, + license: Option, + license_file: Option, + repository: Option, + publish: Option, + edition: Option, + badges: Option>>, + ) -> InheritableFields { + Self { + dependencies, + version, + authors, + description, + homepage, + documentation, + readme, + keywords, + categories, + license, + license_file, + repository, + publish, + edition, + badges, + } + } + + pub fn dependencies(&self) -> Option> { + self.dependencies.clone() + } + + pub fn version(&self) -> Option { + self.version.clone() + } + + pub fn authors(&self) -> Option> { + self.authors.clone() + } + + pub fn description(&self) -> Option { + self.description.clone() + } + + pub fn homepage(&self) -> Option { + self.homepage.clone() + } + + pub fn documentation(&self) -> Option { + self.documentation.clone() + } + + pub fn readme(&self) -> Option { + self.readme.clone() + } + + pub fn keywords(&self) -> Option> { + self.keywords.clone() + } + + pub fn categories(&self) -> Option> { + self.categories.clone() + } + + pub fn license(&self) -> Option { + self.license.clone() + } + + pub fn license_file(&self) -> Option { + self.license_file.clone() + } + + pub fn repository(&self) -> Option { + self.repository.clone() + } + + pub fn publish(&self) -> Option { + self.publish.clone() + } + + pub fn edition(&self) -> Option { + self.edition.clone() + } + + pub fn badges(&self) -> Option>> { + self.badges.clone() + } +} diff --git a/src/cargo/util/toml/mod.rs b/src/cargo/util/toml/mod.rs index 9a3a2c42211..3892080ab23 100644 --- a/src/cargo/util/toml/mod.rs +++ b/src/cargo/util/toml/mod.rs @@ -21,7 +21,9 @@ use crate::core::dependency::{Artifact, ArtifactTarget, DepKind}; use crate::core::manifest::{ManifestMetadata, TargetSourcePath, Warnings}; use crate::core::resolver::ResolveBehavior; use crate::core::{Dependency, Manifest, PackageId, Summary, Target}; -use crate::core::{Edition, EitherManifest, Feature, Features, VirtualManifest, Workspace}; +use crate::core::{ + Edition, EitherManifest, Feature, Features, InheritableFields, VirtualManifest, Workspace, +}; use crate::core::{GitReference, PackageIdSpec, SourceId, WorkspaceConfig, WorkspaceRootConfig}; use crate::sources::{CRATES_IO_INDEX, CRATES_IO_REGISTRY}; use crate::util::errors::{CargoResult, ManifestError}; @@ -113,6 +115,20 @@ pub fn read_manifest_from_str( }; let manifest = Rc::new(manifest); + if let Some(deps) = manifest + .workspace + .as_ref() + .and_then(|ws| ws.dependencies.as_ref()) + { + for (name, dep) in deps { + if dep.is_optional() { + bail!( + "{} is optional, but workspace dependencies cannot be optional", + name + ); + } + } + } return if manifest.project.is_some() || manifest.package.is_some() { let (mut manifest, paths) = TomlManifest::to_real_manifest(&manifest, source_id, package_root, config)?; @@ -197,24 +213,26 @@ type TomlBenchTarget = TomlTarget; #[derive(Clone, Debug, Serialize)] #[serde(untagged)] -pub enum TomlDependency

{ +pub enum TomlDependency { /// In the simple format, only a version is specified, eg. /// `package = ""` Simple(String), + /// `package = { workspace = true }` + Workspace(TomlWorkspaceDependency), /// The simple format is equivalent to a detailed dependency /// specifying only a version, eg. /// `package = { version = "" }` Detailed(DetailedTomlDependency

), } -impl<'de, P: Deserialize<'de>> de::Deserialize<'de> for TomlDependency

{ +impl<'de, P: Deserialize<'de> + Clone> de::Deserialize<'de> for TomlDependency

{ fn deserialize(deserializer: D) -> Result where D: de::Deserializer<'de>, { struct TomlDependencyVisitor

(PhantomData

); - impl<'de, P: Deserialize<'de>> de::Visitor<'de> for TomlDependencyVisitor

{ + impl<'de, P: Deserialize<'de> + Clone> de::Visitor<'de> for TomlDependencyVisitor

{ type Value = TomlDependency

; fn expecting(&self, formatter: &mut fmt::Formatter<'_>) -> fmt::Result { @@ -236,7 +254,38 @@ impl<'de, P: Deserialize<'de>> de::Deserialize<'de> for TomlDependency

{ V: de::MapAccess<'de>, { let mvd = de::value::MapAccessDeserializer::new(map); - DetailedTomlDependency::deserialize(mvd).map(TomlDependency::Detailed) + let details: IntermediateDependency

= IntermediateDependency::deserialize(mvd)?; + if let Some(workspace) = details.workspace { + if workspace { + Ok(TomlDependency::Workspace(TomlWorkspaceDependency { + workspace: true, + features: details.features, + optional: details.optional, + })) + } else { + return Err(de::Error::custom("workspace cannot be false")); + } + } else { + Ok(TomlDependency::Detailed(DetailedTomlDependency { + version: details.version, + registry: details.registry, + registry_index: details.registry_index, + path: details.path, + git: details.git, + branch: details.branch, + tag: details.tag, + rev: details.rev, + features: details.features, + optional: details.optional, + default_features: details.default_features, + default_features2: details.default_features2, + package: details.package, + public: details.public, + artifact: details.artifact, + lib: details.lib, + target: details.target, + })) + } } } @@ -260,9 +309,42 @@ impl ResolveToPath for ConfigRelativePath { } } +// This is here due to parsing of TomlDependency works. +// At the time of writing it can not be derived in anyway I could find. +#[derive(Deserialize, Debug)] +#[serde(rename_all = "kebab-case")] +pub struct IntermediateDependency

{ + workspace: Option, + version: Option, + registry: Option, + registry_index: Option, + path: Option

, + git: Option, + branch: Option, + tag: Option, + rev: Option, + features: Option>, + optional: Option, + default_features: Option, + #[serde(rename = "default_features")] + default_features2: Option, + package: Option, + public: Option, + artifact: Option, + lib: Option, + target: Option, +} + +#[derive(Deserialize, Serialize, Clone, Debug)] +pub struct TomlWorkspaceDependency { + workspace: bool, + features: Option>, + optional: Option, +} + #[derive(Deserialize, Serialize, Clone, Debug)] #[serde(rename_all = "kebab-case")] -pub struct DetailedTomlDependency

{ +pub struct DetailedTomlDependency { version: Option, registry: Option, /// The URL of the `registry` field. @@ -296,7 +378,7 @@ pub struct DetailedTomlDependency

{ } // Explicit implementation so we avoid pulling in P: Default -impl

Default for DetailedTomlDependency

{ +impl Default for DetailedTomlDependency

{ fn default() -> Self { Self { version: Default::default(), @@ -345,7 +427,7 @@ pub struct TomlManifest { replace: Option>, patch: Option>>, workspace: Option, - badges: Option>>, + badges: Option>>>, } #[derive(Deserialize, Serialize, Clone, Debug, Default)] @@ -863,14 +945,16 @@ impl<'de> de::Deserialize<'de> for VecStringOrBool { } } -fn version_trim_whitespace<'de, D>(deserializer: D) -> Result +fn version_trim_whitespace<'de, D>( + deserializer: D, +) -> Result, D::Error> where D: de::Deserializer<'de>, { struct Visitor; impl<'de> de::Visitor<'de> for Visitor { - type Value = semver::Version; + type Value = MaybeWorkspace; fn expecting(&self, formatter: &mut fmt::Formatter<'_>) -> fmt::Result { formatter.write_str("SemVer version") @@ -880,11 +964,74 @@ where where E: de::Error, { - string.trim().parse().map_err(de::Error::custom) + match string.trim().parse().map_err(de::Error::custom) { + Ok(parsed) => Ok(MaybeWorkspace::Defined(parsed)), + Err(e) => Err(e), + } + } + + fn visit_map(self, map: V) -> Result + where + V: de::MapAccess<'de>, + { + let mvd = de::value::MapAccessDeserializer::new(map); + TomlWorkspaceField::deserialize(mvd).map(MaybeWorkspace::Workspace) + } + } + + deserializer.deserialize_any(Visitor) +} + +/// Enum that allows for the parsing of { workspace = true } in a Cargo.toml +/// +/// It allows for things to be inherited from a workspace or defined as needed +#[derive(Deserialize, Serialize, Clone, Debug)] +#[serde(untagged)] +pub enum MaybeWorkspace { + Workspace(TomlWorkspaceField), + Defined(T), +} + +impl MaybeWorkspace { + fn resolve<'a>( + self, + cargo_features: &Features, + label: &str, + inherit: impl FnOnce() -> Option<&'a InheritableFields>, + get_ws: impl FnOnce(&InheritableFields) -> Option, + ) -> CargoResult { + match self { + MaybeWorkspace::Defined(value) => Ok(value), + MaybeWorkspace::Workspace(TomlWorkspaceField { workspace: true }) => { + cargo_features.require(Feature::workspace_inheritance())?; + let inherit = inherit().context(format!( + "You cannot inherit fields from a parent workspace currently, tried to on {}", + label + ))?; + get_ws(inherit).context(format!( + "error reading `{}` from workspace root manifest's `[workspace.{}]`", + label, label + )) + } + MaybeWorkspace::Workspace(TomlWorkspaceField { workspace: false }) => Err(anyhow!( + "workspace cannot be false for key `package.{label}`", + )), + } + } + + /// This does not try to resolve a `MaybeWorkspace` it gets a &T if it is defined. + /// If it is not defined it will return an error + fn inner(&self, label: &str) -> CargoResult<&T> { + match self { + MaybeWorkspace::Workspace(_) => Err(anyhow!("{} has not been resolved yet`", label)), + MaybeWorkspace::Defined(d) => Ok(d), } } +} - deserializer.deserialize_str(Visitor) +#[derive(Deserialize, Serialize, Clone, Debug)] +pub struct TomlWorkspaceField { + workspace: bool, } /// Represents the `package`/`project` sections of a `Cargo.toml`. @@ -896,12 +1043,12 @@ where #[derive(Deserialize, Serialize, Clone, Debug)] #[serde(rename_all = "kebab-case")] pub struct TomlProject { - edition: Option, + edition: Option>, rust_version: Option, name: InternedString, #[serde(deserialize_with = "version_trim_whitespace")] - version: semver::Version, - authors: Option>, + version: MaybeWorkspace, + authors: Option>>, build: Option, metabuild: Option, #[serde(rename = "default-target")] @@ -911,7 +1058,7 @@ pub struct TomlProject { links: Option, exclude: Option>, include: Option>, - publish: Option, + publish: Option>, workspace: Option, im_a_teapot: Option, autobins: Option, @@ -921,15 +1068,15 @@ pub struct TomlProject { default_run: Option, // Package metadata. - description: Option, - homepage: Option, - documentation: Option, - readme: Option, - keywords: Option>, - categories: Option>, - license: Option, - license_file: Option, - repository: Option, + description: Option>, + homepage: Option>, + documentation: Option>, + readme: Option>, + keywords: Option>>, + categories: Option>>, + license: Option>, + license_file: Option>, + repository: Option>, resolver: Option, // Note that this field must come last due to the way toml serialization @@ -937,7 +1084,7 @@ pub struct TomlProject { metadata: Option, } -#[derive(Debug, Deserialize, Serialize)] +#[derive(Debug, Deserialize, Serialize, Clone)] pub struct TomlWorkspace { members: Option>, #[serde(rename = "default-members")] @@ -945,14 +1092,36 @@ pub struct TomlWorkspace { exclude: Option>, resolver: Option, + // Properties that can be inherited by members. + dependencies: Option>, + version: Option, + authors: Option>, + description: Option, + documentation: Option, + readme: Option, + homepage: Option, + repository: Option, + license: Option, + #[serde(rename = "license-file")] + license_file: Option, + keywords: Option>, + categories: Option>, + publish: Option, + edition: Option, + badges: Option>>, + // Note that this field must come last due to the way toml serialization // works which requires tables to be emitted after all values. metadata: Option, } impl TomlProject { - pub fn to_package_id(&self, source_id: SourceId) -> CargoResult { - PackageId::new(self.name, self.version.clone(), source_id) + pub fn to_package_id( + &self, + source_id: SourceId, + version: semver::Version, + ) -> CargoResult { + PackageId::new(self.name, version, source_id) } } @@ -985,20 +1154,23 @@ impl TomlManifest { .clone(); package.workspace = None; package.resolver = ws.resolve_behavior().to_manifest(); - if let Some(license_file) = &package.license_file { - let license_path = Path::new(&license_file); + if let Some(license_file) = package.license_file.clone() { + let file = license_file.inner("license_file").context(anyhow!( + "license-file should have been resolved before prepare_for_publish" + ))?; + let license_path = Path::new(&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( + package.license_file = Some(MaybeWorkspace::Defined( license_path .file_name() .unwrap() .to_str() .unwrap() .to_string(), - ); + )); } } let all = |_d: &TomlDependency| true; @@ -1108,6 +1280,8 @@ impl TomlManifest { version: Some(s.clone()), ..Default::default() })), + // Unreachable as we resolve everything before this + TomlDependency::Workspace(_) => unreachable!(), } } } @@ -1118,6 +1292,14 @@ impl TomlManifest { package_root: &Path, config: &Config, ) -> CargoResult<(Manifest, Vec)> { + // This is for later when we try to find the workspace root + fn get_ws(inheritable: Option<&InheritableFields>) -> Option<&InheritableFields> { + match inheritable { + Some(inheritable) => Some(inheritable), + None => None, + } + } + let mut nested_paths = vec![]; let mut warnings = vec![]; let mut errors = vec![]; @@ -1127,8 +1309,48 @@ impl TomlManifest { let cargo_features = me.cargo_features.as_ref().unwrap_or(&empty); let features = Features::new(cargo_features, config, &mut warnings, source_id.is_path())?; - let project = me.project.as_ref().or_else(|| me.package.as_ref()); - let project = project.ok_or_else(|| anyhow!("no `package` section found"))?; + let project = me.project.clone().or_else(|| me.package.clone()); + let project = &mut project.ok_or_else(|| anyhow!("no `package` section found"))?; + + let workspace_config = match (me.workspace.as_ref(), project.workspace.as_ref()) { + (Some(config), None) => { + let inheritable = InheritableFields::new( + config.dependencies.clone(), + config.version.clone(), + config.authors.clone(), + config.description.clone(), + config.homepage.clone(), + config.documentation.clone(), + config.readme.clone(), + config.keywords.clone(), + config.categories.clone(), + config.license.clone(), + config.license_file.clone(), + config.repository.clone(), + config.publish.clone(), + config.edition.clone(), + config.badges.clone(), + ); + + WorkspaceConfig::Root(WorkspaceRootConfig::new( + package_root, + &config.members, + &config.default_members, + &config.exclude, + &Some(inheritable.clone()), + &config.metadata, + )) + } + (None, root) => WorkspaceConfig::Member { + root: root.cloned(), + }, + (Some(..), Some(..)) => bail!( + "cannot configure both `package.workspace` and \ + `[workspace]`, only one can be specified" + ), + }; + + let inheritable = workspace_config.inheritable(); let package_name = project.name.trim(); if package_name.is_empty() { @@ -1137,12 +1359,29 @@ impl TomlManifest { validate_package_name(package_name, "package name", "")?; - let pkgid = project.to_package_id(source_id)?; + let version = project.version.clone().resolve( + &features, + "version", + || get_ws(inheritable), + |i| i.version(), + )?; - let edition = if let Some(ref edition) = project.edition { - edition + project.version = MaybeWorkspace::Defined(version.clone()); + + let pkgid = project.to_package_id(source_id, version)?; + + let edition = if let Some(edition) = project.edition.clone() { + let edition: Edition = edition + .resolve( + &features, + "edition", + || get_ws(inheritable), + |i| i.edition(), + )? .parse() - .with_context(|| "failed to parse the `edition` key")? + .with_context(|| "failed to parse the `edition` key")?; + project.edition = Some(MaybeWorkspace::Defined(edition.to_string())); + edition } else { Edition::Edition2015 }; @@ -1238,87 +1477,147 @@ impl TomlManifest { } let mut deps = Vec::new(); - let replace; - let patch; - { - let mut cx = Context { - deps: &mut deps, - source_id, - nested_paths: &mut nested_paths, - config, - warnings: &mut warnings, - features: &features, - platform: None, - root: package_root, - }; - - fn process_dependencies( - cx: &mut Context<'_, '_>, - new_deps: Option<&BTreeMap>, - kind: Option, - ) -> CargoResult<()> { - let dependencies = match new_deps { - Some(dependencies) => dependencies, - None => return Ok(()), - }; - for (n, v) in dependencies.iter() { - let dep = v.to_dependency(n, cx, kind)?; - validate_package_name(dep.name_in_toml().as_str(), "dependency name", "")?; - cx.deps.push(dep); - } + let mut cx = Context { + deps: &mut deps, + source_id, + nested_paths: &mut nested_paths, + config, + warnings: &mut warnings, + features: &features, + platform: None, + root: package_root, + }; - Ok(()) + fn process_dependencies( + features: &Features, + cx: &mut Context<'_, '_>, + new_deps: Option<&BTreeMap>, + kind: Option, + inheritable: Option<&InheritableFields>, + ) -> CargoResult>> { + let dependencies = match new_deps { + Some(dependencies) => dependencies, + None => return Ok(None), + }; + let mut deps: BTreeMap = BTreeMap::new(); + for (n, v) in dependencies.iter() { + let resolved = v.clone().resolve( + features, + n, + || get_ws(inheritable), + |i| i.dependencies().map(|deps| deps.get(n).unwrap().clone()), + )?; + let dep = resolved.to_dependency(n, cx, kind)?; + validate_package_name(dep.name_in_toml().as_str(), "dependency name", "")?; + cx.deps.push(dep); + deps.insert(n.to_string(), resolved.clone()); } + Ok(Some(deps)) + } - // Collect the dependencies. - process_dependencies(&mut cx, me.dependencies.as_ref(), None)?; - if me.dev_dependencies.is_some() && me.dev_dependencies2.is_some() { - warn_on_deprecated("dev-dependencies", package_name, "package", cx.warnings); - } - let dev_deps = me - .dev_dependencies - .as_ref() - .or_else(|| me.dev_dependencies2.as_ref()); - process_dependencies(&mut cx, dev_deps, Some(DepKind::Development))?; - if me.build_dependencies.is_some() && me.build_dependencies2.is_some() { - warn_on_deprecated("build-dependencies", package_name, "package", cx.warnings); + // Collect the dependencies. + let dependencies = process_dependencies( + &features, + &mut cx, + me.dependencies.as_ref(), + None, + inheritable, + )?; + if me.dev_dependencies.is_some() && me.dev_dependencies2.is_some() { + warn_on_deprecated("dev-dependencies", package_name, "package", cx.warnings); + } + let dev_deps = me + .dev_dependencies + .as_ref() + .or_else(|| me.dev_dependencies2.as_ref()); + let dev_deps = process_dependencies( + &features, + &mut cx, + dev_deps, + Some(DepKind::Development), + inheritable, + )?; + if me.build_dependencies.is_some() && me.build_dependencies2.is_some() { + warn_on_deprecated("build-dependencies", package_name, "package", cx.warnings); + } + let build_deps = me + .build_dependencies + .as_ref() + .or_else(|| me.build_dependencies2.as_ref()); + let build_deps = process_dependencies( + &features, + &mut cx, + build_deps, + Some(DepKind::Build), + inheritable, + )?; + + let mut target: BTreeMap = BTreeMap::new(); + for (name, platform) in me.target.iter().flatten() { + cx.platform = { + let platform: Platform = name.parse()?; + platform.check_cfg_attributes(cx.warnings); + Some(platform) + }; + let deps = process_dependencies( + &features, + &mut cx, + platform.dependencies.as_ref(), + None, + inheritable, + ) + .unwrap(); + if platform.build_dependencies.is_some() && platform.build_dependencies2.is_some() { + warn_on_deprecated("build-dependencies", name, "platform target", cx.warnings); } - let build_deps = me + let build_deps = platform .build_dependencies .as_ref() - .or_else(|| me.build_dependencies2.as_ref()); - process_dependencies(&mut cx, build_deps, Some(DepKind::Build))?; - - for (name, platform) in me.target.iter().flatten() { - cx.platform = { - let platform: Platform = name.parse()?; - platform.check_cfg_attributes(cx.warnings); - Some(platform) - }; - process_dependencies(&mut cx, platform.dependencies.as_ref(), None)?; - if platform.build_dependencies.is_some() && platform.build_dependencies2.is_some() { - warn_on_deprecated("build-dependencies", name, "platform target", cx.warnings); - } - let build_deps = platform - .build_dependencies - .as_ref() - .or_else(|| platform.build_dependencies2.as_ref()); - process_dependencies(&mut cx, build_deps, Some(DepKind::Build))?; - if platform.dev_dependencies.is_some() && platform.dev_dependencies2.is_some() { - warn_on_deprecated("dev-dependencies", name, "platform target", cx.warnings); - } - let dev_deps = platform - .dev_dependencies - .as_ref() - .or_else(|| platform.dev_dependencies2.as_ref()); - process_dependencies(&mut cx, dev_deps, Some(DepKind::Development))?; + .or_else(|| platform.build_dependencies2.as_ref()); + let build_deps = process_dependencies( + &features, + &mut cx, + build_deps, + Some(DepKind::Build), + inheritable, + ) + .unwrap(); + if platform.dev_dependencies.is_some() && platform.dev_dependencies2.is_some() { + warn_on_deprecated("dev-dependencies", name, "platform target", cx.warnings); } - - replace = me.replace(&mut cx)?; - patch = me.patch(&mut cx)?; + let dev_deps = platform + .dev_dependencies + .as_ref() + .or_else(|| platform.dev_dependencies2.as_ref()); + let dev_deps = process_dependencies( + &features, + &mut cx, + dev_deps, + Some(DepKind::Development), + inheritable, + ) + .unwrap(); + target.insert( + name.clone(), + 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 = me.replace(&mut cx)?; + let patch = me.patch(&mut cx)?; + { let mut names_sources = BTreeMap::new(); for dep in &deps { @@ -1347,42 +1646,158 @@ impl TomlManifest { project.links.as_deref(), )?; + fn resolve_to_option<'a, T>( + features: &Features, + value: Option>, + label: &str, + inheritable_fields: Option<&'a InheritableFields>, + resolve: impl FnOnce(&InheritableFields) -> Option, + ) -> CargoResult> { + match value { + None => Ok(None), + Some(mw) => { + match mw.resolve(&features, label, || get_ws(inheritable_fields), resolve) { + Ok(t) => Ok(Some(t)), + Err(e) => Err(e), + } + } + } + } + let metadata = ManifestMetadata { - description: project.description.clone(), - homepage: project.homepage.clone(), - documentation: project.documentation.clone(), + description: resolve_to_option( + &features, + project.description.clone(), + "description", + inheritable, + |i| i.description(), + )?, + homepage: resolve_to_option( + &features, + project.homepage.clone(), + "homepage", + inheritable, + |i| i.homepage(), + )?, + documentation: resolve_to_option( + &features, + project.documentation.clone(), + "documentation", + inheritable, + |i| i.documentation(), + )?, readme: readme_for_project(package_root, project), - authors: project.authors.clone().unwrap_or_default(), - license: project.license.clone(), - license_file: project.license_file.clone(), - repository: project.repository.clone(), - keywords: project.keywords.clone().unwrap_or_default(), - categories: project.categories.clone().unwrap_or_default(), - badges: me.badges.clone().unwrap_or_default(), + authors: resolve_to_option( + &features, + project.authors.clone(), + "authors", + inheritable, + |i| i.authors(), + )? + .unwrap_or_default(), + license: resolve_to_option( + &features, + project.license.clone(), + "license", + inheritable, + |i| i.license(), + )?, + license_file: resolve_to_option( + &features, + project.license_file.clone(), + "license_file", + inheritable, + |i| i.license_file(), + )?, + repository: resolve_to_option( + &features, + project.repository.clone(), + "repository", + inheritable, + |i| i.repository(), + )?, + keywords: resolve_to_option( + &features, + project.keywords.clone(), + "keywords", + inheritable, + |i| i.keywords(), + )? + .unwrap_or_default(), + categories: resolve_to_option( + &features, + project.categories.clone(), + "categories", + inheritable, + |i| i.categories(), + )? + .unwrap_or_default(), + badges: resolve_to_option(&features, me.badges.clone(), "badges", inheritable, |i| { + i.badges() + })? + .unwrap_or_default(), links: project.links.clone(), }; + project.description = metadata + .description + .clone() + .map(|description| MaybeWorkspace::Defined(description)); + project.homepage = metadata + .homepage + .clone() + .map(|homepage| MaybeWorkspace::Defined(homepage)); + project.documentation = metadata + .documentation + .clone() + .map(|documentation| MaybeWorkspace::Defined(documentation)); + project.readme = metadata + .readme + .clone() + .map(|readme| MaybeWorkspace::Defined(StringOrBool::String(readme))); + project.authors = project + .authors + .as_ref() + .map(|_| MaybeWorkspace::Defined(metadata.authors.clone())); + project.license = metadata + .license + .clone() + .map(|license| MaybeWorkspace::Defined(license)); + project.license_file = metadata + .license_file + .clone() + .map(|license_file| MaybeWorkspace::Defined(license_file)); + project.repository = metadata + .repository + .clone() + .map(|repository| MaybeWorkspace::Defined(repository)); + project.keywords = project + .keywords + .as_ref() + .map(|_| MaybeWorkspace::Defined(metadata.keywords.clone())); + project.categories = project + .categories + .as_ref() + .map(|_| MaybeWorkspace::Defined(metadata.categories.clone())); - let workspace_config = match (me.workspace.as_ref(), project.workspace.as_ref()) { - (Some(config), None) => WorkspaceConfig::Root(WorkspaceRootConfig::new( - package_root, - &config.members, - &config.default_members, - &config.exclude, - &config.metadata, - )), - (None, root) => WorkspaceConfig::Member { - root: root.cloned(), - }, - (Some(..), Some(..)) => bail!( - "cannot configure both `package.workspace` and \ - `[workspace]`, only one can be specified" - ), - }; let profiles = me.profile.clone(); if let Some(profiles) = &profiles { profiles.validate(&features, &mut warnings)?; } - let publish = match project.publish { + + let publish = project.publish.clone().map(|publish| { + publish + .resolve( + &features, + "publish", + || get_ws(inheritable), + |i| i.publish(), + ) + .unwrap() + }); + + project.publish = publish.clone().map(|p| MaybeWorkspace::Defined(p)); + + let publish = match publish { Some(VecStringOrBool::VecString(ref vecstring)) => Some(vecstring.clone()), Some(VecStringOrBool::Bool(false)) => Some(vec![]), None | Some(VecStringOrBool::Bool(true)) => None, @@ -1420,8 +1835,32 @@ impl TomlManifest { .map(|t| CompileTarget::new(&*t)) .transpose()? .map(CompileKind::Target); - let custom_metadata = project.metadata.clone(); + let resolved_toml = TomlManifest { + cargo_features: me.cargo_features.clone(), + package: Some(project.clone()), + 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, + dev_dependencies: dev_deps, + dev_dependencies2: None, + build_dependencies: build_deps, + build_dependencies2: None, + features: me.features.clone(), + target, + replace: me.replace.clone(), + patch: me.patch.clone(), + workspace: me.workspace.clone(), + badges: me + .badges + .as_ref() + .map(|_| MaybeWorkspace::Defined(metadata.badges.clone())), + }; let mut manifest = Manifest::new( summary, default_kind, @@ -1442,7 +1881,7 @@ impl TomlManifest { rust_version, project.im_a_teapot, project.default_run.clone(), - Rc::clone(me), + Rc::new(resolved_toml), project.metabuild.clone().map(|sov| sov.0), resolve_behavior, ); @@ -1546,13 +1985,33 @@ impl TomlManifest { .map(|r| ResolveBehavior::from_manifest(r)) .transpose()?; let workspace_config = match me.workspace { - Some(ref config) => WorkspaceConfig::Root(WorkspaceRootConfig::new( - root, - &config.members, - &config.default_members, - &config.exclude, - &config.metadata, - )), + Some(ref config) => { + let inheritable = InheritableFields::new( + config.dependencies.clone(), + config.version.clone(), + config.authors.clone(), + config.description.clone(), + config.homepage.clone(), + config.documentation.clone(), + config.readme.clone(), + config.keywords.clone(), + config.categories.clone(), + config.license.clone(), + config.license_file.clone(), + config.repository.clone(), + config.publish.clone(), + config.edition.clone(), + config.badges.clone(), + ); + WorkspaceConfig::Root(WorkspaceRootConfig::new( + root, + &config.members, + &config.default_members, + &config.exclude, + &Some(inheritable), + &config.metadata, + )) + } None => { bail!("virtual manifests must be configured with [workspace]"); } @@ -1669,12 +2128,12 @@ impl TomlManifest { /// Returns the name of the README file for a `TomlProject`. fn readme_for_project(package_root: &Path, project: &TomlProject) -> Option { match &project.readme { - None => default_readme_from_package_root(package_root), - Some(value) => match value { + Some(MaybeWorkspace::Defined(value)) => match value { StringOrBool::Bool(false) => None, StringOrBool::Bool(true) => Some("README.md".to_string()), StringOrBool::String(v) => Some(v.clone()), }, + _ => default_readme_from_package_root(package_root), } } @@ -1707,7 +2166,7 @@ fn unique_build_targets(targets: &[Target], package_root: &Path) -> Result<(), S Ok(()) } -impl TomlDependency

{ +impl TomlDependency

{ pub(crate) fn to_dependency_split( &self, name: &str, @@ -1749,6 +2208,7 @@ impl TomlDependency

{ } .to_dependency(name, cx, kind), TomlDependency::Detailed(ref details) => details.to_dependency(name, cx, kind), + TomlDependency::Workspace(_) => unreachable!(), } } @@ -1756,11 +2216,66 @@ impl TomlDependency

{ match self { TomlDependency::Detailed(d) => d.version.is_some(), TomlDependency::Simple(..) => true, + TomlDependency::Workspace(_) => unreachable!(), + } + } + + fn is_optional(&self) -> bool { + match self { + TomlDependency::Detailed(d) => d.optional.unwrap_or(false), + TomlDependency::Simple(..) => false, + TomlDependency::Workspace(_) => unreachable!(), + } + } + + fn resolve<'a>( + self, + cargo_features: &Features, + label: &str, + inherit: impl FnOnce() -> Option<&'a InheritableFields>, + get_ws: impl FnOnce(&InheritableFields) -> Option>, + ) -> CargoResult> { + match self { + TomlDependency::Detailed(d) => Ok(TomlDependency::Detailed(d)), + TomlDependency::Simple(s) => Ok(TomlDependency::Simple(s)), + TomlDependency::Workspace(TomlWorkspaceDependency { + workspace: true, + features, + optional, + }) => { + cargo_features.require(Feature::workspace_inheritance())?; + + let inherit = inherit().context(format!( + "You cannot inherit fields from a parent workspace currently, tried to on `[dependency.{}]`", + label + ))?; + + get_ws(inherit).map_or( + Err(anyhow!("error reading `{label}` from workspace root manifest's `[workspace.{label}]`")), + |dep| { + match dep { + TomlDependency::Simple(s) => Ok(TomlDependency::Simple(s)), + TomlDependency::Detailed(d) => { + let mut dep = d.clone(); + dep.add_features(features); + dep.update_optional(optional); + Ok(TomlDependency::Detailed(dep)) + } + TomlDependency::Workspace(_) => bail!("tried to inherit from `dependency.{label}`, but it also tried to inherit from a workspace") + } + } + ) + } + TomlDependency::Workspace(TomlWorkspaceDependency { + workspace: false, .. + }) => Err(anyhow!( + "workspace cannot be false for key `dependency.{label}`", + )), } } } -impl DetailedTomlDependency

{ +impl DetailedTomlDependency

{ fn to_dependency( &self, name_in_toml: &str, @@ -2003,6 +2518,24 @@ impl DetailedTomlDependency

{ } Ok(dep) } + + fn add_features(&mut self, features: Option>) { + self.features = match (self.features.clone(), 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, + }; + } + + fn update_optional(&mut self, optional: Option) { + self.optional = optional; + } } #[derive(Default, Serialize, Deserialize, Debug, Clone)] @@ -2056,7 +2589,7 @@ impl ser::Serialize for PathValue { } /// Corresponds to a `target` entry, but `TomlTarget` is already used. -#[derive(Serialize, Deserialize, Debug)] +#[derive(Serialize, Deserialize, Debug, Clone)] struct TomlPlatform { dependencies: Option>, #[serde(rename = "build-dependencies")] diff --git a/src/doc/src/reference/unstable.md b/src/doc/src/reference/unstable.md index 1c1db0ba713..0a57d0b4e99 100644 --- a/src/doc/src/reference/unstable.md +++ b/src/doc/src/reference/unstable.md @@ -68,6 +68,7 @@ Each new feature described below should explain how to use it. * [avoid-dev-deps](#avoid-dev-deps) — Prevents the resolver from including dev-dependencies during resolution. * [minimal-versions](#minimal-versions) — Forces the resolver to use the lowest compatible version instead of the highest. * [public-dependency](#public-dependency) — Allows dependencies to be classified as either public or private. + * [workspace-inheritance](#workspace-inheritance) - Allow workspace members to share fields and dependencies * Output behavior * [out-dir](#out-dir) — Adds a directory where artifacts are copied to. * [terminal-width](#terminal-width) — Tells rustc the width of the terminal so that long diagnostic messages can be truncated to be more readable. @@ -1342,3 +1343,65 @@ See the [Features chapter](features.md#dependency-features) for more information The `-Ztimings` option has been stabilized as `--timings` in the 1.60 release. (`--timings=html` and the machine-readable `--timings=json` output remain unstable and require `-Zunstable-options`.) + +### workspace-inheritance + +* RFC: [#2906](https://github.com/rust-lang/rfcs/blob/master/text/2906-cargo-workspace-deduplicate.md) +* Tracking Issue: [#8415](https://github.com/rust-lang/cargo/issues/8415) + +The `workspace-inheritance` feature allows workspace members to inherit fields +and dependencies from a workspace. + +Example 1: + +```toml +# in workspace's Cargo.toml +[workspace.dependencies] +log = "0.3.1" +log2 = { version = "2.0.0", package = "log" } +serde = { git = 'https://github.com/serde-rs/serde' } +wasm-bindgen-cli = { path = "crates/cli" } +``` + +```toml +# in a workspace member's Cargo.toml +[dependencies] +log = { workspace = true } +log2 = { workspace = true } +``` + +Example 2: +```toml +# in workspace's Cargo.toml +[workspace] +version = "1.2.3" +authors = ["Nice Folks"] +description = "..." +documentation = "https://example.github.io/example" +readme = "README.md" +homepage = "https://example.com" +repository = "https://github.com/example/example" +license = "MIT" +license-file = "./LICENSE" +keywords = ["cli"] +categories = ["development-tools"] +publish = false +edition = "2018" +``` + +```toml +# in a workspace member's Cargo.toml +[package] +version = { workspace = true } +authors = { workspace = true } +description = { workspace = true } +documentation = { workspace = true } +readme = { workspace = true } +homepage = { workspace = true } +repository = { workspace = true } +license = { workspace = true } +license-file = { workspace = true } +keywords = { workspace = true } +categories = { workspace = true } +publish = { workspace = true } +``` \ No newline at end of file diff --git a/tests/testsuite/deduplicate_workspace.rs b/tests/testsuite/deduplicate_workspace.rs new file mode 100644 index 00000000000..03193d38a26 --- /dev/null +++ b/tests/testsuite/deduplicate_workspace.rs @@ -0,0 +1,817 @@ +//! Tests for deduplicating Cargo.toml fields with { workspace = true } +use cargo_test_support::registry::{Dependency, Package}; +use cargo_test_support::{basic_lib_manifest, git, paths, project, publish, registry}; + +#[cargo_test] +fn permit_additional_workspace_fields() { + let p = project() + .file( + "Cargo.toml", + r#" + [workspace] + members = ["bar"] + version = "1.2.3" + authors = ["Rustaceans"] + description = "This is a crate" + documentation = "https://www.rust-lang.org/learn" + readme = "README.md" + homepage = "https://www.rust-lang.org" + repository = "https://github.com/example/example" + license = "MIT" + license-file = "LICENSE" + keywords = ["cli"] + categories = ["development-tools"] + publish = false + edition = "2018" + + [workspace.badges] + gitlab = { repository = "https://gitlab.com/rust-lang/rust", branch = "master" } + + [workspace.dependencies] + dep = "0.1" + "#, + ) + .file( + "bar/Cargo.toml", + r#" + [package] + name = "bar" + version = "0.1.0" + authors = [] + workspace = ".." + "#, + ) + .file("bar/src/main.rs", "fn main() {}") + .build(); + + p.cargo("build") + // Should not warn about unused fields. + .with_stderr( + "\ +[COMPILING] bar v0.1.0 ([CWD]/bar) +[FINISHED] dev [unoptimized + debuginfo] target(s) in [..] +", + ) + .run(); + + p.cargo("check").run(); + let lockfile = p.read_lockfile(); + assert!(!lockfile.contains("dep")); +} + +#[cargo_test] +fn deny_optional_dependencies() { + let p = project() + .file( + "Cargo.toml", + r#" + cargo-features = ["workspace-inheritance"] + + [workspace] + members = ["bar"] + + [workspace.dependencies] + dep1 = { version = "0.1", optional = true } + "#, + ) + .file("src/main.rs", "fn main() {}") + .file( + "bar/Cargo.toml", + r#" + [package] + name = "bar" + version = "0.1.0" + authors = [] + workspace = ".." + "#, + ) + .file("bar/src/main.rs", "fn main() {}") + .build(); + + p.cargo("build") + .with_status(101) + .with_stderr( + "\ +[ERROR] failed to parse manifest at `[..]foo/Cargo.toml` + +Caused by: + dep1 is optional, but workspace dependencies cannot be optional +", + ) + .masquerade_as_nightly_cargo() + .run(); +} + +#[cargo_test] +fn inherit_own_workspace_fields() { + registry::init(); + + let p = project().build(); + + let _ = git::repo(&paths::root().join("foo")) + .file( + "Cargo.toml", + r#" + cargo-features = ["workspace-inheritance"] + badges = { workspace = true } + + [package] + name = "foo" + version = { workspace = true } + authors = { workspace = true } + description = { workspace = true } + documentation = { workspace = true } + readme = { workspace = true } + homepage = { workspace = true } + repository = { workspace = true } + license = { workspace = true } + license-file = { workspace = true } + keywords = { workspace = true } + categories = { workspace = true } + publish = { workspace = true } + edition = { workspace = true } + + [workspace] + members = [] + version = "1.2.3" + authors = ["Rustaceans"] + description = "This is a crate" + documentation = "https://www.rust-lang.org/learn" + readme = "README.md" + homepage = "https://www.rust-lang.org" + repository = "https://github.com/example/example" + license = "MIT" + license-file = "LICENSE" + keywords = ["cli"] + categories = ["development-tools"] + publish = true + edition = "2018" + [workspace.badges] + gitlab = { repository = "https://gitlab.com/rust-lang/rust", branch = "master" } + "#, + ) + .file("src/main.rs", "fn main() {}") + .file("LICENSE", "license") + .file("README.md", "README.md") + .build(); + + p.cargo("publish --token sekrit") + .masquerade_as_nightly_cargo() + .run(); + publish::validate_upload_with_contents( + r#" + { + "authors": ["Rustaceans"], + "badges": { + "gitlab": { "branch": "master", "repository": "https://gitlab.com/rust-lang/rust" } + }, + "categories": ["development-tools"], + "deps": [], + "description": "This is a crate", + "documentation": "https://www.rust-lang.org/learn", + "features": {}, + "homepage": "https://www.rust-lang.org", + "keywords": ["cli"], + "license": "MIT", + "license_file": "LICENSE", + "links": null, + "name": "foo", + "readme": "README.md", + "readme_file": "README.md", + "repository": "https://github.com/example/example", + "vers": "1.2.3" + } + "#, + "foo-1.2.3.crate", + &[ + "Cargo.lock", + "Cargo.toml", + "Cargo.toml.orig", + "src/main.rs", + "README.md", + "LICENSE", + ".cargo_vcs_info.json", + ], + &[( + "Cargo.toml", + &format!( + r#"{} +cargo-features = ["workspace-inheritance"] + +[package] +edition = "2018" +name = "foo" +version = "1.2.3" +authors = ["Rustaceans"] +publish = true +description = "This is a crate" +homepage = "https://www.rust-lang.org" +documentation = "https://www.rust-lang.org/learn" +readme = "README.md" +keywords = ["cli"] +categories = ["development-tools"] +license = "MIT" +license-file = "LICENSE" +repository = "https://github.com/example/example" + +[badges.gitlab] +branch = "master" +repository = "https://gitlab.com/rust-lang/rust" +"#, + cargo::core::package::MANIFEST_PREAMBLE + ), + )], + ); +} + +#[cargo_test] +fn inherit_own_dependencies() { + let p = project() + .file( + "Cargo.toml", + r#" + cargo-features = ["workspace-inheritance"] + + [project] + name = "bar" + version = "0.2.0" + authors = [] + + [dependencies] + dep = { workspace = true } + + [build-dependencies] + dep-build = { workspace = true } + + [dev-dependencies] + dep-dev = { workspace = true } + + [workspace] + members = [] + + [workspace.dependencies] + dep = "0.1" + dep-build = "0.8" + dep-dev = "0.5.2" + "#, + ) + .file("src/main.rs", "fn main() {}") + .build(); + + Package::new("dep", "0.1.2").publish(); + Package::new("dep-build", "0.8.2").publish(); + Package::new("dep-dev", "0.5.2").publish(); + + p.cargo("build") + .masquerade_as_nightly_cargo() + .with_stderr( + "\ +[UPDATING] `[..]` index +[DOWNLOADING] crates ... +[DOWNLOADED] dep-build v0.8.2 ([..]) +[DOWNLOADED] dep v0.1.2 ([..]) +[COMPILING] dep v0.1.2 +[COMPILING] bar v0.2.0 ([CWD]) +[FINISHED] dev [unoptimized + debuginfo] target(s) in [..] +", + ) + .run(); + + p.cargo("check").masquerade_as_nightly_cargo().run(); + let lockfile = p.read_lockfile(); + assert!(lockfile.contains("dep")); + assert!(lockfile.contains("dep-dev")); + assert!(lockfile.contains("dep-build")); + p.cargo("publish --token sekrit") + .masquerade_as_nightly_cargo() + .run(); + publish::validate_upload_with_contents( + r#" + { + "authors": [], + "badges": {}, + "categories": [], + "deps": [ + { + "default_features": true, + "features": [], + "kind": "normal", + "name": "dep", + "optional": false, + "registry": "https://github.com/rust-lang/crates.io-index", + "target": null, + "version_req": "^0.1" + }, + { + "default_features": true, + "features": [], + "kind": "dev", + "name": "dep-dev", + "optional": false, + "registry": "https://github.com/rust-lang/crates.io-index", + "target": null, + "version_req": "^0.5.2" + }, + { + "default_features": true, + "features": [], + "kind": "build", + "name": "dep-build", + "optional": false, + "registry": "https://github.com/rust-lang/crates.io-index", + "target": null, + "version_req": "^0.8" + } + ], + "description": null, + "documentation": null, + "features": {}, + "homepage": null, + "keywords": [], + "license": null, + "license_file": null, + "links": null, + "name": "bar", + "readme": null, + "readme_file": null, + "repository": null, + "vers": "0.2.0" + } + "#, + "bar-0.2.0.crate", + &["Cargo.toml", "Cargo.toml.orig", "Cargo.lock", "src/main.rs"], + &[( + "Cargo.toml", + &format!( + r#"{} +cargo-features = ["workspace-inheritance"] + +[package] +name = "bar" +version = "0.2.0" +authors = [] + +[dependencies.dep] +version = "0.1" + +[dev-dependencies.dep-dev] +version = "0.5.2" + +[build-dependencies.dep-build] +version = "0.8" +"#, + cargo::core::package::MANIFEST_PREAMBLE + ), + )], + ); +} + +#[cargo_test] +fn inherit_own_detailed_dependencies() { + let p = project() + .file( + "Cargo.toml", + r#" + cargo-features = ["workspace-inheritance"] + + [project] + name = "bar" + version = "0.2.0" + authors = [] + + [dependencies] + dep = { workspace = true } + + [workspace] + members = [] + + [workspace.dependencies] + dep = { version = "0.1.2", features = ["testing"] } + "#, + ) + .file("src/main.rs", "fn main() {}") + .build(); + + Package::new("dep", "0.1.2") + .feature("testing", &vec![]) + .publish(); + + p.cargo("build") + .masquerade_as_nightly_cargo() + .with_stderr( + "\ +[UPDATING] `[..]` index +[DOWNLOADING] crates ... +[DOWNLOADED] dep v0.1.2 ([..]) +[COMPILING] dep v0.1.2 +[COMPILING] bar v0.2.0 ([CWD]) +[FINISHED] dev [unoptimized + debuginfo] target(s) in [..] +", + ) + .run(); + + p.cargo("check").masquerade_as_nightly_cargo().run(); + let lockfile = p.read_lockfile(); + assert!(lockfile.contains("dep")); + p.cargo("publish --token sekrit") + .masquerade_as_nightly_cargo() + .run(); + publish::validate_upload_with_contents( + r#" + { + "authors": [], + "badges": {}, + "categories": [], + "deps": [ + { + "default_features": true, + "features": ["testing"], + "kind": "normal", + "name": "dep", + "optional": false, + "registry": "https://github.com/rust-lang/crates.io-index", + "target": null, + "version_req": "^0.1.2" + } + ], + "description": null, + "documentation": null, + "features": {}, + "homepage": null, + "keywords": [], + "license": null, + "license_file": null, + "links": null, + "name": "bar", + "readme": null, + "readme_file": null, + "repository": null, + "vers": "0.2.0" + } + "#, + "bar-0.2.0.crate", + &["Cargo.toml", "Cargo.toml.orig", "Cargo.lock", "src/main.rs"], + &[( + "Cargo.toml", + &format!( + r#"{} +cargo-features = ["workspace-inheritance"] + +[package] +name = "bar" +version = "0.2.0" +authors = [] + +[dependencies.dep] +version = "0.1.2" +features = ["testing"] +"#, + cargo::core::package::MANIFEST_PREAMBLE + ), + )], + ); +} + +#[cargo_test] +fn inherit_from_own_undefined_field() { + registry::init(); + + let p = project().build(); + + let _ = git::repo(&paths::root().join("foo")) + .file( + "Cargo.toml", + r#" + cargo-features = ["workspace-inheritance"] + + [package] + name = "foo" + version = "1.2.5" + authors = ["rustaceans"] + description = { workspace = true } + + [workspace] + members = [] + "#, + ) + .file("src/main.rs", "fn main() {}") + .build(); + + p.cargo("build") + .masquerade_as_nightly_cargo() + .with_status(101) + .with_stderr( + "\ +[ERROR] failed to parse manifest at `[CWD]/Cargo.toml` + +Caused by: + error reading `description` from workspace root manifest's `[workspace.description]` +", + ) + .run(); +} + +#[cargo_test] +fn inherited_dependencies_union_features() { + Package::new("dep", "0.1.0") + .feature("fancy", &["fancy_dep"]) + .feature("dancy", &["dancy_dep"]) + .add_dep(Dependency::new("fancy_dep", "0.2").optional(true)) + .add_dep(Dependency::new("dancy_dep", "0.6").optional(true)) + .file("src/lib.rs", "") + .publish(); + + Package::new("fancy_dep", "0.2.4").publish(); + Package::new("dancy_dep", "0.6.8").publish(); + + let p = project() + .file( + "Cargo.toml", + r#" + cargo-features = ["workspace-inheritance"] + + [project] + name = "bar" + version = "0.2.0" + authors = [] + [dependencies] + dep = { workspace = true, features = ["dancy"] } + + [workspace] + members = [] + [workspace.dependencies] + dep = { version = "0.1", features = ["fancy"] } + "#, + ) + .file("src/main.rs", "fn main() {}") + .build(); + + p.cargo("build") + .masquerade_as_nightly_cargo() + .with_stderr( + "\ +[UPDATING] `[..]` index +[DOWNLOADING] crates ... +[DOWNLOADED] fancy_dep v0.2.4 ([..]) +[DOWNLOADED] dep v0.1.0 ([..]) +[DOWNLOADED] dancy_dep v0.6.8 ([..]) +[COMPILING] [..] +[COMPILING] [..] +[COMPILING] dep v0.1.0 +[COMPILING] bar v0.2.0 ([CWD]) +[FINISHED] dev [unoptimized + debuginfo] target(s) in [..] +", + ) + .run(); + + let lockfile = p.read_lockfile(); + assert!(lockfile.contains("dep")); + assert!(lockfile.contains("fancy_dep")); + assert!(lockfile.contains("dancy_dep")); +} + +#[cargo_test] +fn deny_inherit_fields_from_parent_workspace() { + registry::init(); + + let p = project().build(); + + let _ = git::repo(&paths::root().join("foo")) + .file( + "Cargo.toml", + r#" + [workspace] + members = ["bar"] + version = "1.2.3" + "#, + ) + .file("src/main.rs", "fn main() {}") + .file( + "bar/Cargo.toml", + r#" + cargo-features = ["workspace-inheritance"] + + [package] + name = "bar" + workspace = ".." + version = { workspace = true } + "#, + ) + .file("LICENSE", "license") + .file("README.md", "README.md") + .file("bar/src/main.rs", "fn main() {}") + .build(); + + p.cargo("build") + .masquerade_as_nightly_cargo() + .cwd("bar") + .with_status(101) + .with_stderr( + "\ +[ERROR] failed to parse manifest at `[CWD]/Cargo.toml` + +Caused by: + You cannot inherit fields from a parent workspace currently, tried to on version +", + ) + .run(); +} + +#[cargo_test] +fn deny_inherit_dependencies_from_parent_workspace() { + let git_project = git::new("detailed", |project| { + project + .file("Cargo.toml", &basic_lib_manifest("detailed")) + .file( + "src/detailed.rs", + r#" + pub fn hello() -> &'static str { + "hello world" + } + "#, + ) + }); + + // Make a new branch based on the current HEAD commit + let repo = git2::Repository::open(&git_project.root()).unwrap(); + let head = repo.head().unwrap().target().unwrap(); + let head = repo.find_commit(head).unwrap(); + repo.branch("branchy", &head, true).unwrap(); + + let p = project() + .file( + "Cargo.toml", + &format!( + r#" + [workspace] + members = ["bar"] + + [workspace.dependencies] + detailed = {{ git = '{}', branch = "branchy" }} + "#, + git_project.url() + ), + ) + .file( + "bar/Cargo.toml", + r#" + cargo-features = ["workspace-inheritance"] + + [project] + workspace = ".." + name = "bar" + version = "0.2.0" + authors = [] + + [dependencies] + detailed = { workspace = true } + "#, + ) + .file("bar/src/main.rs", "fn main() {}") + .build(); + + p.cargo("build") + .masquerade_as_nightly_cargo() + .with_status(101) + .with_stderr( + "\ +[ERROR] failed to load manifest for workspace member `[CWD]/bar` + +Caused by: + failed to parse manifest at `[CWD]/bar/Cargo.toml` + +Caused by: + You cannot inherit fields from a parent workspace currently, tried to on `[dependency.detailed]` +", + ) + .run(); +} + +#[cargo_test] +fn error_workspace_false() { + registry::init(); + + let p = project().build(); + + let _ = git::repo(&paths::root().join("foo")) + .file( + "Cargo.toml", + r#" + [workspace] + members = ["bar"] + "#, + ) + .file("src/main.rs", "fn main() {}") + .file( + "bar/Cargo.toml", + r#" + [package] + name = "bar" + workspace = ".." + version = "1.2.3" + authors = ["rustaceans"] + description = { workspace = false } + "#, + ) + .file("bar/src/main.rs", "fn main() {}") + .build(); + + p.cargo("build") + .cwd("bar") + .with_status(101) + .with_stderr( + "\ +[ERROR] failed to parse manifest at `[CWD]/Cargo.toml` + +Caused by: + workspace cannot be false for key `package.description` +", + ) + .run(); +} + +#[cargo_test] +fn workspace_inheritance_not_enabled() { + registry::init(); + + let p = project().build(); + + let _ = git::repo(&paths::root().join("foo")) + .file( + "Cargo.toml", + r#" + [package] + name = "foo" + version = "1.2.5" + authors = ["rustaceans"] + description = { workspace = true } + + [workspace] + members = [] + "#, + ) + .file("src/main.rs", "fn main() {}") + .build(); + + p.cargo("build") + .masquerade_as_nightly_cargo() + .with_status(101) + .with_stderr( + "\ +[ERROR] failed to parse manifest at `[CWD]/Cargo.toml` + +Caused by: + feature `workspace-inheritance` is required + + The package requires the Cargo feature called `workspace-inheritance`, \ + but that feature is not stabilized in this version of Cargo (1.[..]). + Consider adding `cargo-features = [\"workspace-inheritance\"]` to the top of Cargo.toml \ + (above the [package] table) to tell Cargo you are opting in to use this unstable feature. + See https://doc.rust-lang.org/nightly/cargo/reference/unstable.html#workspace-inheritance \ + for more information about the status of this feature. +", + ) + .run(); +} + +#[cargo_test] +fn nightly_required() { + registry::init(); + + let p = project().build(); + + let _ = git::repo(&paths::root().join("foo")) + .file( + "Cargo.toml", + r#" + cargo-features = ["workspace-inheritance"] + + [package] + name = "foo" + version = "1.2.5" + authors = ["rustaceans"] + description = { workspace = true } + + [workspace] + members = [] + "#, + ) + .file("src/main.rs", "fn main() {}") + .build(); + + p.cargo("build") + .with_status(101) + .with_stderr( + "\ +[ERROR] failed to parse manifest at `[CWD]/Cargo.toml` + +Caused by: + the cargo feature `workspace-inheritance` requires a nightly version of Cargo, \ + but this is the `stable` channel + See [..] + See https://doc.rust-lang.org/[..]cargo/reference/unstable.html#workspace-inheritance \ + for more information about using this feature. +", + ) + .run(); +} diff --git a/tests/testsuite/main.rs b/tests/testsuite/main.rs index b5faa479275..3a24015f311 100644 --- a/tests/testsuite/main.rs +++ b/tests/testsuite/main.rs @@ -39,6 +39,7 @@ mod cross_compile; mod cross_publish; mod custom_target; mod death; +mod deduplicate_workspace; mod dep_info; mod directory; mod doc; From c4cd152c36057920847b58b12899ef98f22404b0 Mon Sep 17 00:00:00 2001 From: scott Date: Wed, 23 Mar 2022 13:40:16 -0500 Subject: [PATCH 2/3] -- updated both `resolve` to only take a get_ws -- `resolve` not takes `get_ws: impl FnOnce() -> CargoResult` -- removed `MaybeWorkspace` from `readme` and `license-file` -- changed error messages and test names --- src/cargo/core/workspace.rs | 131 ++++++++--- src/cargo/util/toml/mod.rs | 287 +++++++++-------------- tests/testsuite/deduplicate_workspace.rs | 78 ++++-- 3 files changed, 275 insertions(+), 221 deletions(-) diff --git a/src/cargo/core/workspace.rs b/src/cargo/core/workspace.rs index 34e8150c3d1..cd384bef816 100644 --- a/src/cargo/core/workspace.rs +++ b/src/cargo/core/workspace.rs @@ -4,7 +4,7 @@ use std::collections::{BTreeMap, BTreeSet, HashSet}; use std::path::{Path, PathBuf}; use std::rc::Rc; -use anyhow::{bail, Context as _}; +use anyhow::{anyhow, bail, Context as _}; use glob::glob; use itertools::Itertools; use log::debug; @@ -1712,63 +1712,132 @@ impl InheritableFields { } } - pub fn dependencies(&self) -> Option> { - self.dependencies.clone() + pub fn dependencies(&self) -> CargoResult> { + self.dependencies.clone().map_or( + Err(anyhow!("[workspace.dependencies] was not defined")), + |d| Ok(d), + ) } - pub fn version(&self) -> Option { - self.version.clone() + pub fn get_dependency(&self, name: &str) -> CargoResult { + self.dependencies.clone().map_or( + Err(anyhow!("[workspace.dependencies] was not defined")), + |deps| { + deps.get(name).map_or( + Err(anyhow!( + "dependency {} was not found in [workspace.dependencies]", + name + )), + |dep| Ok(dep.clone()), + ) + }, + ) } - pub fn authors(&self) -> Option> { - self.authors.clone() + pub fn version(&self) -> CargoResult { + self.version + .clone() + .map_or(Err(anyhow!("[workspace.version] was not defined")), |d| { + Ok(d) + }) } - pub fn description(&self) -> Option { - self.description.clone() + pub fn authors(&self) -> CargoResult> { + self.authors + .clone() + .map_or(Err(anyhow!("[workspace.authors] was not defined")), |d| { + Ok(d) + }) } - pub fn homepage(&self) -> Option { - self.homepage.clone() + pub fn description(&self) -> CargoResult { + self.description.clone().map_or( + Err(anyhow!("[workspace.description] was not defined")), + |d| Ok(d), + ) } - pub fn documentation(&self) -> Option { - self.documentation.clone() + pub fn homepage(&self) -> CargoResult { + self.homepage + .clone() + .map_or(Err(anyhow!("[workspace.homepage] was not defined")), |d| { + Ok(d) + }) } - pub fn readme(&self) -> Option { - self.readme.clone() + pub fn documentation(&self) -> CargoResult { + self.documentation.clone().map_or( + Err(anyhow!("[workspace.documentation] was not defined")), + |d| Ok(d), + ) + } + + pub fn readme(&self) -> CargoResult { + self.readme + .clone() + .map_or(Err(anyhow!("[workspace.readme] was not defined")), |d| { + Ok(d) + }) } - pub fn keywords(&self) -> Option> { - self.keywords.clone() + pub fn keywords(&self) -> CargoResult> { + self.keywords + .clone() + .map_or(Err(anyhow!("[workspace.keywords] was not defined")), |d| { + Ok(d) + }) } - pub fn categories(&self) -> Option> { - self.categories.clone() + pub fn categories(&self) -> CargoResult> { + self.categories.clone().map_or( + Err(anyhow!("[workspace.categories] was not defined")), + |d| Ok(d), + ) } - pub fn license(&self) -> Option { - self.license.clone() + pub fn license(&self) -> CargoResult { + self.license + .clone() + .map_or(Err(anyhow!("[workspace.license] was not defined")), |d| { + Ok(d) + }) } - pub fn license_file(&self) -> Option { - self.license_file.clone() + pub fn license_file(&self) -> CargoResult { + self.license_file.clone().map_or( + Err(anyhow!("[workspace.license_file] was not defined")), + |d| Ok(d), + ) } - pub fn repository(&self) -> Option { - self.repository.clone() + pub fn repository(&self) -> CargoResult { + self.repository.clone().map_or( + Err(anyhow!("[workspace.repository] was not defined")), + |d| Ok(d), + ) } - pub fn publish(&self) -> Option { - self.publish.clone() + pub fn publish(&self) -> CargoResult { + self.publish + .clone() + .map_or(Err(anyhow!("[workspace.publish] was not defined")), |d| { + Ok(d) + }) } - pub fn edition(&self) -> Option { - self.edition.clone() + pub fn edition(&self) -> CargoResult { + self.edition + .clone() + .map_or(Err(anyhow!("[workspace.edition] was not defined")), |d| { + Ok(d) + }) } - pub fn badges(&self) -> Option>> { - self.badges.clone() + pub fn badges(&self) -> CargoResult>> { + self.badges + .clone() + .map_or(Err(anyhow!("[workspace.badges] was not defined")), |d| { + Ok(d) + }) } } diff --git a/src/cargo/util/toml/mod.rs b/src/cargo/util/toml/mod.rs index 3892080ab23..bd85a94f4c8 100644 --- a/src/cargo/util/toml/mod.rs +++ b/src/cargo/util/toml/mod.rs @@ -127,6 +127,12 @@ pub fn read_manifest_from_str( name ); } + if let TomlDependency::Workspace(_) = dep { + bail!( + "`dependencies.{}` specified `{{ workspace = true}}`, but workspace dependencies cannot do this", + name + ); + } } } return if manifest.project.is_some() || manifest.package.is_some() { @@ -997,18 +1003,13 @@ impl MaybeWorkspace { self, cargo_features: &Features, label: &str, - inherit: impl FnOnce() -> Option<&'a InheritableFields>, - get_ws: impl FnOnce(&InheritableFields) -> Option, + get_ws_field: impl FnOnce() -> CargoResult, ) -> CargoResult { match self { MaybeWorkspace::Defined(value) => Ok(value), MaybeWorkspace::Workspace(TomlWorkspaceField { workspace: true }) => { cargo_features.require(Feature::workspace_inheritance())?; - let inherit = inherit().context(format!( - "You cannot inherit fields from a parent workspace currently, tried to on {}", - label - ))?; - get_ws(inherit).context(format!( + get_ws_field().context(format!( "error reading `{}` from workspace root manifest's `[workspace.{}]`", label, label )) @@ -1018,15 +1019,6 @@ impl MaybeWorkspace { )), } } - - /// This does not try to resolve a `MaybeWorkspace` it gets a &T if it is defined. - /// If it is not defined it will return an error - fn inner(&self, label: &str) -> CargoResult<&T> { - match self { - MaybeWorkspace::Workspace(_) => Err(anyhow!("{} has not been resolved yet`", label)), - MaybeWorkspace::Defined(d) => Ok(d), - } - } } #[derive(Deserialize, Serialize, Clone, Debug)] @@ -1071,11 +1063,11 @@ pub struct TomlProject { description: Option>, homepage: Option>, documentation: Option>, - readme: Option>, + readme: Option, keywords: Option>>, categories: Option>>, license: Option>, - license_file: Option>, + license_file: Option, repository: Option>, resolver: Option, @@ -1154,23 +1146,20 @@ impl TomlManifest { .clone(); package.workspace = None; package.resolver = ws.resolve_behavior().to_manifest(); - if let Some(license_file) = package.license_file.clone() { - let file = license_file.inner("license_file").context(anyhow!( - "license-file should have been resolved before prepare_for_publish" - ))?; - let license_path = Path::new(&file); + if let Some(license_file) = &package.license_file { + 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(MaybeWorkspace::Defined( + package.license_file = Some( license_path .file_name() .unwrap() .to_str() .unwrap() .to_string(), - )); + ); } } let all = |_d: &TomlDependency| true; @@ -1293,10 +1282,12 @@ impl TomlManifest { config: &Config, ) -> CargoResult<(Manifest, Vec)> { // This is for later when we try to find the workspace root - fn get_ws(inheritable: Option<&InheritableFields>) -> Option<&InheritableFields> { + fn get_ws(inheritable: Option<&InheritableFields>) -> CargoResult<&InheritableFields> { match inheritable { - Some(inheritable) => Some(inheritable), - None => None, + Some(inheritable) => Ok(inheritable), + None => Err(anyhow!( + "inheriting from a parent workspace is not implemented yet", + )), } } @@ -1359,12 +1350,10 @@ impl TomlManifest { validate_package_name(package_name, "package name", "")?; - let version = project.version.clone().resolve( - &features, - "version", - || get_ws(inheritable), - |i| i.version(), - )?; + let version = project + .version + .clone() + .resolve(&features, "version", || get_ws(inheritable)?.version())?; project.version = MaybeWorkspace::Defined(version.clone()); @@ -1372,12 +1361,7 @@ impl TomlManifest { let edition = if let Some(edition) = project.edition.clone() { let edition: Edition = edition - .resolve( - &features, - "edition", - || get_ws(inheritable), - |i| i.edition(), - )? + .resolve(&features, "edition", || get_ws(inheritable)?.edition())? .parse() .with_context(|| "failed to parse the `edition` key")?; project.edition = Some(MaybeWorkspace::Defined(edition.to_string())); @@ -1502,12 +1486,9 @@ impl TomlManifest { }; let mut deps: BTreeMap = BTreeMap::new(); for (n, v) in dependencies.iter() { - let resolved = v.clone().resolve( - features, - n, - || get_ws(inheritable), - |i| i.dependencies().map(|deps| deps.get(n).unwrap().clone()), - )?; + let resolved = v + .clone() + .resolve(features, n, || get_ws(inheritable)?.get_dependency(n))?; let dep = resolved.to_dependency(n, cx, kind)?; validate_package_name(dep.name_in_toml().as_str(), "dependency name", "")?; cx.deps.push(dep); @@ -1646,96 +1627,74 @@ impl TomlManifest { project.links.as_deref(), )?; - fn resolve_to_option<'a, T>( - features: &Features, - value: Option>, - label: &str, - inheritable_fields: Option<&'a InheritableFields>, - resolve: impl FnOnce(&InheritableFields) -> Option, - ) -> CargoResult> { - match value { - None => Ok(None), - Some(mw) => { - match mw.resolve(&features, label, || get_ws(inheritable_fields), resolve) { - Ok(t) => Ok(Some(t)), - Err(e) => Err(e), - } - } - } - } - let metadata = ManifestMetadata { - description: resolve_to_option( - &features, - project.description.clone(), - "description", - inheritable, - |i| i.description(), - )?, - homepage: resolve_to_option( - &features, - project.homepage.clone(), - "homepage", - inheritable, - |i| i.homepage(), - )?, - documentation: resolve_to_option( - &features, - project.documentation.clone(), - "documentation", - inheritable, - |i| i.documentation(), - )?, + description: project + .description + .clone() + .map(|mw| { + mw.resolve(&features, "description", || { + get_ws(inheritable)?.description() + }) + }) + .transpose()?, + homepage: project + .homepage + .clone() + .map(|mw| mw.resolve(&features, "homepage", || get_ws(inheritable)?.homepage())) + .transpose()?, + documentation: project + .documentation + .clone() + .map(|mw| { + mw.resolve(&features, "documentation", || { + get_ws(inheritable)?.documentation() + }) + }) + .transpose()?, readme: readme_for_project(package_root, project), - authors: resolve_to_option( - &features, - project.authors.clone(), - "authors", - inheritable, - |i| i.authors(), - )? - .unwrap_or_default(), - license: resolve_to_option( - &features, - project.license.clone(), - "license", - inheritable, - |i| i.license(), - )?, - license_file: resolve_to_option( - &features, - project.license_file.clone(), - "license_file", - inheritable, - |i| i.license_file(), - )?, - repository: resolve_to_option( - &features, - project.repository.clone(), - "repository", - inheritable, - |i| i.repository(), - )?, - keywords: resolve_to_option( - &features, - project.keywords.clone(), - "keywords", - inheritable, - |i| i.keywords(), - )? - .unwrap_or_default(), - categories: resolve_to_option( - &features, - project.categories.clone(), - "categories", - inheritable, - |i| i.categories(), - )? - .unwrap_or_default(), - badges: resolve_to_option(&features, me.badges.clone(), "badges", inheritable, |i| { - i.badges() - })? - .unwrap_or_default(), + authors: project + .authors + .clone() + .map(|mw| mw.resolve(&features, "authors", || get_ws(inheritable)?.authors())) + .transpose()? + .unwrap_or_default(), + license: project + .license + .clone() + .map(|mw| mw.resolve(&features, "license", || get_ws(inheritable)?.license())) + .transpose()?, + license_file: project.license_file.clone(), + repository: project + .repository + .clone() + .map(|mw| { + mw.resolve(&features, "repository", || { + get_ws(inheritable)?.repository() + }) + }) + .transpose()?, + keywords: project + .keywords + .clone() + .map(|mw| mw.resolve(&features, "keywords", || get_ws(inheritable)?.keywords())) + .transpose()? + .unwrap_or_default(), + categories: project + .categories + .clone() + .map(|mw| { + mw.resolve(&features, "categories", || { + get_ws(inheritable)?.categories() + }) + }) + .transpose()? + .unwrap_or_default(), + badges: me + .badges + .clone() + .map(|mw| mw.resolve(&features, "badges", || get_ws(inheritable)?.badges())) + .transpose()? + .unwrap_or_default(), links: project.links.clone(), }; project.description = metadata @@ -1750,10 +1709,6 @@ impl TomlManifest { .documentation .clone() .map(|documentation| MaybeWorkspace::Defined(documentation)); - project.readme = metadata - .readme - .clone() - .map(|readme| MaybeWorkspace::Defined(StringOrBool::String(readme))); project.authors = project .authors .as_ref() @@ -1762,10 +1717,6 @@ impl TomlManifest { .license .clone() .map(|license| MaybeWorkspace::Defined(license)); - project.license_file = metadata - .license_file - .clone() - .map(|license_file| MaybeWorkspace::Defined(license_file)); project.repository = metadata .repository .clone() @@ -1786,12 +1737,7 @@ impl TomlManifest { let publish = project.publish.clone().map(|publish| { publish - .resolve( - &features, - "publish", - || get_ws(inheritable), - |i| i.publish(), - ) + .resolve(&features, "publish", || get_ws(inheritable)?.publish()) .unwrap() }); @@ -2128,12 +2074,12 @@ impl TomlManifest { /// Returns the name of the README file for a `TomlProject`. fn readme_for_project(package_root: &Path, project: &TomlProject) -> Option { match &project.readme { - Some(MaybeWorkspace::Defined(value)) => match value { + None => default_readme_from_package_root(package_root), + Some(value) => match value { StringOrBool::Bool(false) => None, StringOrBool::Bool(true) => Some("README.md".to_string()), StringOrBool::String(v) => Some(v.clone()), }, - _ => default_readme_from_package_root(package_root), } } @@ -2224,7 +2170,7 @@ impl TomlDependency

{ match self { TomlDependency::Detailed(d) => d.optional.unwrap_or(false), TomlDependency::Simple(..) => false, - TomlDependency::Workspace(_) => unreachable!(), + TomlDependency::Workspace(w) => w.optional.unwrap_or(false), } } @@ -2232,8 +2178,7 @@ impl TomlDependency

{ self, cargo_features: &Features, label: &str, - inherit: impl FnOnce() -> Option<&'a InheritableFields>, - get_ws: impl FnOnce(&InheritableFields) -> Option>, + get_ws_dependency: impl FnOnce() -> CargoResult>, ) -> CargoResult> { match self { TomlDependency::Detailed(d) => Ok(TomlDependency::Detailed(d)), @@ -2244,32 +2189,28 @@ impl TomlDependency

{ optional, }) => { cargo_features.require(Feature::workspace_inheritance())?; - - let inherit = inherit().context(format!( - "You cannot inherit fields from a parent workspace currently, tried to on `[dependency.{}]`", - label - ))?; - - get_ws(inherit).map_or( - Err(anyhow!("error reading `{label}` from workspace root manifest's `[workspace.{label}]`")), - |dep| { - match dep { - TomlDependency::Simple(s) => Ok(TomlDependency::Simple(s)), - TomlDependency::Detailed(d) => { - let mut dep = d.clone(); - dep.add_features(features); - dep.update_optional(optional); - Ok(TomlDependency::Detailed(dep)) - } - TomlDependency::Workspace(_) => bail!("tried to inherit from `dependency.{label}`, but it also tried to inherit from a workspace") + get_ws_dependency().context(format!( + "error reading `dependencies.{}` from workspace root manifest's `[workspace.dependencies.{}]`", + label, label + )).map(|dep| { + match dep { + TomlDependency::Simple(s) => TomlDependency::Simple(s), + TomlDependency::Detailed(d) => { + let mut dep = d.clone(); + dep.add_features(features); + dep.update_optional(optional); + TomlDependency::Detailed(dep) } + // We check for this when we parse a toml file + TomlDependency::Workspace(_) => unreachable!(), } - ) + }) } TomlDependency::Workspace(TomlWorkspaceDependency { workspace: false, .. }) => Err(anyhow!( - "workspace cannot be false for key `dependency.{label}`", + "workspace cannot be false for key `dependencies.{}`", + label, )), } } diff --git a/tests/testsuite/deduplicate_workspace.rs b/tests/testsuite/deduplicate_workspace.rs index 03193d38a26..06535a1a01a 100644 --- a/tests/testsuite/deduplicate_workspace.rs +++ b/tests/testsuite/deduplicate_workspace.rs @@ -121,11 +121,9 @@ fn inherit_own_workspace_fields() { authors = { workspace = true } description = { workspace = true } documentation = { workspace = true } - readme = { workspace = true } homepage = { workspace = true } repository = { workspace = true } license = { workspace = true } - license-file = { workspace = true } keywords = { workspace = true } categories = { workspace = true } publish = { workspace = true } @@ -137,11 +135,9 @@ fn inherit_own_workspace_fields() { authors = ["Rustaceans"] description = "This is a crate" documentation = "https://www.rust-lang.org/learn" - readme = "README.md" homepage = "https://www.rust-lang.org" repository = "https://github.com/example/example" license = "MIT" - license-file = "LICENSE" keywords = ["cli"] categories = ["development-tools"] publish = true @@ -151,8 +147,6 @@ fn inherit_own_workspace_fields() { "#, ) .file("src/main.rs", "fn main() {}") - .file("LICENSE", "license") - .file("README.md", "README.md") .build(); p.cargo("publish --token sekrit") @@ -173,11 +167,11 @@ fn inherit_own_workspace_fields() { "homepage": "https://www.rust-lang.org", "keywords": ["cli"], "license": "MIT", - "license_file": "LICENSE", + "license_file": null, "links": null, "name": "foo", - "readme": "README.md", - "readme_file": "README.md", + "readme": null, + "readme_file": null, "repository": "https://github.com/example/example", "vers": "1.2.3" } @@ -188,8 +182,6 @@ fn inherit_own_workspace_fields() { "Cargo.toml", "Cargo.toml.orig", "src/main.rs", - "README.md", - "LICENSE", ".cargo_vcs_info.json", ], &[( @@ -207,11 +199,9 @@ publish = true description = "This is a crate" homepage = "https://www.rust-lang.org" documentation = "https://www.rust-lang.org/learn" -readme = "README.md" keywords = ["cli"] categories = ["development-tools"] license = "MIT" -license-file = "LICENSE" repository = "https://github.com/example/example" [badges.gitlab] @@ -506,6 +496,9 @@ fn inherit_from_own_undefined_field() { Caused by: error reading `description` from workspace root manifest's `[workspace.description]` + +Caused by: + [workspace.description] was not defined ", ) .run(); @@ -571,7 +564,7 @@ fn inherited_dependencies_union_features() { } #[cargo_test] -fn deny_inherit_fields_from_parent_workspace() { +fn error_on_unimplemented_inheritance_fields() { registry::init(); let p = project().build(); @@ -611,14 +604,17 @@ fn deny_inherit_fields_from_parent_workspace() { [ERROR] failed to parse manifest at `[CWD]/Cargo.toml` Caused by: - You cannot inherit fields from a parent workspace currently, tried to on version + error reading `version` from workspace root manifest's `[workspace.version]` + +Caused by: + inheriting from a parent workspace is not implemented yet ", ) .run(); } #[cargo_test] -fn deny_inherit_dependencies_from_parent_workspace() { +fn error_on_unimplemented_inheritance_dependencies() { let git_project = git::new("detailed", |project| { project .file("Cargo.toml", &basic_lib_manifest("detailed")) @@ -681,7 +677,10 @@ Caused by: failed to parse manifest at `[CWD]/bar/Cargo.toml` Caused by: - You cannot inherit fields from a parent workspace currently, tried to on `[dependency.detailed]` + error reading `dependencies.detailed` from workspace root manifest's `[workspace.dependencies.detailed]` + +Caused by: + inheriting from a parent workspace is not implemented yet ", ) .run(); @@ -717,6 +716,7 @@ fn error_workspace_false() { .build(); p.cargo("build") + .masquerade_as_nightly_cargo() .cwd("bar") .with_status(101) .with_stderr( @@ -730,6 +730,50 @@ Caused by: .run(); } +#[cargo_test] +fn error_workspace_dependency_looked_for_workspace_itself() { + registry::init(); + + let p = project().build(); + + let _ = git::repo(&paths::root().join("foo")) + .file( + "Cargo.toml", + r#" + cargo-features = ["workspace-inheritance"] + [package] + name = "bar" + version = "1.2.3" + workspace = ".." + + [dependencies] + dep = { workspace = true } + + [workspace] + members = ["bar"] + + [workspace.dependencies] + dep = { workspace = true } + + "#, + ) + .file("src/main.rs", "fn main() {}") + .build(); + + p.cargo("build") + .masquerade_as_nightly_cargo() + .with_status(101) + .with_stderr( + "\ +[ERROR] failed to parse manifest at `[CWD]/Cargo.toml` + +Caused by: + `dependencies.dep` specified `{ workspace = true}`, but workspace dependencies cannot do this +", + ) + .run(); +} + #[cargo_test] fn workspace_inheritance_not_enabled() { registry::init(); From e6992d4310a4b7c47c47a9af4c3531f3501c7d1b Mon Sep 17 00:00:00 2001 From: scott Date: Wed, 23 Mar 2022 16:47:06 -0500 Subject: [PATCH 3/3] -- renamed deduplicate_workspace to inheritable_workspace_fields -- removed `[]` from error messages in favor of back-ticks --- src/cargo/core/workspace.rs | 34 +++++++++---------- src/cargo/util/toml/mod.rs | 24 ++++++++----- ...ace.rs => inheritable_workspace_fields.rs} | 15 ++++---- tests/testsuite/main.rs | 2 +- 4 files changed, 42 insertions(+), 33 deletions(-) rename tests/testsuite/{deduplicate_workspace.rs => inheritable_workspace_fields.rs} (97%) diff --git a/src/cargo/core/workspace.rs b/src/cargo/core/workspace.rs index cd384bef816..7091e216088 100644 --- a/src/cargo/core/workspace.rs +++ b/src/cargo/core/workspace.rs @@ -1714,18 +1714,18 @@ impl InheritableFields { pub fn dependencies(&self) -> CargoResult> { self.dependencies.clone().map_or( - Err(anyhow!("[workspace.dependencies] was not defined")), + Err(anyhow!("`workspace.dependencies` was not defined")), |d| Ok(d), ) } pub fn get_dependency(&self, name: &str) -> CargoResult { self.dependencies.clone().map_or( - Err(anyhow!("[workspace.dependencies] was not defined")), + Err(anyhow!("`workspace.dependencies` was not defined")), |deps| { deps.get(name).map_or( Err(anyhow!( - "dependency {} was not found in [workspace.dependencies]", + "`dependency.{}` was not found in `workspace.dependencies`", name )), |dep| Ok(dep.clone()), @@ -1737,7 +1737,7 @@ impl InheritableFields { pub fn version(&self) -> CargoResult { self.version .clone() - .map_or(Err(anyhow!("[workspace.version] was not defined")), |d| { + .map_or(Err(anyhow!("`workspace.version` was not defined")), |d| { Ok(d) }) } @@ -1745,14 +1745,14 @@ impl InheritableFields { pub fn authors(&self) -> CargoResult> { self.authors .clone() - .map_or(Err(anyhow!("[workspace.authors] was not defined")), |d| { + .map_or(Err(anyhow!("`workspace.authors` was not defined")), |d| { Ok(d) }) } pub fn description(&self) -> CargoResult { self.description.clone().map_or( - Err(anyhow!("[workspace.description] was not defined")), + Err(anyhow!("`workspace.description` was not defined")), |d| Ok(d), ) } @@ -1760,14 +1760,14 @@ impl InheritableFields { pub fn homepage(&self) -> CargoResult { self.homepage .clone() - .map_or(Err(anyhow!("[workspace.homepage] was not defined")), |d| { + .map_or(Err(anyhow!("`workspace.homepage` was not defined")), |d| { Ok(d) }) } pub fn documentation(&self) -> CargoResult { self.documentation.clone().map_or( - Err(anyhow!("[workspace.documentation] was not defined")), + Err(anyhow!("`workspace.documentation` was not defined")), |d| Ok(d), ) } @@ -1775,7 +1775,7 @@ impl InheritableFields { pub fn readme(&self) -> CargoResult { self.readme .clone() - .map_or(Err(anyhow!("[workspace.readme] was not defined")), |d| { + .map_or(Err(anyhow!("`workspace.readme` was not defined")), |d| { Ok(d) }) } @@ -1783,14 +1783,14 @@ impl InheritableFields { pub fn keywords(&self) -> CargoResult> { self.keywords .clone() - .map_or(Err(anyhow!("[workspace.keywords] was not defined")), |d| { + .map_or(Err(anyhow!("`workspace.keywords` was not defined")), |d| { Ok(d) }) } pub fn categories(&self) -> CargoResult> { self.categories.clone().map_or( - Err(anyhow!("[workspace.categories] was not defined")), + Err(anyhow!("`workspace.categories` was not defined")), |d| Ok(d), ) } @@ -1798,21 +1798,21 @@ impl InheritableFields { pub fn license(&self) -> CargoResult { self.license .clone() - .map_or(Err(anyhow!("[workspace.license] was not defined")), |d| { + .map_or(Err(anyhow!("`workspace.license` was not defined")), |d| { Ok(d) }) } pub fn license_file(&self) -> CargoResult { self.license_file.clone().map_or( - Err(anyhow!("[workspace.license_file] was not defined")), + Err(anyhow!("`workspace.license_file` was not defined")), |d| Ok(d), ) } pub fn repository(&self) -> CargoResult { self.repository.clone().map_or( - Err(anyhow!("[workspace.repository] was not defined")), + Err(anyhow!("`workspace.repository` was not defined")), |d| Ok(d), ) } @@ -1820,7 +1820,7 @@ impl InheritableFields { pub fn publish(&self) -> CargoResult { self.publish .clone() - .map_or(Err(anyhow!("[workspace.publish] was not defined")), |d| { + .map_or(Err(anyhow!("`workspace.publish` was not defined")), |d| { Ok(d) }) } @@ -1828,7 +1828,7 @@ impl InheritableFields { pub fn edition(&self) -> CargoResult { self.edition .clone() - .map_or(Err(anyhow!("[workspace.edition] was not defined")), |d| { + .map_or(Err(anyhow!("`workspace.edition` was not defined")), |d| { Ok(d) }) } @@ -1836,7 +1836,7 @@ impl InheritableFields { pub fn badges(&self) -> CargoResult>> { self.badges .clone() - .map_or(Err(anyhow!("[workspace.badges] was not defined")), |d| { + .map_or(Err(anyhow!("`workspace.badges` was not defined")), |d| { Ok(d) }) } diff --git a/src/cargo/util/toml/mod.rs b/src/cargo/util/toml/mod.rs index bd85a94f4c8..b49bdce5139 100644 --- a/src/cargo/util/toml/mod.rs +++ b/src/cargo/util/toml/mod.rs @@ -129,7 +129,8 @@ pub fn read_manifest_from_str( } if let TomlDependency::Workspace(_) = dep { bail!( - "`dependencies.{}` specified `{{ workspace = true}}`, but workspace dependencies cannot do this", + "`workspace.dependencies.{}` specified `{{ workspace = true }}`, but \ + workspace dependencies cannot do this", name ); } @@ -1010,12 +1011,13 @@ impl MaybeWorkspace { MaybeWorkspace::Workspace(TomlWorkspaceField { workspace: true }) => { cargo_features.require(Feature::workspace_inheritance())?; get_ws_field().context(format!( - "error reading `{}` from workspace root manifest's `[workspace.{}]`", + "error inheriting `{}` from workspace root manifest's `workspace.{}`", label, label )) } MaybeWorkspace::Workspace(TomlWorkspaceField { workspace: false }) => Err(anyhow!( - "workspace cannot be false for key `package.{label}`", + "`workspace=false` is unsupported for `package.{}`", + label, )), } } @@ -2190,7 +2192,7 @@ impl TomlDependency

{ }) => { cargo_features.require(Feature::workspace_inheritance())?; get_ws_dependency().context(format!( - "error reading `dependencies.{}` from workspace root manifest's `[workspace.dependencies.{}]`", + "error reading `dependencies.{}` from workspace root manifest's `workspace.dependencies.{}`", label, label )).map(|dep| { match dep { @@ -2200,16 +2202,22 @@ impl TomlDependency

{ dep.add_features(features); dep.update_optional(optional); TomlDependency::Detailed(dep) - } - // We check for this when we parse a toml file - TomlDependency::Workspace(_) => unreachable!(), + }, + TomlDependency::Workspace(_) => { + unreachable!( + "We check that no workspace defines dependencies with \ + `{{ workspace = true }}` when we read a manifest from a string. \ + this should not happen but did on {}", + label + ) + }, } }) } TomlDependency::Workspace(TomlWorkspaceDependency { workspace: false, .. }) => Err(anyhow!( - "workspace cannot be false for key `dependencies.{}`", + "`workspace=false` is unsupported for `package.dependencies.{}`", label, )), } diff --git a/tests/testsuite/deduplicate_workspace.rs b/tests/testsuite/inheritable_workspace_fields.rs similarity index 97% rename from tests/testsuite/deduplicate_workspace.rs rename to tests/testsuite/inheritable_workspace_fields.rs index 06535a1a01a..4fa4484ff56 100644 --- a/tests/testsuite/deduplicate_workspace.rs +++ b/tests/testsuite/inheritable_workspace_fields.rs @@ -1,4 +1,4 @@ -//! Tests for deduplicating Cargo.toml fields with { workspace = true } +//! Tests for inheriting Cargo.toml fields with { workspace = true } use cargo_test_support::registry::{Dependency, Package}; use cargo_test_support::{basic_lib_manifest, git, paths, project, publish, registry}; @@ -495,10 +495,10 @@ fn inherit_from_own_undefined_field() { [ERROR] failed to parse manifest at `[CWD]/Cargo.toml` Caused by: - error reading `description` from workspace root manifest's `[workspace.description]` + error inheriting `description` from workspace root manifest's `workspace.description` Caused by: - [workspace.description] was not defined + `workspace.description` was not defined ", ) .run(); @@ -604,7 +604,7 @@ fn error_on_unimplemented_inheritance_fields() { [ERROR] failed to parse manifest at `[CWD]/Cargo.toml` Caused by: - error reading `version` from workspace root manifest's `[workspace.version]` + error inheriting `version` from workspace root manifest's `workspace.version` Caused by: inheriting from a parent workspace is not implemented yet @@ -677,7 +677,7 @@ Caused by: failed to parse manifest at `[CWD]/bar/Cargo.toml` Caused by: - error reading `dependencies.detailed` from workspace root manifest's `[workspace.dependencies.detailed]` + error reading `dependencies.detailed` from workspace root manifest's `workspace.dependencies.detailed` Caused by: inheriting from a parent workspace is not implemented yet @@ -724,7 +724,7 @@ fn error_workspace_false() { [ERROR] failed to parse manifest at `[CWD]/Cargo.toml` Caused by: - workspace cannot be false for key `package.description` + `workspace=false` is unsupported for `package.description` ", ) .run(); @@ -768,7 +768,8 @@ fn error_workspace_dependency_looked_for_workspace_itself() { [ERROR] failed to parse manifest at `[CWD]/Cargo.toml` Caused by: - `dependencies.dep` specified `{ workspace = true}`, but workspace dependencies cannot do this + `workspace.dependencies.dep` specified `{ workspace = true }`, but workspace dependencies \ + cannot do this ", ) .run(); diff --git a/tests/testsuite/main.rs b/tests/testsuite/main.rs index 3a24015f311..a89324f9f95 100644 --- a/tests/testsuite/main.rs +++ b/tests/testsuite/main.rs @@ -39,7 +39,6 @@ mod cross_compile; mod cross_publish; mod custom_target; mod death; -mod deduplicate_workspace; mod dep_info; mod directory; mod doc; @@ -58,6 +57,7 @@ mod git_auth; mod git_gc; mod glob_targets; mod help; +mod inheritable_workspace_fields; mod init; mod install; mod install_upgrade;