Allow setting private attributes from initializers

Only in builtins. This simplifies cc_binary and cc_test implementation. Computed default can't be used because it causes a regression.

PiperOrigin-RevId: 579815726
Change-Id: I75356f3f5d74cb58a8b0d5fb4fda4be6aac59e58

src/main/java/com/google/devtools/build/lib/analysis/starlark/StarlarkRuleClassFunctions.java

@@ -54,6 +54,7 @@
 import com.google.devtools.build.lib.cmdline.BazelModuleContext;
 import com.google.devtools.build.lib.cmdline.Label;
 import com.google.devtools.build.lib.cmdline.LabelSyntaxException;
+import com.google.devtools.build.lib.cmdline.RepositoryMapping;
 import com.google.devtools.build.lib.cmdline.RepositoryName;
 import com.google.devtools.build.lib.events.Event;
 import com.google.devtools.build.lib.events.EventHandler;
@@ -174,6 +175,9 @@ public class StarlarkRuleClassFunctions implements StarlarkRuleFunctionsApi {
  allowlistEntry("", "extend_rule_testing"),
  allowlistEntry("", "subrule_testing"));
 
+ public static final ImmutableSet<AllowlistEntry> ALLOWLIST_RULE_EXTENSION_API_EXPERIMENTAL =
+ ImmutableSet.of(allowlistEntry("", "initializer_testing/builtins"));
+
  /** Parent rule class for test Starlark rules. */
  public static RuleClass getTestBaseRule(RuleDefinitionEnvironment env) {
  RepositoryName toolsRepository = env.getToolsRepository();
@@ -1062,14 +1066,23 @@ public Object call(StarlarkThread thread, Tuple args, Dict<String, Object> kwarg
  : Dict.cast(ret, String.class, Object.class, "rule's initializer return value");
 
  for (var arg : newKwargs.keySet()) {
- Attribute attr = currentRuleClass.getAttributeByNameMaybe(arg);
- if (attr != null && !(attr.isPublic() && attr.starlarkDefined())) {
+ checkAttributeName(arg);
+ if (arg.startsWith("_")) {
+ // allow setting private attributes from initializers in builtins
+ Label definitionLabel = ruleClass.getRuleDefinitionEnvironmentLabel();
+ BuiltinRestriction.failIfLabelOutsideAllowlist(
+ definitionLabel,
+ RepositoryMapping.ALWAYS_FALLBACK,
+ ALLOWLIST_RULE_EXTENSION_API_EXPERIMENTAL);
+ }
+ String nativeName = arg.startsWith("_") ? "$" + arg.substring(1) : arg;
+ Attribute attr = currentRuleClass.getAttributeByNameMaybe(nativeName);
+ if (attr != null && !attr.starlarkDefined()) {
  throw Starlark.errorf(
- "Initializer can only set public, Starlark defined attributes, not '%s'", arg);
+ "Initializer can only set Starlark defined attributes, not '%s'", arg);
  }
+ kwargs.putEntry(nativeName, newKwargs.get(arg));
  }
-
- kwargs.putEntries(newKwargs);
  }
 
  BuildLangTypedAttributeValuesMap attributeValues =

src/main/java/com/google/devtools/build/lib/packages/BuiltinRestriction.java

@@ -16,6 +16,7 @@
 import com.google.auto.value.AutoValue;
 import com.google.devtools.build.lib.cmdline.BazelModuleContext;
 import com.google.devtools.build.lib.cmdline.Label;
+import com.google.devtools.build.lib.cmdline.RepositoryMapping;
 import com.google.devtools.build.lib.vfs.PathFragment;
 import java.util.Collection;
 import net.starlark.java.eval.EvalException;
@@ -59,12 +60,9 @@ static AllowlistEntry create(String apparentRepoName, PathFragment packagePrefix
  return new AutoValue_BuiltinRestriction_AllowlistEntry(apparentRepoName, packagePrefix);
  }
 
- final boolean allows(BazelModuleContext moduleContext) {
- return moduleContext
- .label()
- .getRepository()
- .equals(moduleContext.repoMapping().get(apparentRepoName()))
- && moduleContext.label().getPackageFragment().startsWith(packagePrefix());
+ final boolean allows(Label label, RepositoryMapping repoMapping) {
+ return label.getRepository().equals(repoMapping.get(apparentRepoName()))
+ && label.getPackageFragment().startsWith(packagePrefix());
  }
  }
 
@@ -95,12 +93,21 @@ public static void failIfCalledOutsideAllowlist(
  */
  public static void failIfModuleOutsideAllowlist(
  BazelModuleContext moduleContext, Collection<AllowlistEntry> allowlist) throws EvalException {
- if (moduleContext.label().getRepository().getNameWithAt().equals("@_builtins")) {
+ failIfLabelOutsideAllowlist(moduleContext.label(), moduleContext.repoMapping(), allowlist);
+ }
+
+ /**
+ * Throws {@code EvalException} if the given {@link Label} is not within either 1) the builtins
+ * repository, or 2) a package or subpackage of an entry in the given allowlist.
+ */
+ public static void failIfLabelOutsideAllowlist(
+ Label label, RepositoryMapping repoMapping, Collection<AllowlistEntry> allowlist)
+ throws EvalException {
+ if (label.getRepository().getNameWithAt().equals("@_builtins")) {
  return;
  }
- if (allowlist.stream().noneMatch(e -> e.allows(moduleContext))) {
- throw Starlark.errorf(
- "file '%s' cannot use private API", moduleContext.label().getCanonicalForm());
+ if (allowlist.stream().noneMatch(e -> e.allows(label, repoMapping))) {
+ throw Starlark.errorf("file '%s' cannot use private API", label.getCanonicalForm());
  }
  }
 }

src/test/java/com/google/devtools/build/lib/starlark/StarlarkRuleClassFunctionsTest.java

@@ -2997,7 +2997,8 @@ public void initializer_incorrectReturnDicts() throws Exception {
  }
 
  @Test
- public void initializer_failsSettingInheritedPublicParameter() throws Exception {
+ public void initializer_failsSettingBaseAttribute() throws Exception {
+ // 'args' is an attribute defined for all executable rules
  scratch.file(
  "initializer_testing/b.bzl",
  "def initializer(srcs = [], deps = []):",
@@ -3020,8 +3021,66 @@ public void initializer_failsSettingInheritedPublicParameter() throws Exception
  reporter.addHandler(ev.getEventCollector());
  getConfiguredTarget("//initializer_testing:my_target");
 
- ev.assertContainsError(
- "Initializer can only set public, Starlark defined attributes, not 'args'");
+ ev.assertContainsError("Initializer can only set Starlark defined attributes, not 'args'");
+ }
+
+ @Test
+ public void initializer_failsSettingPrivateAttribute_outsideBuiltins() throws Exception {
+ scratch.file(
+ "initializer_testing/b.bzl",
+ "def initializer(srcs = [], deps = []):",
+ " return {'srcs': srcs, '_tool': ':my_tool'}",
+ "def impl(ctx): ",
+ " pass",
+ "my_rule = rule(impl,",
+ " initializer = initializer,",
+ " attrs = {",
+ " 'srcs': attr.label_list(allow_files = ['ml']),",
+ " '_tool': attr.label(),",
+ " })");
+ scratch.file(
+ "initializer_testing/BUILD", //
+ "load(':b.bzl','my_rule')",
+ "filegroup(name='my_tool')",
+ "my_rule(name = 'my_target', srcs = ['a.ml'])");
+
+ reporter.removeHandler(failFastHandler);
+ reporter.addHandler(ev.getEventCollector());
+ getConfiguredTarget("//initializer_testing:my_target");
+
+ ev.assertContainsError("file '//initializer_testing:b.bzl' cannot use private API");
+ }
+
+ @Test
+ public void initializer_settingPrivateAttribute_insideBuiltins() throws Exception {
+ scratch.file("initializer_testing/builtins/BUILD");
+ scratch.file(
+ "initializer_testing/builtins/b.bzl",
+ "def initializer(srcs = [], deps = []):",
+ " return {'srcs': srcs, '_tool': ':my_tool'}",
+ "MyInfo = provider()",
+ "def impl(ctx): ",
+ " return MyInfo(_tool = str(ctx.attr._tool.label))",
+ "my_rule = rule(impl,",
+ " initializer = initializer,",
+ " attrs = {",
+ " 'srcs': attr.label_list(allow_files = ['ml']),",
+ " '_tool': attr.label(),",
+ " })");
+ scratch.file(
+ "initializer_testing/BUILD", //
+ "load('//initializer_testing/builtins:b.bzl','my_rule')",
+ "filegroup(name='my_tool')",
+ "my_rule(name = 'my_target', srcs = ['a.ml'])");
+
+ ConfiguredTarget myTarget = getConfiguredTarget("//initializer_testing:my_target");
+ StructImpl info =
+ (StructImpl)
+ myTarget.get(
+ new StarlarkProvider.Key(
+ Label.parseCanonical("//initializer_testing/builtins:b.bzl"), "MyInfo"));
+
+ assertThat(info.getValue("_tool").toString()).isEqualTo("@@//initializer_testing:my_tool");
  }
 
  @Test