Skip to content

Commit 7f49531

Browse files
brandjoncopybara-github
authored andcommitted
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 bazelbuild#7899. RELNOTES: None PiperOrigin-RevId: 249263849
1 parent 0ff19c6 commit 7f49531

File tree

2 files changed

+64
-9
lines changed

2 files changed

+64
-9
lines changed

src/test/shell/bazel/python_version_test.sh

+57-9
Original file line numberDiff line numberDiff line change
@@ -52,18 +52,12 @@ case "$(uname -s | tr [:upper:] [:lower:])" in
5252
msys*)
5353
# As of 2018-08-14, Bazel on Windows only supports MSYS Bash.
5454
declare -r is_windows=true
55-
# As of 2019-04-26, this test is disabled on Windows (via "no_windows" tag),
55+
# As of 2019-05-18, this test is disabled on Windows (via "no_windows" tag),
5656
# so this code shouldn't even have run.
57-
# TODO(bazelbuild/continuous-integration#578): Enable this test for Windows.
57+
# TODO(#7844): Enable this test for Windows once our autodetecting toolchain
58+
# works transparently for this platform.
5859
fail "This test does not run on Windows."
5960
;;
60-
darwin*)
61-
# As of 2019-04-26, this test is disabled on mac, but there's no "no_mac" tag
62-
# so we just have to trivially succeed.
63-
# TODO(bazelbuild/continuous-integration#578): Enable this test for Mac.
64-
echo "This test does not run on Mac; exiting early." >&2
65-
exit 0
66-
;;
6761
*)
6862
declare -r is_windows=false
6963
;;
@@ -654,4 +648,58 @@ configuration, which uses Python 3"
654648
expect_not_log "program that was built in the host configuration"
655649
}
656650

651+
# Regression test for (bazelbuild/continuous-integration#578): Ensure that a
652+
# py_binary built with the autodetecting toolchain works when used as a tool
653+
# from Starlark rule. In particular, the wrapper script that launches the real
654+
# second-stage interpreter must be able to tolerate PATH not being set.
655+
function test_py_binary_with_autodetecting_toolchain_usable_as_tool() {
656+
mkdir -p test
657+
658+
cat > test/BUILD << 'EOF'
659+
load(":tooluser.bzl", "tooluser_rule")
660+
661+
py_binary(
662+
name = "tool",
663+
srcs = ["tool.py"],
664+
)
665+
666+
tooluser_rule(
667+
name = "tooluser",
668+
out = "out.txt",
669+
)
670+
EOF
671+
cat > test/tooluser.bzl << EOF
672+
def _tooluser_rule_impl(ctx):
673+
ctx.actions.run(
674+
inputs = [],
675+
outputs = [ctx.outputs.out],
676+
executable = ctx.executable._tool,
677+
arguments = [ctx.outputs.out.path],
678+
)
679+
680+
tooluser_rule = rule(
681+
implementation = _tooluser_rule_impl,
682+
attrs = {
683+
"_tool": attr.label(
684+
executable = True,
685+
default = "//test:tool",
686+
# cfg param is required but its value doesn't matter
687+
cfg = "target"),
688+
"out": attr.output(),
689+
},
690+
)
691+
EOF
692+
cat > test/tool.py << EOF
693+
import sys
694+
with open(sys.argv[1], 'wt') as out:
695+
print("Tool output", file=out)
696+
EOF
697+
698+
bazel build //test:tooluser \
699+
--incompatible_use_python_toolchains=true \
700+
|| fail "bazel build failed"
701+
cat bazel-bin/test/out.txt &> $TEST_log
702+
expect_log "Tool output"
703+
}
704+
657705
run_suite "Tests for how the Python rules handle Python 2 vs Python 3"

tools/python/pywrapper_template.txt

+7
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,13 @@ die() {
1515
exit 1
1616
}
1717

18+
# Make sure PATH is exported. If we're called with PATH unset, as happens when
19+
# we're invoked as a tool during the build, the shell will initialize its own
20+
# PATH but not necessarily export it. This would break our call to `which`. See
21+
# https://github.com/bazelbuild/continuous-integration/issues/578 for more
22+
# information.
23+
export PATH
24+
1825
# Try the "python%VERSION%" command name first, then fall back on "python".
1926
PYTHON_BIN=`which python%VERSION% || echo ""`
2027
USED_FALLBACK=0

0 commit comments

Comments
 (0)