From 3d07652c3810e38776f9c0e04c8e32f25fc1380e Mon Sep 17 00:00:00 2001 From: Scott Schafer Date: Thu, 7 Apr 2022 16:34:34 -0500 Subject: [PATCH] Part 3 of RFC2906 - Add support for inheriting `license-path`, and `depednency.path` --- Cargo.toml | 1 + src/cargo/core/mod.rs | 4 +- src/cargo/core/workspace.rs | 40 ++++++++- src/cargo/util/toml/mod.rs | 81 +++++++++++++++---- .../testsuite/inheritable_workspace_fields.rs | 62 +++++++++++++- 5 files changed, 165 insertions(+), 23 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index ee1ce8155..9212d9a1e 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -45,6 +45,7 @@ libgit2-sys = "0.13.2" memchr = "2.1.3" opener = "0.5" os_info = "3.0.7" +pathdiff = "0.2.1" percent-encoding = "2.0" rustfix = "0.6.0" semver = { version = "1.0.3", features = ["serde"] } diff --git a/src/cargo/core/mod.rs b/src/cargo/core/mod.rs index 633b29907..9667be09e 100644 --- a/src/cargo/core/mod.rs +++ b/src/cargo/core/mod.rs @@ -11,8 +11,8 @@ pub use self::shell::{Shell, Verbosity}; pub use self::source::{GitReference, Source, SourceId, SourceMap}; pub use self::summary::{FeatureMap, FeatureValue, Summary}; pub use self::workspace::{ - find_workspace_root, InheritableFields, MaybePackage, Workspace, WorkspaceConfig, - WorkspaceRootConfig, + find_workspace_root, resolve_relative_path, InheritableFields, MaybePackage, Workspace, + WorkspaceConfig, WorkspaceRootConfig, }; pub mod compiler; diff --git a/src/cargo/core/workspace.rs b/src/cargo/core/workspace.rs index 7de07f53a..4c9a5936f 100644 --- a/src/cargo/core/workspace.rs +++ b/src/cargo/core/workspace.rs @@ -27,6 +27,8 @@ use crate::util::toml::{ }; use crate::util::{config::ConfigRelativePath, Config, Filesystem, IntoUrl}; use cargo_util::paths; +use cargo_util::paths::normalize_path; +use pathdiff::diff_paths; /// The core abstraction in Cargo for working with a workspace of crates. /// @@ -1650,6 +1652,7 @@ pub struct InheritableFields { publish: Option, edition: Option, badges: Option>>, + ws_root: PathBuf, } impl InheritableFields { @@ -1669,6 +1672,7 @@ impl InheritableFields { publish: Option, edition: Option, badges: Option>>, + ws_root: PathBuf, ) -> InheritableFields { Self { dependencies, @@ -1686,6 +1690,7 @@ impl InheritableFields { publish, edition, badges, + ws_root, } } @@ -1780,10 +1785,10 @@ impl InheritableFields { }) } - pub fn license_file(&self) -> CargoResult { + pub fn license_file(&self, package_root: &Path) -> CargoResult { self.license_file.clone().map_or( Err(anyhow!("`workspace.license_file` was not defined")), - |d| Ok(d), + |d| resolve_relative_path("license-file", &self.ws_root, package_root, &d), ) } @@ -1817,6 +1822,37 @@ impl InheritableFields { Ok(d) }) } + + pub fn ws_root(&self) -> &PathBuf { + &self.ws_root + } +} + +pub fn resolve_relative_path( + label: &str, + old_root: &Path, + new_root: &Path, + rel_path: &str, +) -> CargoResult { + let joined_path = normalize_path(&old_root.join(rel_path)); + match diff_paths(joined_path, new_root) { + None => Err(anyhow!( + "`{}` was defined in {} but could not be resolved with {}", + label, + old_root.display(), + new_root.display() + )), + Some(path) => Ok(path + .to_str() + .ok_or_else(|| { + anyhow!( + "`{}` resolved to non-UTF value (`{}`)", + label, + path.display() + ) + })? + .to_owned()), + } } fn parse_manifest(manifest_path: &Path, config: &Config) -> CargoResult { diff --git a/src/cargo/util/toml/mod.rs b/src/cargo/util/toml/mod.rs index 7dde82669..0d1c5b9bf 100644 --- a/src/cargo/util/toml/mod.rs +++ b/src/cargo/util/toml/mod.rs @@ -20,7 +20,9 @@ use crate::core::compiler::{CompileKind, CompileTarget}; use crate::core::dependency::{Artifact, ArtifactTarget, DepKind}; use crate::core::manifest::{ManifestMetadata, TargetSourcePath, Warnings}; use crate::core::resolver::ResolveBehavior; -use crate::core::{find_workspace_root, Dependency, Manifest, PackageId, Summary, Target}; +use crate::core::{ + find_workspace_root, resolve_relative_path, Dependency, Manifest, PackageId, Summary, Target, +}; use crate::core::{ Edition, EitherManifest, Feature, Features, InheritableFields, VirtualManifest, Workspace, }; @@ -1021,6 +1023,12 @@ impl MaybeWorkspace { )), } } + fn as_defined(&self) -> Option<&T> { + match self { + MaybeWorkspace::Workspace(_) => None, + MaybeWorkspace::Defined(defined) => Some(defined), + } + } } #[derive(Deserialize, Serialize, Clone, Debug)] @@ -1069,7 +1077,7 @@ pub struct TomlProject { keywords: Option>>, categories: Option>>, license: Option>, - license_file: Option, + license_file: Option>, repository: Option>, resolver: Option, @@ -1149,19 +1157,22 @@ impl TomlManifest { package.workspace = None; package.resolver = ws.resolve_behavior().to_manifest(); if let Some(license_file) = &package.license_file { + let license_file = license_file + .as_defined() + .context("license file should have been resolved before `prepare_for_publish()`")?; let license_path = Path::new(&license_file); let abs_license_path = paths::normalize_path(&package_root.join(license_path)); if abs_license_path.strip_prefix(package_root).is_err() { // This path points outside of the package root. `cargo package` // will copy it into the root, so adjust the path to this location. - package.license_file = Some( + package.license_file = Some(MaybeWorkspace::Defined( license_path .file_name() .unwrap() .to_str() .unwrap() .to_string(), - ); + )); } } let all = |_d: &TomlDependency| true; @@ -1340,6 +1351,7 @@ impl TomlManifest { config.publish.clone(), config.edition.clone(), config.badges.clone(), + package_root.to_path_buf(), ); WorkspaceConfig::Root(WorkspaceRootConfig::new( @@ -1506,13 +1518,12 @@ impl TomlManifest { }; let mut deps: BTreeMap = BTreeMap::new(); for (n, v) in dependencies.iter() { - let resolved = v.clone().resolve(features, n, || { + let resolved = v.clone().resolve(features, n, cx, || { get_ws( cx.config, cx.root.join("Cargo.toml"), workspace_config.clone(), - )? - .get_dependency(n) + ) })?; let dep = resolved.to_dependency(n, cx, kind)?; validate_package_name(dep.name_in_toml().as_str(), "dependency name", "")?; @@ -1702,7 +1713,16 @@ impl TomlManifest { }) }) .transpose()?, - license_file: project.license_file.clone(), + license_file: project + .license_file + .clone() + .map(|mw| { + mw.resolve(&features, "license", || { + get_ws(config, resolved_path.clone(), workspace_config.clone())? + .license_file(package_root) + }) + }) + .transpose()?, repository: project .repository .clone() @@ -1766,6 +1786,10 @@ impl TomlManifest { .license .clone() .map(|license| MaybeWorkspace::Defined(license)); + project.license_file = metadata + .license_file + .clone() + .map(|license_file| MaybeWorkspace::Defined(license_file)); project.repository = metadata .repository .clone() @@ -1999,6 +2023,7 @@ impl TomlManifest { config.publish.clone(), config.edition.clone(), config.badges.clone(), + root.to_path_buf(), ); WorkspaceConfig::Root(WorkspaceRootConfig::new( root, @@ -2240,13 +2265,16 @@ impl TomlDependency

