From b127a4c8946c5683cb3aada40841e24d761b4af8 Mon Sep 17 00:00:00 2001 From: scott Date: Fri, 1 Apr 2022 15:15:22 -0500 Subject: [PATCH 1/4] Fixed bug on `TomlDependncy::Simple` where it would not inherit `optional` or `features` correctly --- src/cargo/util/toml/mod.rs | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-) diff --git a/src/cargo/util/toml/mod.rs b/src/cargo/util/toml/mod.rs index b49bdce5139..3db1cf4649d 100644 --- a/src/cargo/util/toml/mod.rs +++ b/src/cargo/util/toml/mod.rs @@ -2196,7 +2196,18 @@ impl TomlDependency

{ label, label )).map(|dep| { match dep { - TomlDependency::Simple(s) => TomlDependency::Simple(s), + TomlDependency::Simple(s) => { + if optional.is_some() || features.is_some() { + TomlDependency::Detailed(DetailedTomlDependency::

{ + version: Some(s), + optional, + features, + ..Default::default() + }) + } else { + TomlDependency::Simple(s) + } + }, TomlDependency::Detailed(d) => { let mut dep = d.clone(); dep.add_features(features); From 035cb09f0563ec241edb6abe688843160a7671f4 Mon Sep 17 00:00:00 2001 From: scott Date: Mon, 4 Apr 2022 15:06:52 -0500 Subject: [PATCH 2/4] Extracted loop over ancestor paths when searching for a workspace root to its own function --- src/cargo/core/workspace.rs | 117 +++++++++++++++++++++++++----------- 1 file changed, 82 insertions(+), 35 deletions(-) diff --git a/src/cargo/core/workspace.rs b/src/cargo/core/workspace.rs index 7091e216088..5393983ed64 100644 --- a/src/cargo/core/workspace.rs +++ b/src/cargo/core/workspace.rs @@ -615,43 +615,33 @@ impl<'cfg> Workspace<'cfg> { } } - for path in paths::ancestors(manifest_path, None).skip(2) { - if path.ends_with("target/package") { - break; - } - - let ances_manifest_path = path.join("Cargo.toml"); - debug!("find_root - trying {}", ances_manifest_path.display()); - if ances_manifest_path.exists() { - match *self.packages.load(&ances_manifest_path)?.workspace_config() { - WorkspaceConfig::Root(ref ances_root_config) => { - debug!("find_root - found a root checking exclusion"); - if !ances_root_config.is_excluded(manifest_path) { - debug!("find_root - found!"); - return Ok(Some(ances_manifest_path)); + find_root_iter(manifest_path, self.config) + .find_map(|ances_manifest_path| { + debug!("find_root - trying {}", ances_manifest_path.display()); + let manifest = self.packages.load(&ances_manifest_path); + match manifest { + Ok(manifest) => match *manifest.workspace_config() { + WorkspaceConfig::Root(ref ances_root_config) => { + debug!("find_root - found a root checking exclusion"); + if !ances_root_config.is_excluded(manifest_path) { + debug!("find_root - found!"); + Some(Ok(ances_manifest_path)) + } else { + None + } } - } - WorkspaceConfig::Member { - root: Some(ref path_to_root), - } => { - debug!("find_root - found pointer"); - return Ok(Some(read_root_pointer(&ances_manifest_path, path_to_root))); - } - WorkspaceConfig::Member { .. } => {} + WorkspaceConfig::Member { + root: Some(ref path_to_root), + } => { + debug!("find_root - found pointer"); + Some(Ok(read_root_pointer(&ances_manifest_path, path_to_root))) + } + WorkspaceConfig::Member { .. } => None, + }, + Err(e) => Some(Err(e)), } - } - - // Don't walk across `CARGO_HOME` when we're looking for the - // workspace root. Sometimes a package will be organized with - // `CARGO_HOME` pointing inside of the workspace root or in the - // current package, but we don't want to mistakenly try to put - // crates.io crates into the workspace by accident. - if self.config.home() == path { - break; - } - } - - Ok(None) + }) + .transpose() } /// After the root of a workspace has been located, probes for all members @@ -1841,3 +1831,60 @@ impl InheritableFields { }) } } + +pub fn find_root_iter<'a>( + manifest_path: &'a Path, + config: &'a Config, +) -> impl Iterator + 'a { + LookBehind::new(paths::ancestors(manifest_path, None).skip(2)) + .into_iter() + .take_while(|path| !path.curr.ends_with("target/package")) + // Don't walk across `CARGO_HOME` when we're looking for the + // workspace root. Sometimes a package will be organized with + // `CARGO_HOME` pointing inside of the workspace root or in the + // current package, but we don't want to mistakenly try to put + // crates.io crates into the workspace by accident. + .take_while(|path| { + if let Some(last) = path.last { + config.home() != last + } else { + true + } + }) + .map(|path| path.curr.join("Cargo.toml")) + .filter(|ances_manifest_path| ances_manifest_path.exists()) +} + +struct LookBehindWindow<'a, T: ?Sized> { + curr: &'a T, + last: Option<&'a T>, +} + +struct LookBehind<'a, T: ?Sized, K: Iterator> { + iter: K, + last: Option<&'a T>, +} + +impl<'a, T: ?Sized, K: Iterator> LookBehind<'a, T, K> { + fn new(items: K) -> Self { + Self { + iter: items, + last: None, + } + } +} + +impl<'a, T: ?Sized, K: Iterator> Iterator for LookBehind<'a, T, K> { + type Item = LookBehindWindow<'a, T>; + + fn next(&mut self) -> Option { + match self.iter.next() { + None => None, + Some(next) => { + let last = self.last; + self.last = Some(next); + Some(LookBehindWindow { curr: next, last }) + } + } + } +} From 0cd55465a4179c60f3c308e5f9a80471bc9e6a44 Mon Sep 17 00:00:00 2001 From: scott Date: Mon, 4 Apr 2022 15:32:46 -0500 Subject: [PATCH 3/4] Part 2 of RFC2906 - allow inheriting from parent workspaces --- src/cargo/core/manifest.rs | 9 + src/cargo/core/mod.rs | 3 +- src/cargo/core/workspace.rs | 62 +- src/cargo/util/toml/mod.rs | 135 +++-- .../testsuite/inheritable_workspace_fields.rs | 551 +++++++++++++++++- 5 files changed, 685 insertions(+), 75 deletions(-) diff --git a/src/cargo/core/manifest.rs b/src/cargo/core/manifest.rs index 34d5062bdb0..bea5d181839 100644 --- a/src/cargo/core/manifest.rs +++ b/src/cargo/core/manifest.rs @@ -26,6 +26,15 @@ pub enum EitherManifest { Virtual(VirtualManifest), } +impl EitherManifest { + pub(crate) fn workspace_config(&self) -> &WorkspaceConfig { + match *self { + EitherManifest::Real(ref r) => r.workspace_config(), + EitherManifest::Virtual(ref v) => v.workspace_config(), + } + } +} + /// Contains all the information about a package, as loaded from a `Cargo.toml`. /// /// This is deserialized using the [`TomlManifest`] type. diff --git a/src/cargo/core/mod.rs b/src/cargo/core/mod.rs index b52fdef63de..633b29907fa 100644 --- a/src/cargo/core/mod.rs +++ b/src/cargo/core/mod.rs @@ -11,7 +11,8 @@ 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::{ - InheritableFields, MaybePackage, Workspace, WorkspaceConfig, WorkspaceRootConfig, + find_workspace_root, InheritableFields, MaybePackage, Workspace, WorkspaceConfig, + WorkspaceRootConfig, }; pub mod compiler; diff --git a/src/cargo/core/workspace.rs b/src/cargo/core/workspace.rs index 5393983ed64..2f7cdbcda3c 100644 --- a/src/cargo/core/workspace.rs +++ b/src/cargo/core/workspace.rs @@ -591,16 +591,6 @@ impl<'cfg> Workspace<'cfg> { /// Returns an error if `manifest_path` isn't actually a valid manifest or /// if some other transient error happens. fn find_root(&mut self, manifest_path: &Path) -> CargoResult> { - fn read_root_pointer(member_manifest: &Path, root_link: &str) -> PathBuf { - let path = member_manifest - .parent() - .unwrap() - .join(root_link) - .join("Cargo.toml"); - debug!("find_root - pointer {}", path.display()); - paths::normalize_path(&path) - } - { let current = self.packages.load(manifest_path)?; match *current.workspace_config() { @@ -1643,6 +1633,10 @@ impl WorkspaceRootConfig { .collect::, _>>()?; Ok(res) } + + pub fn inheritable(&self) -> &InheritableFields { + &self.inheritable_fields + } } /// A group of fields that are inheritable by members of the workspace @@ -1832,7 +1826,53 @@ impl InheritableFields { } } -pub fn find_root_iter<'a>( +fn parse_manifest(manifest_path: &Path, config: &Config) -> CargoResult { + let key = manifest_path.parent().unwrap(); + let source_id = SourceId::for_path(key)?; + let (manifest, _nested_paths) = read_manifest(manifest_path, source_id, config)?; + Ok(manifest) +} + +pub fn find_workspace_root(manifest_path: &Path, config: &Config) -> CargoResult> { + find_root_iter(manifest_path, config) + .find_map(|ances_manifest_path| { + let manifest = parse_manifest(&ances_manifest_path, config); + match manifest { + Ok(manifest) => match *manifest.workspace_config() { + WorkspaceConfig::Root(ref ances_root_config) => { + debug!("find_root - found a root checking exclusion"); + if !ances_root_config.is_excluded(manifest_path) { + debug!("find_root - found!"); + Some(Ok(ances_manifest_path)) + } else { + None + } + } + WorkspaceConfig::Member { + root: Some(ref path_to_root), + } => { + debug!("find_root - found pointer"); + Some(Ok(read_root_pointer(&ances_manifest_path, path_to_root))) + } + WorkspaceConfig::Member { .. } => None, + }, + Err(e) => Some(Err(e)), + } + }) + .transpose() +} + +fn read_root_pointer(member_manifest: &Path, root_link: &str) -> PathBuf { + let path = member_manifest + .parent() + .unwrap() + .join(root_link) + .join("Cargo.toml"); + debug!("find_root - pointer {}", path.display()); + paths::normalize_path(&path) +} + +fn find_root_iter<'a>( manifest_path: &'a Path, config: &'a Config, ) -> impl Iterator + 'a { diff --git a/src/cargo/util/toml/mod.rs b/src/cargo/util/toml/mod.rs index 3db1cf4649d..7dde82669b7 100644 --- a/src/cargo/util/toml/mod.rs +++ b/src/cargo/util/toml/mod.rs @@ -20,7 +20,7 @@ use crate::core::compiler::{CompileKind, CompileTarget}; 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::{find_workspace_root, Dependency, Manifest, PackageId, Summary, Target}; use crate::core::{ Edition, EitherManifest, Feature, Features, InheritableFields, VirtualManifest, Workspace, }; @@ -1283,13 +1283,30 @@ 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>) -> CargoResult<&InheritableFields> { - match inheritable { - Some(inheritable) => Ok(inheritable), - None => Err(anyhow!( - "inheriting from a parent workspace is not implemented yet", - )), + fn get_ws( + config: &Config, + resolved_path: PathBuf, + workspace_config: WorkspaceConfig, + ) -> CargoResult { + match workspace_config { + WorkspaceConfig::Root(root) => Ok(root.inheritable().clone()), + WorkspaceConfig::Member { + root: Some(ref path_to_root), + } => { + let path = resolved_path + .parent() + .unwrap() + .join(path_to_root) + .join("Cargo.toml"); + let root_path = paths::normalize_path(&path); + inheritable_from_path(config, root_path) + } + WorkspaceConfig::Member { root: None } => { + match find_workspace_root(&resolved_path, config)? { + Some(path_to_root) => inheritable_from_path(config, path_to_root), + None => Err(anyhow!("failed to find a workspace root")), + } + } } } @@ -1343,8 +1360,6 @@ impl TomlManifest { ), }; - let inheritable = workspace_config.inheritable(); - let package_name = project.name.trim(); if package_name.is_empty() { bail!("package name cannot be an empty string") @@ -1352,10 +1367,11 @@ impl TomlManifest { validate_package_name(package_name, "package name", "")?; - let version = project - .version - .clone() - .resolve(&features, "version", || get_ws(inheritable)?.version())?; + let resolved_path = package_root.join("Cargo.toml"); + + let version = project.version.clone().resolve(&features, "version", || { + get_ws(config, resolved_path.clone(), workspace_config.clone())?.version() + })?; project.version = MaybeWorkspace::Defined(version.clone()); @@ -1363,7 +1379,9 @@ impl TomlManifest { let edition = if let Some(edition) = project.edition.clone() { let edition: Edition = edition - .resolve(&features, "edition", || get_ws(inheritable)?.edition())? + .resolve(&features, "edition", || { + get_ws(config, resolved_path.clone(), workspace_config.clone())?.edition() + })? .parse() .with_context(|| "failed to parse the `edition` key")?; project.edition = Some(MaybeWorkspace::Defined(edition.to_string())); @@ -1480,7 +1498,7 @@ impl TomlManifest { cx: &mut Context<'_, '_>, new_deps: Option<&BTreeMap>, kind: Option, - inheritable: Option<&InheritableFields>, + workspace_config: &WorkspaceConfig, ) -> CargoResult>> { let dependencies = match new_deps { Some(dependencies) => dependencies, @@ -1488,9 +1506,14 @@ impl TomlManifest { }; let mut deps: BTreeMap = BTreeMap::new(); for (n, v) in dependencies.iter() { - let resolved = v - .clone() - .resolve(features, n, || get_ws(inheritable)?.get_dependency(n))?; + let resolved = v.clone().resolve(features, n, || { + get_ws( + cx.config, + cx.root.join("Cargo.toml"), + workspace_config.clone(), + )? + .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); @@ -1505,7 +1528,7 @@ impl TomlManifest { &mut cx, me.dependencies.as_ref(), None, - inheritable, + &workspace_config, )?; if me.dev_dependencies.is_some() && me.dev_dependencies2.is_some() { warn_on_deprecated("dev-dependencies", package_name, "package", cx.warnings); @@ -1519,7 +1542,7 @@ impl TomlManifest { &mut cx, dev_deps, Some(DepKind::Development), - inheritable, + &workspace_config, )?; if me.build_dependencies.is_some() && me.build_dependencies2.is_some() { warn_on_deprecated("build-dependencies", package_name, "package", cx.warnings); @@ -1533,7 +1556,7 @@ impl TomlManifest { &mut cx, build_deps, Some(DepKind::Build), - inheritable, + &workspace_config, )?; let mut target: BTreeMap = BTreeMap::new(); @@ -1548,7 +1571,7 @@ impl TomlManifest { &mut cx, platform.dependencies.as_ref(), None, - inheritable, + &workspace_config, ) .unwrap(); if platform.build_dependencies.is_some() && platform.build_dependencies2.is_some() { @@ -1563,7 +1586,7 @@ impl TomlManifest { &mut cx, build_deps, Some(DepKind::Build), - inheritable, + &workspace_config, ) .unwrap(); if platform.dev_dependencies.is_some() && platform.dev_dependencies2.is_some() { @@ -1578,7 +1601,7 @@ impl TomlManifest { &mut cx, dev_deps, Some(DepKind::Development), - inheritable, + &workspace_config, ) .unwrap(); target.insert( @@ -1635,21 +1658,27 @@ impl TomlManifest { .clone() .map(|mw| { mw.resolve(&features, "description", || { - get_ws(inheritable)?.description() + get_ws(config, resolved_path.clone(), workspace_config.clone())? + .description() }) }) .transpose()?, homepage: project .homepage .clone() - .map(|mw| mw.resolve(&features, "homepage", || get_ws(inheritable)?.homepage())) + .map(|mw| { + mw.resolve(&features, "homepage", || { + get_ws(config, resolved_path.clone(), workspace_config.clone())?.homepage() + }) + }) .transpose()?, documentation: project .documentation .clone() .map(|mw| { mw.resolve(&features, "documentation", || { - get_ws(inheritable)?.documentation() + get_ws(config, resolved_path.clone(), workspace_config.clone())? + .documentation() }) }) .transpose()?, @@ -1657,13 +1686,21 @@ impl TomlManifest { authors: project .authors .clone() - .map(|mw| mw.resolve(&features, "authors", || get_ws(inheritable)?.authors())) + .map(|mw| { + mw.resolve(&features, "authors", || { + get_ws(config, resolved_path.clone(), workspace_config.clone())?.authors() + }) + }) .transpose()? .unwrap_or_default(), license: project .license .clone() - .map(|mw| mw.resolve(&features, "license", || get_ws(inheritable)?.license())) + .map(|mw| { + mw.resolve(&features, "license", || { + get_ws(config, resolved_path.clone(), workspace_config.clone())?.license() + }) + }) .transpose()?, license_file: project.license_file.clone(), repository: project @@ -1671,14 +1708,19 @@ impl TomlManifest { .clone() .map(|mw| { mw.resolve(&features, "repository", || { - get_ws(inheritable)?.repository() + get_ws(config, resolved_path.clone(), workspace_config.clone())? + .repository() }) }) .transpose()?, keywords: project .keywords .clone() - .map(|mw| mw.resolve(&features, "keywords", || get_ws(inheritable)?.keywords())) + .map(|mw| { + mw.resolve(&features, "keywords", || { + get_ws(config, resolved_path.clone(), workspace_config.clone())?.keywords() + }) + }) .transpose()? .unwrap_or_default(), categories: project @@ -1686,7 +1728,8 @@ impl TomlManifest { .clone() .map(|mw| { mw.resolve(&features, "categories", || { - get_ws(inheritable)?.categories() + get_ws(config, resolved_path.clone(), workspace_config.clone())? + .categories() }) }) .transpose()? @@ -1694,7 +1737,11 @@ impl TomlManifest { badges: me .badges .clone() - .map(|mw| mw.resolve(&features, "badges", || get_ws(inheritable)?.badges())) + .map(|mw| { + mw.resolve(&features, "badges", || { + get_ws(config, resolved_path.clone(), workspace_config.clone())?.badges() + }) + }) .transpose()? .unwrap_or_default(), links: project.links.clone(), @@ -1739,7 +1786,9 @@ impl TomlManifest { let publish = project.publish.clone().map(|publish| { publish - .resolve(&features, "publish", || get_ws(inheritable)?.publish()) + .resolve(&features, "publish", || { + get_ws(config, resolved_path.clone(), workspace_config.clone())?.publish() + }) .unwrap() }); @@ -2073,6 +2122,22 @@ impl TomlManifest { } } +fn inheritable_from_path( + config: &Config, + resolved_path: PathBuf, +) -> CargoResult { + let key = resolved_path.parent().unwrap(); + let source_id = SourceId::for_path(key)?; + let (man, _) = read_manifest(&resolved_path, source_id, config)?; + match man.workspace_config() { + WorkspaceConfig::Root(root) => Ok(root.inheritable().clone()), + _ => bail!( + "root of a workspace inferred but wasn't a root: {}", + resolved_path.display() + ), + } +} + /// Returns the name of the README file for a `TomlProject`. fn readme_for_project(package_root: &Path, project: &TomlProject) -> Option { match &project.readme { diff --git a/tests/testsuite/inheritable_workspace_fields.rs b/tests/testsuite/inheritable_workspace_fields.rs index 4fa4484ff56..427a2adf0b0 100644 --- a/tests/testsuite/inheritable_workspace_fields.rs +++ b/tests/testsuite/inheritable_workspace_fields.rs @@ -1,6 +1,6 @@ //! 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}; +use cargo_test_support::{basic_lib_manifest, git, path2url, paths, project, publish, registry}; #[cargo_test] fn permit_additional_workspace_fields() { @@ -564,7 +564,7 @@ fn inherited_dependencies_union_features() { } #[cargo_test] -fn error_on_unimplemented_inheritance_fields() { +fn inherit_workspace_fields() { registry::init(); let p = project().build(); @@ -576,45 +576,403 @@ fn error_on_unimplemented_inheritance_fields() { [workspace] members = ["bar"] version = "1.2.3" + authors = ["Rustaceans"] + description = "This is a crate" + documentation = "https://www.rust-lang.org/learn" + homepage = "https://www.rust-lang.org" + repository = "https://github.com/example/example" + 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( "bar/Cargo.toml", r#" + badges = { workspace = true } cargo-features = ["workspace-inheritance"] - [package] name = "bar" workspace = ".." version = { workspace = true } + authors = { workspace = true } + description = { workspace = true } + documentation = { workspace = true } + homepage = { workspace = true } + repository = { workspace = true } + keywords = { workspace = true } + categories = { workspace = true } + publish = { workspace = true } + edition = { workspace = true } + "#, + ) + .file("bar/src/main.rs", "fn main() {}") + .build(); + + p.cargo("publish --token sekrit") + .masquerade_as_nightly_cargo() + .cwd("bar") + .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": null, + "license_file": null, + "links": null, + "name": "bar", + "readme": null, + "readme_file": null, + "repository": "https://github.com/example/example", + "vers": "1.2.3" + } + "#, + "bar-1.2.3.crate", + &[ + "Cargo.lock", + "Cargo.toml", + "Cargo.toml.orig", + "src/main.rs", + ".cargo_vcs_info.json", + ], + &[( + "Cargo.toml", + &format!( + r#"{} +cargo-features = ["workspace-inheritance"] + +[package] +edition = "2018" +name = "bar" +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" +keywords = ["cli"] +categories = ["development-tools"] +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_dependencies() { + let p = project() + .file( + "Cargo.toml", + r#" + [workspace] + members = ["bar"] + [workspace.dependencies] + dep = "0.1" + dep-build = "0.8" + dep-dev = "0.5.2" + "#, + ) + .file( + "bar/Cargo.toml", + r#" + cargo-features = ["workspace-inheritance"] + + [project] + workspace = ".." + name = "bar" + version = "0.2.0" + authors = [] + [dependencies] + dep = { workspace = true } + [build-dependencies] + dep-build = { workspace = true } + [dev-dependencies] + dep-dev = { workspace = true } "#, ) - .file("LICENSE", "license") - .file("README.md", "README.md") .file("bar/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() + .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]/bar) +[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() .cwd("bar") - .with_status(101) + .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_target_dependencies() { + let p = project() + .file( + "Cargo.toml", + r#" + [workspace] + members = ["bar"] + [workspace.dependencies] + dep = "0.1" + "#, + ) + .file( + "bar/Cargo.toml", + r#" + cargo-features = ["workspace-inheritance"] + + [project] + workspace = ".." + name = "bar" + version = "0.2.0" + authors = [] + [target.'cfg(unix)'.dependencies] + dep = { workspace = true } + [target.'cfg(windows)'.dependencies] + dep = { workspace = true } + "#, + ) + .file("bar/src/main.rs", "fn main() {}") + .build(); + + Package::new("dep", "0.1.2").publish(); + + p.cargo("build") + .masquerade_as_nightly_cargo() .with_stderr( "\ -[ERROR] failed to parse manifest at `[CWD]/Cargo.toml` +[UPDATING] `[..]` index +[DOWNLOADING] crates ... +[DOWNLOADED] dep v0.1.2 ([..]) +[COMPILING] dep v0.1.2 +[COMPILING] bar v0.2.0 ([CWD]/bar) +[FINISHED] dev [unoptimized + debuginfo] target(s) in [..] +", + ) + .run(); -Caused by: - error inheriting `version` from workspace root manifest's `workspace.version` + let lockfile = p.read_lockfile(); + assert!(lockfile.contains("dep")); +} -Caused by: - inheriting from a parent workspace is not implemented yet +#[cargo_test] +fn inherit_dependency_override_optional() { + Package::new("dep", "0.1.0").publish(); + + let p = project() + .file( + "Cargo.toml", + r#" + [workspace] + members = ["bar"] + [workspace.dependencies] + dep = "0.1.0" + "#, + ) + .file( + "bar/Cargo.toml", + r#" + cargo-features = ["workspace-inheritance"] + + [project] + workspace = ".." + name = "bar" + version = "0.2.0" + authors = [] + [dependencies] + dep = { workspace = true, optional = true } + "#, + ) + .file("bar/src/main.rs", "fn main() {}") + .build(); + + p.cargo("build") + .masquerade_as_nightly_cargo() + .with_stderr( + "\ +[UPDATING] `[..]` index +[COMPILING] bar v0.2.0 ([CWD]/bar) +[FINISHED] dev [unoptimized + debuginfo] target(s) in [..] +", + ) + .run(); +} + +#[cargo_test] +fn inherit_dependency_features() { + Package::new("dep", "0.1.0") + .feature("fancy", &["fancy_dep"]) + .add_dep(Dependency::new("fancy_dep", "0.2").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 = ["fancy"] } + + [workspace] + members = [] + [workspace.dependencies] + dep = "0.1" + "#, + ) + .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 ([..]) +[COMPILING] fancy_dep v0.2.4 +[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")); } #[cargo_test] -fn error_on_unimplemented_inheritance_dependencies() { +fn inherit_detailed_dependencies() { let git_project = git::new("detailed", |project| { project .file("Cargo.toml", &basic_lib_manifest("detailed")) @@ -641,7 +999,6 @@ fn error_on_unimplemented_inheritance_dependencies() { r#" [workspace] members = ["bar"] - [workspace.dependencies] detailed = {{ git = '{}', branch = "branchy" }} "#, @@ -658,7 +1015,6 @@ fn error_on_unimplemented_inheritance_dependencies() { name = "bar" version = "0.2.0" authors = [] - [dependencies] detailed = { workspace = true } "#, @@ -666,23 +1022,19 @@ fn error_on_unimplemented_inheritance_dependencies() { .file("bar/src/main.rs", "fn main() {}") .build(); + let git_root = git_project.root(); + p.cargo("build") .masquerade_as_nightly_cargo() - .with_status(101) - .with_stderr( + .with_stderr(&format!( "\ -[ERROR] failed to load manifest for workspace member `[CWD]/bar` - -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` - -Caused by: - inheriting from a parent workspace is not implemented yet -", - ) +[UPDATING] git repository `{}`\n\ +[COMPILING] detailed v0.5.0 ({}?branch=branchy#[..])\n\ +[COMPILING] bar v0.2.0 ([CWD]/bar)\n\ +[FINISHED] dev [unoptimized + debuginfo] target(s) in [..]\n", + path2url(&git_root), + path2url(&git_root), + )) .run(); } @@ -775,6 +1127,149 @@ Caused by: .run(); } +#[cargo_test] +fn error_malformed_workspace_root() { + registry::init(); + + let p = project().build(); + + let _ = git::repo(&paths::root().join("foo")) + .file( + "Cargo.toml", + r#" + [workspace] + members = [invalid toml + "#, + ) + .file("src/main.rs", "fn main() {}") + .file( + "bar/Cargo.toml", + r#" + cargo-features = ["workspace-inheritance"] + + [package] + name = "bar" + workspace = ".." + version = "1.2.3" + authors = ["rustaceans"] + "#, + ) + .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 `[..]/foo/Cargo.toml` + +Caused by: + [..] + +Caused by: + [..] + | + 3 | members = [invalid toml + | ^ + Unexpected `i` + Expected newline or `#` +", + ) + .run(); +} + +#[cargo_test] +fn error_no_root_workspace() { + registry::init(); + + let p = project().build(); + + let _ = git::repo(&paths::root().join("foo")) + .file( + "bar/Cargo.toml", + r#" + cargo-features = ["workspace-inheritance"] + + [package] + name = "bar" + workspace = ".." + version = "1.2.3" + authors = ["rustaceans"] + description = { workspace = true } + "#, + ) + .file("src/main.rs", "fn main() {}") + .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 `[..]/Cargo.toml` + +Caused by: + error inheriting `description` from workspace root manifest's `workspace.description` + +Caused by: + root of a workspace inferred but wasn't a root: [..]/Cargo.toml +", + ) + .run(); +} + +#[cargo_test] +fn error_inherit_unspecified_dependency() { + 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#" + cargo-features = ["workspace-inheritance"] + + [package] + name = "bar" + workspace = ".." + version = "1.2.3" + authors = ["rustaceans"] + [dependencies] + foo = { workspace = true } + "#, + ) + .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: + error reading `dependencies.foo` from workspace root manifest's `workspace.dependencies.foo` + +Caused by: + `workspace.dependencies` was not defined +", + ) + .run(); +} + #[cargo_test] fn workspace_inheritance_not_enabled() { registry::init(); From 8b3ce981caa65d48e91436f78dcad26b8263fbc6 Mon Sep 17 00:00:00 2001 From: scott Date: Tue, 5 Apr 2022 11:28:58 -0500 Subject: [PATCH 4/4] Change searching for a workspace root from find_map() to explicit loop --- src/cargo/core/workspace.rs | 90 ++++++++++++++++--------------------- 1 file changed, 38 insertions(+), 52 deletions(-) diff --git a/src/cargo/core/workspace.rs b/src/cargo/core/workspace.rs index 2f7cdbcda3c..7de07f53a3e 100644 --- a/src/cargo/core/workspace.rs +++ b/src/cargo/core/workspace.rs @@ -605,33 +605,26 @@ impl<'cfg> Workspace<'cfg> { } } - find_root_iter(manifest_path, self.config) - .find_map(|ances_manifest_path| { - debug!("find_root - trying {}", ances_manifest_path.display()); - let manifest = self.packages.load(&ances_manifest_path); - match manifest { - Ok(manifest) => match *manifest.workspace_config() { - WorkspaceConfig::Root(ref ances_root_config) => { - debug!("find_root - found a root checking exclusion"); - if !ances_root_config.is_excluded(manifest_path) { - debug!("find_root - found!"); - Some(Ok(ances_manifest_path)) - } else { - None - } - } - WorkspaceConfig::Member { - root: Some(ref path_to_root), - } => { - debug!("find_root - found pointer"); - Some(Ok(read_root_pointer(&ances_manifest_path, path_to_root))) - } - WorkspaceConfig::Member { .. } => None, - }, - Err(e) => Some(Err(e)), + for ances_manifest_path in find_root_iter(manifest_path, self.config) { + debug!("find_root - trying {}", ances_manifest_path.display()); + match *self.packages.load(&ances_manifest_path)?.workspace_config() { + WorkspaceConfig::Root(ref ances_root_config) => { + debug!("find_root - found a root checking exclusion"); + if !ances_root_config.is_excluded(manifest_path) { + debug!("find_root - found!"); + return Ok(Some(ances_manifest_path)); + } } - }) - .transpose() + WorkspaceConfig::Member { + root: Some(ref path_to_root), + } => { + debug!("find_root - found pointer"); + return Ok(Some(read_root_pointer(&ances_manifest_path, path_to_root))); + } + WorkspaceConfig::Member { .. } => {} + } + } + Ok(None) } /// After the root of a workspace has been located, probes for all members @@ -1834,32 +1827,26 @@ fn parse_manifest(manifest_path: &Path, config: &Config) -> CargoResult CargoResult> { - find_root_iter(manifest_path, config) - .find_map(|ances_manifest_path| { - let manifest = parse_manifest(&ances_manifest_path, config); - match manifest { - Ok(manifest) => match *manifest.workspace_config() { - WorkspaceConfig::Root(ref ances_root_config) => { - debug!("find_root - found a root checking exclusion"); - if !ances_root_config.is_excluded(manifest_path) { - debug!("find_root - found!"); - Some(Ok(ances_manifest_path)) - } else { - None - } - } - WorkspaceConfig::Member { - root: Some(ref path_to_root), - } => { - debug!("find_root - found pointer"); - Some(Ok(read_root_pointer(&ances_manifest_path, path_to_root))) - } - WorkspaceConfig::Member { .. } => None, - }, - Err(e) => Some(Err(e)), + for ances_manifest_path in find_root_iter(manifest_path, config) { + debug!("find_root - trying {}", ances_manifest_path.display()); + match *parse_manifest(&ances_manifest_path, config)?.workspace_config() { + WorkspaceConfig::Root(ref ances_root_config) => { + debug!("find_root - found a root checking exclusion"); + if !ances_root_config.is_excluded(manifest_path) { + debug!("find_root - found!"); + return Ok(Some(ances_manifest_path)); + } } - }) - .transpose() + WorkspaceConfig::Member { + root: Some(ref path_to_root), + } => { + debug!("find_root - found pointer"); + return Ok(Some(read_root_pointer(&ances_manifest_path, path_to_root))); + } + WorkspaceConfig::Member { .. } => {} + } + } + Ok(None) } fn read_root_pointer(member_manifest: &Path, root_link: &str) -> PathBuf { @@ -1877,7 +1864,6 @@ fn find_root_iter<'a>( config: &'a Config, ) -> impl Iterator + 'a { LookBehind::new(paths::ancestors(manifest_path, None).skip(2)) - .into_iter() .take_while(|path| !path.curr.ends_with("target/package")) // Don't walk across `CARGO_HOME` when we're looking for the // workspace root. Sometimes a package will be organized with