diff --git a/README.md b/README.md index a10dfcd..2c790eb 100644 --- a/README.md +++ b/README.md @@ -108,9 +108,9 @@ load("@rules_runfiles_group//runfiles_group:providers.bzl", providers.append(RunfilesGroupInfo(**groups)) providers.append(RunfilesGroupMetadataInfo(groups = { - "interpreter": lib.group_metadata(rank = -2, do_not_merge = True), - "std": lib.group_metadata(rank = -1), - "app_code": lib.group_metadata(rank = 0, weight = 100, executable_group = True), + "foo_runfiles_group#interpreter": lib.group_metadata(rank = -2, do_not_merge = True), + "foo_runfiles_group#std": lib.group_metadata(rank = -1), + "foo_runfiles_group#app_code": lib.group_metadata(rank = 0, weight = 100, executable_group = True), })) ``` @@ -139,6 +139,22 @@ There may be different preferences for splitting files into groups. A good way t > [!CAUTION] > Merging groups by merging their `depset`s is cheap. Calling `.to_list()` on a depset is expensive and should be avoided during analysis. Build group hierarchies purely through `depset(transitive = [...])`. +### Naming groups + +Group names are arbitrary strings and live in a shared namespace across all `RunfilesGroupInfo` providers merged into the same binary. If two rulesets independently define a group called `"interpreter"` — say, one for Node.js and one for Python — those groups will be merged together, which may be undesirable. + +To avoid this, **prefix all group names with a string unique to your ruleset**, separated by a delimiter like `#`: + +```starlark +groups["my_rules_runfiles_group#interpreter"] = ... +groups["my_rules_runfiles_group#std"] = ... +groups["my_rules_runfiles_group#" + loadpath + ":" + ctx.label.name] = ... +``` + +This ensures that `my_rules_runfiles_group#interpreter` and `other_rules_runfiles_group#interpreter` remain distinct, even when both rulesets contribute groups to the same binary target. + +When merging groups (e.g., in `merge_to_limit`), pass a custom `merged_group_name` callback that strips the prefix from the second group name before joining. This keeps merged names readable — `my_rules_runfiles_group#foo+bar` instead of `my_rules_runfiles_group#foo+my_rules_runfiles_group#bar`. + ### Handling `deps` and `data` Most rules have the attributes `deps` and `data`. You should implement support for them carefully. @@ -156,13 +172,13 @@ data_groups = lib.collect_groups(ctx.attr.data) groups = {} groups.update(dep_groups.groups) groups.update(data_groups.groups) -groups["app_code"] = depset(my_own_files, transitive = data_groups.ungrouped) +groups["foo_runfiles_group#app_code"] = depset(my_own_files, transitive = data_groups.ungrouped) metadata = lib.merge_metadata(dep_groups.metadata, data_groups.metadata) # executable_group has been stripped from dep metadata by collect_groups. # Set it on our own group instead: metadata = lib.merge_metadata(metadata, RunfilesGroupMetadataInfo(groups = { - "app_code": lib.group_metadata(executable_group = True), + "foo_runfiles_group#app_code": lib.group_metadata(executable_group = True), })) ``` diff --git a/example/producer/rules/skip_interpreter.bzl b/example/producer/rules/skip_interpreter.bzl index b0250cd..4eecd0b 100644 --- a/example/producer/rules/skip_interpreter.bzl +++ b/example/producer/rules/skip_interpreter.bzl @@ -6,16 +6,18 @@ load( "RunfilesGroupTransformInfo", ) +_GROUP_PREFIX = "starlark_runfiles_group#" + def _skip_interpreter_transform(runfiles_group_info, runfiles_group_metadata_info): new_groups = {} for group_name in lib.group_names(runfiles_group_info): - if group_name == "interpreter": + if group_name == _GROUP_PREFIX + "interpreter": continue new_groups[group_name] = getattr(runfiles_group_info, group_name) new_metadata = None if runfiles_group_metadata_info != None: - new_meta_groups = {k: v for k, v in runfiles_group_metadata_info.groups.items() if k != "interpreter"} + new_meta_groups = {k: v for k, v in runfiles_group_metadata_info.groups.items() if k != _GROUP_PREFIX + "interpreter"} if new_meta_groups: new_metadata = RunfilesGroupMetadataInfo(groups = new_meta_groups) diff --git a/example/producer/rules/starlark_binary.bzl b/example/producer/rules/starlark_binary.bzl index 95939c6..bc2b869 100644 --- a/example/producer/rules/starlark_binary.bzl +++ b/example/producer/rules/starlark_binary.bzl @@ -5,6 +5,8 @@ load("@rules_runfiles_group//runfiles_group:lib.bzl", "lib") load("@rules_runfiles_group//runfiles_group:providers.bzl", "RunfilesGroupInfo", "RunfilesGroupMetadataInfo") load("//producer/providers:providers.bzl", "StarlarkInfo") +_GROUP_PREFIX = "starlark_runfiles_group#" + def _canonical_repo_name(ctx): return ctx.label.repo_name or "_main" @@ -128,23 +130,23 @@ def _starlark_binary_impl(ctx): own_repo = ctx.attr.repository # Special group: interpreter - groups["interpreter"] = depset( + groups[_GROUP_PREFIX + "interpreter"] = depset( [interpreter_exe], transitive = [interpreter_info.default_runfiles.files], ) - metadata["interpreter"] = lib.group_metadata(rank = -2, do_not_merge = True) + metadata[_GROUP_PREFIX + "interpreter"] = lib.group_metadata(rank = -2, do_not_merge = True) # Special group: std - groups["std"] = stdlib[DefaultInfo].default_runfiles.files - metadata["std"] = lib.group_metadata(rank = -1) + groups[_GROUP_PREFIX + "std"] = stdlib[DefaultInfo].default_runfiles.files + metadata[_GROUP_PREFIX + "std"] = lib.group_metadata(rank = -1) entrypoint_files = depset([output, entrypoint, loadmap, properties]) # Dep groups if ctx.attr.runfiles_grouping == "by_target": groups.update(data_groups.groups) - groups["entrypoint"] = depset(transitive = [entrypoint_files] + data_groups.ungrouped) - metadata["entrypoint"] = lib.group_metadata(rank = 2, executable_group = True) + groups[_GROUP_PREFIX + "entrypoint"] = depset(transitive = [entrypoint_files] + data_groups.ungrouped) + metadata[_GROUP_PREFIX + "entrypoint"] = lib.group_metadata(rank = 2, executable_group = True) for name in data_groups.groups: dep_weight = _get_dep_weight(dep_metadata, name) if _extract_repo(name) == own_repo: @@ -175,11 +177,11 @@ def _starlark_binary_impl(ctx): if w != None: repo_weights[repo] = repo_weights.get(repo, 0) + w for repo, ds in repo_depsets.items(): - groups[repo or "_main"] = depset(transitive = ds) + groups[_GROUP_PREFIX + (repo or "_main")] = depset(transitive = ds) if repo == own_repo: - metadata[repo or "_main"] = lib.group_metadata(rank = 1, weight = repo_weights.get(repo, None), executable_group = True) + metadata[_GROUP_PREFIX + (repo or "_main")] = lib.group_metadata(rank = 1, weight = repo_weights.get(repo, None), executable_group = True) elif repo in repo_weights: - metadata[repo or "_main"] = lib.group_metadata(weight = repo_weights[repo]) + metadata[_GROUP_PREFIX + (repo or "_main")] = lib.group_metadata(weight = repo_weights[repo]) providers.append(RunfilesGroupInfo(**groups)) providers.append(RunfilesGroupMetadataInfo(groups = metadata)) @@ -189,9 +191,11 @@ def _starlark_binary_impl(ctx): def _extract_repo(group_name): """Extracts the friendly repo name from a loadpath-based group name. - "@fizzbuzz//:fizzbuzz" -> "fizzbuzz" - "//src:lib_a" -> "" + "starlark_runfiles_group#@fizzbuzz//:fizzbuzz" -> "fizzbuzz" + "starlark_runfiles_group#//src:lib_a" -> "" """ + if group_name.startswith(_GROUP_PREFIX): + group_name = group_name[len(_GROUP_PREFIX):] if not group_name.startswith("@"): return "" idx = group_name.find("//") diff --git a/example/producer/rules/starlark_library.bzl b/example/producer/rules/starlark_library.bzl index 4bfbf9f..b6aac49 100644 --- a/example/producer/rules/starlark_library.bzl +++ b/example/producer/rules/starlark_library.bzl @@ -4,6 +4,8 @@ load("@rules_runfiles_group//runfiles_group:lib.bzl", "lib") load("@rules_runfiles_group//runfiles_group:providers.bzl", "RunfilesGroupInfo", "RunfilesGroupMetadataInfo") load("//producer/providers:providers.bzl", "StarlarkInfo") +_GROUP_PREFIX = "starlark_runfiles_group#" + def _canonical_repo_name(ctx): return ctx.label.repo_name or "_main" @@ -28,7 +30,7 @@ def _starlark_library_impl(ctx): for dep in ctx.attr.data: runfiles = runfiles.merge(dep[DefaultInfo].default_runfiles) - group_name = loadpath + ":" + ctx.label.name + group_name = _GROUP_PREFIX + loadpath + ":" + ctx.label.name dep_groups = lib.collect_groups(ctx.attr.deps) data_groups = lib.collect_groups(ctx.attr.data) diff --git a/example/src/BUILD.bazel b/example/src/BUILD.bazel index 1454034..cba83e4 100644 --- a/example/src/BUILD.bazel +++ b/example/src/BUILD.bazel @@ -7,6 +7,8 @@ load("//producer/rules:starlark_library.bzl", "starlark_library") package(default_visibility = ["//visibility:public"]) +P = "starlark_runfiles_group#" + # This is a mixin users can add to aspect_hints of a binary target to skip the interpreter group. skip_interpreter(name = "skip_interpreter") @@ -137,11 +139,11 @@ runfiles_group_analysis_test( name = "test_adder_groups", binaries = [":adder"], expected_group_names = [ - "interpreter", - "std", - "//src:lib_a", - "//src:lib_b", - "entrypoint", + P + "interpreter", + P + "std", + P + "//src:lib_a", + P + "//src:lib_b", + P + "entrypoint", ], overlapping_group_behavior = "error", ) @@ -150,9 +152,9 @@ runfiles_group_analysis_test( name = "test_hasher_groups", binaries = [":hasher"], expected_group_names = [ - "interpreter", - "std", - "_main", + P + "interpreter", + P + "std", + P + "_main", ], overlapping_group_behavior = "error", ) @@ -161,11 +163,11 @@ runfiles_group_analysis_test( name = "test_3p_deps_demo_groups", binaries = [":3p_deps_demo"], expected_group_names = [ - "interpreter", - "std", - "fizzbuzz", - "stringutil", - "_main", + P + "interpreter", + P + "std", + P + "fizzbuzz", + P + "stringutil", + P + "_main", ], overlapping_group_behavior = "error", ) @@ -176,11 +178,12 @@ runfiles_group_analysis_test( name = "test_adder_groups_merged", binaries = [":adder"], expected_group_names = [ - "interpreter", - "std", - "//src:lib_a+//src:lib_b", - "entrypoint", + P + "interpreter", + P + "std", + P + "//src:lib_a+//src:lib_b", + P + "entrypoint", ], + group_name_prefix = P, max_groups = 4, overlapping_group_behavior = "error", ) @@ -192,10 +195,11 @@ runfiles_group_analysis_test( # so merging cannot reduce below 3. expected_group_count = 3, expected_group_names = [ - "interpreter", - "std", - "_main", + P + "interpreter", + P + "std", + P + "_main", ], + group_name_prefix = P, max_groups = 2, overlapping_group_behavior = "error", ) @@ -205,11 +209,12 @@ runfiles_group_analysis_test( binaries = [":3p_deps_demo"], # fizzbuzz and stringutil (both rank 0) merge into fizzbuzz+stringutil. expected_group_names = [ - "interpreter", - "std", - "fizzbuzz+stringutil", - "_main", + P + "interpreter", + P + "std", + P + "fizzbuzz+stringutil", + P + "_main", ], + group_name_prefix = P, max_groups = 4, overlapping_group_behavior = "error", ) @@ -229,14 +234,14 @@ runfiles_group_analysis_test( name = "test_merge_demo_groups", binaries = [":merge_demo"], expected_group_names = [ - "interpreter", - "std", - "colors", - "limits", - "mathlib", - "templates", - "units", - "_main", + P + "interpreter", + P + "std", + P + "colors", + P + "limits", + P + "mathlib", + P + "templates", + P + "units", + P + "_main", ], overlapping_group_behavior = "error", ) @@ -247,13 +252,14 @@ runfiles_group_analysis_test( name = "test_merge_demo_max6", binaries = [":merge_demo"], expected_group_names = [ - "interpreter", - "std", - "limits+units+colors", - "mathlib", - "templates", - "_main", + P + "interpreter", + P + "std", + P + "limits+units+colors", + P + "mathlib", + P + "templates", + P + "_main", ], + group_name_prefix = P, max_groups = 6, overlapping_group_behavior = "error", ) @@ -263,12 +269,13 @@ runfiles_group_analysis_test( name = "test_merge_demo_max5", binaries = [":merge_demo"], expected_group_names = [ - "interpreter", - "std", - "limits+units+colors+mathlib", - "templates", - "_main", + P + "interpreter", + P + "std", + P + "limits+units+colors+mathlib", + P + "templates", + P + "_main", ], + group_name_prefix = P, max_groups = 5, overlapping_group_behavior = "error", ) @@ -278,11 +285,12 @@ runfiles_group_analysis_test( name = "test_merge_demo_max4", binaries = [":merge_demo"], expected_group_names = [ - "interpreter", - "std", - "limits+units+colors+mathlib+templates", - "_main", + P + "interpreter", + P + "std", + P + "limits+units+colors+mathlib+templates", + P + "_main", ], + group_name_prefix = P, max_groups = 4, overlapping_group_behavior = "error", ) @@ -294,11 +302,12 @@ runfiles_group_analysis_test( binaries = [":merge_demo"], expected_group_count = 4, expected_group_names = [ - "interpreter", - "std", - "limits+units+colors+mathlib+templates", - "_main", + P + "interpreter", + P + "std", + P + "limits+units+colors+mathlib+templates", + P + "_main", ], + group_name_prefix = P, max_groups = 3, overlapping_group_behavior = "error", ) diff --git a/runfiles_group/private/rules/runfiles_group_analysis_test.bzl b/runfiles_group/private/rules/runfiles_group_analysis_test.bzl index 484ac00..2c693a3 100644 --- a/runfiles_group/private/rules/runfiles_group_analysis_test.bzl +++ b/runfiles_group/private/rules/runfiles_group_analysis_test.bzl @@ -29,6 +29,14 @@ def _indent(text): def _join_group_names(lighter_name, _lighter_weight, heavier_name, _heavier_weight): return lighter_name + "+" + heavier_name +def _make_join_group_names(prefix): + def _join(lighter_name, _lighter_weight, heavier_name, _heavier_weight): + stripped = heavier_name + if stripped.startswith(prefix): + stripped = stripped[len(prefix):] + return lighter_name + "+" + stripped + return _join + def _test_one(ctx, binary_attr): issues = [] success = True @@ -93,11 +101,12 @@ def _test_one(ctx, binary_attr): metadata = binary_attr[RunfilesGroupMetadataInfo] if RunfilesGroupMetadataInfo in binary_attr else None if ctx.attr.max_groups >= 0: + join_fn = _make_join_group_names(ctx.attr.group_name_prefix) if ctx.attr.group_name_prefix else _join_group_names merged = lib.merge_to_limit( rgi, metadata, max_groups = ctx.attr.max_groups, - merged_group_name = _join_group_names, + merged_group_name = join_fn, ) rgi = merged.runfiles_group_info metadata = merged.runfiles_group_metadata_info @@ -211,6 +220,13 @@ to assert the actual reachable count. -1 means no check (the test fails if group """, default = -1, ), + "group_name_prefix": attr.string( + doc = """\ +If set, merged group names will strip this prefix from the second (heavier) group name +before joining with '+'. This avoids repeating a common prefix in merged names. +For example, with prefix "p#", merging "p#foo" and "p#bar" produces "p#foo+bar" instead of "p#foo+p#bar". +""", + ), }, analysis_test = True, )