Enable Python toolchains by default

This flips --incompatible_use_python_toolchains, which deprecates --python_top (and for the most part, --python_path). See [#7899](#7899) for more on the change and migration procedure.

RELNOTES[INC]: Python rules now determine the Python runtime using toolchains rather than `--python_top` and `--python_path`, which are deprecated. See [#7899](#7899) for information on declaring Python toolchains and migrating your code. As a side-benefit, this addresses #4815 (incorrect interpreter version used) on non-Windows platforms. You can temporarily opt out of this change with `--incompatible_use_python_toolchains=false`.

Fixes #7899, fixes #7375, significant progress on #4815.

PiperOrigin-RevId: 246219480

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

@@ -231,7 +231,7 @@ public String getTypeDescription() {
 
   @Option(
       name = "incompatible_use_python_toolchains",
-      defaultValue = "false",
+      defaultValue = "true",
       documentationCategory = OptionDocumentationCategory.GENERIC_INPUTS,
       effectTags = {OptionEffectTag.LOADING_AND_ANALYSIS},
       metadataTags = {

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

@@ -79,6 +79,8 @@ private String getInterpreterPathFromStub(ConfiguredTarget pyExecutableTarget) {
         "Failed to find the '%python_binary%' key in the stub script's template expansion action");
   }
 
+  // TODO(#8169): Delete tests of the legacy --python_top / --python_path behavior.
+
   @Test
   public void runtimeSetByPythonTop() throws Exception {
     scratch.file(

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

@@ -33,8 +33,9 @@ public void pythonTop() throws Exception {
     scratch.file(
         "a/BUILD",
         "py_runtime(name='b', files=[], interpreter='c')");
-    BazelPythonConfiguration config = create("--python_top=//a:b")
-        .getFragment(BazelPythonConfiguration.class);
+    BazelPythonConfiguration config =
+        create("--incompatible_use_python_toolchains=false", "--python_top=//a:b")
+            .getFragment(BazelPythonConfiguration.class);
     assertThat(config.getPythonTop()).isNotNull();
   }
 
@@ -43,7 +44,7 @@ public void pythonTop_malformedLabel() {
     OptionsParsingException expected =
         assertThrows(
             OptionsParsingException.class,
-            () -> create("--python_top=//a:!b:"));
+            () -> create("--incompatible_use_python_toolchains=false", "--python_top=//a:!b:"));
     assertThat(expected).hasMessageThat().contains("While parsing option --python_top");
   }
 

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

@@ -54,10 +54,24 @@ public void setup(MockToolsConfig config) throws IOException {
         "toolchain_type(name = 'toolchain_type')",
         "constraint_setting(name = 'py2_interpreter_path')",
         "constraint_setting(name = 'py3_interpreter_path')",
-        "py_runtime_pair(name = 'dummy_py_runtime_pair')",
+        "py_runtime(",
+        "    name = 'py2_interpreter',",
+        "    interpreter_path = '/usr/bin/mockpython2',",
+        "    python_version = 'PY2',",
+        ")",
+        "py_runtime(",
+        "    name = 'py3_interpreter',",
+        "    interpreter_path = '/usr/bin/mockpython3',",
+        "    python_version = 'PY3',",
+        ")",
+        "py_runtime_pair(",
+        "    name = 'default_py_runtime_pair',",
+        "    py2_runtime = ':py2_interpreter',",
+        "    py3_runtime = ':py3_interpreter',",
+        ")",
         "toolchain(",
-        "    name = 'dummy_toolchain',",
-        "    toolchain = ':dummy_py_runtime_pair',",
+        "    name = 'default_python_toolchain',",
+        "    toolchain = ':default_py_runtime_pair',",
         "    toolchain_type = ':toolchain_type',",
         ")",
         "exports_files(['precompile.py'])",

src/test/java/com/google/devtools/build/lib/rules/python/BUILD

@@ -254,6 +254,7 @@ java_test(
         "//src/main/java/com/google/devtools/build/lib:python-rules",
         "//src/main/java/com/google/devtools/build/lib/actions",
         "//src/test/java/com/google/devtools/build/lib:analysis_testutil",
+        "//src/test/java/com/google/devtools/build/lib:testutil",
         "//third_party:junit4",
         "//third_party:truth",
     ],

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

@@ -20,6 +20,7 @@
 import com.google.devtools.build.lib.analysis.ConfiguredTarget;
 import com.google.devtools.build.lib.analysis.util.BuildViewTestCase;
 import com.google.devtools.build.lib.packages.StructImpl;
+import com.google.devtools.build.lib.testutil.TestConstants;
 import org.junit.Test;
 import org.junit.runner.RunWith;
 import org.junit.runners.JUnit4;
@@ -174,6 +175,10 @@ public void runtimeSandwich() throws Exception {
         ")");
     scratch.file(
         "pkg/BUILD",
+        "load('"
+            + TestConstants.TOOLS_REPOSITORY
+            + "//tools/python:toolchain.bzl', "
+            + "'py_runtime_pair')",
         "load(':rules.bzl', 'userruntime')",
         "py_runtime(",
         "    name = 'pyruntime',",
@@ -187,13 +192,22 @@ public void runtimeSandwich() throws Exception {
         "    interpreter = ':userintr',",
         "    files = ['userdata.txt'],",
         ")",
+        "py_runtime_pair(",
+        "    name = 'userruntime_pair',",
+        "    py2_runtime = 'userruntime',",
+        ")",
+        "toolchain(",
+        "    name = 'usertoolchain',",
+        "    toolchain = ':userruntime_pair',",
+        "    toolchain_type = '"
+            + TestConstants.TOOLS_REPOSITORY
+            + "//tools/python:toolchain_type',",
+        ")",
         "py_binary(",
         "    name = 'pybin',",
         "    srcs = ['pybin.py'],",
         ")");
-    String pythonTopLabel =
-        analysisMock.pySupport().createPythonTopEntryPoint(mockToolsConfig, "//pkg:userruntime");
-    useConfiguration("--python_top=" + pythonTopLabel);
+    useConfiguration("--extra_toolchains=//pkg:usertoolchain");
     ConfiguredTarget target = getConfiguredTarget("//pkg:pybin");
     assertThat(collectRunfiles(target))
         .containsAtLeast(getSourceArtifact("pkg/data.txt"), getSourceArtifact("pkg/userdata.txt"));

src/test/shell/bazel/android/android_instrumentation_test_integration_test.sh

@@ -29,6 +29,11 @@ fail_if_no_android_sdk
 source "${CURRENT_DIR}/../../integration_test_setup.sh" \
   || { echo "integration_test_setup.sh not found!" >&2; exit 1; }
 
+# TODO(#8169): Make this test compatible with Python toolchains. Blocked on the
+# fact that there's no PY3 environment on our Mac workers
+# (bazelbuild/continuous-integration#578).
+add_to_bazelrc "build --incompatible_use_python_toolchains=false"
+
 function setup_android_instrumentation_test_env() {
   mkdir -p java/com/bin/res/values
   mkdir -p javatests/com/bin

src/test/shell/bazel/python_version_test.sh

@@ -43,9 +43,6 @@ fi
 source "$(rlocation "io_bazel/src/test/shell/integration_test_setup.sh")" \
   || { echo "integration_test_setup.sh not found!" >&2; exit 1; }
 
-# TODO(bazelbuild/continuous-integration#578): Enable this test for Mac and
-# Windows.
-
 # `uname` returns the current platform, e.g "MSYS_NT-10.0" or "Linux".
 # `tr` converts all upper case letters to lower case.
 # `case` matches the result if the `uname | tr` expression to string prefixes
@@ -55,15 +52,15 @@ case "$(uname -s | tr [:upper:] [:lower:])" in
 msys*)
   # As of 2018-08-14, Bazel on Windows only supports MSYS Bash.
   declare -r is_windows=true
-  # As of 2018-12-17, this test is disabled on windows (via "no_windows" tag),
-  # so this code shouldn't even have run. See the TODO at
-  # use_system_python_2_3_runtimes.
+  # As of 2019-04-26, this test is disabled on Windows (via "no_windows" tag),
+  # so this code shouldn't even have run.
+  # TODO(bazelbuild/continuous-integration#578): Enable this test for Windows.
   fail "This test does not run on Windows."
   ;;
 darwin*)
-  # As of 2018-12-17, this test is disabled on mac, but there's no "no_mac" tag
-  # so we just have to trivially succeed. See the TODO at
-  # use_system_python_2_3_runtimes.
+  # As of 2019-04-26, this test is disabled on mac, but there's no "no_mac" tag
+  # so we just have to trivially succeed.
+  # TODO(bazelbuild/continuous-integration#578): Enable this test for Mac.
   echo "This test does not run on Mac; exiting early." >&2
   exit 0
   ;;
@@ -79,44 +76,6 @@ if "$is_windows"; then
   export MSYS2_ARG_CONV_EXCL="*"
 fi
 
-# Use a py_runtime that invokes either the system's Python 2 or Python 3
-# interpreter based on the Python mode. On Unix this is a workaround for #4815.
-#
-# TODO(brandjon): Replace this with the autodetecting Python toolchain.
-function use_system_python_2_3_runtimes() {
-  PYTHON2_BIN=$(which python2 || echo "")
-  PYTHON3_BIN=$(which python3 || echo "")
-  # Debug output.
-  echo "Python 2 interpreter: ${PYTHON2_BIN:-"Not found"}"
-  echo "Python 3 interpreter: ${PYTHON3_BIN:-"Not found"}"
-  # Fail if either isn't present.
-  if [[ -z "${PYTHON2_BIN:-}" || -z "${PYTHON3_BIN:-}" ]]; then
-    fail "Can't use system interpreter: Could not find one or both of \
-'python2', 'python3'"
-  fi
-
-  # Point Python builds at a py_runtime target defined in a //tools package of
-  # the main repo. This is not related to @bazel_tools//tools/python.
-  add_to_bazelrc "build --python_top=//tools/python:default_runtime"
-
-  mkdir -p tools/python
-
-  cat > tools/python/BUILD << EOF
-package(default_visibility=["//visibility:public"])
-
-py_runtime(
-    name = "default_runtime",
-    files = [],
-    interpreter_path = select({
-        "@bazel_tools//tools/python:PY2": "${PYTHON2_BIN}",
-        "@bazel_tools//tools/python:PY3": "${PYTHON3_BIN}",
-    }),
-)
-EOF
-}
-
-use_system_python_2_3_runtimes
-
 #### TESTS #############################################################
 
 # Sanity test that our environment setup above works.
@@ -202,6 +161,8 @@ function test_build_python_zip_works_with_py_runtime() {
   mkdir -p test
 
   cat > test/BUILD << EOF
+load("@bazel_tools//tools/python:toolchain.bzl", "py_runtime_pair")
+
 py_binary(
     name = "pybin",
     srcs = ["pybin.py"],
@@ -212,6 +173,17 @@ py_runtime(
     interpreter = ":mockpy.sh",
     python_version = "PY3",
 )
+
+py_runtime_pair(
+    name = "mock_runtime_pair",
+    py3_runtime = ":mock_runtime",
+)
+
+toolchain(
+    name = "mock_toolchain",
+    toolchain = ":mock_runtime_pair",
+    toolchain_type = "@bazel_tools//tools/python:toolchain_type",
+)
 EOF
   cat > test/pybin.py << EOF
 # This doesn't actually run because we use a mock Python runtime that never
@@ -224,7 +196,8 @@ echo "I am mockpy!"
 EOF
   chmod u+x test/mockpy.sh
 
-  bazel run //test:pybin --python_top=//test:mock_runtime --build_python_zip \
+  bazel run //test:pybin \
+      --extra_toolchains=//test:mock_toolchain --build_python_zip \
       &> $TEST_log || fail "bazel run failed"
   expect_log "I am mockpy!"
 }

src/test/shell/integration/runfiles_test.sh

@@ -51,14 +51,18 @@ msys*|mingw*|cygwin*)
   ;;
 esac
 
+# We disable Python toolchains in EXTRA_BUILD_FLAGS because it throws off the
+# counts and manifest checks in test_foo_runfiles.
+# TODO(#8169): Update this test and remove the toolchain opt-out.
 if "$is_windows"; then
   export MSYS_NO_PATHCONV=1
   export MSYS2_ARG_CONV_EXCL="*"
   export EXT=".exe"
-  export EXTRA_BUILD_FLAGS="--enable_runfiles --build_python_zip=0"
+  export EXTRA_BUILD_FLAGS="--incompatible_use_python_toolchains=false \
+--enable_runfiles --build_python_zip=0"
 else
   export EXT=""
-  export EXTRA_BUILD_FLAGS=""
+  export EXTRA_BUILD_FLAGS="--incompatible_use_python_toolchains=false"
 fi
 
 #### SETUP #############################################################

src/test/shell/testenv.sh

@@ -569,11 +569,13 @@ function use_fake_python_runtimes_for_testsuite() {
     PYTHON3_FILENAME="python3.sh"
   fi
 
-  add_to_bazelrc "build --python_top=//tools/python:default_runtime"
+  add_to_bazelrc "build --extra_toolchains=//tools/python:fake_python_toolchain"
 
   mkdir -p tools/python
 
   cat > tools/python/BUILD << EOF
+load("@bazel_tools//tools/python:toolchain.bzl", "py_runtime_pair")
+
 package(default_visibility=["//visibility:public"])
 
 sh_binary(
@@ -582,15 +584,27 @@ sh_binary(
 )
 
 py_runtime(
-    name = "default_runtime",
-    files = select({
-        "@bazel_tools//tools/python:PY3": [":${PYTHON3_FILENAME}"],
-        "//conditions:default": [":${PYTHON2_FILENAME}"],
-    }),
-    interpreter = select({
-        "@bazel_tools//tools/python:PY3": ":${PYTHON3_FILENAME}",
-        "//conditions:default": ":${PYTHON2_FILENAME}",
-    }),
+    name = "fake_py2_interpreter",
+    interpreter = ":${PYTHON2_FILENAME}",
+    python_version = "PY2",
+)
+
+py_runtime(
+    name = "fake_py3_interpreter",
+    interpreter = ":${PYTHON3_FILENAME}",
+    python_version = "PY3",
+)
+
+py_runtime_pair(
+    name = "fake_py_runtime_pair",
+    py2_runtime = ":fake_py2_interpreter",
+    py3_runtime = ":fake_py3_interpreter",
+)
+
+toolchain(
+    name = "fake_python_toolchain",
+    toolchain = ":fake_py_runtime_pair",
+    toolchain_type = "@bazel_tools//tools/python:toolchain_type",
 )
 EOF