{ TomlDependency::Workspace(w) => w.optional.unwrap_or(false), } } +} +impl TomlDependency { fn resolve<'a>( self, cargo_features: &Features, label: &str, - get_ws_dependency: impl FnOnce() -> CargoResult>, - ) -> CargoResult> { + cx: &mut Context<'_, '_>, + get_inheritable: impl FnOnce() -> CargoResult, + ) -> CargoResult { match self { TomlDependency::Detailed(d) => Ok(TomlDependency::Detailed(d)), TomlDependency::Simple(s) => Ok(TomlDependency::Simple(s)), @@ -2256,28 +2284,30 @@ impl TomlDependency

{ optional, }) => { cargo_features.require(Feature::workspace_inheritance())?; - get_ws_dependency().context(format!( + let inheritable = get_inheritable()?; + inheritable.get_dependency(label).context(format!( "error reading `dependencies.{}` from workspace root manifest's `workspace.dependencies.{}`", label, label )).map(|dep| { match dep { TomlDependency::Simple(s) => { if optional.is_some() || features.is_some() { - TomlDependency::Detailed(DetailedTomlDependency::

{ + Ok(TomlDependency::Detailed(DetailedTomlDependency { version: Some(s), optional, features, ..Default::default() - }) + })) } else { - TomlDependency::Simple(s) + Ok(TomlDependency::Simple(s)) } }, TomlDependency::Detailed(d) => { let mut dep = d.clone(); dep.add_features(features); dep.update_optional(optional); - TomlDependency::Detailed(dep) + dep.resolve_path(label,inheritable.ws_root(), cx.root)?; + Ok(TomlDependency::Detailed(dep)) }, TomlDependency::Workspace(_) => { unreachable!( @@ -2288,7 +2318,7 @@ impl TomlDependency

{ ) }, } - }) + })? } TomlDependency::Workspace(TomlWorkspaceDependency { workspace: false, .. @@ -2543,7 +2573,9 @@ impl DetailedTomlDependency

{ } Ok(dep) } +} +impl DetailedTomlDependency { fn add_features(&mut self, features: Option>) { self.features = match (self.features.clone(), features.clone()) { (Some(dep_feat), Some(inherit_feat)) => Some( @@ -2561,6 +2593,23 @@ impl DetailedTomlDependency

{ fn update_optional(&mut self, optional: Option) { self.optional = optional; } + + fn resolve_path( + &mut self, + name: &str, + root_path: &Path, + package_root: &Path, + ) -> CargoResult<()> { + if let Some(rel_path) = &self.path { + self.path = Some(resolve_relative_path( + name, + root_path, + package_root, + rel_path, + )?) + } + Ok(()) + } } #[derive(Default, Serialize, Deserialize, Debug, Clone)] diff --git a/tests/testsuite/inheritable_workspace_fields.rs b/tests/testsuite/inheritable_workspace_fields.rs index 427a2adf0..75e1edf43 100644 --- a/tests/testsuite/inheritable_workspace_fields.rs +++ b/tests/testsuite/inheritable_workspace_fields.rs @@ -1,6 +1,8 @@ //! Tests for inheriting Cargo.toml fields with { workspace = true } use cargo_test_support::registry::{Dependency, Package}; -use cargo_test_support::{basic_lib_manifest, git, path2url, paths, project, publish, registry}; +use cargo_test_support::{ + basic_lib_manifest, basic_manifest, git, path2url, paths, project, publish, registry, +}; #[cargo_test] fn permit_additional_workspace_fields() { @@ -581,6 +583,8 @@ fn inherit_workspace_fields() { documentation = "https://www.rust-lang.org/learn" homepage = "https://www.rust-lang.org" repository = "https://github.com/example/example" + license = "MIT" + license-file = "LICENSE" keywords = ["cli"] categories = ["development-tools"] publish = true @@ -604,12 +608,15 @@ fn inherit_workspace_fields() { documentation = { 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("bar/src/main.rs", "fn main() {}") .build(); @@ -631,8 +638,8 @@ fn inherit_workspace_fields() { "features": {}, "homepage": "https://www.rust-lang.org", "keywords": ["cli"], - "license": null, - "license_file": null, + "license": "MIT", + "license_file": "../LICENSE", "links": null, "name": "bar", "readme": null, @@ -647,6 +654,7 @@ fn inherit_workspace_fields() { "Cargo.toml", "Cargo.toml.orig", "src/main.rs", + "LICENSE", ".cargo_vcs_info.json", ], &[( @@ -666,6 +674,8 @@ homepage = "https://www.rust-lang.org" documentation = "https://www.rust-lang.org/learn" keywords = ["cli"] categories = ["development-tools"] +license = "MIT" +license-file = "LICENSE" repository = "https://github.com/example/example" [badges.gitlab] @@ -1038,6 +1048,52 @@ fn inherit_detailed_dependencies() { .run(); } +#[cargo_test] +fn inherit_path_dependencies() { + let p = project() + .file( + "Cargo.toml", + r#" + [workspace] + members = ["bar"] + [workspace.dependencies] + dep = { path = "dep" } + "#, + ) + .file( + "bar/Cargo.toml", + r#" + cargo-features = ["workspace-inheritance"] + + [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") + .masquerade_as_nightly_cargo() + .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 error_workspace_false() { registry::init();