From b75dd6b8a81e8796268c575b9ac3cbc22f24335d Mon Sep 17 00:00:00 2001 From: Daniel Paoliello Date: Wed, 3 Jan 2024 14:24:11 -0800 Subject: [PATCH] Implement support for base paths --- crates/cargo-util-schemas/src/manifest/mod.rs | 2 + src/cargo/core/features.rs | 2 + src/cargo/sources/path.rs | 46 ++- src/cargo/util/toml/mod.rs | 62 ++- tests/testsuite/cargo/z_help/stdout.term.svg | 30 +- tests/testsuite/git.rs | 101 +++++ tests/testsuite/path.rs | 381 ++++++++++++++++++ 7 files changed, 596 insertions(+), 28 deletions(-) diff --git a/crates/cargo-util-schemas/src/manifest/mod.rs b/crates/cargo-util-schemas/src/manifest/mod.rs index df0bb51dcfde..60741f0ff16f 100644 --- a/crates/cargo-util-schemas/src/manifest/mod.rs +++ b/crates/cargo-util-schemas/src/manifest/mod.rs @@ -776,6 +776,7 @@ pub struct TomlDetailedDependency { // `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. pub path: Option

, + pub base: Option, pub git: Option, pub branch: Option, pub tag: Option, @@ -815,6 +816,7 @@ impl Default for TomlDetailedDependency

{ registry: Default::default(), registry_index: Default::default(), path: Default::default(), + base: Default::default(), git: Default::default(), branch: Default::default(), tag: Default::default(), diff --git a/src/cargo/core/features.rs b/src/cargo/core/features.rs index c44b8d51eb4e..66eaa501bae9 100644 --- a/src/cargo/core/features.rs +++ b/src/cargo/core/features.rs @@ -776,6 +776,7 @@ unstable_cli_options!( no_index_update: bool = ("Do not update the registry index even if the cache is outdated"), package_workspace: bool = ("Handle intra-workspace dependencies when packaging"), panic_abort_tests: bool = ("Enable support to run tests with -Cpanic=abort"), + path_bases: bool = ("Allow paths that resolve relatively to a base specified in the config"), profile_rustflags: bool = ("Enable the `rustflags` option in profiles in .cargo/config.toml file"), public_dependency: bool = ("Respect a dependency's `public` field in Cargo.toml to control public/private dependencies"), publish_timeout: bool = ("Enable the `publish.timeout` key in .cargo/config.toml file"), @@ -1280,6 +1281,7 @@ impl CliUnstable { "package-workspace" => self.package_workspace= parse_empty(k, v)?, "panic-abort-tests" => self.panic_abort_tests = parse_empty(k, v)?, "public-dependency" => self.public_dependency = parse_empty(k, v)?, + "path-bases" => self.path_bases = parse_empty(k, v)?, "profile-rustflags" => self.profile_rustflags = parse_empty(k, v)?, "trim-paths" => self.trim_paths = parse_empty(k, v)?, "publish-timeout" => self.publish_timeout = parse_empty(k, v)?, diff --git a/src/cargo/sources/path.rs b/src/cargo/sources/path.rs index 4159da352754..9c35e01ab62a 100644 --- a/src/cargo/sources/path.rs +++ b/src/cargo/sources/path.rs @@ -1,3 +1,4 @@ +use std::borrow::Cow; use std::collections::{HashMap, HashSet}; use std::fmt::{self, Debug, Formatter}; use std::fs; @@ -14,7 +15,7 @@ use crate::sources::IndexSummary; use crate::util::errors::CargoResult; use crate::util::important_paths::find_project_manifest_exact; use crate::util::internal; -use crate::util::toml::read_manifest; +use crate::util::toml::{lookup_path_base, read_manifest}; use crate::util::GlobalContext; use anyhow::Context as _; use cargo_util::paths; @@ -878,7 +879,7 @@ fn read_packages( } } -fn nested_paths(manifest: &Manifest) -> Vec { +fn nested_paths(manifest: &Manifest) -> Vec<(String, PathBuf, Option)> { let mut nested_paths = Vec::new(); let normalized = manifest.normalized_toml(); let dependencies = normalized @@ -900,7 +901,7 @@ fn nested_paths(manifest: &Manifest) -> Vec { }), ); for dep_table in dependencies { - for dep in dep_table.values() { + for (name, dep) in dep_table.iter() { let cargo_util_schemas::manifest::InheritableDependency::Value(dep) = dep else { continue; }; @@ -910,7 +911,11 @@ fn nested_paths(manifest: &Manifest) -> Vec { let Some(path) = dep.path.as_ref() else { continue; }; - nested_paths.push(PathBuf::from(path.as_str())); + nested_paths.push(( + name.to_string(), + PathBuf::from(path.as_str()), + dep.base.clone(), + )); } } nested_paths @@ -1000,8 +1005,36 @@ fn read_nested_packages( // // TODO: filesystem/symlink implications? if !source_id.is_registry() { - for p in nested.iter() { - let path = paths::normalize_path(&path.join(p)); + let mut manifest_gctx = None; + + for (name, p, base) in nested.iter() { + let p = if let Some(base) = base { + // If the dependency has a path base, then load the global context for the current + // manifest and use it to resolve the path base. + let manifest_gctx = match manifest_gctx { + Some(ref gctx) => gctx, + None => { + let mut new_manifest_gctx = match GlobalContext::default() { + Ok(gctx) => gctx, + Err(err) => return Err(err), + }; + if let Err(err) = new_manifest_gctx.reload_rooted_at(&manifest_path) { + return Err(err); + } + manifest_gctx.insert(new_manifest_gctx) + } + }; + match lookup_path_base(base, name, manifest_gctx, manifest_path.parent().unwrap()) { + Ok(base) => Cow::Owned(base.join(p)), + Err(err) => { + errors.push(err); + continue; + } + } + } else { + Cow::Borrowed(p) + }; + let path = paths::normalize_path(&path.join(p.as_path())); let result = read_nested_packages(&path, all_packages, source_id, gctx, visited, errors); // Ignore broken manifests found on git repositories. @@ -1019,6 +1052,7 @@ fn read_nested_packages( ); errors.push(err); } else { + trace!("Failed to manifest: {:?}", err); return Err(err); } } diff --git a/src/cargo/util/toml/mod.rs b/src/cargo/util/toml/mod.rs index 353189917892..e7be289829f1 100644 --- a/src/cargo/util/toml/mod.rs +++ b/src/cargo/util/toml/mod.rs @@ -901,13 +901,17 @@ impl InheritableFields { }; let mut dep = dep.clone(); if let manifest::TomlDependency::Detailed(detailed) = &mut dep { - if let Some(rel_path) = &detailed.path { - detailed.path = Some(resolve_relative_path( - name, - self.ws_root(), - package_root, - rel_path, - )?); + if detailed.base.is_none() { + // If this is a path dependency without a base, then update the path to be relative + // to the workspace root instead. + if let Some(rel_path) = &detailed.path { + detailed.path = Some(resolve_relative_path( + name, + self.ws_root(), + package_root, + rel_path, + )?); + } } } Ok(dep) @@ -2135,7 +2139,17 @@ fn to_dependency_source_id( // always end up hashing to the same value no matter where it's // built from. if manifest_ctx.source_id.is_path() { - let path = manifest_ctx.root.join(path); + let path = if let Some(base) = orig.base.as_ref() { + if !manifest_ctx.gctx.cli_unstable().path_bases { + bail!("usage of path bases requires `-Z path-bases`"); + } + + lookup_path_base(&base, name_in_toml, manifest_ctx.gctx, manifest_ctx.root)? + .join(path) + } else { + // This is a standard path with no prefix. + manifest_ctx.root.join(path) + }; let path = paths::normalize_path(&path); SourceId::for_path(&path) } else { @@ -2151,6 +2165,37 @@ fn to_dependency_source_id( } } +pub(crate) fn lookup_path_base( + base: &str, + name_in_toml: &str, + gctx: &GlobalContext, + manifest_root: &Path, +) -> CargoResult { + // Look up the relevant base in the Config and use that as the root. + if let Some(path_bases) = + gctx.get::>(&format!("path-bases.{base}"))? + { + Ok(path_bases.resolve_path(gctx)) + } else { + // Otherwise, check the built-in bases. + match base { + "workspace" => { + if let Some(workspace_root) = find_workspace_root(manifest_root, gctx)? { + Ok(workspace_root.parent().unwrap().to_path_buf()) + } else { + bail!( + "dependency ({name_in_toml}) is using the `workspace` built-in path base outside of a workspace." + ) + } + } + _ => bail!( + "dependency ({name_in_toml}) uses an undefined path base `{base}`. \ + You must add an entry for `{base}` in the Cargo configuration [path-bases] table." + ), + } + } +} + pub trait ResolveToPath { fn resolve(&self, gctx: &GlobalContext) -> PathBuf; } @@ -2865,6 +2910,7 @@ fn prepare_toml_for_publish( let mut d = d.clone(); // Path dependencies become crates.io deps. d.path.take(); + d.base.take(); // Same with git dependencies. d.git.take(); d.branch.take(); diff --git a/tests/testsuite/cargo/z_help/stdout.term.svg b/tests/testsuite/cargo/z_help/stdout.term.svg index e5386620e464..2549003e5180 100644 --- a/tests/testsuite/cargo/z_help/stdout.term.svg +++ b/tests/testsuite/cargo/z_help/stdout.term.svg @@ -66,33 +66,35 @@ -Z panic-abort-tests Enable support to run tests with -Cpanic=abort - -Z profile-rustflags Enable the `rustflags` option in profiles in .cargo/config.toml file + -Z path-bases Allow paths that resolve relatively to a base specified in the config - -Z public-dependency Respect a dependency's `public` field in Cargo.toml to control public/private dependencies + -Z profile-rustflags Enable the `rustflags` option in profiles in .cargo/config.toml file - -Z publish-timeout Enable the `publish.timeout` key in .cargo/config.toml file + -Z public-dependency Respect a dependency's `public` field in Cargo.toml to control public/private dependencies - -Z rustdoc-map Allow passing external documentation mappings to rustdoc + -Z publish-timeout Enable the `publish.timeout` key in .cargo/config.toml file - -Z rustdoc-scrape-examples Allows Rustdoc to scrape code examples from reverse-dependencies + -Z rustdoc-map Allow passing external documentation mappings to rustdoc - -Z script Enable support for single-file, `.rs` packages + -Z rustdoc-scrape-examples Allows Rustdoc to scrape code examples from reverse-dependencies - -Z target-applies-to-host Enable the `target-applies-to-host` key in the .cargo/config.toml file + -Z script Enable support for single-file, `.rs` packages - -Z trim-paths Enable the `trim-paths` option in profiles + -Z target-applies-to-host Enable the `target-applies-to-host` key in the .cargo/config.toml file - -Z unstable-options Allow the usage of unstable options + -Z trim-paths Enable the `trim-paths` option in profiles - + -Z unstable-options Allow the usage of unstable options - Run with `cargo -Z [FLAG] [COMMAND]` + - + Run with `cargo -Z [FLAG] [COMMAND]` - See https://doc.rust-lang.org/nightly/cargo/reference/unstable.html for more information about these flags. + - + See https://doc.rust-lang.org/nightly/cargo/reference/unstable.html for more information about these flags. + + diff --git a/tests/testsuite/git.rs b/tests/testsuite/git.rs index 21943c3e29c4..8dfcdb0407c6 100644 --- a/tests/testsuite/git.rs +++ b/tests/testsuite/git.rs @@ -380,6 +380,107 @@ hello world .run(); } +#[cargo_test] +fn dependency_in_submodule_via_path_base() { + // Using a submodule prevents the dependency from being discovered during the directory walk, + // so Cargo will need to follow the path dependency to discover it. + + let git_project = git::new("dep1", |project| { + project + .file(".cargo/config.toml", "[path-bases]\nsubmodules = 'submods'") + .file( + "Cargo.toml", + r#" + [package] + + name = "dep1" + version = "0.5.0" + edition = "2015" + authors = ["carlhuda@example.com"] + + [dependencies.dep2] + + version = "0.5.0" + path = "dep2" + base = "submodules" + + [lib] + + name = "dep1" + "#, + ) + .file( + "src/dep1.rs", + r#" + extern crate dep2; + + pub fn hello() -> &'static str { + dep2::hello() + } + "#, + ) + }); + + let git_project2 = git::new("dep2", |project| { + project + .file("Cargo.toml", &basic_lib_manifest("dep2")) + .file( + "src/dep2.rs", + r#" + pub fn hello() -> &'static str { + "hello world" + } + "#, + ) + }); + + let repo = git2::Repository::open(&git_project.root()).unwrap(); + let url = git_project2.root().to_url().to_string(); + git::add_submodule(&repo, &url, Path::new("submods/dep2")); + git::commit(&repo); + + let p = project() + .file( + "Cargo.toml", + &format!( + r#" + [package] + + name = "foo" + version = "0.5.0" + edition = "2015" + authors = ["wycats@example.com"] + + [dependencies.dep1] + + version = "0.5.0" + git = '{}' + + [[bin]] + + name = "foo" + "#, + git_project.url() + ), + ) + .file( + "src/foo.rs", + &main_file(r#""{}", dep1::hello()"#, &["dep1"]), + ) + .build(); + + p.cargo("build").run(); + + assert!(p.bin("foo").is_file()); + + p.process(&p.bin("foo")) + .with_stdout_data(str![[r#" +hello world + +"#]]) + .run(); +} + #[cargo_test] fn cargo_compile_with_malformed_nested_paths() { let git_project = git::new("dep1", |project| { diff --git a/tests/testsuite/path.rs b/tests/testsuite/path.rs index 17179a520cc8..bb15d62faa8f 100644 --- a/tests/testsuite/path.rs +++ b/tests/testsuite/path.rs @@ -589,6 +589,387 @@ Caused by: .run(); } +#[cargo_test] +fn path_bases_not_stable() { + let bar = project() + .at("bar") + .file("Cargo.toml", &basic_manifest("bar", "0.5.0")) + .file("src/lib.rs", "") + .build(); + + let p = project() + .file( + ".cargo/config.toml", + &format!( + "[path-bases]\ntest = '{}'", + bar.root().parent().unwrap().display() + ), + ) + .file( + "Cargo.toml", + r#" + [package] + + name = "foo" + version = "0.5.0" + authors = ["wycats@example.com"] + + [dependencies.bar] + path = 'bar' + base = 'test' + "#, + ) + .file("src/lib.rs", "") + .build(); + + p.cargo("build") + .with_status(101) + .with_stderr_data( + "\ +[ERROR] failed to parse manifest at `[..]/foo/Cargo.toml` + +Caused by: + usage of path bases requires `-Z path-bases` +", + ) + .run(); +} + +#[cargo_test] +fn patch_with_base() { + let bar = project() + .at("bar") + .file("Cargo.toml", &basic_manifest("bar", "0.5.0")) + .file("src/lib.rs", "pub fn hello() {}") + .build(); + Package::new("bar", "0.5.0").publish(); + + let p = project() + .file( + ".cargo/config.toml", + &format!( + "[path-bases]\ntest = '{}'", + bar.root().parent().unwrap().display() + ), + ) + .file( + "Cargo.toml", + r#" + [package] + name = "foo" + version = "0.5.0" + authors = ["wycats@example.com"] + edition = "2018" + + [dependencies] + bar = "0.5.0" + + [patch.crates-io.bar] + path = 'bar' + base = 'test' + "#, + ) + .file("src/lib.rs", "use bar::hello as _;") + .build(); + + p.cargo("build -v -Zpath-bases") + .masquerade_as_nightly_cargo(&["path-bases"]) + .run(); +} + +#[cargo_test] +fn path_with_base() { + let bar = project() + .at("bar") + .file("Cargo.toml", &basic_manifest("bar", "0.5.0")) + .file("src/lib.rs", "") + .build(); + + let p = project() + .file( + ".cargo/config.toml", + &format!( + "[path-bases]\ntest = '{}'", + bar.root().parent().unwrap().display() + ), + ) + .file( + "Cargo.toml", + r#" + [package] + + name = "foo" + version = "0.5.0" + authors = ["wycats@example.com"] + + [dependencies.bar] + path = 'bar' + base = 'test' + "#, + ) + .file("src/lib.rs", "") + .build(); + + p.cargo("build -v -Zpath-bases") + .masquerade_as_nightly_cargo(&["path-bases"]) + .run(); +} + +#[cargo_test] +fn workspace_with_base() { + let bar = project() + .at("dep_with_base") + .file("Cargo.toml", &basic_manifest("dep_with_base", "0.5.0")) + .file("src/lib.rs", "") + .build(); + + let p = project() + .file( + ".cargo/config.toml", + &format!( + "[path-bases]\ntest = '{}'", + bar.root().parent().unwrap().display() + ), + ) + .file( + "Cargo.toml", + r#" + [package] + name = "parent" + version = "0.1.0" + authors = [] + + [workspace] + members = ["child"] + + [workspace.dependencies.dep_with_base] + path = 'dep_with_base' + base = 'test' + "#, + ) + .file("src/main.rs", "fn main() {}") + .file( + "child/Cargo.toml", + r#" + [package] + name = "child" + version = "0.1.0" + authors = [] + workspace = ".." + + [dependencies.dep_with_base] + workspace = true + "#, + ) + .file("child/src/main.rs", "fn main() {}"); + let p = p.build(); + + p.cargo("build -v -Zpath-bases") + .masquerade_as_nightly_cargo(&["path-bases"]) + .run(); +} + +#[cargo_test] +fn path_with_relative_base() { + project() + .at("shared_proj/bar") + .file("Cargo.toml", &basic_manifest("bar", "0.5.0")) + .file("src/lib.rs", "") + .build(); + + let p = project() + .file( + "../.cargo/config.toml", + "[path-bases]\ntest = 'shared_proj'", + ) + .file( + "Cargo.toml", + r#" + [package] + + name = "foo" + version = "0.5.0" + authors = ["wycats@example.com"] + + [dependencies.bar] + path = 'bar' + base = 'test' + "#, + ) + .file("src/lib.rs", "") + .build(); + + p.cargo("build -v -Zpath-bases") + .masquerade_as_nightly_cargo(&["path-bases"]) + .run(); +} + +#[cargo_test] +fn workspace_builtin_base() { + project() + .at("dep_with_base") + .file("Cargo.toml", &basic_manifest("dep_with_base", "0.5.0")) + .file("src/lib.rs", "") + .build(); + + let p = project() + .file( + "Cargo.toml", + r#" + [package] + name = "parent" + version = "0.1.0" + authors = [] + + [workspace] + members = ["child"] + "#, + ) + .file("src/main.rs", "fn main() {}") + .file( + "child/Cargo.toml", + r#" + [package] + name = "child" + version = "0.1.0" + authors = [] + workspace = ".." + + [dependencies.dep_with_base] + path = '../dep_with_base' + base = 'workspace' + "#, + ) + .file("child/src/main.rs", "fn main() {}"); + let p = p.build(); + + p.cargo("build -v -Zpath-bases") + .masquerade_as_nightly_cargo(&["path-bases"]) + .run(); +} + +#[cargo_test] +fn shadow_workspace_builtin_base() { + let bar = project() + .at("dep_with_base") + .file("Cargo.toml", &basic_manifest("dep_with_base", "0.5.0")) + .file("src/lib.rs", "") + .build(); + + let p = project() + .file( + ".cargo/config.toml", + &format!( + "[path-bases]\nworkspace = '{}/subdir'", + bar.root().parent().unwrap().display() + ), + ) + .file( + "Cargo.toml", + r#" + [package] + name = "parent" + version = "0.1.0" + authors = [] + + [workspace] + members = ["child"] + + [workspace.dependencies.dep_with_base] + path = '../dep_with_base' + base = 'workspace' + "#, + ) + .file("src/main.rs", "fn main() {}") + .file( + "child/Cargo.toml", + r#" + [package] + name = "child" + version = "0.1.0" + authors = [] + workspace = ".." + + [dependencies.dep_with_base] + workspace = true + "#, + ) + .file("child/src/main.rs", "fn main() {}"); + let p = p.build(); + + p.cargo("build -v -Zpath-bases") + .masquerade_as_nightly_cargo(&["path-bases"]) + .run(); +} + +#[cargo_test] +fn unknown_base() { + let p = project() + .file( + "Cargo.toml", + r#" + [package] + + name = "foo" + version = "0.5.0" + authors = ["wycats@example.com"] + + [dependencies.bar] + path = 'bar' + base = 'test' + "#, + ) + .file("src/lib.rs", "") + .build(); + + p.cargo("build -Zpath-bases") + .masquerade_as_nightly_cargo(&["path-bases"]) + .with_status(101) + .with_stderr_data( + "\ +[ERROR] failed to parse manifest at `[..]/foo/Cargo.toml` + +Caused by: + dependency (bar) uses an undefined path base `test`. You must add an entry for `test` in the Cargo configuration [path-bases] table. +", + ) + .run(); +} + +#[cargo_test] +fn workspace_builtin_base_not_a_workspace() { + let p = project() + .file( + "Cargo.toml", + r#" + [package] + + name = "foo" + version = "0.5.0" + authors = ["wycats@example.com"] + + [dependencies.bar] + path = 'bar' + base = 'workspace' + "#, + ) + .file("src/lib.rs", "") + .build(); + + p.cargo("build -Zpath-bases") + .masquerade_as_nightly_cargo(&["path-bases"]) + .with_status(101) + .with_stderr_data( + "\ +[ERROR] failed to parse manifest at `[..]/foo/Cargo.toml` + +Caused by: + dependency (bar) is using the `workspace` built-in path base outside of a workspace. +", + ) + .run(); +} + #[cargo_test] fn override_relative() { let bar = project()