Automated rollback of commit cecc26b814441f5b7634b42905a0d7be0a321fa8.
*** Reason for rollback ***
Breaks downstream:
- https://buildkite.com/bazel/rules-java-java/builds/3286
- https://buildkite.com/bazel/rules-cc/builds/4524
*** Original change description ***
Avoid repo mapping lookup to check builtin restrictions
BuiltinRestriction currently accounts for 10-20% of the total CPU time for `bazel build --nobuild //src:bazel`. This is improved by avoiding the repeated creation of "non-visible" `RepositoryName` objects, which trigger a `SpellChecker` computation. Instead, the apparent repo names, which are in fact either module names or legacy WORKSPACE repo names that no longer matter in Bazel 9...
PiperOrigin-RevId: 854510657
Change-Id: I3c13b2269c7961583111ec3e20bdd67e141d7277
diff --git a/src/main/java/com/google/devtools/build/lib/analysis/RuleConfiguredTargetBuilder.java b/src/main/java/com/google/devtools/build/lib/analysis/RuleConfiguredTargetBuilder.java
index 8cd1efe..48347b9 100644
--- a/src/main/java/com/google/devtools/build/lib/analysis/RuleConfiguredTargetBuilder.java
+++ b/src/main/java/com/google/devtools/build/lib/analysis/RuleConfiguredTargetBuilder.java
@@ -42,6 +42,7 @@
import com.google.devtools.build.lib.analysis.test.TestProvider.TestParams;
import com.google.devtools.build.lib.analysis.test.TestTagsProvider;
import com.google.devtools.build.lib.cmdline.Label;
+import com.google.devtools.build.lib.cmdline.RepositoryMapping;
import com.google.devtools.build.lib.collect.nestedset.NestedSet;
import com.google.devtools.build.lib.collect.nestedset.NestedSetBuilder;
import com.google.devtools.build.lib.collect.nestedset.Order;
@@ -370,7 +371,9 @@
// only allow native and builtins to override transitive validation propagation
if (rdeLabel != null
&& BuiltinRestriction.isNotAllowed(
- rdeLabel, BuiltinRestriction.INTERNAL_STARLARK_API_ALLOWLIST)) {
+ rdeLabel,
+ RepositoryMapping.EMPTY,
+ BuiltinRestriction.INTERNAL_STARLARK_API_ALLOWLIST)) {
ruleContext.ruleError(rdeLabel + " cannot access the _transitive_validation private API");
return;
}
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 548bdb6..4b3f0f4 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
@@ -1836,7 +1836,9 @@
// allow setting private attributes from initializers in builtins
Label definitionLabel = currentRuleClass.getRuleDefinitionEnvironmentLabel();
BuiltinRestriction.failIfLabelOutsideAllowlist(
- definitionLabel, ALLOWLIST_RULE_EXTENSION_API_EXPERIMENTAL);
+ definitionLabel,
+ targetDefinitionContext.getMainRepoMapping(),
+ ALLOWLIST_RULE_EXTENSION_API_EXPERIMENTAL);
}
String nativeName = arg.startsWith("_") ? "$" + arg.substring(1) : arg;
Attribute attr =
diff --git a/src/main/java/com/google/devtools/build/lib/packages/BuiltinRestriction.java b/src/main/java/com/google/devtools/build/lib/packages/BuiltinRestriction.java
index 8945fc2..f4778e4 100644
--- a/src/main/java/com/google/devtools/build/lib/packages/BuiltinRestriction.java
+++ b/src/main/java/com/google/devtools/build/lib/packages/BuiltinRestriction.java
@@ -17,7 +17,7 @@
import com.google.common.collect.ImmutableList;
import com.google.devtools.build.lib.cmdline.BazelModuleContext;
import com.google.devtools.build.lib.cmdline.Label;
-import com.google.devtools.build.lib.cmdline.RepositoryName;
+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;
@@ -47,6 +47,7 @@
BuiltinRestriction.allowlistEntry("", "tools/build_defs/android"),
BuiltinRestriction.allowlistEntry("", "third_party/bazel_rules/rules_android"),
BuiltinRestriction.allowlistEntry("rules_android", ""),
+ BuiltinRestriction.allowlistEntry("build_bazel_rules_android", ""),
// Apple rules
BuiltinRestriction.allowlistEntry("", "third_party/apple_crosstool"),
@@ -54,6 +55,7 @@
"", "third_party/cpptoolchains/portable_llvm/build_defs"),
BuiltinRestriction.allowlistEntry("", "third_party/bazel_rules/rules_apple"),
BuiltinRestriction.allowlistEntry("rules_apple", ""),
+ BuiltinRestriction.allowlistEntry("build_bazel_rules_apple", ""),
// Cc rules
BuiltinRestriction.allowlistEntry("", "third_party/bazel_rules/rules_cc"),
@@ -82,6 +84,7 @@
// Proto rules
BuiltinRestriction.allowlistEntry("", "third_party/protobuf"),
BuiltinRestriction.allowlistEntry("protobuf", ""),
+ BuiltinRestriction.allowlistEntry("com_google_protobuf", ""),
// Shell rules
BuiltinRestriction.allowlistEntry("rules_shell", ""));
@@ -117,25 +120,10 @@
return new AutoValue_BuiltinRestriction_AllowlistEntry(apparentRepoName, packagePrefix);
}
- final boolean allows(Label label) {
- return reposMatch(apparentRepoName(), label.getRepository())
+ final boolean allows(Label label, RepositoryMapping repoMapping) {
+ return label.getRepository().equals(repoMapping.get(apparentRepoName()))
&& label.getPackageFragment().startsWith(packagePrefix());
}
-
- private static boolean reposMatch(String allowedName, RepositoryName givenName) {
- if (allowedName.equals(RepositoryName.MAIN.getName())) {
- return givenName.equals(RepositoryName.MAIN);
- }
- if (allowedName.equals(RepositoryName.BAZEL_TOOLS.getName())) {
- return givenName.equals(RepositoryName.BAZEL_TOOLS);
- }
- if (allowedName.equals(RepositoryName.BUILTINS.getName())) {
- return givenName.equals(RepositoryName.BUILTINS);
- }
- // allowedName is a module name and givenName is a real canonical repo name, so it belongs to
- // any version of that module if and only if it contains <allowedName>+ as a prefix.
- return givenName.getName().startsWith(allowedName + "+");
- }
}
/**
@@ -177,16 +165,17 @@
*/
public static void failIfModuleOutsideAllowlist(
BazelModuleContext moduleContext, Collection<AllowlistEntry> allowlist) throws EvalException {
- failIfLabelOutsideAllowlist(moduleContext.label(), allowlist);
+ 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, Collection<AllowlistEntry> allowlist)
+ public static void failIfLabelOutsideAllowlist(
+ Label label, RepositoryMapping repoMapping, Collection<AllowlistEntry> allowlist)
throws EvalException {
- if (isNotAllowed(label, allowlist)) {
+ if (isNotAllowed(label, repoMapping, allowlist)) {
throw Starlark.errorf("file '%s' cannot use private API", label.getCanonicalForm());
}
}
@@ -195,10 +184,11 @@
* Returns true if the given {@link Label} is not within both 1) the builtins repository, or 2) a
* package or subpackage of an entry in the given allowlist.
*/
- public static boolean isNotAllowed(Label label, Collection<AllowlistEntry> allowlist) {
+ public static boolean isNotAllowed(
+ Label label, RepositoryMapping repoMapping, Collection<AllowlistEntry> allowlist) {
if (label.getRepository().getName().equals("_builtins")) {
return false;
}
- return allowlist.stream().noneMatch(e -> e.allows(label));
+ return allowlist.stream().noneMatch(e -> e.allows(label, repoMapping));
}
}
