Skip to content

Commit

Permalink
Fix problems with the non-strict Python toolchain
Browse files Browse the repository at this point in the history
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 bazelbuild#8576, follow-up to bazelbuild#8547.

RELNOTES: None
PiperOrigin-RevId: 251937305
  • Loading branch information
brandjon authored and copybara-github committed Jun 6, 2019
1 parent dc6d3b5 commit 50fa3ec
Show file tree
Hide file tree
Showing 4 changed files with 26 additions and 12 deletions.
Original file line number Diff line number Diff line change
@@ -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")
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,7 @@ public List<String> 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
+ "')"));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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',",
")",
Expand Down
25 changes: 16 additions & 9 deletions tools/python/pywrapper_template.txt
Original file line number Diff line number Diff line change
Expand Up @@ -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`
Expand All @@ -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
Expand All @@ -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` \
Expand All @@ -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
Expand Down

0 comments on commit 50fa3ec

Please sign in to comment.