Skip to content

Commit bbd41a4

Browse files
committed
Rework how workspace roots are discovered.
This centralizes the workspace discovery code into one location `find_workspace_root_with_loader` so that it isn't duplicated.
1 parent 2189462 commit bbd41a4

File tree

1 file changed

+58
-41
lines changed

1 file changed

+58
-41
lines changed

src/cargo/core/workspace.rs

Lines changed: 58 additions & 41 deletions
Original file line numberDiff line numberDiff line change
@@ -133,6 +133,34 @@ impl WorkspaceConfig {
133133
WorkspaceConfig::Member { .. } => None,
134134
}
135135
}
136+
137+
/// Returns the path of the workspace root based on this `[workspace]` configuration.
138+
///
139+
/// Returns `None` if the root is not explicitly known.
140+
///
141+
/// * `self_path` is the path of the manifest this `WorkspaceConfig` is located.
142+
/// * `look_from` is the path where discovery started (usually the current
143+
/// working directory), used for `workspace.exclude` checking.
144+
fn get_ws_root(&self, self_path: &Path, look_from: &Path) -> Option<PathBuf> {
145+
match self {
146+
WorkspaceConfig::Root(ances_root_config) => {
147+
debug!("find_root - found a root checking exclusion");
148+
if !ances_root_config.is_excluded(look_from) {
149+
debug!("find_root - found!");
150+
Some(self_path.to_owned())
151+
} else {
152+
None
153+
}
154+
}
155+
WorkspaceConfig::Member {
156+
root: Some(path_to_root),
157+
} => {
158+
debug!("find_root - found pointer");
159+
Some(read_root_pointer(self_path, path_to_root))
160+
}
161+
WorkspaceConfig::Member { .. } => None,
162+
}
163+
}
136164
}
137165

138166
/// Intermediate configuration of a workspace root in a manifest.
@@ -606,26 +634,13 @@ impl<'cfg> Workspace<'cfg> {
606634
}
607635
}
608636

609-
for ances_manifest_path in find_root_iter(manifest_path, self.config) {
610-
debug!("find_root - trying {}", ances_manifest_path.display());
611-
match *self.packages.load(&ances_manifest_path)?.workspace_config() {
612-
WorkspaceConfig::Root(ref ances_root_config) => {
613-
debug!("find_root - found a root checking exclusion");
614-
if !ances_root_config.is_excluded(manifest_path) {
615-
debug!("find_root - found!");
616-
return Ok(Some(ances_manifest_path));
617-
}
618-
}
619-
WorkspaceConfig::Member {
620-
root: Some(ref path_to_root),
621-
} => {
622-
debug!("find_root - found pointer");
623-
return Ok(Some(read_root_pointer(&ances_manifest_path, path_to_root)));
624-
}
625-
WorkspaceConfig::Member { .. } => {}
626-
}
627-
}
628-
Ok(None)
637+
find_workspace_root_with_loader(manifest_path, self.config, |self_path| {
638+
Ok(self
639+
.packages
640+
.load(self_path)?
641+
.workspace_config()
642+
.get_ws_root(self_path, manifest_path))
643+
})
629644
}
630645

631646
/// After the root of a workspace has been located, probes for all members
@@ -1669,31 +1684,33 @@ pub fn resolve_relative_path(
16691684
}
16701685
}
16711686

1672-
fn parse_manifest(manifest_path: &Path, config: &Config) -> CargoResult<EitherManifest> {
1673-
let key = manifest_path.parent().unwrap();
1674-
let source_id = SourceId::for_path(key)?;
1675-
let (manifest, _nested_paths) = read_manifest(manifest_path, source_id, config)?;
1676-
Ok(manifest)
1687+
/// Finds the path of the root of the workspace.
1688+
pub fn find_workspace_root(manifest_path: &Path, config: &Config) -> CargoResult<Option<PathBuf>> {
1689+
// FIXME(ehuss): Loading and parsing manifests just to find the root seems
1690+
// very inefficient. I think this should be reconsidered.
1691+
find_workspace_root_with_loader(manifest_path, config, |self_path| {
1692+
let key = self_path.parent().unwrap();
1693+
let source_id = SourceId::for_path(key)?;
1694+
let (manifest, _nested_paths) = read_manifest(self_path, source_id, config)?;
1695+
Ok(manifest
1696+
.workspace_config()
1697+
.get_ws_root(self_path, manifest_path))
1698+
})
16771699
}
16781700

1679-
pub fn find_workspace_root(manifest_path: &Path, config: &Config) -> CargoResult<Option<PathBuf>> {
1701+
/// Finds the path of the root of the workspace.
1702+
///
1703+
/// This uses a callback to determine if the given path tells us what the
1704+
/// workspace root is.
1705+
fn find_workspace_root_with_loader(
1706+
manifest_path: &Path,
1707+
config: &Config,
1708+
mut loader: impl FnMut(&Path) -> CargoResult<Option<PathBuf>>,
1709+
) -> CargoResult<Option<PathBuf>> {
16801710
for ances_manifest_path in find_root_iter(manifest_path, config) {
16811711
debug!("find_root - trying {}", ances_manifest_path.display());
1682-
match *parse_manifest(&ances_manifest_path, config)?.workspace_config() {
1683-
WorkspaceConfig::Root(ref ances_root_config) => {
1684-
debug!("find_root - found a root checking exclusion");
1685-
if !ances_root_config.is_excluded(manifest_path) {
1686-
debug!("find_root - found!");
1687-
return Ok(Some(ances_manifest_path));
1688-
}
1689-
}
1690-
WorkspaceConfig::Member {
1691-
root: Some(ref path_to_root),
1692-
} => {
1693-
debug!("find_root - found pointer");
1694-
return Ok(Some(read_root_pointer(&ances_manifest_path, path_to_root)));
1695-
}
1696-
WorkspaceConfig::Member { .. } => {}
1712+
if let Some(ws_root_path) = loader(&ances_manifest_path)? {
1713+
return Ok(Some(ws_root_path));
16971714
}
16981715
}
16991716
Ok(None)

0 commit comments

Comments
 (0)