From 9d559b6dadc7e76575fa87b4422ea22ed1f185ae Mon Sep 17 00:00:00 2001 From: Scott Schafer <23045215+Muscraft@users.noreply.github.com> Date: Wed, 26 May 2021 10:03:46 -0500 Subject: [PATCH 01/10] Add additional fields to Cargo.toml for workspaces --- src/cargo/core/compiler/standard_lib.rs | 10 +- src/cargo/core/workspace.rs | 91 +++++++++++--- src/cargo/util/toml/mod.rs | 153 ++++++++++++++---------- 3 files changed, 173 insertions(+), 81 deletions(-) diff --git a/src/cargo/core/compiler/standard_lib.rs b/src/cargo/core/compiler/standard_lib.rs index 6b76a5681be..7461f152c3c 100644 --- a/src/cargo/core/compiler/standard_lib.rs +++ b/src/cargo/core/compiler/standard_lib.rs @@ -60,13 +60,9 @@ pub fn resolve_std<'cfg>( String::from("library/alloc"), String::from("library/test"), ]; - let ws_config = crate::core::WorkspaceConfig::Root(crate::core::WorkspaceRootConfig::new( - &src_path, - &Some(members), - /*default_members*/ &None, - /*exclude*/ &None, - /*custom_metadata*/ &None, - )); + let ws_config = crate::core::WorkspaceConfig::Root( + crate::core::WorkspaceRootConfig::from_members(&src_path, members), + ); let virtual_manifest = crate::core::VirtualManifest::new( /*replace*/ Vec::new(), patch, diff --git a/src/cargo/core/workspace.rs b/src/cargo/core/workspace.rs index 789a69dd144..ad828e84c32 100644 --- a/src/cargo/core/workspace.rs +++ b/src/cargo/core/workspace.rs @@ -21,7 +21,10 @@ 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::{ + map_deps, read_manifest, StringOrBool, TomlDependency, TomlProfiles, TomlWorkspace, + VecStringOrBool, +}; use crate::util::{config::ConfigRelativePath, Config, Filesystem, IntoUrl}; use cargo_util::paths; @@ -133,6 +136,23 @@ pub struct WorkspaceRootConfig { default_members: Option>, exclude: Vec, custom_metadata: Option, + + // Properties that can be inherited by members. + version: Option, + authors: Option>, + description: Option, + documentation: Option, + readme: Option, + homepage: Option, + repository: Option, + license: Option, + license_file: Option, + keywords: Option>, + categories: Option>, + publish: Option, + edition: Option, + dependencies: Option>, + badges: Option>>, } impl<'cfg> Workspace<'cfg> { @@ -1565,22 +1585,65 @@ impl MaybePackage { } impl WorkspaceRootConfig { - /// Creates a new Intermediate Workspace Root configuration. - pub fn new( - root_dir: &Path, - members: &Option>, - default_members: &Option>, - exclude: &Option>, - custom_metadata: &Option, - ) -> WorkspaceRootConfig { - WorkspaceRootConfig { + pub fn from_members(root_dir: &Path, members: Vec) -> WorkspaceRootConfig { + Self { root_dir: root_dir.to_path_buf(), - members: members.clone(), - default_members: default_members.clone(), - exclude: exclude.clone().unwrap_or_default(), - custom_metadata: custom_metadata.clone(), + members: Some(members), + default_members: None, + exclude: Vec::new(), + custom_metadata: None, + dependencies: None, + version: None, + authors: None, + description: None, + documentation: None, + readme: None, + homepage: None, + repository: None, + license: None, + license_file: None, + keywords: None, + categories: None, + publish: None, + edition: None, + badges: None, } } + /// Creates a new Intermediate Workspace Root configuration from a toml workspace. + pub fn from_toml_workspace( + root_dir: &Path, + config: &Config, + toml_workspace: &TomlWorkspace, + ) -> CargoResult { + let dependencies = map_deps( + config, + toml_workspace.dependencies.as_ref(), + |_d: &TomlDependency| true, + )?; + + Ok(Self { + root_dir: root_dir.to_path_buf(), + members: toml_workspace.members.clone(), + default_members: toml_workspace.default_members.clone(), + exclude: toml_workspace.exclude.clone().unwrap_or_default(), + custom_metadata: toml_workspace.metadata.clone(), + dependencies, + version: toml_workspace.version.clone(), + authors: toml_workspace.authors.clone(), + description: toml_workspace.description.clone(), + documentation: toml_workspace.documentation.clone(), + readme: toml_workspace.readme.clone(), + homepage: toml_workspace.homepage.clone(), + repository: toml_workspace.repository.clone(), + license: toml_workspace.license.clone(), + license_file: toml_workspace.license_file.clone(), + keywords: toml_workspace.keywords.clone(), + categories: toml_workspace.categories.clone(), + publish: toml_workspace.publish.clone(), + edition: toml_workspace.edition.clone(), + badges: toml_workspace.badges.clone(), + }) + } /// Checks the path against the `excluded` list. /// diff --git a/src/cargo/util/toml/mod.rs b/src/cargo/util/toml/mod.rs index dc0e61b7a71..abbabe9d9f7 100644 --- a/src/cargo/util/toml/mod.rs +++ b/src/cargo/util/toml/mod.rs @@ -99,6 +99,22 @@ fn do_read_manifest( }; 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)?; @@ -780,6 +796,48 @@ impl<'de> de::Deserialize<'de> for VecStringOrBool { } } +pub fn map_deps( + config: &Config, + deps: Option<&BTreeMap>, + filter: impl Fn(&TomlDependency) -> bool, +) -> CargoResult>> { + let deps = match deps { + Some(deps) => deps, + None => return Ok(None), + }; + let deps = deps + .iter() + .filter(|(_k, v)| filter(v)) + .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) => { + let mut d = d.clone(); + // Path dependencies become crates.io deps. + d.path.take(); + // Same with git dependencies. + d.git.take(); + d.branch.take(); + d.tag.take(); + d.rev.take(); + // registry specifications are elaborated to the index URL + if let Some(registry) = d.registry.take() { + let src = SourceId::alt_registry(config, ®istry)?; + d.registry_index = Some(src.url().to_string()); + } + Ok(TomlDependency::Detailed(d)) + } + TomlDependency::Simple(s) => Ok(TomlDependency::Detailed(DetailedTomlDependency { + version: Some(s.clone()), + ..Default::default() + })), + } +} + fn version_trim_whitespace<'de, D>(deserializer: D) -> Result where D: de::Deserializer<'de>, @@ -856,15 +914,33 @@ pub struct TomlProject { #[derive(Debug, Deserialize, Serialize)] pub struct TomlWorkspace { - members: Option>, + pub members: Option>, #[serde(rename = "default-members")] - default_members: Option>, - exclude: Option>, + pub default_members: Option>, + pub exclude: Option>, resolver: Option, + // Properties that can be inherited by members. + pub dependencies: Option>, + pub version: Option, + pub authors: Option>, + pub description: Option, + pub documentation: Option, + pub readme: Option, + pub homepage: Option, + pub repository: Option, + pub license: Option, + #[serde(rename = "license-file")] + pub license_file: Option, + pub keywords: Option>, + pub categories: Option>, + pub publish: Option, + pub edition: Option, + pub 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, + pub metadata: Option, } impl TomlProject { @@ -985,48 +1061,6 @@ impl TomlManifest { badges: self.badges.clone(), cargo_features: self.cargo_features.clone(), }); - - fn map_deps( - config: &Config, - deps: Option<&BTreeMap>, - filter: impl Fn(&TomlDependency) -> bool, - ) -> CargoResult>> { - let deps = match deps { - Some(deps) => deps, - None => return Ok(None), - }; - let deps = deps - .iter() - .filter(|(_k, v)| filter(v)) - .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) => { - let mut d = d.clone(); - // Path dependencies become crates.io deps. - d.path.take(); - // Same with git dependencies. - d.git.take(); - d.branch.take(); - d.tag.take(); - d.rev.take(); - // registry specifications are elaborated to the index URL - if let Some(registry) = d.registry.take() { - let src = SourceId::alt_registry(config, ®istry)?; - d.registry_index = Some(src.url().to_string()); - } - Ok(TomlDependency::Detailed(d)) - } - TomlDependency::Simple(s) => Ok(TomlDependency::Detailed(DetailedTomlDependency { - version: Some(s.clone()), - ..Default::default() - })), - } - } } pub fn to_real_manifest( @@ -1298,13 +1332,9 @@ impl TomlManifest { }; 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, - )), + (Some(toml_workspace), None) => WorkspaceConfig::Root( + WorkspaceRootConfig::from_toml_workspace(package_root, &config, toml_workspace)?, + ), (None, root) => WorkspaceConfig::Member { root: root.cloned(), }, @@ -1484,13 +1514,9 @@ 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 toml_workspace) => WorkspaceConfig::Root( + WorkspaceRootConfig::from_toml_workspace(root, config, toml_workspace)?, + ), None => { bail!("virtual manifests must be configured with [workspace]"); } @@ -1697,6 +1723,13 @@ impl TomlDependency

{ TomlDependency::Simple(..) => true, } } + + fn is_optional(&self) -> bool { + match self { + TomlDependency::Detailed(d) => d.optional.unwrap_or(false), + TomlDependency::Simple(..) => false, + } + } } impl DetailedTomlDependency

