From f899cc2a833206dff4f7d9f6eb9b7f815ee19239 Mon Sep 17 00:00:00 2001 From: Daniel Paoliello Date: Fri, 16 Aug 2024 15:14:17 -0700 Subject: [PATCH] Implement base paths (RFC 3529) 2/n: support for nested paths --- src/cargo/sources/path.rs | 42 ++++++++++-- src/cargo/util/toml/mod.rs | 21 +++--- src/doc/src/reference/unstable.md | 8 ++- tests/testsuite/git.rs | 105 ++++++++++++++++++++++++++++++ 4 files changed, 159 insertions(+), 17 deletions(-) diff --git a/src/cargo/sources/path.rs b/src/cargo/sources/path.rs index d4acb1d2095..6d27b4f8abc 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; @@ -5,7 +6,9 @@ use std::io; use std::path::{Path, PathBuf}; use std::task::Poll; -use crate::core::{Dependency, EitherManifest, Manifest, Package, PackageId, SourceId}; +use crate::core::{ + find_workspace_root, Dependency, EitherManifest, Manifest, Package, PackageId, SourceId, +}; use crate::ops; use crate::sources::source::MaybePackage; use crate::sources::source::QueryKind; @@ -14,15 +17,17 @@ 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 anyhow::{anyhow, Context as _}; use cargo_util::paths; +use cargo_util_schemas::manifest::PathBaseName; use filetime::FileTime; use gix::bstr::{BString, ByteVec}; use gix::dir::entry::Status; use gix::index::entry::Stage; use ignore::gitignore::GitignoreBuilder; +use lazycell::LazyCell; use tracing::{debug, info, trace, warn}; use walkdir::WalkDir; @@ -878,7 +883,7 @@ fn read_packages( } } -fn nested_paths(manifest: &Manifest) -> Vec { +fn nested_paths(manifest: &Manifest) -> Vec<(PathBuf, Option)> { let mut nested_paths = Vec::new(); let normalized = manifest.normalized_toml(); let dependencies = normalized @@ -910,7 +915,7 @@ 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((PathBuf::from(path.as_str()), dep.base.clone())); } } nested_paths @@ -1000,8 +1005,31 @@ 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 workspace_root_cell: LazyCell = LazyCell::new(); + + for (p, base) in nested.iter() { + let p = if let Some(base) = base { + let workspace_root = || { + workspace_root_cell + .try_borrow_with(|| { + find_workspace_root(&manifest_path, gctx)? + .ok_or_else(|| anyhow!("failed to find a workspace root")) + }) + .map(|p| p.as_path()) + }; + // Pass in `None` for the `cargo-features` not to skip verification: when the + // package is loaded as a dependency, then it will be checked. + match lookup_path_base(base, gctx, &workspace_root, None) { + 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. diff --git a/src/cargo/util/toml/mod.rs b/src/cargo/util/toml/mod.rs index 44b3b6812c8..0bd4cd24184 100644 --- a/src/cargo/util/toml/mod.rs +++ b/src/cargo/util/toml/mod.rs @@ -312,7 +312,7 @@ fn normalize_toml( inherit_cell .try_borrow_with(|| load_inheritable_fields(gctx, manifest_file, &workspace_config)) }; - let workspace_root = || inherit().map(|fields| fields.ws_root()); + let workspace_root = || inherit().map(|fields| fields.ws_root().as_path()); if let Some(original_package) = original_toml.package() { let package_name = &original_package.name; @@ -538,7 +538,7 @@ fn normalize_toml( fn normalize_patch<'a>( gctx: &GlobalContext, original_patch: Option<&BTreeMap>>, - workspace_root: &dyn Fn() -> CargoResult<&'a PathBuf>, + workspace_root: &dyn Fn() -> CargoResult<&'a Path>, features: &Features, ) -> CargoResult>>> { if let Some(patch) = original_patch { @@ -757,7 +757,7 @@ fn normalize_dependencies<'a>( activated_opt_deps: &HashSet<&str>, kind: Option, inherit: &dyn Fn() -> CargoResult<&'a InheritableFields>, - workspace_root: &dyn Fn() -> CargoResult<&'a PathBuf>, + workspace_root: &dyn Fn() -> CargoResult<&'a Path>, package_root: &Path, warnings: &mut Vec, ) -> CargoResult>> { @@ -839,12 +839,13 @@ fn normalize_dependencies<'a>( fn normalize_path_dependency<'a>( gctx: &GlobalContext, detailed_dep: &mut TomlDetailedDependency, - workspace_root: &dyn Fn() -> CargoResult<&'a PathBuf>, + workspace_root: &dyn Fn() -> CargoResult<&'a Path>, features: &Features, ) -> CargoResult<()> { if let Some(base) = detailed_dep.base.take() { if let Some(path) = detailed_dep.path.as_mut() { - let new_path = lookup_path_base(&base, gctx, workspace_root, features)?.join(&path); + let new_path = + lookup_path_base(&base, gctx, workspace_root, Some(features))?.join(&path); *path = new_path.to_str().unwrap().to_string(); } else { bail!("`base` can only be used with path dependencies"); @@ -2225,10 +2226,12 @@ fn to_dependency_source_id( pub(crate) fn lookup_path_base<'a>( base: &PathBaseName, gctx: &GlobalContext, - workspace_root: &dyn Fn() -> CargoResult<&'a PathBuf>, - features: &Features, + workspace_root: &dyn Fn() -> CargoResult<&'a Path>, + features: Option<&Features>, ) -> CargoResult { - features.require(Feature::path_bases())?; + if let Some(features) = features { + features.require(Feature::path_bases())?; + } // HACK: The `base` string is user controlled, but building the path is safe from injection // attacks since the `PathBaseName` type restricts the characters that can be used to exclude `.` @@ -2240,7 +2243,7 @@ pub(crate) fn lookup_path_base<'a>( } else { // Otherwise, check the built-in bases. match base.as_str() { - "workspace" => Ok(workspace_root()?.clone()), + "workspace" => Ok(workspace_root()?.to_path_buf()), _ => bail!( "path base `{base}` is undefined. \ You must add an entry for `{base}` in the Cargo configuration [path-bases] table." diff --git a/src/doc/src/reference/unstable.md b/src/doc/src/reference/unstable.md index ce7f2cb6dd2..4e1646bcad4 100644 --- a/src/doc/src/reference/unstable.md +++ b/src/doc/src/reference/unstable.md @@ -1612,7 +1612,7 @@ character, and must not be empty. If the name of path base used in a dependency is neither in the configuration nor one of the built-in path base, then Cargo will raise an error. -#### Built-in path bases +### Built-in path bases Cargo provides implicit path bases that can be used without the need to specify them in a `[path-bases]` table. @@ -1626,6 +1626,12 @@ will prefer the value in the configuration. The allows Cargo to add new built-in path bases without compatibility issues (as existing uses will shadow the built-in name). +### Path bases in git dependencies and patches + +Configuration files in git dependencies and patches are not loaded by Cargo and +so any path bases used in those packages will need to be defined in some +configuration that is loaded by Cargo. + ## lockfile-path * Original Issue: [#5707](https://github.com/rust-lang/cargo/issues/5707) * Tracking Issue: [#14421](https://github.com/rust-lang/cargo/issues/14421) diff --git a/tests/testsuite/git.rs b/tests/testsuite/git.rs index 21943c3e29c..986d7701263 100644 --- a/tests/testsuite/git.rs +++ b/tests/testsuite/git.rs @@ -17,6 +17,8 @@ use cargo_test_support::registry::Package; use cargo_test_support::{basic_lib_manifest, basic_manifest, git, main_file, project}; use cargo_test_support::{sleep_ms, str, t, Project}; +use crate::config::write_config_at; + #[cargo_test] fn cargo_compile_simple_git_dep() { let project = project(); @@ -380,6 +382,109 @@ 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.toml", + r#" + cargo-features = ["path-bases"] + + [package] + name = "dep1" + version = "0.5.0" + edition = "2015" + authors = ["carlhuda@example.com"] + + [dependencies] + dep2 = { version = "0.5.0", base = "submodules", path = "dep2" } + + [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); + + write_config_at( + paths::home().join(".cargo/config.toml"), + r#" + [path-bases] + submodules = '../dep1/submods' + "#, + ); + + 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") + .masquerade_as_nightly_cargo(&["path-bases"]) + .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| {