Introduce --incompatible_use_python_toolchains

This renames --experimental_use_python_toolchains to --incompatible. It also adds the behavior to the flag that

  1) py_runtime's python_version attribute becomes manadatory, and

  2) the --python_top flag is disallowed (it is superseded by toolchains), as well as --python2_path and --python3_path (they were already no-ops). --python_path is strongly discouraged but not yet banned due to an existing use on Windows (#7901).

Feature issue is #7375, incompatible change migration issue is #7899.

RELNOTES[INC]: Introduced --incompatible_use_python_toolchains, which supersedes --python_top/--python_path. See #7899 and #7375 for more information.

PiperOrigin-RevId: 241134532

src/main/java/com/google/devtools/build/lib/bazel/rules/python/BazelPythonConfiguration.java

@@ -24,9 +24,10 @@
 import com.google.devtools.build.lib.analysis.config.InvalidConfigurationException;
 import com.google.devtools.build.lib.cmdline.Label;
 import com.google.devtools.build.lib.concurrent.ThreadSafety.Immutable;
-import com.google.devtools.build.lib.util.OS;
+import com.google.devtools.build.lib.events.Event;
+import com.google.devtools.build.lib.events.EventHandler;
+import com.google.devtools.build.lib.rules.python.PythonOptions;
 import com.google.devtools.build.lib.vfs.PathFragment;
-import com.google.devtools.common.options.Converter;
 import com.google.devtools.common.options.Option;
 import com.google.devtools.common.options.OptionDocumentationCategory;
 import com.google.devtools.common.options.OptionEffectTag;
@@ -35,77 +36,61 @@
 /** Bazel-specific Python configuration. */
 @Immutable
 public class BazelPythonConfiguration extends BuildConfiguration.Fragment {
-  /**
-  * A path converter for python3 path
-  */
-  public static class Python3PathConverter implements Converter<String> {
-    @Override
-    public String convert(String input) {
-      if (input.equals("auto")) {
-        return OS.getCurrent() == OS.WINDOWS ? "python" : "python3";
-      }
-      return input;
-    }
-
-    @Override
-    public String getTypeDescription() {
-      return "An option for python3 path";
-    }
-  }
 
   /** Bazel-specific Python configuration options. */
   public static final class Options extends FragmentOptions {
     @Option(
-      name = "python2_path",
-      defaultValue = "python",
-      documentationCategory = OptionDocumentationCategory.TOOLCHAIN,
-      effectTags = {OptionEffectTag.LOADING_AND_ANALYSIS, OptionEffectTag.AFFECTS_OUTPUTS},
-      metadataTags = {OptionMetadataTag.DEPRECATED},
-      help =
-          "Local path to the Python2 executable. "
-              + "Deprecated, please use python_path or python_top instead."
-    )
+        name = "python2_path",
+        defaultValue = "null",
+        documentationCategory = OptionDocumentationCategory.TOOLCHAIN,
+        effectTags = {OptionEffectTag.NO_OP},
+        metadataTags = {OptionMetadataTag.DEPRECATED},
+        help = "Deprecated, no-op. Disabled by `--incompatible_use_python_toolchains`.")
     public String python2Path;
 
     @Option(
-      name = "python3_path",
-      converter = Python3PathConverter.class,
-      defaultValue = "auto",
-      documentationCategory = OptionDocumentationCategory.TOOLCHAIN,
-      effectTags = {OptionEffectTag.LOADING_AND_ANALYSIS, OptionEffectTag.AFFECTS_OUTPUTS},
-      metadataTags = {OptionMetadataTag.DEPRECATED},
-      help =
-          "Local path to the Python3 executable. "
-              + "Deprecated, please use python_path or python_top instead."
-    )
+        name = "python3_path",
+        defaultValue = "null",
+        documentationCategory = OptionDocumentationCategory.TOOLCHAIN,
+        effectTags = {OptionEffectTag.NO_OP},
+        metadataTags = {OptionMetadataTag.DEPRECATED},
+        help = "Deprecated, no-op. Disabled by `--incompatible_use_python_toolchains`.")
     public String python3Path;
 
     @Option(
-      name = "python_top",
-      converter = LabelConverter.class,
-      defaultValue = "null",
-      documentationCategory = OptionDocumentationCategory.TOOLCHAIN,
-      effectTags = {OptionEffectTag.LOADING_AND_ANALYSIS, OptionEffectTag.AFFECTS_OUTPUTS},
-      help = "The label of py_runtime rule used for the Python interpreter invoked by Bazel."
-    )
+        name = "python_top",
+        converter = LabelConverter.class,
+        defaultValue = "null",
+        documentationCategory = OptionDocumentationCategory.TOOLCHAIN,
+        effectTags = {OptionEffectTag.LOADING_AND_ANALYSIS, OptionEffectTag.AFFECTS_OUTPUTS},
+        help =
+            "The label of a py_runtime representing the Python interpreter invoked to run Python "
+                + "targets on the target platform. Deprecated; disabled by "
+                + "--incompatible_use_python_toolchains.")
     public Label pythonTop;
 
     @Option(
-      name = "python_path",
-      defaultValue = "python",
-      documentationCategory = OptionDocumentationCategory.TOOLCHAIN,
-      effectTags = {OptionEffectTag.LOADING_AND_ANALYSIS, OptionEffectTag.AFFECTS_OUTPUTS},
-      help = "The absolute path of the Python interpreter invoked by Bazel."
-    )
+        name = "python_path",
+        defaultValue = "null",
+        documentationCategory = OptionDocumentationCategory.TOOLCHAIN,
+        effectTags = {OptionEffectTag.LOADING_AND_ANALYSIS, OptionEffectTag.AFFECTS_OUTPUTS},
+        help =
+            "The absolute path of the Python interpreter invoked to run Python targets on the "
+                + "target platform. Deprecated; disabled by --incompatible_use_python_toolchains.")
     public String pythonPath;
 
     @Option(
-      name = "experimental_python_import_all_repositories",
-      defaultValue = "true",
-      documentationCategory = OptionDocumentationCategory.UNDOCUMENTED,
-      effectTags = {OptionEffectTag.LOADING_AND_ANALYSIS},
-      help = "Do not use."
-    )
+        name = "experimental_python_import_all_repositories",
+        defaultValue = "true",
+        documentationCategory = OptionDocumentationCategory.UNDOCUMENTED,
+        effectTags = {OptionEffectTag.LOADING_AND_ANALYSIS},
+        help =
+            "If true, the roots of repositories in the runfiles tree are added to PYTHONPATH, so "
+                + "that imports like `import mytoplevelpackage.package.module` are valid."
+                + " Regardless of whether this flag is true, the runfiles root itself is also"
+                + " added to the PYTHONPATH, so "
+                + "`import myreponame.mytoplevelpackage.package.module` is valid. The latter form "
+                + "is less likely to experience import name collisions.")
     public boolean experimentalPythonImportAllRepositories;
 
      /**
@@ -143,7 +128,7 @@ public Class<? extends Fragment> creates() {
 
     @Override
     public ImmutableSet<Class<? extends FragmentOptions>> requiredOptions() {
-      return ImmutableSet.<Class<? extends FragmentOptions>>of(Options.class);
+      return ImmutableSet.of(Options.class, PythonOptions.class);
     }
   }
 
@@ -153,20 +138,43 @@ private BazelPythonConfiguration(Options options) {
     this.options = options;
   }
 
-  public String getPython2Path() {
-    return options.python2Path;
-  }
-
-  public String getPython3Path() {
-    return options.python3Path;
+  @Override
+  public void reportInvalidOptions(EventHandler reporter, BuildOptions buildOptions) {
+    PythonOptions pythonOpts = buildOptions.get(PythonOptions.class);
+    Options opts = buildOptions.get(Options.class);
+    if (pythonOpts.incompatibleUsePythonToolchains) {
+      // Forbid deprecated flags.
+      if (opts.python2Path != null) {
+        reporter.handle(
+            Event.error(
+                "`--python2_path` is disabled by `--incompatible_use_python_toolchains`. Since "
+                    + "`--python2_path` is a deprecated no-op, there is no need to pass it."));
+      }
+      if (opts.python3Path != null) {
+        reporter.handle(
+            Event.error(
+                "`--python3_path` is disabled by `--incompatible_use_python_toolchains`. Since "
+                    + "`--python3_path` is a deprecated no-op, there is no need to pass it."));
+      }
+      if (opts.pythonTop != null) {
+        reporter.handle(
+            Event.error(
+                "`--python_top` is disabled by `--incompatible_use_python_toolchains`. Instead of "
+                    + "configuring the Python runtime directly, register a Python toolchain. See "
+                    + "https://github.com/bazelbuild/bazel/issues/7899. You can temporarily revert "
+                    + "to the legacy flag-based way of specifying toolchains by setting "
+                    + "`--incompatible_use_python_toolchains=false`."));
+      }
+      // TODO(#7901): Also prohibit --python_path here.
+    }
   }
 
   public Label getPythonTop() {
     return options.pythonTop;
   }
 
   public String getPythonPath() {
-    return options.pythonPath;
+    return options.pythonPath == null ? "python" : options.pythonPath;
   }
 
   public boolean getImportAllRepositories() {

src/main/java/com/google/devtools/build/lib/rules/python/PyRuntime.java

@@ -33,6 +33,8 @@ public final class PyRuntime implements RuleConfiguredTargetFactory {
 
   @Override
   public ConfiguredTarget create(RuleContext ruleContext) throws ActionConflictException {
+    PythonConfiguration pyConfig = ruleContext.getFragment(PythonConfiguration.class);
+
     NestedSet<Artifact> files =
         PrerequisiteArtifacts.nestedSet(ruleContext, "files", Mode.TARGET);
     Artifact interpreter = ruleContext.getPrerequisiteArtifact("interpreter", Mode.TARGET);
@@ -58,15 +60,24 @@ public ConfiguredTarget create(RuleContext ruleContext) throws ActionConflictExc
     }
 
     if (pythonVersion == PythonVersion._INTERNAL_SENTINEL) {
-      // Use the same default as py_binary/py_test would use for their python_version attribute.
-      // (Of course, in our case there's no configuration transition involved.)
-      pythonVersion = ruleContext.getFragment(PythonConfiguration.class).getDefaultPythonVersion();
+      if (pyConfig.useToolchains()) {
+        ruleContext.attributeError(
+            "python_version",
+            "When using Python toolchains, this attribute must be set explicitly to either 'PY2' "
+                + "or 'PY3'. See https://github.com/bazelbuild/bazel/issues/7899 for more "
+                + "information. You can temporarily avoid this error by reverting to the legacy "
+                + "Python runtime mechanism (`--incompatible_use_python_toolchains=false`).");
+      } else {
+        // Use the same default as py_binary/py_test would use for their python_version attribute.
+        // (Of course, in our case there's no configuration transition involved.)
+        pythonVersion = pyConfig.getDefaultPythonVersion();
+      }
     }
-    Preconditions.checkState(pythonVersion.isTargetValue());
 
     if (ruleContext.hasErrors()) {
       return null;
     }
+    Preconditions.checkState(pythonVersion.isTargetValue());
 
     PyRuntimeInfo provider =
         hermetic

src/main/java/com/google/devtools/build/lib/rules/python/PythonOptions.java

@@ -230,11 +230,14 @@ public String getTypeDescription() {
   public boolean incompatibleDisallowLegacyPyProvider;
 
   @Option(
-      // TODO(brandjon): Rename to --incompatible_use_python_toolchains when ready to make available
-      name = "experimental_use_python_toolchains",
+      name = "incompatible_use_python_toolchains",
       defaultValue = "false",
       documentationCategory = OptionDocumentationCategory.GENERIC_INPUTS,
       effectTags = {OptionEffectTag.LOADING_AND_ANALYSIS},
+      metadataTags = {
+        OptionMetadataTag.INCOMPATIBLE_CHANGE,
+        OptionMetadataTag.TRIGGERED_BY_ALL_INCOMPATIBLE_CHANGES
+      },
       help =
           "If set to true, executable native Python rules will use the Python runtime specified by "
               + "the Python toolchain, rather than the runtime given by legacy flags like "

src/test/java/com/google/devtools/build/lib/bazel/rules/python/BazelPyBinaryConfiguredTargetTest.java

@@ -94,7 +94,7 @@ public void runtimeSetByPythonTop() throws Exception {
         ")");
     String pythonTop =
         analysisMock.pySupport().createPythonTopEntryPoint(mockToolsConfig, "//pkg:my_py_runtime");
-    useConfiguration("--experimental_use_python_toolchains=false", "--python_top=" + pythonTop);
+    useConfiguration("--incompatible_use_python_toolchains=false", "--python_top=" + pythonTop);
     String path = getInterpreterPathFromStub(getConfiguredTarget("//pkg:pybin"));
     assertThat(path).isEqualTo("/system/python2");
   }
@@ -107,7 +107,7 @@ public void runtimeSetByPythonPath() throws Exception {
         "    name = 'pybin',",
         "    srcs = ['pybin.py'],",
         ")");
-    useConfiguration("--experimental_use_python_toolchains=false", "--python_path=/system/python2");
+    useConfiguration("--incompatible_use_python_toolchains=false", "--python_path=/system/python2");
     String path = getInterpreterPathFromStub(getConfiguredTarget("//pkg:pybin"));
     assertThat(path).isEqualTo("/system/python2");
   }
@@ -120,7 +120,7 @@ public void runtimeDefaultsToPythonSystemCommand() throws Exception {
         "    name = 'pybin',",
         "    srcs = ['pybin.py'],",
         ")");
-    useConfiguration("--experimental_use_python_toolchains=false");
+    useConfiguration("--incompatible_use_python_toolchains=false");
     String path = getInterpreterPathFromStub(getConfiguredTarget("//pkg:pybin"));
     assertThat(path).isEqualTo("python");
   }
@@ -141,7 +141,7 @@ public void pythonTopTakesPrecedenceOverPythonPath() throws Exception {
     String pythonTop =
         analysisMock.pySupport().createPythonTopEntryPoint(mockToolsConfig, "//pkg:my_py_runtime");
     useConfiguration(
-        "--experimental_use_python_toolchains=false",
+        "--incompatible_use_python_toolchains=false",
         "--python_top=" + pythonTop,
         "--python_path=/better/not/be/this/one");
     String path = getInterpreterPathFromStub(getConfiguredTarget("//pkg:pybin"));
@@ -205,7 +205,7 @@ public void runtimeObtainedFromToolchain() throws Exception {
         "    python_version = 'PY3',",
         ")");
     useConfiguration(
-        "--experimental_use_python_toolchains=true",
+        "--incompatible_use_python_toolchains=true",
         "--extra_toolchains=//toolchains:py_toolchain");
 
     String py2Path = getInterpreterPathFromStub(getConfiguredTarget("//pkg:py2_bin"));
@@ -225,7 +225,7 @@ public void toolchainCanOmitUnusedRuntimeVersion() throws Exception {
         "    python_version = 'PY2',",
         ")");
     useConfiguration(
-        "--experimental_use_python_toolchains=true",
+        "--incompatible_use_python_toolchains=true",
         "--extra_toolchains=//toolchains:py_toolchain_for_py2_only");
 
     String path = getInterpreterPathFromStub(getConfiguredTarget("//pkg:py2_bin"));
@@ -237,24 +237,14 @@ public void toolchainTakesPrecedenceOverLegacyFlags() throws Exception {
     defineToolchains();
     scratch.file(
         "pkg/BUILD",
-        "py_runtime(",
-        "    name = 'dont_use_this_runtime',",
-        "    interpreter_path = '/dont/use/me',",
-        "    python_version = 'PY2',",
-        ")",
         "py_binary(",
         "    name = 'py2_bin',",
         "    srcs = ['py2_bin.py'],",
         "    python_version = 'PY2',",
         ")");
-    String pythonTop =
-        analysisMock
-            .pySupport()
-            .createPythonTopEntryPoint(mockToolsConfig, "//pkg:dont_use_this_runtime");
     useConfiguration(
-        "--experimental_use_python_toolchains=true",
+        "--incompatible_use_python_toolchains=true",
         "--extra_toolchains=//toolchains:py_toolchain",
-        "--python_top=" + pythonTop,
         "--python_path=/better/not/be/this/one");
 
     String path = getInterpreterPathFromStub(getConfiguredTarget("//pkg:py2_bin"));
@@ -273,7 +263,7 @@ public void toolchainIsMissingNeededRuntime() throws Exception {
         "    python_version = 'PY3',",
         ")");
     useConfiguration(
-        "--experimental_use_python_toolchains=true",
+        "--incompatible_use_python_toolchains=true",
         "--extra_toolchains=//toolchains:py_toolchain_for_py2_only");
 
     getConfiguredTarget("//pkg:py3_bin");
@@ -324,7 +314,7 @@ private void analyzePy2BinaryTargetUsingCustomToolchain() throws Exception {
         "    python_version = 'PY2',",
         ")");
     useConfiguration(
-        "--experimental_use_python_toolchains=true",
+        "--incompatible_use_python_toolchains=true",
         "--extra_toolchains=//toolchains:custom_toolchain");
     getConfiguredTarget("//pkg:pybin");
   }

src/test/java/com/google/devtools/build/lib/bazel/rules/python/BazelPythonConfigurationTest.java

@@ -61,4 +61,23 @@ public void pythonPathMustBeAbsolute() {
         () -> create("--python_path=pajama"));
     assertThat(expected).hasMessageThat().contains("python_path must be an absolute path");
   }
+
+  @Test
+  public void legacyFlagsDeprecatedByPythonToolchains() throws Exception {
+    checkError(
+        "`--python2_path` is disabled by `--incompatible_use_python_toolchains`",
+        "--incompatible_use_python_toolchains=true",
+        "--python2_path=/system/python2");
+    checkError(
+        "`--python3_path` is disabled by `--incompatible_use_python_toolchains`",
+        "--incompatible_use_python_toolchains=true",
+        "--python3_path=/system/python3");
+    checkError(
+        "`--python_top` is disabled by `--incompatible_use_python_toolchains`",
+        "--incompatible_use_python_toolchains=true",
+        "--python_top=//mypkg:my_py_runtime");
+    // TODO(#7901): Also test that --python_path is disallowed (once implemented). Currently we
+    // still need it to communicate the location of python.exe on Windows from the client to the
+    // server.
+  }
 }

src/test/java/com/google/devtools/build/lib/rules/python/PyRuntimeConfiguredTargetTest.java

@@ -73,6 +73,8 @@ public void nonhermeticRuntime() throws Exception {
   @Test
   public void pythonVersionDefault() throws Exception {
     ensureDefaultIsPY2();
+    // When using toolchains, the python_version attribute is mandatory.
+    useConfiguration("--incompatible_use_python_toolchains=false");
     scratch.file(
         "pkg/BUILD",
         "py_runtime(",
@@ -170,4 +172,19 @@ public void badPythonVersionAttribute() throws Exception {
 
     assertContainsEvent("invalid value in 'python_version' attribute");
   }
+
+  @Test
+  public void versionAttributeMandatoryWhenUsingToolchains() throws Exception {
+    reporter.removeHandler(failFastHandler);
+    useConfiguration("--incompatible_use_python_toolchains=true");
+    scratch.file(
+        "pkg/BUILD",
+        "py_runtime(",
+        "    name = 'myruntime',",
+        "    interpreter_path = '/system/interpreter',",
+        ")");
+    getConfiguredTarget("//pkg:myruntime");
+
+    assertContainsEvent("must be set explicitly to either 'PY2' or 'PY3'");
+  }
 }