{ From 62ba1f13511e8b27460f3bfc1db3ceac20c8394c Mon Sep 17 00:00:00 2001 From: Scott Schafer <23045215+Muscraft@users.noreply.github.com> Date: Wed, 26 May 2021 10:03:53 -0500 Subject: [PATCH 02/10] Add tests for additional fields in Cargo.toml for workspaces --- tests/testsuite/deduplicate_workspace.rs | 99 ++++++++++++++++++++++++ tests/testsuite/main.rs | 1 + 2 files changed, 100 insertions(+) create mode 100644 tests/testsuite/deduplicate_workspace.rs diff --git a/tests/testsuite/deduplicate_workspace.rs b/tests/testsuite/deduplicate_workspace.rs new file mode 100644 index 00000000000..10f8c0f1ce9 --- /dev/null +++ b/tests/testsuite/deduplicate_workspace.rs @@ -0,0 +1,99 @@ +//! Tests for deduplicating Cargo.toml fields with { workspace = true } +use cargo_test_support::project; + +#[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/rusu", branch = "master" } + + [workspace.dependencies] + dep1 = "0.1" + "#, + ) + .file( + "bar/Cargo.toml", + r#" + [package] + name = "bar" + version = "0.2.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.2.0 ([CWD]/bar) +[FINISHED] dev [unoptimized + debuginfo] target(s) in [..] +", + ) + .run(); + + p.cargo("check").run(); + let lockfile = p.read_lockfile(); + assert!(!lockfile.contains("dep1")); +} + +#[cargo_test] +fn deny_optional_dependencies() { + let p = project() + .file( + "Cargo.toml", + r#" + [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.2.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 `[..]Cargo.toml` + +Caused by: + dep1 is optional, but workspace dependencies cannot be optional +", + ) + .run(); +} diff --git a/tests/testsuite/main.rs b/tests/testsuite/main.rs index 8cff78e7f33..5de58bcec16 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 0e7696c1e29e59862854d7d14d8587ce2a644b06 Mon Sep 17 00:00:00 2001 From: Scott Schafer <23045215+Muscraft@users.noreply.github.com> Date: Fri, 11 Jun 2021 11:49:05 -0500 Subject: [PATCH 03/10] Delayed parsing of manifests into packages until after a root config has been found and added IntermediateManifest --- src/cargo/core/manifest.rs | 66 +++++++++++++++++++++++++++- src/cargo/core/workspace.rs | 23 ++++++++-- src/cargo/ops/cargo_read_manifest.rs | 26 +++++++++-- src/cargo/util/toml/mod.rs | 55 +++++++++++++++++++---- 4 files changed, 152 insertions(+), 18 deletions(-) diff --git a/src/cargo/core/manifest.rs b/src/cargo/core/manifest.rs index 7f5e1f55ee1..c69d97bf1c3 100644 --- a/src/cargo/core/manifest.rs +++ b/src/cargo/core/manifest.rs @@ -13,7 +13,7 @@ use url::Url; use crate::core::compiler::{CompileKind, CrateType}; use crate::core::resolver::ResolveBehavior; -use crate::core::{Dependency, PackageId, PackageIdSpec, SourceId, Summary}; +use crate::core::{Dependency, Package, PackageId, PackageIdSpec, SourceId, Summary}; use crate::core::{Edition, Feature, Features, WorkspaceConfig}; use crate::util::errors::*; use crate::util::interning::InternedString; @@ -21,10 +21,19 @@ use crate::util::toml::{TomlManifest, TomlProfiles}; use crate::util::{short_hash, Config, Filesystem}; pub enum EitherManifest { - Real(Manifest), + Real(IntermediateManifest), Virtual(VirtualManifest), } +impl EitherManifest { + pub(crate) fn workspace_config(&self) -> &WorkspaceConfig { + match *self { + EitherManifest::Real(ref im) => im.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. @@ -968,3 +977,56 @@ impl Warnings { &self.0 } } + +/// This type is used to deserialize `Cargo.toml` files. +#[derive(Debug, Clone)] +pub struct IntermediateManifest { + workspace: WorkspaceConfig, + pkg_id: PackageId, + original: Rc, + warnings: Warnings, +} + +impl IntermediateManifest { + pub fn new( + workspace: WorkspaceConfig, + pkg_id: PackageId, + original: Rc, + ) -> IntermediateManifest { + IntermediateManifest { + workspace, + pkg_id, + original, + warnings: Warnings::new(), + } + } + + pub fn to_package( + &self, + manifest_path: &Path, + config: &Config, + ) -> CargoResult<(Package, Vec)> { + let (mut manifest, nested_paths) = TomlManifest::to_real_manifest( + &self.original, + self.pkg_id.source_id(), + manifest_path.parent().unwrap(), + config, + ) + .with_context(|| format!("failed to parse manifest at `{}`", manifest_path.display())) + .map_err(|err| ManifestError::new(err, manifest_path.into()))?; + + for warning in self.warnings.warnings() { + manifest.warnings_mut().add_warning(warning.message.clone()); + } + + Ok((Package::new(manifest, manifest_path), nested_paths)) + } + + pub fn warnings_mut(&mut self) -> &mut Warnings { + &mut self.warnings + } + + pub fn workspace_config(&self) -> &WorkspaceConfig { + &self.workspace + } +} diff --git a/src/cargo/core/workspace.rs b/src/cargo/core/workspace.rs index ad828e84c32..b2dbc022e44 100644 --- a/src/cargo/core/workspace.rs +++ b/src/cargo/core/workspace.rs @@ -178,6 +178,7 @@ impl<'cfg> Workspace<'cfg> { ws.custom_metadata = ws .load_workspace_config()? .and_then(|cfg| cfg.custom_metadata); + ws.load_current_manifest()?; ws.find_members()?; ws.set_resolve_behavior(); ws.validate()?; @@ -614,7 +615,7 @@ impl<'cfg> Workspace<'cfg> { } { - let current = self.packages.load(manifest_path)?; + let current = self.parse_manifest(manifest_path)?; match *current.workspace_config() { WorkspaceConfig::Root(_) => { debug!("find_root - is root {}", manifest_path.display()); @@ -635,7 +636,10 @@ impl<'cfg> Workspace<'cfg> { 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() { + match *self + .parse_manifest(&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) { @@ -666,6 +670,13 @@ impl<'cfg> Workspace<'cfg> { Ok(None) } + fn parse_manifest(&mut self, manifest_path: &Path) -> CargoResult { + let key = manifest_path.parent().unwrap(); + let source_id = SourceId::for_path(key)?; + let (manifest, _nested_paths) = read_manifest(manifest_path, source_id, self.config)?; + Ok(manifest) + } + /// After the root of a workspace has been located, probes for all members /// of a workspace. /// @@ -1537,6 +1548,10 @@ impl<'cfg> Workspace<'cfg> { ms } + fn load_current_manifest(&mut self) -> CargoResult<()> { + self.packages.load(&self.current_manifest)?; + Ok(()) + } } impl<'cfg> Packages<'cfg> { @@ -1566,7 +1581,9 @@ impl<'cfg> Packages<'cfg> { read_manifest(manifest_path, source_id, self.config)?; Ok(v.insert(match manifest { EitherManifest::Real(manifest) => { - MaybePackage::Package(Package::new(manifest, manifest_path)) + let (pkg, _nested_paths) = + manifest.to_package(manifest_path, self.config)?; + MaybePackage::Package(pkg) } EitherManifest::Virtual(vm) => MaybePackage::Virtual(vm), })) diff --git a/src/cargo/ops/cargo_read_manifest.rs b/src/cargo/ops/cargo_read_manifest.rs index d55208b88f1..5e2ba02f818 100644 --- a/src/cargo/ops/cargo_read_manifest.rs +++ b/src/cargo/ops/cargo_read_manifest.rs @@ -21,7 +21,7 @@ pub fn read_package( path.display(), source_id ); - let (manifest, nested) = read_manifest(path, source_id, config)?; + let (manifest, _nested) = read_manifest(path, source_id, config)?; let manifest = match manifest { EitherManifest::Real(manifest) => manifest, EitherManifest::Virtual(..) => anyhow::bail!( @@ -31,7 +31,7 @@ pub fn read_package( ), }; - Ok((Package::new(manifest, path), nested)) + Ok(manifest.to_package(path, config)?) } pub fn read_packages( @@ -151,7 +151,7 @@ fn read_nested_packages( let manifest_path = find_project_manifest_exact(path, "Cargo.toml")?; - let (manifest, nested) = match read_manifest(&manifest_path, source_id, config) { + let (manifest, _nested) = match read_manifest(&manifest_path, source_id, config) { Err(err) => { // Ignore malformed manifests found on git repositories // @@ -174,7 +174,25 @@ fn read_nested_packages( EitherManifest::Real(manifest) => manifest, EitherManifest::Virtual(..) => return Ok(()), }; - let pkg = Package::new(manifest, &manifest_path); + + let (pkg, nested) = match manifest.to_package(&manifest_path, config) { + Err(err) => { + // Ignore malformed manifests found on git repositories + // + // git source try to find and read all manifests from the repository + // but since it's not possible to exclude folders from this search + // it's safer to ignore malformed manifests to avoid + // + // TODO: Add a way to exclude folders? + info!( + "skipping malformed package found at `{}`", + path.to_string_lossy() + ); + errors.push(err.into()); + return Ok(()); + } + Ok(tuple) => tuple, + }; let pkg_id = pkg.package_id(); use std::collections::hash_map::Entry; diff --git a/src/cargo/util/toml/mod.rs b/src/cargo/util/toml/mod.rs index abbabe9d9f7..236f9ca8b1d 100644 --- a/src/cargo/util/toml/mod.rs +++ b/src/cargo/util/toml/mod.rs @@ -17,7 +17,7 @@ use url::Url; use crate::core::compiler::{CompileKind, CompileTarget}; use crate::core::dependency::DepKind; -use crate::core::manifest::{ManifestMetadata, TargetSourcePath, Warnings}; +use crate::core::manifest::{IntermediateManifest, 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}; @@ -117,15 +117,8 @@ fn do_read_manifest( return if manifest.project.is_some() || manifest.package.is_some() { let (mut manifest, paths) = - TomlManifest::to_real_manifest(&manifest, source_id, package_root, config)?; + TomlManifest::to_intermediate(&manifest, source_id, package_root, config)?; add_unused(manifest.warnings_mut()); - if manifest.targets().iter().all(|t| t.is_custom_build()) { - bail!( - "no targets specified in the manifest\n\ - either src/lib.rs, src/main.rs, a [lib] section, or \ - [[bin]] section must be present" - ) - } Ok((EitherManifest::Real(manifest), paths)) } else { let (mut m, paths) = @@ -1063,6 +1056,42 @@ impl TomlManifest { }); } + pub fn to_intermediate( + me: &Rc, + source_id: SourceId, + package_root: &Path, + config: &Config, + ) -> CargoResult<(IntermediateManifest, Vec)> { + let project = me.project.as_ref().or_else(|| me.package.as_ref()); + let project = project.ok_or_else(|| anyhow!("no `package` section found"))?; + + let package_name = project.name.trim(); + if package_name.is_empty() { + bail!("package name cannot be an empty string") + } + + validate_package_name(package_name, "package name", "")?; + + let pkgid = project.to_package_id(source_id)?; + + let workspace_config = match (me.workspace.as_ref(), project.workspace.as_ref()) { + (Some(toml_workspace), None) => WorkspaceConfig::Root( + WorkspaceRootConfig::from_toml_workspace(package_root, &config, toml_workspace)?, + ), + (None, root) => WorkspaceConfig::Member { + root: root.cloned(), + }, + (Some(..), Some(..)) => bail!( + "cannot configure both `package.workspace` and \ + `[workspace]`, only one can be specified" + ), + }; + + let manifest = IntermediateManifest::new(workspace_config, pkgid, Rc::clone(me)); + + Ok((manifest, vec![])) + } + pub fn to_real_manifest( me: &Rc, source_id: SourceId, @@ -1427,6 +1456,14 @@ impl TomlManifest { manifest.feature_gate()?; + if manifest.targets().iter().all(|t| t.is_custom_build()) { + bail!( + "no targets specified in the manifest\n\ + either src/lib.rs, src/main.rs, a [lib] section, or \ + [[bin]] section must be present" + ) + } + Ok((manifest, nested_paths)) } From 36bce7f0feea7fc40127dee41acdd5f084c86ec5 Mon Sep 17 00:00:00 2001 From: Scott Schafer Date: Tue, 13 Jul 2021 14:53:40 -0500 Subject: [PATCH 04/10] - Added inheriting TomlProject fields - Added inheriting dependencies from Workspaces - Added tests for inheriting TomlProject fields and dependencies - Fixed tests as needed Signed-off-by: Scott Schafer <23045215+Muscraft@users.noreply.github.com> --- crates/cargo-test-support/src/lib.rs | 13 + src/cargo/core/manifest.rs | 18 +- src/cargo/core/mod.rs | 5 +- src/cargo/core/registry.rs | 15 +- src/cargo/core/source/source_id.rs | 10 +- src/cargo/core/workspace.rs | 371 ++++---- src/cargo/ops/cargo_generate_lockfile.rs | 7 +- src/cargo/ops/cargo_install.rs | 8 +- src/cargo/ops/cargo_package.rs | 73 +- src/cargo/ops/cargo_read_manifest.rs | 44 +- .../ops/common_for_install_and_uninstall.rs | 11 +- src/cargo/ops/registry.rs | 8 +- src/cargo/ops/resolve.rs | 7 +- src/cargo/ops/vendor.rs | 4 +- src/cargo/sources/config.rs | 10 +- src/cargo/sources/directory.rs | 10 +- src/cargo/sources/git/source.rs | 10 +- src/cargo/sources/path.rs | 24 +- src/cargo/sources/registry/mod.rs | 9 +- src/cargo/util/toml/mod.rs | 775 +++++++++++++--- tests/testsuite/bad_config.rs | 4 + tests/testsuite/build_script.rs | 6 +- tests/testsuite/deduplicate_workspace.rs | 827 +++++++++++++++++- tests/testsuite/features.rs | 8 +- tests/testsuite/metadata.rs | 2 +- tests/testsuite/package.rs | 49 +- tests/testsuite/path.rs | 28 +- tests/testsuite/publish.rs | 70 +- tests/testsuite/workspaces.rs | 12 +- 29 files changed, 2037 insertions(+), 401 deletions(-) diff --git a/crates/cargo-test-support/src/lib.rs b/crates/cargo-test-support/src/lib.rs index 17e01d14de1..33178d0c41f 100644 --- a/crates/cargo-test-support/src/lib.rs +++ b/crates/cargo-test-support/src/lib.rs @@ -997,6 +997,19 @@ pub fn basic_lib_manifest(name: &str) -> String { ) } +pub fn basic_workspace_manifest(name: &str, workspace: &str) -> String { + format!( + r#" + [package] + name = "{}" + version = "0.1.0" + authors = [] + workspace = "{}" + "#, + name, workspace + ) +} + pub fn path2url>(p: P) -> Url { Url::from_file_path(p).ok().unwrap() } diff --git a/src/cargo/core/manifest.rs b/src/cargo/core/manifest.rs index c69d97bf1c3..283abffac50 100644 --- a/src/cargo/core/manifest.rs +++ b/src/cargo/core/manifest.rs @@ -13,7 +13,9 @@ use url::Url; use crate::core::compiler::{CompileKind, CrateType}; use crate::core::resolver::ResolveBehavior; -use crate::core::{Dependency, Package, PackageId, PackageIdSpec, SourceId, Summary}; +use crate::core::{ + Dependency, InheritableFields, Package, PackageId, PackageIdSpec, SourceId, Summary, +}; use crate::core::{Edition, Feature, Features, WorkspaceConfig}; use crate::util::errors::*; use crate::util::interning::InternedString; @@ -982,7 +984,7 @@ impl Warnings { #[derive(Debug, Clone)] pub struct IntermediateManifest { workspace: WorkspaceConfig, - pkg_id: PackageId, + source_id: SourceId, original: Rc, warnings: Warnings, } @@ -990,12 +992,12 @@ pub struct IntermediateManifest { impl IntermediateManifest { pub fn new( workspace: WorkspaceConfig, - pkg_id: PackageId, + source_id: SourceId, original: Rc, ) -> IntermediateManifest { IntermediateManifest { workspace, - pkg_id, + source_id, original, warnings: Warnings::new(), } @@ -1005,12 +1007,14 @@ impl IntermediateManifest { &self, manifest_path: &Path, config: &Config, + inheritable_fields: &InheritableFields, ) -> CargoResult<(Package, Vec)> { let (mut manifest, nested_paths) = TomlManifest::to_real_manifest( &self.original, - self.pkg_id.source_id(), + self.source_id, manifest_path.parent().unwrap(), config, + inheritable_fields, ) .with_context(|| format!("failed to parse manifest at `{}`", manifest_path.display())) .map_err(|err| ManifestError::new(err, manifest_path.into()))?; @@ -1022,6 +1026,10 @@ impl IntermediateManifest { Ok((Package::new(manifest, manifest_path), nested_paths)) } + pub fn original(&self) -> &Rc { + &self.original + } + pub fn warnings_mut(&mut self) -> &mut Warnings { &mut self.warnings } diff --git a/src/cargo/core/mod.rs b/src/cargo/core/mod.rs index aec49b143bd..633b29907fa 100644 --- a/src/cargo/core/mod.rs +++ b/src/cargo/core/mod.rs @@ -10,7 +10,10 @@ 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::{ + find_workspace_root, InheritableFields, MaybePackage, Workspace, WorkspaceConfig, + WorkspaceRootConfig, +}; pub mod compiler; pub mod dependency; diff --git a/src/cargo/core/registry.rs b/src/cargo/core/registry.rs index 35d30eb680e..bb1a349b577 100644 --- a/src/cargo/core/registry.rs +++ b/src/cargo/core/registry.rs @@ -1,7 +1,7 @@ use std::collections::{HashMap, HashSet}; -use crate::core::PackageSet; use crate::core::{Dependency, PackageId, Source, SourceId, SourceMap, Summary}; +use crate::core::{InheritableFields, PackageSet}; use crate::sources::config::SourceConfigMap; use crate::util::errors::CargoResult; use crate::util::interning::InternedString; @@ -78,6 +78,7 @@ pub struct PackageRegistry<'cfg> { patches: HashMap>, patches_locked: bool, patches_available: HashMap>, + inheritable_fields: InheritableFields, } /// A map of all "locked packages" which is filled in when parsing a lock file @@ -123,7 +124,10 @@ pub struct LockedPatchDependency { } impl<'cfg> PackageRegistry<'cfg> { - pub fn new(config: &'cfg Config) -> CargoResult> { + pub fn new( + config: &'cfg Config, + inherit: InheritableFields, + ) -> CargoResult> { let source_config = SourceConfigMap::new(config)?; Ok(PackageRegistry { config, @@ -136,6 +140,7 @@ impl<'cfg> PackageRegistry<'cfg> { patches: HashMap::new(), patches_locked: false, patches_available: HashMap::new(), + inheritable_fields: inherit, }) } @@ -424,7 +429,11 @@ impl<'cfg> PackageRegistry<'cfg> { fn load(&mut self, source_id: SourceId, kind: Kind) -> CargoResult<()> { (|| { debug!("loading source {}", source_id); - let source = self.source_config.load(source_id, &self.yanked_whitelist)?; + let source = self.source_config.load( + source_id, + &self.yanked_whitelist, + &self.inheritable_fields, + )?; assert_eq!(source.source_id(), source_id); if kind == Kind::Override { diff --git a/src/cargo/core/source/source_id.rs b/src/cargo/core/source/source_id.rs index 4299722239d..b82b23bde26 100644 --- a/src/cargo/core/source/source_id.rs +++ b/src/cargo/core/source/source_id.rs @@ -1,4 +1,4 @@ -use crate::core::PackageId; +use crate::core::{InheritableFields, PackageId}; use crate::sources::DirectorySource; use crate::sources::{GitSource, PathSource, RegistrySource, CRATES_IO_INDEX}; use crate::util::{CanonicalUrl, CargoResult, Config, IntoUrl}; @@ -272,6 +272,7 @@ impl SourceId { self, config: &'a Config, yanked_whitelist: &HashSet, + inherit: &InheritableFields, ) -> CargoResult> { trace!("loading SourceId; {}", self); match self.inner.kind { @@ -281,7 +282,12 @@ impl SourceId { Ok(p) => p, Err(()) => panic!("path sources cannot be remote"), }; - Ok(Box::new(PathSource::new(&path, self, config))) + Ok(Box::new(PathSource::new( + &path, + self, + config, + inherit.clone(), + ))) } SourceKind::Registry => Ok(Box::new(RegistrySource::remote( self, diff --git a/src/cargo/core/workspace.rs b/src/cargo/core/workspace.rs index b2dbc022e44..d1ee5a67dc1 100644 --- a/src/cargo/core/workspace.rs +++ b/src/cargo/core/workspace.rs @@ -22,8 +22,7 @@ use crate::util::errors::{CargoResult, ManifestError}; use crate::util::interning::InternedString; use crate::util::lev_distance; use crate::util::toml::{ - map_deps, read_manifest, StringOrBool, TomlDependency, TomlProfiles, TomlWorkspace, - VecStringOrBool, + read_manifest, StringOrBool, TomlDependency, TomlProfiles, TomlWorkspace, VecStringOrBool, }; use crate::util::{config::ConfigRelativePath, Config, Filesystem, IntoUrl}; use cargo_util::paths; @@ -97,6 +96,9 @@ pub struct Workspace<'cfg> { /// Workspace-level custom metadata custom_metadata: Option, + + /// Fields that can be inherited by members of this workspace + inheritable_fields: InheritableFields, } // Separate structure for tracking loaded packages (to avoid loading anything @@ -136,23 +138,52 @@ pub struct WorkspaceRootConfig { default_members: Option>, exclude: Vec, custom_metadata: Option, + inheritable_fields: InheritableFields, +} + +impl WorkspaceRootConfig { + pub fn inheritable_fields(&self) -> &InheritableFields { + &self.inheritable_fields + } +} - // Properties that can be inherited by members. - version: Option, - authors: Option>, - description: Option, - documentation: Option, - readme: Option, - homepage: Option, - repository: Option, - license: Option, - license_file: Option, - keywords: Option>, - categories: Option>, - publish: Option, - edition: Option, - dependencies: Option>, - badges: Option>>, +/// A group of fields that are inheritable by members of the workspace +#[derive(Clone, Debug, Default)] +pub struct InheritableFields { + pub dependencies: Option>, + pub version: Option, + pub authors: Option>, + pub description: Option, + pub homepage: Option, + pub documentation: Option, + pub readme: Option, + pub keywords: Option>, + pub categories: Option>, + pub license: Option, + pub license_file: Option, + pub repository: Option, + pub publish: Option, + pub edition: Option, + pub badges: Option>>, + pub ws_path: Option, +} + +impl InheritableFields { + pub fn dependency(&self, name: &String) -> Option { + self.dependencies + .as_ref() + .map(|deps| deps.get(name)) + .flatten() + .cloned() + } + + pub fn badge(&self, name: &String) -> Option> { + self.badges + .as_ref() + .map(|badges| badges.get(name)) + .flatten() + .cloned() + } } impl<'cfg> Workspace<'cfg> { @@ -172,12 +203,13 @@ impl<'cfg> Workspace<'cfg> { manifest_path ) } else { - ws.root_manifest = ws.find_root(manifest_path)?; + ws.root_manifest = find_workspace_root(manifest_path, config)?; } - ws.custom_metadata = ws - .load_workspace_config()? - .and_then(|cfg| cfg.custom_metadata); + let mut cfg = ws.load_workspace_config()?; + ws.custom_metadata = cfg.as_mut().and_then(|c| c.custom_metadata.take()); + ws.inheritable_fields = cfg.map(|c| c.inheritable_fields).unwrap_or_default(); + ws.load_current_manifest()?; ws.find_members()?; ws.set_resolve_behavior(); @@ -204,6 +236,7 @@ impl<'cfg> Workspace<'cfg> { ignore_lock: false, resolve_behavior: ResolveBehavior::V1, custom_metadata: None, + inheritable_fields: Default::default(), } } @@ -240,8 +273,10 @@ impl<'cfg> Workspace<'cfg> { config: &'cfg Config, target_dir: Option, require_optional_deps: bool, + inherit: InheritableFields, ) -> CargoResult> { let mut ws = Workspace::new_default(package.manifest_path().to_path_buf(), config); + ws.inheritable_fields = inherit; ws.is_ephemeral = true; ws.require_optional_deps = require_optional_deps; let key = ws.current_manifest.parent().unwrap(); @@ -540,6 +575,11 @@ impl<'cfg> Workspace<'cfg> { }) } + /// Returns a reference to fields which members of this workspace can inherit + pub fn inheritable_fields(&self) -> &InheritableFields { + &self.inheritable_fields + } + /// Returns true if the package is a member of the workspace. pub fn is_member(&self, pkg: &Package) -> bool { self.member_ids.contains(&pkg.package_id()) @@ -578,7 +618,7 @@ impl<'cfg> Workspace<'cfg> { // If we didn't find a root, it must mean there is no [workspace] section, and thus no // metadata. if let Some(root_path) = &self.root_manifest { - let root_package = self.packages.load(root_path)?; + let root_package = self.packages.load(root_path, &self.inheritable_fields)?; match root_package.workspace_config() { WorkspaceConfig::Root(ref root_config) => { return Ok(Some(root_config.clone())); @@ -594,89 +634,6 @@ impl<'cfg> Workspace<'cfg> { Ok(None) } - /// Finds the root of a workspace for the crate whose manifest is located - /// at `manifest_path`. - /// - /// This will parse the `Cargo.toml` at `manifest_path` and then interpret - /// the workspace configuration, optionally walking up the filesystem - /// looking for other workspace roots. - /// - /// 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.parse_manifest(manifest_path)?; - match *current.workspace_config() { - WorkspaceConfig::Root(_) => { - debug!("find_root - is root {}", manifest_path.display()); - return Ok(Some(manifest_path.to_path_buf())); - } - WorkspaceConfig::Member { - root: Some(ref path_to_root), - } => return Ok(Some(read_root_pointer(manifest_path, path_to_root))), - WorkspaceConfig::Member { root: None } => {} - } - } - - 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 - .parse_manifest(&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)); - } - } - 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 { .. } => {} - } - } - - // 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) - } - - fn parse_manifest(&mut self, manifest_path: &Path) -> CargoResult { - let key = manifest_path.parent().unwrap(); - let source_id = SourceId::for_path(key)?; - let (manifest, _nested_paths) = read_manifest(manifest_path, source_id, self.config)?; - Ok(manifest) - } - /// After the root of a workspace has been located, probes for all members /// of a workspace. /// @@ -753,7 +710,6 @@ impl<'cfg> Workspace<'cfg> { } else { self.default_members.push(self.current_manifest.clone()) } - self.find_path_deps(&root_manifest_path, &root_manifest_path, false) } @@ -769,15 +725,17 @@ impl<'cfg> Workspace<'cfg> { } if is_path_dep && !manifest_path.parent().unwrap().starts_with(self.root()) - && self.find_root(&manifest_path)? != self.root_manifest + && find_workspace_root(&manifest_path, self.config)? != self.root_manifest { // If `manifest_path` is a path dependency outside of the workspace, // don't add it, or any of its dependencies, as a members. return Ok(()); } - if let WorkspaceConfig::Root(ref root_config) = - *self.packages.load(root_manifest)?.workspace_config() + if let WorkspaceConfig::Root(ref root_config) = *self + .packages + .load(root_manifest, &self.inheritable_fields)? + .workspace_config() { if root_config.is_excluded(&manifest_path) { return Ok(()); @@ -786,9 +744,11 @@ impl<'cfg> Workspace<'cfg> { debug!("find_members - {}", manifest_path.display()); self.members.push(manifest_path.clone()); - let candidates = { - let pkg = match *self.packages.load(&manifest_path)? { + let pkg = match *self + .packages + .load(&manifest_path, &self.inheritable_fields)? + { MaybePackage::Package(ref p) => p, MaybePackage::Virtual(_) => return Ok(()), }; @@ -911,7 +871,7 @@ impl<'cfg> Workspace<'cfg> { fn validate_members(&mut self) -> CargoResult<()> { for member in self.members.clone() { - let root = self.find_root(&member)?; + let root = find_workspace_root(&member, self.config)?; if root == self.root_manifest { continue; } @@ -1050,7 +1010,12 @@ impl<'cfg> Workspace<'cfg> { return Ok(p); } let source_id = SourceId::for_path(manifest_path.parent().unwrap())?; - let (package, _nested_paths) = ops::read_package(manifest_path, source_id, self.config)?; + let (package, _nested_paths) = ops::read_package( + manifest_path, + source_id, + self.config, + &self.inheritable_fields, + )?; loaded.insert(manifest_path.to_path_buf(), package.clone()); Ok(package) } @@ -1076,7 +1041,12 @@ impl<'cfg> Workspace<'cfg> { MaybePackage::Package(ref p) => p.clone(), MaybePackage::Virtual(_) => continue, }; - let mut src = PathSource::new(pkg.root(), pkg.package_id().source_id(), self.config); + let mut src = PathSource::new( + pkg.root(), + pkg.package_id().source_id(), + self.config, + self.inheritable_fields.clone(), + ); src.preload_with(pkg); registry.add_preloaded(Box::new(src)); } @@ -1549,7 +1519,8 @@ impl<'cfg> Workspace<'cfg> { ms } fn load_current_manifest(&mut self) -> CargoResult<()> { - self.packages.load(&self.current_manifest)?; + self.packages + .load(&self.current_manifest, &self.inheritable_fields)?; Ok(()) } } @@ -1571,7 +1542,11 @@ impl<'cfg> Packages<'cfg> { self.packages.get_mut(manifest_path.parent().unwrap()) } - fn load(&mut self, manifest_path: &Path) -> CargoResult<&MaybePackage> { + fn load( + &mut self, + manifest_path: &Path, + inheritable_fields: &InheritableFields, + ) -> CargoResult<&MaybePackage> { let key = manifest_path.parent().unwrap(); match self.packages.entry(key.to_path_buf()) { Entry::Occupied(e) => Ok(e.into_mut()), @@ -1581,8 +1556,16 @@ impl<'cfg> Packages<'cfg> { read_manifest(manifest_path, source_id, self.config)?; Ok(v.insert(match manifest { EitherManifest::Real(manifest) => { + // This checks if the manifest is also a workspace root so you can get + // the proper inheritable fields before making a package + let inherit = + if let WorkspaceConfig::Root(root) = manifest.workspace_config() { + root.inheritable_fields.clone() + } else { + inheritable_fields.clone() + }; let (pkg, _nested_paths) = - manifest.to_package(manifest_path, self.config)?; + manifest.to_package(manifest_path, self.config, &inherit)?; MaybePackage::Package(pkg) } EitherManifest::Virtual(vm) => MaybePackage::Virtual(vm), @@ -1609,57 +1592,38 @@ impl WorkspaceRootConfig { default_members: None, exclude: Vec::new(), custom_metadata: None, - dependencies: None, - version: None, - authors: None, - description: None, - documentation: None, - readme: None, - homepage: None, - repository: None, - license: None, - license_file: None, - keywords: None, - categories: None, - publish: None, - edition: None, - badges: None, + inheritable_fields: Default::default(), } } /// Creates a new Intermediate Workspace Root configuration from a toml workspace. - pub fn from_toml_workspace( - root_dir: &Path, - config: &Config, - toml_workspace: &TomlWorkspace, - ) -> CargoResult { - let dependencies = map_deps( - config, - toml_workspace.dependencies.as_ref(), - |_d: &TomlDependency| true, - )?; + pub fn from_toml_workspace(root_dir: &Path, tw: &TomlWorkspace) -> WorkspaceRootConfig { + let inheritable_fields = InheritableFields { + dependencies: tw.dependencies.clone(), + version: tw.version.clone(), + authors: tw.authors.clone(), + description: tw.description.clone(), + homepage: tw.homepage.clone(), + documentation: tw.documentation.clone(), + readme: tw.readme.clone(), + keywords: tw.keywords.clone(), + categories: tw.categories.clone(), + license: tw.license.clone(), + license_file: tw.license_file.clone(), + repository: tw.repository.clone(), + publish: tw.publish.clone(), + edition: tw.edition.clone(), + badges: tw.badges.clone(), + ws_path: Some(root_dir.to_path_buf()), + }; - Ok(Self { + WorkspaceRootConfig { root_dir: root_dir.to_path_buf(), - members: toml_workspace.members.clone(), - default_members: toml_workspace.default_members.clone(), - exclude: toml_workspace.exclude.clone().unwrap_or_default(), - custom_metadata: toml_workspace.metadata.clone(), - dependencies, - version: toml_workspace.version.clone(), - authors: toml_workspace.authors.clone(), - description: toml_workspace.description.clone(), - documentation: toml_workspace.documentation.clone(), - readme: toml_workspace.readme.clone(), - homepage: toml_workspace.homepage.clone(), - repository: toml_workspace.repository.clone(), - license: toml_workspace.license.clone(), - license_file: toml_workspace.license_file.clone(), - keywords: toml_workspace.keywords.clone(), - categories: toml_workspace.categories.clone(), - publish: toml_workspace.publish.clone(), - edition: toml_workspace.edition.clone(), - badges: toml_workspace.badges.clone(), - }) + members: tw.members.clone(), + default_members: tw.default_members.clone(), + exclude: tw.exclude.clone().unwrap_or_default(), + custom_metadata: tw.metadata.clone(), + inheritable_fields, + } } /// Checks the path against the `excluded` list. @@ -1725,3 +1689,82 @@ impl WorkspaceRootConfig { Ok(res) } } + +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) +} + +/// Finds the root of a workspace for the crate whose manifest is located +/// at `manifest_path`. +/// +/// This will parse the `Cargo.toml` at `manifest_path` and then interpret +/// the workspace configuration, optionally walking up the filesystem +/// looking for other workspace roots. +/// +/// Returns an error if `manifest_path` isn't actually a valid manifest or +/// if some other transient error happens. +pub fn find_workspace_root(manifest_path: &Path, config: &Config) -> 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) + } + + { + match *parse_manifest(manifest_path, config)?.workspace_config() { + WorkspaceConfig::Root(_) => { + debug!("find_root - is root {}", manifest_path.display()); + return Ok(Some(manifest_path.to_path_buf())); + } + WorkspaceConfig::Member { + root: Some(ref path_to_root), + } => return Ok(Some(read_root_pointer(manifest_path, path_to_root))), + WorkspaceConfig::Member { root: None } => {} + } + } + + 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 *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)); + } + } + 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 { .. } => {} + } + } + + // 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 config.home() == path { + break; + } + } + + Ok(None) +} diff --git a/src/cargo/ops/cargo_generate_lockfile.rs b/src/cargo/ops/cargo_generate_lockfile.rs index 181e25b128b..0798cc28e24 100644 --- a/src/cargo/ops/cargo_generate_lockfile.rs +++ b/src/cargo/ops/cargo_generate_lockfile.rs @@ -21,7 +21,7 @@ pub struct UpdateOptions<'a> { } pub fn generate_lockfile(ws: &Workspace<'_>) -> CargoResult<()> { - let mut registry = PackageRegistry::new(ws.config())?; + let mut registry = PackageRegistry::new(ws.config(), ws.inheritable_fields().clone())?; let mut resolve = ops::resolve_with_previous( &mut registry, ws, @@ -58,7 +58,8 @@ pub fn update_lockfile(ws: &Workspace<'_>, opts: &UpdateOptions<'_>) -> CargoRes // Precise option specified, so calculate a previous_resolve required // by precise package update later. Some(_) => { - let mut registry = PackageRegistry::new(opts.config)?; + let mut registry = + PackageRegistry::new(opts.config, ws.inheritable_fields().clone())?; ops::resolve_with_previous( &mut registry, ws, @@ -73,7 +74,7 @@ pub fn update_lockfile(ws: &Workspace<'_>, opts: &UpdateOptions<'_>) -> CargoRes } } }; - let mut registry = PackageRegistry::new(opts.config)?; + let mut registry = PackageRegistry::new(opts.config, ws.inheritable_fields().clone())?; let mut to_avoid = HashSet::new(); if opts.to_update.is_empty() { diff --git a/src/cargo/ops/cargo_install.rs b/src/cargo/ops/cargo_install.rs index 8965b89480b..53d1101fb3e 100644 --- a/src/cargo/ops/cargo_install.rs +++ b/src/cargo/ops/cargo_install.rs @@ -4,7 +4,9 @@ use std::sync::Arc; use std::{env, fs}; use crate::core::compiler::{CompileKind, DefaultExecutor, Executor, Freshness, UnitOutput}; -use crate::core::{Dependency, Edition, Package, PackageId, Source, SourceId, Workspace}; +use crate::core::{ + Dependency, Edition, InheritableFields, Package, PackageId, Source, SourceId, Workspace, +}; use crate::ops::common_for_install_and_uninstall::*; use crate::sources::{GitSource, PathSource, SourceConfigMap}; use crate::util::errors::CargoResult; @@ -234,7 +236,7 @@ fn install_one( config, )? } else if let Some(dep) = dep { - let mut source = map.load(source_id, &HashSet::new())?; + let mut source = map.load(source_id, &HashSet::new(), &InheritableFields::default())?; if let Ok(Some(pkg)) = installed_exact_package(dep.clone(), &mut source, config, opts, root, &dst, force) { @@ -590,7 +592,7 @@ fn make_ws_rustc_target<'cfg>( let mut ws = if source_id.is_git() || source_id.is_path() { Workspace::new(pkg.manifest_path(), config)? } else { - Workspace::ephemeral(pkg, config, None, false)? + Workspace::ephemeral(pkg, config, None, false, InheritableFields::default())? }; ws.set_ignore_lock(config.lock_update_allowed()); ws.set_require_optional_deps(false); diff --git a/src/cargo/ops/cargo_package.rs b/src/cargo/ops/cargo_package.rs index e38b25f3397..4fb3a8e25fb 100644 --- a/src/cargo/ops/cargo_package.rs +++ b/src/cargo/ops/cargo_package.rs @@ -71,7 +71,12 @@ pub fn package(ws: &Workspace<'_>, opts: &PackageOpts<'_>) -> CargoResult

{ /// specifying only a version, eg. /// `package = { version = "" }` Detailed(DetailedTomlDependency

), + Workspace(TomlWorkspaceDependency), } impl<'de, P: Deserialize<'de>> de::Deserialize<'de> for TomlDependency

