diff --git a/README.md b/README.md index 38fd2147..469d11be 100644 --- a/README.md +++ b/README.md @@ -656,7 +656,7 @@ Options: --fineGrainedHashExternalReposFile --labelOnlyModuleExtensions - Module extensions (`@rules_rs//rs:extensions.bzl%crate`) whose repositories use each other only through labels, never by resolving a toolchain from one another. A fine-grained target in one of their repositories then does not change whenever any other repository of the extension does + Module extensions (`@rules_rs//rs:extensions.bzl%crate`) whose repositories are used only through labels, never by resolving a toolchain from one of them. The module that owns such an extension then does not stand in for its repositories, so a lockfile bump impacts only the targets that depend on the bumped repository --isolatedDependencyRoot Assert that consumers outside this root-isolated usage reach it through ordinary target inputs or BUILD loads. The named root import must cover every generated repository. Keep the whole usage on each such consumer --useCquery[=] @@ -814,7 +814,7 @@ Options: --fineGrainedHashExternalReposFile --labelOnlyModuleExtensions - Module extensions (`@rules_rs//rs:extensions.bzl%crate`) whose repositories use each other only through labels, never by resolving a toolchain from one another. A fine-grained target in one of their repositories then does not change whenever any other repository of the extension does + Module extensions (`@rules_rs//rs:extensions.bzl%crate`) whose repositories are used only through labels, never by resolving a toolchain from one of them. The module that owns such an extension then does not stand in for its repositories, so a lockfile bump impacts only the targets that depend on the bumped repository --isolatedDependencyRoot Assert that consumers outside this root-isolated usage reach it through ordinary target inputs or BUILD loads. The named root import must cover every generated repository. Keep the whole usage on each such consumer --useCquery[=] @@ -914,13 +914,16 @@ A few inputs are normalized or extended on the way in: in the workspace, a `file://%workspace%` registry URL) is hashed relative to the workspace, so identical checkouts at different paths hash the same. In query mode the module that owns a module extension stands in for every repository the - extension generates, because a toolchain resolved from one of them is not a query input. A - fine-grained target inside one of those repositories that uses a rule from the owning module - therefore changes whenever any repository of the extension does. Listing the extension in - `--labelOnlyModuleExtensions` (for example `@rules_rs//rs:extensions.bzl%crate`) asserts that - its repositories use each other only through labels, so such a target reaches the owning module - without them. Do not list an extension whose targets resolve a toolchain the same extension - generates: that change would be missed. An extension whose name another `.bzl` file of the same + extension generates, because a toolchain resolved from one of them is not a query input. Every + target that uses a rule of the owning module, every target of a module with a `bazel_dep` path + to it, and every fine-grained target inside one of those repositories therefore changes whenever + any repository of the extension does: a `go.mod` bump through gazelle's `go_deps` or a + `pnpm-lock.yaml` bump through rules_js's `npm` impacts most of the workspace. Listing the + extension in `--labelOnlyModuleExtensions` (for example + `@gazelle//:extensions.bzl%go_deps` or `@rules_rs//rs:extensions.bzl%crate`) asserts that its + repositories are used only through labels, which the query reports, so the owning module no + longer stands in for them. Do not list an extension that generates a toolchain, such as + `rules_go`'s `go_sdk`: a change to it would be missed. An extension whose name another `.bzl` file of the same module also exports is reported and ignored, since Bazel tells their repositories apart only by a numeric suffix. `--isolatedDependencyRoot=@crates` asserts that every outside consumer of this diff --git a/src/bazel.rs b/src/bazel.rs index c0ad5872..c5b1abc4 100644 --- a/src/bazel.rs +++ b/src/bazel.rs @@ -49,9 +49,8 @@ pub struct BazelOptions { pub fine_grained_external_repos: BTreeSet, /// Module extensions, as `bazel mod show_extension` names them /// (`@rules_rs//rs:extensions.bzl%crate`), whose repositories use each other only through - /// labels: no target in one of them resolves a toolchain from another. A target inside such an - /// extension's repository reaches the module that owns the extension without the extension's - /// other repositories (see `extension_view_node`). + /// labels: no target resolves a toolchain from one of them. The module that owns such an + /// extension then does not stand in for its repositories (see `lower_repositories`). pub label_only_module_extensions: BTreeSet, /// A root-isolated extension import whose external consumers are discoverable through /// ordinary target inputs and BUILD loads. @@ -723,7 +722,7 @@ impl BazelOptions { }; let mut scope = IsolatedRepoScope::default(); let label_only = LabelOnlyRepositories { - sibling_extensions: self + extensions: self .label_only_extension_prefixes(&canonical_to_apparent, &extension_repos.generated), owner_exemptions: if let Some(root) = &self.isolated_dependency_root { if let Some(extension) = failed.extensions.first() { @@ -1924,7 +1923,8 @@ impl IsolatedRepoScope { #[derive(Default)] struct LabelOnlyRepositories { - sibling_extensions: BTreeSet, + /// Usage prefixes of `label_only_module_extensions`; their owners get no stand-in edges. + extensions: BTreeSet, owner_exemptions: BTreeSet, } @@ -2434,15 +2434,15 @@ fn lower_repositories( // and each imported repository of an extension depends on the siblings nobody imports (the // `@bd_crates` hub reaches its `bd_crates__*` spokes). Spokes themselves stay leaves, so no // cycle forms. - // For an extension the user lists in `label_only_module_extensions`, a target inside one of - // its repositories reaches the owning module through a view without the extension's own - // repositories (`extension_view_node`): the user asserts that it uses its siblings only - // through labels, which the query reports. Otherwise every fine-grained spoke would change - // whenever any other spoke does. The view is opt-in because a toolchain resolved from a - // sibling is invisible to a query, however it is registered. + // An extension the user lists in `label_only_module_extensions` gets no owner edges: the user + // asserts that its repositories are reached only through labels, which the query reports, + // never through a resolved toolchain. Otherwise every target of the owning module, of every + // module with a `bazel_dep` path to it, and every fine-grained spoke would change whenever + // any repository of the extension does (issue #603). It is opt-in because a toolchain + // resolved from one of them is invisible to a query, however it is registered. // An explicitly label-only imported repository needs no owner fallback. Its own node, // the hub-to-spoke inputs and the owner's edges to other generators remain. - let mut module_extension_repos = BTreeMap::>>::new(); + let mut module_extension_repos = BTreeMap::>::new(); let mut extension_spokes = BTreeMap::>::new(); if let Some(imported) = extension_edges { for repository in &repositories { @@ -2453,15 +2453,15 @@ fn lower_repositories( if let Some((module_repo, _definition_hash)) = extension_repo_owner(extension, &module_definitions).filter( |(module_repo, _definition_hash)| { - !module_repo.is_empty() && !label_only.owner_exemptions.contains(canonical) + !module_repo.is_empty() + && !label_only.owner_exemptions.contains(canonical) + && !label_only.extensions.contains(extension) }, ) { module_extension_repos .entry(module_repo.clone()) .or_default() - .entry(extension.to_owned()) - .or_default() .extend(node_names(canonical)); } if !imported.contains(canonical) { @@ -2490,8 +2490,7 @@ fn lower_repositories( .filter_map(|dependency| module_to_canonical.get(dependency)) .flat_map(|canonical| node_names(canonical)) .collect::>(); - let extensions = module_extension_repos.get(canonical); - for repos in extensions.into_iter().flat_map(BTreeMap::values) { + if let Some(repos) = module_extension_repos.get(canonical) { dependencies.extend(repos.iter().cloned()); } let spokes = extension @@ -2509,17 +2508,12 @@ fn lower_repositories( if names.is_empty() { continue; } - let views = extensions - .into_iter() - .flatten() - .filter(|(extension, _)| label_only.sibling_extensions.contains(*extension)) - .collect::>(); let files = module_files .remove(&index) .unwrap_or_else(|| prepare_repository(&mut repository, &workspace)); let rule = repository_rule(repository, files, owner_definition_hash); - // All apparent names and views have the same definition. Move it into the final output. - let mut rules = std::iter::repeat_n(rule, names.len() * (1 + views.len())); + // All apparent names have the same definition. Move it into the final output. + let mut rules = std::iter::repeat_n(rule, names.len()); for apparent in names { // Imported hubs of one extension share its ordered hidden siblings. Pass // the same allocation to the hash traversal when no owner-specific edge @@ -2543,26 +2537,11 @@ fn lower_repositories( let mut target = repository_target(rules.next().unwrap(), &apparent, inputs); transform(&mut target, shared); targets.push(target); - for (extension, repos) in &views { - let mut view = repository_target( - rules.next().unwrap(), - &extension_view_node(&apparent, extension), - &dependencies.difference(repos).cloned().collect(), - ); - transform(&mut view, None); - targets.push(view); - } } } targets } -/// The node a target in a repository of `extension` reaches its owning module `node` through: -/// the module without the extension's own repositories (see `lower_repositories`). -pub(crate) fn extension_view_node(node: &str, extension: &str) -> String { - format!("{node}%{extension}") -} - fn parse_bazel_version(text: &str) -> Option { let version = text.lines().find_map(|line| { line.strip_prefix("Build label: ") @@ -4557,7 +4536,7 @@ mod tests { } #[test] - fn apparent_names_and_views_share_one_read_of_the_repository_inputs() { + fn apparent_names_share_one_read_of_the_repository_inputs() { let workspace = tempfile::tempdir().unwrap(); let patch = workspace.path().join("dep.patch"); fs::write(&patch, "first").unwrap(); @@ -4585,10 +4564,7 @@ mod tests { &parse_repo_mapping(r#"{"before":"before+","a":"dep+","b":"dep+"}"#), &BTreeMap::new(), Some(&BTreeSet::new()), - &LabelOnlyRepositories { - sibling_extensions: BTreeSet::from(["dep++ext".into()]), - ..Default::default() - }, + &LabelOnlyRepositories::default(), workspace.path(), &mut |target, _| { // The module definitions have been hashed, but dep's targets have not been emitted. @@ -4619,9 +4595,7 @@ mod tests { [ ("//external:before", None), ("//external:a", first), - ("//external:a%dep++ext", first), ("//external:b", first), - ("//external:b%dep++ext", first), ("//external:dep++ext+spoke", Some(second.as_str())), ] ); @@ -5367,17 +5341,11 @@ exit 1 &mapping, &edges, Some(&imported), - &LabelOnlyRepositories { - sibling_extensions: BTreeSet::from([ - "rules_go++go_sdk".to_owned(), - "rules_rs++crate".to_owned(), - ]), - ..Default::default() - }, + &LabelOnlyRepositories::default(), Path::new("."), &mut materialize_repository_inputs, ); - assert_eq!(query.len(), 10); + assert_eq!(query.len(), 8); // The owning module reaches everything its extensions generate, after its module deps. assert_eq!( inputs_of(&query, "//external:rules_go"), @@ -5395,16 +5363,6 @@ exit 1 "//external:rules_rs++crate+bd_crates__anyhow-1.0.104", ] ); - // Its view from inside one extension leaves out that extension's repositories, and keeps - // those of its other usages, an isolated usage of the same extension included. - assert_eq!( - inputs_of(&query, "//external:rules_go%rules_go++go_sdk"), - ["//external:rules_rs"] - ); - assert_eq!( - inputs_of(&query, "//external:rules_rs%rules_rs++crate"), - ["//external:rules_rs++_crate+++isolated_crates+crate_repo"] - ); // An imported repository reaches the siblings of its extension nobody imports. assert_eq!( inputs_of(&query, "//external:bd_crates"), @@ -5423,6 +5381,42 @@ exit 1 .is_empty()); assert!(inputs_of(&query, "//external:+local+root_ext_repo").is_empty()); + // A label-only extension's repositories leave its owner's node, while those of its other + // usages stay, an isolated usage of the same extension included (issue #603). The + // hub-to-spoke edges remain. + let label_only = lower_repositories( + repositories(), + &mapping, + &edges, + Some(&imported), + &LabelOnlyRepositories { + extensions: BTreeSet::from([ + "rules_go++go_sdk".to_owned(), + "rules_rs++crate".to_owned(), + ]), + ..Default::default() + }, + Path::new("."), + &mut materialize_repository_inputs, + ); + assert_eq!(label_only.len(), 8); + assert_eq!( + inputs_of(&label_only, "//external:rules_go"), + ["//external:rules_rs"] + ); + assert_eq!( + inputs_of(&label_only, "//external:rules_rs"), + ["//external:rules_rs++_crate+++isolated_crates+crate_repo"] + ); + assert_eq!( + inputs_of(&label_only, "//external:bd_crates"), + ["//external:rules_rs++crate+bd_crates__anyhow-1.0.104"] + ); + assert_eq!( + inputs_of(&label_only, "//external:rules_go++go_sdk+go_toolchains"), + ["//external:rules_go++go_sdk+go_default_sdk"] + ); + let cquery = lower_repositories( repositories(), &mapping, diff --git a/src/hash.rs b/src/hash.rs index 9084eddf..5ccb0a24 100644 --- a/src/hash.rs +++ b/src/hash.rs @@ -1072,12 +1072,6 @@ impl HashContext<'_> { builder.direct(self.always_affected_seed); } - // Only a listed extension has views, so a run without the flag never resolves the - // rule's repository, which can cost a `bazel mod` call. - let extension = (!self.options.bazel.use_cquery - && !self.options.bazel.label_only_module_extensions.is_empty()) - .then(|| self.rule_extension(&rule.rule.name)) - .flatten(); let mut inputs: Vec> = if rule.input_positions.is_some() { rule.rule .rule_input @@ -1122,29 +1116,10 @@ impl HashContext<'_> { } } for (position, encoded_input) in inputs.into_iter().enumerate() { - let mut input_slot = rule + let input_slot = rule .input_positions .as_ref() .and_then(|ids| ids.get(position).copied().flatten()); - // A target in an extension's repository reaches its owning module through the view - // without that extension's repositories, when there is one (query mode only). - let encoded_input = match extension.as_deref().and_then(|extension| { - let node = encoded_input.strip_prefix("//external:")?; - let view = format!( - "//external:{}", - crate::bazel::extension_view_node(node, extension) - ); - self.rules - .get(&view) - .is_some_and(|slot| slot.rule.is_some()) - .then_some(view) - }) { - Some(view) => { - input_slot = None; - Cow::Owned(view) - } - None => encoded_input, - }; builder.direct(encoded_input.as_bytes()); let input_label = target_label(&encoded_input); match self.resolve_input(&encoded_input, input_label, input_slot, path) { @@ -1266,24 +1241,6 @@ impl HashContext<'_> { .and_then(|slot| slot.resolving(Cow::Owned(fallback.clone()))) } - /// The usage prefix of the module extension that generated the repository `label` is in - /// (`rules_rs++crate` for `@@rules_rs++crate+crates__anyhow//:anyhow`), if any. - fn rule_extension(&self, label: &str) -> Option { - let label = target_label(label); - let repo = label.strip_prefix('@')?.split_once("//")?.0; - let canonical = match repo.strip_prefix('@') { - Some("") => return None, - Some(canonical) => canonical, - None => self - .source_hasher - .external_resolver - .repo_mapping() - .get(repo)? - .as_str(), - }; - crate::bazel::extension_usage_prefix(canonical).map(str::to_owned) - } - fn rule_bzl_seed(&mut self, rule: &Rule) -> Result> { if rule.instantiation_stack.is_empty() { return Ok(None); diff --git a/src/main.rs b/src/main.rs index c182d97d..6b3cb7a4 100644 --- a/src/main.rs +++ b/src/main.rs @@ -119,10 +119,10 @@ struct HashingArgs { #[arg( long = "labelOnlyModuleExtensions", value_delimiter = ',', - help = "Module extensions (`@rules_rs//rs:extensions.bzl%crate`) whose repositories use each \ - other only through labels, never by resolving a toolchain from one another. A \ - fine-grained target in one of their repositories then does not change whenever any \ - other repository of the extension does" + help = "Module extensions (`@rules_rs//rs:extensions.bzl%crate`) whose repositories are \ + used only through labels, never by resolving a toolchain from one of them. The \ + module that owns such an extension then does not stand in for its repositories, so \ + a lockfile bump impacts only the targets that depend on the bumped repository" )] label_only_module_extensions: Vec, diff --git a/tests/e2e/external.rs b/tests/e2e/external.rs index 7bbc1231..c70d638d 100644 --- a/tests/e2e/external.rs +++ b/tests/e2e/external.rs @@ -370,6 +370,69 @@ fn fine_grained_spoke_does_not_reach_its_siblings_through_the_owning_module() { ); } +// Issue #603. Module `ext` owns the `@crates` extension, whose spokes are named after their +// version the way rules_js's `npm` names `npm__user-agents__2.1.229`, so a bump replaces spoke `b` +// rather than changing it. In query mode `//external:ext` depends on every repository its +// extensions generate, so the bump reaches every target that uses `ext`'s own rule (`//:uses_ext`, +// like a js_binary) and every target of a module with a `bazel_dep` path to `ext` +// (`//:uses_dependent`, whose `dependent` reaches `ext` through `middle`, like rules_jest reaching +// gazelle). Neither names anything in `@crates`. With the extension listed in +// `--labelOnlyModuleExtensions`, `ext` no longer stands in for its repositories, so the bump +// impacts only the hub's consumers, with or without fine-graining; a change to `ext` itself must +// still reach both. +#[test] +fn label_only_extension_bump_does_not_reach_users_of_the_owner_or_its_dependents() { + if !supports_mod_show_repo() { + return; + } + let label_only = ["--labelOnlyModuleExtensions", "@ext//:crates.bzl%crates"]; + let fine_grained = [ + label_only[0], + label_only[1], + "--fineGrainedHashExternalRepos", + "@crates", + ]; + let main = |impacted: BTreeSet| -> BTreeSet { + impacted + .into_iter() + .map(|label| label.trim_start_matches("@@").to_owned()) + .filter(|label| label.starts_with("//") && !label.starts_with("//external:")) + .collect() + }; + let set = |labels: &[&str]| -> BTreeSet { + labels.iter().map(|label| label.to_string()).collect() + }; + let first = copy_workspace("extension_owner_dependent_module"); + let bumped = copy_workspace("extension_owner_dependent_module"); + edit(&bumped.path().join("versions.json"), |text| { + text.replace("\"b\": \"1.0\"", "\"b\": \"2.0\"") + }); + // Without the opt-in the conservative owner edge stays and the bump reaches both, which is + // what the issue reports. + let impacted = main(diff_local(first.path(), bumped.path(), &[])); + assert!( + impacted.contains("//:uses_ext") && impacted.contains("//:uses_dependent"), + "{impacted:?}" + ); + + // Without fine-graining every consumer of the `@crates` hub sees the bump. + let impacted = main(diff_local(first.path(), bumped.path(), &label_only)); + assert_eq!(impacted, set(&["//:uses_a", "//:uses_b"]), "label-only"); + let impacted = main(diff_local(first.path(), bumped.path(), &fine_grained)); + assert_eq!(impacted, set(&["//:uses_b"]), "label-only, fine-grained"); + + // `dependent` and `//:uses_ext` really depend on `ext`: a change to its own tree reaches them. + let changed_owner = copy_workspace("extension_owner_dependent_module"); + fs::write(changed_owner.path().join("ext/tool.sh"), "exit 1\n").unwrap(); + for flags in [&label_only[..], &fine_grained[..]] { + let impacted = main(diff_local(first.path(), changed_owner.path(), flags)); + assert!( + impacted.contains("//:uses_ext") && impacted.contains("//:uses_dependent"), + "{flags:?}: {impacted:?}" + ); + } +} + const ISOLATED_ROOT: &[&str] = &["--isolatedDependencyRoot", "@hub"]; fn assert_product(workspace: &std::path::Path, target: &str, file: &str, expected: &str) { diff --git a/tests/resources/workspaces/extension_owner_dependent_module/.bazelignore b/tests/resources/workspaces/extension_owner_dependent_module/.bazelignore new file mode 100644 index 00000000..285b146f --- /dev/null +++ b/tests/resources/workspaces/extension_owner_dependent_module/.bazelignore @@ -0,0 +1,3 @@ +ext +middle +dependent diff --git a/tests/resources/workspaces/extension_owner_dependent_module/BUILD.bazel b/tests/resources/workspaces/extension_owner_dependent_module/BUILD.bazel new file mode 100644 index 00000000..49e77971 --- /dev/null +++ b/tests/resources/workspaces/extension_owner_dependent_module/BUILD.bazel @@ -0,0 +1,26 @@ +load("@dependent//:defs.bzl", "wrap") +load("@ext//:defs.bzl", "crate") + +filegroup( + name = "uses_a", + srcs = ["@crates//:a"], +) + +filegroup( + name = "uses_b", + srcs = ["@crates//:b"], +) + +# Uses `ext`'s own rule and nothing from `@crates`, like a js_binary that does not +# depend on the bumped npm package. +crate( + name = "uses_ext", + srcs = ["input.txt"], +) + +# Uses a rule of `dependent` and nothing from `@crates`, like a `jest_test` from +# rules_jest. +wrap( + name = "uses_dependent", + srcs = ["input.txt"], +) diff --git a/tests/resources/workspaces/extension_owner_dependent_module/MODULE.bazel b/tests/resources/workspaces/extension_owner_dependent_module/MODULE.bazel new file mode 100644 index 00000000..1d357cef --- /dev/null +++ b/tests/resources/workspaces/extension_owner_dependent_module/MODULE.bazel @@ -0,0 +1,19 @@ +module(name = "extension_owner_dependent_module_test") + +bazel_dep(name = "ext") +local_path_override(module_name = "ext", path = "ext") + +bazel_dep(name = "dependent") +local_path_override(module_name = "dependent", path = "dependent") + +local_path_override(module_name = "middle", path = "middle") + +# Module `ext` owns an extension that generates a hub `@crates` and one spoke per +# entry of versions.json, named after its version, like rules_js's `npm` or +# gazelle's `go_deps`. Root targets use `ext`'s rule directly, like a js_binary +# uses rules_js. Module `dependent` reaches `ext` only through `middle`'s +# `bazel_dep`, like rules_jest -> rules_js -> protobuf -> rules_android -> gazelle. +# See issue #603 and external.rs. +crates = use_extension("@ext//:crates.bzl", "crates") +crates.lock(file = "//:versions.json") +use_repo(crates, "crates") diff --git a/tests/resources/workspaces/extension_owner_dependent_module/dependent/BUILD.bazel b/tests/resources/workspaces/extension_owner_dependent_module/dependent/BUILD.bazel new file mode 100644 index 00000000..13bdef8e --- /dev/null +++ b/tests/resources/workspaces/extension_owner_dependent_module/dependent/BUILD.bazel @@ -0,0 +1 @@ +exports_files(["tool.sh"]) diff --git a/tests/resources/workspaces/extension_owner_dependent_module/dependent/MODULE.bazel b/tests/resources/workspaces/extension_owner_dependent_module/dependent/MODULE.bazel new file mode 100644 index 00000000..67b92f3e --- /dev/null +++ b/tests/resources/workspaces/extension_owner_dependent_module/dependent/MODULE.bazel @@ -0,0 +1,3 @@ +module(name = "dependent") + +bazel_dep(name = "middle") diff --git a/tests/resources/workspaces/extension_owner_dependent_module/dependent/defs.bzl b/tests/resources/workspaces/extension_owner_dependent_module/dependent/defs.bzl new file mode 100644 index 00000000..ed48e425 --- /dev/null +++ b/tests/resources/workspaces/extension_owner_dependent_module/dependent/defs.bzl @@ -0,0 +1,10 @@ +def _wrap_impl(ctx): + return [DefaultInfo(files = depset(ctx.files.srcs))] + +wrap = rule( + implementation = _wrap_impl, + attrs = { + "srcs": attr.label_list(allow_files = True), + "_tool": attr.label(default = "@dependent//:tool.sh", allow_single_file = True), + }, +) diff --git a/tests/resources/workspaces/extension_owner_dependent_module/dependent/tool.sh b/tests/resources/workspaces/extension_owner_dependent_module/dependent/tool.sh new file mode 100644 index 00000000..ca916d09 --- /dev/null +++ b/tests/resources/workspaces/extension_owner_dependent_module/dependent/tool.sh @@ -0,0 +1 @@ +exit 0 diff --git a/tests/resources/workspaces/extension_owner_dependent_module/ext/BUILD.bazel b/tests/resources/workspaces/extension_owner_dependent_module/ext/BUILD.bazel new file mode 100644 index 00000000..13bdef8e --- /dev/null +++ b/tests/resources/workspaces/extension_owner_dependent_module/ext/BUILD.bazel @@ -0,0 +1 @@ +exports_files(["tool.sh"]) diff --git a/tests/resources/workspaces/extension_owner_dependent_module/ext/MODULE.bazel b/tests/resources/workspaces/extension_owner_dependent_module/ext/MODULE.bazel new file mode 100644 index 00000000..deccca41 --- /dev/null +++ b/tests/resources/workspaces/extension_owner_dependent_module/ext/MODULE.bazel @@ -0,0 +1,6 @@ +module(name = "ext") + +# Like a rules module that uses its own extension for its tools: `ext` imports a +# repository the extension generates, which defines no toolchain. +crates = use_extension("//:crates.bzl", "crates") +use_repo(crates, "ext_tools") diff --git a/tests/resources/workspaces/extension_owner_dependent_module/ext/crates.bzl b/tests/resources/workspaces/extension_owner_dependent_module/ext/crates.bzl new file mode 100644 index 00000000..e2a44e0d --- /dev/null +++ b/tests/resources/workspaces/extension_owner_dependent_module/ext/crates.bzl @@ -0,0 +1,34 @@ +def _spoke_impl(rctx): + rctx.file("data.txt", rctx.attr.version) + rctx.file("BUILD.bazel", """load("@ext//:defs.bzl", "crate") +crate(name = "lib", srcs = ["data.txt"], visibility = ["//visibility:public"]) +""") + +spoke = repository_rule(implementation = _spoke_impl, attrs = {"version": attr.string()}) + +def _hub_impl(rctx): + rctx.file("BUILD.bazel", "\n".join([ + 'alias(name = "{}", actual = "@{}//:lib", visibility = ["//visibility:public"])'.format(name, spoke) + for name, spoke in rctx.attr.spokes.items() + ])) + +hub = repository_rule(implementation = _hub_impl, attrs = {"spokes": attr.string_dict()}) + +def _crates_impl(mctx): + versions = {} + for mod in mctx.modules: + for lock in mod.tags.lock: + versions.update(json.decode(mctx.read(lock.file))) + spokes = {} + for name, version in versions.items(): + # Like npm's `npm__user-agents__2.1.229`: a bump replaces the spoke rather + # than changing it in place. + spokes[name] = "crates__{}__{}".format(name, version.replace(".", "_")) + spoke(name = spokes[name], version = version) + hub(name = "crates", spokes = spokes) + hub(name = "ext_tools", spokes = {}) + +crates = module_extension( + implementation = _crates_impl, + tag_classes = {"lock": tag_class(attrs = {"file": attr.label()})}, +) diff --git a/tests/resources/workspaces/extension_owner_dependent_module/ext/defs.bzl b/tests/resources/workspaces/extension_owner_dependent_module/ext/defs.bzl new file mode 100644 index 00000000..86fd4345 --- /dev/null +++ b/tests/resources/workspaces/extension_owner_dependent_module/ext/defs.bzl @@ -0,0 +1,10 @@ +def _crate_impl(ctx): + return [DefaultInfo(files = depset(ctx.files.srcs))] + +crate = rule( + implementation = _crate_impl, + attrs = { + "srcs": attr.label_list(allow_files = True), + "_tool": attr.label(default = "@ext//:tool.sh", allow_single_file = True), + }, +) diff --git a/tests/resources/workspaces/extension_owner_dependent_module/ext/tool.sh b/tests/resources/workspaces/extension_owner_dependent_module/ext/tool.sh new file mode 100644 index 00000000..ca916d09 --- /dev/null +++ b/tests/resources/workspaces/extension_owner_dependent_module/ext/tool.sh @@ -0,0 +1 @@ +exit 0 diff --git a/tests/resources/workspaces/extension_owner_dependent_module/input.txt b/tests/resources/workspaces/extension_owner_dependent_module/input.txt new file mode 100644 index 00000000..ce013625 --- /dev/null +++ b/tests/resources/workspaces/extension_owner_dependent_module/input.txt @@ -0,0 +1 @@ +hello diff --git a/tests/resources/workspaces/extension_owner_dependent_module/middle/BUILD.bazel b/tests/resources/workspaces/extension_owner_dependent_module/middle/BUILD.bazel new file mode 100644 index 00000000..e69de29b diff --git a/tests/resources/workspaces/extension_owner_dependent_module/middle/MODULE.bazel b/tests/resources/workspaces/extension_owner_dependent_module/middle/MODULE.bazel new file mode 100644 index 00000000..1d495a74 --- /dev/null +++ b/tests/resources/workspaces/extension_owner_dependent_module/middle/MODULE.bazel @@ -0,0 +1,3 @@ +module(name = "middle") + +bazel_dep(name = "ext") diff --git a/tests/resources/workspaces/extension_owner_dependent_module/versions.json b/tests/resources/workspaces/extension_owner_dependent_module/versions.json new file mode 100644 index 00000000..1dd29ea0 --- /dev/null +++ b/tests/resources/workspaces/extension_owner_dependent_module/versions.json @@ -0,0 +1 @@ +{"a": "1.0", "b": "1.0"} diff --git a/tools/e2e/rust_e2e_cases.bzl b/tools/e2e/rust_e2e_cases.bzl index 53cda3f8..14ac6d94 100644 --- a/tools/e2e/rust_e2e_cases.bzl +++ b/tools/e2e/rust_e2e_cases.bzl @@ -131,6 +131,10 @@ RUST_E2E_SUITES = [ "name": "external::isolated_dependency_root_tracks_repository_read_bytes", "timeout": "moderate", }, + { + "name": "external::label_only_extension_bump_does_not_reach_users_of_the_owner_or_its_dependents", + "timeout": "moderate", + }, { "name": "external::remote_proto_bump_impacts_consumer", "timeout": "moderate", diff --git a/tools/readme_template.md b/tools/readme_template.md index 4173a6d6..7af963f4 100644 --- a/tools/readme_template.md +++ b/tools/readme_template.md @@ -624,13 +624,16 @@ A few inputs are normalized or extended on the way in: in the workspace, a `file://%workspace%` registry URL) is hashed relative to the workspace, so identical checkouts at different paths hash the same. In query mode the module that owns a module extension stands in for every repository the - extension generates, because a toolchain resolved from one of them is not a query input. A - fine-grained target inside one of those repositories that uses a rule from the owning module - therefore changes whenever any repository of the extension does. Listing the extension in - `--labelOnlyModuleExtensions` (for example `@rules_rs//rs:extensions.bzl%crate`) asserts that - its repositories use each other only through labels, so such a target reaches the owning module - without them. Do not list an extension whose targets resolve a toolchain the same extension - generates: that change would be missed. An extension whose name another `.bzl` file of the same + extension generates, because a toolchain resolved from one of them is not a query input. Every + target that uses a rule of the owning module, every target of a module with a `bazel_dep` path + to it, and every fine-grained target inside one of those repositories therefore changes whenever + any repository of the extension does: a `go.mod` bump through gazelle's `go_deps` or a + `pnpm-lock.yaml` bump through rules_js's `npm` impacts most of the workspace. Listing the + extension in `--labelOnlyModuleExtensions` (for example + `@gazelle//:extensions.bzl%go_deps` or `@rules_rs//rs:extensions.bzl%crate`) asserts that its + repositories are used only through labels, which the query reports, so the owning module no + longer stands in for them. Do not list an extension that generates a toolchain, such as + `rules_go`'s `go_sdk`: a change to it would be missed. An extension whose name another `.bzl` file of the same module also exports is reported and ignored, since Bazel tells their repositories apart only by a numeric suffix. `--isolatedDependencyRoot=@crates` asserts that every outside consumer of this