diff --git a/src/bazel.rs b/src/bazel.rs index 07026a89..7b01c534 100644 --- a/src/bazel.rs +++ b/src/bazel.rs @@ -1532,7 +1532,9 @@ impl BazelOptions { }; if let Some(compatible) = compatible { targets.retain(|target| { - target_name(target).is_some_and(|name| compatible.contains(target_label(name))) + target_name(target).is_some_and(|name| { + compatible.contains(main_repo_label_key(target_label(name))) + }) }); } Ok(targets) @@ -1552,12 +1554,10 @@ impl BazelOptions { if " &str { split_configured(key).0 } +/// One spelling of a main-repository label, for comparing labels Bazel printed in +/// different forms: `@@//pkg:name` (Bzlmod), `@//pkg:name` (WORKSPACE mode) and the +/// apparent `//pkg:name` that `repr()` gives a main-repository alias all become +/// `//pkg:name`. Other labels are returned unchanged. +fn main_repo_label_key(label: &str) -> &str { + label + .strip_prefix("@@") + .or_else(|| label.strip_prefix('@')) + .filter(|rest| rest.starts_with("//")) + .unwrap_or(label) +} + pub fn target_name(target: &Target) -> Option<&str> { match target.r#type { target::Discriminator::Rule => target.rule.as_option().map(|rule| rule.name.as_str()), @@ -3797,6 +3809,19 @@ mod tests { ); } + #[test] + fn main_repo_label_key_puts_main_repository_labels_in_one_form() { + // Bzlmod, WORKSPACE mode and repr() of a main-repository alias. + for label in ["@@//pkg:name", "@//pkg:name", "//pkg:name"] { + assert_eq!(main_repo_label_key(label), "//pkg:name", "{label}"); + } + assert_eq!(main_repo_label_key("@@//:tool"), "//:tool"); + // External repositories keep their name, in either spelling. + for label in ["@@rules_cc+//cc:defs", "@rules_cc//cc:defs", "@@foo//:bar"] { + assert_eq!(main_repo_label_key(label), label, "{label}"); + } + } + #[test] fn target_names_and_lowering_cover_all_target_types() { let source = Target { diff --git a/tests/e2e/regressions.rs b/tests/e2e/regressions.rs index dbe91f5f..42d4c2b4 100644 --- a/tests/e2e/regressions.rs +++ b/tests/e2e/regressions.rs @@ -1340,3 +1340,64 @@ fn main_repo_alias_change_reaches_consumer_cquery() { ); } } + +/// Follow-up to for WORKSPACE mode: there +/// `--consistent_labels` prints main-repository targets as `@//:tool`, not the Bzlmod +/// `@@//:tool`, so prefixing the alias's apparent label with `@@` still left it unmatched, +/// dropped as if it were platform-incompatible together with every edge through it. +#[test] +fn main_repo_alias_change_reaches_consumer_cquery_workspace_mode() { + let first = copy_workspace("cquery_main_repo_alias_workspace"); + let second = copy_workspace("cquery_main_repo_alias_workspace"); + fs::write(second.path().join("bar/baz.txt"), "after\n").unwrap(); + + let output = tempfile::tempdir().unwrap(); + let from = output.path().join("from.json"); + let to = output.path().join("to.json"); + let edges_file = output.path().join("edges.json"); + let result = output.path().join("impacted.txt"); + assert!(generate(first.path(), &from, &["--useCquery"]) + .status + .success()); + assert!(generate( + second.path(), + &to, + &[ + "--useCquery", + "--depEdgesFile", + edges_file.to_str().unwrap() + ], + ) + .status + .success()); + assert!(impacted(second.path(), &from, &to, &result, &[]) + .status + .success()); + + let hashes = hashes(&to); + assert!( + hashes.keys().any(|label| label.ends_with("//:tool")), + "the alias //:tool was dropped from the cquery hashes: {:?}", + hashes.keys().collect::>() + ); + let edges = dep_edges(&edges_file); + let wrapper_edges = edges + .iter() + .find(|(label, _)| label.ends_with("//:wrapper")) + .map(|(_, deps)| deps.clone()) + .expect("//:wrapper has dep edges"); + assert!( + wrapper_edges.iter().any(|dep| dep.ends_with("//:tool")), + "//:wrapper lost its edge to the alias //:tool: {wrapper_edges:?}" + ); + + let impacted = read_lines(&result); + for name in ["//bar:baz.txt", "//bar:baz", "//:tool", "//:wrapper"] { + assert!( + impacted + .iter() + .any(|label| label == name || label.trim_start_matches('@') == name), + "{name} should be impacted through the alias under cquery, got {impacted:?}" + ); + } +} diff --git a/tests/resources/workspaces/cquery_main_repo_alias_workspace/.bazelrc b/tests/resources/workspaces/cquery_main_repo_alias_workspace/.bazelrc new file mode 100644 index 00000000..409363ea --- /dev/null +++ b/tests/resources/workspaces/cquery_main_repo_alias_workspace/.bazelrc @@ -0,0 +1,4 @@ +# Main-repository labels print as `@//pkg:name` in WORKSPACE mode, unlike the +# `@@//pkg:name` of Bzlmod; run this fixture in WORKSPACE mode. +common --noenable_bzlmod +common --enable_workspace diff --git a/tests/resources/workspaces/cquery_main_repo_alias_workspace/BUILD.bazel b/tests/resources/workspaces/cquery_main_repo_alias_workspace/BUILD.bazel new file mode 100644 index 00000000..a03cb29a --- /dev/null +++ b/tests/resources/workspaces/cquery_main_repo_alias_workspace/BUILD.bazel @@ -0,0 +1,14 @@ +# A main-repository alias in front of the file that changes. `//:wrapper` +# reaches `bar/baz.txt` only through the alias, so it must be impacted when +# the file changes. +alias( + name = "tool", + actual = "//bar:baz", +) + +genrule( + name = "wrapper", + srcs = [":tool"], + outs = ["wrapper.txt"], + cmd = "cat $(SRCS) > $@", +) diff --git a/tests/resources/workspaces/cquery_main_repo_alias_workspace/README.md b/tests/resources/workspaces/cquery_main_repo_alias_workspace/README.md new file mode 100644 index 00000000..d6ac6968 --- /dev/null +++ b/tests/resources/workspaces/cquery_main_repo_alias_workspace/README.md @@ -0,0 +1,23 @@ +# Main-repository alias under cquery, in WORKSPACE mode + +The WORKSPACE-mode counterpart of `cquery_main_repo_alias` (GitHub Issue #612). + +## The graph + +`//:wrapper` (a `genrule`) lists `//:tool` in its `srcs`. `//:tool` is an +`alias` of `//bar:baz`, a `filegroup` over `bar/baz.txt`. + +## Expected behaviour + +Changing `bar/baz.txt` impacts `//bar:baz.txt`, `//bar:baz`, `//:tool`, +`//:wrapper` and `//:wrapper.txt`, in query mode and in cquery mode alike. + +## Behaviour before the fix + +The compatible-target cquery names an alias by `repr(target)`, which prints a +main-repository alias with its apparent label (`//:tool`). The fix for #612 +prefixed it with `@@`, the main repository's canonical form under Bzlmod. In +WORKSPACE mode, though, `--consistent_labels` prints main-repository targets +as `@//:tool`, so `@@//:tool` still never matches, the alias is dropped as if +it were incompatible, and so is every edge from `//:wrapper` through it. Under +cquery only `//bar:baz` and `//bar:baz.txt` are reported as impacted. diff --git a/tests/resources/workspaces/cquery_main_repo_alias_workspace/WORKSPACE b/tests/resources/workspaces/cquery_main_repo_alias_workspace/WORKSPACE new file mode 100644 index 00000000..43ba6189 --- /dev/null +++ b/tests/resources/workspaces/cquery_main_repo_alias_workspace/WORKSPACE @@ -0,0 +1 @@ +workspace(name = "cquery_main_repo_alias_workspace") diff --git a/tests/resources/workspaces/cquery_main_repo_alias_workspace/bar/BUILD.bazel b/tests/resources/workspaces/cquery_main_repo_alias_workspace/bar/BUILD.bazel new file mode 100644 index 00000000..e31cb197 --- /dev/null +++ b/tests/resources/workspaces/cquery_main_repo_alias_workspace/bar/BUILD.bazel @@ -0,0 +1,5 @@ +filegroup( + name = "baz", + srcs = ["baz.txt"], + visibility = ["//visibility:public"], +) diff --git a/tests/resources/workspaces/cquery_main_repo_alias_workspace/bar/baz.txt b/tests/resources/workspaces/cquery_main_repo_alias_workspace/bar/baz.txt new file mode 100644 index 00000000..90be1f30 --- /dev/null +++ b/tests/resources/workspaces/cquery_main_repo_alias_workspace/bar/baz.txt @@ -0,0 +1 @@ +before diff --git a/tools/e2e/rust_e2e_cases.bzl b/tools/e2e/rust_e2e_cases.bzl index 921847cc..18967fe1 100644 --- a/tools/e2e/rust_e2e_cases.bzl +++ b/tools/e2e/rust_e2e_cases.bzl @@ -235,6 +235,10 @@ RUST_E2E_SUITES = [ "name": "regressions::main_repo_alias_change_reaches_consumer_cquery", "timeout": "moderate", }, + { + "name": "regressions::main_repo_alias_change_reaches_consumer_cquery_workspace_mode", + "timeout": "moderate", + }, { "name": "regressions::main_repo_alias_change_reaches_consumer_query", "timeout": "moderate",