{ @@ -252,7 +255,35 @@ 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, + })) + } } } @@ -276,6 +307,42 @@ impl ResolveToPath for ConfigRelativePath { } } +#[derive(Deserialize, Serialize, Clone, Debug)] +#[serde(rename_all = "kebab-case")] +pub struct IntermediateDependency

{ + workspace: Option, + version: Option, + registry: Option, + /// The URL of the `registry` field. + /// This is an internal implementation detail. When Cargo creates a + /// package, it replaces `registry` with `registry-index` so that the + /// manifest contains the correct URL. All users won't have the same + /// registry names configured, so Cargo can't rely on just the name for + /// crates published by other users. + registry_index: Option, + // `path` is relative to the file it appears in. If that's a `Cargo.toml`, it'll be relative to + // that TOML file, and if it's a `.cargo/config` file, it'll be relative to that file. + 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, +} + +#[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

{ @@ -351,7 +418,8 @@ pub struct TomlManifest { replace: Option>, patch: Option>>, workspace: Option, - badges: Option>>, + #[serde(deserialize_with = "deserialize_workspace_badges", default)] + badges: Option>>>, } #[derive(Deserialize, Serialize, Clone, Debug, Default)] @@ -828,17 +896,105 @@ fn map_dependency(config: &Config, dep: &TomlDependency) -> CargoResult unreachable!(), } } -fn version_trim_whitespace<'de, D>(deserializer: D) -> Result +#[derive(Deserialize, Serialize, Clone, Debug)] +#[serde(untagged)] +pub enum MaybeWorkspace { + Workspace(TomlWorkspaceField), + Defined(T), +} + +#[derive(Debug, Deserialize)] +#[serde(untagged)] +enum MaybeWorkspaceBadge { + Workspace(TomlWorkspaceField), + Defined(BTreeMap>), +} + +/// This exists only to provide a nicer error message. +fn deserialize_workspace_badges<'de, D>( + deserializer: D, +) -> Result>>>, D::Error> +where + D: de::Deserializer<'de>, +{ + match Option::deserialize(deserializer) { + Ok(None) => Ok(None), + Ok(Some(MaybeWorkspaceBadge::Defined(badges))) => Ok(Some(MaybeWorkspace::Defined(badges))), + Ok(Some(MaybeWorkspaceBadge::Workspace(ws))) if ws.workspace => { + Ok(Some(MaybeWorkspace::Workspace(TomlWorkspaceField { + workspace: true, + }))) + } + Ok(Some(MaybeWorkspaceBadge::Workspace(_))) => { + Err(de::Error::custom("workspace cannot be false")) + } + + Err(_) => Err(de::Error::custom( + "expected a table of badges or { workspace = true }", + )), + } +} + +#[derive(Deserialize, Serialize, Clone, Debug)] +pub struct TomlWorkspaceField { + workspace: bool, +} + +impl MaybeWorkspace { + fn from_option(value: &Option>) -> Option + where + T: Clone, + { + match value { + Some(MaybeWorkspace::Defined(value)) => Some(value.clone()), + _ => None, + } + } +} + +/// Parses an optional field, defaulting to the workspace's value. +fn ws_default( + value: Option>, + workspace: &InheritableFields, + f: F, + label: &str, +) -> CargoResult>> +where + T: std::fmt::Debug + Clone, + F: FnOnce(&InheritableFields) -> &Option, +{ + match (value, workspace) { + (None, _) => Ok(None), + (Some(MaybeWorkspace::Defined(value)), _) => Ok(Some(MaybeWorkspace::Defined(value))), + (Some(MaybeWorkspace::Workspace(TomlWorkspaceField { workspace: true })), ws) => f(ws) + .clone() + .ok_or_else(|| { + anyhow!( + "error reading `{0}` from workspace root manifest's `[workspace.{0}]`", + label + ) + }) + .map(|value| Some(MaybeWorkspace::Defined(value))), + (Some(MaybeWorkspace::Workspace(TomlWorkspaceField { workspace: false })), _) => Err( + anyhow!("workspace cannot be false for key `package.{0}`", label), + ), + } +} + +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") @@ -848,11 +1004,30 @@ 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).and_then(|t| { + if t.workspace { + Ok(MaybeWorkspace::Workspace(TomlWorkspaceField { + workspace: true, + })) + } else { + Err(de::Error::custom("workspace cannot be false")) + } + }) } } - deserializer.deserialize_str(Visitor) + deserializer.deserialize_any(Visitor) } /// Represents the `package`/`project` sections of a `Cargo.toml`. @@ -864,12 +1039,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")] @@ -879,7 +1054,7 @@ pub struct TomlProject { links: Option, exclude: Option>, include: Option>, - publish: Option, + publish: Option>, workspace: Option, im_a_teapot: Option, autobins: Option, @@ -889,15 +1064,16 @@ 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>, + #[serde(rename = "license-file")] + license_file: Option>, + repository: Option>, resolver: Option, // Note that this field must come last due to the way toml serialization @@ -905,7 +1081,7 @@ pub struct TomlProject { metadata: Option, } -#[derive(Debug, Deserialize, Serialize)] +#[derive(Debug, Deserialize, Serialize, Clone)] pub struct TomlWorkspace { pub members: Option>, #[serde(rename = "default-members")] @@ -937,8 +1113,8 @@ pub struct TomlWorkspace { } 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: Version) -> CargoResult { + PackageId::new(self.name, version, source_id) } } @@ -957,6 +1133,7 @@ impl TomlManifest { /// Prepares the manifest for publishing. // - Path and git components of dependency specifications are removed. // - License path is updated to point within the package. + // No need to check for MaybeWorkspace since this should only be called on a package pub fn prepare_for_publish( &self, ws: &Workspace<'_>, @@ -971,20 +1148,20 @@ impl TomlManifest { .clone(); package.workspace = None; package.resolver = ws.resolve_behavior().to_manifest(); - if let Some(license_file) = &package.license_file { + if let Some(MaybeWorkspace::Defined(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( + package.license_file = Some(MaybeWorkspace::Defined( license_path .file_name() .unwrap() .to_str() .unwrap() .to_string(), - ); + )); } } let all = |_d: &TomlDependency| true; @@ -1060,7 +1237,6 @@ impl TomlManifest { me: &Rc, source_id: SourceId, package_root: &Path, - config: &Config, ) -> CargoResult<(IntermediateManifest, Vec)> { let project = me.project.as_ref().or_else(|| me.package.as_ref()); let project = project.ok_or_else(|| anyhow!("no `package` section found"))?; @@ -1072,11 +1248,9 @@ impl TomlManifest { validate_package_name(package_name, "package name", "")?; - let pkgid = project.to_package_id(source_id)?; - let workspace_config = match (me.workspace.as_ref(), project.workspace.as_ref()) { (Some(toml_workspace), None) => WorkspaceConfig::Root( - WorkspaceRootConfig::from_toml_workspace(package_root, &config, toml_workspace)?, + WorkspaceRootConfig::from_toml_workspace(package_root, toml_workspace), ), (None, root) => WorkspaceConfig::Member { root: root.cloned(), @@ -1087,7 +1261,7 @@ impl TomlManifest { ), }; - let manifest = IntermediateManifest::new(workspace_config, pkgid, Rc::clone(me)); + let manifest = IntermediateManifest::new(workspace_config, source_id, Rc::clone(me)); Ok((manifest, vec![])) } @@ -1097,6 +1271,7 @@ impl TomlManifest { source_id: SourceId, package_root: &Path, config: &Config, + inherit: &InheritableFields, ) -> CargoResult<(Manifest, Vec)> { let mut nested_paths = vec![]; let mut warnings = vec![]; @@ -1107,8 +1282,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"))?; + + TomlManifest::parse_toml_project(project, inherit)?; + + project.license_file = match (project.license_file.clone(), inherit.license_file.as_ref()) { + (None, _) => None, + (Some(MaybeWorkspace::Defined(defined)), _) => Some(MaybeWorkspace::Defined(defined)), + (Some(MaybeWorkspace::Workspace(_)), None) => { + bail!("error reading license-file: workspace root does not defined [workspace.license-file]"); + } + (Some(MaybeWorkspace::Workspace(_)), Some(ws_license_file)) => { + Some(MaybeWorkspace::Defined(join_relative_path( + inherit.ws_path.clone().unwrap().as_path(), + package_root, + ws_license_file, + )?)) + } + }; + + project.readme = match (project.readme.clone(), inherit.readme.as_ref()) { + (None, _) => match default_readme_from_package_root(package_root) { + None => None, + Some(readme) => Some(MaybeWorkspace::Defined(StringOrBool::String(readme))), + }, + (Some(MaybeWorkspace::Defined(defined)), _) => Some(MaybeWorkspace::Defined(defined)), + (Some(MaybeWorkspace::Workspace(_)), None) => { + bail!("error reading readme: workspace root does not defined [workspace.readme]") + } + (Some(MaybeWorkspace::Workspace(_)), Some(defined)) => match defined { + StringOrBool::String(file) => Some(MaybeWorkspace::Defined(StringOrBool::String( + join_relative_path( + inherit.ws_path.clone().unwrap().as_path(), + package_root, + file, + )?, + ))), + StringOrBool::Bool(val) => { + Some(MaybeWorkspace::Defined(StringOrBool::Bool(val.clone()))) + } + }, + }; let package_name = project.name.trim(); if package_name.is_empty() { @@ -1117,9 +1332,14 @@ impl TomlManifest { validate_package_name(package_name, "package name", "")?; - let pkgid = project.to_package_id(source_id)?; + let version = if let MaybeWorkspace::Defined(version) = project.version.clone() { + version + } else { + bail!("no version specified") + }; + let pkgid = project.to_package_id(source_id, version)?; - let edition = if let Some(ref edition) = project.edition { + let edition = if let Some(MaybeWorkspace::Defined(ref edition)) = project.edition { features .require(Feature::edition()) .with_context(|| "editions are unstable")?; @@ -1249,72 +1469,87 @@ impl TomlManifest { 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, - }; + 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(()), + fn process_dependencies( + cx: &mut Context<'_, '_>, + new_deps: Option<&BTreeMap>, + kind: Option, + inherit: &InheritableFields, + ) -> CargoResult>> { + let dependencies = match new_deps { + Some(dependencies) => dependencies, + None => return Ok(None), + }; + let mut proccessed_deps: BTreeMap = BTreeMap::new(); + for (n, v) in dependencies.iter() { + let toml_dep = match v.clone() { + TomlDependency::Simple(ref version) => TomlDependency::Simple(version.clone()), + TomlDependency::Detailed(mut details) => { + details.infer_path_version(cx, n)?; + TomlDependency::Detailed(details) + } + TomlDependency::Workspace(ws_dep_details) => { + let mut details = ws_dep_details.to_detailed_dependency(cx, inherit, n)?; + details.infer_path_version(cx, n)?; + TomlDependency::Detailed(details) + } }; - 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); - } - - Ok(()) + proccessed_deps.insert(n.clone(), toml_dep.clone()); + let dep = toml_dep.to_dependency(n, cx, kind)?; + validate_package_name(dep.name_in_toml().as_str(), "dependency name", "")?; + cx.deps.push(dep); } - // Collect the dependencies. - process_dependencies(&mut cx, me.dependencies.as_ref(), None)?; - let dev_deps = me - .dev_dependencies - .as_ref() - .or_else(|| me.dev_dependencies2.as_ref()); - process_dependencies(&mut cx, dev_deps, Some(DepKind::Development))?; - let build_deps = me + Ok(Some(proccessed_deps)) + } + + // Collect the dependencies. + let dependencies = process_dependencies(&mut cx, me.dependencies.as_ref(), None, inherit)?; + let dev_deps = me + .dev_dependencies + .as_ref() + .or_else(|| me.dev_dependencies2.as_ref()); + let dev_dependencies = + process_dependencies(&mut cx, dev_deps, Some(DepKind::Development), inherit)?; + let build_deps = me + .build_dependencies + .as_ref() + .or_else(|| me.build_dependencies2.as_ref()); + let build_dependencies = + process_dependencies(&mut cx, build_deps, Some(DepKind::Build), inherit)?; + + for (name, platform) in me.target.iter().flatten() { + cx.platform = { + let platform: Platform = name.parse()?; + platform.check_cfg_attributes(&mut cx.warnings); + Some(platform) + }; + process_dependencies(&mut cx, platform.dependencies.as_ref(), None, inherit)?; + 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(&mut cx.warnings); - Some(platform) - }; - process_dependencies(&mut cx, platform.dependencies.as_ref(), None)?; - let build_deps = platform - .build_dependencies - .as_ref() - .or_else(|| platform.build_dependencies2.as_ref()); - process_dependencies(&mut cx, build_deps, Some(DepKind::Build))?; - let dev_deps = platform - .dev_dependencies - .as_ref() - .or_else(|| platform.dev_dependencies2.as_ref()); - process_dependencies(&mut cx, dev_deps, Some(DepKind::Development))?; - } - - replace = me.replace(&mut cx)?; - patch = me.patch(&mut cx)?; + .or_else(|| platform.build_dependencies2.as_ref()); + process_dependencies(&mut cx, build_deps, Some(DepKind::Build), inherit)?; + let dev_deps = platform + .dev_dependencies + .as_ref() + .or_else(|| platform.dev_dependencies2.as_ref()); + process_dependencies(&mut cx, dev_deps, Some(DepKind::Development), inherit)?; } + replace = me.replace(&mut cx)?; + patch = me.patch(&mut cx)?; + { let mut names_sources = BTreeMap::new(); for dep in &deps { @@ -1345,24 +1580,31 @@ impl TomlManifest { let unstable = config.cli_unstable(); summary.unstable_gate(unstable.namespaced_features, unstable.weak_dep_features)?; + let badges = ws_default( + me.badges.clone(), + inherit, + |inherit| &inherit.badges, + "badges", + )?; + let metadata = ManifestMetadata { - description: project.description.clone(), - homepage: project.homepage.clone(), - documentation: project.documentation.clone(), + description: MaybeWorkspace::from_option(&project.description), + homepage: MaybeWorkspace::from_option(&project.homepage), + documentation: MaybeWorkspace::from_option(&project.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: MaybeWorkspace::from_option(&project.authors).unwrap_or_default(), + license: MaybeWorkspace::from_option(&project.license), + license_file: MaybeWorkspace::from_option(&project.license_file), + repository: MaybeWorkspace::from_option(&project.repository), + keywords: MaybeWorkspace::from_option(&project.keywords).unwrap_or_default(), + categories: MaybeWorkspace::from_option(&project.categories).unwrap_or_default(), + badges: MaybeWorkspace::from_option(&badges).unwrap_or_default(), links: project.links.clone(), }; let workspace_config = match (me.workspace.as_ref(), project.workspace.as_ref()) { (Some(toml_workspace), None) => WorkspaceConfig::Root( - WorkspaceRootConfig::from_toml_workspace(package_root, &config, toml_workspace)?, + WorkspaceRootConfig::from_toml_workspace(package_root, toml_workspace), ), (None, root) => WorkspaceConfig::Member { root: root.cloned(), @@ -1376,10 +1618,15 @@ impl TomlManifest { if let Some(profiles) = &profiles { profiles.validate(&features, &mut warnings)?; } - let publish = match project.publish { - Some(VecStringOrBool::VecString(ref vecstring)) => Some(vecstring.clone()), - Some(VecStringOrBool::Bool(false)) => Some(vec![]), - None | Some(VecStringOrBool::Bool(true)) => None, + + let publish = if let Some(MaybeWorkspace::Defined(publish)) = project.publish.clone() { + match publish { + VecStringOrBool::VecString(ref vecstring) => Some(vecstring.clone()), + VecStringOrBool::Bool(false) => Some(vec![]), + VecStringOrBool::Bool(true) => None, + } + } else { + None }; if summary.features().contains_key("default-features") { @@ -1416,6 +1663,29 @@ impl TomlManifest { .map(CompileKind::Target); let custom_metadata = project.metadata.clone(); + + let toml_manifest = TomlManifest { + cargo_features: me.cargo_features.clone(), + package: None, + project: Some(project.clone()), + profile: profiles.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_dependencies2: me.dev_dependencies2.clone(), + build_dependencies, + build_dependencies2: me.build_dependencies2.clone(), + features: me.features.clone(), + target: me.target.clone(), + replace: me.replace.clone(), + patch: me.patch.clone(), + workspace: me.workspace.clone(), + badges, + }; let mut manifest = Manifest::new( summary, default_kind, @@ -1436,7 +1706,7 @@ impl TomlManifest { rust_version, project.im_a_teapot, project.default_run.clone(), - Rc::clone(me), + Rc::new(toml_manifest), project.metabuild.clone().map(|sov| sov.0), resolve_behavior, ); @@ -1552,7 +1822,7 @@ impl TomlManifest { .transpose()?; let workspace_config = match me.workspace { Some(ref toml_workspace) => WorkspaceConfig::Root( - WorkspaceRootConfig::from_toml_workspace(root, config, toml_workspace)?, + WorkspaceRootConfig::from_toml_workspace(root, toml_workspace), ), None => { bail!("virtual manifests must be configured with [workspace]"); @@ -1571,6 +1841,80 @@ impl TomlManifest { )) } + fn parse_toml_project( + project: &mut TomlProject, + inherit: &InheritableFields, + ) -> CargoResult<()> { + project.version = ws_default( + Some(project.version.clone()), + inherit, + |inherit| &inherit.version, + "version", + )? + .ok_or_else(|| anyhow!("no version specified"))?; + project.edition = ws_default( + project.edition.clone(), + inherit, + |inherit| &inherit.edition, + "edition", + )?; + project.description = ws_default( + project.description.clone(), + inherit, + |inherit| &inherit.description, + "description", + )?; + project.homepage = ws_default( + project.homepage.clone(), + inherit, + |inherit| &inherit.homepage, + "homepage", + )?; + project.documentation = ws_default( + project.documentation.clone(), + inherit, + |inherit| &inherit.documentation, + "documentation", + )?; + project.authors = ws_default( + project.authors.clone(), + inherit, + |inherit| &inherit.authors, + "authors", + )?; + project.license = ws_default( + project.license.clone(), + inherit, + |inherit| &inherit.license, + "license", + )?; + project.repository = ws_default( + project.repository.clone(), + inherit, + |inherit| &inherit.repository, + "repository", + )?; + project.keywords = ws_default( + project.keywords.clone(), + inherit, + |inherit| &inherit.keywords, + "keywords", + )?; + project.categories = ws_default( + project.categories.clone(), + inherit, + |inherit| &inherit.categories, + "categories", + )?; + project.publish = ws_default( + project.publish.clone(), + inherit, + |inherit| &inherit.publish, + "publish", + )?; + Ok(()) + } + fn replace(&self, cx: &mut Context<'_, '_>) -> CargoResult> { if self.patch.is_some() && self.replace.is_some() { bail!("cannot specify both [replace] and [patch]"); @@ -1667,16 +2011,66 @@ impl TomlManifest { self.features.as_ref() } } +// This is taken from https://github.com/Manishearth/pathdiff/blob/30bb4010a7f420d8f367b0a0699ca42b813ce73d/src/lib.rs +fn join_relative_path( + root_path: &Path, + current_path: &Path, + relative_path: &str, +) -> CargoResult { + let path = root_path; + let base = current_path; + + let rel = if path.is_absolute() != base.is_absolute() { + if path.is_absolute() { + Some(PathBuf::from(path)) + } else { + None + } + } else { + let mut ita = path.components(); + let mut itb = base.components(); + let mut comps: Vec> = vec![]; + loop { + match (ita.next(), itb.next()) { + (None, None) => break, + (Some(a), None) => { + comps.push(a); + comps.extend(ita.by_ref()); + break; + } + (None, _) => comps.push(Component::ParentDir), + (Some(a), Some(b)) if comps.is_empty() && a == b => (), + (Some(a), Some(b)) if b == Component::CurDir => comps.push(a), + (Some(_), Some(b)) if b == Component::ParentDir => (), + (Some(a), Some(_)) => { + comps.push(Component::ParentDir); + for _ in itb { + comps.push(Component::ParentDir); + } + comps.push(a); + comps.extend(ita.by_ref()); + break; + } + } + } + Some(comps.iter().map(|c| c.as_os_str()).collect()) + }; + rel.unwrap() + .join(relative_path) + .into_os_string() + .into_string() + .map_err(|_| anyhow!("could not convert path into `String`")) +} /// 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), } } @@ -1751,6 +2145,7 @@ impl TomlDependency

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

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

{ match self { TomlDependency::Detailed(d) => d.optional.unwrap_or(false), TomlDependency::Simple(..) => false, + TomlDependency::Workspace(_) => unreachable!(), + } + } +} + +impl TomlWorkspaceDependency { + fn to_detailed_dependency( + &self, + cx: &mut Context<'_, '_>, + inherit: &InheritableFields, + name: &str, + ) -> CargoResult { + if let Some(deps) = &inherit.dependencies { + if let Some(dep) = deps.get(name) { + match dep { + TomlDependency::Simple(version) => Ok(DetailedTomlDependency { + optional: self.optional.clone(), + features: self.features.clone(), + version: Some(version.to_owned()), + ..Default::default() + }), + TomlDependency::Detailed(inherit_details) => { + let features = + match (self.features.clone(), inherit_details.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, + }; + let path = if let Some(p) = inherit_details.path.clone() { + join_relative_path( + inherit.ws_path.clone().unwrap().as_path(), + cx.root, + p.as_str(), + ) + .map_or(None, |path| Some(path)) + } else { + None + }; + Ok(DetailedTomlDependency { + version: inherit_details.version.clone(), + registry: inherit_details.registry.clone(), + registry_index: inherit_details.registry_index.clone(), + path, + git: inherit_details.git.clone(), + branch: inherit_details.branch.clone(), + tag: inherit_details.tag.clone(), + rev: inherit_details.rev.clone(), + features, + optional: self.optional.clone(), + default_features: inherit_details.default_features.clone(), + default_features2: inherit_details.default_features2.clone(), + package: inherit_details.package.clone(), + public: inherit_details.public.clone(), + }) + } + TomlDependency::Workspace(_) => unreachable!(), + } + } else { + bail!( + "failed to get dependency `{}`, not found in [workspace.dependencies]", + name + ) + } + } else { + bail!( + "failed to get dependency `{}`, [workspace.dependencies] does not exist", + name + ) } } } @@ -1978,6 +2448,79 @@ impl DetailedTomlDependency

{ } Ok(dep) } + + fn infer_path_version(&mut self, cx: &mut Context<'_, '_>, name: &str) -> CargoResult<()> { + if let (None, Some(p)) = (&self.version, &self.path) { + let base_path = &cx.root.join(p.resolve(cx.config)); + let (manifest, _) = + read_manifest(&base_path.join("Cargo.toml"), cx.source_id, cx.config) + .with_context(|| format!("failed to get dependency `{}`", name))?; + self.version = if let EitherManifest::Real(ref intermediate) = manifest { + let toml = intermediate.original(); + let package = toml + .package + .as_ref() + .or_else(|| toml.project.as_ref()) + .ok_or_else(|| anyhow!("no `package` section found"))?; + let v = match package.version { + MaybeWorkspace::Workspace(_) => { + let root_path = + find_workspace_root(&base_path.join("Cargo.toml"), cx.config)? + .expect("workspace was referenced, none found"); + let (root_man, _) = read_manifest(&root_path, cx.source_id, cx.config) + .with_context(|| format!("failed to get workspace for `{}`", name))?; + if let WorkspaceConfig::Root(ws_config) = root_man.workspace_config() { + let inherit = ws_config.inheritable_fields().clone(); + match (package.publish.as_ref(), inherit.publish) { + ( + Some(MaybeWorkspace::Defined(VecStringOrBool::Bool(false))), + _, + ) => None, + (_, Some(VecStringOrBool::Bool(false))) => None, + _ => Some(inherit.version.expect(&format!( + "workspace does not define version information required by {}", + name + ))), + } + } else { + bail!( + "workspace does not define version information required by {}", + name + ) + } + } + MaybeWorkspace::Defined(ref version) => match package.publish { + Some(MaybeWorkspace::Defined(VecStringOrBool::Bool(false))) => None, + Some(MaybeWorkspace::Workspace(_)) => { + let root_path = + find_workspace_root(&base_path.join("Cargo.toml"), cx.config)? + .expect("workspace was referenced, none found"); + let (root_man, _) = read_manifest(&root_path, cx.source_id, cx.config) + .with_context(|| format!("failed to get dependency `{}`", name))?; + if let WorkspaceConfig::Root(ws_config) = root_man.workspace_config() { + let inherit = ws_config.inheritable_fields().clone(); + match inherit.publish { + Some(VecStringOrBool::Bool(false)) => None, + _ => Some(version.clone()), + } + } else { + bail!( + "workspace does not define version information required by {}", + name + ) + } + } + _ => Some(version.clone()), + }, + }; + v.map(|ver| ver.to_string()) + } else { + None + }; + } + + Ok(()) + } } #[derive(Default, Serialize, Deserialize, Debug, Clone)] @@ -2029,7 +2572,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/tests/testsuite/bad_config.rs b/tests/testsuite/bad_config.rs index be242bff22b..b829747ab4f 100644 --- a/tests/testsuite/bad_config.rs +++ b/tests/testsuite/bad_config.rs @@ -1104,6 +1104,8 @@ fn both_git_and_path_specified() { "#, ) .file("src/lib.rs", "") + .file("bar/Cargo.toml", &basic_manifest("bar", "0.0.1")) + .file("bar/lib.rs", "") .build(); foo.cargo("build -v") @@ -1174,6 +1176,8 @@ fn ignored_git_revision() { "#, ) .file("src/lib.rs", "") + .file("bar/Cargo.toml", &basic_manifest("bar", "0.0.1")) + .file("bar/lib.rs", "") .build(); foo.cargo("build -v") diff --git a/tests/testsuite/build_script.rs b/tests/testsuite/build_script.rs index e9a24a32683..a1f74486681 100644 --- a/tests/testsuite/build_script.rs +++ b/tests/testsuite/build_script.rs @@ -769,7 +769,7 @@ fn links_duplicates() { .with_stderr("\ error: failed to select a version for `a-sys`. ... required by package `foo v0.5.0 ([..])` -versions that meet the requirements `*` are: 0.5.0 +versions that meet the requirements `^0.5.0` are: 0.5.0 the package `a-sys` links to the native library `a`, but it conflicts with a previous package which links to `a` as well: package `foo v0.5.0 ([..])` @@ -889,7 +889,7 @@ fn links_duplicates_deep_dependency() { error: failed to select a version for `a-sys`. ... required by package `a v0.5.0 ([..])` ... which is depended on by `foo v0.5.0 ([..])` -versions that meet the requirements `*` are: 0.5.0 +versions that meet the requirements `^0.5.0` are: 0.5.0 the package `a-sys` links to the native library `a`, but it conflicts with a previous package which links to `a` as well: package `foo v0.5.0 ([..])` @@ -4048,7 +4048,7 @@ fn links_duplicates_with_cycle() { .with_stderr("\ error: failed to select a version for `a`. ... required by package `foo v0.5.0 ([..])` -versions that meet the requirements `*` are: 0.5.0 +versions that meet the requirements `^0.5.0` are: 0.5.0 the package `a` links to the native library `a`, but it conflicts with a previous package which links to `a` as well: package `foo v0.5.0 ([..])` diff --git a/tests/testsuite/deduplicate_workspace.rs b/tests/testsuite/deduplicate_workspace.rs index 10f8c0f1ce9..c38945abcef 100644 --- a/tests/testsuite/deduplicate_workspace.rs +++ b/tests/testsuite/deduplicate_workspace.rs @@ -1,5 +1,9 @@ //! Tests for deduplicating Cargo.toml fields with { workspace = true } -use cargo_test_support::project; +use cargo_test_support::registry::{Dependency, Package}; +use cargo_test_support::{ + basic_lib_manifest, basic_manifest, basic_workspace_manifest, git, path2url, paths, project, + publish, registry, +}; #[cargo_test] fn permit_additional_workspace_fields() { @@ -17,36 +21,307 @@ fn permit_additional_workspace_fields() { homepage = "https://www.rust-lang.org" repository = "https://github.com/example/example" license = "MIT" - license-file = "./LICENSE" + license-file = "LICENSE" keywords = ["cli"] categories = ["development-tools"] publish = false edition = "2018" [workspace.badges] - gitlab = { repository = "https://gitlab.com/rust-lang/rusu", branch = "master" } + gitlab = { repository = "https://gitlab.com/rust-lang/rust", branch = "master" } [workspace.dependencies] - dep1 = "0.1" + dep = "0.1" "#, ) + .file("bar/Cargo.toml", &basic_workspace_manifest("bar", "..")) + .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#" + [workspace] + members = ["bar"] + + [workspace.dependencies] + dep1 = { version = "0.1", optional = true } + "#, + ) + .file("src/main.rs", "fn main() {}") + .file("bar/Cargo.toml", &basic_workspace_manifest("bar", "..")) + .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 +", + ) + .run(); +} + +#[cargo_test] +fn inherit_workspace_fields() { + registry::init(); + + let p = project().build(); + + let _ = git::repo(&paths::root().join("foo")) + .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 = 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 } + [package] name = "bar" + workspace = ".." + 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 } + "#, + ) + .file("LICENSE", "license") + .file("README.md", "README.md") + .file("bar/src/main.rs", "fn main() {}") + .build(); + + p.cargo("publish --token sekrit").cwd("bar").run(); + publish::validate_upload( + 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": "bar", + "readme": "README.md", + "readme_file": "../README.md", + "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", + "README.md", + "LICENSE", + ".cargo_vcs_info.json", + ], + ); +} + +#[cargo_test] +fn inherit_own_workspace_fields() { + registry::init(); + + let p = project().build(); + + let _ = git::repo(&paths::root().join("foo")) + .file( + "Cargo.toml", + r#" + 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").run(); + publish::validate_upload( + 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_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#" + [project] + workspace = ".." + name = "bar" version = "0.2.0" authors = [] - workspace = ".." + + [dependencies] + dep = { workspace = true } + + [build-dependencies] + dep-build = { workspace = true } + + [dev-dependencies] + dep-dev = { workspace = true } + "#, ) .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") - // Should not warn about unused fields. .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 [..] ", @@ -55,11 +330,79 @@ fn permit_additional_workspace_fields() { p.cargo("check").run(); let lockfile = p.read_lockfile(); - assert!(!lockfile.contains("dep1")); + assert!(lockfile.contains("dep")); + assert!(lockfile.contains("dep-dev")); + assert!(lockfile.contains("dep-build")); } #[cargo_test] -fn deny_optional_dependencies() { +fn inherited_detailed_dependencies() { + 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#" + [project] + workspace = ".." + name = "bar" + version = "0.2.0" + authors = [] + + [dependencies] + detailed = { workspace = true } + "#, + ) + .file("bar/src/main.rs", "fn main() {}") + .build(); + + let git_root = git_project.root(); + + p.cargo("build") + .with_stderr(&format!( + "\ +[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(); +} + +#[cargo_test] +fn inherit_path_dependencies() { let p = project() .file( "Cargo.toml", @@ -68,31 +411,489 @@ fn deny_optional_dependencies() { members = ["bar"] [workspace.dependencies] - dep1 = { version = "0.1", optional = true } + dep = { path = "dep" } "#, ) - .file("src/main.rs", "fn main() {}") .file( "bar/Cargo.toml", r#" - [package] + [project] + workspace = ".." + name = "bar" + version = "0.2.0" + authors = [] + + [dependencies] + dep = { workspace = true } + "#, + ) + .file("bar/src/main.rs", "fn main() {}") + .file("dep/Cargo.toml", &basic_manifest("dep", "0.9.0")) + .file("dep/src/lib.rs", "") + .build(); + + p.cargo("build") + .with_stderr( + "\ +[COMPILING] dep v0.9.0 ([CWD]/dep) +[COMPILING] bar v0.2.0 ([CWD]/bar) +[FINISHED] dev [unoptimized + debuginfo] target(s) in [..] +", + ) + .run(); + + let lockfile = p.read_lockfile(); + assert!(lockfile.contains("dep")); +} + +#[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#" + [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") + .with_stderr( + "\ +[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(); + + let lockfile = p.read_lockfile(); + assert!(lockfile.contains("dep")); +} + +#[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#" + [workspace] + members = ["bar"] + + [workspace.dependencies] + dep = { version = "0.1", features = ["fancy"] } + "#, + ) + .file( + "bar/Cargo.toml", + r#" + [project] + workspace = ".." + name = "bar" + version = "0.2.0" + authors = [] + + [dependencies] + dep = { workspace = true, features = ["dancy"] } + "#, + ) + .file("bar/src/main.rs", "fn main() {}") + .build(); + + p.cargo("build") + .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]/bar) +[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 inherited_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#" + [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") + .with_stderr( + "\ +[UPDATING] `[..]` index +[COMPILING] bar v0.2.0 ([CWD]/bar) +[FINISHED] dev [unoptimized + debuginfo] target(s) in [..] +", + ) + .run(); +} + +#[cargo_test] +fn path_dependency_has_workspace_version() { + let p = project() + .file( + "foo_root/Cargo.toml", + r#" + [workspace] + members = ["foo_member"] + version = "0.4.5" + "#, + ) + .file( + "foo_member/Cargo.toml", + r#" + [project] + workspace = "../foo_root" + name = "foo_member" + version = { workspace = true } + authors = [] + "#, + ) + .file("foo_member/src/main.rs", "fn main() {}") + .file( + "Cargo.toml", + r#" + [project] + name = "baz" + version = "0.1.2" + + [dependencies] + foo_member = { path = "./foo_member" } + "#, + ) + .file("src/main.rs", "fn main() {}") + .build(); + + Package::new("foo_member", "0.4.5").publish(); + p.cargo("package") + .with_stderr( + "\ +[WARNING] [..] +[..] +[PACKAGING] baz [..] +[UPDATING] `[..]` index +[VERIFYING] baz [..] +[DOWNLOADING] crates ... +[DOWNLOADED] foo_member v0.4.5 [..] +[COMPILING] foo_member v0.4.5 +[COMPILING] baz v0.1.2 [..] +[FINISHED] dev [unoptimized + debuginfo] target(s) in [..] +", + ) + .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#" + [package] + name = "bar" workspace = ".." + version = "1.2.3" + authors = ["rustaceans"] "#, ) .file("bar/src/main.rs", "fn main() {}") .build(); p.cargo("build") + .cwd("bar") .with_status(101) .with_stderr( "\ -[ERROR] failed to parse manifest at `[..]Cargo.toml` +[ERROR] failed to parse manifest at `[..]/foo/Cargo.toml` Caused by: - dep1 is optional, but workspace dependencies cannot be optional + [..] + +Caused by: + [..] +", + ) + .run(); +} + +#[cargo_test] +fn error_inherit_from_undefined_field() { + 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 = true } + "#, + ) + .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: + error reading `description` from workspace root manifest's `[workspace.description]` +", + ) + .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 error_no_root_workspace() { + registry::init(); + + let p = project().build(); + + let _ = git::repo(&paths::root().join("foo")) + .file( + "bar/Cargo.toml", + r#" + [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") + .cwd("bar") + .with_status(101) + .with_stderr( + "\ +[ERROR] root of a workspace inferred but wasn't a root: [..]/Cargo.toml +", + ) + .run(); +} + +#[cargo_test] +fn error_badges_wrapping() { + registry::init(); + + let p = project().build(); + + let _ = git::repo(&paths::root().join("foo")) + .file( + "Cargo.toml", + r#" + [package] + name = "bar" + version = "1.2.3" + authors = ["rustaceans"] + + [badges] + gitlab = "1.2.3" + "#, + ) + .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: + expected a table of badges or { workspace = true } for key `badges` +", + ) + .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#" + [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") + .cwd("bar") + .with_status(101) + .with_stderr( + "\ +[ERROR] failed to parse manifest at `[CWD]/Cargo.toml` + +Caused by: + failed to get dependency `foo`, [workspace.dependencies] does not exist ", ) .run(); diff --git a/tests/testsuite/features.rs b/tests/testsuite/features.rs index 49a61301bac..cd7b1861fe6 100644 --- a/tests/testsuite/features.rs +++ b/tests/testsuite/features.rs @@ -91,6 +91,8 @@ fn invalid3() { "#, ) .file("src/main.rs", "") + .file("foo/Cargo.toml", &basic_manifest("foo", "0.1.2")) + .file("foo/src/lib.rs", "") .build(); p.cargo("build") @@ -134,7 +136,7 @@ fn invalid4() { "\ error: failed to select a version for `bar`. ... required by package `foo v0.0.1 ([..])` -versions that meet the requirements `*` are: 0.0.1 +versions that meet the requirements `^0.0.1` are: 0.0.1 the package `foo` depends on `bar`, with features: `bar` but `bar` does not have these features. @@ -168,6 +170,8 @@ fn invalid5() { "#, ) .file("src/main.rs", "") + .file("bar/Cargo.toml", &basic_manifest("bar", "0.0.1")) + .file("bar/src/lib.rs", "") .build(); p.cargo("build") @@ -346,7 +350,7 @@ fn invalid10() { p.cargo("build").with_stderr("\ error: failed to select a version for `bar`. ... required by package `foo v0.0.1 ([..])` -versions that meet the requirements `*` are: 0.0.1 +versions that meet the requirements `^0.0.1` are: 0.0.1 the package `foo` depends on `bar`, with features: `baz` but `bar` does not have these features. It has a required dependency with that name, but only optional dependencies can be used as features. diff --git a/tests/testsuite/metadata.rs b/tests/testsuite/metadata.rs index 312603d984f..e6a6a7f8ad6 100644 --- a/tests/testsuite/metadata.rs +++ b/tests/testsuite/metadata.rs @@ -2027,7 +2027,7 @@ fn deps_with_bin_only() { { "name": "bdep", "source": null, - "req": "*", + "req": "^0.5.0", "kind": null, "rename": null, "optional": false, diff --git a/tests/testsuite/package.rs b/tests/testsuite/package.rs index b9d1e06cec1..8182bb1c9b6 100644 --- a/tests/testsuite/package.rs +++ b/tests/testsuite/package.rs @@ -261,6 +261,7 @@ in package source #[cargo_test] fn path_dependency_no_version() { + Package::new("bar", "0.1.0").publish(); let p = project() .file( "Cargo.toml", @@ -281,19 +282,29 @@ fn path_dependency_no_version() { .file("bar/src/lib.rs", "") .build(); - p.cargo("package") - .with_status(101) - .with_stderr( - "\ -[WARNING] manifest has no documentation, homepage or repository. -See https://doc.rust-lang.org/cargo/reference/manifest.html#package-metadata for more info. -[ERROR] all dependencies must have a version specified when packaging. -dependency `bar` does not specify a version\n\ -Note: The packaged dependency will use the version from crates.io, -the `path` specification will be removed from the dependency declaration. -", - ) - .run(); + p.cargo("package --no-verify").run(); + + let f = File::open(&p.root().join("target/package/foo-0.0.1.crate")).unwrap(); + let rewritten_toml = format!( + r#"{} +[package] +name = "foo" +version = "0.0.1" +authors = [] +description = "foo" +license = "MIT" +[dependencies.bar] +version = "0.1.0" +"#, + cargo::core::package::MANIFEST_PREAMBLE, + ); + + validate_crate_contents( + f, + "foo-0.0.1.crate", + &["Cargo.lock", "Cargo.toml", "Cargo.toml.orig", "src/main.rs"], + &[("Cargo.toml", &rewritten_toml)], + ); } #[cargo_test] @@ -1934,10 +1945,14 @@ src/main.rs .with_status(101) .with_stderr( "\ -[ERROR] all dependencies must have a version specified when packaging. -dependency `bar` does not specify a version -Note: The packaged dependency will use the version from crates.io, -the `path` specification will be removed from the dependency declaration. +[PACKAGING] foo v0.1.0 [..] +[UPDATING] `[..]` index +[ERROR] failed to prepare local package for uploading + +Caused by: + no matching package named `bar` found + location searched: registry `https://github.com/rust-lang/crates.io-index` + required by package `foo v0.1.0 [..]` ", ) .run(); diff --git a/tests/testsuite/path.rs b/tests/testsuite/path.rs index eac28090a0e..8d26154f0c0 100644 --- a/tests/testsuite/path.rs +++ b/tests/testsuite/path.rs @@ -511,16 +511,13 @@ fn error_message_for_missing_manifest() { .with_status(101) .with_stderr( "\ -[ERROR] failed to get `bar` as a dependency of package `foo v0.5.0 [..]` +[ERROR] failed to parse manifest at `[..]/foo/Cargo.toml` Caused by: - failed to load source for dependency `bar` + failed to get dependency `bar` Caused by: - Unable to update [CWD]/src/bar - -Caused by: - failed to read `[..]bar/Cargo.toml` + failed to read `[..]/bar/Cargo.toml` Caused by: [..] (os error [..]) @@ -1038,6 +1035,17 @@ fn deep_path_error() { "#, ) .file("b/src/lib.rs", "") + .file( + "c/Cargo.toml", + r#" + [package] + name = "c" + version = "0.1.0" + [dependencies] + d = {path="../d"} + "#, + ) + .file("c/src/lib.rs", "") .build(); p.cargo("check") @@ -1055,7 +1063,13 @@ Caused by: Unable to update [..]/foo/c Caused by: - failed to read `[..]/foo/c/Cargo.toml` + failed to parse manifest at `[..]/foo/c/Cargo.toml` + +Caused by: + failed to get dependency `d` + +Caused by: + failed to read `[..]/d/Cargo.toml` Caused by: [..] diff --git a/tests/testsuite/publish.rs b/tests/testsuite/publish.rs index f05ca6d2847..dfcbfb6204e 100644 --- a/tests/testsuite/publish.rs +++ b/tests/testsuite/publish.rs @@ -301,7 +301,7 @@ fn path_dependency_no_version() { r#" [project] name = "foo" - version = "0.0.1" + version = "0.8.9" authors = [] license = "MIT" description = "foo" @@ -315,18 +315,45 @@ fn path_dependency_no_version() { .file("bar/src/lib.rs", "") .build(); - p.cargo("publish --token sekrit") - .with_status(101) - .with_stderr( - "\ -[UPDATING] [..] index -[ERROR] all dependencies must have a version specified when publishing. -dependency `bar` does not specify a version -Note: The published dependency will use the version from crates.io, -the `path` specification will be removed from the dependency declaration. -", - ) - .run(); + Package::new("bar", "0.0.1").publish(); + p.cargo("publish --no-verify --token sekrit").run(); + + publish::validate_upload( + r#" + { + "authors": [], + "badges": {}, + "categories": [], + "deps": [ + { + "default_features": true, + "features": [], + "kind": "normal", + "name": "bar", + "optional": false, + "registry": "https://github.com/rust-lang/crates.io-index", + "target": null, + "version_req": "^0.0.1" + } + ], + "description": "foo", + "documentation": null, + "features": {}, + "homepage": null, + "keywords": [], + "license": "MIT", + "license_file": null, + "links": null, + "name": "foo", + "readme": null, + "readme_file": null, + "repository": null, + "vers": "0.8.9" + } + "#, + "foo-0.8.9.crate", + &["Cargo.lock", "Cargo.toml", "Cargo.toml.orig", "src/main.rs"], + ); } #[cargo_test] @@ -1296,7 +1323,18 @@ fn publish_dev_dep_no_version() { "authors": [], "badges": {}, "categories": [], - "deps": [], + "deps": [ + { + "default_features": true, + "features": [], + "kind": "dev", + "name": "bar", + "optional": false, + "registry": "https://github.com/rust-lang/crates.io-index", + "target": null, + "version_req": "^0.0.1" + } + ], "description": "foo", "documentation": "foo", "features": {}, @@ -1327,8 +1365,8 @@ homepage = "foo" documentation = "foo" license = "MIT" repository = "foo" - -[dev-dependencies] +[dev-dependencies.bar] +version = "0.0.1" "#, cargo::core::package::MANIFEST_PREAMBLE ), diff --git a/tests/testsuite/workspaces.rs b/tests/testsuite/workspaces.rs index 777c6d13af5..075009e4666 100644 --- a/tests/testsuite/workspaces.rs +++ b/tests/testsuite/workspaces.rs @@ -2306,13 +2306,10 @@ fn invalid_missing() { .with_status(101) .with_stderr( "\ -[ERROR] failed to get `x` as a dependency of package `foo v0.1.0 [..]` +[ERROR] failed to parse manifest at `[CWD]/Cargo.toml` Caused by: - failed to load source for dependency `x` - -Caused by: - Unable to update [..]/foo/x + failed to get dependency `x` Caused by: failed to read `[..]foo/x/Cargo.toml` @@ -2361,7 +2358,10 @@ fn member_dep_missing() { [ERROR] failed to load manifest for workspace member `[..]/bar` Caused by: - failed to load manifest for dependency `baz` + failed to parse manifest at `[..]/bar/Cargo.toml` + +Caused by: + failed to get dependency `baz` Caused by: failed to read `[..]foo/bar/baz/Cargo.toml` From 3327880c6e38fd2e78fff220ff2b91cddb168d50 Mon Sep 17 00:00:00 2001 From: Scott Schafer <23045215+Muscraft@users.noreply.github.com> Date: Tue, 13 Jul 2021 15:28:27 -0500 Subject: [PATCH 05/10] - Fixed one call to SourceConfigMap.load() after merging Signed-off-by: Scott Schafer <23045215+Muscraft@users.noreply.github.com> --- src/cargo/core/compiler/future_incompat.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/cargo/core/compiler/future_incompat.rs b/src/cargo/core/compiler/future_incompat.rs index d9638deddac..f6eeabef1de 100644 --- a/src/cargo/core/compiler/future_incompat.rs +++ b/src/cargo/core/compiler/future_incompat.rs @@ -246,7 +246,7 @@ fn render_suggestions( let mut sources: HashMap<_, _> = source_ids .into_iter() .filter_map(|sid| { - let source = map.load(sid, &HashSet::new()).ok()?; + let source = map.load(sid, &HashSet::new(), ws.inheritable_fields()).ok()?; Some((sid, source)) }) .collect(); From 43487e97e59589d2463e6d5334980e7357643c02 Mon Sep 17 00:00:00 2001 From: Scott Schafer <23045215+Muscraft@users.noreply.github.com> Date: Wed, 14 Jul 2021 16:31:16 -0500 Subject: [PATCH 06/10] Fixed CI problem with rust fmt Signed-off-by: Scott Schafer <23045215+Muscraft@users.noreply.github.com> --- src/cargo/core/compiler/future_incompat.rs | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/cargo/core/compiler/future_incompat.rs b/src/cargo/core/compiler/future_incompat.rs index f6eeabef1de..5aaa296d10d 100644 --- a/src/cargo/core/compiler/future_incompat.rs +++ b/src/cargo/core/compiler/future_incompat.rs @@ -246,7 +246,9 @@ fn render_suggestions( let mut sources: HashMap<_, _> = source_ids .into_iter() .filter_map(|sid| { - let source = map.load(sid, &HashSet::new(), ws.inheritable_fields()).ok()?; + let source = map + .load(sid, &HashSet::new(), ws.inheritable_fields()) + .ok()?; Some((sid, source)) }) .collect(); From 12302adb076002fc6df988ae7fb41f65f953e4a7 Mon Sep 17 00:00:00 2001 From: Scott Schafer <23045215+Muscraft@users.noreply.github.com> Date: Thu, 19 Aug 2021 11:18:02 -0500 Subject: [PATCH 07/10] Added tests to ensure published manifests do not contain workspace = true Signed-off-by: Scott Schafer <23045215+Muscraft@users.noreply.github.com> --- tests/testsuite/deduplicate_workspace.rs | 131 ++++++++++++++++++++++- 1 file changed, 129 insertions(+), 2 deletions(-) diff --git a/tests/testsuite/deduplicate_workspace.rs b/tests/testsuite/deduplicate_workspace.rs index c38945abcef..e484fabc323 100644 --- a/tests/testsuite/deduplicate_workspace.rs +++ b/tests/testsuite/deduplicate_workspace.rs @@ -144,7 +144,7 @@ fn inherit_workspace_fields() { .build(); p.cargo("publish --token sekrit").cwd("bar").run(); - publish::validate_upload( + publish::validate_upload_with_contents( r#" { "authors": ["Rustaceans"], @@ -178,6 +178,32 @@ fn inherit_workspace_fields() { "LICENSE", ".cargo_vcs_info.json", ], + &[( + "Cargo.toml", + &format!( + r#"{} +[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" +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 + ), + )], ); } @@ -235,7 +261,7 @@ fn inherit_own_workspace_fields() { .build(); p.cargo("publish --token sekrit").run(); - publish::validate_upload( + publish::validate_upload_with_contents( r#" { "authors": ["Rustaceans"], @@ -269,6 +295,32 @@ fn inherit_own_workspace_fields() { "LICENSE", ".cargo_vcs_info.json", ], + &[( + "Cargo.toml", + &format!( + r#"{} +[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 + ), + )], ); } @@ -333,6 +385,81 @@ fn inherit_dependencies() { assert!(lockfile.contains("dep")); assert!(lockfile.contains("dep-dev")); assert!(lockfile.contains("dep-build")); + p.cargo("publish --token sekrit").cwd("bar").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#"{} +[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] From f60aabf42a5b124ef0ef0a1c7526c1a6feb4b5ed Mon Sep 17 00:00:00 2001 From: Scott Schafer <23045215+Muscraft@users.noreply.github.com> Date: Mon, 11 Oct 2021 14:41:31 -0500 Subject: [PATCH 08/10] - Added cache for InheritableFields - Changed parsing of manifests, so they only get parsed once - Removed InheritableFields from places it does not belong - Updated tests Signed-off-by: Scott Schafer <23045215+Muscraft@users.noreply.github.com> --- src/cargo/core/compiler/future_incompat.rs | 4 +- src/cargo/core/manifest.rs | 2 +- src/cargo/core/registry.rs | 15 +- src/cargo/core/source/source_id.rs | 10 +- src/cargo/core/workspace.rs | 207 +++++++++++++--- src/cargo/ops/cargo_generate_lockfile.rs | 7 +- src/cargo/ops/cargo_install.rs | 2 +- src/cargo/ops/cargo_package.rs | 11 +- src/cargo/ops/cargo_read_manifest.rs | 43 +--- .../ops/common_for_install_and_uninstall.rs | 11 +- src/cargo/ops/registry.rs | 8 +- src/cargo/ops/resolve.rs | 7 +- src/cargo/ops/vendor.rs | 4 +- src/cargo/sources/config.rs | 10 +- src/cargo/sources/directory.rs | 9 +- src/cargo/sources/git/source.rs | 9 +- src/cargo/sources/path.rs | 24 +- src/cargo/sources/registry/mod.rs | 9 +- src/cargo/util/config/mod.rs | 5 +- src/cargo/util/toml/mod.rs | 232 +++++++++++++----- tests/testsuite/package.rs | 2 +- tests/testsuite/workspaces.rs | 4 +- 22 files changed, 391 insertions(+), 244 deletions(-) diff --git a/src/cargo/core/compiler/future_incompat.rs b/src/cargo/core/compiler/future_incompat.rs index 5aaa296d10d..d9638deddac 100644 --- a/src/cargo/core/compiler/future_incompat.rs +++ b/src/cargo/core/compiler/future_incompat.rs @@ -246,9 +246,7 @@ fn render_suggestions( let mut sources: HashMap<_, _> = source_ids .into_iter() .filter_map(|sid| { - let source = map - .load(sid, &HashSet::new(), ws.inheritable_fields()) - .ok()?; + let source = map.load(sid, &HashSet::new()).ok()?; Some((sid, source)) }) .collect(); diff --git a/src/cargo/core/manifest.rs b/src/cargo/core/manifest.rs index c6d2a795ccd..da8bd892504 100644 --- a/src/cargo/core/manifest.rs +++ b/src/cargo/core/manifest.rs @@ -1019,7 +1019,7 @@ impl IntermediateManifest { &self, manifest_path: &Path, config: &Config, - inheritable_fields: &InheritableFields, + inheritable_fields: Option<&InheritableFields>, ) -> CargoResult<(Package, Vec)> { let (mut manifest, nested_paths) = TomlManifest::to_real_manifest( &self.original, diff --git a/src/cargo/core/registry.rs b/src/cargo/core/registry.rs index bb1a349b577..35d30eb680e 100644 --- a/src/cargo/core/registry.rs +++ b/src/cargo/core/registry.rs @@ -1,7 +1,7 @@ use std::collections::{HashMap, HashSet}; +use crate::core::PackageSet; use crate::core::{Dependency, PackageId, Source, SourceId, SourceMap, Summary}; -use crate::core::{InheritableFields, PackageSet}; use crate::sources::config::SourceConfigMap; use crate::util::errors::CargoResult; use crate::util::interning::InternedString; @@ -78,7 +78,6 @@ pub struct PackageRegistry<'cfg> { patches: HashMap>, patches_locked: bool, patches_available: HashMap>, - inheritable_fields: InheritableFields, } /// A map of all "locked packages" which is filled in when parsing a lock file @@ -124,10 +123,7 @@ pub struct LockedPatchDependency { } impl<'cfg> PackageRegistry<'cfg> { - pub fn new( - config: &'cfg Config, - inherit: InheritableFields, - ) -> CargoResult> { + pub fn new(config: &'cfg Config) -> CargoResult> { let source_config = SourceConfigMap::new(config)?; Ok(PackageRegistry { config, @@ -140,7 +136,6 @@ impl<'cfg> PackageRegistry<'cfg> { patches: HashMap::new(), patches_locked: false, patches_available: HashMap::new(), - inheritable_fields: inherit, }) } @@ -429,11 +424,7 @@ impl<'cfg> PackageRegistry<'cfg> { fn load(&mut self, source_id: SourceId, kind: Kind) -> CargoResult<()> { (|| { debug!("loading source {}", source_id); - let source = self.source_config.load( - source_id, - &self.yanked_whitelist, - &self.inheritable_fields, - )?; + let source = self.source_config.load(source_id, &self.yanked_whitelist)?; assert_eq!(source.source_id(), source_id); if kind == Kind::Override { diff --git a/src/cargo/core/source/source_id.rs b/src/cargo/core/source/source_id.rs index eefc11a170e..085c78dae54 100644 --- a/src/cargo/core/source/source_id.rs +++ b/src/cargo/core/source/source_id.rs @@ -1,4 +1,4 @@ -use crate::core::{InheritableFields, PackageId}; +use crate::core::PackageId; use crate::sources::{DirectorySource, CRATES_IO_DOMAIN, CRATES_IO_INDEX, CRATES_IO_REGISTRY}; use crate::sources::{GitSource, PathSource, RegistrySource}; use crate::util::{CanonicalUrl, CargoResult, Config, IntoUrl}; @@ -286,7 +286,6 @@ impl SourceId { self, config: &'a Config, yanked_whitelist: &HashSet, - inherit: &InheritableFields, ) -> CargoResult> { trace!("loading SourceId; {}", self); match self.inner.kind { @@ -296,12 +295,7 @@ impl SourceId { Ok(p) => p, Err(()) => panic!("path sources cannot be remote"), }; - Ok(Box::new(PathSource::new( - &path, - self, - config, - inherit.clone(), - ))) + Ok(Box::new(PathSource::new(&path, self, config))) } SourceKind::Registry => Ok(Box::new(RegistrySource::remote( self, diff --git a/src/cargo/core/workspace.rs b/src/cargo/core/workspace.rs index d1ee5a67dc1..cd55c3ef3bd 100644 --- a/src/cargo/core/workspace.rs +++ b/src/cargo/core/workspace.rs @@ -203,14 +203,13 @@ impl<'cfg> Workspace<'cfg> { manifest_path ) } else { - ws.root_manifest = find_workspace_root(manifest_path, config)?; + ws.root_manifest = ws.find_root(manifest_path)?; } let mut cfg = ws.load_workspace_config()?; ws.custom_metadata = cfg.as_mut().and_then(|c| c.custom_metadata.take()); ws.inheritable_fields = cfg.map(|c| c.inheritable_fields).unwrap_or_default(); - ws.load_current_manifest()?; ws.find_members()?; ws.set_resolve_behavior(); ws.validate()?; @@ -618,7 +617,9 @@ impl<'cfg> Workspace<'cfg> { // If we didn't find a root, it must mean there is no [workspace] section, and thus no // metadata. if let Some(root_path) = &self.root_manifest { - let root_package = self.packages.load(root_path, &self.inheritable_fields)?; + let root_package = self + .packages + .load(root_path, None, &self.inheritable_fields)?; match root_package.workspace_config() { WorkspaceConfig::Root(ref root_config) => { return Ok(Some(root_config.clone())); @@ -634,6 +635,156 @@ impl<'cfg> Workspace<'cfg> { Ok(None) } + /// Finds the root of a workspace for the crate whose manifest is located + /// at `manifest_path`. + /// + /// This will parse the `Cargo.toml` at `manifest_path` and then interpret + /// the workspace configuration, optionally walking up the filesystem + /// looking for other workspace roots. + /// + /// 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 mut to_finalize_parse: Vec<(PathBuf, EitherManifest)> = Vec::new(); + + { + let current = self.parse_manifest(manifest_path)?; + match current.workspace_config().clone() { + WorkspaceConfig::Root(root) => { + debug!("find_root - is root {}", manifest_path.display()); + self.packages + .load(manifest_path, Some(current), &root.inheritable_fields)?; + self.config + .inheritable + .borrow_mut() + .insert(PathBuf::from(manifest_path), root.inheritable_fields); + return Ok(Some(manifest_path.to_path_buf())); + } + WorkspaceConfig::Member { + root: Some(ref path_to_root), + } => { + let path = read_root_pointer(manifest_path, path_to_root); + let root = self.parse_manifest(&path)?; + let inherit = if let WorkspaceConfig::Root(root) = root.workspace_config() { + root.inheritable_fields.clone() + } else { + bail!( + "root of a workspace inferred but wasn't a root: {}", + path.display() + ) + }; + self.packages.load(manifest_path, Some(current), &inherit)?; + self.packages.load(&path, Some(root), &inherit)?; + self.config + .inheritable + .borrow_mut() + .insert(path.clone(), inherit); + return Ok(Some(path)); + } + WorkspaceConfig::Member { root: None } => { + to_finalize_parse.push((PathBuf::from(manifest_path), current)) + } + } + } + + 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() { + let current = self.parse_manifest(&ances_manifest_path)?; + match current.workspace_config().clone() { + 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!"); + for (man_path, manifest) in to_finalize_parse { + self.packages.load( + &man_path, + Some(manifest), + &ances_root_config.inheritable_fields, + )?; + } + self.packages.load( + &ances_manifest_path, + Some(current), + &ances_root_config.inheritable_fields, + )?; + self.config.inheritable.borrow_mut().insert( + PathBuf::from(manifest_path), + ances_root_config.inheritable_fields.clone(), + ); + return Ok(Some(ances_manifest_path)); + } + } + WorkspaceConfig::Member { + root: Some(ref path_to_root), + } => { + debug!("find_root - found pointer"); + let path = read_root_pointer(&ances_manifest_path, path_to_root); + let root = self.parse_manifest(&path)?; + let inherit = if let WorkspaceConfig::Root(root) = root.workspace_config() { + root.inheritable_fields.clone() + } else { + bail!( + "root of a workspace inferred but wasn't a root: {}", + path.display() + ) + }; + for (man_path, manifest) in to_finalize_parse { + self.packages.load(&man_path, Some(manifest), &inherit)?; + } + self.packages.load(&path, Some(root), &inherit)?; + self.config + .inheritable + .borrow_mut() + .insert(path.clone(), inherit); + return Ok(Some(path)); + } + WorkspaceConfig::Member { .. } => { + to_finalize_parse.push((ances_manifest_path, current)) + } + } + } + + // 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; + } + } + + for (man_path, manifest) in to_finalize_parse { + self.packages + .load(&man_path, Some(manifest), &InheritableFields::default())?; + } + + Ok(None) + } + + fn parse_manifest(&mut self, manifest_path: &Path) -> CargoResult { + let key = manifest_path.parent().unwrap(); + let source_id = SourceId::for_path(key)?; + let (manifest, _nested_paths) = read_manifest(manifest_path, source_id, self.config)?; + Ok(manifest) + } + /// After the root of a workspace has been located, probes for all members /// of a workspace. /// @@ -725,7 +876,7 @@ impl<'cfg> Workspace<'cfg> { } if is_path_dep && !manifest_path.parent().unwrap().starts_with(self.root()) - && find_workspace_root(&manifest_path, self.config)? != self.root_manifest + && self.find_root(&manifest_path)? != self.root_manifest { // If `manifest_path` is a path dependency outside of the workspace, // don't add it, or any of its dependencies, as a members. @@ -734,7 +885,7 @@ impl<'cfg> Workspace<'cfg> { if let WorkspaceConfig::Root(ref root_config) = *self .packages - .load(root_manifest, &self.inheritable_fields)? + .load(root_manifest, None, &self.inheritable_fields)? .workspace_config() { if root_config.is_excluded(&manifest_path) { @@ -747,7 +898,7 @@ impl<'cfg> Workspace<'cfg> { let candidates = { let pkg = match *self .packages - .load(&manifest_path, &self.inheritable_fields)? + .load(&manifest_path, None, &self.inheritable_fields)? { MaybePackage::Package(ref p) => p, MaybePackage::Virtual(_) => return Ok(()), @@ -871,7 +1022,7 @@ impl<'cfg> Workspace<'cfg> { fn validate_members(&mut self) -> CargoResult<()> { for member in self.members.clone() { - let root = find_workspace_root(&member, self.config)?; + let root = self.find_root(&member)?; if root == self.root_manifest { continue; } @@ -1014,7 +1165,7 @@ impl<'cfg> Workspace<'cfg> { manifest_path, source_id, self.config, - &self.inheritable_fields, + Some(&self.inheritable_fields), )?; loaded.insert(manifest_path.to_path_buf(), package.clone()); Ok(package) @@ -1041,12 +1192,7 @@ impl<'cfg> Workspace<'cfg> { MaybePackage::Package(ref p) => p.clone(), MaybePackage::Virtual(_) => continue, }; - let mut src = PathSource::new( - pkg.root(), - pkg.package_id().source_id(), - self.config, - self.inheritable_fields.clone(), - ); + let mut src = PathSource::new(pkg.root(), pkg.package_id().source_id(), self.config); src.preload_with(pkg); registry.add_preloaded(Box::new(src)); } @@ -1518,11 +1664,6 @@ impl<'cfg> Workspace<'cfg> { ms } - fn load_current_manifest(&mut self) -> CargoResult<()> { - self.packages - .load(&self.current_manifest, &self.inheritable_fields)?; - Ok(()) - } } impl<'cfg> Packages<'cfg> { @@ -1545,15 +1686,22 @@ impl<'cfg> Packages<'cfg> { fn load( &mut self, manifest_path: &Path, + manifest: Option, inheritable_fields: &InheritableFields, ) -> CargoResult<&MaybePackage> { let key = manifest_path.parent().unwrap(); match self.packages.entry(key.to_path_buf()) { Entry::Occupied(e) => Ok(e.into_mut()), Entry::Vacant(v) => { - let source_id = SourceId::for_path(key)?; - let (manifest, _nested_paths) = - read_manifest(manifest_path, source_id, self.config)?; + let manifest = match manifest { + Some(either) => either, + None => { + let source_id = SourceId::for_path(key)?; + let (manifest, _nested_paths) = + read_manifest(manifest_path, source_id, self.config)?; + manifest + } + }; Ok(v.insert(match manifest { EitherManifest::Real(manifest) => { // This checks if the manifest is also a workspace root so you can get @@ -1565,7 +1713,7 @@ impl<'cfg> Packages<'cfg> { inheritable_fields.clone() }; let (pkg, _nested_paths) = - manifest.to_package(manifest_path, self.config, &inherit)?; + manifest.to_package(manifest_path, self.config, Some(&inherit))?; MaybePackage::Package(pkg) } EitherManifest::Virtual(vm) => MaybePackage::Virtual(vm), @@ -1717,19 +1865,6 @@ pub fn find_workspace_root(manifest_path: &Path, config: &Config) -> CargoResult paths::normalize_path(&path) } - { - match *parse_manifest(manifest_path, config)?.workspace_config() { - WorkspaceConfig::Root(_) => { - debug!("find_root - is root {}", manifest_path.display()); - return Ok(Some(manifest_path.to_path_buf())); - } - WorkspaceConfig::Member { - root: Some(ref path_to_root), - } => return Ok(Some(read_root_pointer(manifest_path, path_to_root))), - WorkspaceConfig::Member { root: None } => {} - } - } - for path in paths::ancestors(manifest_path, None).skip(2) { if path.ends_with("target/package") { break; diff --git a/src/cargo/ops/cargo_generate_lockfile.rs b/src/cargo/ops/cargo_generate_lockfile.rs index 0798cc28e24..181e25b128b 100644 --- a/src/cargo/ops/cargo_generate_lockfile.rs +++ b/src/cargo/ops/cargo_generate_lockfile.rs @@ -21,7 +21,7 @@ pub struct UpdateOptions<'a> { } pub fn generate_lockfile(ws: &Workspace<'_>) -> CargoResult<()> { - let mut registry = PackageRegistry::new(ws.config(), ws.inheritable_fields().clone())?; + let mut registry = PackageRegistry::new(ws.config())?; let mut resolve = ops::resolve_with_previous( &mut registry, ws, @@ -58,8 +58,7 @@ pub fn update_lockfile(ws: &Workspace<'_>, opts: &UpdateOptions<'_>) -> CargoRes // Precise option specified, so calculate a previous_resolve required // by precise package update later. Some(_) => { - let mut registry = - PackageRegistry::new(opts.config, ws.inheritable_fields().clone())?; + let mut registry = PackageRegistry::new(opts.config)?; ops::resolve_with_previous( &mut registry, ws, @@ -74,7 +73,7 @@ pub fn update_lockfile(ws: &Workspace<'_>, opts: &UpdateOptions<'_>) -> CargoRes } } }; - let mut registry = PackageRegistry::new(opts.config, ws.inheritable_fields().clone())?; + let mut registry = PackageRegistry::new(opts.config)?; let mut to_avoid = HashSet::new(); if opts.to_update.is_empty() { diff --git a/src/cargo/ops/cargo_install.rs b/src/cargo/ops/cargo_install.rs index d746721aeb0..64dbf5657fb 100644 --- a/src/cargo/ops/cargo_install.rs +++ b/src/cargo/ops/cargo_install.rs @@ -141,7 +141,7 @@ impl<'cfg, 'a> InstallablePackage<'cfg, 'a> { config, )? } else if let Some(dep) = dep { - let mut source = map.load(source_id, &HashSet::new(), &InheritableFields::default())?; + let mut source = map.load(source_id, &HashSet::new())?; if let Ok(Some(pkg)) = installed_exact_package( dep.clone(), &mut source, diff --git a/src/cargo/ops/cargo_package.rs b/src/cargo/ops/cargo_package.rs index a8b9c825a7b..bd33c68300b 100644 --- a/src/cargo/ops/cargo_package.rs +++ b/src/cargo/ops/cargo_package.rs @@ -68,12 +68,7 @@ pub fn package_one( opts: &PackageOpts<'_>, ) -> CargoResult> { let config = ws.config(); - let mut src = PathSource::new( - pkg.root(), - pkg.package_id().source_id(), - config, - ws.inheritable_fields().clone(), - ); + let mut src = PathSource::new(pkg.root(), pkg.package_id().source_id(), config); src.update()?; if opts.check_metadata { @@ -376,7 +371,7 @@ fn build_lock(ws: &Workspace<'_>, orig_pkg: &Package) -> CargoResult { source_id, package_root, config, - ws.inheritable_fields(), + Some(ws.inheritable_fields()), )?; let new_pkg = Package::new(manifest, orig_pkg.manifest_path()); @@ -776,7 +771,7 @@ fn run_verify( // Manufacture an ephemeral workspace to ensure that even if the top-level // package has a workspace we can still build our new crate. let id = SourceId::for_path(&dst)?; - let mut src = PathSource::new(&dst, id, ws.config(), ws.inheritable_fields().clone()); + let mut src = PathSource::new(&dst, id, ws.config()); let new_pkg = src.root_package()?; let pkg_fingerprint = hash_all(&dst)?; let ws = Workspace::ephemeral(new_pkg, config, None, true, ws.inheritable_fields().clone())?; diff --git a/src/cargo/ops/cargo_read_manifest.rs b/src/cargo/ops/cargo_read_manifest.rs index 1793f7c8015..3311834949e 100644 --- a/src/cargo/ops/cargo_read_manifest.rs +++ b/src/cargo/ops/cargo_read_manifest.rs @@ -3,9 +3,7 @@ use std::fs; use std::io; use std::path::{Path, PathBuf}; -use crate::core::{ - EitherManifest, InheritableFields, Package, PackageId, SourceId, WorkspaceConfig, -}; +use crate::core::{EitherManifest, InheritableFields, Package, PackageId, SourceId}; use crate::util::errors::CargoResult; use crate::util::important_paths::find_project_manifest_exact; use crate::util::toml::read_manifest; @@ -17,7 +15,7 @@ pub fn read_package( path: &Path, source_id: SourceId, config: &Config, - inherit: &InheritableFields, + inherit: Option<&InheritableFields>, ) -> CargoResult<(Package, Vec)> { trace!( "read_package; path={}; source-id={}", @@ -40,7 +38,6 @@ pub fn read_packages( path: &Path, source_id: SourceId, config: &Config, - inherit: &InheritableFields, ) -> CargoResult> { let mut all_packages = HashMap::new(); let mut visited = HashSet::::new(); @@ -83,7 +80,6 @@ pub fn read_packages( config, &mut visited, &mut errors, - inherit, )?; } Ok(true) @@ -148,7 +144,6 @@ fn read_nested_packages( config: &Config, visited: &mut HashSet, errors: &mut Vec, - inherit: &InheritableFields, ) -> CargoResult<()> { if !visited.insert(path.to_path_buf()) { return Ok(()); @@ -179,28 +174,7 @@ fn read_nested_packages( EitherManifest::Real(manifest) => manifest, EitherManifest::Virtual(..) => return Ok(()), }; - - let inheritable_fields = match manifest.workspace_config() { - WorkspaceConfig::Root(ws_root) => ws_root.inheritable_fields().clone(), - WorkspaceConfig::Member { - root: Some(ref path_to_root), - } => { - let path = Path::new(path_to_root); - let (root_manifest, _) = read_manifest( - &path.join("Cargo.toml"), - SourceId::for_path(path.parent().unwrap())?, - config, - ) - .unwrap(); - if let WorkspaceConfig::Root(ws_config) = root_manifest.workspace_config().clone() { - ws_config.inheritable_fields().clone() - } else { - inherit.clone() - } - } - WorkspaceConfig::Member { root: None } => inherit.clone(), - }; - let (pkg, nested) = match manifest.to_package(&manifest_path, config, &inheritable_fields) { + let (pkg, nested) = match manifest.to_package(&manifest_path, config, None) { Err(err) => { // Ignore malformed manifests found on git repositories // @@ -245,15 +219,8 @@ fn read_nested_packages( if !source_id.is_registry() { for p in nested.iter() { let path = paths::normalize_path(&path.join(p)); - let result = read_nested_packages( - &path, - all_packages, - source_id, - config, - visited, - errors, - inherit, - ); + let result = + read_nested_packages(&path, all_packages, source_id, config, visited, errors); // Ignore broken manifests found on git repositories. // // A well formed manifest might still fail to load due to reasons diff --git a/src/cargo/ops/common_for_install_and_uninstall.rs b/src/cargo/ops/common_for_install_and_uninstall.rs index d3cd4873b17..444e57cfddb 100644 --- a/src/cargo/ops/common_for_install_and_uninstall.rs +++ b/src/cargo/ops/common_for_install_and_uninstall.rs @@ -9,9 +9,7 @@ use anyhow::{bail, format_err, Context as _}; use serde::{Deserialize, Serialize}; use crate::core::compiler::Freshness; -use crate::core::{ - Dependency, FeatureValue, InheritableFields, Package, PackageId, Source, SourceId, -}; +use crate::core::{Dependency, FeatureValue, Package, PackageId, Source, SourceId}; use crate::ops::{self, CompileFilter, CompileOptions}; use crate::sources::PathSource; use crate::util::errors::CargoResult; @@ -517,12 +515,7 @@ pub fn path_source(source_id: SourceId, config: &Config) -> CargoResult SourceId::for_registry(&i.into_url()?), _ => { let map = SourceConfigMap::new(config)?; - let src = map.load( - SourceId::crates_io(config)?, - &HashSet::new(), - &InheritableFields::default(), - )?; + let src = map.load(SourceId::crates_io(config)?, &HashSet::new())?; Ok(src.replaced_source_id()) } } diff --git a/src/cargo/ops/resolve.rs b/src/cargo/ops/resolve.rs index 85ec6ff3870..c305bc3a353 100644 --- a/src/cargo/ops/resolve.rs +++ b/src/cargo/ops/resolve.rs @@ -63,7 +63,7 @@ version. This may also occur with an optional dependency that is not enabled."; /// This is a simple interface used by commands like `clean`, `fetch`, and /// `package`, which don't specify any options or features. pub fn resolve_ws<'a>(ws: &Workspace<'a>) -> CargoResult<(PackageSet<'a>, Resolve)> { - let mut registry = PackageRegistry::new(ws.config(), ws.inheritable_fields().clone())?; + let mut registry = PackageRegistry::new(ws.config())?; let resolve = resolve_with_registry(ws, &mut registry)?; let packages = get_resolved_packages(&resolve, registry)?; Ok((packages, resolve)) @@ -88,7 +88,7 @@ pub fn resolve_ws_with_opts<'cfg>( has_dev_units: HasDevUnits, force_all_targets: ForceAllTargets, ) -> CargoResult> { - let mut registry = PackageRegistry::new(ws.config(), ws.inheritable_fields().clone())?; + let mut registry = PackageRegistry::new(ws.config())?; let mut add_patches = true; let resolve = if ws.ignore_lock() { None @@ -495,8 +495,7 @@ pub fn add_overrides<'a>( for (path, definition) in paths { let id = SourceId::for_path(&path)?; - let mut source = - PathSource::new_recursive(&path, id, ws.config(), ws.inheritable_fields().clone()); + let mut source = PathSource::new_recursive(&path, id, ws.config()); source.update().with_context(|| { format!( "failed to update path override `{}` \ diff --git a/src/cargo/ops/vendor.rs b/src/cargo/ops/vendor.rs index b0b5ad98c97..adae834c243 100644 --- a/src/cargo/ops/vendor.rs +++ b/src/cargo/ops/vendor.rs @@ -1,5 +1,5 @@ use crate::core::shell::Verbosity; -use crate::core::{GitReference, InheritableFields, Workspace}; +use crate::core::{GitReference, Workspace}; use crate::ops; use crate::sources::path::PathSource; use crate::sources::CRATES_IO_REGISTRY; @@ -212,7 +212,7 @@ fn sync( )?; let _ = fs::remove_dir_all(&dst); - let pathsource = PathSource::new(src, id.source_id(), config, InheritableFields::default()); + let pathsource = PathSource::new(src, id.source_id(), config); let paths = pathsource.list_files(pkg)?; let mut map = BTreeMap::new(); cp_sources(src, &paths, &dst, &mut map, &mut tmp_buf) diff --git a/src/cargo/sources/config.rs b/src/cargo/sources/config.rs index f39c9d503c3..0e2b24efb64 100644 --- a/src/cargo/sources/config.rs +++ b/src/cargo/sources/config.rs @@ -4,7 +4,7 @@ //! structure usable by Cargo itself. Currently this is primarily used to map //! sources to one another via the `replace-with` key in `.cargo/config`. -use crate::core::{GitReference, InheritableFields, PackageId, Source, SourceId}; +use crate::core::{GitReference, PackageId, Source, SourceId}; use crate::sources::{ReplacedSource, CRATES_IO_REGISTRY}; use crate::util::config::{self, ConfigRelativePath, OptValue}; use crate::util::errors::CargoResult; @@ -103,13 +103,12 @@ impl<'cfg> SourceConfigMap<'cfg> { &self, id: SourceId, yanked_whitelist: &HashSet, - inherit: &InheritableFields, ) -> CargoResult> { debug!("loading: {}", id); let mut name = match self.id2name.get(&id) { Some(name) => name, - None => return id.load(self.config, yanked_whitelist, inherit), + None => return id.load(self.config, yanked_whitelist), }; let mut cfg_loc = ""; let orig_name = name; @@ -131,7 +130,7 @@ impl<'cfg> SourceConfigMap<'cfg> { name = s; cfg_loc = c; } - None if id == cfg.id => return id.load(self.config, yanked_whitelist, inherit), + None if id == cfg.id => return id.load(self.config, yanked_whitelist), None => { new_id = cfg.id.with_precise(id.precise().map(|s| s.to_string())); break; @@ -155,9 +154,8 @@ impl<'cfg> SourceConfigMap<'cfg> { .iter() .map(|p| p.map_source(id, new_id)) .collect(), - inherit, )?; - let old_src = id.load(self.config, yanked_whitelist, inherit)?; + let old_src = id.load(self.config, yanked_whitelist)?; if !new_src.supports_checksums() && old_src.supports_checksums() { bail!( "\ diff --git a/src/cargo/sources/directory.rs b/src/cargo/sources/directory.rs index d753644e3f6..a7cbb89ae0e 100644 --- a/src/cargo/sources/directory.rs +++ b/src/cargo/sources/directory.rs @@ -3,7 +3,7 @@ use std::fmt::{self, Debug, Formatter}; use std::path::{Path, PathBuf}; use crate::core::source::MaybePackage; -use crate::core::{Dependency, InheritableFields, Package, PackageId, Source, SourceId, Summary}; +use crate::core::{Dependency, Package, PackageId, Source, SourceId, Summary}; use crate::sources::PathSource; use crate::util::errors::CargoResult; use crate::util::Config; @@ -112,12 +112,7 @@ impl<'cfg> Source for DirectorySource<'cfg> { if !path.join("Cargo.toml").exists() { continue; } - let mut src = PathSource::new( - &path, - self.source_id, - self.config, - InheritableFields::default(), - ); + let mut src = PathSource::new(&path, self.source_id, self.config); src.update()?; let mut pkg = src.root_package()?; diff --git a/src/cargo/sources/git/source.rs b/src/cargo/sources/git/source.rs index b8381b73ccf..45cbe62c926 100644 --- a/src/cargo/sources/git/source.rs +++ b/src/cargo/sources/git/source.rs @@ -1,6 +1,6 @@ use crate::core::source::{MaybePackage, Source, SourceId}; +use crate::core::GitReference; use crate::core::{Dependency, Package, PackageId, Summary}; -use crate::core::{GitReference, InheritableFields}; use crate::sources::git::utils::GitRemote; use crate::sources::PathSource; use crate::util::errors::CargoResult; @@ -175,12 +175,7 @@ impl<'cfg> Source for GitSource<'cfg> { .join(short_id.as_str()); db.copy_to(actual_rev, &checkout_path, self.config)?; let source_id = self.source_id.with_precise(Some(actual_rev.to_string())); - let path_source = PathSource::new_recursive( - &checkout_path, - source_id, - self.config, - InheritableFields::default(), - ); + let path_source = PathSource::new_recursive(&checkout_path, source_id, self.config); self.path_source = Some(path_source); self.locked_rev = Some(actual_rev); diff --git a/src/cargo/sources/path.rs b/src/cargo/sources/path.rs index 9fdb4ef6323..54b46ef62a7 100644 --- a/src/cargo/sources/path.rs +++ b/src/cargo/sources/path.rs @@ -4,7 +4,7 @@ use std::fs; use std::path::{Path, PathBuf}; use crate::core::source::MaybePackage; -use crate::core::{Dependency, InheritableFields, Package, PackageId, Source, SourceId, Summary}; +use crate::core::{Dependency, Package, PackageId, Source, SourceId, Summary}; use crate::ops; use crate::util::{internal, CargoResult, Config}; use anyhow::Context as _; @@ -21,7 +21,6 @@ pub struct PathSource<'cfg> { packages: Vec, config: &'cfg Config, recursive: bool, - inherit: InheritableFields, } impl<'cfg> PathSource<'cfg> { @@ -29,12 +28,7 @@ impl<'cfg> PathSource<'cfg> { /// /// This source will only return the package at precisely the `path` /// specified, and it will be an error if there's not a package at `path`. - pub fn new( - path: &Path, - source_id: SourceId, - config: &'cfg Config, - inherit: InheritableFields, - ) -> PathSource<'cfg> { + pub fn new(path: &Path, source_id: SourceId, config: &'cfg Config) -> PathSource<'cfg> { PathSource { source_id, path: path.to_path_buf(), @@ -42,7 +36,6 @@ impl<'cfg> PathSource<'cfg> { packages: Vec::new(), config, recursive: false, - inherit, } } @@ -54,15 +47,10 @@ impl<'cfg> PathSource<'cfg> { /// /// Note that this should be used with care and likely shouldn't be chosen /// by default! - pub fn new_recursive( - root: &Path, - id: SourceId, - config: &'cfg Config, - inherit: InheritableFields, - ) -> PathSource<'cfg> { + pub fn new_recursive(root: &Path, id: SourceId, config: &'cfg Config) -> PathSource<'cfg> { PathSource { recursive: true, - ..PathSource::new(root, id, config, inherit) + ..PathSource::new(root, id, config) } } @@ -92,10 +80,10 @@ impl<'cfg> PathSource<'cfg> { if self.updated { Ok(self.packages.clone()) } else if self.recursive { - ops::read_packages(&self.path, self.source_id, self.config, &self.inherit) + ops::read_packages(&self.path, self.source_id, self.config) } else { let path = self.path.join("Cargo.toml"); - let (pkg, _) = ops::read_package(&path, self.source_id, self.config, &self.inherit)?; + let (pkg, _) = ops::read_package(&path, self.source_id, self.config, None)?; Ok(vec![pkg]) } } diff --git a/src/cargo/sources/registry/mod.rs b/src/cargo/sources/registry/mod.rs index c9a20cc252f..bc3c73ad4a8 100644 --- a/src/cargo/sources/registry/mod.rs +++ b/src/cargo/sources/registry/mod.rs @@ -174,7 +174,7 @@ use tar::Archive; use crate::core::dependency::{DepKind, Dependency}; use crate::core::source::MaybePackage; -use crate::core::{InheritableFields, Package, PackageId, Source, SourceId, Summary}; +use crate::core::{Package, PackageId, Source, SourceId, Summary}; use crate::sources::PathSource; use crate::util::hex; use crate::util::interning::InternedString; @@ -663,12 +663,7 @@ impl<'cfg> RegistrySource<'cfg> { let path = self .unpack_package(package, path) .with_context(|| format!("failed to unpack package `{}`", package))?; - let mut src = PathSource::new( - &path, - self.source_id, - self.config, - InheritableFields::default(), - ); + let mut src = PathSource::new(&path, self.source_id, self.config); src.update()?; let mut pkg = match src.download(package)? { MaybePackage::Ready(pkg) => pkg, diff --git a/src/cargo/util/config/mod.rs b/src/cargo/util/config/mod.rs index e42b01fa11b..f723ba817ec 100644 --- a/src/cargo/util/config/mod.rs +++ b/src/cargo/util/config/mod.rs @@ -68,7 +68,7 @@ use std::time::Instant; use self::ConfigValue as CV; use crate::core::compiler::rustdoc::RustdocExternMap; use crate::core::shell::Verbosity; -use crate::core::{features, CliUnstable, Shell, SourceId, Workspace}; +use crate::core::{features, CliUnstable, InheritableFields, Shell, SourceId, Workspace}; use crate::ops; use crate::util::errors::CargoResult; use crate::util::toml as cargo_toml; @@ -200,6 +200,8 @@ pub struct Config { /// NOTE: this should be set before `configure()`. If calling this from an integration test, /// consider using `ConfigBuilder::enable_nightly_features` instead. pub nightly_features_allowed: bool, + + pub inheritable: RefCell>, } impl Config { @@ -282,6 +284,7 @@ impl Config { progress_config: ProgressConfig::default(), env_config: LazyCell::new(), nightly_features_allowed: matches!(&*features::channel(), "nightly" | "dev"), + inheritable: RefCell::new(HashMap::new()), } } diff --git a/src/cargo/util/toml/mod.rs b/src/cargo/util/toml/mod.rs index 23a8643fcd3..013d2b9fb63 100644 --- a/src/cargo/util/toml/mod.rs +++ b/src/cargo/util/toml/mod.rs @@ -1346,7 +1346,7 @@ impl TomlManifest { source_id: SourceId, package_root: &Path, config: &Config, - inherit: &InheritableFields, + inherit: Option<&InheritableFields>, ) -> CargoResult<(Manifest, Vec)> { let mut nested_paths = vec![]; let mut warnings = vec![]; @@ -1360,9 +1360,40 @@ impl TomlManifest { let project = me.project.clone().or_else(|| me.package.clone()); let project = &mut project.ok_or_else(|| anyhow!("no `package` section found"))?; - TomlManifest::parse_toml_project(project, inherit)?; + let workspace_config = match (me.workspace.as_ref(), project.workspace.as_ref()) { + (Some(toml_workspace), None) => WorkspaceConfig::Root( + WorkspaceRootConfig::from_toml_workspace(package_root, toml_workspace), + ), + (None, root) => WorkspaceConfig::Member { + root: root.cloned(), + }, + (Some(..), Some(..)) => bail!( + "cannot configure both `package.workspace` and \ + `[workspace]`, only one can be specified" + ), + }; - project.license_file = match (project.license_file.clone(), inherit.license_file.as_ref()) { + let inheritable = if inherit.is_some() { + inherit.unwrap().to_owned() + } else { + match find_inheritable( + source_id, + config, + package_root.join("Cargo.toml"), + project.name.to_string(), + workspace_config, + )? { + None => InheritableFields::default(), + Some(inherit) => inherit, + } + }; + + TomlManifest::parse_toml_project(project, &inheritable)?; + + project.license_file = match ( + project.license_file.clone(), + inheritable.license_file.as_ref(), + ) { (None, _) => None, (Some(MaybeWorkspace::Defined(defined)), _) => Some(MaybeWorkspace::Defined(defined)), (Some(MaybeWorkspace::Workspace(_)), None) => { @@ -1370,14 +1401,14 @@ impl TomlManifest { } (Some(MaybeWorkspace::Workspace(_)), Some(ws_license_file)) => { Some(MaybeWorkspace::Defined(join_relative_path( - inherit.ws_path.clone().unwrap().as_path(), + inheritable.ws_path.clone().unwrap().as_path(), package_root, ws_license_file, )?)) } }; - project.readme = match (project.readme.clone(), inherit.readme.as_ref()) { + project.readme = match (project.readme.clone(), inheritable.readme.as_ref()) { (None, _) => match default_readme_from_package_root(package_root) { None => None, Some(readme) => Some(MaybeWorkspace::Defined(StringOrBool::String(readme))), @@ -1389,7 +1420,7 @@ impl TomlManifest { (Some(MaybeWorkspace::Workspace(_)), Some(defined)) => match defined { StringOrBool::String(file) => Some(MaybeWorkspace::Defined(StringOrBool::String( join_relative_path( - inherit.ws_path.clone().unwrap().as_path(), + inheritable.ws_path.clone().unwrap().as_path(), package_root, file, )?, @@ -1568,19 +1599,20 @@ impl TomlManifest { } // Collect the dependencies. - let dependencies = process_dependencies(&mut cx, me.dependencies.as_ref(), None, inherit)?; + let dependencies = + process_dependencies(&mut cx, me.dependencies.as_ref(), None, &inheritable)?; let dev_deps = me .dev_dependencies .as_ref() .or_else(|| me.dev_dependencies2.as_ref()); let dev_dependencies = - process_dependencies(&mut cx, dev_deps, Some(DepKind::Development), inherit)?; + process_dependencies(&mut cx, dev_deps, Some(DepKind::Development), &inheritable)?; let build_deps = me .build_dependencies .as_ref() .or_else(|| me.build_dependencies2.as_ref()); let build_dependencies = - process_dependencies(&mut cx, build_deps, Some(DepKind::Build), inherit)?; + process_dependencies(&mut cx, build_deps, Some(DepKind::Build), &inheritable)?; for (name, platform) in me.target.iter().flatten() { cx.platform = { @@ -1588,17 +1620,17 @@ impl TomlManifest { platform.check_cfg_attributes(&mut cx.warnings); Some(platform) }; - process_dependencies(&mut cx, platform.dependencies.as_ref(), None, inherit)?; + process_dependencies(&mut cx, platform.dependencies.as_ref(), None, &inheritable)?; let build_deps = platform .build_dependencies .as_ref() .or_else(|| platform.build_dependencies2.as_ref()); - process_dependencies(&mut cx, build_deps, Some(DepKind::Build), inherit)?; + process_dependencies(&mut cx, build_deps, Some(DepKind::Build), &inheritable)?; let dev_deps = platform .dev_dependencies .as_ref() .or_else(|| platform.dev_dependencies2.as_ref()); - process_dependencies(&mut cx, dev_deps, Some(DepKind::Development), inherit)?; + process_dependencies(&mut cx, dev_deps, Some(DepKind::Development), &inheritable)?; } replace = me.replace(&mut cx)?; @@ -1636,7 +1668,7 @@ impl TomlManifest { let badges = ws_default( me.badges.clone(), - inherit, + &inheritable, |inherit| &inherit.badges, "badges", )?; @@ -2498,11 +2530,42 @@ impl DetailedTomlDependency

