From 7f495315749478e75a3424726cc273a535b7c3b8 Mon Sep 17 00:00:00 2001 From: brandjon Date: Tue, 21 May 2019 09:20:01 -0700 Subject: [PATCH] Fix the autodetecting Python toolchain on Mac See bazelbuild/continuous-integration#578 for context. The gist of it is that when PATH is not set (as happens when a binary using this toolchain is used as a tool from Starlark), the shell comes up with its own PATH to run the pywrapper script. However, it does not necessarily export this PATH, causing "which" to misbehave and fail to locate a Python interpreter. The fix is to add "export PATH" and a regression test. Fixes bazelbuild/continuous-integration#578, work toward #7899. RELNOTES: None PiperOrigin-RevId: 249263849 --- src/test/shell/bazel/python_version_test.sh | 66 ++++++++++++++++++--- tools/python/pywrapper_template.txt | 7 +++ 2 files changed, 64 insertions(+), 9 deletions(-) diff --git a/src/test/shell/bazel/python_version_test.sh b/src/test/shell/bazel/python_version_test.sh index 096f7d163922ee..a01977a8798ea8 100755 --- a/src/test/shell/bazel/python_version_test.sh +++ b/src/test/shell/bazel/python_version_test.sh @@ -52,18 +52,12 @@ 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), + # As of 2019-05-18, 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. + # TODO(#7844): Enable this test for Windows once our autodetecting toolchain + # works transparently for this platform. 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. - echo "This test does not run on Mac; exiting early." >&2 - exit 0 - ;; *) declare -r is_windows=false ;; @@ -654,4 +648,58 @@ configuration, which uses Python 3" expect_not_log "program that was built in the host configuration" } +# Regression test for (bazelbuild/continuous-integration#578): Ensure that a +# py_binary built with the autodetecting toolchain works when used as a tool +# from Starlark rule. In particular, the wrapper script that launches the real +# second-stage interpreter must be able to tolerate PATH not being set. +function test_py_binary_with_autodetecting_toolchain_usable_as_tool() { + mkdir -p test + + cat > test/BUILD << 'EOF' +load(":tooluser.bzl", "tooluser_rule") + +py_binary( + name = "tool", + srcs = ["tool.py"], +) + +tooluser_rule( + name = "tooluser", + out = "out.txt", +) +EOF + cat > test/tooluser.bzl << EOF +def _tooluser_rule_impl(ctx): + ctx.actions.run( + inputs = [], + outputs = [ctx.outputs.out], + executable = ctx.executable._tool, + arguments = [ctx.outputs.out.path], + ) + +tooluser_rule = rule( + implementation = _tooluser_rule_impl, + attrs = { + "_tool": attr.label( + executable = True, + default = "//test:tool", + # cfg param is required but its value doesn't matter + cfg = "target"), + "out": attr.output(), + }, +) +EOF + cat > test/tool.py << EOF +import sys +with open(sys.argv[1], 'wt') as out: + print("Tool output", file=out) +EOF + + bazel build //test:tooluser \ + --incompatible_use_python_toolchains=true \ + || fail "bazel build failed" + cat bazel-bin/test/out.txt &> $TEST_log + expect_log "Tool output" +} + run_suite "Tests for how the Python rules handle Python 2 vs Python 3" diff --git a/tools/python/pywrapper_template.txt b/tools/python/pywrapper_template.txt index 5153ca51a97aa8..f4dbd02618c975 100644 --- a/tools/python/pywrapper_template.txt +++ b/tools/python/pywrapper_template.txt @@ -15,6 +15,13 @@ die() { exit 1 } +# Make sure PATH is exported. If we're called with PATH unset, as happens when +# we're invoked as a tool during the build, the shell will initialize its own +# PATH but not necessarily export it. This would break our call to `which`. See +# https://github.com/bazelbuild/continuous-integration/issues/578 for more +# information. +export PATH + # Try the "python%VERSION%" command name first, then fall back on "python". PYTHON_BIN=`which python%VERSION% || echo ""` USED_FALLBACK=0