Implement base paths (RFC 3529) 2/n: support for nested paths

Author: dpaoliello

src/cargo/sources/path.rs

@@ -1,11 +1,14 @@
+use std::borrow::Cow;
 use std::collections::{HashMap, HashSet};
 use std::fmt::{self, Debug, Formatter};
 use std::fs;
 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<PathBuf> {
+fn nested_paths(manifest: &Manifest) -> Vec<(PathBuf, Option<PathBaseName>)> {
     let mut nested_paths = Vec::new();
     let normalized = manifest.normalized_toml();
     let dependencies = normalized
@@ -910,7 +915,7 @@ fn nested_paths(manifest: &Manifest) -> Vec<PathBuf> {
             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<PathBuf> = 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.

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<String, BTreeMap<PackageName, TomlDependency>>>,
-    workspace_root: &dyn Fn() -> CargoResult<&'a PathBuf>,
+    workspace_root: &dyn Fn() -> CargoResult<&'a Path>,
     features: &Features,
 ) -> CargoResult<Option<BTreeMap<String, BTreeMap<PackageName, TomlDependency>>>> {
     if let Some(patch) = original_patch {
@@ -757,7 +757,7 @@ fn normalize_dependencies<'a>(
     activated_opt_deps: &HashSet<&str>,
     kind: Option<DepKind>,
     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<String>,
 ) -> CargoResult<Option<BTreeMap<manifest::PackageName, manifest::InheritableDependency>>> {
@@ -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<P: ResolveToPath + Clone>(
 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<PathBuf> {
-    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."

src/doc/src/reference/unstable.md

@@ -1611,7 +1611,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.
@@ -1625,6 +1625,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.
+
 # Stabilized and removed features
 
 ## Compile progress

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 = ["[email protected]"]
+
+                    [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 = ["[email protected]"]
+
+                    [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| {