diff --git a/src/cargo/util/toml/mod.rs b/src/cargo/util/toml/mod.rs index 28cdb99fdd8..bc8db23cedf 100644 --- a/src/cargo/util/toml/mod.rs +++ b/src/cargo/util/toml/mod.rs @@ -131,14 +131,6 @@ pub fn read_manifest_from_str( name ); } - if let TomlDependency::Workspace(_) = dep { - bail!( - "{} was specified as `workspace.dependencies.{}.workspace = true`, but \ - workspace dependencies cannot specify `workspace = true`", - name, - name - ); - } } } return if manifest.project.is_some() || manifest.package.is_some() { @@ -229,8 +221,6 @@ 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 = "" }` @@ -266,43 +256,9 @@ impl<'de, P: Deserialize<'de> + Clone> de::Deserialize<'de> for TomlDependency

, { let mvd = de::value::MapAccessDeserializer::new(map); - let details: IntermediateDependency

= IntermediateDependency::deserialize(mvd)?; - if let Some(workspace) = details.workspace { - if workspace { - Ok(TomlDependency::Workspace(TomlWorkspaceDependency { - workspace: true, - features: details.features, - default_features: details.default_features, - default_features2: details.default_features2, - 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, - })) - } + DetailedTomlDependency::deserialize(mvd).map(TomlDependency::Detailed) } } - deserializer.deserialize_any(TomlDependencyVisitor(PhantomData)) } } @@ -323,43 +279,6 @@ 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)] -#[serde(rename_all = "kebab-case")] -pub struct TomlWorkspaceDependency { - workspace: bool, - features: Option>, - default_features: Option, - #[serde(rename = "default_features")] - default_features2: Option, - optional: Option, -} - #[derive(Deserialize, Serialize, Clone, Debug)] #[serde(rename_all = "kebab-case")] pub struct DetailedTomlDependency { @@ -433,19 +352,19 @@ pub struct TomlManifest { example: Option>, test: Option>, bench: Option>, - dependencies: Option>, - dev_dependencies: Option>, + dependencies: Option>, + dev_dependencies: Option>, #[serde(rename = "dev_dependencies")] - dev_dependencies2: Option>, - build_dependencies: Option>, + dev_dependencies2: Option>, + build_dependencies: Option>, #[serde(rename = "build_dependencies")] - build_dependencies2: Option>, + build_dependencies2: Option>, features: Option>>, target: Option>, replace: Option>, patch: Option>>, workspace: Option, - badges: Option>>>, + badges: Option>>>, } #[derive(Deserialize, Serialize, Clone, Debug, Default)] @@ -1008,14 +927,14 @@ impl<'de> de::Deserialize<'de> for VecStringOrBool { fn version_trim_whitespace<'de, D>( deserializer: D, -) -> Result, D::Error> +) -> Result, D::Error> where D: de::Deserializer<'de>, { struct Visitor; impl<'de> de::Visitor<'de> for Visitor { - type Value = MaybeWorkspace; + type Value = MaybeWorkspaceField; fn expecting(&self, formatter: &mut fmt::Formatter<'_>) -> fmt::Result { formatter.write_str("SemVer version") @@ -1043,52 +962,87 @@ where deserializer.deserialize_any(Visitor) } -/// Enum that allows for the parsing of `field.workspace = true` in a Cargo.toml +/// This Trait exists to make [`MaybeWorkspace::Workspace`] generic. It makes deserialization of +/// [`MaybeWorkspace`] much easier, as well as making error messages for +/// [`MaybeWorkspace::resolve`] much nicer /// -/// It allows for things to be inherited from a workspace or defined as needed +/// Implementors should have a field `workspace` with the type of `bool`. It is used to ensure +/// `workspace` is not `false` in a `Cargo.toml` +pub trait WorkspaceInherit { + /// This is the workspace table that is being inherited from. + /// For example `[workspace.dependencies]` would be the table "dependencies" + fn inherit_toml_table(&self) -> &str; + + /// This is used to output the value of the implementors `workspace` field + fn workspace(&self) -> bool; +} + +/// An enum that allows for inheriting keys from a workspace in a Cargo.toml. #[derive(Serialize, Clone, Debug)] #[serde(untagged)] -pub enum MaybeWorkspace { - Workspace(TomlWorkspaceField), +pub enum MaybeWorkspace { + /// The "defined" type, or the type that that is used when not inheriting from a workspace. Defined(T), + /// The type when inheriting from a workspace. + Workspace(W), } -impl<'de, T: Deserialize<'de>> de::Deserialize<'de> for MaybeWorkspace { - fn deserialize(deserializer: D) -> Result, D::Error> +impl<'de, T: Deserialize<'de>, W: WorkspaceInherit + de::Deserialize<'de>> de::Deserialize<'de> + for MaybeWorkspace +{ + fn deserialize(deserializer: D) -> Result, D::Error> where D: de::Deserializer<'de>, { let value = serde_value::Value::deserialize(deserializer)?; - if let Ok(workspace) = TomlWorkspaceField::deserialize(serde_value::ValueDeserializer::< - D::Error, - >::new(value.clone())) - { - return Ok(MaybeWorkspace::Workspace(workspace)); + + if let Ok(w) = W::deserialize(serde_value::ValueDeserializer::::new( + value.clone(), + )) { + return if w.workspace() { + Ok(MaybeWorkspace::Workspace(w)) + } else { + Err(de::Error::custom("`workspace` cannot be false")) + }; } T::deserialize(serde_value::ValueDeserializer::::new(value)) .map(MaybeWorkspace::Defined) } } -impl MaybeWorkspace { +impl MaybeWorkspace { fn resolve<'a>( self, label: &str, - get_ws_field: impl FnOnce() -> CargoResult, + get_ws_inheritable: impl FnOnce() -> CargoResult, + ) -> CargoResult { + match self { + MaybeWorkspace::Defined(value) => Ok(value), + MaybeWorkspace::Workspace(w) => get_ws_inheritable().with_context(|| { + format!( + "error inheriting `{label}` from workspace root manifest's `workspace.{}.{label}`", + w.inherit_toml_table(), + ) + }), + } + } + + fn resolve_with_self<'a>( + self, + label: &str, + get_ws_inheritable: impl FnOnce(&W) -> CargoResult, ) -> CargoResult { match self { MaybeWorkspace::Defined(value) => Ok(value), - MaybeWorkspace::Workspace(TomlWorkspaceField { workspace: true }) => get_ws_field() - .context(format!( - "error inheriting `{}` from workspace root manifest's `workspace.package.{}`", - label, label - )), - MaybeWorkspace::Workspace(TomlWorkspaceField { workspace: false }) => Err(anyhow!( - "`workspace=false` is unsupported for `package.{}`", - label, - )), + MaybeWorkspace::Workspace(w) => get_ws_inheritable(&w).with_context(|| { + format!( + "error inheriting `{label}` from workspace root manifest's `workspace.{}.{label}`", + w.inherit_toml_table(), + ) + }), } } + fn as_defined(&self) -> Option<&T> { match self { MaybeWorkspace::Workspace(_) => None, @@ -1097,11 +1051,119 @@ impl MaybeWorkspace { } } +type MaybeWorkspaceDependency = MaybeWorkspace; + +#[derive(Deserialize, Serialize, Clone, Debug)] +#[serde(rename_all = "kebab-case")] +pub struct TomlWorkspaceDependency { + workspace: bool, + features: Option>, + default_features: Option, + #[serde(rename = "default_features")] + default_features2: Option, + optional: Option, +} + +impl WorkspaceInherit for TomlWorkspaceDependency { + fn inherit_toml_table(&self) -> &str { + "dependencies" + } + + fn workspace(&self) -> bool { + self.workspace + } +} + +impl TomlWorkspaceDependency { + fn resolve<'a>( + &self, + name: &str, + inheritable: impl FnOnce() -> CargoResult<&'a InheritableFields>, + cx: &mut Context<'_, '_>, + ) -> CargoResult { + fn default_features_msg(label: &str, ws_def_feat: Option, cx: &mut Context<'_, '_>) { + let ws_def_feat = match ws_def_feat { + Some(true) => "true", + Some(false) => "false", + None => "not specified", + }; + cx.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 self.default_features.is_some() && self.default_features2.is_some() { + warn_on_deprecated("default-features", name, "dependency", cx.warnings); + } + inheritable()?.get_dependency(name, cx.root).map(|d| { + match d { + TomlDependency::Simple(s) => { + if let Some(false) = self.default_features.or(self.default_features2) { + default_features_msg(name, None, cx); + } + if self.optional.is_some() || self.features.is_some() { + TomlDependency::Detailed(DetailedTomlDependency { + version: Some(s), + optional: self.optional, + features: self.features.clone(), + ..Default::default() + }) + } else { + TomlDependency::Simple(s) + } + } + TomlDependency::Detailed(d) => { + let mut d = d.clone(); + match ( + self.default_features.or(self.default_features2), + d.default_features.or(d.default_features2), + ) { + // 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), cx); + } + // member: default-features = false and + // workspace: dep = "1.0" should ignore member default-features + (Some(false), None) => { + default_features_msg(name, None, cx); + } + _ => {} + } + d.add_features(self.features.clone()); + d.update_optional(self.optional); + TomlDependency::Detailed(d) + } + } + }) + } +} + +type MaybeWorkspaceField = MaybeWorkspace; + #[derive(Deserialize, Serialize, Clone, Debug)] pub struct TomlWorkspaceField { workspace: bool, } +impl WorkspaceInherit for TomlWorkspaceField { + fn inherit_toml_table(&self) -> &str { + "package" + } + + fn workspace(&self) -> bool { + self.workspace + } +} + /// Represents the `package`/`project` sections of a `Cargo.toml`. /// /// Note that the order of the fields matters, since this is the order they @@ -1111,12 +1173,12 @@ pub struct TomlWorkspaceField { #[derive(Deserialize, Serialize, Clone, Debug)] #[serde(rename_all = "kebab-case")] pub struct TomlPackage { - edition: Option>, - rust_version: Option>, + edition: Option>, + rust_version: Option>, name: InternedString, #[serde(deserialize_with = "version_trim_whitespace")] - version: MaybeWorkspace, - authors: Option>>, + version: MaybeWorkspaceField, + authors: Option>>, build: Option, metabuild: Option, #[serde(rename = "default-target")] @@ -1124,9 +1186,9 @@ pub struct TomlPackage { #[serde(rename = "forced-target")] forced_target: Option, links: Option, - exclude: Option>>, - include: Option>>, - publish: Option>, + exclude: Option>>, + include: Option>>, + publish: Option>, workspace: Option, im_a_teapot: Option, autobins: Option, @@ -1136,15 +1198,15 @@ pub struct TomlPackage { 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 @@ -1217,7 +1279,7 @@ impl InheritableFields { ) } - pub fn get_dependency(&self, name: &str) -> CargoResult { + pub fn get_dependency(&self, name: &str, package_root: &Path) -> CargoResult { self.dependencies.clone().map_or( Err(anyhow!("`workspace.dependencies` was not defined")), |deps| { @@ -1226,7 +1288,13 @@ impl InheritableFields { "`dependency.{}` was not found in `workspace.dependencies`", name )), - |dep| Ok(dep.clone()), + |dep| { + let mut dep = dep.clone(); + if let TomlDependency::Detailed(detailed) = &mut dep { + detailed.resolve_path(name, self.ws_root(), package_root)? + } + Ok(dep) + }, ) }, ) @@ -1532,24 +1600,33 @@ impl TomlManifest { fn map_deps( config: &Config, - deps: Option<&BTreeMap>, + deps: Option<&BTreeMap>, filter: impl Fn(&TomlDependency) -> bool, - ) -> CargoResult>> { + ) -> CargoResult>> { let deps = match deps { Some(deps) => deps, None => return Ok(None), }; let deps = deps .iter() - .filter(|(_k, v)| filter(v)) + .filter(|(_k, v)| { + if let MaybeWorkspace::Defined(def) = v { + filter(def) + } else { + false + } + }) .map(|(k, v)| Ok((k.clone(), map_dependency(config, v)?))) .collect::>>()?; Ok(Some(deps)) } - fn map_dependency(config: &Config, dep: &TomlDependency) -> CargoResult { - match dep { - TomlDependency::Detailed(d) => { + fn map_dependency( + config: &Config, + dep: &MaybeWorkspaceDependency, + ) -> CargoResult { + let dep = match dep { + MaybeWorkspace::Defined(TomlDependency::Detailed(d)) => { let mut d = d.clone(); // Path dependencies become crates.io deps. d.path.take(); @@ -1562,15 +1639,16 @@ impl TomlManifest { if let Some(registry) = d.registry.take() { d.registry_index = Some(config.get_registry_index(®istry)?.to_string()); } - Ok(TomlDependency::Detailed(d)) + Ok(d) } - TomlDependency::Simple(s) => Ok(TomlDependency::Detailed(DetailedTomlDependency { + MaybeWorkspace::Defined(TomlDependency::Simple(s)) => Ok(DetailedTomlDependency { version: Some(s.clone()), ..Default::default() - })), - // Unreachable as we resolve everything before this - TomlDependency::Workspace(_) => unreachable!(), - } + }), + _ => unreachable!(), + }; + dep.map(TomlDependency::Detailed) + .map(MaybeWorkspace::Defined) } } @@ -1822,29 +1900,31 @@ impl TomlManifest { fn process_dependencies( cx: &mut Context<'_, '_>, - new_deps: Option<&BTreeMap>, + new_deps: Option<&BTreeMap>, kind: Option, workspace_config: &WorkspaceConfig, inherit_cell: &LazyCell, - ) -> CargoResult>> { + ) -> CargoResult>> { let dependencies = match new_deps { Some(dependencies) => dependencies, None => return Ok(None), }; - let inherit = || { + let inheritable = || { inherit_cell.try_borrow_with(|| { get_ws(cx.config, &cx.root.join("Cargo.toml"), &workspace_config) }) }; - let mut deps: BTreeMap = BTreeMap::new(); + let mut deps: BTreeMap = BTreeMap::new(); for (n, v) in dependencies.iter() { - let resolved = v.clone().resolve(n, cx, || inherit())?; + let resolved = v + .clone() + .resolve_with_self(n, |dep| dep.resolve(n, inheritable, cx))?; 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()); + deps.insert(n.to_string(), MaybeWorkspace::Defined(resolved.clone())); } Ok(Some(deps)) } @@ -2557,7 +2637,6 @@ impl TomlDependency

{ } .to_dependency(name, cx, kind), TomlDependency::Detailed(ref details) => details.to_dependency(name, cx, kind), - TomlDependency::Workspace(_) => unreachable!(), } } @@ -2565,7 +2644,6 @@ impl TomlDependency

{ match self { TomlDependency::Detailed(d) => d.version.is_some(), TomlDependency::Simple(..) => true, - TomlDependency::Workspace(_) => unreachable!(), } } @@ -2573,111 +2651,6 @@ impl TomlDependency

{ match self { TomlDependency::Detailed(d) => d.optional.unwrap_or(false), TomlDependency::Simple(..) => false, - TomlDependency::Workspace(w) => w.optional.unwrap_or(false), - } - } -} - -impl TomlDependency { - fn resolve<'a>( - self, - label: &str, - cx: &mut Context<'_, '_>, - get_inheritable: impl FnOnce() -> CargoResult<&'a InheritableFields>, - ) -> CargoResult { - fn default_features_msg(label: &str, ws_def_feat: Option, cx: &mut Context<'_, '_>) { - let ws_def_feat = match ws_def_feat { - Some(true) => "true", - Some(false) => "false", - None => "not specified", - }; - cx.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" - )) - } - match self { - TomlDependency::Detailed(d) => Ok(TomlDependency::Detailed(d)), - TomlDependency::Simple(s) => Ok(TomlDependency::Simple(s)), - TomlDependency::Workspace(TomlWorkspaceDependency { - workspace: true, - features, - optional, - default_features, - default_features2, - }) => { - if default_features.is_some() && default_features2.is_some() { - warn_on_deprecated("default-features", label, "dependency", cx.warnings); - } - let inheritable = get_inheritable()?; - inheritable.get_dependency(label).context(format!( - "error reading `dependencies.{}` from workspace root manifest's `workspace.dependencies.{}`", - label, label - )).map(|dep| { - match dep { - TomlDependency::Simple(s) => { - if let Some(false) = default_features.or(default_features2) { - default_features_msg(label, None, cx); - } - if optional.is_some() || features.is_some() { - Ok(TomlDependency::Detailed(DetailedTomlDependency { - version: Some(s), - optional, - features, - ..Default::default() - })) - } else { - Ok(TomlDependency::Simple(s)) - } - }, - TomlDependency::Detailed(d) => { - let mut dep = d.clone(); - match ( - default_features.or(default_features2), - d.default_features.or(d.default_features2) - ) { - // member: default-features = true and - // workspace: default-features = false should turn on - // default-features - (Some(true), Some(false)) => { - dep.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(label, Some(true), cx); - } - // member: default-features = false and - // workspace: dep = "1.0" should ignore member default-features - (Some(false), None) => { - default_features_msg(label, None, cx); - } - _ => {} - } - dep.add_features(features); - dep.update_optional(optional); - dep.resolve_path(label,inheritable.ws_root(), cx.root)?; - Ok(TomlDependency::Detailed(dep)) - }, - 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=false` is unsupported for `package.dependencies.{}`", - label, - )), } } } @@ -3017,15 +2990,15 @@ impl ser::Serialize for PathValue { /// Corresponds to a `target` entry, but `TomlTarget` is already used. #[derive(Serialize, Deserialize, Debug, Clone)] struct TomlPlatform { - dependencies: Option>, + dependencies: Option>, #[serde(rename = "build-dependencies")] - build_dependencies: Option>, + build_dependencies: Option>, #[serde(rename = "build_dependencies")] - build_dependencies2: Option>, + build_dependencies2: Option>, #[serde(rename = "dev-dependencies")] - dev_dependencies: Option>, + dev_dependencies: Option>, #[serde(rename = "dev_dependencies")] - dev_dependencies2: Option>, + dev_dependencies2: Option>, } impl TomlTarget { diff --git a/tests/testsuite/inheritable_workspace_fields.rs b/tests/testsuite/inheritable_workspace_fields.rs index fe4667f6263..705036ab184 100644 --- a/tests/testsuite/inheritable_workspace_fields.rs +++ b/tests/testsuite/inheritable_workspace_fields.rs @@ -1174,7 +1174,7 @@ fn error_workspace_false() { [ERROR] failed to parse manifest at `[CWD]/Cargo.toml` Caused by: - `workspace=false` is unsupported for `package.description` + `workspace` cannot be false for key `package.description` ", ) .run(); @@ -1193,13 +1193,12 @@ fn error_workspace_dependency_looked_for_workspace_itself() { [package] name = "bar" version = "1.2.3" - workspace = ".." [dependencies] dep.workspace = true [workspace] - members = ["bar"] + members = [] [workspace.dependencies] dep.workspace = true @@ -1213,11 +1212,12 @@ fn error_workspace_dependency_looked_for_workspace_itself() { .with_status(101) .with_stderr( "\ -[ERROR] failed to parse manifest at `[CWD]/Cargo.toml` - -Caused by: - dep was specified as `workspace.dependencies.dep.workspace = true`, but \ - workspace dependencies cannot specify `workspace = true` +[WARNING] [CWD]/Cargo.toml: dependency (dep) specified without providing a local path, Git repository, or version to use. This will be considered an error in future versions +[WARNING] [CWD]/Cargo.toml: unused manifest key: workspace.dependencies.dep.workspace +[UPDATING] `dummy-registry` index +[ERROR] no matching package named `dep` found +location searched: registry `crates-io` +required by package `bar v1.2.3 ([CWD])` ", ) .run(); @@ -1348,7 +1348,7 @@ fn error_inherit_unspecified_dependency() { [ERROR] failed to parse manifest at `[CWD]/Cargo.toml` Caused by: - error reading `dependencies.foo` from workspace root manifest's `workspace.dependencies.foo` + error inheriting `foo` from workspace root manifest's `workspace.dependencies.foo` Caused by: `workspace.dependencies` was not defined @@ -1498,3 +1498,48 @@ fn inherit_def_feat_false_member_def_feat_true() { ) .run(); } + +#[cargo_test] +fn cannot_inherit_in_patch() { + Package::new("bar", "0.1.0").publish(); + + let p = project() + .file( + "Cargo.toml", + r#" + [workspace] + members = [] + + [workspace.dependencies] + bar = { path = "bar" } + + [package] + name = "foo" + version = "0.2.0" + + [patch.crates-io] + bar.workspace = true + + [dependencies] + bar = "0.1.0" + + "#, + ) + .file("src/main.rs", "fn main() {}") + .build(); + + p.cargo("check") + .with_status(101) + .with_stderr( + "\ +[WARNING] [CWD]/Cargo.toml: dependency (bar) specified without providing a local path, Git repository, or version to use. This will be considered an error in future versions +[WARNING] [CWD]/Cargo.toml: unused manifest key: patch.crates-io.bar.workspace +[UPDATING] `dummy-registry` index +[ERROR] failed to resolve patches for `/~https://github.com/rust-lang/crates.io-index` + +Caused by: + patch for `bar` in `/~https://github.com/rust-lang/crates.io-index` points to the same source, but patches must point to different sources +", + ) + .run(); +}