diff --git a/dev-tools/ls-apis/src/cargo.rs b/dev-tools/ls-apis/src/cargo.rs index eeb2316d820..9f2db80b638 100644 --- a/dev-tools/ls-apis/src/cargo.rs +++ b/dev-tools/ls-apis/src/cargo.rs @@ -11,6 +11,9 @@ use camino::Utf8Path; use camino::Utf8PathBuf; use cargo_metadata::{CargoOpt, Package}; use cargo_metadata::{DependencyKind, PackageId}; +use iddqd::IdOrdItem; +use iddqd::IdOrdMap; +use iddqd::id_upcast; use std::collections::BTreeSet; use std::collections::{BTreeMap, VecDeque}; @@ -234,23 +237,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 every dependency path from `root` 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> { + struct Remaining<'n> { + node: &'n cargo_metadata::Node, path: DepPath, } @@ -268,6 +264,7 @@ impl Workspace { path: DepPath::for_pkg(root.id.clone()), }]; let mut seen: BTreeSet = BTreeSet::new(); + let mut found: IdOrdMap> = IdOrdMap::new(); while let Some(Remaining { node: next, path }) = remaining.pop() { for d in &next.deps { @@ -288,7 +285,14 @@ 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 + .entry(&dep_pkg.id) + .or_insert_with(|| PackageWalkOutcome { + package: dep_pkg, + dep_paths: Vec::new(), + }) + .dep_paths + .push(path.clone()); if seen.contains(did) { continue; } @@ -299,7 +303,7 @@ impl Workspace { } } - Ok(()) + Ok(WalkOutcome { found }) } /// Return all package ids for the given `pkgname` @@ -343,6 +347,38 @@ 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 every dependency path from `root` + /// to that package. + pub found: IdOrdMap>, +} + +/// A single entry in [`WalkOutcome::found`]: one package and every dependency +/// path from the walk's `root` to it. +#[derive(Debug)] +pub struct PackageWalkOutcome<'a> { + /// The package that was found during the walk. + pub package: &'a Package, + + /// The list of dependency paths from the walk's `root` to this package. + /// + /// A package reachable by more than one path appears once per path. + pub dep_paths: Vec, +} + +impl<'a> IdOrdItem for PackageWalkOutcome<'a> { + type Key<'b> + = &'a PackageId + where + Self: 'b; + fn key(&self) -> Self::Key<'_> { + &self.package.id + } + id_upcast!(); +} + /// Describes a "dependency path": a path through the Cargo dependency graph /// from one package to another, which describes how one package depends on /// another diff --git a/dev-tools/ls-apis/src/system_apis.rs b/dev-tools/ls-apis/src/system_apis.rs index 01e618c84de..f8729946cbb 100644 --- a/dev-tools/ls-apis/src/system_apis.rs +++ b/dev-tools/ls-apis/src/system_apis.rs @@ -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; @@ -173,14 +172,21 @@ impl SystemApis { let (workspace, server_pkg) = workspaces.find_package_workspace(dunit_pkg)?; let dep_path = DepPath::for_pkg(server_pkg.id.clone()); - tracker.found_package(dunit_pkg, dunit_pkg, &dep_path); + tracker.found_package( + dunit_pkg, + dunit_pkg, + std::slice::from_ref(&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 pkg_outcome in &outcome.found { + tracker.found_package( + dunit_pkg, + &pkg_outcome.package.name, + &pkg_outcome.dep_paths, + ); + } } } @@ -219,17 +225,8 @@ 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 {:?}", @@ -237,6 +234,13 @@ impl SystemApis { server_pkgname ) })?; + for pkg_outcome in &outcome.found { + deps_tracker.found_dependency( + server_pkgname, + &pkg_outcome.package.name, + &pkg_outcome.dep_paths, + ); + } } let (apis_consumed, api_consumers) = @@ -1414,7 +1418,7 @@ impl<'a> ServerComponentsTracker<'a> { } /// Record that deployment unit package `dunit_pkgname` depends on package - /// `pkgname` via dependency chain `dep_path` + /// `pkgname` via each of the given dependency chains `dep_paths` /// /// This only records anything if `pkgname` turns out to be a known API /// client package name, in which case this records that the server @@ -1423,14 +1427,16 @@ impl<'a> ServerComponentsTracker<'a> { &mut self, dunit_pkgname: &ServerComponentName, pkgname: &str, - dep_path: &DepPath, + dep_paths: &[DepPath], ) { let Some(apis) = self.known_server_packages.get(pkgname) else { return; }; - for api in apis { - self.found_api_producer(api, dunit_pkgname, dep_path); + for dep_path in dep_paths { + for api in apis { + self.found_api_producer(api, dunit_pkgname, dep_path); + } } } @@ -1488,7 +1494,7 @@ impl<'a> ClientDependenciesTracker<'a> { } /// Record that comopnent `server_pkgname` consumes package `pkgname` via - /// dependency chain `dep_path` + /// each of the given dependency chains `dep_paths` /// /// This only records cases where `pkgname` is a known client package for /// one of our APIs, in which case it records that this server component @@ -1497,7 +1503,7 @@ impl<'a> ClientDependenciesTracker<'a> { &mut self, server_pkgname: &ServerComponentName, pkgname: &str, - dep_path: &DepPath, + dep_paths: &[DepPath], ) { let Some(api) = self.api_metadata.client_pkgname_lookup(pkgname) else { return; @@ -1506,18 +1512,22 @@ impl<'a> ClientDependenciesTracker<'a> { // This is the name of a known client package. Record it. let status = api.restricted_to_consumers.status(server_pkgname); let client_pkgname = ClientPackageName::from(pkgname.to_owned()); - self.api_consumers - .entry(client_pkgname.clone()) - .or_insert_with(IdOrdMap::new) - .entry(&server_pkgname) - .or_insert_with(|| ApiConsumer::new(server_pkgname.clone(), status)) - .add_path(dep_path.clone()); - self.apis_consumed - .entry(server_pkgname.clone()) - .or_insert_with(BTreeMap::new) - .entry(client_pkgname) - .or_insert_with(Vec::new) - .push(dep_path.clone()); + for dep_path in dep_paths { + self.api_consumers + .entry(client_pkgname.clone()) + .or_insert_with(IdOrdMap::new) + .entry(&server_pkgname) + .or_insert_with(|| { + ApiConsumer::new(server_pkgname.clone(), status.clone()) + }) + .add_path(dep_path.clone()); + self.apis_consumed + .entry(server_pkgname.clone()) + .or_insert_with(BTreeMap::new) + .entry(client_pkgname.clone()) + .or_insert_with(Vec::new) + .push(dep_path.clone()); + } } }