Add new flag --incompatible_disable_native_repo_rules

This new flag defaults to `false`. When set to `true`, native repo rules cannot be used in WORKSPACE; their Starlark counterparts must be used. Native repo rules already can't be used in Bzlmod.

Work towards #18285.

src/main/java/com/google/devtools/build/lib/analysis/ConfiguredRuleClassProvider.java

@@ -182,6 +182,13 @@ public Builder addWorkspaceFilePrefix(String contents) {
       return this;
     }
 
+    @CanIgnoreReturnValue
+    @VisibleForTesting
+    public Builder clearWorkspaceFilePrefixForTesting() {
+      defaultWorkspaceFilePrefix.delete(0, defaultWorkspaceFilePrefix.length());
+      return this;
+    }
+
     @CanIgnoreReturnValue
     public Builder addWorkspaceFileSuffix(String contents) {
       defaultWorkspaceFileSuffix.append(contents);

src/main/java/com/google/devtools/build/lib/bazel/BazelRepositoryModule.java

@@ -158,6 +158,7 @@ public class BazelRepositoryModule extends BlazeModule {
 
   private Optional<Path> vendorDirectory;
   private List<String> allowedYankedVersions = ImmutableList.of();
+  private boolean disableNativeRepoRules;
   private SingleExtensionEvalFunction singleExtensionEvalFunction;
   private final ExecutorService repoFetchingWorkerThreadPool =
       Executors.newFixedThreadPool(
@@ -343,6 +344,7 @@ public void beforeCommand(CommandEnvironment env) throws AbruptExitException {
       if (repoOptions.repositoryDownloaderRetries >= 0) {
         downloadManager.setRetries(repoOptions.repositoryDownloaderRetries);
       }
+      disableNativeRepoRules = repoOptions.disableNativeRepoRules;
 
       repositoryCache.setHardlink(repoOptions.useHardlinks);
       if (repoOptions.experimentalScaleTimeouts > 0.0) {
@@ -594,7 +596,9 @@ public ImmutableList<Injected> getPrecomputedValues() {
         PrecomputedValue.injected(RepositoryDelegatorFunction.IS_VENDOR_COMMAND, false),
         PrecomputedValue.injected(RepositoryDelegatorFunction.VENDOR_DIRECTORY, vendorDirectory),
         PrecomputedValue.injected(
-            YankedVersionsUtil.ALLOWED_YANKED_VERSIONS, allowedYankedVersions));
+            YankedVersionsUtil.ALLOWED_YANKED_VERSIONS, allowedYankedVersions),
+        PrecomputedValue.injected(
+            RepositoryDelegatorFunction.DISABLE_NATIVE_REPO_RULES, disableNativeRepoRules));
   }
 
   @Override

src/main/java/com/google/devtools/build/lib/bazel/repository/LocalConfigPlatformFunction.java

@@ -55,7 +55,8 @@ public RepositoryDirectoryValue.Builder fetch(
       Environment env,
       Map<RepoRecordedInput, String> recordedInputValues,
       SkyKey key)
-      throws RepositoryFunctionException {
+      throws RepositoryFunctionException, InterruptedException {
+    ensureNativeRepoRuleEnabled(rule, env, "the platform defined at @platforms//host");
     String name = rule.getName();
     try {
       outputDirectory.createDirectoryAndParents();

src/main/java/com/google/devtools/build/lib/bazel/repository/RepositoryOptions.java

@@ -97,6 +97,18 @@ public class RepositoryOptions extends OptionsBase {
               + " still run an arbitrary executable that accesses the Internet.")
   public boolean disableDownload;
 
+  @Option(
+      name = "incompatible_disable_native_repo_rules",
+      defaultValue = "false",
+      documentationCategory = OptionDocumentationCategory.BAZEL_CLIENT_OPTIONS,
+      effectTags = {OptionEffectTag.BAZEL_INTERNAL_CONFIGURATION},
+      help =
+          "If false, native repo rules can be used in WORKSPACE; otherwise, Starlark repo rules "
+              + "must be used instead. Native repo rules include local_repository, "
+              + "new_local_repository, local_config_platform, android_sdk_repository, and "
+              + "android_ndk_repository.")
+  public boolean disableNativeRepoRules;
+
   @Option(
       name = "experimental_repository_downloader_retries",
       defaultValue = "0",

src/main/java/com/google/devtools/build/lib/bazel/repository/local_config_platform.WORKSPACE

@@ -1,3 +1,4 @@
+load("@bazel_tools//tools/build_defs/repo:local.bzl", "local_repository")
 load("@bazel_tools//tools/build_defs/repo:utils.bzl", "maybe")
 
 maybe(

src/main/java/com/google/devtools/build/lib/bazel/rules/android/AndroidNdkRepositoryFunction.java

@@ -272,6 +272,7 @@ public RepositoryDirectoryValue.Builder fetch(
       Map<RepoRecordedInput, String> recordedInputValues,
       SkyKey key)
       throws InterruptedException, RepositoryFunctionException {
+    ensureNativeRepoRuleEnabled(rule, env, "https://github.com/bazelbuild/rules_android_ndk");
     Map<String, String> environ =
         declareEnvironmentDependencies(recordedInputValues, env, PATH_ENV_VAR_AS_SET);
     if (environ == null) {

src/main/java/com/google/devtools/build/lib/bazel/rules/android/AndroidSdkRepositoryFunction.java

@@ -201,6 +201,7 @@ public RepositoryDirectoryValue.Builder fetch(
       Map<RepoRecordedInput, String> recordedInputValues,
       SkyKey key)
       throws RepositoryFunctionException, InterruptedException {
+    ensureNativeRepoRuleEnabled(rule, env, "https://github.com/bazelbuild/rules_android");
     Map<String, String> environ =
         declareEnvironmentDependencies(recordedInputValues, env, PATH_ENV_VAR_AS_SET);
     if (environ == null) {

src/main/java/com/google/devtools/build/lib/bazel/rules/java/rules_java_builtin.WORKSPACE

@@ -1,4 +1,5 @@
-local_repository(
+load("@bazel_tools//tools/build_defs/repo:local.bzl", rules_java_builtin_local_repository = "local_repository")
+rules_java_builtin_local_repository(
     name = "rules_java_builtin",
     path = __embedded_dir__ + "/rules_java",
     repo_mapping = {"@rules_java" : "@rules_java_builtin"}

src/main/java/com/google/devtools/build/lib/rules/repository/LocalRepositoryFunction.java

@@ -46,6 +46,8 @@ public RepositoryDirectoryValue.Builder fetch(
       Map<RepoRecordedInput, String> recordedInputValues,
       SkyKey key)
       throws InterruptedException, RepositoryFunctionException {
+    ensureNativeRepoRuleEnabled(
+        rule, env, "load(\"@bazel_tools//tools/build_defs/repo:local.bzl\", \"local_repository\")");
     // DO NOT MODIFY THIS! It's being deprecated in favor of Starlark counterparts.
     // See https://github.com/bazelbuild/bazel/issues/18285
     String userDefinedPath = RepositoryFunction.getPathAttr(rule);

src/main/java/com/google/devtools/build/lib/rules/repository/NewLocalRepositoryFunction.java

@@ -62,6 +62,10 @@ public RepositoryDirectoryValue.Builder fetch(
       Map<RepoRecordedInput, String> recordedInputValues,
       SkyKey key)
       throws InterruptedException, RepositoryFunctionException {
+    ensureNativeRepoRuleEnabled(
+        rule,
+        env,
+        "load(\"@bazel_tools//tools/build_defs/repo:local.bzl\", \"new_local_repository\")");
     // DO NOT MODIFY THIS! It's being deprecated in favor of Starlark counterparts.
     // See https://github.com/bazelbuild/bazel/issues/18285
 

src/main/java/com/google/devtools/build/lib/rules/repository/RepositoryDelegatorFunction.java

@@ -90,6 +90,9 @@ public final class RepositoryDelegatorFunction implements SkyFunction {
   public static final Precomputed<Optional<Path>> VENDOR_DIRECTORY =
       new Precomputed<>("vendor_directory");
 
+  public static final Precomputed<Boolean> DISABLE_NATIVE_REPO_RULES =
+      new Precomputed<>("disable_native_repo_rules");
+
   // The marker file version is inject in the rule key digest so the rule key is always different
   // when we decide to update the format.
   private static final int MARKER_FILE_VERSION = 7;

src/main/java/com/google/devtools/build/lib/rules/repository/RepositoryFunction.java

@@ -187,6 +187,27 @@ public abstract RepositoryDirectoryValue.Builder fetch(
       SkyKey key)
       throws InterruptedException, RepositoryFunctionException;
 
+  protected static void ensureNativeRepoRuleEnabled(Rule rule, Environment env, String replacement)
+      throws RepositoryFunctionException, InterruptedException {
+    if (!isWorkspaceRepo(rule)) {
+      // If this native repo rule is used in a Bzlmod context, always allow it. This is because
+      // we're still using the native `local_repository` for `local_path_override`, and it's
+      // nontrivial to migrate that one to the Starlark version.
+      return;
+    }
+    if (!RepositoryDelegatorFunction.DISABLE_NATIVE_REPO_RULES.get(env)) {
+      return;
+    }
+    throw new RepositoryFunctionException(
+        Starlark.errorf(
+            "Native repo rule %s is disabled since the flag "
+                + "--incompatible_disable_native_repo_rules is set. Native repo rules are "
+                + "deprecated; please migrate to their Starlark counterparts. For %s, please use "
+                + "%s.",
+            rule.getRuleClass(), rule.getRuleClass(), replacement),
+        Transience.PERSISTENT);
+  }
+
   /**
    * Verify the data provided by the marker file to check if a refetch is needed. Returns true if
    * the data is up to date and no refetch is needed and false if the data is obsolete and a refetch

src/main/java/com/google/devtools/build/lib/skyframe/packages/BazelPackageLoader.java

@@ -24,6 +24,7 @@
 import com.google.devtools.build.lib.bazel.bzlmod.ModuleFileFunction;
 import com.google.devtools.build.lib.bazel.bzlmod.RegistryFactory;
 import com.google.devtools.build.lib.bazel.bzlmod.RegistryFactoryImpl;
+import com.google.devtools.build.lib.bazel.bzlmod.RepoSpecFunction;
 import com.google.devtools.build.lib.bazel.bzlmod.YankedVersionsUtil;
 import com.google.devtools.build.lib.bazel.repository.RepositoryOptions;
 import com.google.devtools.build.lib.bazel.repository.cache.RepositoryCache;
@@ -51,6 +52,7 @@
 import com.google.devtools.build.skyframe.SkyFunction;
 import com.google.devtools.build.skyframe.SkyFunctionName;
 import com.google.errorprone.annotations.CanIgnoreReturnValue;
+import java.util.Map;
 import java.util.Optional;
 import java.util.concurrent.atomic.AtomicBoolean;
 import java.util.concurrent.atomic.AtomicReference;
@@ -112,8 +114,10 @@ private Builder(Root workspaceDir, Path installBase, Path outputBase, AtomicBool
               RepositoryDelegatorFunction.FORCE_FETCH_CONFIGURE,
               RepositoryDelegatorFunction.FORCE_FETCH_DISABLED),
           PrecomputedValue.injected(RepositoryDelegatorFunction.VENDOR_DIRECTORY, Optional.empty()),
-          PrecomputedValue.injected(ModuleFileFunction.REGISTRIES, ImmutableList.of()),
+          PrecomputedValue.injected(
+              ModuleFileFunction.REGISTRIES, BazelRepositoryModule.DEFAULT_REGISTRIES),
           PrecomputedValue.injected(ModuleFileFunction.IGNORE_DEV_DEPS, false),
+          PrecomputedValue.injected(RepositoryDelegatorFunction.DISABLE_NATIVE_REPO_RULES, false),
           PrecomputedValue.injected(
               BazelModuleResolutionFunction.CHECK_DIRECT_DEPENDENCIES,
               RepositoryOptions.CheckDirectDepsMode.OFF),
@@ -136,7 +140,7 @@ public BazelPackageLoader buildImpl() {
           new RegistryFactoryImpl(
               directories.getWorkspace(), downloadManager, Suppliers.ofInstance(ImmutableMap.of()));
 
-      // Allow tests to override SkyFunctions.MODULE_FILE to use fake registry
+      // Allow tests to override the following functions to use fake registry
       if (!this.extraSkyFunctions.containsKey(SkyFunctions.MODULE_FILE)) {
         addExtraSkyFunctions(
             ImmutableMap.of(
@@ -145,7 +149,16 @@ public BazelPackageLoader buildImpl() {
                     ruleClassProvider.getBazelStarlarkEnvironment(),
                     registryFactory,
                     directories.getWorkspace(),
-                    ImmutableMap.of())));
+                    ModuleFileFunction.getBuiltinModules(directories.getEmbeddedBinariesRoot())
+                        .entrySet()
+                        .stream()
+                        .filter(e -> e.getKey().equals("bazel_tools"))
+                        .collect(
+                            ImmutableMap.toImmutableMap(Map.Entry::getKey, Map.Entry::getValue)))));
+      }
+      if (!this.extraSkyFunctions.containsKey(SkyFunctions.REPO_SPEC)) {
+        addExtraSkyFunctions(
+            ImmutableMap.of(SkyFunctions.REPO_SPEC, new RepoSpecFunction(registryFactory)));
       }
 
       addExtraSkyFunctions(

src/test/java/com/google/devtools/build/lib/analysis/mock/BazelAnalysisMock.java

@@ -886,6 +886,15 @@ def http_file(**kwargs):
         def http_jar(**kwargs):
             pass
         """);
+    config.create(
+        "embedded_tools/tools/build_defs/repo/local.bzl",
+        """
+        def local_repository(**kwargs):
+            pass
+
+        def new_local_repository(**kwargs):
+            pass
+        """);
     config.create("embedded_tools/tools/jdk/jdk_build_file.bzl", "JDK_BUILD_TEMPLATE = ''");
     config.create(
         "embedded_tools/tools/jdk/local_java_repository.bzl",

src/test/java/com/google/devtools/build/lib/analysis/util/AnalysisMock.java

@@ -200,6 +200,7 @@ public ImmutableList<PrecomputedValue.Injected> getPrecomputedValues() {
             RepositoryDelegatorFunction.FORCE_FETCH,
             RepositoryDelegatorFunction.FORCE_FETCH_DISABLED),
         PrecomputedValue.injected(RepositoryDelegatorFunction.VENDOR_DIRECTORY, Optional.empty()),
+        PrecomputedValue.injected(RepositoryDelegatorFunction.DISABLE_NATIVE_REPO_RULES, false),
         PrecomputedValue.injected(ModuleFileFunction.REGISTRIES, ImmutableList.of()),
         PrecomputedValue.injected(ModuleFileFunction.IGNORE_DEV_DEPS, false),
         PrecomputedValue.injected(ModuleFileFunction.MODULE_OVERRIDES, ImmutableMap.of()),

src/test/java/com/google/devtools/build/lib/analysis/util/AnalysisTestCase.java

@@ -237,6 +237,8 @@ protected void useRuleClassProvider(ConfiguredRuleClassProvider ruleClassProvide
                     PrecomputedValue.injected(
                         ModuleFileFunction.REGISTRIES, ImmutableList.of(registry.getUrl())),
                     PrecomputedValue.injected(ModuleFileFunction.IGNORE_DEV_DEPS, false),
+                    PrecomputedValue.injected(
+                        RepositoryDelegatorFunction.DISABLE_NATIVE_REPO_RULES, false),
                     PrecomputedValue.injected(
                         ModuleFileFunction.MODULE_OVERRIDES, ImmutableMap.of()),
                     PrecomputedValue.injected(
@@ -289,6 +291,8 @@ private void reinitializeSkyframeExecutor() {
             PrecomputedValue.injected(
                 ModuleFileFunction.REGISTRIES, ImmutableList.of(registry.getUrl())),
             PrecomputedValue.injected(ModuleFileFunction.IGNORE_DEV_DEPS, false),
+            PrecomputedValue.injected(
+                RepositoryDelegatorFunction.DISABLE_NATIVE_REPO_RULES, false),
             PrecomputedValue.injected(
                 BazelModuleResolutionFunction.CHECK_DIRECT_DEPENDENCIES,
                 CheckDirectDepsMode.WARNING),

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

@@ -171,6 +171,7 @@ public void setup() throws Exception {
     ConfiguredRuleClassProvider.Builder builder = new ConfiguredRuleClassProvider.Builder();
     TestRuleClassProvider.addStandardRules(builder);
     builder
+        .clearWorkspaceFilePrefixForTesting()
         .clearWorkspaceFileSuffixForTesting()
         .addStarlarkBootstrap(new RepositoryBootstrap(new StarlarkRepositoryModule()));
     ConfiguredRuleClassProvider ruleClassProvider = builder.build();

src/test/java/com/google/devtools/build/lib/blackbox/tests/workspace/WorkspaceBlackBoxTest.java

@@ -265,7 +265,6 @@ public void testBadRepoName() throws Exception {
     context().write(WORKSPACE, "local_repository(name = '@a', path = 'abc')");
     context().write("BUILD");
     ProcessResult result = context().bazel().shouldFail().build("//...");
-    assertThat(result.errString())
-        .contains("Error in local_repository: invalid repository name '@a'");
+    assertThat(result.errString()).contains("invalid repository name '@a'");
   }
 }

src/test/java/com/google/devtools/build/lib/packages/BUILD

@@ -156,6 +156,7 @@ java_library(
         "//src/main/java/com/google/devtools/build/lib/analysis:analysis_cluster",
         "//src/main/java/com/google/devtools/build/lib/analysis:blaze_directories",
         "//src/main/java/com/google/devtools/build/lib/analysis:server_directories",
+        "//src/main/java/com/google/devtools/build/lib/bazel/bzlmod:resolution_impl",
         "//src/main/java/com/google/devtools/build/lib/bazel/rules/python",
         "//src/main/java/com/google/devtools/build/lib/clock",
         "//src/main/java/com/google/devtools/build/lib/cmdline",
@@ -167,6 +168,7 @@ java_library(
         "//src/main/java/com/google/devtools/build/lib/rules/proto",
         "//src/main/java/com/google/devtools/build/lib/rules/python",
         "//src/main/java/com/google/devtools/build/lib/skyframe:precomputed_value",
+        "//src/main/java/com/google/devtools/build/lib/skyframe:sky_functions",
         "//src/main/java/com/google/devtools/build/lib/skyframe:skyframe_cluster",
         "//src/main/java/com/google/devtools/build/lib/skyframe/packages:PackageFactoryBuilderWithSkyframeForTesting",
         "//src/main/java/com/google/devtools/build/lib/util",
@@ -176,6 +178,7 @@ java_library(
         "//src/main/java/com/google/devtools/build/lib/vfs",
         "//src/main/java/com/google/devtools/build/lib/vfs:pathfragment",
         "//src/main/java/com/google/devtools/common/options",
+        "//src/test/java/com/google/devtools/build/lib/bazel/bzlmod:util",
         "//src/test/java/com/google/devtools/build/lib/rules/python:PythonTestUtils",
         "//src/test/java/com/google/devtools/build/lib/testutil",
         "//src/test/java/com/google/devtools/build/lib/testutil:SkyframeExecutorTestHelper",

src/test/java/com/google/devtools/build/lib/packages/util/PackageLoadingTestCase.java

@@ -22,6 +22,8 @@
 import com.google.devtools.build.lib.analysis.ConfiguredRuleClassProvider;
 import com.google.devtools.build.lib.analysis.RuleDefinition;
 import com.google.devtools.build.lib.analysis.ServerDirectories;
+import com.google.devtools.build.lib.bazel.bzlmod.FakeRegistry;
+import com.google.devtools.build.lib.bazel.bzlmod.ModuleFileFunction;
 import com.google.devtools.build.lib.clock.BlazeClock;
 import com.google.devtools.build.lib.cmdline.Label;
 import com.google.devtools.build.lib.cmdline.LabelSyntaxException;
@@ -40,6 +42,7 @@
 import com.google.devtools.build.lib.runtime.QuiescingExecutorsImpl;
 import com.google.devtools.build.lib.skyframe.BazelSkyframeExecutorConstants;
 import com.google.devtools.build.lib.skyframe.PrecomputedValue;
+import com.google.devtools.build.lib.skyframe.SkyFunctions;
 import com.google.devtools.build.lib.skyframe.SkyframeExecutor;
 import com.google.devtools.build.lib.testutil.FoundationTestCase;
 import com.google.devtools.build.lib.testutil.SkyframeExecutorTestHelper;
@@ -60,8 +63,8 @@
 import org.junit.Before;
 
 /**
- * This is a specialization of {@link FoundationTestCase} that's useful for
- * implementing tests of the "packages" library.
+ * This is a specialization of {@link FoundationTestCase} that's useful for implementing tests of
+ * the "packages" library.
  */
 public abstract class PackageLoadingTestCase extends FoundationTestCase {
 
@@ -104,6 +107,14 @@ public final void initializeSkyframeExecutor() throws Exception {
     packageFactory =
         loadingMock
             .getPackageFactoryBuilderForTesting(directories)
+            .setExtraSkyFunctions(
+                ImmutableMap.of(
+                    SkyFunctions.MODULE_FILE,
+                    new ModuleFileFunction(
+                        ruleClassProvider.getBazelStarlarkEnvironment(),
+                        FakeRegistry.DEFAULT_FACTORY,
+                        directories.getWorkspace(),
+                        ImmutableMap.of())))
             .setPackageValidator(
                 (pkg, handler) -> {
                   // Delegate to late-bound this.validator.
@@ -212,8 +223,10 @@ protected void setBuildLanguageOptions(String... options) throws Exception {
   }
 
   protected Target getTarget(String label)
-      throws NoSuchPackageException, NoSuchTargetException,
-      LabelSyntaxException, InterruptedException {
+      throws NoSuchPackageException,
+          NoSuchTargetException,
+          LabelSyntaxException,
+          InterruptedException {
     return getTarget(Label.parseCanonical(label));
   }
 
@@ -258,8 +271,9 @@ protected static String genRule(String rule, String name, String... body) {
   }
 
   /**
-   * A utility function which generates the "deps" clause for a build file
-   * rule from a list of targets.
+   * A utility function which generates the "deps" clause for a build file rule from a list of
+   * targets.
+   *
    * @param depTargets the list of targets.
    * @return a string containing the deps clause
    */

src/test/java/com/google/devtools/build/lib/pkgcache/BUILD

@@ -104,6 +104,8 @@ java_library(
         "//src/main/java/com/google/devtools/build/lib/analysis:analysis_cluster",
         "//src/main/java/com/google/devtools/build/lib/analysis:blaze_directories",
         "//src/main/java/com/google/devtools/build/lib/analysis:server_directories",
+        "//src/main/java/com/google/devtools/build/lib/bazel/bzlmod:resolution_impl",
+        "//src/test/java/com/google/devtools/build/lib/bazel/bzlmod:util",
         "//src/main/java/com/google/devtools/build/lib/clock",
         "//src/main/java/com/google/devtools/build/lib/cmdline",
         "//src/main/java/com/google/devtools/build/lib/events",
@@ -113,6 +115,7 @@ java_library(
         "//src/main/java/com/google/devtools/build/lib/rules:repository/repository_function",
         "//src/main/java/com/google/devtools/build/lib/skyframe:diff_awareness",
         "//src/main/java/com/google/devtools/build/lib/skyframe:precomputed_value",
+        "//src/main/java/com/google/devtools/build/lib/skyframe:sky_functions",
         "//src/main/java/com/google/devtools/build/lib/skyframe:skyframe_cluster",
         "//src/main/java/com/google/devtools/build/lib/util:abrupt_exit_exception",
         "//src/main/java/com/google/devtools/build/lib/util/io",