From 00d1d4db4c9a92a0b6249b275a872a34b89f55bd Mon Sep 17 00:00:00 2001 From: Hazel Sudzilouski Date: Tue, 18 Aug 2026 15:32:46 -0700 Subject: [PATCH 1/4] Fix transformRuleInput prefix collision between fine-grained hub and spoke repos fineGrainedHashExternalRepos matching used an unbounded string prefix, so with hub-and-spoke repos (rules_python pip: @pip hub, @pip_ spokes) a spoke label looked like it belonged to the fine-grained hub and was never rewritten to its //external: synthetic target -- the only node whose hash flips when the spoke's pinned version changes. Consumers behind hub aliases were therefore never impacted by version bumps. Match on the full repo name up to the // boundary instead. Co-Authored-By: Claude Opus 5 (1M context) --- .../kotlin/com/bazel_diff/bazel/BazelRule.kt | 7 +++++- .../com/bazel_diff/bazel/BazelRuleTest.kt | 25 +++++++++++++++++++ 2 files changed, 31 insertions(+), 1 deletion(-) diff --git a/cli/src/main/kotlin/com/bazel_diff/bazel/BazelRule.kt b/cli/src/main/kotlin/com/bazel_diff/bazel/BazelRule.kt index 9156e44..d4f5760 100644 --- a/cli/src/main/kotlin/com/bazel_diff/bazel/BazelRule.kt +++ b/cli/src/main/kotlin/com/bazel_diff/bazel/BazelRule.kt @@ -145,7 +145,12 @@ class BazelRule(private val rule: Build.Rule) { ): String { if (isNotMainRepo(ruleInput) && ruleInput.startsWith("@") && - fineGrainedHashExternalRepos.none { ruleInput.startsWith(it) }) { + // Match on the full repo name (up to the `//` boundary), not a bare string + // prefix: with hub-and-spoke repos (e.g. rules_python's `@pip` hub and + // `@pip_` spokes), a bare prefix check makes every spoke label look + // like it belongs to the fine-grained hub, so it is never rewritten to its + // `//external:` seed and changes stop propagating. + fineGrainedHashExternalRepos.none { ruleInput == it || ruleInput.startsWith("$it//") }) { val splitRule = ruleInput.split("//".toRegex()).dropLastWhile { it.isEmpty() }.toTypedArray() if (splitRule.size == 2) { var externalRule = splitRule[0] diff --git a/cli/src/test/kotlin/com/bazel_diff/bazel/BazelRuleTest.kt b/cli/src/test/kotlin/com/bazel_diff/bazel/BazelRuleTest.kt index 5e12cfe..3bb7478 100644 --- a/cli/src/test/kotlin/com/bazel_diff/bazel/BazelRuleTest.kt +++ b/cli/src/test/kotlin/com/bazel_diff/bazel/BazelRuleTest.kt @@ -189,6 +189,31 @@ class BazelRuleTest { assertThat(inputs).isEqualTo(listOf("//:legacy_dep")) } + // Hub-and-spoke external repos (e.g. rules_python's `@pip` hub with one `@pip_` spoke + // repo per package): a spoke label shares the hub name as a string prefix but is a different + // repo, so marking the hub fine-grained must not stop spoke inputs from being rewritten to + // their `//external:` synthetic target -- that target is the only node whose hash + // flips when the spoke's pinned version changes. + @Test + fun testFineGrainedRepoNameDoesNotPrefixMatchSpokeRepos() { + val rule = + Rule.newBuilder() + .setRuleClass("alias") + .setName("@pip//numpy:pkg") + .addRuleInput("@pip_numpy//:pkg") + .addRuleInput("@pip//numpy:other") + .build() + + val inputs = + BazelRule(rule) + .ruleInputList(useCquery = false, fineGrainedHashExternalRepos = setOf("@pip")) + + // The spoke input is rewritten to its seed; the hub-internal input stays raw because the + // hub itself is fine-grained. + assertThat(inputs.contains("//external:pip_numpy")).isEqualTo(true) + assertThat(inputs.contains("//external:pip")).isEqualTo(false) + } + // Pins the round-trip behaviour `RuleHasher` relies on: the full encoded string lives in the // hash, and the bare label is what gets looked up in `allRulesMap` / `sourceDigests` and // tracked in `deps`. If the round-trip ever drifts the user-facing JSON would start emitting From 304e60fb533d6f19d043febfb0b1c2ebb659b74b Mon Sep 17 00:00:00 2001 From: Hazel Sudzilouski Date: Tue, 18 Aug 2026 16:03:04 -0700 Subject: [PATCH 2/4] Add hub/spoke e2e regression test for the fine-grained prefix collision New WORKSPACE-mode fixture (hub_spoke_external) models pip-style hub-and-spoke external repos with a custom repository rule: a @pip hub of alias packages in front of a @pip_numpy spoke whose repository-rule attrs embed the pinned version. Bumping the version flips only the //external:pip_numpy seed; the consumer behind the hub alias must follow the chain @pip//numpy:pkg -> @pip_numpy//:lib -> //external:pip_numpy. Fails without the transformRuleInput boundary fix (the spoke label prefix-matched the fine-grained hub name and was never rewritten to its seed); passes with it. Co-Authored-By: Claude Opus 5 (1M context) --- .../test/kotlin/com/bazel_diff/e2e/E2ETest.kt | 102 ++++++++++++++++++ .../workspaces/hub_spoke_external/.bazelrc | 4 + .../workspaces/hub_spoke_external/BUILD | 8 ++ .../workspaces/hub_spoke_external/WORKSPACE | 13 +++ .../hub_spoke_external/fake_pip.bzl | 43 ++++++++ 5 files changed, 170 insertions(+) create mode 100644 cli/src/test/resources/workspaces/hub_spoke_external/.bazelrc create mode 100644 cli/src/test/resources/workspaces/hub_spoke_external/BUILD create mode 100644 cli/src/test/resources/workspaces/hub_spoke_external/WORKSPACE create mode 100644 cli/src/test/resources/workspaces/hub_spoke_external/fake_pip.bzl diff --git a/cli/src/test/kotlin/com/bazel_diff/e2e/E2ETest.kt b/cli/src/test/kotlin/com/bazel_diff/e2e/E2ETest.kt index a385aa2..f43960d 100644 --- a/cli/src/test/kotlin/com/bazel_diff/e2e/E2ETest.kt +++ b/cli/src/test/kotlin/com/bazel_diff/e2e/E2ETest.kt @@ -2689,6 +2689,108 @@ class E2ETest { .isEqualTo(true) } + // ------------------------------------------------------------------------ + // Fine-grained hub name must not prefix-match spoke repos (hub-and-spoke) + // ------------------------------------------------------------------------ + // Hub-and-spoke external repos -- e.g. rules_python's pip_parse in WORKSPACE mode, with a + // `@pip` hub of alias packages and one `@pip_` spoke repo per package -- put the + // change-detection signal in the spoke: bumping a pinned version rewrites the spoke + // repository rule's attrs, so only `//external:pip_` flips. Consumers reference the hub + // alias (`@pip//numpy:pkg`), so the chain to the flipping seed runs through the alias's + // input on the spoke label. Related shape: the alias-wrap chain in issue #197. + // + // The `hub_spoke_external` fixture wires this up with a custom repository rule (WORKSPACE + // mode -- the collision lives on the classic //external: path): + // @pip_numpy -> spoke repo; `version` attr is the pin literal + // @pip -> hub repo; numpy/BUILD alias `pkg` -> @pip_numpy//:lib + // //:consumer -> genrule consuming @pip//numpy:pkg + // + // With `--fineGrainedHashExternalRepos @pip`, `transformRuleInput` used to match + // fine-grained repos by unbounded string prefix, so the alias's spoke input + // (`@pip_numpy//:lib`) looked like it belonged to the fine-grained hub and was never + // rewritten to `//external:pip_numpy` -- the only node whose hash flips on the version + // bump. Consumers were silently dropped from the impacted set. The fix in + // `BazelRule.transformRuleInput` matches repo names up to the `//` boundary. + @Test + fun testHubSpokeVersionBumpImpactsConsumer_fineGrainedHubPrefixCollision() { + val workspaceA = copyTestWorkspace("hub_spoke_external") + val workspaceB = copyTestWorkspace("hub_spoke_external") + + // Bump only the spoke's pinned version in B -- a repository-rule attribute literal, the + // same signal a real pip lockfile bump produces. + val workspaceFileInB = File(workspaceB, "WORKSPACE") + workspaceFileInB.writeText( + workspaceFileInB.readText().replace("version = \"1.0\"", "version = \"2.0\"")) + + val outputDir = temp.newFolder() + val from = File(outputDir, "starting_hashes.json") + val to = File(outputDir, "final_hashes.json") + val impactedTargetsOutput = File(outputDir, "impacted_targets.txt") + + val cli = CommandLine(BazelDiff()) + + // Only the hub is listed: users name the repo their BUILD files reference and don't + // expect to enumerate every generated spoke behind it. + val fineGrained = "@pip" + + assertThat( + cli.execute( + "generate-hashes", + "-w", + workspaceA.absolutePath, + "-b", + "bazel", + "--fineGrainedHashExternalRepos", + fineGrained, + from.absolutePath)) + .isEqualTo(0) + assertThat( + cli.execute( + "generate-hashes", + "-w", + workspaceB.absolutePath, + "-b", + "bazel", + "--fineGrainedHashExternalRepos", + fineGrained, + to.absolutePath)) + .isEqualTo(0) + assertThat( + cli.execute( + "get-impacted-targets", + "-w", + workspaceB.absolutePath, + "-b", + "bazel", + "-sh", + from.absolutePath, + "-fh", + to.absolutePath, + "-o", + impactedTargetsOutput.absolutePath)) + .isEqualTo(0) + + val impacted = impactedTargetsOutput.readLines().filter { it.isNotBlank() }.toSet() + + // Sanity: the spoke seed must flip on the version bump; if this fails the fixture no + // longer reproduces the signal and the regression assertion below proves nothing. + val spokeSeedImpacted = impacted.any { it == "//external:pip_numpy" } + assertThat(spokeSeedImpacted) + .transform( + "//external:pip_numpy should be impacted by the spoke version bump. Got impacted: $impacted") { + it + } + .isEqualTo(true) + + val consumerImpacted = impacted.any { it == "//:consumer" || it == "@@//:consumer" } + assertThat(consumerImpacted) + .transform( + "//:consumer should be impacted (chain: @pip//numpy:pkg -> @pip_numpy//:lib -> //external:pip_numpy). Got impacted: $impacted") { + it + } + .isEqualTo(true) + } + // ------------------------------------------------------------------------ // Hermetic fine-grained external-repo hashing across workspaces // ------------------------------------------------------------------------ diff --git a/cli/src/test/resources/workspaces/hub_spoke_external/.bazelrc b/cli/src/test/resources/workspaces/hub_spoke_external/.bazelrc new file mode 100644 index 0000000..eb11625 --- /dev/null +++ b/cli/src/test/resources/workspaces/hub_spoke_external/.bazelrc @@ -0,0 +1,4 @@ +# The prefix collision under test lives on the WORKSPACE //external: path; +# run this fixture in WORKSPACE mode. +common --noenable_bzlmod +common --enable_workspace diff --git a/cli/src/test/resources/workspaces/hub_spoke_external/BUILD b/cli/src/test/resources/workspaces/hub_spoke_external/BUILD new file mode 100644 index 0000000..3ad490a --- /dev/null +++ b/cli/src/test/resources/workspaces/hub_spoke_external/BUILD @@ -0,0 +1,8 @@ +# Consumer behind the hub alias: +# //:consumer -> @pip//numpy:pkg (hub alias) -> @pip_numpy//:lib (spoke) +genrule( + name = "consumer", + srcs = ["@pip//numpy:pkg"], + outs = ["consumer.out"], + cmd = "cat $(SRCS) > $@", +) diff --git a/cli/src/test/resources/workspaces/hub_spoke_external/WORKSPACE b/cli/src/test/resources/workspaces/hub_spoke_external/WORKSPACE new file mode 100644 index 0000000..974cb79 --- /dev/null +++ b/cli/src/test/resources/workspaces/hub_spoke_external/WORKSPACE @@ -0,0 +1,13 @@ +workspace(name = "hub_spoke_external_test") + +load("//:fake_pip.bzl", "fake_hub", "fake_spoke") + +fake_spoke( + name = "pip_numpy", + version = "1.0", +) + +fake_hub( + name = "pip", + spoke = "pip_numpy", +) diff --git a/cli/src/test/resources/workspaces/hub_spoke_external/fake_pip.bzl b/cli/src/test/resources/workspaces/hub_spoke_external/fake_pip.bzl new file mode 100644 index 0000000..d1b6002 --- /dev/null +++ b/cli/src/test/resources/workspaces/hub_spoke_external/fake_pip.bzl @@ -0,0 +1,43 @@ +"""Hub-and-spoke external repos modeled after rules_python's pip_parse (WORKSPACE mode). + +- One spoke repo per package whose repository-rule attrs embed the pinned + version, so a version bump flips the //external: seed hash. +- A hub repo of alias packages that consumers reference: + @pip//numpy:pkg -> @pip_numpy//:lib. +""" + +def _fake_spoke_impl(rctx): + rctx.file("WORKSPACE", "workspace(name = \"{}\")\n".format(rctx.name)) + rctx.file( + "BUILD", + "filegroup(\n" + + " name = \"lib\",\n" + + " srcs = [\"payload.txt\"],\n" + + " visibility = [\"//visibility:public\"],\n" + + ")\n", + ) + rctx.file("payload.txt", "version {}\n".format(rctx.attr.version)) + +fake_spoke = repository_rule( + implementation = _fake_spoke_impl, + # The version literal in this attr is the change-detection signal, exactly + # like the requirement string in a real pip spoke repo. + attrs = {"version": attr.string(mandatory = True)}, +) + +def _fake_hub_impl(rctx): + rctx.file("WORKSPACE", "workspace(name = \"{}\")\n".format(rctx.name)) + rctx.file("BUILD", "") + rctx.file( + "numpy/BUILD", + "alias(\n" + + " name = \"pkg\",\n" + + " actual = \"@{}//:lib\",\n".format(rctx.attr.spoke) + + " visibility = [\"//visibility:public\"],\n" + + ")\n", + ) + +fake_hub = repository_rule( + implementation = _fake_hub_impl, + attrs = {"spoke": attr.string(mandatory = True)}, +) From 2ddccf8478a3aa75464cecad294d13de1f1b3b94 Mon Sep 17 00:00:00 2001 From: Maxwell Elliott Date: Wed, 19 Aug 2026 11:44:50 -0400 Subject: [PATCH 3/4] Port the fine-grained prefix collision fix to the Rust implementation transform_external_input matched fine-grained repos by unbounded string prefix, the same bug the Kotlin transformRuleInput had: with hub-and-spoke repos a spoke label (@pip_numpy//:lib) looked like it belonged to the fine-grained hub (@pip) and was never rewritten to //external:pip_numpy, the only node whose hash flips on a pinned version bump. Match the repo name up to the // boundary, and cover it with a hash unit test plus a Rust e2e test over the same hub_spoke_external fixture. Co-authored-by: Cursor --- src/hash.rs | 47 ++++++++++++++++++++++++++++++++++++++++--- tests/e2e/external.rs | 29 ++++++++++++++++++++++++++ 2 files changed, 73 insertions(+), 3 deletions(-) diff --git a/src/hash.rs b/src/hash.rs index 023f19c..c615786 100644 --- a/src/hash.rs +++ b/src/hash.rs @@ -669,9 +669,15 @@ fn transform_external_input<'a>( let is_main = input.starts_with("//") || input.starts_with("@//") || input.starts_with("@@//"); if !is_main && input.starts_with('@') - && !fine_grained_repos - .iter() - .any(|repo| input.starts_with(repo)) + // Match on the full repo name (up to the `//` boundary), not a bare string prefix: with + // hub-and-spoke repos (e.g. rules_python's `@pip` hub and `@pip_` spokes), a bare + // prefix check makes every spoke label look like it belongs to the fine-grained hub, so it + // is never rewritten to its `//external:` seed and changes stop propagating. + && !fine_grained_repos.iter().any(|repo| { + input + .strip_prefix(repo.as_str()) + .is_some_and(|rest| rest.is_empty() || rest.starts_with("//")) + }) { if let Some((repo, _)) = input.split_once("//") { return Cow::Owned(format!("//external:{}", repo.trim_start_matches('@'))); @@ -1223,6 +1229,41 @@ mod tests { ); } + // Hub-and-spoke external repos (e.g. rules_python's `@pip` hub with one `@pip_` spoke + // repo per package): a spoke label shares the hub name as a string prefix but is a different + // repo, so marking the hub fine-grained must not stop spoke inputs from being rewritten to + // their `//external:` synthetic target -- that target is the only node whose hash flips + // when the spoke's pinned version changes. + #[test] + fn fine_grained_repo_name_does_not_prefix_match_spoke_repos() { + let fine_grained = BTreeSet::from(["@pip".to_owned()]); + assert_eq!( + transform_external_input("@pip_numpy//:lib", &fine_grained), + "//external:pip_numpy" + ); + assert_eq!( + transform_external_input("@pip//numpy:pkg", &fine_grained), + "@pip//numpy:pkg" + ); + + let rule = Rule { + name: "@pip//numpy:pkg".into(), + rule_class: "alias".into(), + rule_input: vec!["@pip_numpy//:lib".into(), "@pip//numpy:other".into()], + ..Default::default() + }; + // The spoke input keeps its raw label and gains the seed; the hub-internal input stays raw + // because the hub itself is fine-grained. + assert_eq!( + rule_inputs(&rule, false, &fine_grained), + [ + "//external:pip_numpy", + "@pip//numpy:other", + "@pip_numpy//:lib" + ] + ); + } + #[test] fn absolute_external_instantiation_frames_are_not_main_repo_inputs() { let frames = [ diff --git a/tests/e2e/external.rs b/tests/e2e/external.rs index c33eb28..db00eaa 100644 --- a/tests/e2e/external.rs +++ b/tests/e2e/external.rs @@ -229,6 +229,35 @@ fn remote_proto_bump_impacts_consumer() { .any(|label| label.ends_with("proto_dep//:greeting_java_proto"))); } +// Hub-and-spoke external repos -- e.g. rules_python's pip_parse in WORKSPACE mode, with a `@pip` +// hub of alias packages and one `@pip_` spoke repo per package -- keep the change-detection +// signal in the spoke: bumping a pinned version rewrites the spoke repository rule's attrs, so only +// `//external:pip_numpy` flips. Users list the hub their BUILD files reference (`@pip`) rather than +// every generated spoke, and the consumer reaches the flipping seed through +// `@pip//numpy:pkg -> @pip_numpy//:lib -> //external:pip_numpy`. Fine-grained matching used to be an +// unbounded string prefix, so the spoke label looked like it belonged to the hub and was never +// rewritten to its seed, silently dropping consumers from the impacted set. +#[test] +fn hub_spoke_version_bump_impacts_consumer_behind_fine_grained_hub() { + let first = copy_workspace("hub_spoke_external"); + let second = copy_workspace("hub_spoke_external"); + edit(&second.path().join("WORKSPACE"), |text| { + text.replace("version = \"1.0\"", "version = \"2.0\"") + }); + let impacted = diff_local( + first.path(), + second.path(), + &["--fineGrainedHashExternalRepos", "@pip"], + ); + // Sanity: if the spoke seed stops flipping the fixture no longer reproduces the signal and the + // consumer assertion below proves nothing. + assert!(impacted.contains("//external:pip_numpy"), "{impacted:?}"); + assert!( + impacted.contains("//:consumer") || impacted.contains("@@//:consumer"), + "{impacted:?}" + ); +} + fn diff_local( first: &std::path::Path, second: &std::path::Path, From f63804c4f5b95aed3bebb9021db99506c73907f0 Mon Sep 17 00:00:00 2001 From: Maxwell Elliott Date: Wed, 19 Aug 2026 12:32:06 -0400 Subject: [PATCH 4/4] Skip the hub/spoke e2e fixture on Bazel 9, which has no WORKSPACE The hub_spoke_external fixture declares @pip and @pip_numpy in a WORKSPACE file, and Bazel 9 removed WORKSPACE support outright -- --enable_workspace is inert there, so the fixture's repos never exist and generate-hashes fails with "No repository visible as '@pip'". That failed the jre21 9.x jobs while 8.x passed. Gate the Kotlin and Rust e2e cases on a Bazel that still loads WORKSPACE, matching the version gating the bzlmod e2e tests already use. The matching logic itself stays pinned on every version by BazelRuleTest and the Rust hash unit tests. Co-authored-by: Cursor --- .../test/kotlin/com/bazel_diff/e2e/E2ETest.kt | 18 ++++++++++++++++++ tests/e2e/external.rs | 5 +++++ tests/e2e/support/mod.rs | 6 ++++++ 3 files changed, 29 insertions(+) diff --git a/cli/src/test/kotlin/com/bazel_diff/e2e/E2ETest.kt b/cli/src/test/kotlin/com/bazel_diff/e2e/E2ETest.kt index f43960d..89cf932 100644 --- a/cli/src/test/kotlin/com/bazel_diff/e2e/E2ETest.kt +++ b/cli/src/test/kotlin/com/bazel_diff/e2e/E2ETest.kt @@ -1929,6 +1929,20 @@ class E2ETest { hasModShowRepo) } + /** + * Skips the calling test unless the local Bazel still loads WORKSPACE files. Bazel 9 removed + * WORKSPACE support outright: `--enable_workspace` is inert there, so a fixture whose repos are + * declared in a WORKSPACE file has no `@repo` to query at all. + */ + private fun assumeBazelSupportsWorkspace() { + val version = getBazelVersion() + org.junit.Assume.assumeNotNull(version) + val v = version!! + org.junit.Assume.assumeTrue( + "Requires a Bazel with WORKSPACE support (current: ${v.first}.${v.second}.${v.third})", + v.first < 9) + } + @Test fun testGenerateHashesIsHermeticAcrossWorkspacePaths() { // Two checkouts of byte-identical sources at different absolute paths -- standing in for the @@ -2713,6 +2727,10 @@ class E2ETest { // `BazelRule.transformRuleInput` matches repo names up to the `//` boundary. @Test fun testHubSpokeVersionBumpImpactsConsumer_fineGrainedHubPrefixCollision() { + // WORKSPACE-only fixture: Bazel 9 dropped WORKSPACE, so `@pip` does not exist there at all. + // The matching logic itself stays pinned on every version by `BazelRuleTest`. + assumeBazelSupportsWorkspace() + val workspaceA = copyTestWorkspace("hub_spoke_external") val workspaceB = copyTestWorkspace("hub_spoke_external") diff --git a/tests/e2e/external.rs b/tests/e2e/external.rs index db00eaa..b70d032 100644 --- a/tests/e2e/external.rs +++ b/tests/e2e/external.rs @@ -239,6 +239,11 @@ fn remote_proto_bump_impacts_consumer() { // rewritten to its seed, silently dropping consumers from the impacted set. #[test] fn hub_spoke_version_bump_impacts_consumer_behind_fine_grained_hub() { + // WORKSPACE-only fixture: Bazel 9 dropped WORKSPACE, so `@pip` does not exist there at all. + // The matching logic itself stays pinned on every version by the `hash` unit tests. + if !supports_workspace() { + return; + } let first = copy_workspace("hub_spoke_external"); let second = copy_workspace("hub_spoke_external"); edit(&second.path().join("WORKSPACE"), |text| { diff --git a/tests/e2e/support/mod.rs b/tests/e2e/support/mod.rs index 13f280a..2f0ab82 100644 --- a/tests/e2e/support/mod.rs +++ b/tests/e2e/support/mod.rs @@ -148,6 +148,12 @@ pub fn supports_mod_show_repo() -> bool { version >= (8, 6, 0) && version != (9, 0, 0) } +/// Whether the local Bazel still loads WORKSPACE files. Bazel 9 removed WORKSPACE support +/// outright, so a fixture whose repos are declared in a WORKSPACE file has no `@repo` to query. +pub fn supports_workspace() -> bool { + bazel_version().0 < 9 +} + /// The `bazel-diff` binary under test. /// /// `CARGO_BIN_EXE_*` is a cargo-only compile-time variable, so under Bazel the