Skip to content

Commit a581c67

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 b2f0d12 commit a581c67

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,
@@ -2105,6 +2112,101 @@ mod test {
21052112
);
21062113
}
21072114

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

2488-
// No alias for this workspace member dep should appear in the hub.
2590+
// No alias rule should appear in the hub at all.
24892591
assert!(
2490-
!build_file_content.contains("dep_crate"),
2592+
!build_file_content.contains("alias("),
24912593
"no alias should be emitted for workspace member dep without override:\n{build_file_content}"
24922594
);
24932595
}

0 commit comments

Comments
 (0)