Skip to content

Commit c54a500

Browse files
committed
cargo-bazel: fix hub alias for workspace-member deps with build scripts
Problem: When workspace member A depends on workspace member B (where B has both a library target and a build script), cargo-bazel emits hub aliases pointing to @crate_index__B-<ver>//:B. However, extensions.bzl never creates spoke repos for workspace members (repository == null), so these aliases are dangling and cause "no such package" errors during analysis. Solution: In render_module_build_file, detect when a dep is itself a workspace member (context.workspace_members.contains_key(&dep.id)) before calling crate_label(). If it is a workspace member, look up override_targets["lib"] and use that label instead; if no override is set, skip the alias entirely rather than emitting a dangling reference. Callers set override_targets["lib"] via the crate.annotation( override_target_lib = ...) mechanism in MODULE.bazel, which records the hand-written workspace target to redirect to. Testing: - hub_alias_for_workspace_member_dep_with_build_script_uses_override_target: verifies that the hub alias points to the override label, not a spoke repo. - hub_alias_omitted_for_workspace_member_dep_with_no_override: verifies that no alias is emitted when override_targets["lib"] is absent.
1 parent 01821fa commit c54a500

1 file changed

Lines changed: 231 additions & 50 deletions

File tree

crate_universe/src/rendering.rs

