From 8bd98f8e6fe1d99de2d001cf8c20b6d882e373de Mon Sep 17 00:00:00 2001 From: Ryan Northey Date: Mon, 24 Aug 2026 19:37:57 +0000 Subject: [PATCH 1/3] bazel/deps: Handle transitions in reachability aspect Signed-off-by: Ryan Northey --- bazel/dependency/BUILD | 6 + bazel/dependency/reachability.bzl | 594 +++++++++++++++--- bazel/dependency/test/BUILD | 149 ++++- .../test/exclusions_reachability_test.sh | 17 + .../reachability_config_validation_test.bzl | 31 + .../test/reachability_merge_defines_test.bzl | 27 + .../reachability_package_pattern_test.bzl | 38 ++ bazel/dependency/test/reachability_test.sh | 90 ++- .../test/reachability_test_rules.bzl | 11 + .../test/transitive/chain/one/BUILD | 6 + .../test/transitive/chain/two/BUILD | 6 + .../test/transitive/ext/config_default/BUILD | 6 + .../test/transitive/ext/config_extra/BUILD | 6 + .../dependency/test/transitive/ext/deep/BUILD | 6 + .../test/transitive/ext/direct/BUILD | 6 + .../test/transitive/ext/excluded/BUILD | 6 + .../dependency/test/transitive/ext/one/BUILD | 6 + .../test/transitive/ext/testonly/BUILD | 7 + .../dependency/test/transitive/ext/two/BUILD | 6 + bazel/dependency/test/transitive/shared/BUILD | 6 + 20 files changed, 942 insertions(+), 88 deletions(-) create mode 100644 bazel/dependency/test/reachability_config_validation_test.bzl create mode 100644 bazel/dependency/test/reachability_merge_defines_test.bzl create mode 100644 bazel/dependency/test/reachability_package_pattern_test.bzl create mode 100644 bazel/dependency/test/reachability_test_rules.bzl create mode 100644 bazel/dependency/test/transitive/chain/one/BUILD create mode 100644 bazel/dependency/test/transitive/chain/two/BUILD create mode 100644 bazel/dependency/test/transitive/ext/config_default/BUILD create mode 100644 bazel/dependency/test/transitive/ext/config_extra/BUILD create mode 100644 bazel/dependency/test/transitive/ext/deep/BUILD create mode 100644 bazel/dependency/test/transitive/ext/direct/BUILD create mode 100644 bazel/dependency/test/transitive/ext/excluded/BUILD create mode 100644 bazel/dependency/test/transitive/ext/one/BUILD create mode 100644 bazel/dependency/test/transitive/ext/testonly/BUILD create mode 100644 bazel/dependency/test/transitive/ext/two/BUILD create mode 100644 bazel/dependency/test/transitive/shared/BUILD diff --git a/bazel/dependency/BUILD b/bazel/dependency/BUILD index 625d6447cc..1cf128bf89 100644 --- a/bazel/dependency/BUILD +++ b/bazel/dependency/BUILD @@ -23,6 +23,12 @@ string_list_flag( visibility = ["//visibility:public"], ) +string_list_flag( + name = "_attribution_patterns", + build_setting_default = [], + visibility = ["//visibility:public"], +) + updater( name = "update", dependencies = "//:dependency_versions", diff --git a/bazel/dependency/reachability.bzl b/bazel/dependency/reachability.bzl index 7c36fd5ba1..44ccf3d9fe 100644 --- a/bazel/dependency/reachability.bzl +++ b/bazel/dependency/reachability.bzl @@ -27,6 +27,39 @@ sh_test( ) ``` +For multi-configuration analysis, construct a reachability rule whose transition +outputs are fixed up front, then pass a config matrix: + +```starlark +load( + "@envoy_toolshed//dependency:reachability.bzl", + "dependency_reachability_macro", + "dependency_reachability_rule", +) + +_envoy_dependency_reachability = dependency_reachability_rule( + flags = ["//bazel:wasm_runtime"], + defines = True, +) + +envoy_dependency_reachability = dependency_reachability_macro(_envoy_dependency_reachability) + +envoy_dependency_reachability( + name = "dep-reachability", + roots = ["//source/exe:envoy_main_common_with_core_extensions_lib"], + configs = { + "default": {}, + "wasmtime": {"//bazel:wasm_runtime": "wasmtime"}, + "wamr": {"//bazel:wasm_runtime": "wamr"}, + "legacy-define": {"wasm": "v8"}, + }, +) +``` + +`flags` must be fixed at construction because transition `outputs` are static: +they cannot vary per target instantiation. Prefer Starlark build settings where +possible. `defines = True` is supported for legacy `--define`-based matrices. + Building the target writes `.json`: ```json @@ -35,6 +68,7 @@ Building the target writes `.json`: "": { "name": "", "production": true, + "configs": ["default"], "reached_by": [ {"root": "//source/exe:envoy_main_common_with_core_extensions_lib", "production": true} ], @@ -47,6 +81,13 @@ Building the target writes `.json`: "attrs": ["deps"], "roots": ["//source/exe:envoy_main_common_with_core_extensions_lib"] } + ], + "attributed_packages": [ + { + "package": "//source/extensions/filters/network/example", + "production": true, + "roots": ["//source/exe:envoy_main_common_with_core_extensions_lib"] + } ] } } @@ -73,6 +114,31 @@ Building the target writes `.json`: | "\\(.target)\\t\\(.attrs | join(","))"' dep-reachability.json ``` +- `attributed_packages` is present only when `attribution_patterns` is + non-empty. It lists the matching main-repo packages whose transitive closure + reaches the external repository, even when the only direct cross-repo edge is + inside a shared non-matching package. `production` is true when at least one + non-testonly path exists from that package to the repository; otherwise the + attribution is testonly-only. This keeps the output proportional to the + number of matching packages rather than to every intra-repo edge. For + example, to ask which extension packages consume a repository: + + ```console + jq -r '.dependencies[""].attributed_packages[] + | select(.production) + | .package' dep-reachability.json + ``` + +- `configs` lists the analyzed config names in which the repository is reached. + Reachability data is only as complete as the declared matrix: if a dependency + is reachable in no declared config, it is absent from the emitted JSON. + +When a dependency is reached in multiple analyzed configs, data is merged by +union semantics: `targets`/`configs` are unioned, `consumers[*].roots` and +`consumers[*].attrs` are unioned, `attributed_packages[*].roots` are unioned, +and `production`/`reached_by[*].production`/`consumers[*].testonly`/ +`attributed_packages[*].production` are merged with logical OR. + The aspect emits raw truth: no repository is filtered. Policy (ignore lists, test-only exemptions, bucketing by surface/extension/contrib) belongs in the consumer of the JSON. @@ -92,14 +158,24 @@ Exclusion settings (`excluded_edges`, `excluded_patterns`) are transported via Starlark build settings rather than `--features`, so they survive Bazel's exec configuration transition. They therefore apply uniformly across target and exec configurations: edges reached through `cfg = "exec"` attributes are excluded -just as reliably as edges in the target configuration. +just as reliably as edges in the target configuration. The same transport is +used for `attribution_patterns`, so opted-in package attribution applies +uniformly across all analyzed configurations. All three settings are carried +through every branch of the split transition. Note on exclusion semantics: an excluded repository is neither recorded nor descended into. Pruning descent means that repositories reachable *only through* an excluded repository also disappear from the output, even if they do not themselves match any exclusion pattern. This is a deliberate trade-off: it keeps the walk bounded but means the output can depend on which repository sits first -on a path. +on a path. The same pruning applies to `attributed_packages`: excluded +repositories are not attributed, and packages are not attributed to repositories +reachable only through an excluded repository. + +`attribution_patterns` is opt-in and defaults to empty. When left unset, the +emitted JSON is byte-identical to the historical output and no attribution sets +are propagated through the aspect. Opt in only when a consumer specifically +needs transitive main-repo package attribution. """ load("@bazel_skylib//rules:common_settings.bzl", "BuildSettingInfo") @@ -111,15 +187,32 @@ load("@bazel_skylib//rules:common_settings.bzl", "BuildSettingInfo") # for direct use; they are an implementation detail of dependency_reachability. _EXCLUDED_EDGES_SETTING = "//dependency:_excluded_edges" _EXCLUDED_PATTERNS_SETTING = "//dependency:_excluded_patterns" +_ATTRIBUTION_PATTERNS_SETTING = "//dependency:_attribution_patterns" DependencyReachabilityInfo = provider( - doc = "Cross-repository dependency edges in a target's transitive closure.", + doc = "Cross-repository dependency edges (and optional package attributions) in a target's transitive closure.", fields = { "edges": "depset of edge structs for all reachable cross-repo edges", "production_edges": ( "depset of edge structs reachable without traversing any " + "testonly target" ), + "repos": ( + "depset of reachable canonical repository names when package " + + "attribution is enabled, else None" + ), + "production_repos": ( + "depset of reachable canonical repository names on at least one " + + "non-testonly path when package attribution is enabled, else None" + ), + "attributed_packages": ( + "dict(repo -> depset(package strings)) for matching main-repo " + + "packages that transitively reach each repo, else None" + ), + "production_attributed_packages": ( + "dict(repo -> depset(package strings)) for matching main-repo " + + "packages with at least one non-testonly path to each repo, else None" + ), }, ) @@ -144,6 +237,11 @@ def _label_string(label): return "//%s:%s" % (label.package, label.name) return str(label) +def _package_string(label): + if label.package: + return "//%s" % label.package + return "//" + def _attr_targets(rule_attr, name): value = getattr(rule_attr, name, None) if value == None: @@ -172,30 +270,117 @@ def _repo_is_excluded(repo_name, patterns): return True return False -def _reachability_transition_impl(settings, attr): +def _matches_package_pattern(package, pattern): + if pattern == "//...": + return True + if pattern.endswith("/..."): + prefix = pattern[:-4] + return package == prefix or package.startswith(prefix + "/") + return package == pattern + +def _package_matches_patterns(package, patterns): + for pattern in patterns: + if _matches_package_pattern(package, pattern): + return True + return False + +package_pattern_matches = _matches_package_pattern + +def _append_repo_package_depsets(dest, source): + if source == None: + return + for repo in source.keys(): + dest.setdefault(repo, []).append(source[repo]) + +def _add_repo_package(dest, repo, package): + dest.setdefault(repo, {})[package] = True + +def _materialize_repo_packages(direct, transitive): + repos = {} + for repo in direct.keys(): + repos[repo] = True + for repo in transitive.keys(): + repos[repo] = True return { - _EXCLUDED_EDGES_SETTING: attr.excluded_edges, - _EXCLUDED_PATTERNS_SETTING: attr.excluded_patterns, + repo: depset( + direct = sorted(direct.get(repo, {}).keys()), + transitive = transitive.get(repo, []), + ) + for repo in sorted(repos.keys()) } -_reachability_transition = transition( - implementation = _reachability_transition_impl, - inputs = [], - outputs = [ - _EXCLUDED_EDGES_SETTING, - _EXCLUDED_PATTERNS_SETTING, - ], -) +def _decode_configs(configs_attr): + configs = {} + for config in sorted(configs_attr.keys()): + values = {} + for assignment in configs_attr[config]: + if "=" not in assignment: + fail("Invalid config assignment '{}' in config '{}' (expected '=')".format(assignment, config)) + key, value = assignment.split("=", 1) + values[key] = value + configs[config] = values + if not configs: + return {"default": {}} + return configs + +def config_validation_error(configs, flags, defines): + allowed = {flag: True for flag in flags} + declared = sorted(flags) + for config in sorted(configs.keys()): + for label in sorted(configs[config].keys()): + if label.startswith("//"): + if label not in allowed: + return "Config '{}' varies '{}' but it is not declared in flags. Declared flags: {}".format( + config, + label, + declared, + ) + elif not defines: + return "Config '{}' varies define '{}' but this rule was constructed with defines = False".format( + config, + label, + ) + return None + +def _validate_config_labels(configs, flags, defines): + error = config_validation_error(configs, flags, defines) + if error != None: + fail(error) + +def _merge_defines(existing, values): + merged = {} + for define in existing: + if "=" not in define: + continue + key, value = define.split("=", 1) + merged[key] = value + for key in sorted(values.keys()): + if not key.startswith("//"): + merged[key] = values[key] + return ["{}={}".format(key, merged[key]) for key in sorted(merged.keys())] + +merge_defines = _merge_defines def _reachability_aspect_impl(target, ctx): consumer = _label_string(target.label) consumer_repo = target.label.repo_name + consumer_package = _package_string(target.label) testonly = bool(getattr(ctx.rule.attr, "testonly", False)) excluded_edges = {edge: True for edge in ctx.attr._excluded_edges[BuildSettingInfo].value} excluded_patterns = ctx.attr._excluded_patterns[BuildSettingInfo].value + attribution_patterns = ctx.attr._attribution_patterns[BuildSettingInfo].value + collect_attributions = bool(attribution_patterns) edges = [] transitive = [] transitive_production = [] + reachable_repos = [] + reachable_repos_transitive = [] + production_reachable_repos = [] + production_reachable_repos_transitive = [] + attributed_packages_direct = {} + attributed_packages_transitive = {} + production_attributed_packages_direct = {} + production_attributed_packages_transitive = {} for attr_name in [a for a in dir(ctx.rule.attr) if not a.startswith("_")]: if attr_name in excluded_edges: continue @@ -213,10 +398,36 @@ def _reachability_aspect_impl(target, ctx): target = _label_string(dep.label), testonly = testonly, )) + if collect_attributions: + reachable_repos.append(dep_repo) + if not testonly: + production_reachable_repos.append(dep_repo) if DependencyReachabilityInfo in dep: info = dep[DependencyReachabilityInfo] transitive.append(info.edges) transitive_production.append(info.production_edges) + if collect_attributions: + reachable_repos_transitive.append(info.repos) + if not testonly: + production_reachable_repos_transitive.append(info.production_repos) + _append_repo_package_depsets(attributed_packages_transitive, info.attributed_packages) + if not testonly: + _append_repo_package_depsets( + production_attributed_packages_transitive, + info.production_attributed_packages, + ) + if collect_attributions and not consumer_repo and _package_matches_patterns(consumer_package, attribution_patterns): + for repo in depset( + direct = reachable_repos, + transitive = reachable_repos_transitive, + ).to_list(): + _add_repo_package(attributed_packages_direct, repo, consumer_package) + if not testonly: + for repo in depset( + direct = production_reachable_repos, + transitive = production_reachable_repos_transitive, + ).to_list(): + _add_repo_package(production_attributed_packages_direct, repo, consumer_package) return [DependencyReachabilityInfo( edges = depset(edges, transitive = transitive), production_edges = ( @@ -224,6 +435,42 @@ def _reachability_aspect_impl(target, ctx): if testonly else depset(edges, transitive = transitive_production) ), + repos = ( + None + if not collect_attributions + else depset( + direct = reachable_repos, + transitive = reachable_repos_transitive, + ) + ), + production_repos = ( + None + if not collect_attributions + else ( + depset() + if testonly + else depset( + direct = production_reachable_repos, + transitive = production_reachable_repos_transitive, + ) + ) + ), + attributed_packages = ( + None + if not collect_attributions + else _materialize_repo_packages( + attributed_packages_direct, + attributed_packages_transitive, + ) + ), + production_attributed_packages = ( + None + if not collect_attributions + else _materialize_repo_packages( + production_attributed_packages_direct, + production_attributed_packages_transitive, + ) + ), )] reachability_aspect = aspect( @@ -238,6 +485,10 @@ reachability_aspect = aspect( default = _EXCLUDED_PATTERNS_SETTING, providers = [BuildSettingInfo], ), + "_attribution_patterns": attr.label( + default = _ATTRIBUTION_PATTERNS_SETTING, + providers = [BuildSettingInfo], + ), }, doc = ( "Collects cross-repository dependency edges over all public " + @@ -246,88 +497,194 @@ reachability_aspect = aspect( "each edge arrives on. Implicit/private attributes " + "(_-prefixed) are skipped at recording time. Resolved toolchain " + "dependencies are not visited because toolchains_aspects is not set. " + - "Exclusion settings are read from Starlark build settings " + - "(_excluded_edges, _excluded_patterns) that survive the exec " + - "configuration transition, so exclusions apply uniformly across " + - "target and exec configurations." + "Exclusion settings and optional attribution patterns are read from " + + "Starlark build settings that survive the exec configuration " + + "transition, so they apply uniformly across target and exec " + + "configurations." ), ) -def _dependency_reachability_impl(ctx): - deps = {} - for target in ctx.attr.roots: - root = _label_string(target.label) - info = target[DependencyReachabilityInfo] - production = {edge: True for edge in info.production_edges.to_list()} - for edge in info.edges.to_list(): - entry = deps.setdefault(edge.repo, dict( - name = edge.name, - reached_by = {}, - targets = {}, - consumers = {}, - )) - entry["targets"][edge.target] = True - reached = entry["reached_by"].setdefault(root, dict( - production = False, - )) - if edge in production: - reached["production"] = True - consumer = entry["consumers"].setdefault(edge.consumer, dict( - repo = edge.consumer_repo, - testonly = edge.testonly, - attrs = {}, - roots = {}, - )) - consumer["attrs"][edge.attr] = True - consumer["roots"][root] = True - dependencies = {} - for repo in sorted(deps.keys()): - entry = deps[repo] - reached_by = [ - dict( - root = root, - production = entry["reached_by"][root]["production"], - ) - for root in sorted(entry["reached_by"].keys()) - ] - dependencies[repo] = dict( - name = entry["name"], - production = any([ - reached["production"] - for reached in reached_by - ]), - reached_by = reached_by, - targets = sorted(entry["targets"].keys()), - consumers = [ +def _record_edge(deps, config, root, edge, production): + entry = deps.setdefault(edge.repo, dict( + name = edge.name, + reached_by = {}, + targets = {}, + consumers = {}, + configs = {}, + attributed_packages = {}, + )) + entry["configs"][config] = True + entry["targets"][edge.target] = True + reached = entry["reached_by"].setdefault(root, dict( + production = False, + )) + if production: + reached["production"] = True + consumer = entry["consumers"].setdefault(edge.consumer, dict( + repo = edge.consumer_repo, + testonly = False, + attrs = {}, + roots = {}, + )) + consumer["testonly"] = consumer["testonly"] or edge.testonly + consumer["attrs"][edge.attr] = True + consumer["roots"][root] = True + +def _record_attributed_package(deps, config, root, repo, package, production): + entry = deps[repo] + entry["configs"][config] = True + attributed = entry["attributed_packages"].setdefault(package, dict( + production = False, + roots = {}, + )) + if production: + attributed["production"] = True + attributed["roots"][root] = True + +def _dependency_reachability_impl(): + def _impl(ctx): + deps = {} + emit_attributed_packages = bool(ctx.attr.attribution_patterns) + # Split transitions fan out each root once per config, so flattening + # depsets here scales with the declared matrix size. + for config in sorted(ctx.split_attr.roots.keys()): + for target in ctx.split_attr.roots[config]: + root = _label_string(target.label) + info = target[DependencyReachabilityInfo] + production = {edge: True for edge in info.production_edges.to_list()} + for edge in info.edges.to_list(): + _record_edge( + deps, + config, + root, + edge, + production = edge in production, + ) + if emit_attributed_packages: + production_attributed_packages = { + repo: { + package: True + for package in info.production_attributed_packages[repo].to_list() + } + for repo in sorted(info.production_attributed_packages.keys()) + } + for repo in sorted(info.attributed_packages.keys()): + for package in info.attributed_packages[repo].to_list(): + _record_attributed_package( + deps, + config, + root, + repo, + package, + production = package in production_attributed_packages.get(repo, {}), + ) + dependencies = {} + for repo in sorted(deps.keys()): + entry = deps[repo] + reached_by = [ dict( - target = consumer, - repo = entry["consumers"][consumer]["repo"], - testonly = entry["consumers"][consumer]["testonly"], - attrs = sorted(entry["consumers"][consumer]["attrs"].keys()), - roots = sorted(entry["consumers"][consumer]["roots"].keys()), + root = root, + production = entry["reached_by"][root]["production"], ) - for consumer in sorted(entry["consumers"].keys()) - ], + for root in sorted(entry["reached_by"].keys()) + ] + dependencies[repo] = dict( + name = entry["name"], + production = any([ + reached["production"] + for reached in reached_by + ]), + configs = sorted(entry["configs"].keys()), + reached_by = reached_by, + targets = sorted(entry["targets"].keys()), + consumers = [ + dict( + target = consumer, + repo = entry["consumers"][consumer]["repo"], + testonly = entry["consumers"][consumer]["testonly"], + attrs = sorted(entry["consumers"][consumer]["attrs"].keys()), + roots = sorted(entry["consumers"][consumer]["roots"].keys()), + ) + for consumer in sorted(entry["consumers"].keys()) + ], + ) + if emit_attributed_packages: + dependencies[repo]["attributed_packages"] = [ + dict( + package = package, + production = entry["attributed_packages"][package]["production"], + roots = sorted(entry["attributed_packages"][package]["roots"].keys()), + ) + for package in sorted(entry["attributed_packages"].keys()) + ] + output = ctx.actions.declare_file("%s.json" % ctx.label.name) + ctx.actions.write( + output = output, + content = json.encode_indent( + dict(dependencies = dependencies), + indent = " ", + ) + "\n", ) - output = ctx.actions.declare_file("%s.json" % ctx.label.name) - ctx.actions.write( - output = output, - content = json.encode_indent( - dict(dependencies = dependencies), - indent = " ", - ) + "\n", + return [DefaultInfo(files = depset([output]))] + + return _impl + +def _dependency_reachability_transition(flags, defines): + def _impl(settings, attr): + configs = _decode_configs(attr.configs) + _validate_config_labels(configs, flags, defines) + transitioned = {} + for config in sorted(configs.keys()): + values = configs[config] + output = {} + for flag in flags: + output[flag] = values.get(flag, settings[flag]) + if defines: + output["//command_line_option:define"] = _merge_defines( + settings["//command_line_option:define"], + values, + ) + # Carry exclusion settings through every branch of the split so they + # apply uniformly across all analyzed configurations. + output[_EXCLUDED_EDGES_SETTING] = attr.excluded_edges + output[_EXCLUDED_PATTERNS_SETTING] = attr.excluded_patterns + output[_ATTRIBUTION_PATTERNS_SETTING] = attr.attribution_patterns + transitioned[config] = output + return transitioned + + flag_inputs = list(flags) + if defines: + flag_inputs.append("//command_line_option:define") + transition_outputs = flag_inputs + [ + _EXCLUDED_EDGES_SETTING, + _EXCLUDED_PATTERNS_SETTING, + _ATTRIBUTION_PATTERNS_SETTING, + ] + return transition( + implementation = _impl, + inputs = flag_inputs, + outputs = transition_outputs, ) - return [DefaultInfo(files = depset([output]))] -def dependency_reachability_rule(): +def _dependency_reachability_rule(flags = [], defines = False): + """Construct the internal dependency_reachability rule. + + flags: list of build setting labels (string_flag/bool_flag/label_flag) that + instantiated targets may vary. Fixed at construction because transition + outputs must be static. + defines: whether --define may also be varied (adds + //command_line_option:define to outputs). Prefer Starlark settings; + this exists for legacy define-based consumers. + """ + reachability_transition = _dependency_reachability_transition(flags, defines) return rule( - implementation = _dependency_reachability_impl, + implementation = _dependency_reachability_impl(), attrs = { "roots": attr.label_list( aspects = [reachability_aspect], allow_files = True, mandatory = True, - cfg = _reachability_transition, + cfg = reachability_transition, doc = ( "Concrete root targets to analyze. Each entry must be a " + "resolved label — Bazel target patterns such as " + @@ -341,6 +698,15 @@ def dependency_reachability_rule(): "the root is." ), ), + "configs": attr.string_list_dict( + default = {"default": []}, + doc = ( + "Configuration matrix keyed by config name. Each value is " + + "a list of '=' assignments. " + + "Use the dependency_reachability macro form to pass a " + + "dict of assignment maps." + ), + ), "excluded_edges": attr.string_list( default = [], doc = ( @@ -359,6 +725,17 @@ def dependency_reachability_rule(): "matching." ), ), + "attribution_patterns": attr.string_list( + default = [], + doc = ( + "Optional main-repo package patterns to attribute " + + "transitively to each reachable external repository. " + + "Supported forms are exact package matches (`//pkg`) and " + + "recursive subtree matches (`//pkg/...`). When empty, " + + "attributed_packages is omitted and no attribution sets are " + + "propagated through the aspect." + ), + ), "_allowlist_function_transition": attr.label( default = "@bazel_tools//tools/allowlists/function_transition_allowlist", ), @@ -366,10 +743,55 @@ def dependency_reachability_rule(): doc = ( "Writes a JSON reachability map describing, for every external " + "repository reachable from the given root targets, which targets " + - "reach it and whether any non-testonly path exists. Root targets " + - "must be concrete labels — Bazel target patterns like " + - "`//source/extensions/...` cannot be used as rule attribute values." + "reach it and whether any non-testonly path exists. Optionally, " + + "matching main-repo packages can also be attributed transitively " + + "via attribution_patterns. Root targets must be concrete labels — " + + "Bazel target patterns like `//source/extensions/...` cannot be " + + "used as rule attribute values." ), ) -dependency_reachability = dependency_reachability_rule() +def dependency_reachability_macro(impl): + def _macro(name, roots, configs = None, **kwargs): + impl( + name = name, + roots = roots, + configs = _encode_configs(configs), + **kwargs + ) + + return _macro + +def _encode_configs(configs): + if configs == None: + return {"default": []} + if type(configs) != "dict": + fail("configs must be a dict of config-name -> dict(flag_or_define -> value)") + if not configs: + return {"default": []} + encoded = {} + for config in sorted(configs.keys()): + assignments = configs[config] + if type(assignments) != "dict": + fail("configs['{}'] must be a dict(flag_or_define -> value)".format(config)) + encoded[config] = [ + "{}={}".format(key, assignments[key]) + for key in sorted(assignments.keys()) + ] + return encoded + +def dependency_reachability_rule(flags = [], defines = False): + """Construct a dependency_reachability rule varying the given build settings. + + flags: list of build setting labels (string_flag/bool_flag/label_flag) that + instantiated targets may vary. Fixed at construction because transition + outputs must be static. + defines: whether --define may also be varied (adds + //command_line_option:define to outputs). Prefer Starlark settings; + this exists for legacy define-based consumers. + """ + return _dependency_reachability_rule(flags, defines) + +_dependency_reachability = dependency_reachability_rule() + +dependency_reachability = dependency_reachability_macro(_dependency_reachability) diff --git a/bazel/dependency/test/BUILD b/bazel/dependency/test/BUILD index 1d4f07f67d..3e02c8e57d 100644 --- a/bazel/dependency/test/BUILD +++ b/bazel/dependency/test/BUILD @@ -1,8 +1,13 @@ load("@bazel_skylib//:bzl_library.bzl", "bzl_library") +load("@bazel_skylib//rules:common_settings.bzl", "string_flag") load("@rules_shell//shell:sh_test.bzl", "sh_test") load("//dependency:reachability.bzl", "dependency_reachability") load(":custom_rule_test.bzl", "custom_library_rule") load(":reachability_apparent_name_test.bzl", "apparent_name_test") +load(":reachability_config_validation_test.bzl", "reachability_config_validation_test") +load(":reachability_merge_defines_test.bzl", "reachability_merge_defines_test") +load(":reachability_package_pattern_test.bzl", "reachability_package_pattern_test") +load(":reachability_test_rules.bzl", "dependency_reachability_with_mode") sh_test( name = "updater_test", @@ -48,12 +53,134 @@ dependency_reachability( ], ) +dependency_reachability( + name = "reachability_empty_attribution_patterns", + testonly = True, + roots = [ + ":core_root", + ":test_root", + ], + attribution_patterns = [], +) + +string_flag( + name = "reachability_mode", + build_setting_default = "default", + values = ["default", "extra"], +) + +config_setting( + name = "reachability_mode_extra", + flag_values = {":reachability_mode": "extra"}, +) + +filegroup( + name = "variant_external_consumer", + srcs = [ + "@bazel_skylib//lib:paths", + ] + select({ + ":reachability_mode_extra": ["@jq_toolchains//:resolved_toolchain"], + "//conditions:default": [], + }), +) + +filegroup( + name = "variant_core_root", + srcs = [":variant_external_consumer"], +) + +filegroup( + name = "variant_test_root", + testonly = True, + srcs = [ + ":variant_external_consumer", + "@bazel_skylib//lib:sets", + ], +) + +dependency_reachability_with_mode( + name = "reachability_multiconfig", + testonly = True, + roots = [ + ":variant_core_root", + ":variant_test_root", + ], + configs = { + "default": {}, + "extra": {"//dependency/test:reachability_mode": "extra"}, + }, +) + +filegroup( + name = "attribution_root", + srcs = [ + "//dependency/test/transitive/ext/deep:root", + "//dependency/test/transitive/ext/direct:root", + "//dependency/test/transitive/ext/one:root", + "//dependency/test/transitive/ext/two:root", + ], +) + +filegroup( + name = "attribution_testonly_root", + testonly = True, + srcs = ["//dependency/test/transitive/ext/testonly:root"], +) + +dependency_reachability( + name = "reachability_attribution", + testonly = True, + roots = [ + ":attribution_root", + ":attribution_testonly_root", + ], + attribution_patterns = ["//dependency/test/transitive/ext/..."], +) + +filegroup( + name = "attribution_variant_root", + srcs = select({ + ":reachability_mode_extra": ["//dependency/test/transitive/ext/config_extra:root"], + "//conditions:default": ["//dependency/test/transitive/ext/config_default:root"], + }), +) + +dependency_reachability_with_mode( + name = "reachability_attribution_multiconfig", + testonly = True, + roots = [":attribution_variant_root"], + attribution_patterns = ["//dependency/test/transitive/ext/..."], + configs = { + "default": {}, + "extra": {"//dependency/test:reachability_mode": "extra"}, + }, +) + +dependency_reachability( + name = "reachability_attribution_excluded_baseline", + testonly = True, + roots = ["//dependency/test/transitive/ext/excluded:root"], + attribution_patterns = ["//dependency/test/transitive/ext/..."], +) + +dependency_reachability( + name = "reachability_attribution_excluded", + testonly = True, + roots = ["//dependency/test/transitive/ext/excluded:root"], + attribution_patterns = ["//dependency/test/transitive/ext/..."], + excluded_patterns = ["rules_pkg*"], +) + sh_test( name = "reachability_test", size = "small", srcs = ["reachability_test.sh"], data = [ ":reachability", + ":reachability_empty_attribution_patterns", + ":reachability_attribution", + ":reachability_attribution_multiconfig", + ":reachability_multiconfig", "@jq_toolchains//:resolved_toolchain", ], env = { @@ -196,6 +323,8 @@ sh_test( ":reachability_excluded_rules_pkg_transitive", ":reachability_empty_exclusions", ":reachability_empty_exclusions_explicit", + ":reachability_attribution_excluded_baseline", + ":reachability_attribution_excluded", "@jq_toolchains//:resolved_toolchain", ], env = { @@ -206,7 +335,13 @@ sh_test( bzl_library( name = "reachability_apparent_name_test_lib", - srcs = ["reachability_apparent_name_test.bzl"], + srcs = [ + "reachability_apparent_name_test.bzl", + "reachability_config_validation_test.bzl", + "reachability_merge_defines_test.bzl", + "reachability_package_pattern_test.bzl", + "reachability_test_rules.bzl", + ], deps = [ "@bazel_skylib//lib:unittest", ], @@ -215,3 +350,15 @@ bzl_library( apparent_name_test( name = "apparent_name_test", ) + +reachability_config_validation_test( + name = "reachability_config_validation_test", +) + +reachability_merge_defines_test( + name = "reachability_merge_defines_test", +) + +reachability_package_pattern_test( + name = "reachability_package_pattern_test", +) diff --git a/bazel/dependency/test/exclusions_reachability_test.sh b/bazel/dependency/test/exclusions_reachability_test.sh index c77abaed75..fb29f46ae9 100755 --- a/bazel/dependency/test/exclusions_reachability_test.sh +++ b/bazel/dependency/test/exclusions_reachability_test.sh @@ -21,6 +21,8 @@ REACHABILITY_RULES_PKG_TRANSITIVE_JSON="${RUNFILES_DIR}/dependency/test/reachabi REACHABILITY_EXCLUDED_RULES_PKG_TRANSITIVE_JSON="${RUNFILES_DIR}/dependency/test/reachability_excluded_rules_pkg_transitive.json" REACHABILITY_EMPTY_EXCLUSIONS_JSON="${RUNFILES_DIR}/dependency/test/reachability_empty_exclusions.json" REACHABILITY_EMPTY_EXCLUSIONS_EXPLICIT_JSON="${RUNFILES_DIR}/dependency/test/reachability_empty_exclusions_explicit.json" +REACHABILITY_ATTRIBUTION_EXCLUDED_BASELINE_JSON="${RUNFILES_DIR}/dependency/test/reachability_attribution_excluded_baseline.json" +REACHABILITY_ATTRIBUTION_EXCLUDED_JSON="${RUNFILES_DIR}/dependency/test/reachability_attribution_excluded.json" FAILED=0 @@ -83,6 +85,21 @@ check "combined excluded_edges and excluded_patterns apply together" \ "0" \ "${REACHABILITY_EDGE_AND_PATTERN_JSON}" +check "baseline attribution records the unexcluded repository" \ + '.dependencies | to_entries[] | select(.value.name == "rules_pkg") | .value.attributed_packages[0].package' \ + "//dependency/test/transitive/ext/excluded" \ + "${REACHABILITY_ATTRIBUTION_EXCLUDED_BASELINE_JSON}" + +check "excluded repository is not attributed" \ + '[.dependencies[] | select(.name == "rules_pkg")] | length' \ + "0" \ + "${REACHABILITY_ATTRIBUTION_EXCLUDED_JSON}" + +check "nothing is attributed through an excluded repository" \ + '[.dependencies[] | .attributed_packages[]?] | length' \ + "0" \ + "${REACHABILITY_ATTRIBUTION_EXCLUDED_JSON}" + if ! cmp -s "${REACHABILITY_EMPTY_EXCLUSIONS_JSON}" "${REACHABILITY_EMPTY_EXCLUSIONS_EXPLICIT_JSON}"; then echo "FAIL: explicit empty exclusions must be byte-identical to defaults" >&2 FAILED=1 diff --git a/bazel/dependency/test/reachability_config_validation_test.bzl b/bazel/dependency/test/reachability_config_validation_test.bzl new file mode 100644 index 0000000000..68119fed02 --- /dev/null +++ b/bazel/dependency/test/reachability_config_validation_test.bzl @@ -0,0 +1,31 @@ +load("@bazel_skylib//lib:unittest.bzl", "asserts", "unittest") +load("//dependency:reachability.bzl", "config_validation_error") + +def _reachability_config_validation_test_impl(ctx): + env = unittest.begin(ctx) + + message = config_validation_error( + {"invalid": {"//dependency/test:reachability_mode": "extra"}}, + [], + False, + ) + asserts.equals( + env, + "Config 'invalid' varies '//dependency/test:reachability_mode' but it is not declared in flags. Declared flags: []", + message, + ) + + message = config_validation_error( + {"invalid_define": {"wasm": "wasmtime"}}, + ["//dependency/test:reachability_mode"], + False, + ) + asserts.equals( + env, + "Config 'invalid_define' varies define 'wasm' but this rule was constructed with defines = False", + message, + ) + + return unittest.end(env) + +reachability_config_validation_test = unittest.make(_reachability_config_validation_test_impl) diff --git a/bazel/dependency/test/reachability_merge_defines_test.bzl b/bazel/dependency/test/reachability_merge_defines_test.bzl new file mode 100644 index 0000000000..7946f45f44 --- /dev/null +++ b/bazel/dependency/test/reachability_merge_defines_test.bzl @@ -0,0 +1,27 @@ +load("@bazel_skylib//lib:unittest.bzl", "asserts", "unittest") +load("//dependency:reachability.bzl", "merge_defines") + +def _reachability_merge_defines_test_impl(ctx): + env = unittest.begin(ctx) + + merged = merge_defines( + [ + "existing=old", + "skipmalformed", + "override=old", + ], + { + "//dependency/test:reachability_mode": "extra", + "override": "new", + "added": "value", + }, + ) + asserts.equals( + env, + ["added=value", "existing=old", "override=new"], + merged, + ) + + return unittest.end(env) + +reachability_merge_defines_test = unittest.make(_reachability_merge_defines_test_impl) diff --git a/bazel/dependency/test/reachability_package_pattern_test.bzl b/bazel/dependency/test/reachability_package_pattern_test.bzl new file mode 100644 index 0000000000..9846064091 --- /dev/null +++ b/bazel/dependency/test/reachability_package_pattern_test.bzl @@ -0,0 +1,38 @@ +load("@bazel_skylib//lib:unittest.bzl", "asserts", "unittest") +load("//dependency:reachability.bzl", "package_pattern_matches") + +def _reachability_package_pattern_test_impl(ctx): + env = unittest.begin(ctx) + + asserts.true( + env, + package_pattern_matches( + "//dependency/test/transitive/ext/direct", + "//dependency/test/transitive/ext/...", + ), + ) + asserts.true( + env, + package_pattern_matches( + "//dependency/test/transitive/ext", + "//dependency/test/transitive/ext/...", + ), + ) + asserts.true( + env, + package_pattern_matches( + "//dependency/test/transitive/ext/direct", + "//dependency/test/transitive/ext/direct", + ), + ) + asserts.false( + env, + package_pattern_matches( + "//dependency/test/transitive/shared", + "//dependency/test/transitive/ext/...", + ), + ) + + return unittest.end(env) + +reachability_package_pattern_test = unittest.make(_reachability_package_pattern_test_impl) diff --git a/bazel/dependency/test/reachability_test.sh b/bazel/dependency/test/reachability_test.sh index 9e35107677..09391c0b1e 100755 --- a/bazel/dependency/test/reachability_test.sh +++ b/bazel/dependency/test/reachability_test.sh @@ -15,6 +15,10 @@ if [ -n "${TEST_SRCDIR:-}" ]; then RUNFILES_DIR="${TEST_SRCDIR}/_main" fi REACHABILITY_JSON="${RUNFILES_DIR}/dependency/test/reachability.json" + REACHABILITY_ATTRIBUTION_JSON="${RUNFILES_DIR}/dependency/test/reachability_attribution.json" + REACHABILITY_ATTRIBUTION_MULTICONFIG_JSON="${RUNFILES_DIR}/dependency/test/reachability_attribution_multiconfig.json" + REACHABILITY_EMPTY_ATTRIBUTION_PATTERNS_JSON="${RUNFILES_DIR}/dependency/test/reachability_empty_attribution_patterns.json" + REACHABILITY_MULTICONFIG_JSON="${RUNFILES_DIR}/dependency/test/reachability_multiconfig.json" else echo "This test must be run under Bazel" >&2 exit 1 @@ -28,8 +32,9 @@ check() { local description="$1" local query="$2" local expected="$3" + local file="${4:-${REACHABILITY_JSON}}" local actual - actual="$("${JQ}" -r "${query}" "${REACHABILITY_JSON}")" + actual="$("${JQ}" -r "${query}" "${file}")" if [ "${actual}" != "${expected}" ]; then echo "FAIL: ${description}" >&2 echo " query: ${query}" >&2 @@ -45,6 +50,7 @@ check() { # ("bazel_skylib") and bzlmod ("bazel_skylib+"/"bazel_skylib~") builds, so # select entries via the emitted apparent name. SKYLIB='.dependencies | to_entries[] | select(.value.name == "bazel_skylib") | .value' +JQ_TOOLCHAINS='.dependencies | to_entries[] | select(.value.name == "jq_toolchains") | .value' check "bazel_skylib is the only reported dependency" \ '[.dependencies[] | .name] | unique | join(",")' \ @@ -74,6 +80,13 @@ check "in-repo consumers are recorded with testonly attribution" \ "${SKYLIB} | [.consumers[] | select(.repo == \"\") | \"\(.target) \(.testonly)\"] | sort | join(\",\")" \ "//dependency/test:external_consumer false,//dependency/test:test_root true" +if ! cmp -s "${REACHABILITY_JSON}" "${REACHABILITY_EMPTY_ATTRIBUTION_PATTERNS_JSON}"; then + echo "FAIL: explicit empty attribution patterns must be byte-identical to defaults" >&2 + FAILED=1 +else + echo "PASS: explicit empty attribution patterns must be byte-identical to defaults" +fi + check "shared consumer is attributed to both roots" \ "${SKYLIB} | .consumers[] | select(.target == \"//dependency/test:external_consumer\") | .roots | sort | join(\",\")" \ "//dependency/test:core_root,//dependency/test:test_root" @@ -82,6 +95,81 @@ check "testonly consumer is only attributed to the test root" \ "${SKYLIB} | .consumers[] | select(.target == \"//dependency/test:test_root\") | .roots | join(\",\")" \ "//dependency/test:test_root" +check "multi-config bazel_skylib records all configs" \ + "${SKYLIB} | .configs | sort | join(\",\")" \ + "default,extra" \ + "${REACHABILITY_MULTICONFIG_JSON}" + +check "multi-config bazel_skylib remains production because one root is non-testonly" \ + "${SKYLIB} | .production" \ + "true" \ + "${REACHABILITY_MULTICONFIG_JSON}" + +check "multi-config bazel_skylib reached_by preserves both roots with unioned production" \ + "${SKYLIB} | [.reached_by[] | \"\(.root) \(.production)\"] | sort | join(\",\")" \ + "//dependency/test:variant_core_root true,//dependency/test:variant_test_root false" \ + "${REACHABILITY_MULTICONFIG_JSON}" + +check "multi-config bazel_skylib testonly root remains scoped to that root" \ + "${SKYLIB} | .consumers[] | select(.target == \"//dependency/test:variant_test_root\") | .roots | sort | join(\",\")" \ + "//dependency/test:variant_test_root" \ + "${REACHABILITY_MULTICONFIG_JSON}" + +check "config-gated jq_toolchains dependency appears in merged output" \ + "${JQ_TOOLCHAINS} | .name" \ + "jq_toolchains" \ + "${REACHABILITY_MULTICONFIG_JSON}" + +check "config-gated jq_toolchains dependency is attributed to extra only" \ + "${JQ_TOOLCHAINS} | .configs | join(\",\")" \ + "extra" \ + "${REACHABILITY_MULTICONFIG_JSON}" + +check "config-gated jq_toolchains dependency tracks only the root and consumer that reach it" \ + "${JQ_TOOLCHAINS} | [.consumers[] | \"\(.target) \(.testonly) \(.roots | sort | join(\"|\"))\"] | sort | join(\",\")" \ + "//dependency/test:variant_external_consumer false //dependency/test:variant_core_root|//dependency/test:variant_test_root" \ + "${REACHABILITY_MULTICONFIG_JSON}" + +check "config-gated jq_toolchains consumer attrs are recorded and unioned across configs" \ + "${JQ_TOOLCHAINS} | .consumers[] | select(.target == \"//dependency/test:variant_external_consumer\") | .attrs | join(\",\")" \ + "srcs" \ + "${REACHABILITY_MULTICONFIG_JSON}" + +check "multi-config bazel_skylib consumer attrs are recorded" \ + "${SKYLIB} | .consumers[] | select(.target == \"//dependency/test:variant_external_consumer\") | .attrs | join(\",\")" \ + "srcs" \ + "${REACHABILITY_MULTICONFIG_JSON}" + +check "transitive attribution includes extension-like packages through shared intermediates" \ + "${SKYLIB} | [.attributed_packages[].package] | sort | join(\",\")" \ + "//dependency/test/transitive/ext/deep,//dependency/test/transitive/ext/direct,//dependency/test/transitive/ext/one,//dependency/test/transitive/ext/testonly,//dependency/test/transitive/ext/two" \ + "${REACHABILITY_ATTRIBUTION_JSON}" + +check "transitive attribution roots are unioned per package" \ + "${SKYLIB} | .attributed_packages[] | select(.package == \"//dependency/test/transitive/ext/one\") | .roots | join(\",\")" \ + "//dependency/test:attribution_root" \ + "${REACHABILITY_ATTRIBUTION_JSON}" + +check "multiple levels of intermediate targets still attribute the originating package" \ + "${SKYLIB} | [.attributed_packages[] | select(.package == \"//dependency/test/transitive/ext/deep\") | .production] | join(\",\")" \ + "true" \ + "${REACHABILITY_ATTRIBUTION_JSON}" + +check "testonly-only attribution remains distinguishable from production attribution" \ + "${SKYLIB} | [.attributed_packages[] | select(.package == \"//dependency/test/transitive/ext/direct\" or .package == \"//dependency/test/transitive/ext/testonly\") | \"\(.package) \(.production)\"] | sort | join(\",\")" \ + "//dependency/test/transitive/ext/direct true,//dependency/test/transitive/ext/testonly false" \ + "${REACHABILITY_ATTRIBUTION_JSON}" + +check "attribution unions matching packages across multiconfig analysis" \ + "${SKYLIB} | [.attributed_packages[].package] | sort | join(\",\")" \ + "//dependency/test/transitive/ext/config_default,//dependency/test/transitive/ext/config_extra" \ + "${REACHABILITY_ATTRIBUTION_MULTICONFIG_JSON}" + +check "attribution multiconfig still records merged config names" \ + "${SKYLIB} | .configs | sort | join(\",\")" \ + "default,extra" \ + "${REACHABILITY_ATTRIBUTION_MULTICONFIG_JSON}" + if [ "${FAILED}" -ne 0 ]; then exit 1 fi diff --git a/bazel/dependency/test/reachability_test_rules.bzl b/bazel/dependency/test/reachability_test_rules.bzl new file mode 100644 index 0000000000..6fb060110c --- /dev/null +++ b/bazel/dependency/test/reachability_test_rules.bzl @@ -0,0 +1,11 @@ +load( + "//dependency:reachability.bzl", + "dependency_reachability_macro", + "dependency_reachability_rule", +) + +_dependency_reachability_with_mode = dependency_reachability_rule( + flags = ["//dependency/test:reachability_mode"], +) + +dependency_reachability_with_mode = dependency_reachability_macro(_dependency_reachability_with_mode) diff --git a/bazel/dependency/test/transitive/chain/one/BUILD b/bazel/dependency/test/transitive/chain/one/BUILD new file mode 100644 index 0000000000..17e963cd4f --- /dev/null +++ b/bazel/dependency/test/transitive/chain/one/BUILD @@ -0,0 +1,6 @@ +package(default_visibility = ["//visibility:public"]) + +filegroup( + name = "root", + srcs = ["//dependency/test/transitive/chain/two:root"], +) diff --git a/bazel/dependency/test/transitive/chain/two/BUILD b/bazel/dependency/test/transitive/chain/two/BUILD new file mode 100644 index 0000000000..2a6208ff83 --- /dev/null +++ b/bazel/dependency/test/transitive/chain/two/BUILD @@ -0,0 +1,6 @@ +package(default_visibility = ["//visibility:public"]) + +filegroup( + name = "root", + srcs = ["//dependency/test/transitive/shared:skylib_consumer"], +) diff --git a/bazel/dependency/test/transitive/ext/config_default/BUILD b/bazel/dependency/test/transitive/ext/config_default/BUILD new file mode 100644 index 0000000000..2a6208ff83 --- /dev/null +++ b/bazel/dependency/test/transitive/ext/config_default/BUILD @@ -0,0 +1,6 @@ +package(default_visibility = ["//visibility:public"]) + +filegroup( + name = "root", + srcs = ["//dependency/test/transitive/shared:skylib_consumer"], +) diff --git a/bazel/dependency/test/transitive/ext/config_extra/BUILD b/bazel/dependency/test/transitive/ext/config_extra/BUILD new file mode 100644 index 0000000000..2a6208ff83 --- /dev/null +++ b/bazel/dependency/test/transitive/ext/config_extra/BUILD @@ -0,0 +1,6 @@ +package(default_visibility = ["//visibility:public"]) + +filegroup( + name = "root", + srcs = ["//dependency/test/transitive/shared:skylib_consumer"], +) diff --git a/bazel/dependency/test/transitive/ext/deep/BUILD b/bazel/dependency/test/transitive/ext/deep/BUILD new file mode 100644 index 0000000000..d2839eb426 --- /dev/null +++ b/bazel/dependency/test/transitive/ext/deep/BUILD @@ -0,0 +1,6 @@ +package(default_visibility = ["//visibility:public"]) + +filegroup( + name = "root", + srcs = ["//dependency/test/transitive/chain/one:root"], +) diff --git a/bazel/dependency/test/transitive/ext/direct/BUILD b/bazel/dependency/test/transitive/ext/direct/BUILD new file mode 100644 index 0000000000..1ce1d63fe9 --- /dev/null +++ b/bazel/dependency/test/transitive/ext/direct/BUILD @@ -0,0 +1,6 @@ +package(default_visibility = ["//visibility:public"]) + +filegroup( + name = "root", + srcs = ["@bazel_skylib//lib:sets"], +) diff --git a/bazel/dependency/test/transitive/ext/excluded/BUILD b/bazel/dependency/test/transitive/ext/excluded/BUILD new file mode 100644 index 0000000000..3505df1af3 --- /dev/null +++ b/bazel/dependency/test/transitive/ext/excluded/BUILD @@ -0,0 +1,6 @@ +package(default_visibility = ["//visibility:public"]) + +filegroup( + name = "root", + srcs = ["@rules_pkg//pkg:filter_directory"], +) diff --git a/bazel/dependency/test/transitive/ext/one/BUILD b/bazel/dependency/test/transitive/ext/one/BUILD new file mode 100644 index 0000000000..2a6208ff83 --- /dev/null +++ b/bazel/dependency/test/transitive/ext/one/BUILD @@ -0,0 +1,6 @@ +package(default_visibility = ["//visibility:public"]) + +filegroup( + name = "root", + srcs = ["//dependency/test/transitive/shared:skylib_consumer"], +) diff --git a/bazel/dependency/test/transitive/ext/testonly/BUILD b/bazel/dependency/test/transitive/ext/testonly/BUILD new file mode 100644 index 0000000000..5baf537ef5 --- /dev/null +++ b/bazel/dependency/test/transitive/ext/testonly/BUILD @@ -0,0 +1,7 @@ +package(default_visibility = ["//visibility:public"]) + +filegroup( + name = "root", + testonly = True, + srcs = ["//dependency/test/transitive/shared:skylib_consumer"], +) diff --git a/bazel/dependency/test/transitive/ext/two/BUILD b/bazel/dependency/test/transitive/ext/two/BUILD new file mode 100644 index 0000000000..2a6208ff83 --- /dev/null +++ b/bazel/dependency/test/transitive/ext/two/BUILD @@ -0,0 +1,6 @@ +package(default_visibility = ["//visibility:public"]) + +filegroup( + name = "root", + srcs = ["//dependency/test/transitive/shared:skylib_consumer"], +) diff --git a/bazel/dependency/test/transitive/shared/BUILD b/bazel/dependency/test/transitive/shared/BUILD new file mode 100644 index 0000000000..a9d1d2ce69 --- /dev/null +++ b/bazel/dependency/test/transitive/shared/BUILD @@ -0,0 +1,6 @@ +package(default_visibility = ["//visibility:public"]) + +filegroup( + name = "skylib_consumer", + srcs = ["@bazel_skylib//lib:paths"], +) From 1f9829abbcf370f13e6477f9c70b47d1d9724f6a Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Mon, 24 Aug 2026 22:11:25 +0000 Subject: [PATCH 2/3] Initial plan From 7b55840bcfa14d4297c43bad9dd743ba6eeff145 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Mon, 24 Aug 2026 22:21:20 +0000 Subject: [PATCH 3/3] bazel/deps: Fix attribution performance and robustness issues Co-authored-by: phlax <454682+phlax@users.noreply.github.com> --- bazel/dependency/reachability.bzl | 120 ++++++++++++++---------------- 1 file changed, 57 insertions(+), 63 deletions(-) diff --git a/bazel/dependency/reachability.bzl b/bazel/dependency/reachability.bzl index 44ccf3d9fe..a0cb0cb7eb 100644 --- a/bazel/dependency/reachability.bzl +++ b/bazel/dependency/reachability.bzl @@ -206,12 +206,12 @@ DependencyReachabilityInfo = provider( "non-testonly path when package attribution is enabled, else None" ), "attributed_packages": ( - "dict(repo -> depset(package strings)) for matching main-repo " + - "packages that transitively reach each repo, else None" + "depset of struct(package=..., repo=...) pairs for matching " + + "main-repo packages that transitively reach each repo, else None" ), "production_attributed_packages": ( - "dict(repo -> depset(package strings)) for matching main-repo " + - "packages with at least one non-testonly path to each repo, else None" + "depset of struct(package=..., repo=...) pairs for matching " + + "main-repo packages with at least one non-testonly path to each repo, else None" ), }, ) @@ -286,28 +286,6 @@ def _package_matches_patterns(package, patterns): package_pattern_matches = _matches_package_pattern -def _append_repo_package_depsets(dest, source): - if source == None: - return - for repo in source.keys(): - dest.setdefault(repo, []).append(source[repo]) - -def _add_repo_package(dest, repo, package): - dest.setdefault(repo, {})[package] = True - -def _materialize_repo_packages(direct, transitive): - repos = {} - for repo in direct.keys(): - repos[repo] = True - for repo in transitive.keys(): - repos[repo] = True - return { - repo: depset( - direct = sorted(direct.get(repo, {}).keys()), - transitive = transitive.get(repo, []), - ) - for repo in sorted(repos.keys()) - } def _decode_configs(configs_attr): configs = {} @@ -377,10 +355,28 @@ def _reachability_aspect_impl(target, ctx): reachable_repos_transitive = [] production_reachable_repos = [] production_reachable_repos_transitive = [] - attributed_packages_direct = {} - attributed_packages_transitive = {} - production_attributed_packages_direct = {} - production_attributed_packages_transitive = {} + # Flat lists of struct(package=..., repo=...) pairs accumulated at this node. + # Transitive pairs from deps flow up as depsets without per-node flattening, + # eliminating the dict-of-depsets allocation of the previous design (Issue 2). + # + # NOTE (Issue 1): the .to_list() calls in the matching-package block below + # are unavoidable with a pure bottom-up aspect. Attribution requires pairing + # a matching package with *all* repos in its transitive closure, but in a + # bottom-up traversal the package's ancestors have not yet been visited when + # the node is processed, so there is no way to know which packages above a + # given repo edge will eventually match. The only correct alternative would + # require top-down information (e.g. a second pass), which is not available + # in a single Bazel aspect. The cross-product-at-root approximation + # (propagate matching_packages and repos as independent depsets, multiply at + # the rule) is deliberately rejected: it would incorrectly attribute package + # B to repo R1 whenever any other matching package A also reaches R1, even + # if B has no path to R1. The per-node flatten is therefore kept, but its + # cost is bounded: it fires only at nodes whose package matches + # attribution_patterns, and only when attribution is enabled. + attributed_pairs_direct = [] + attributed_pairs_transitive = [] + production_attributed_pairs_direct = [] + production_attributed_pairs_transitive = [] for attr_name in [a for a in dir(ctx.rule.attr) if not a.startswith("_")]: if attr_name in excluded_edges: continue @@ -407,27 +403,26 @@ def _reachability_aspect_impl(target, ctx): transitive.append(info.edges) transitive_production.append(info.production_edges) if collect_attributions: - reachable_repos_transitive.append(info.repos) - if not testonly: + if info.repos != None: + reachable_repos_transitive.append(info.repos) + if not testonly and info.production_repos != None: production_reachable_repos_transitive.append(info.production_repos) - _append_repo_package_depsets(attributed_packages_transitive, info.attributed_packages) - if not testonly: - _append_repo_package_depsets( - production_attributed_packages_transitive, - info.production_attributed_packages, - ) + if info.attributed_packages != None: + attributed_pairs_transitive.append(info.attributed_packages) + if not testonly and info.production_attributed_packages != None: + production_attributed_pairs_transitive.append(info.production_attributed_packages) if collect_attributions and not consumer_repo and _package_matches_patterns(consumer_package, attribution_patterns): for repo in depset( direct = reachable_repos, transitive = reachable_repos_transitive, ).to_list(): - _add_repo_package(attributed_packages_direct, repo, consumer_package) + attributed_pairs_direct.append(struct(package = consumer_package, repo = repo)) if not testonly: for repo in depset( direct = production_reachable_repos, transitive = production_reachable_repos_transitive, ).to_list(): - _add_repo_package(production_attributed_packages_direct, repo, consumer_package) + production_attributed_pairs_direct.append(struct(package = consumer_package, repo = repo)) return [DependencyReachabilityInfo( edges = depset(edges, transitive = transitive), production_edges = ( @@ -458,17 +453,21 @@ def _reachability_aspect_impl(target, ctx): attributed_packages = ( None if not collect_attributions - else _materialize_repo_packages( - attributed_packages_direct, - attributed_packages_transitive, + else depset( + direct = attributed_pairs_direct, + transitive = attributed_pairs_transitive, ) ), production_attributed_packages = ( None if not collect_attributions - else _materialize_repo_packages( - production_attributed_packages_direct, - production_attributed_packages_transitive, + else ( + depset() + if testonly + else depset( + direct = production_attributed_pairs_direct, + transitive = production_attributed_pairs_transitive, + ) ) ), )] @@ -561,23 +560,18 @@ def _dependency_reachability_impl(): production = edge in production, ) if emit_attributed_packages: - production_attributed_packages = { - repo: { - package: True - for package in info.production_attributed_packages[repo].to_list() - } - for repo in sorted(info.production_attributed_packages.keys()) - } - for repo in sorted(info.attributed_packages.keys()): - for package in info.attributed_packages[repo].to_list(): - _record_attributed_package( - deps, - config, - root, - repo, - package, - production = package in production_attributed_packages.get(repo, {}), - ) + prod_pairs = {} + for p in info.production_attributed_packages.to_list(): + prod_pairs.setdefault(p.repo, {})[p.package] = True + for pair in info.attributed_packages.to_list(): + _record_attributed_package( + deps, + config, + root, + pair.repo, + pair.package, + production = pair.package in prod_pairs.get(pair.repo, {}), + ) dependencies = {} for repo in sorted(deps.keys()): entry = deps[repo]