{ } fn infer_path_version(&mut self, cx: &mut Context<'_, '_>, name: &str) -> CargoResult<()> { + fn find_version( + base_path: PathBuf, + source_id: SourceId, + config: &Config, + name: &str, + package: &Box, + workspace_config: WorkspaceConfig, + ) -> CargoResult> { + let inherit = match find_inheritable( + source_id, + config, + base_path, + name.to_string(), + workspace_config, + )? { + Some(inheritable) => inheritable, + None => bail!( + "workspace does not define version information required by {}", + name + ), + }; + match (package.publish.as_ref(), inherit.publish) { + (Some(MaybeWorkspace::Defined(VecStringOrBool::Bool(false))), _) => Ok(None), + (_, Some(VecStringOrBool::Bool(false))) => Ok(None), + _ => Ok(Some(inherit.version.expect(&format!( + "workspace does not define version information required by {}", + name + )))), + } + } if let (None, Some(p)) = (&self.version, &self.path) { - let base_path = &cx.root.join(p.resolve(cx.config)); - let (manifest, _) = - read_manifest(&base_path.join("Cargo.toml"), cx.source_id, cx.config) - .with_context(|| format!("failed to get dependency `{}`", name))?; + let base_path = &cx.root.join(p.resolve(cx.config)).join("Cargo.toml"); + let key = base_path.parent().unwrap(); + let source_id = SourceId::for_path(key)?; + let (manifest, _) = read_manifest(base_path, source_id, cx.config) + .with_context(|| format!("failed to get dependency `{}`", name))?; self.version = if let EitherManifest::Real(ref intermediate) = manifest { let toml = intermediate.original(); let package = toml @@ -2510,54 +2573,40 @@ impl DetailedTomlDependency

{ .as_ref() .or_else(|| toml.project.as_ref()) .ok_or_else(|| anyhow!("no `package` section found"))?; - let v = match package.version { - MaybeWorkspace::Workspace(_) => { - let root_path = - find_workspace_root(&base_path.join("Cargo.toml"), cx.config)? - .expect("workspace was referenced, none found"); - let (root_man, _) = read_manifest(&root_path, cx.source_id, cx.config) - .with_context(|| format!("failed to get workspace for `{}`", name))?; - if let WorkspaceConfig::Root(ws_config) = root_man.workspace_config() { - let inherit = ws_config.inheritable_fields().clone(); - match (package.publish.as_ref(), inherit.publish) { - ( - Some(MaybeWorkspace::Defined(VecStringOrBool::Bool(false))), - _, - ) => None, - (_, Some(VecStringOrBool::Bool(false))) => None, - _ => Some(inherit.version.expect(&format!( - "workspace does not define version information required by {}", - name - ))), - } - } else { - bail!( - "workspace does not define version information required by {}", - name - ) - } + let workspace_config = match (toml.workspace.as_ref(), package.workspace.as_ref()) { + (Some(toml_workspace), None) => { + WorkspaceConfig::Root(WorkspaceRootConfig::from_toml_workspace( + &cx.root.join(p.resolve(cx.config)), + toml_workspace, + )) } + (None, root) => WorkspaceConfig::Member { + root: root.cloned(), + }, + (Some(..), Some(..)) => bail!( + "cannot configure both `package.workspace` and \ + `[workspace]`, only one can be specified" + ), + }; + let v = match package.version { + MaybeWorkspace::Workspace(_) => find_version( + base_path.clone(), + source_id, + cx.config, + name, + package, + workspace_config, + )?, MaybeWorkspace::Defined(ref version) => match package.publish { Some(MaybeWorkspace::Defined(VecStringOrBool::Bool(false))) => None, - Some(MaybeWorkspace::Workspace(_)) => { - let root_path = - find_workspace_root(&base_path.join("Cargo.toml"), cx.config)? - .expect("workspace was referenced, none found"); - let (root_man, _) = read_manifest(&root_path, cx.source_id, cx.config) - .with_context(|| format!("failed to get dependency `{}`", name))?; - if let WorkspaceConfig::Root(ws_config) = root_man.workspace_config() { - let inherit = ws_config.inheritable_fields().clone(); - match inherit.publish { - Some(VecStringOrBool::Bool(false)) => None, - _ => Some(version.clone()), - } - } else { - bail!( - "workspace does not define version information required by {}", - name - ) - } - } + Some(MaybeWorkspace::Workspace(_)) => find_version( + base_path.clone(), + source_id, + cx.config, + name, + package, + workspace_config, + )?, _ => Some(version.clone()), }, }; @@ -2670,3 +2719,62 @@ impl fmt::Debug for PathValue { self.0.fmt(f) } } + +fn find_inheritable( + source_id: SourceId, + config: &Config, + resolved_path: PathBuf, + name: String, + workspace_config: WorkspaceConfig, +) -> CargoResult> { + fn inheritable( + source_id: SourceId, + config: &Config, + resolved_path: PathBuf, + name: String, + ) -> CargoResult { + let mut inheritable_map = config.inheritable.borrow_mut(); + match inheritable_map.get(&resolved_path) { + Some(inheritable) => Ok(inheritable.clone()), + None => { + let (man, _) = read_manifest(&resolved_path, source_id, config) + .expect(format!("failed to get workspace for `{}`", name).as_str()); + match man.workspace_config() { + WorkspaceConfig::Root(root) => { + let fields = root.inheritable_fields().clone(); + inheritable_map.insert(resolved_path, fields.clone()); + Ok(fields) + } + _ => bail!( + "root of a workspace inferred but wasn't a root: {}", + resolved_path.display() + ), + } + } + } + } + + match workspace_config { + WorkspaceConfig::Root(root) => Ok(Some(root.inheritable_fields().clone())), + WorkspaceConfig::Member { + root: Some(ref path_to_root), + } => { + let path = resolved_path + .parent() + .unwrap() + .join(path_to_root) + .join("Cargo.toml"); + debug!("find_root - pointer {}", path.display()); + let root_path = paths::normalize_path(&path); + inheritable(source_id, config, root_path, name).map(|inherit| Some(inherit)) + } + WorkspaceConfig::Member { root: None } => { + match find_workspace_root(&resolved_path, config)? { + Some(path_to_root) => { + inheritable(source_id, config, path_to_root, name).map(|inherit| Some(inherit)) + } + None => Ok(None), + } + } + } +} diff --git a/tests/testsuite/package.rs b/tests/testsuite/package.rs index 5f500d33fa0..c896030b703 100644 --- a/tests/testsuite/package.rs +++ b/tests/testsuite/package.rs @@ -1951,7 +1951,7 @@ src/main.rs Caused by: no matching package named `bar` found - location searched: registry `https://github.com/rust-lang/crates.io-index` + location searched: registry `crates-io` required by package `foo v0.1.0 [..]` ", ) diff --git a/tests/testsuite/workspaces.rs b/tests/testsuite/workspaces.rs index 075009e4666..2f4649db78e 100644 --- a/tests/testsuite/workspaces.rs +++ b/tests/testsuite/workspaces.rs @@ -551,9 +551,7 @@ fn dangling_member() { .with_status(101) .with_stderr( "\ -error: package `[..]` is a member of the wrong workspace -expected: [..] -actual: [..] +error: root of a workspace inferred but wasn't a root: [..] ", ) .run(); From a7d41d652795b1829049209c48658c7dc18caec2 Mon Sep 17 00:00:00 2001 From: Scott Schafer <23045215+Muscraft@users.noreply.github.com> Date: Mon, 11 Oct 2021 16:40:23 -0500 Subject: [PATCH 09/10] - Fixed test for inherit_workspace_fields Signed-off-by: Scott Schafer <23045215+Muscraft@users.noreply.github.com> --- tests/testsuite/deduplicate_workspace.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/testsuite/deduplicate_workspace.rs b/tests/testsuite/deduplicate_workspace.rs index e484fabc323..59903c79189 100644 --- a/tests/testsuite/deduplicate_workspace.rs +++ b/tests/testsuite/deduplicate_workspace.rs @@ -159,11 +159,11 @@ fn inherit_workspace_fields() { "homepage": "https://www.rust-lang.org", "keywords": ["cli"], "license": "MIT", - "license_file": "../LICENSE", + "license_file": "[..]LICENSE", "links": null, "name": "bar", "readme": "README.md", - "readme_file": "../README.md", + "readme_file": "[..]README.md", "repository": "https://github.com/example/example", "vers": "1.2.3" } From 155932d30a8fe5866ef4d1ff9e3066f7615d3514 Mon Sep 17 00:00:00 2001 From: Scott Schafer <23045215+Muscraft@users.noreply.github.com> Date: Mon, 11 Oct 2021 19:18:06 -0500 Subject: [PATCH 10/10] - Fixed test for inherit_workspace_fields Signed-off-by: Scott Schafer <23045215+Muscraft@users.noreply.github.com> --- tests/testsuite/deduplicate_workspace.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/testsuite/deduplicate_workspace.rs b/tests/testsuite/deduplicate_workspace.rs index 59903c79189..69345f1d8e6 100644 --- a/tests/testsuite/deduplicate_workspace.rs +++ b/tests/testsuite/deduplicate_workspace.rs @@ -191,7 +191,7 @@ publish = true description = "This is a crate" homepage = "https://www.rust-lang.org" documentation = "https://www.rust-lang.org/learn" -readme = "../README.md" +readme = "[..]README.md" keywords = ["cli"] categories = ["development-tools"] license = "MIT"