Lines changed: 231 additions & 50 deletions
Original file line numberDiff line numberDiff line change
@@ -210,65 +210,71 @@ impl Renderer {
210210
.unwrap_or(&self.config.default_alias_rule);
211211

212212
if let Some(library_target_name) = &krate.library_target_name {
213-
// Avoid adding the <crate_name>-<version> alias if there are
214-
// more than 1 dependency referencing the same crate at the
215-
// same version, but one of them is aliased.
216-
//
217-
// Without this check we would add duplicate aliases in
218-
// scenarios like the following:
219-
//
220-
// itertools = "0.11.24"
221-
// itertools_other = { version = "0.11.24", package = "itertools" }
222-
//
223-
let add_primary_alias = dep.alias.is_none()
224-
|| !context.has_duplicate_workspace_member_dep_by_version(&dep);
225-
226-
if add_primary_alias {
227-
dependencies.push(Alias {
228-
rule: alias_rule.rule(),
229-
name: format!("{}-{}", krate.name, krate.version),
230-
actual: self.crate_label(
213+
// Workspace-member deps have no spoke repo
214+
// (@{index}__<name>-<version>) — extensions.bzl skips spoke
215+
// creation for all workspace members (repository == null).
216+
// Use override_targets["lib"] when set; skip otherwise to
217+
// avoid emitting dangling alias targets.
218+
let maybe_actual: Option<Label> =
219+
if context.workspace_members.contains_key(&dep.id) {
220+
krate.override_targets.get("lib").cloned()
221+
} else {
222+
Some(self.crate_label(
231223
&krate.name,
232224
&krate.version.to_string(),
233225
library_target_name,
234-
),
235-
tags: BTreeSet::from(["manual".to_owned()]),
236-
});
237-
}
238-
239-
let shorthand = if let Some(rename) = dep.alias.as_ref() {
240-
// when the alias is the same as the crate name, don't create the alias
241-
if krate.name != *rename {
226+
))
227+
};
228+
229+
if let Some(actual) = maybe_actual {
230+
// Avoid adding the <crate_name>-<version> alias if there are
231+
// more than 1 dependency referencing the same crate at the
232+
// same version, but one of them is aliased.
233+
//
234+
// Without this check we would add duplicate aliases in
235+
// scenarios like the following:
236+
//
237+
// itertools = "0.11.24"
238+
// itertools_other = { version = "0.11.24", package = "itertools" }
239+
//
240+
let add_primary_alias = dep.alias.is_none()
241+
|| !context.has_duplicate_workspace_member_dep_by_version(&dep);
242+
243+
if add_primary_alias {
242244
dependencies.push(Alias {
243245
rule: alias_rule.rule(),
244-
name: format!("{}-{}", rename, krate.version),
245-
actual: self.crate_label(
246-
&krate.name,
247-
&krate.version.to_string(),
248-
library_target_name,
249-
),
246+
name: format!("{}-{}", krate.name, krate.version),
247+
actual: actual.clone(),
250248
tags: BTreeSet::from(["manual".to_owned()]),
251249
});
252250
}
253-
rename
254-
} else {
255-
&krate.name
256-
};
257251

258-
// Add a shorthand for crate names as long as there isn't a duplicate
259-
// entry. Shorthands for duplicate entries would lead to ambiguous
260-
// dependencies.
261-
if !context.has_duplicate_workspace_member_dep_by_alias(&dep) {
262-
dependencies.push(Alias {
263-
rule: alias_rule.rule(),
264-
name: shorthand.clone(),
265-
actual: self.crate_label(
266-
&krate.name,
267-
&krate.version.to_string(),
268-
library_target_name,
269-
),
270-
tags: BTreeSet::from(["manual".to_owned()]),
271-
});
252+
let shorthand = if let Some(rename) = dep.alias.as_ref() {
253+
// when the alias is the same as the crate name, don't create the alias
254+
if krate.name != *rename {
255+
dependencies.push(Alias {
256+
rule: alias_rule.rule(),
257+
name: format!("{}-{}", rename, krate.version),
258+
actual: actual.clone(),
259+
tags: BTreeSet::from(["manual".to_owned()]),
260+
});
261+
}
262+
rename
263+
} else {
264+
&krate.name
265+
};
266+
267+
// Add a shorthand for crate names as long as there isn't a duplicate
268+
// entry. Shorthands for duplicate entries would lead to ambiguous
269+
// dependencies.
270+
if !context.has_duplicate_workspace_member_dep_by_alias(&dep) {
271+
dependencies.push(Alias {
272+
rule: alias_rule.rule(),
273+
name: shorthand.clone(),
274+
actual: actual.clone(),
275+
tags: BTreeSet::from(["manual".to_owned()]),
276+
});
277+
}
272278
}
273279
}
274280

@@ -2350,4 +2356,179 @@ mod test {
23502356
"proc-macro lib must appear in proc_macro_deps:\n{binary_section}"
23512357
);
23522358
}
2359+
2360+
/// When a workspace-member crate is itself a dep of another workspace member,
2361+
/// and has both a library target and a build script, the hub BUILD.bazel alias
2362+
/// must point to `override_targets["lib"]` rather than a non-existent spoke repo.
2363+
#[test]
2364+
fn hub_alias_for_workspace_member_dep_with_build_script_uses_override_target() {
2365+
let mut context = Context::default();
2366+
2367+
// consumer: a workspace member
2368+
let consumer_id = CrateId::new("consumer".to_owned(), VERSION_ZERO_ONE_ZERO);
2369+
context
2370+
.workspace_members
2371+
.insert(consumer_id.clone(), "consumer".into());
2372+
2373+
// dep_crate: another workspace member with library + build script
2374+
let dep_id = CrateId::new("dep_crate".to_owned(), VERSION_ZERO_ONE_ZERO);
2375+
context
2376+
.workspace_members
2377+
.insert(dep_id.clone(), "dep_crate".into());
2378+
context.crates.insert(
2379+
dep_id.clone(),
2380+
CrateContext {
2381+
name: dep_id.name.clone(),
2382+
version: dep_id.version.clone(),
2383+
package_url: None,
2384+
targets: BTreeSet::from([
2385+
Rule::Library(TargetAttributes {
2386+
crate_name: "dep_crate".to_owned(),
2387+
crate_root: Some("src/lib.rs".to_owned()),
2388+
..TargetAttributes::default()
2389+
}),
2390+
Rule::BuildScript(TargetAttributes {
2391+
crate_name: "build_script_build".to_owned(),
2392+
crate_root: Some("build.rs".to_owned()),
2393+
..TargetAttributes::default()
2394+
}),
2395+
]),
2396+
library_target_name: Some("dep_crate".to_owned()),
2397+
common_attrs: CommonAttributes::default(),
2398+
build_script_attrs: Some(BuildScriptAttributes::default()),
2399+
repository: None,
2400+
license: None,
2401+
license_ids: BTreeSet::default(),
2402+
license_file: None,
2403+
additive_build_file_content: None,
2404+
disable_pipelining: false,
2405+
extra_aliased_targets: BTreeMap::default(),
2406+
alias_rule: None,
2407+
override_targets: BTreeMap::from([(
2408+
"lib".to_owned(),
2409+
Label::from_str("//tools/dep_crate:dep_crate").unwrap(),
2410+
)]),
2411+
},
2412+
);
2413+
context.crates.insert(
2414+
consumer_id.clone(),
2415+
CrateContext {
2416+
name: consumer_id.name.clone(),
2417+
version: consumer_id.version.clone(),
2418+
package_url: None,
2419+
targets: BTreeSet::from([Rule::Library(mock_target_attributes())]),
2420+
library_target_name: Some("consumer".to_owned()),
2421+
common_attrs: CommonAttributes {
2422+
deps: Select::from_value(BTreeSet::from([CrateDependency {
2423+
id: dep_id.clone(),
2424+
target: "dep_crate".to_owned(),
2425+
alias: None,
2426+
local_path: None,
2427+
}])),
2428+
..Default::default()
2429+
},
2430+
build_script_attrs: None,
2431+
repository: None,
2432+
license: None,
2433+
license_ids: BTreeSet::default(),
2434+
license_file: None,
2435+
additive_build_file_content: None,
2436+
disable_pipelining: false,
2437+
extra_aliased_targets: BTreeMap::default(),
2438+
alias_rule: None,
2439+
override_targets: BTreeMap::default(),
2440+
},
2441+
);
2442+
2443+
let renderer = Renderer::new(mock_render_config(None), mock_supported_platform_triples());
2444+
let output = renderer.render(&context, None).unwrap();
2445+
let build_file_content = output.get(&PathBuf::from("BUILD.bazel")).unwrap();
2446+
2447+
// Must use the override label, not a (non-existent) spoke repo.
2448+
assert!(
2449+
build_file_content.contains("//tools/dep_crate:dep_crate"),
2450+
"expected override label in hub alias:\n{build_file_content}"
2451+
);
2452+
assert!(
2453+
!build_file_content.contains("@test_rendering__dep_crate"),
2454+
"must not emit spoke-repo alias for workspace member:\n{build_file_content}"
2455+
);
2456+
}
2457+
2458+
/// When a workspace-member dep has no override_targets["lib"], no hub alias
2459+
/// should be emitted to avoid dangling references to missing spoke repos.
2460+
#[test]
2461+
fn hub_alias_omitted_for_workspace_member_dep_with_no_override() {
2462+
let mut context = Context::default();
2463+
2464+
let consumer_id = CrateId::new("consumer".to_owned(), VERSION_ZERO_ONE_ZERO);
2465+
context
2466+
.workspace_members
2467+
.insert(consumer_id.clone(), "consumer".into());
2468+
2469+
let dep_id = CrateId::new("dep_crate".to_owned(), VERSION_ZERO_ONE_ZERO);
2470+
context
2471+
.workspace_members
2472+
.insert(dep_id.clone(), "dep_crate".into());
2473+
context.crates.insert(
2474+
dep_id.clone(),
2475+
CrateContext {
2476+
name: dep_id.name.clone(),
2477+
version: dep_id.version.clone(),
2478+
package_url: None,
2479+
targets: BTreeSet::from([Rule::Library(mock_target_attributes())]),
2480+
library_target_name: Some("dep_crate".to_owned()),
2481+
common_attrs: CommonAttributes::default(),
2482+
build_script_attrs: None,
2483+
repository: None,
2484+
license: None,
2485+
license_ids: BTreeSet::default(),
2486+
license_file: None,
2487+
additive_build_file_content: None,
2488+
disable_pipelining: false,
2489+
extra_aliased_targets: BTreeMap::default(),
2490+
alias_rule: None,
2491+
override_targets: BTreeMap::default(), // no override set
2492+
},
2493+
);
2494+
context.crates.insert(
2495+
consumer_id.clone(),
2496+
CrateContext {
2497+
name: consumer_id.name.clone(),
2498+
version: consumer_id.version.clone(),
2499+
package_url: None,
2500+
targets: BTreeSet::from([Rule::Library(mock_target_attributes())]),
2501+
library_target_name: Some("consumer".to_owned()),
2502+
common_attrs: CommonAttributes {
2503+
deps: Select::from_value(BTreeSet::from([CrateDependency {
2504+
id: dep_id.clone(),
2505+
target: "dep_crate".to_owned(),
2506+
alias: None,
2507+
local_path: None,
2508+
}])),
2509+
..Default::default()
2510+
},
2511+
build_script_attrs: None,
2512+
repository: None,
2513+
license: None,
2514+
license_ids: BTreeSet::default(),
2515+
license_file: None,
2516+
additive_build_file_content: None,
2517+
disable_pipelining: false,
2518+
extra_aliased_targets: BTreeMap::default(),
2519+
alias_rule: None,
2520+
override_targets: BTreeMap::default(),
2521+
},
2522+
);
2523+
2524+
let renderer = Renderer::new(mock_render_config(None), mock_supported_platform_triples());
2525+
let output = renderer.render(&context, None).unwrap();
2526+
let build_file_content = output.get(&PathBuf::from("BUILD.bazel")).unwrap();
2527+
2528+
// No alias for this workspace member dep should appear in the hub.
2529+
assert!(
2530+
!build_file_content.contains("dep_crate"),
2531+
"no alias should be emitted for workspace member dep without override:\n{build_file_content}"
2532+
);
2533+
}
23532534
}

0 commit comments

Comments
 (0)