Skip to content

Commit

Permalink
Avoid exporting PATH unnecessarily
Browse files Browse the repository at this point in the history
This is a modification of PR bazelbuild#8415, which changed `which` to `command -v` so it works when PATH isn't exported. `command` is more standard for this kind of use case (https://github.com/koalaman/shellcheck/wiki/SC2230). Unfortunately, `command -v` doesn't check the executable bit, so it's not as useful for us.

The previous fix for this issue (bazelbuild@7f49531) was based on exporting PATH, but this changes the environment seen by the exec'd payload Python code.

The solution in this commit is to not export PATH but rather explicitly pass it to each invocation of `which`.

Fixes bazelbuild#8414. See also bazelbuild/continuous-integration#578. Closes bazelbuild#8415.

PiperOrigin-RevId: 249334274
  • Loading branch information
keith authored and copybara-github committed May 21, 2019
1 parent b7a9615 commit ddce723
Showing 1 changed file with 20 additions and 8 deletions.
28 changes: 20 additions & 8 deletions tools/python/pywrapper_template.txt
Original file line number Diff line number Diff line change
Expand Up @@ -15,18 +15,30 @@ 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
# We use `which` to locate the Python interpreter command on PATH. `command -v`
# is another option, but it doesn't check whether the file it finds has the
# executable bit.
#
# A tricky situation happens when this wrapper is invoked as part of running a
# tool, e.g. passing a py_binary target to `ctx.actions.run()`. Bazel will unset
# the PATH variable. Then the shell will see there's no PATH and initialize its
# own, sometimes without exporting it. This causes `which` to fail and this
# script to think there's no Python interpreter installed. To avoid this we
# explicitly pass PATH to each `which` invocation. We can't just export PATH
# because that would modify the environment seen by the final user Python
# program.
#
# See also:
#
# https://github.com/bazelbuild/continuous-integration/issues/578
# https://github.com/bazelbuild/bazel/issues/8414
# https://github.com/bazelbuild/bazel/issues/8415

# Try the "python%VERSION%" command name first, then fall back on "python".
PYTHON_BIN=`which python%VERSION% || echo ""`
PYTHON_BIN=`PATH="$PATH" which python%VERSION% || echo ""`
USED_FALLBACK=0
if [ -z "${PYTHON_BIN:-}" ]; then
PYTHON_BIN=`which python || echo ""`
PYTHON_BIN=`PATH="$PATH" which python || echo ""`
USED_FALLBACK=1
fi
if [ -z "${PYTHON_BIN:-}" ]; then
Expand Down

0 comments on commit ddce723

Please sign in to comment.