Skip to content

Commit ca46b02

Browse files
committed
Enhance alias handling for external dependencies in the renderer
- Update logic to suppress rename aliases for external dependencies that match the crate's name. - Add tests to ensure primary and shorthand aliases are emitted correctly while preventing duplicates. Signed-off-by: Thomas Lam <thomaslam@canva.com>
1 parent b068d79 commit ca46b02

1 file changed

Lines changed: 109 additions & 7 deletions

File tree

crate_universe/src/rendering.rs

Lines changed: 109 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -211,13 +211,20 @@ impl Renderer {
211211

212212
if let Some(library_target_name) = &krate.library_target_name {
213213
// 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.
214+
// (@{index}__<name>-<version>): extensions.bzl skips spoke
215+
// creation when crate["repository"] == None
216+
// (crate_universe/extensions.bzl:722-724). Use
217+
// override_targets[key] when set; skip otherwise to avoid
218+
// emitting dangling alias targets.
218219
let maybe_actual: Option<Label> = if context.workspace_members.contains_key(&dep.id)
219220
{
220-
krate.override_targets.get("lib").cloned()
221+
krate.targets.iter().find_map(|rule| match rule {
222+
Rule::Library(..) | Rule::ProcMacro(..) => krate
223+
.override_targets
224+
.get(rule.override_target_key())
225+
.cloned(),
226+
_ => None,
227+
})
221228
} else {
222229
Some(self.crate_label(
223230
&krate.name,
@@ -2098,6 +2105,101 @@ mod test {
20982105
);
20992106
}
21002107

2108+
/// Tests that when an external (non-workspace-member) dep is aliased under
2109+
/// a name identical to the crate's own name, the rename alias is suppressed
2110+
/// (it would be a duplicate of the primary alias), but the primary
2111+
/// `<name>-<version>` and shorthand aliases are still emitted exactly once.
2112+
#[test]
2113+
fn external_dep_with_rename_same_as_crate_name() {
2114+
let mut context = Context::default();
2115+
let consumer_id = CrateId::new("consumer".to_owned(), VERSION_ZERO_ONE_ZERO);
2116+
context
2117+
.workspace_members
2118+
.insert(consumer_id.clone(), "consumer".into());
2119+
2120+
// External dep — intentionally NOT inserted into workspace_members.
2121+
let dep_id = CrateId::new("mock_dep".to_owned(), VERSION_ZERO_ONE_ZERO);
2122+
context.crates.insert(
2123+
dep_id.clone(),
2124+
CrateContext {
2125+
name: dep_id.name.clone(),
2126+
version: dep_id.version.clone(),
2127+
package_url: None,
2128+
targets: BTreeSet::from([Rule::Library(TargetAttributes {
2129+
crate_name: "mock_dep".to_owned(),
2130+
crate_root: Some("src/lib.rs".to_owned()),
2131+
..TargetAttributes::default()
2132+
})]),
2133+
library_target_name: Some("mock_dep".to_owned()),
2134+
common_attrs: CommonAttributes::default(),
2135+
build_script_attrs: None,
2136+
repository: None,
2137+
license: None,
2138+
license_ids: BTreeSet::default(),
2139+
license_file: None,
2140+
additive_build_file_content: None,
2141+
disable_pipelining: false,
2142+
extra_aliased_targets: BTreeMap::default(),
2143+
alias_rule: None,
2144+
override_targets: BTreeMap::default(),
2145+
},
2146+
);
2147+
context.crates.insert(
2148+
consumer_id.clone(),
2149+
CrateContext {
2150+
name: consumer_id.name.clone(),
2151+
version: consumer_id.version.clone(),
2152+
package_url: None,
2153+
targets: BTreeSet::from([Rule::Library(mock_target_attributes())]),
2154+
library_target_name: Some("consumer".to_owned()),
2155+
common_attrs: CommonAttributes {
2156+
deps: Select::from_value(BTreeSet::from([CrateDependency {
2157+
id: dep_id.clone(),
2158+
target: "mock_dep".to_owned(),
2159+
// alias == crate name: the `name != rename` guard must fire
2160+
alias: Some("mock_dep".into()),
2161+
local_path: None,
2162+
}])),
2163+
..Default::default()
2164+
},
2165+
build_script_attrs: None,
2166+
repository: None,
2167+
license: None,
2168+
license_ids: BTreeSet::default(),
2169+
license_file: None,
2170+
additive_build_file_content: None,
2171+
disable_pipelining: false,
2172+
extra_aliased_targets: BTreeMap::default(),
2173+
alias_rule: None,
2174+
override_targets: BTreeMap::default(),
2175+
},
2176+
);
2177+
2178+
let renderer = Renderer::new(mock_render_config(None), mock_supported_platform_triples());
2179+
let output = renderer.render(&context, None).unwrap();
2180+
let build_file_content = output.get(&PathBuf::from("BUILD.bazel")).unwrap();
2181+
2182+
println!("{build_file_content}");
2183+
// Primary versioned alias and shorthand must both be present.
2184+
assert!(
2185+
build_file_content.contains("\"mock_dep-0.1.0\""),
2186+
"primary alias must be emitted:\n{build_file_content}"
2187+
);
2188+
assert!(
2189+
build_file_content.contains("\"mock_dep\""),
2190+
"shorthand alias must be emitted:\n{build_file_content}"
2191+
);
2192+
// The rename alias would produce a second `name = "mock_dep-0.1.0"` entry —
2193+
// verify the guard suppresses it so the name appears exactly once.
2194+
assert_eq!(
2195+
build_file_content
2196+
.matches("name = \"mock_dep-0.1.0\"")
2197+
.count(),
2198+
1,
2199+
"rename alias must not be emitted as a duplicate:\n{build_file_content}"
2200+
);
2201+
}
2202+
21012203
/// Tests a situation where there are two dependencies referencing the same
21022204
/// crate at the same version, but with different aliases.
21032205
#[test]
@@ -2411,9 +2513,9 @@ mod test {
24112513
let output = renderer.render(&context, None).unwrap();
24122514
let build_file_content = output.get(&PathBuf::from("BUILD.bazel")).unwrap();
24132515

2414-
// No alias for this workspace member dep should appear in the hub.
2516+
// No alias rule should appear in the hub at all.
24152517
assert!(
2416-
!build_file_content.contains("dep_crate"),
2518+
!build_file_content.contains("alias("),
24172519
"no alias should be emitted for workspace member dep without override:\n{build_file_content}"
24182520
);
24192521
}

0 commit comments

Comments
 (0)