Skip to content

Commit cf6cbf6

Browse files
authored
fix!: deduplicate discovered interface packages (#2)
Signed-off-by: Esteve Fernandez <esteve@apache.org>
1 parent ac14877 commit cf6cbf6

1 file changed

Lines changed: 104 additions & 7 deletions

File tree

build.rs

Lines changed: 104 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
use ament_rs::{search_paths::get_search_paths, AMENT_PREFIX_PATH_ENV_VAR};
22
use cargo_toml::Manifest;
3+
use std::collections::{HashMap, HashSet, VecDeque};
34
use std::path::{Path, PathBuf};
45
use std::process::Command;
56
use std::{env, fs};
@@ -19,19 +20,26 @@ fn is_marked_for_inclusion(path: &PathBuf) -> bool {
1920
.unwrap_or(false)
2021
}
2122

22-
fn star_deps_to_use(manifest: &Manifest) -> String {
23+
fn star_dep_names(manifest: &Manifest) -> Vec<String> {
2324
// Find all dependencies for this crate that have a `*` version requirement.
2425
// We will assume that these are other exported dependencies that need symbols
2526
// exposed in their module.
2627
manifest
2728
.dependencies
2829
.iter()
2930
.filter(|(_, version)| version.req() == "*")
30-
.map(|(name, _)| format!("use crate::{name};\n"))
31-
.collect::<String>()
31+
.map(|(name, _)| name.to_owned())
32+
.collect()
33+
}
34+
35+
fn star_deps_to_use(manifest: &Manifest) -> String {
36+
star_dep_names(manifest)
37+
.into_iter()
38+
.map(|name| format!("use crate::{name};\n"))
39+
.collect()
3240
}
3341

34-
fn crate_name_from_ament_package_dir(package_dir: &PathBuf) -> &str {
42+
fn crate_name_from_ament_package_dir(package_dir: &Path) -> &str {
3543
package_dir
3644
.parent()
3745
.and_then(|parent| parent.file_name())
@@ -64,8 +72,11 @@ fn main() {
6472

6573
let ament_prefix_paths = get_search_paths().unwrap_or_default();
6674

67-
// Re-export any generated interface crates that we find
68-
let export_crate_tomls: Vec<PathBuf> = ament_prefix_paths
75+
// Re-export any generated interface crates that we find. AMENT_PREFIX_PATH
76+
// can contain overlays and underlays that provide the same package, so keep
77+
// the first provider according to the search path order.
78+
let mut discovered_packages = HashSet::new();
79+
let export_candidates: Vec<PathBuf> = ament_prefix_paths
6980
.iter()
7081
.map(PathBuf::from)
7182
.flat_map(|base_path| {
@@ -83,7 +94,93 @@ fn main() {
8394
.filter_map(|entry| entry.ok())
8495
.map(|entry| entry.path())
8596
.filter(|path| path.file_name() == Some(std::ffi::OsStr::new("Cargo.toml")))
86-
.filter(is_marked_for_inclusion)
97+
.filter(|path| {
98+
path.parent()
99+
.map(crate_name_from_ament_package_dir)
100+
.map(|package| discovered_packages.insert(package.to_owned()))
101+
.unwrap_or(false)
102+
})
103+
.collect();
104+
105+
let candidate_by_package: HashMap<String, PathBuf> = export_candidates
106+
.iter()
107+
.filter_map(|path| {
108+
path.parent()
109+
.map(crate_name_from_ament_package_dir)
110+
.map(|package| (package.to_owned(), path.to_owned()))
111+
})
112+
.collect();
113+
114+
let dependencies_by_package: HashMap<String, Vec<String>> = candidate_by_package
115+
.iter()
116+
.filter_map(|(package, cargo_toml)| {
117+
Manifest::from_path(cargo_toml)
118+
.ok()
119+
.map(|manifest| (package.to_owned(), star_dep_names(&manifest)))
120+
})
121+
.collect();
122+
123+
// Include dependencies of exported packages too. Some distro packages export
124+
// generated crates whose metadata is incomplete, but their generated Rust code
125+
// still imports dependency packages through the ros-env crate root.
126+
let mut included_packages: HashSet<String> = export_candidates
127+
.iter()
128+
.filter(|path| is_marked_for_inclusion(path))
129+
.filter_map(|path| {
130+
path.parent()
131+
.map(crate_name_from_ament_package_dir)
132+
.map(str::to_owned)
133+
})
134+
.collect();
135+
let mut pending_packages: VecDeque<String> = included_packages.iter().cloned().collect();
136+
137+
while let Some(package) = pending_packages.pop_front() {
138+
let Some(dependencies) = dependencies_by_package.get(&package) else {
139+
continue;
140+
};
141+
142+
for dependency in dependencies {
143+
if candidate_by_package.contains_key(dependency)
144+
&& included_packages.insert(dependency.clone())
145+
{
146+
pending_packages.push_back(dependency.clone());
147+
}
148+
}
149+
}
150+
151+
loop {
152+
let invalid_packages: Vec<String> = included_packages
153+
.iter()
154+
.filter(|package| {
155+
dependencies_by_package
156+
.get(*package)
157+
.map(|dependencies| {
158+
dependencies
159+
.iter()
160+
.any(|dependency| !included_packages.contains(dependency))
161+
})
162+
.unwrap_or(false)
163+
})
164+
.cloned()
165+
.collect();
166+
167+
if invalid_packages.is_empty() {
168+
break;
169+
}
170+
171+
for package in invalid_packages {
172+
included_packages.remove(&package);
173+
}
174+
}
175+
176+
let export_crate_tomls: Vec<PathBuf> = export_candidates
177+
.into_iter()
178+
.filter(|path| {
179+
path.parent()
180+
.map(crate_name_from_ament_package_dir)
181+
.map(|package| included_packages.contains(package))
182+
.unwrap_or(false)
183+
})
87184
.collect();
88185

89186
// Make sure the script re-runs if any of the sources we want to include change.

0 commit comments

Comments
 (0)