diff --git a/src/main/java/com/google/devtools/build/lib/rules/python/PythonOptions.java b/src/main/java/com/google/devtools/build/lib/rules/python/PythonOptions.java index f69cd517b94600..e14b36f76af2bb 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/python/PythonOptions.java +++ b/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 = "true", + defaultValue = "false", documentationCategory = OptionDocumentationCategory.GENERIC_INPUTS, effectTags = {OptionEffectTag.LOADING_AND_ANALYSIS}, metadataTags = { diff --git a/src/test/java/com/google/devtools/build/lib/bazel/rules/python/BazelPyBinaryConfiguredTargetTest.java b/src/test/java/com/google/devtools/build/lib/bazel/rules/python/BazelPyBinaryConfiguredTargetTest.java index 7fbe0e1982ac0e..a878000b0a28bb 100644 --- a/src/test/java/com/google/devtools/build/lib/bazel/rules/python/BazelPyBinaryConfiguredTargetTest.java +++ b/src/test/java/com/google/devtools/build/lib/bazel/rules/python/BazelPyBinaryConfiguredTargetTest.java @@ -79,8 +79,6 @@ 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( diff --git a/src/test/java/com/google/devtools/build/lib/bazel/rules/python/BazelPythonConfigurationTest.java b/src/test/java/com/google/devtools/build/lib/bazel/rules/python/BazelPythonConfigurationTest.java index 6849c39c902f3c..5b5eb2dfa72651 100644 --- a/src/test/java/com/google/devtools/build/lib/bazel/rules/python/BazelPythonConfigurationTest.java +++ b/src/test/java/com/google/devtools/build/lib/bazel/rules/python/BazelPythonConfigurationTest.java @@ -33,9 +33,8 @@ public void pythonTop() throws Exception { scratch.file( "a/BUILD", "py_runtime(name='b', files=[], interpreter='c')"); - BazelPythonConfiguration config = - create("--incompatible_use_python_toolchains=false", "--python_top=//a:b") - .getFragment(BazelPythonConfiguration.class); + BazelPythonConfiguration config = create("--python_top=//a:b") + .getFragment(BazelPythonConfiguration.class); assertThat(config.getPythonTop()).isNotNull(); } @@ -44,7 +43,7 @@ public void pythonTop_malformedLabel() { OptionsParsingException expected = assertThrows( OptionsParsingException.class, - () -> create("--incompatible_use_python_toolchains=false", "--python_top=//a:!b:")); + () -> create("--python_top=//a:!b:")); assertThat(expected).hasMessageThat().contains("While parsing option --python_top"); } diff --git a/src/test/java/com/google/devtools/build/lib/packages/util/BazelMockPythonSupport.java b/src/test/java/com/google/devtools/build/lib/packages/util/BazelMockPythonSupport.java index 3a39c71fdf4a80..ae665ffc6871f1 100644 --- a/src/test/java/com/google/devtools/build/lib/packages/util/BazelMockPythonSupport.java +++ b/src/test/java/com/google/devtools/build/lib/packages/util/BazelMockPythonSupport.java @@ -54,24 +54,10 @@ 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(", - " 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',", - ")", + "py_runtime_pair(name = 'dummy_py_runtime_pair')", "toolchain(", - " name = 'default_python_toolchain',", - " toolchain = ':default_py_runtime_pair',", + " name = 'dummy_toolchain',", + " toolchain = ':dummy_py_runtime_pair',", " toolchain_type = ':toolchain_type',", ")", "exports_files(['precompile.py'])", diff --git a/src/test/java/com/google/devtools/build/lib/rules/python/BUILD b/src/test/java/com/google/devtools/build/lib/rules/python/BUILD index caabb0862665b1..2e81f708b04f05 100644 --- a/src/test/java/com/google/devtools/build/lib/rules/python/BUILD +++ b/src/test/java/com/google/devtools/build/lib/rules/python/BUILD @@ -254,7 +254,6 @@ 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", ], diff --git a/src/test/java/com/google/devtools/build/lib/rules/python/PythonStarlarkApiTest.java b/src/test/java/com/google/devtools/build/lib/rules/python/PythonStarlarkApiTest.java index 28c4ffe73dff38..a46772335c587e 100644 --- a/src/test/java/com/google/devtools/build/lib/rules/python/PythonStarlarkApiTest.java +++ b/src/test/java/com/google/devtools/build/lib/rules/python/PythonStarlarkApiTest.java @@ -20,7 +20,6 @@ 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; @@ -175,10 +174,6 @@ 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',", @@ -192,22 +187,13 @@ 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'],", ")"); - useConfiguration("--extra_toolchains=//pkg:usertoolchain"); + String pythonTopLabel = + analysisMock.pySupport().createPythonTopEntryPoint(mockToolsConfig, "//pkg:userruntime"); + useConfiguration("--python_top=" + pythonTopLabel); ConfiguredTarget target = getConfiguredTarget("//pkg:pybin"); assertThat(collectRunfiles(target)) .containsAtLeast(getSourceArtifact("pkg/data.txt"), getSourceArtifact("pkg/userdata.txt")); diff --git a/src/test/shell/bazel/android/android_instrumentation_test_integration_test.sh b/src/test/shell/bazel/android/android_instrumentation_test_integration_test.sh index 2239ad46683a30..36f76b17fffa8d 100755 --- a/src/test/shell/bazel/android/android_instrumentation_test_integration_test.sh +++ b/src/test/shell/bazel/android/android_instrumentation_test_integration_test.sh @@ -29,11 +29,6 @@ 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 diff --git a/src/test/shell/bazel/python_version_test.sh b/src/test/shell/bazel/python_version_test.sh index e1e1f27ce20601..e463f4d9531576 100755 --- a/src/test/shell/bazel/python_version_test.sh +++ b/src/test/shell/bazel/python_version_test.sh @@ -43,6 +43,9 @@ 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 @@ -52,15 +55,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 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. + # 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. fail "This test does not run on Windows." ;; darwin*) - # 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. + # 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. echo "This test does not run on Mac; exiting early." >&2 exit 0 ;; @@ -76,6 +79,44 @@ 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. @@ -161,8 +202,6 @@ 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"], @@ -173,17 +212,6 @@ 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 @@ -196,8 +224,7 @@ echo "I am mockpy!" EOF chmod u+x test/mockpy.sh - bazel run //test:pybin \ - --extra_toolchains=//test:mock_toolchain --build_python_zip \ + bazel run //test:pybin --python_top=//test:mock_runtime --build_python_zip \ &> $TEST_log || fail "bazel run failed" expect_log "I am mockpy!" } diff --git a/src/test/shell/integration/runfiles_test.sh b/src/test/shell/integration/runfiles_test.sh index 6e6f68bc8dc6d0..b089adfb4acf3a 100755 --- a/src/test/shell/integration/runfiles_test.sh +++ b/src/test/shell/integration/runfiles_test.sh @@ -51,18 +51,14 @@ 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="--incompatible_use_python_toolchains=false \ ---enable_runfiles --build_python_zip=0" + export EXTRA_BUILD_FLAGS="--enable_runfiles --build_python_zip=0" else export EXT="" - export EXTRA_BUILD_FLAGS="--incompatible_use_python_toolchains=false" + export EXTRA_BUILD_FLAGS="" fi #### SETUP ############################################################# diff --git a/src/test/shell/testenv.sh b/src/test/shell/testenv.sh index 999e7321b2eee8..a3717f263514df 100755 --- a/src/test/shell/testenv.sh +++ b/src/test/shell/testenv.sh @@ -569,13 +569,11 @@ function use_fake_python_runtimes_for_testsuite() { PYTHON3_FILENAME="python3.sh" fi - add_to_bazelrc "build --extra_toolchains=//tools/python:fake_python_toolchain" + add_to_bazelrc "build --python_top=//tools/python:default_runtime" 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( @@ -584,27 +582,15 @@ sh_binary( ) py_runtime( - 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", + 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}", + }), ) EOF