Optionally restrict toolchain registration to single packages

This restriction is tied to an experimental feature flag --experimental_single_package_toolchain_binding. This flag will exist as an option for projects to restrict more complex bindings in WORKSPACE and MODULE files, but will not be flipped true-by-default for the foreseeable future.

PiperOrigin-RevId: 631870945
Change-Id: I93f1eda65c2d8f6af34f7e43bc15dca0e6a0d616

src/main/java/com/google/devtools/build/lib/bazel/bzlmod/BUILD

@@ -162,6 +162,7 @@ java_library(
         "//src/main/java/com/google/devtools/build/lib/cmdline",
         "//src/main/java/com/google/devtools/build/lib/events",
         "//src/main/java/com/google/devtools/build/lib/packages",
+        "//src/main/java/com/google/devtools/build/lib/packages/semantics",
         "//src/main/java/com/google/devtools/build/lib/rules:repository/repo_recorded_input",
         "//src/main/java/com/google/devtools/build/lib/skyframe:sky_functions",
         "//src/main/java/com/google/devtools/build/lib/skyframe/serialization:visible-for-serialization",

src/main/java/com/google/devtools/build/lib/bazel/bzlmod/ModuleFileGlobals.java

@@ -33,6 +33,7 @@
 import com.google.devtools.build.lib.cmdline.RepositoryName;
 import com.google.devtools.build.lib.events.EventHandler;
 import com.google.devtools.build.lib.packages.StarlarkExportable;
+import com.google.devtools.build.lib.packages.semantics.BuildLanguageOptions;
 import com.google.devtools.build.lib.vfs.PathFragment;
 import java.util.Map;
 import java.util.regex.Pattern;
@@ -369,9 +370,21 @@ public void registerToolchains(
     if (context.shouldIgnoreDevDeps() && devDependency) {
       return;
     }
-    context
-        .getModuleBuilder()
-        .addToolchainsToRegister(checkAllAbsolutePatterns(toolchainLabels, "register_toolchains"));
+    ImmutableList<String> checkedToolchainLabels =
+        checkAllAbsolutePatterns(toolchainLabels, "register_toolchains");
+    if (thread
+        .getSemantics()
+        .getBool(BuildLanguageOptions.EXPERIMENTAL_SINGLE_PACKAGE_TOOLCHAIN_BINDING)) {
+      for (String label : checkedToolchainLabels) {
+        if (label.contains("...")) {
+          throw Starlark.errorf(
+              "invalid target pattern \"%s\": register_toolchain target patterns may only refer to "
+                  + "targets within a single package",
+              label);
+        }
+      }
+    }
+    context.getModuleBuilder().addToolchainsToRegister(checkedToolchainLabels);
   }
 
   @StarlarkMethod(

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

@@ -30,6 +30,7 @@
 import com.google.devtools.build.lib.cmdline.TargetPattern;
 import com.google.devtools.build.lib.packages.RuleFactory.InvalidRuleException;
 import com.google.devtools.build.lib.packages.TargetDefinitionContext.NameConflictException;
+import com.google.devtools.build.lib.packages.semantics.BuildLanguageOptions;
 import com.google.devtools.build.lib.starlarkbuildapi.WorkspaceGlobalsApi;
 import com.google.devtools.build.lib.vfs.PathFragment;
 import java.util.List;
@@ -118,9 +119,23 @@ public void registerToolchains(Sequence<?> toolchainLabels, StarlarkThread threa
     Package.Builder builder =
         Package.Builder.fromOrFailDisallowingSymbolicMacros(thread, "register_toolchains()");
     List<String> patterns = Sequence.cast(toolchainLabels, String.class, "toolchain_labels");
+
+    ImmutableList<TargetPattern> targetPatterns = parsePatterns(patterns, builder, thread);
+
+    if (thread
+        .getSemantics()
+        .getBool(BuildLanguageOptions.EXPERIMENTAL_SINGLE_PACKAGE_TOOLCHAIN_BINDING)) {
+      for (TargetPattern tp : targetPatterns) {
+        if (tp.getType() == TargetPattern.Type.TARGETS_BELOW_DIRECTORY) {
+          throw Starlark.errorf(
+              "invalid target pattern \"%s\": register_toolchain target patterns may only refer to "
+                  + "targets within a single package",
+              tp.getOriginalPattern());
+        }
+      }
+    }
     builder.addRegisteredToolchains(
-        parsePatterns(patterns, builder, thread),
-        originatesInWorkspaceSuffix(thread.getCallStack()));
+        targetPatterns, originatesInWorkspaceSuffix(thread.getCallStack()));
   }
 
   @Override

src/main/java/com/google/devtools/build/lib/packages/semantics/BuildLanguageOptions.java

@@ -157,6 +157,17 @@ public final class BuildLanguageOptions extends OptionsBase {
               + " evaluation to set their visibility for the purpose of load() statements.")
   public boolean experimentalBzlVisibility;
 
+  @Option(
+      name = "experimental_single_package_toolchain_binding",
+      defaultValue = "false",
+      documentationCategory = OptionDocumentationCategory.STARLARK_SEMANTICS,
+      effectTags = {OptionEffectTag.LOADING_AND_ANALYSIS},
+      metadataTags = {OptionMetadataTag.INCOMPATIBLE_CHANGE},
+      help =
+          "If enabled, the register_toolchain function may not include target patterns which may "
+              + "refer to more than one package.")
+  public boolean experimentalSinglePackageToolchainBinding;
+
   @Option(
       name = "check_bzl_visibility",
       defaultValue = "true",
@@ -748,6 +759,9 @@ public StarlarkSemantics toStarlarkSemantics() {
             .setBool(CHECK_BZL_VISIBILITY, checkBzlVisibility)
             .setBool(
                 EXPERIMENTAL_ENABLE_ANDROID_MIGRATION_APIS, experimentalEnableAndroidMigrationApis)
+            .setBool(
+                EXPERIMENTAL_SINGLE_PACKAGE_TOOLCHAIN_BINDING,
+                experimentalSinglePackageToolchainBinding)
             .setBool(EXPERIMENTAL_ENABLE_FIRST_CLASS_MACROS, experimentalEnableFirstClassMacros)
             .setBool(EXPERIMENTAL_ENABLE_SCL_DIALECT, experimentalEnableSclDialect)
             .setBool(ENABLE_BZLMOD, enableBzlmod)
@@ -853,6 +867,8 @@ public StarlarkSemantics toStarlarkSemantics() {
       "-experimental_disable_external_package";
   public static final String EXPERIMENTAL_ENABLE_ANDROID_MIGRATION_APIS =
       "-experimental_enable_android_migration_apis";
+  public static final String EXPERIMENTAL_SINGLE_PACKAGE_TOOLCHAIN_BINDING =
+      "-experimental_single_package_toolchain_binding";
   public static final String EXPERIMENTAL_ENABLE_FIRST_CLASS_MACROS =
       "-experimental_enable_first_class_macros";
   public static final String EXPERIMENTAL_ENABLE_SCL_DIALECT = "-experimental_enable_scl_dialect";

src/test/java/com/google/devtools/build/lib/bazel/bzlmod/ModuleFileFunctionTest.java

@@ -1476,4 +1476,55 @@ public void isolatedUsage_notEnabled() throws Exception {
             + "and thus unavailable with the current flags. It may be enabled by setting "
             + "--experimental_isolated_extension_usages");
   }
+
+  @Test
+  public void testRegisterToolchains_singlePackageRestriction_error() throws Exception {
+    // Test intentionally introduces errors.
+    reporter.removeHandler(failFastHandler);
+    PrecomputedValue.STARLARK_SEMANTICS.set(
+        differencer,
+        StarlarkSemantics.builder()
+            .setBool(BuildLanguageOptions.EXPERIMENTAL_SINGLE_PACKAGE_TOOLCHAIN_BINDING, true)
+            .build());
+
+    scratch.overwriteFile(
+        rootDirectory.getRelative("MODULE.bazel").getPathString(),
+        "module(name='aaa')",
+        "register_toolchains('//bar/...')");
+
+    FakeRegistry registry = registryFactory.newFakeRegistry("/foo");
+    ModuleFileFunction.REGISTRIES.set(differencer, ImmutableSet.of(registry.getUrl()));
+
+    EvaluationResult<RootModuleFileValue> result =
+        evaluator.evaluate(
+            ImmutableList.of(ModuleFileValue.KEY_FOR_ROOT_MODULE), evaluationContext);
+    assertThat(result.hasError()).isTrue();
+
+    assertContainsEvent(
+        "invalid target pattern \"//bar/...\": register_toolchain target patterns "
+            + "may only refer to targets within a single package");
+  }
+
+  @Test
+  public void testRegisterToolchains_singlePackageRestriction() throws Exception {
+    PrecomputedValue.STARLARK_SEMANTICS.set(
+        differencer,
+        StarlarkSemantics.builder()
+            .setBool(BuildLanguageOptions.EXPERIMENTAL_SINGLE_PACKAGE_TOOLCHAIN_BINDING, true)
+            .build());
+
+    scratch.overwriteFile(
+        rootDirectory.getRelative("MODULE.bazel").getPathString(),
+        "module(name='aaa')",
+        "register_toolchains('//:whatever')",
+        "register_toolchains('//bar:all')",
+        "register_toolchains('//qux:baz')");
+    FakeRegistry registry = registryFactory.newFakeRegistry("/foo");
+    ModuleFileFunction.REGISTRIES.set(differencer, ImmutableSet.of(registry.getUrl()));
+
+    EvaluationResult<RootModuleFileValue> result =
+        evaluator.evaluate(
+            ImmutableList.of(ModuleFileValue.KEY_FOR_ROOT_MODULE), evaluationContext);
+    assertThat(result.hasError()).isFalse();
+  }
 }

src/test/java/com/google/devtools/build/lib/packages/semantics/ConsistencyTest.java

@@ -123,6 +123,7 @@ private static BuildLanguageOptions buildRandomOptions(Random rand) throws Excep
         "--experimental_builtins_dummy=" + rand.nextBoolean(),
         "--experimental_bzl_visibility=" + rand.nextBoolean(),
         "--experimental_enable_android_migration_apis=" + rand.nextBoolean(),
+        "--experimental_single_package_toolchain_binding=" + rand.nextBoolean(),
         "--enable_bzlmod=" + rand.nextBoolean(),
         "--enable_workspace=" + rand.nextBoolean(),
         "--experimental_isolated_extension_usages=" + rand.nextBoolean(),
@@ -167,6 +168,8 @@ private static StarlarkSemantics buildRandomSemantics(Random rand) {
         .setBool(BuildLanguageOptions.EXPERIMENTAL_BZL_VISIBILITY, rand.nextBoolean())
         .setBool(
             BuildLanguageOptions.EXPERIMENTAL_ENABLE_ANDROID_MIGRATION_APIS, rand.nextBoolean())
+        .setBool(
+            BuildLanguageOptions.EXPERIMENTAL_SINGLE_PACKAGE_TOOLCHAIN_BINDING, rand.nextBoolean())
         .setBool(BuildLanguageOptions.ENABLE_BZLMOD, rand.nextBoolean())
         .setBool(BuildLanguageOptions.ENABLE_WORKSPACE, rand.nextBoolean())
         .setBool(BuildLanguageOptions.EXPERIMENTAL_ISOLATED_EXTENSION_USAGES, rand.nextBoolean())

src/test/java/com/google/devtools/build/lib/skyframe/WorkspaceFileFunctionTest.java

@@ -253,6 +253,42 @@ public void testRegisterToolchainsInvalidPattern() throws Exception {
     assertContainsEvent("error parsing target pattern");
   }
 
+  @Test
+  public void testRegisterToolchains_singlePackageRestriction() throws Exception {
+    setBuildLanguageOptions("--experimental_single_package_toolchain_binding");
+
+    String[] lines = {
+      "register_toolchains('//foo:all')",
+      "register_toolchains('//bar:bar')",
+      "register_toolchains('//pkg/to/baz')"
+    };
+    createWorkspaceFile(lines);
+
+    SkyKey key = ExternalPackageFunction.key();
+    EvaluationResult<PackageValue> evaluationResult = eval(key);
+    Package pkg = evaluationResult.get(key).getPackage();
+    assertThat(pkg.containsErrors()).isFalse();
+  }
+
+  @Test
+  public void testRegisterToolchains_singlePackageRestriction_error() throws Exception {
+    // Test intentionally introduces errors.
+    reporter.removeHandler(failFastHandler);
+
+    setBuildLanguageOptions("--experimental_single_package_toolchain_binding");
+
+    String[] lines = {"register_toolchains('//foo/...')"};
+    createWorkspaceFile(lines);
+
+    SkyKey key = ExternalPackageFunction.key();
+    EvaluationResult<PackageValue> evaluationResult = eval(key);
+    Package pkg = evaluationResult.get(key).getPackage();
+    assertThat(pkg.containsErrors()).isTrue();
+    assertContainsEvent(
+        "invalid target pattern \"//foo/...\": register_toolchain target patterns "
+            + "may only refer to targets within a single package");
+  }
+
   @Test
   public void testNoWorkspaceFile() throws Exception {
     // Create and immediately delete to make sure we got the right file.