Skip to content

Commit

Permalink
Let Python stubs respect RUNFILES_* while finding the module space
Browse files Browse the repository at this point in the history
When invoking a py_binary() through an sh_binary() using the Bash
runfiles library, the location of the py_binary() will be resolved from
the runfiles manifest file. This means that the argv[0] of the Python
stub script will not point to a location under the runfiles directory of
the shell script, but to a location in Bazel's execroot.

Normally this does not lead to any issues, as argv[0] + ".runfiles"
happens to point to be a valid runfiles directory as well. It does
become problematic when --nobuild_runfile_links is provided, as in that
case only the outer shell script is guaranteed to have a runfiles
directory; not the inner Python script.

This change extends the Python stub template to also consider
RUNFILES_DIR when no runfiles directory can be found. Even though it's
not technically correct, we also attempt to derive the runfiles
directory from RUNFILES_MANIFEST_FILE. I suspect that this is a
necessity as long as py_binary()s cannot operate purely using a manifest
file. They currently depend on a concrete instantiation of the runfiles
directory.

Fixes: bazelbuild#11997
  • Loading branch information
EdSchouten committed Feb 7, 2022
1 parent 56df8e9 commit e126a36
Show file tree
Hide file tree
Showing 2 changed files with 67 additions and 6 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -95,8 +95,22 @@ def CreatePythonPathEntries(python_imports, module_space):
parts = python_imports.split(':')
return [module_space] + ['%s/%s' % (module_space, path) for path in parts]

def FindModuleSpace():
def FindModuleSpace(rel_path):
"""Finds the runfiles tree."""
# When the calling process used the runfiles manifest to resolve the
# location of this stub script, the path may be expanded. This means
# argv[0] may no longer point to a location inside the runfiles
# directory. We should therefore respect RUNFILES_DIR and
# RUNFILES_MANIFEST_FILE set by the caller.
runfiles_dir = os.environ.get('RUNFILES_DIR', None)
if not runfiles_dir:
runfiles_manifest_file = os.environ.get('RUNFILES_MANIFEST_FILE', '')
if (runfiles_manifest_file.endswith('.runfiles_manifest') or
runfiles_manifest_file.endswith('.runfiles/MANIFEST')):
runfiles_dir = runfiles_manifest_file[:-9]
if runfiles_dir and os.path.exists(os.path.join(runfiles_dir, rel_path)):
return runfiles_dir

stub_filename = sys.argv[0]
if not os.path.isabs(stub_filename):
stub_filename = os.path.join(os.getcwd(), stub_filename)
Expand Down Expand Up @@ -286,10 +300,14 @@ def Main():

new_env = {}

rel_path = '%main%'
if IsWindows():
rel_path = rel_path.replace('/', os.sep)

if IsRunningFromZip():
module_space = CreateModuleSpace()
else:
module_space = FindModuleSpace()
module_space = FindModuleSpace(rel_path)

python_imports = '%imports%'
python_path_entries = CreatePythonPathEntries(python_imports, module_space)
Expand Down Expand Up @@ -317,10 +335,6 @@ def Main():
# Now look for my main python source file.
# The magic string percent-main-percent is replaced with the filename of the
# main file of the Python binary in BazelPythonSemantics.java.
rel_path = '%main%'
if IsWindows():
rel_path = rel_path.replace('/', os.sep)

main_filename = os.path.join(module_space, rel_path)
main_filename = GetWindowsPathWithUNCPrefix(main_filename)
assert os.path.exists(main_filename), \
Expand Down
47 changes: 47 additions & 0 deletions src/test/shell/integration/python_stub_test.sh
Original file line number Diff line number Diff line change
Expand Up @@ -178,4 +178,51 @@ EOF
&> $TEST_log || fail "bazel run failed"
}

# When invoking a Python binary using the runfiles manifest, the stub
# script's argv[0] will point to a location in the execroot; not the
# runfiles directory of the caller. The stub script should still be
# capable of finding its runfiles directory by considering RUNFILES_DIR
# and RUNFILES_MANIFEST_FILE set by the caller.
function test_python_through_bash_without_runfile_links() {
mkdir -p python_through_bash

cat > python_through_bash/BUILD << EOF
py_binary(
name = "inner",
srcs = ["inner.py"],
)
sh_binary(
name = "outer",
srcs = ["outer.sh"],
data = [":inner"],
deps = ["@bazel_tools//tools/bash/runfiles"],
)
EOF

cat > python_through_bash/outer.sh << 'EOF'
#!/bin/bash
# --- begin runfiles.bash initialization v2 ---
# Copy-pasted from the Bazel Bash runfiles library v2.
set -uo pipefail; f=bazel_tools/tools/bash/runfiles/runfiles.bash
source "${RUNFILES_DIR:-/dev/null}/$f" 2>/dev/null || \
source "$(grep -sm1 "^$f " "${RUNFILES_MANIFEST_FILE:-/dev/null}" | cut -f2- -d' ')" 2>/dev/null || \
source "$0.runfiles/$f" 2>/dev/null || \
source "$(grep -sm1 "^$f " "$0.runfiles_manifest" | cut -f2- -d' ')" 2>/dev/null || \
source "$(grep -sm1 "^$f " "$0.exe.runfiles_manifest" | cut -f2- -d' ')" 2>/dev/null || \
{ echo>&2 "ERROR: cannot find $f"; exit 1; }; f=; set -e
# --- end runfiles.bash initialization v2 ---
exec "$(rlocation main/python_through_bash/inner)"
EOF
chmod +x python_through_bash/outer.sh

touch python_through_bash/inner.py

bazel run --nobuild_runfile_links //python_through_bash:outer \
&> $TEST_log || fail "bazel run failed"
expect_log "I am Python"
}

run_suite "Tests for the Python rules without Python execution"

0 comments on commit e126a36

Please sign in to comment.