Skip to content

Commit

Permalink
Add a non-strict autodetecting Python toolchain
Browse files Browse the repository at this point in the history
This toolchain works just like the standard autodetecting toolchain, except that if it falls back on the `python` command, it doesn't care what version that interpreter is. This allows users to opt into the legacy behavior of bazelbuild#4815 while still enabling Python toolchains.

This is particularly useful for Mac users who do not have a Python 3 runtime installed. Naturally, such users can only have Python 2 code in their builds, but it's still possible that these Python targets get analyzed by Bazel as PY3. For example, the target author may have forgotten to set the `python_version = "PY2"` attribute, or the downstream user may have not added `--host_force_python=PY2` to their bazelrc. Previously this worked because under bazelbuild#4815 Bazel would just use Python 2 for PY3 targets. Strict version checking breaks these builds, but the new non-strict toolchain provides a workaround.

Fixes bazelbuild#8547

RELNOTES: None
PiperOrigin-RevId: 251535571
  • Loading branch information
brandjon authored and copybara-github committed Jun 4, 2019
1 parent 7561e44 commit 052167e
Show file tree
Hide file tree
Showing 5 changed files with 114 additions and 34 deletions.
7 changes: 6 additions & 1 deletion tools/python/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -28,15 +28,20 @@ test_suite(
expand_pyversion_template(
name = "_generate_wrappers",
out2 = ":py2wrapper.sh",
out2_nonstrict = ":py2wrapper_nonstrict.sh",
out3 = ":py3wrapper.sh",
out3_nonstrict = ":py3wrapper_nonstrict.sh",
template = "pywrapper_template.txt",
visibility = ["//visibility:private"],
)

py_test(
name = "pywrapper_test",
srcs = ["pywrapper_test.py"],
data = [":py2wrapper.sh"],
data = [
":py2wrapper.sh",
":py2wrapper_nonstrict.sh",
],
python_version = "PY2",
deps = ["//src/test/py/bazel:test_base"],
)
24 changes: 18 additions & 6 deletions tools/python/pywrapper_template.txt
Original file line number Diff line number Diff line change
Expand Up @@ -53,19 +53,31 @@ documentation for py_runtime_pair \
(https://github.com/bazelbuild/bazel/blob/master/tools/python/toolchain.bzl)."
fi

# Verify that we grabbed an interpreter with the right version.
VERSION_STR=`"$PYTHON_BIN" -V 2>&1` \
|| die "Could not get interpreter version via '$PYTHON_BIN -V'"
if ! echo "$VERSION_STR" | grep -q " %VERSION%\." ; then
die "According to '$PYTHON_BIN -V', version is '$VERSION_STR', but we \
STRICT=%STRICT%
if [ "$STRICT" = "1" ]; then
# Verify that we grabbed an interpreter with the right version.
VERSION_STR=`"$PYTHON_BIN" -V 2>&1` \
|| die "Could not get interpreter version via '$PYTHON_BIN -V'"
if ! echo "$VERSION_STR" | grep -q " %VERSION%\." ; then
die "According to '$PYTHON_BIN -V', version is '$VERSION_STR', but we \
need version %VERSION%. PATH is:
$PATH
Please ensure an interpreter with version %VERSION% is available on this \
platform as 'python%VERSION%' or 'python', or else register an appropriate \
Python toolchain as per the documentation for py_runtime_pair \
(https://github.com/bazelbuild/bazel/blob/master/tools/python/toolchain.bzl)."
(https://github.com/bazelbuild/bazel/blob/master/tools/python/toolchain.bzl).
Note that prior to Bazel 0.27, there was no check to ensure that the \
interpreter's version matched the version declared by the target (#4815). If \
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` \
on the command line or adding it to your bazelrc."
fi
fi

exec "$PYTHON_BIN" "$@"
34 changes: 27 additions & 7 deletions tools/python/pywrapper_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -94,14 +94,20 @@ def setup_tool(self, cmd):
path, msg="Could not locate '%s' command on PATH" % cmd)
self.CopyFile(path, os.path.join("dir", cmd), executable=True)

def locate_runfile(self, runfile_path):
resolved_path = self.Rlocation(runfile_path)
self.assertIsNotNone(
resolved_path, msg="Could not locate %s in runfiles" % runfile_path)
return resolved_path

def setUp(self):
super(PywrapperTest, self).setUp()

# Locate script under test.
wrapper_path = self.Rlocation("io_bazel/tools/python/py2wrapper.sh")
self.assertIsNotNone(
wrapper_path, msg="Could not locate py2wrapper.sh in runfiles")
self.wrapper_path = wrapper_path
# Locate scripts under test.
self.wrapper_path = \
self.locate_runfile("io_bazel/tools/python/py2wrapper.sh")
self.nonstrict_wrapper_path = \
self.locate_runfile("io_bazel/tools/python/py2wrapper_nonstrict.sh")

# Setup scratch directory with all executables the script depends on.
#
Expand All @@ -112,10 +118,10 @@ def setUp(self):
self.setup_tool("echo")
self.setup_tool("grep")

def run_wrapper(self, title_for_logging=None):
def run_with_restricted_path(self, program, title_for_logging=None):
new_env = dict(os.environ)
new_env["PATH"] = self.Path("dir")
proc = subprocess.Popen([self.wrapper_path],
proc = subprocess.Popen([program],
stdout=subprocess.PIPE,
stderr=subprocess.PIPE,
universal_newlines=True,
Expand All @@ -136,6 +142,13 @@ def run_wrapper(self, title_for_logging=None):
""") % (title_for_logging, proc.returncode, out, err))
return proc.returncode, out, err

def run_wrapper(self, title_for_logging):
return self.run_with_restricted_path(self.wrapper_path, title_for_logging)

def run_nonstrict_wrapper(self, title_for_logging):
return self.run_with_restricted_path(self.nonstrict_wrapper_path,
title_for_logging)

def assert_wrapper_success(self, returncode, out, err):
self.assertEqual(returncode, 0, msg="Expected to exit without error")
self.assertEqual(
Expand Down Expand Up @@ -190,6 +203,13 @@ def test_interpreter_not_executable(self):
self.assert_wrapper_failure(returncode, out, err,
"Neither 'python2' nor 'python' were found")

def test_wrong_version_ok_for_nonstrict(self):
self.ScratchFile(
"dir/python2", MockPythonLines.WRONG_VERSION, executable=True)
returncode, out, err = \
self.run_nonstrict_wrapper("test_wrong_version_ok_for_nonstrict")
self.assert_wrapper_success(returncode, out, err)


if __name__ == "__main__":
unittest.main()
37 changes: 37 additions & 0 deletions tools/python/toolchain.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -122,6 +122,8 @@ def define_autodetecting_toolchain(
windows_config_setting):
"""Defines the autodetecting Python toolchain.
This includes both strict and non-strict variants.
For use only by @bazel_tools//tools/python:BUILD; see the documentation
comment there.
Expand All @@ -146,6 +148,8 @@ def define_autodetecting_toolchain(
template = pywrapper_template,
out2 = ":py2wrapper.sh",
out3 = ":py3wrapper.sh",
out2_nonstrict = ":py2wrapper_nonstrict.sh",
out3_nonstrict = ":py3wrapper_nonstrict.sh",
visibility = ["//visibility:private"],
)

Expand All @@ -169,6 +173,20 @@ def define_autodetecting_toolchain(
visibility = ["//visibility:private"],
)

native.py_runtime(
name = "_autodetecting_py2_runtime_nonstrict",
interpreter = ":py2wrapper_nonstrict.sh",
python_version = "PY2",
visibility = ["//visibility:private"],
)

native.py_runtime(
name = "_autodetecting_py3_runtime_nonstrict",
interpreter = ":py3wrapper_nonstrict.sh",
python_version = "PY3",
visibility = ["//visibility:private"],
)

# This is a dummy runtime whose interpreter_path triggers the native rule
# logic to use the legacy behavior on Windows.
# TODO(#7844): Remove this target.
Expand All @@ -193,9 +211,28 @@ def define_autodetecting_toolchain(
visibility = ["//visibility:public"],
)

py_runtime_pair(
name = "_autodetecting_py_runtime_pair_nonstrict",
py2_runtime = select({
# Same hack as above.
# TODO(#7844): Remove this hack.
windows_config_setting: ":_sentinel_py2_runtime",
"//conditions:default": ":_autodetecting_py2_runtime_nonstrict",
}),
py3_runtime = ":_autodetecting_py3_runtime_nonstrict",
visibility = ["//visibility:public"],
)

native.toolchain(
name = name,
toolchain = ":_autodetecting_py_runtime_pair",
toolchain_type = ":toolchain_type",
visibility = ["//visibility:public"],
)

native.toolchain(
name = name + "_nonstrict",
toolchain = ":_autodetecting_py_runtime_pair_nonstrict",
toolchain_type = ":toolchain_type",
visibility = ["//visibility:public"],
)
46 changes: 26 additions & 20 deletions tools/python/utils.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -20,20 +20,22 @@ file is less likely to cause bootstrapping issues.
"""

def _expand_pyversion_template_impl(ctx):
if ctx.outputs.out2:
ctx.actions.expand_template(
template = ctx.file.template,
output = ctx.outputs.out2,
substitutions = {"%VERSION%": "2"},
is_executable = True,
)
if ctx.outputs.out3:
ctx.actions.expand_template(
template = ctx.file.template,
output = ctx.outputs.out3,
substitutions = {"%VERSION%": "3"},
is_executable = True,
)
for output, version, strict in [
(ctx.outputs.out2, "2", "1"),
(ctx.outputs.out3, "3", "1"),
(ctx.outputs.out2_nonstrict, "2", "0"),
(ctx.outputs.out3_nonstrict, "3", "0"),
]:
if output:
ctx.actions.expand_template(
template = ctx.file.template,
output = output,
substitutions = {
"%VERSION%": version,
"%STRICT%": strict,
},
is_executable = True,
)

expand_pyversion_template = rule(
implementation = _expand_pyversion_template_impl,
Expand All @@ -42,12 +44,16 @@ expand_pyversion_template = rule(
allow_single_file = True,
doc = "The input template file.",
),
"out2": attr.output(doc = """\
The output file produced by substituting "%VERSION%" with "2"."""),
"out3": attr.output(doc = """\
The output file produced by substituting "%VERSION%" with "3"."""),
"out2": attr.output(doc = "The Python 2 strict wrapper."),
"out3": attr.output(doc = "The Python 3 strict wrapper."),
"out2_nonstrict": attr.output(
doc = "The Python 2 non-strict wrapper.",
),
"out3_nonstrict": attr.output(
doc = "The Python 3 non-strict wrapper.",
),
},
doc = """\
Given a template file, generates two expansions by replacing the substring
"%VERSION%" with "2" and "3".""",
Given the pywrapper template file, generates expansions for both versions of
Python and both levels of strictness.""",
)

0 comments on commit 052167e

Please sign in to comment.