add a cache for discovered workspace roots

Author: Muscraft

src/cargo/core/workspace.rs

@@ -620,27 +620,23 @@ impl<'cfg> Workspace<'cfg> {
     /// Returns an error if `manifest_path` isn't actually a valid manifest or
     /// if some other transient error happens.
     fn find_root(&mut self, manifest_path: &Path) -> CargoResult<Option<PathBuf>> {
+        let current = self.packages.load(manifest_path)?;
+        match current
+            .workspace_config()
+            .get_ws_root(manifest_path, manifest_path)
         {
-            let current = self.packages.load(manifest_path)?;
-            match *current.workspace_config() {
-                WorkspaceConfig::Root(_) => {
-                    debug!("find_root - is root {}", manifest_path.display());
-                    return Ok(Some(manifest_path.to_path_buf()));
-                }
-                WorkspaceConfig::Member {
-                    root: Some(ref path_to_root),
-                } => return Ok(Some(read_root_pointer(manifest_path, path_to_root))),
-                WorkspaceConfig::Member { root: None } => {}
+            Some(root_path) => {
+                debug!("find_root - is root {}", manifest_path.display());
+                Ok(Some(root_path))
             }
+            None => find_workspace_root_with_loader(manifest_path, self.config, |self_path| {
+                Ok(self
+                    .packages
+                    .load(self_path)?
+                    .workspace_config()
+                    .get_ws_root(self_path, manifest_path))
+            }),
         }
-
-        find_workspace_root_with_loader(manifest_path, self.config, |self_path| {
-            Ok(self
-                .packages
-                .load(self_path)?
-                .workspace_config()
-                .get_ws_root(self_path, manifest_path))
-        })
     }
 
     /// After the root of a workspace has been located, probes for all members
@@ -1686,8 +1682,6 @@ pub fn resolve_relative_path(
 
 /// Finds the path of the root of the workspace.
 pub fn find_workspace_root(manifest_path: &Path, config: &Config) -> CargoResult<Option<PathBuf>> {
-    // FIXME(ehuss): Loading and parsing manifests just to find the root seems
-    // very inefficient. I think this should be reconsidered.
     find_workspace_root_with_loader(manifest_path, config, |self_path| {
         let key = self_path.parent().unwrap();
         let source_id = SourceId::for_path(key)?;
@@ -1707,6 +1701,14 @@ fn find_workspace_root_with_loader(
     config: &Config,
     mut loader: impl FnMut(&Path) -> CargoResult<Option<PathBuf>>,
 ) -> CargoResult<Option<PathBuf>> {
+    // Check if there are any workspace roots that have already been found that would work
+    for (ws_root, ws_root_config) in config.ws_roots.borrow().iter() {
+        if manifest_path.starts_with(ws_root) && !ws_root_config.is_excluded(manifest_path) {
+            // Add `Cargo.toml` since ws_root is the root and not the file
+            return Ok(Some(ws_root.join("Cargo.toml").clone()));
+        }
+    }
+
     for ances_manifest_path in find_root_iter(manifest_path, config) {
         debug!("find_root - trying {}", ances_manifest_path.display());
         if let Some(ws_root_path) = loader(&ances_manifest_path)? {

src/cargo/util/config/mod.rs

@@ -68,7 +68,7 @@ use std::time::Instant;
 use self::ConfigValue as CV;
 use crate::core::compiler::rustdoc::RustdocExternMap;
 use crate::core::shell::Verbosity;
-use crate::core::{features, CliUnstable, Shell, SourceId, Workspace};
+use crate::core::{features, CliUnstable, Shell, SourceId, Workspace, WorkspaceRootConfig};
 use crate::ops;
 use crate::util::errors::CargoResult;
 use crate::util::toml as cargo_toml;
@@ -202,6 +202,8 @@ pub struct Config {
     /// NOTE: this should be set before `configure()`. If calling this from an integration test,
     /// consider using `ConfigBuilder::enable_nightly_features` instead.
     pub nightly_features_allowed: bool,
+    /// WorkspaceRootConfigs that have been found
+    pub ws_roots: RefCell<HashMap<PathBuf, WorkspaceRootConfig>>,
 }
 
 impl Config {
@@ -285,6 +287,7 @@ impl Config {
             progress_config: ProgressConfig::default(),
             env_config: LazyCell::new(),
             nightly_features_allowed: matches!(&*features::channel(), "nightly" | "dev"),
+            ws_roots: RefCell::new(HashMap::new()),
         }
     }
 

src/cargo/util/toml/mod.rs

@@ -1549,18 +1549,23 @@ impl TomlManifest {
         let project = &mut project.ok_or_else(|| anyhow!("no `package` section found"))?;
 
         let workspace_config = match (me.workspace.as_ref(), project.workspace.as_ref()) {
-            (Some(config), None) => {
-                let mut inheritable = config.package.clone().unwrap_or_default();
+            (Some(toml_config), None) => {
+                let mut inheritable = toml_config.package.clone().unwrap_or_default();
                 inheritable.update_ws_path(package_root.to_path_buf());
-                inheritable.update_deps(config.dependencies.clone());
-                WorkspaceConfig::Root(WorkspaceRootConfig::new(
+                inheritable.update_deps(toml_config.dependencies.clone());
+                let ws_root_config = WorkspaceRootConfig::new(
                     package_root,
-                    &config.members,
-                    &config.default_members,
-                    &config.exclude,
+                    &toml_config.members,
+                    &toml_config.default_members,
+                    &toml_config.exclude,
                     &Some(inheritable),
-                    &config.metadata,
-                ))
+                    &toml_config.metadata,
+                );
+                config
+                    .ws_roots
+                    .borrow_mut()
+                    .insert(package_root.to_path_buf(), ws_root_config.clone());
+                WorkspaceConfig::Root(ws_root_config)
             }
             (None, root) => WorkspaceConfig::Member {
                 root: root.cloned(),
@@ -2206,18 +2211,23 @@ impl TomlManifest {
             .map(|r| ResolveBehavior::from_manifest(r))
             .transpose()?;
         let workspace_config = match me.workspace {
-            Some(ref config) => {
-                let mut inheritable = config.package.clone().unwrap_or_default();
+            Some(ref toml_config) => {
+                let mut inheritable = toml_config.package.clone().unwrap_or_default();
                 inheritable.update_ws_path(root.to_path_buf());
-                inheritable.update_deps(config.dependencies.clone());
-                WorkspaceConfig::Root(WorkspaceRootConfig::new(
+                inheritable.update_deps(toml_config.dependencies.clone());
+                let ws_root_config = WorkspaceRootConfig::new(
                     root,
-                    &config.members,
-                    &config.default_members,
-                    &config.exclude,
+                    &toml_config.members,
+                    &toml_config.default_members,
+                    &toml_config.exclude,
                     &Some(inheritable),
-                    &config.metadata,
-                ))
+                    &toml_config.metadata,
+                );
+                config
+                    .ws_roots
+                    .borrow_mut()
+                    .insert(root.to_path_buf(), ws_root_config.clone());
+                WorkspaceConfig::Root(ws_root_config)
             }
             None => {
                 bail!("virtual manifests must be configured with [workspace]");
@@ -2334,16 +2344,30 @@ impl TomlManifest {
 
 fn inheritable_from_path(
     config: &Config,
-    resolved_path: PathBuf,
+    workspace_path: PathBuf,
 ) -> CargoResult<InheritableFields> {
-    let key = resolved_path.parent().unwrap();
-    let source_id = SourceId::for_path(key)?;
-    let (man, _) = read_manifest(&resolved_path, source_id, config)?;
+    // Workspace path should have Cargo.toml at the end
+    let workspace_path_root = workspace_path.parent().unwrap();
+
+    // Let the borrow exit scope so that it can be picked up if there is a need to
+    // read a manifest
+    if let Some(ws_root) = config.ws_roots.borrow().get(workspace_path_root) {
+        return Ok(ws_root.inheritable().clone());
+    };
+
+    let source_id = SourceId::for_path(workspace_path_root)?;
+    let (man, _) = read_manifest(&workspace_path, source_id, config)?;
     match man.workspace_config() {
-        WorkspaceConfig::Root(root) => Ok(root.inheritable().clone()),
+        WorkspaceConfig::Root(root) => {
+            config
+                .ws_roots
+                .borrow_mut()
+                .insert(workspace_path, root.clone());
+            Ok(root.inheritable().clone())
+        }
         _ => bail!(
             "root of a workspace inferred but wasn't a root: {}",
-            resolved_path.display()
+            workspace_path.display()
         ),
     }
 }