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;
 
@@ -300,7 +305,7 @@ impl<'gctx> RecursivePathSource<'gctx> {
     /// Discovers packages inside this source if it hasn't yet done.
     pub fn load(&mut self) -> CargoResult<()> {
         if !self.loaded {
-            self.packages = read_packages(&self.path, self.source_id, self.gctx)?;
+            self.packages = read_packages(&self.path, self.source_id)?;
             self.loaded = true;
         }
 
@@ -808,7 +813,6 @@ fn last_modified_file(
 fn read_packages(
     path: &Path,
     source_id: SourceId,
-    gctx: &GlobalContext,
 ) -> CargoResult<HashMap<PackageId, Vec<Package>>> {
     let mut all_packages = HashMap::new();
     let mut visited = HashSet::<PathBuf>::new();
@@ -844,14 +848,7 @@ fn read_packages(
         }
 
         if has_manifest(dir) {
-            read_nested_packages(
-                dir,
-                &mut all_packages,
-                source_id,
-                gctx,
-                &mut visited,
-                &mut errors,
-            )?;
+            read_nested_packages(dir, &mut all_packages, source_id, &mut visited, &mut errors)?;
         }
         Ok(true)
     })?;
@@ -878,7 +875,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 +907,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
@@ -952,7 +949,6 @@ fn read_nested_packages(
     path: &Path,
     all_packages: &mut HashMap<PackageId, Vec<Package>>,
     source_id: SourceId,
-    gctx: &GlobalContext,
     visited: &mut HashSet<PathBuf>,
     errors: &mut Vec<anyhow::Error>,
 ) -> CargoResult<()> {
@@ -961,8 +957,10 @@ fn read_nested_packages(
     }
 
     let manifest_path = find_project_manifest_exact(path, "Cargo.toml")?;
+    let mut manifest_gctx = GlobalContext::default()?;
+    manifest_gctx.reload_rooted_at(&manifest_path)?;
 
-    let manifest = match read_manifest(&manifest_path, source_id, gctx) {
+    let manifest = match read_manifest(&manifest_path, source_id, &manifest_gctx) {
         Err(err) => {
             // Ignore malformed manifests found on git repositories
             //
@@ -1000,10 +998,32 @@ 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 result =
-                read_nested_packages(&path, all_packages, source_id, gctx, visited, errors);
+        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, &manifest_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, &manifest_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, visited, errors);
             // Ignore broken manifests found on git repositories.
             //
             // A well formed manifest might still fail to load due to reasons

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."

tests/testsuite/git.rs

@@ -380,6 +380,108 @@ 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",
+                r#"
+                    [path-bases]
+                    submodules = 'submods'
+                "#,
+            )
+            .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);
+
+    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| {