From 052167e907373ac7ea43238c3049739f6e94a9d1 Mon Sep 17 00:00:00 2001 From: brandjon Date: Tue, 4 Jun 2019 16:04:06 -0700 Subject: [PATCH] Add a non-strict autodetecting Python toolchain 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 #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 #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 #8547 RELNOTES: None PiperOrigin-RevId: 251535571 --- tools/python/BUILD | 7 ++++- tools/python/pywrapper_template.txt | 24 +++++++++++---- tools/python/pywrapper_test.py | 34 ++++++++++++++++----- tools/python/toolchain.bzl | 37 +++++++++++++++++++++++ tools/python/utils.bzl | 46 ++++++++++++++++------------- 5 files changed, 114 insertions(+), 34 deletions(-) diff --git a/tools/python/BUILD b/tools/python/BUILD index 04d9c239eab305..310284576b85d3 100644 --- a/tools/python/BUILD +++ b/tools/python/BUILD @@ -28,7 +28,9 @@ 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"], ) @@ -36,7 +38,10 @@ expand_pyversion_template( 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"], ) diff --git a/tools/python/pywrapper_template.txt b/tools/python/pywrapper_template.txt index d38720ddfb4e82..631a5996796da9 100644 --- a/tools/python/pywrapper_template.txt +++ b/tools/python/pywrapper_template.txt @@ -53,11 +53,13 @@ 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 @@ -65,7 +67,17 @@ $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" "$@" diff --git a/tools/python/pywrapper_test.py b/tools/python/pywrapper_test.py index fa4123902ed080..07732a627988e3 100644 --- a/tools/python/pywrapper_test.py +++ b/tools/python/pywrapper_test.py @@ -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. # @@ -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, @@ -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( @@ -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() diff --git a/tools/python/toolchain.bzl b/tools/python/toolchain.bzl index 6e8a62abcdcdfa..fa0d30f384bcb4 100644 --- a/tools/python/toolchain.bzl +++ b/tools/python/toolchain.bzl @@ -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. @@ -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"], ) @@ -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. @@ -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"], + ) diff --git a/tools/python/utils.bzl b/tools/python/utils.bzl index 9278e32efc67fd..7422608c723696 100644 --- a/tools/python/utils.bzl +++ b/tools/python/utils.bzl @@ -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, @@ -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.""", )