From 50fa3ec27efdd95771c70faa38a4543d4fed44f2 Mon Sep 17 00:00:00 2001 From: brandjon Date: Thu, 6 Jun 2019 15:15:39 -0700 Subject: [PATCH] Fix problems with the non-strict Python toolchain 1. Change the Python workspace suffix from registering all toolchains (via `:all`) in the @bazel_tools//tools/python package, to just registering the normal autodetecting toolchain. This ensures that the non-strict toolchain never inadvertently gets higher precedence than the standard toolchain. (This may have been working before by chance, but let's be safe and not rely on target pattern expansion order.) A side-effect of this is that we need to make our analysis and shell integration tests use the standard toolchain's name. 2. Fix bad quoting that caused the suggested command line flag to not be printed 3. Now that the wrapper is shared, there's two possible names for the toolchain, so output the right one in error messages. Fixes #8576, follow-up to #8547. RELNOTES: None PiperOrigin-RevId: 251937305 --- .../lib/bazel/rules/python/python.WORKSPACE | 8 +++++- .../lib/analysis/mock/BazelAnalysisMock.java | 2 +- .../packages/util/BazelMockPythonSupport.java | 3 ++- tools/python/pywrapper_template.txt | 25 ++++++++++++------- 4 files changed, 26 insertions(+), 12 deletions(-) diff --git a/src/main/java/com/google/devtools/build/lib/bazel/rules/python/python.WORKSPACE b/src/main/java/com/google/devtools/build/lib/bazel/rules/python/python.WORKSPACE index baf03b5e2b4809..2691ea4b68f6f6 100644 --- a/src/main/java/com/google/devtools/build/lib/bazel/rules/python/python.WORKSPACE +++ b/src/main/java/com/google/devtools/build/lib/bazel/rules/python/python.WORKSPACE @@ -1 +1,7 @@ -register_toolchains("@bazel_tools//tools/python:all") +# Don't use the target pattern :all, because we don't want to register +# :autodetecting_toolchain_nonstrict. That toolchain opts out of Python version +# checking, so the user should have to explicitly declare it on their command +# line or WORKSPACE file. (And even if we did register it here, we still +# couldn't use :all because we'd want to ensure it has the lowest possible +# priority.) +register_toolchains("@bazel_tools//tools/python:autodetecting_toolchain") diff --git a/src/test/java/com/google/devtools/build/lib/analysis/mock/BazelAnalysisMock.java b/src/test/java/com/google/devtools/build/lib/analysis/mock/BazelAnalysisMock.java index a29911ce2b67b4..f6cae4e12ea532 100644 --- a/src/test/java/com/google/devtools/build/lib/analysis/mock/BazelAnalysisMock.java +++ b/src/test/java/com/google/devtools/build/lib/analysis/mock/BazelAnalysisMock.java @@ -78,7 +78,7 @@ public List getWorkspaceContents(MockToolsConfig config) { "bind(name = 'android/sdk', actual='@bazel_tools//tools/android:sdk')", "register_toolchains('@bazel_tools//tools/cpp:all')", "register_toolchains('@bazel_tools//tools/jdk:all')", - "register_toolchains('@bazel_tools//tools/python:all')", + "register_toolchains('@bazel_tools//tools/python:autodetecting_toolchain')", "local_repository(name = 'local_config_platform', path = '" + localConfigPlatformWorkspace + "')")); 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..4f1a1522053264 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 @@ -70,7 +70,8 @@ public void setup(MockToolsConfig config) throws IOException { " py3_runtime = ':py3_interpreter',", ")", "toolchain(", - " name = 'default_python_toolchain',", + " # The Python workspace suffix looks to register a toolchain of this name.", + " name = 'autodetecting_toolchain',", " toolchain = ':default_py_runtime_pair',", " toolchain_type = ':toolchain_type',", ")", diff --git a/tools/python/pywrapper_template.txt b/tools/python/pywrapper_template.txt index 631a5996796da9..236a3813d7aab5 100644 --- a/tools/python/pywrapper_template.txt +++ b/tools/python/pywrapper_template.txt @@ -5,14 +5,22 @@ # https://groups.google.com/forum/?nomobile=true#!topic/bazel-dev/4Ql_7eDcLC0 # We do lose the ability to set -o pipefail. -FAILURE_HEADER="\ +STRICT=%STRICT% + +if [ "$STRICT" = "1" ]; then + FAILURE_HEADER="\ Error occurred while attempting to use the default Python toolchain \ (@bazel_tools//tools/python:autodetecting_toolchain)." +else + FAILURE_HEADER="\ +Error occurred while attempting to use the non-strict autodetecting Python \ +toolchain (@bazel_tools//tools/python:autodetecting_toolchain_nonstrict)." +fi die() { - echo "$FAILURE_HEADER" 1>&2 - echo "$1" 1>&2 - exit 1 + echo "$FAILURE_HEADER" 1>&2 + echo "$1" 1>&2 + exit 1 } # We use `which` to locate the Python interpreter command on PATH. `command -v` @@ -38,11 +46,11 @@ die() { PYTHON_BIN=`PATH="$PATH" which python%VERSION% || echo ""` USED_FALLBACK=0 if [ -z "${PYTHON_BIN:-}" ]; then - PYTHON_BIN=`PATH="$PATH" which python || echo ""` - USED_FALLBACK=1 + PYTHON_BIN=`PATH="$PATH" which python || echo ""` + USED_FALLBACK=1 fi if [ -z "${PYTHON_BIN:-}" ]; then - die "Neither 'python%VERSION%' nor 'python' were found on the target \ + die "Neither 'python%VERSION%' nor 'python' were found on the target \ platform's PATH, which is: $PATH @@ -53,7 +61,6 @@ documentation for py_runtime_pair \ (https://github.com/bazelbuild/bazel/blob/master/tools/python/toolchain.bzl)." fi -STRICT=%STRICT% if [ "$STRICT" = "1" ]; then # Verify that we grabbed an interpreter with the right version. VERSION_STR=`"$PYTHON_BIN" -V 2>&1` \ @@ -75,7 +82,7 @@ your build worked prior to Bazel 0.27, and you're sure your targets do not \ require Python %VERSION%, you can opt out of this version check by using the \ non-strict autodetecting toolchain instead of the standard autodetecting \ toolchain. This can be done by passing the flag \ -`--extra_toolchains=@bazel_tools//tools/python:autodetecting_toolchain_nonstrict` \ +\`--extra_toolchains=@bazel_tools//tools/python:autodetecting_toolchain_nonstrict\` \ on the command line or adding it to your bazelrc." fi fi