Skip to content
Open
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
38 changes: 21 additions & 17 deletions dev-tools/ls-apis/src/cargo.rs
Original file line number Diff line number Diff line change
Expand Up @@ -234,23 +234,16 @@ impl Workspace {
Ok(path)
}

/// Iterate over the required dependencies of package `root`, invoking
/// `func` for each one as:
/// Walks the required (normal and build) dependencies of package `root`.
///
/// ```ignore
/// func(package: &Package, dep_path: &DepPath)
/// ```
///
/// where `package` is the package that is (directly or indirectly) a
/// dependency of `root` and `dep_path` describes the dependency path from
/// `root` to `package`.
pub fn walk_required_deps_recursively(
&self,
/// Returns a [`WalkOutcome`] describing every package reachable from
/// `root`, each paired with a dependency path to it.
pub fn walk_required_deps_recursively<'a>(
&'a self,
root: &Package,
func: &mut dyn FnMut(&Package, &DepPath),
) -> Result<()> {
struct Remaining<'a> {
node: &'a cargo_metadata::Node,
) -> Result<WalkOutcome<'a>> {
struct Remaining<'n> {
node: &'n cargo_metadata::Node,
path: DepPath,
}

Expand All @@ -268,6 +261,7 @@ impl Workspace {
path: DepPath::for_pkg(root.id.clone()),
}];
let mut seen: BTreeSet<PackageId> = BTreeSet::new();
let mut found: Vec<(&'a Package, DepPath)> = Vec::new();

while let Some(Remaining { node: next, path }) = remaining.pop() {
for d in &next.deps {
Expand All @@ -288,7 +282,7 @@ impl Workspace {
// package metadata.
let dep_pkg = self.packages_by_id.get(did).unwrap();
let dep_node = self.nodes_by_id.get(did).unwrap();
func(dep_pkg, &path);
found.push((dep_pkg, path.clone()));
if seen.contains(did) {
continue;
}
Expand All @@ -299,7 +293,7 @@ impl Workspace {
}
}

Ok(())
Ok(WalkOutcome { found })
}

/// Return all package ids for the given `pkgname`
Expand Down Expand Up @@ -343,6 +337,16 @@ fn cargo_toml_parent(
Ok(path)
}

/// The result of [`Workspace::walk_required_deps_recursively`].
pub struct WalkOutcome<'a> {
/// Every package encountered as a required (normal or build) dependency of
/// the walk's `root`, each paired with a dependency path from `root` to
/// that package.
///
/// A package reachable by more than one path appears once per path.
pub found: Vec<(&'a Package, DepPath)>,
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should I make this something like a BTreeMap<&'a Package, Vec<DepPath>>? I didn't feel like it made much of a difference either way.

}

/// Describes a "dependency path": a path through the Cargo dependency graph
/// from one package to another, which describes how one package depends on
/// another
Expand Down
32 changes: 14 additions & 18 deletions dev-tools/ls-apis/src/system_apis.rs
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,6 @@ use crate::workspaces::Workspaces;
use anyhow::Result;
use anyhow::{Context, anyhow, bail};
use camino::Utf8PathBuf;
use cargo_metadata::Package;
use iddqd::IdOrdItem;
use iddqd::IdOrdMap;
use iddqd::id_upcast;
Expand Down Expand Up @@ -175,12 +174,11 @@ impl SystemApis {
let dep_path = DepPath::for_pkg(server_pkg.id.clone());
tracker.found_package(dunit_pkg, dunit_pkg, &dep_path);

workspace.walk_required_deps_recursively(
server_pkg,
&mut |p: &Package, dep_path: &DepPath| {
tracker.found_package(dunit_pkg, &p.name, dep_path);
},
)?;
let outcome =
workspace.walk_required_deps_recursively(server_pkg)?;
for (dep_pkg, dep_path) in &outcome.found {
tracker.found_package(dunit_pkg, &dep_pkg.name, dep_path);
}
}
}

Expand Down Expand Up @@ -219,24 +217,22 @@ impl SystemApis {
for server_pkgname in server_component_units.keys() {
let (workspace, pkg) =
workspaces.find_package_workspace(server_pkgname)?;
workspace
.walk_required_deps_recursively(
pkg,
&mut |p: &Package, dep_path: &DepPath| {
deps_tracker.found_dependency(
server_pkgname,
&p.name,
dep_path,
);
},
)
let outcome = workspace
.walk_required_deps_recursively(pkg)
.with_context(|| {
format!(
"iterating dependencies of workspace {:?} package {:?}",
workspace.name(),
server_pkgname
)
})?;
for (dep_pkg, dep_path) in &outcome.found {
deps_tracker.found_dependency(
server_pkgname,
&dep_pkg.name,
dep_path,
);
}
}

let (apis_consumed, api_consumers) =
Expand Down
Loading