Skip to content

Commit 528181a

Browse files
committed
fix(toolchains): use posix-compatible exec -a alternative (#3010)
The `exec -a` command doesn't work in dash, the default shell for Ubuntu/debian. To work around, use `sh -c`, which is posix and dash compatible. This allows changing the argv0 while invoking a different command. Also adds a test to verify the the runtime_env toolchain works with bootstrap script. Fixes #3009 (cherry picked from commit c4543cd)
1 parent a89ec64 commit 528181a

File tree

4 files changed

+45
-6
lines changed

4 files changed

+45
-6
lines changed

python/private/runtime_env_toolchain_interpreter.sh

Lines changed: 7 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -71,14 +71,15 @@ if [ -e "$self_dir/pyvenv.cfg" ] || [ -e "$self_dir/../pyvenv.cfg" ]; then
7171
if [ ! -e "$PYTHON_BIN" ]; then
7272
die "ERROR: Python interpreter does not exist: $PYTHON_BIN"
7373
fi
74-
# PYTHONEXECUTABLE is also used because `exec -a` doesn't fully trick the
75-
# pyenv wrappers.
74+
# PYTHONEXECUTABLE is also used because switching argv0 doesn't fully trick
75+
# the pyenv wrappers.
7676
# NOTE: The PYTHONEXECUTABLE envvar only works for non-Mac starting in Python 3.11
7777
export PYTHONEXECUTABLE="$venv_bin"
78-
# Python looks at argv[0] to determine sys.executable, so use exec -a
79-
# to make it think it's the venv's binary, not the actual one invoked.
80-
# NOTE: exec -a isn't strictly posix-compatible, but very widespread
81-
exec -a "$venv_bin" "$PYTHON_BIN" "$@"
78+
# Python looks at argv[0] to determine sys.executable, so set that to the venv
79+
# binary, not the actual one invoked.
80+
# NOTE: exec -a would be simpler, but isn't posix-compatible, and dash shell
81+
# (Ubuntu/debian default) doesn't support it; see #3009.
82+
exec sh -c "$PYTHON_BIN \$@" "$venv_bin" "$@"
8283
else
8384
exec "$PYTHON_BIN" "$@"
8485
fi

tests/runtime_env_toolchain/BUILD.bazel

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -40,3 +40,26 @@ py_reconfig_test(
4040
tags = ["no-remote-exec"],
4141
deps = ["//python/runfiles"],
4242
)
43+
44+
py_reconfig_test(
45+
name = "bootstrap_script_test",
46+
srcs = ["toolchain_runs_test.py"],
47+
bootstrap_impl = "script",
48+
data = [
49+
"//tests/support:current_build_settings",
50+
],
51+
extra_toolchains = [
52+
"//python/runtime_env_toolchains:all",
53+
# Necessary for RBE CI
54+
CC_TOOLCHAIN,
55+
],
56+
main = "toolchain_runs_test.py",
57+
# With bootstrap=script, the build version must match the runtime version
58+
# because the venv has the version in the lib/site-packages dir name.
59+
python_version = PYTHON_VERSION,
60+
# Our RBE has Python 3.6, which is too old for the language features
61+
# we use now. Using the runtime-env toolchain on RBE is pretty
62+
# questionable anyways.
63+
tags = ["no-remote-exec"],
64+
deps = ["//python/runfiles"],
65+
)

tests/runtime_env_toolchain/toolchain_runs_test.py

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
import json
22
import pathlib
33
import platform
4+
import sys
45
import unittest
56

67
from python.runfiles import runfiles
@@ -23,6 +24,14 @@ def test_ran(self):
2324
settings["interpreter"]["short_path"],
2425
)
2526

27+
if settings["bootstrap_impl"] == "script":
28+
# Verify we're running in a venv
29+
self.assertNotEqual(sys.prefix, sys.base_prefix)
30+
# .venv/ occurs for a build-time venv.
31+
# For a runtime created venv, it goes into a temp dir, so
32+
# look for the /bin/ dir as an indicator.
33+
self.assertRegex(sys.executable, r"[.]venv/|/bin/")
34+
2635

2736
if __name__ == "__main__":
2837
unittest.main()

tests/support/sh_py_run_test.bzl

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -135,6 +135,7 @@ def _current_build_settings_impl(ctx):
135135
ctx.actions.write(
136136
output = info,
137137
content = json.encode({
138+
"bootstrap_impl": ctx.attr._bootstrap_impl_flag[config_common.FeatureFlagInfo].value,
138139
"interpreter": {
139140
"short_path": runtime.interpreter.short_path if runtime.interpreter else None,
140141
},
@@ -153,6 +154,11 @@ Writes information about the current build config to JSON for testing.
153154
This is so tests can verify information about the build config used for them.
154155
""",
155156
implementation = _current_build_settings_impl,
157+
attrs = {
158+
"_bootstrap_impl_flag": attr.label(
159+
default = "//python/config_settings:bootstrap_impl",
160+
),
161+
},
156162
toolchains = [
157163
TARGET_TOOLCHAIN_TYPE,
158164
],

0 commit comments

Comments
 (0)