Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

add a cache for discovered workspace roots #10776

Merged
merged 2 commits into from
Jul 5, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
123 changes: 71 additions & 52 deletions src/cargo/core/workspace.rs
Original file line number Diff line number Diff line change
Expand Up @@ -133,6 +133,34 @@ impl WorkspaceConfig {
WorkspaceConfig::Member { .. } => None,
}
}

/// Returns the path of the workspace root based on this `[workspace]` configuration.
///
/// Returns `None` if the root is not explicitly known.
///
/// * `self_path` is the path of the manifest this `WorkspaceConfig` is located.
/// * `look_from` is the path where discovery started (usually the current
/// working directory), used for `workspace.exclude` checking.
fn get_ws_root(&self, self_path: &Path, look_from: &Path) -> Option<PathBuf> {
match self {
WorkspaceConfig::Root(ances_root_config) => {
debug!("find_root - found a root checking exclusion");
if !ances_root_config.is_excluded(look_from) {
debug!("find_root - found!");
Some(self_path.to_owned())
} else {
None
}
}
WorkspaceConfig::Member {
root: Some(path_to_root),
} => {
debug!("find_root - found pointer");
Some(read_root_pointer(self_path, path_to_root))
}
WorkspaceConfig::Member { .. } => None,
}
}
}

/// Intermediate configuration of a workspace root in a manifest.
Expand Down Expand Up @@ -592,40 +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 } => {}
}
}

for ances_manifest_path in find_root_iter(manifest_path, self.config) {
debug!("find_root - trying {}", ances_manifest_path.display());
match *self.packages.load(&ances_manifest_path)?.workspace_config() {
WorkspaceConfig::Root(ref ances_root_config) => {
debug!("find_root - found a root checking exclusion");
if !ances_root_config.is_excluded(manifest_path) {
debug!("find_root - found!");
return Ok(Some(ances_manifest_path));
}
}
WorkspaceConfig::Member {
root: Some(ref path_to_root),
} => {
debug!("find_root - found pointer");
return Ok(Some(read_root_pointer(&ances_manifest_path, path_to_root)));
}
WorkspaceConfig::Member { .. } => {}
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))
}),
}
Ok(None)
}

/// After the root of a workspace has been located, probes for all members
Expand Down Expand Up @@ -1669,31 +1680,39 @@ pub fn resolve_relative_path(
}
}

fn parse_manifest(manifest_path: &Path, config: &Config) -> CargoResult<EitherManifest> {
let key = manifest_path.parent().unwrap();
let source_id = SourceId::for_path(key)?;
let (manifest, _nested_paths) = read_manifest(manifest_path, source_id, config)?;
Ok(manifest)
/// Finds the path of the root of the workspace.
pub fn find_workspace_root(manifest_path: &Path, config: &Config) -> CargoResult<Option<PathBuf>> {
find_workspace_root_with_loader(manifest_path, config, |self_path| {
let key = self_path.parent().unwrap();
let source_id = SourceId::for_path(key)?;
let (manifest, _nested_paths) = read_manifest(self_path, source_id, config)?;
Ok(manifest
.workspace_config()
.get_ws_root(self_path, manifest_path))
})
}

pub fn find_workspace_root(manifest_path: &Path, config: &Config) -> CargoResult<Option<PathBuf>> {
/// Finds the path of the root of the workspace.
///
/// This uses a callback to determine if the given path tells us what the
/// workspace root is.
fn find_workspace_root_with_loader(
manifest_path: &Path,
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());
match *parse_manifest(&ances_manifest_path, config)?.workspace_config() {
WorkspaceConfig::Root(ref ances_root_config) => {
debug!("find_root - found a root checking exclusion");
if !ances_root_config.is_excluded(manifest_path) {
debug!("find_root - found!");
return Ok(Some(ances_manifest_path));
}
}
WorkspaceConfig::Member {
root: Some(ref path_to_root),
} => {
debug!("find_root - found pointer");
return Ok(Some(read_root_pointer(&ances_manifest_path, path_to_root)));
}
WorkspaceConfig::Member { .. } => {}
if let Some(ws_root_path) = loader(&ances_manifest_path)? {
return Ok(Some(ws_root_path));
}
}
Ok(None)
Expand Down
5 changes: 4 additions & 1 deletion src/cargo/util/config/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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()),
}
}

Expand Down
72 changes: 48 additions & 24 deletions src/cargo/util/toml/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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(),
Expand Down Expand Up @@ -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]");
Expand Down Expand Up @@ -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()
),
}
}
Expand Down