Allowlist subrule usage in Starlark aspects. Previously, Starlark rules could use subrules when `--experimental_rule_extension_api` was disabled if their defining `.bzl` file was included in `//tools/allowlists/subrules_allowlist`, whereas Starlark aspects still enforced the hardcoded load-time `BuiltinRestriction` allowlist. Replace the load-time `BuiltinRestriction` check in `StarlarkRuleClassFunctions.aspect()` with an implicit `$allowlist_subrules` attribute when `--experimental_rule_extension_api` is disabled and the defining `.bzl` file is outside `@_builtins`. Validate the aspect's defining `.bzl` label against the `subrules` allowlist during aspect analysis in `StarlarkAspectFactory`, and remove the now-unused `ALLOWLIST_RULE_EXTENSION_API` allowlist. PiperOrigin-RevId: 1000014706 Change-Id: I3d33b6f08a2b83f9f96931a3145ea423f2c687d8
diff --git a/src/main/java/com/google/devtools/build/lib/analysis/starlark/StarlarkRuleClassFunctions.java b/src/main/java/com/google/devtools/build/lib/analysis/starlark/StarlarkRuleClassFunctions.java index 822ccbb..46590e5 100644 --- a/src/main/java/com/google/devtools/build/lib/analysis/starlark/StarlarkRuleClassFunctions.java +++ b/src/main/java/com/google/devtools/build/lib/analysis/starlark/StarlarkRuleClassFunctions.java
@@ -243,12 +243,6 @@ .build()) .build(); - public static final BuiltinRestriction.Allowlist ALLOWLIST_RULE_EXTENSION_API = - BuiltinRestriction.Allowlist.of( - mainRepoAllowlistEntry("initializer_testing"), - mainRepoAllowlistEntry("extend_rule_testing"), - mainRepoAllowlistEntry("subrule_testing")); - public static final BuiltinRestriction.Allowlist ALLOWLIST_RULE_EXTENSION_API_EXPERIMENTAL = BuiltinRestriction.Allowlist.of( mainRepoAllowlistEntry("third_party/bazel_rules/rules_cc"), @@ -1366,17 +1360,12 @@ StarlarkThread thread) throws EvalException { // Ensure we're initializing a .bzl file. - BzlInitThreadContext.fromOrFail(thread, "aspect()"); + BzlInitThreadContext bazelContext = BzlInitThreadContext.fromOrFail(thread, "aspect()"); LabelConverter labelConverter = LabelConverter.forBzlEvaluatingThread(thread); ImmutableList<Pair<String, StarlarkAttrModule.Descriptor>> descriptors = attrObjectToAttributesList(attrs); - if (!subrulesUnchecked.isEmpty()) { - if (!thread.getSemantics().getBool(BuildLanguageOptions.EXPERIMENTAL_RULE_EXTENSION_API)) { - BuiltinRestriction.failIfCalledOutsideAllowlist(thread, ALLOWLIST_RULE_EXTENSION_API); - } - } ImmutableList<StarlarkSubrule> subrules = Sequence.cast(subrulesUnchecked, StarlarkSubrule.class, "subrules").getImmutableList(); ImmutableList<Pair<String, Descriptor>> subruleAttributes = @@ -1390,6 +1379,20 @@ } ImmutableList.Builder<Attribute> attributes = ImmutableList.builder(); + if (!subrulesUnchecked.isEmpty()) { + if (!thread.getSemantics().getBool(BuildLanguageOptions.EXPERIMENTAL_RULE_EXTENSION_API) + && !BazelModuleContext.ofInnermostBzlOrThrow(thread) + .label() + .getRepository() + .equals(RepositoryName.BUILTINS)) { + attributes.add( + attr("$allowlist_subrules", LABEL) + .cfg(ExecutionTransitionFactory.createFactory()) + .mandatoryBuiltinProviders(ImmutableList.of(PackageSpecificationProvider.class)) + .value(bazelContext.getToolsLabel("//tools/allowlists/subrules_allowlist")) + .build()); + } + } ImmutableSet.Builder<String> requiredParams = ImmutableSet.builder(); for (Pair<String, Descriptor> nameDescriptorPair : descriptors) { String nativeName = nameDescriptorPair.first;
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/StarlarkAspectFactory.java b/src/main/java/com/google/devtools/build/lib/skyframe/StarlarkAspectFactory.java index 851694a..288152f 100644 --- a/src/main/java/com/google/devtools/build/lib/skyframe/StarlarkAspectFactory.java +++ b/src/main/java/com/google/devtools/build/lib/skyframe/StarlarkAspectFactory.java
@@ -14,6 +14,7 @@ package com.google.devtools.build.lib.skyframe; import com.google.devtools.build.lib.actions.ActionConflictException; +import com.google.devtools.build.lib.analysis.Allowlist; import com.google.devtools.build.lib.analysis.CachingAnalysisEnvironment; import com.google.devtools.build.lib.analysis.ConfiguredAspect; import com.google.devtools.build.lib.analysis.ConfiguredAspectFactory; @@ -51,6 +52,16 @@ AspectParameters parameters, RepositoryName toolsRepository) throws InterruptedException, ActionConflictException { + if (ruleContext + .getMainAspect() + .getDefinition() + .getAttributes() + .containsKey("$allowlist_subrules") + && !Allowlist.isAvailableFor( + ruleContext, "subrules", starlarkAspect.getAspectClass().getExtensionLabel())) { + ruleContext.ruleError("Non-allowlisted attempt to use subrules."); + return errorConfiguredAspect(ruleContext); + } RequiredConfigFragmentsProvider requiredConfigFragments; Object aspectStarlarkObject; try {
diff --git a/src/test/java/com/google/devtools/build/lib/analysis/starlark/StarlarkSubruleTest.java b/src/test/java/com/google/devtools/build/lib/analysis/starlark/StarlarkSubruleTest.java index fbaec11..c74bac5 100644 --- a/src/test/java/com/google/devtools/build/lib/analysis/starlark/StarlarkSubruleTest.java +++ b/src/test/java/com/google/devtools/build/lib/analysis/starlark/StarlarkSubruleTest.java
@@ -58,8 +58,6 @@ public class StarlarkSubruleTest extends BuildViewTestCase { private final BazelEvaluationTestCase ev = new BazelEvaluationTestCase("//subrule_testing:label"); - private final BazelEvaluationTestCase evOutsideAllowlist = - new BazelEvaluationTestCase("//foo:bar"); @Test public void testSubruleFunctionSymbol_notVisibleInBUILD() throws Exception { @@ -1929,9 +1927,30 @@ @Test public void testSubrulesParamForAspect_isPrivateAPI() throws Exception { - evOutsideAllowlist.setSemantics("--noexperimental_rule_extension_api"); - evOutsideAllowlist.checkEvalErrorContains( - "'//foo:bar' cannot use private API", "aspect(implementation = lambda: 0, subrules = [1])"); + setBuildLanguageOptions("--noexperimental_rule_extension_api"); + scratch.file( + "foo/myrule.bzl", + """ + def _impl(ctx): + pass + my_subrule = subrule(implementation = _impl) + my_aspect = aspect(_impl, subrules = [my_subrule]) + my_rule = rule(_impl, attrs = {'dep': attr.label(aspects = [my_aspect])}) + """); + scratch.file( + "foo/BUILD", + """ + load("myrule.bzl", "my_rule") + + my_rule(name = "bar") + my_rule(name = "foo", dep = ":bar") + """); + + reporter.removeHandler(failFastHandler); + reporter.addHandler(ev.getEventCollector()); + getConfiguredTarget("//foo"); + + ev.assertContainsError("Non-allowlisted attempt to use subrules."); } private StructImpl getProvider(String targetLabel, String providerLabel, String providerName)