From 5d86b2d76c8496ab076ec921a021bd8c68c94a2e Mon Sep 17 00:00:00 2001 From: Tamir Duberstein Date: Tue, 3 Nov 2026 18:21:16 -0500 Subject: [PATCH] Track toolchain registration implementations in query mode Bazel omits the implementation of a native toolchain declaration from its ordinary query inputs. Read the implementation label from the toolchain attribute so changing the implementation also impacts its registration. For an unflattened select, include each reported implementation. Invalidate cached ordinary graphs and regenerate older partial recipes that lack these edges. The regression builds a changed toolchain product and checks the registration distance. --- src/bazel.rs | 31 +++++++++- src/cache.rs | 3 + src/main.rs | 51 ++++++++++++++--- src/partial.rs | 2 +- tests/e2e/external.rs | 56 +++++++++++++++++++ .../declaration/BUILD.bazel | 23 ++++++++ tools/e2e/rust_e2e_cases.bzl | 4 ++ 7 files changed, 160 insertions(+), 10 deletions(-) create mode 100644 tests/resources/workspaces/extension_toolchain_through_owner/declaration/BUILD.bazel diff --git a/src/bazel.rs b/src/bazel.rs index 4691e2b4..27a9738a 100644 --- a/src/bazel.rs +++ b/src/bazel.rs @@ -2741,7 +2741,7 @@ fn decode_target_stream( .map(|bytes| { ::decode_from_slice(bytes) .map_err(anyhow::Error::from) - .map(lower_target) + .map(lower_query_target) }) .collect_into_vec(&mut decoded); let start = targets.len(); @@ -2936,6 +2936,35 @@ pub fn target_name(target: &Target) -> Option<&str> { } } +fn lower_query_target(mut target: Target) -> Option { + if let Some(rule) = target + .rule + .as_option_mut() + .filter(|r| r.rule_class == "toolchain") + { + // Bazel's native toolchain rule points at its implementation with a NODEP_LABEL. + // Query prints the label as a nodep string and omits it from rule_input. + if let Some(implementation) = rule + .attribute + .iter() + .find(|a| a.name == "toolchain" && a.nodep == Some(true)) + { + rule.rule_input + .extend(implementation.string_value.iter().cloned()); + if let Some(select) = implementation.selector_list.as_option() { + rule.rule_input + .extend(select.elements.iter().flat_map(|element| { + element + .entries + .iter() + .filter_map(|entry| entry.string_value.clone()) + })); + } + } + } + lower_target(target) +} + fn lower_target(mut target: Target) -> Option { match target.r#type { target::Discriminator::Rule diff --git a/src/cache.rs b/src/cache.rs index 61aaba24..fd170fff 100644 --- a/src/cache.rs +++ b/src/cache.rs @@ -81,6 +81,9 @@ pub fn configuration_fingerprint( hasher.update(value.as_bytes()); hasher.update([0]); } + if !options.bazel.use_cquery { + hasher.update(b"queryToolchainImplementations=1\0"); + } if let Some(root) = &options.bazel.isolated_dependency_root { hasher.update(b"isolatedDependencyRoot="); hasher.update(root.as_bytes()); diff --git a/src/main.rs b/src/main.rs index 546e7a37..61a7f6d6 100644 --- a/src/main.rs +++ b/src/main.rs @@ -831,28 +831,35 @@ fn partial_graph_hashes( Some(path) => { let base = HashFileData::read(path) .with_context(|| format!("failed to read --baseHashes {}", path.display()))?; - match base.tree.as_deref() { - _ if base.partial.is_none() => { + match (base.partial.as_ref(), base.tree.as_deref()) { + (None, _) => { format!("{} carries no partial-graph recipes", path.display()) } - _ if args.dep_edges_file.is_some() - && base.dep_edges.is_empty() - && !base.hashes.is_empty() => + (Some(recipes), _) if recipes.version != partial::RECIPE_VERSION => format!( + "{} carries partial-graph recipe version {}, expected {}", + path.display(), + recipes.version, + partial::RECIPE_VERSION + ), + (_, _) + if args.dep_edges_file.is_some() + && base.dep_edges.is_empty() + && !base.hashes.is_empty() => { format!( "{} records no dependency edges, which --depEdgesFile needs", path.display() ) } - None => format!("{} records no git tree", path.display()), - Some(base_tree) if base_tree == tree_id => { + (_, None) => format!("{} records no git tree", path.display()), + (_, Some(base_tree)) if base_tree == tree_id => { eprintln!( "[Info] partial graph: {} already describes tree {tree_id}", path.display() ); return Ok(base); } - Some(base_tree) => { + (_, Some(base_tree)) => { let policy = DerivationPolicy { seed_filepaths: &options.seed_filepaths, rebuild_patterns: &rebuild_patterns, @@ -896,6 +903,15 @@ fn ensure_same_mode(from: &HashFileData, to: &HashFileData) -> Result<()> { "the starting and final hashes were not generated in the same mode: one carries --partialGraph recipes and the other does not, and hashes of the two modes are not comparable" ); } + if let (Some(from), Some(to)) = (&from.partial, &to.partial) { + if from.version != to.version { + bail!( + "the starting and final hashes carry different partial-graph recipe versions ({} and {}); regenerate both with the same bazel-diff build", + from.version, + to.version + ); + } + } Ok(()) } @@ -2458,6 +2474,19 @@ mod tests { .unwrap(); calls.get() }; + // An older formula must regenerate even when the base describes the same tree. + fs::write(repo.path().join("file.txt"), "one").unwrap(); + assert_eq!(expected_tree(&[&output, &base_path]), base.tree); + let mut outdated = base.clone(); + outdated.partial.as_mut().unwrap().version = partial::RECIPE_VERSION - 1; + fs::write( + &base_path, + serde_json::to_vec(&outdated.serialized(true, false)).unwrap(), + ) + .unwrap(); + assert_eq!(counted(&args), 1); + fs::write(repo.path().join("file.txt"), "two").unwrap(); + let mut no_recipes = base.clone(); no_recipes.partial = None; fs::write( @@ -2558,5 +2587,11 @@ mod tests { assert!(error.contains("same mode"), "{error}"); let error = run_explain(&explain, false).unwrap_err().to_string(); assert!(error.contains("same mode"), "{error}"); + + partial.partial.as_mut().unwrap().version = bazel_diff::partial::RECIPE_VERSION; + let mut older = partial.clone(); + older.partial.as_mut().unwrap().version = bazel_diff::partial::RECIPE_VERSION - 1; + let error = ensure_same_mode(&older, &partial).unwrap_err().to_string(); + assert!(error.contains("recipe versions"), "{error}"); } } diff --git a/src/partial.rs b/src/partial.rs index c46463ea..1c5c5a73 100644 --- a/src/partial.rs +++ b/src/partial.rs @@ -43,7 +43,7 @@ type DigestBytes = [u8; 32]; /// Bumped when the recipe format or the evaluation formula changes, so an entry an older /// release wrote is regenerated rather than evaluated under a formula it was not recorded for. -pub const RECIPE_VERSION: u32 = 1; +pub const RECIPE_VERSION: u32 = 2; // --------------------------------------------------------------------------------------------- // Git tree index diff --git a/tests/e2e/external.rs b/tests/e2e/external.rs index c70d638d..ae4949bd 100644 --- a/tests/e2e/external.rs +++ b/tests/e2e/external.rs @@ -598,6 +598,62 @@ fn isolated_dependency_root_keeps_hidden_spoke_changes() { assert!(!changed.contains("//outside:uses_owner"), "{changed:?}"); } +#[test] +fn toolchain_declarations_follow_their_implementations() { + let workspace = copy_workspace("extension_toolchain_through_owner"); + let root = workspace.path(); + edit(&root.join("MODULE.bazel"), |module| { + format!("{module}\nregister_toolchains(\"//declaration:registration\")\n") + }); + let out = tempfile::tempdir().unwrap(); + let before = out.path().join("before.json"); + let after = out.path().join("after.json"); + let edges = out.path().join("edges.json"); + let result = out.path().join("impacted.json"); + let args = [ + "--bazelCommandOptions=--noproto:flatten_selects", + "--includeTargetType", + "-d", + edges.to_str().unwrap(), + ]; + for (hashes, value) in [(&before, "before"), (&after, "after")] { + if value == "after" { + edit(&root.join("declaration/BUILD.bazel"), |text| { + text.replace("\"before\"", "\"after\"") + }); + } + assert_product( + root, + "//declaration:consumer", + "declaration/consumer.txt", + value, + ); + assert!(generate(root, hashes, &args).status.success()); + } + assert!(impacted( + root, + &before, + &after, + &result, + &["-d", edges.to_str().unwrap()] + ) + .status + .success()); + let changes: Vec = + serde_json::from_slice(&fs::read(result).unwrap()).unwrap(); + let distance = |name: &str| { + changes + .iter() + .find(|row| row["label"] == name) + .and_then(|row| row["targetDistance"].as_u64()) + }; + assert_eq!(distance("//declaration:primary"), Some(0)); + assert_eq!(distance("//declaration:registration"), Some(1)); + assert_eq!(distance("//declaration:choice"), Some(1)); + assert_eq!(distance("//declaration:other"), None); + assert!(dep_edges(&edges)["//declaration:choice"].contains("//declaration:other")); +} + // Module `ext`'s extension generates `@app`, whose rule resolves a toolchain, and the repositories // that toolchain can come from. A query never shows a resolved toolchain as an input, so with // `@app` fine-grained the change has to reach `//:uses_app` through `ext`, however `ext` registers diff --git a/tests/resources/workspaces/extension_toolchain_through_owner/declaration/BUILD.bazel b/tests/resources/workspaces/extension_toolchain_through_owner/declaration/BUILD.bazel new file mode 100644 index 00000000..cb83f8ed --- /dev/null +++ b/tests/resources/workspaces/extension_toolchain_through_owner/declaration/BUILD.bazel @@ -0,0 +1,23 @@ +load("@ext//:defs.bzl", "app_lib", "tool") + +tool(name = "primary", version = "before") +tool(name = "other", version = "other") + +config_setting(name = "alternative", values = {"define": "tool=other"}) + +toolchain( + name = "registration", + toolchain = ":primary", + toolchain_type = "@ext//:toolchain_type", +) + +toolchain( + name = "choice", + toolchain = select({ + ":alternative": ":other", + "//conditions:default": ":primary", + }), + toolchain_type = "@ext//:toolchain_type", +) + +app_lib(name = "consumer") diff --git a/tools/e2e/rust_e2e_cases.bzl b/tools/e2e/rust_e2e_cases.bzl index c4daaa2c..1ea4a425 100644 --- a/tools/e2e/rust_e2e_cases.bzl +++ b/tools/e2e/rust_e2e_cases.bzl @@ -143,6 +143,10 @@ RUST_E2E_SUITES = [ "name": "external::returned_external_source_reaches_consumer", "timeout": "moderate", }, + { + "name": "external::toolchain_declarations_follow_their_implementations", + "timeout": "moderate", + }, { "name": "external::transitive_external_repo_change_reaches_consumer", "timeout": "moderate",