fix: make bootstrap_impl=script compute correct directory when RUNFILES_MANIFEST_FILE set (#2177)
The script-based bootstrap wasn't computing the correct runfiles
directory when
`RUNFILES_MANIFEST_FILE` was set. The path it computed stripped off the
manifest
file name, but didn't re-add the `.runfiles` suffix to point to the
runfiles
directory.
To fix, just re-append the `.runfiles` suffix after it removes the
manifest file
name portion of the path.
Reproducing this is a bit tricky and it's difficult to reproduce the
necessary build
flags in a test; all of the following must be met:
* `--enable_runfiles=false`, but this cannot be set by transitions, only
via command line
* `--build_runfile_manifests=true` (this can be set in a transition, but
see below)
* Due to https://github.com/bazelbuild/bazel/issues/7994, even if a
manifest is created,
the RUNFILES_MANIFEST_FILE env var won't be set _unless_ the test
strategy is local
(i.e. not sandboxed, which is the default).
To work around those issues, the test just recreates the necessary
envvar state and
invokes the binary. The underlying files may not exist, but that's OK
for the code
paths were testing.
Fixes https://github.com/bazelbuild/rules_python/issues/2186
---------
Co-authored-by: Richard Levasseur <rlevasseur@google.com>
diff --git a/CHANGELOG.md b/CHANGELOG.md
index 6a901bb..e468e2b 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -44,6 +44,11 @@
stage2 bootstrap template.
* (bzlmod) Properly handle relative path URLs in parse_simpleapi_html.bzl
* (gazelle) Correctly resolve deps that have top-level module overlap with a gazelle_python.yaml dep module
+* (rules) Make `RUNFILES_MANIFEST_FILE`-based invocations work when used with
+ {obj}`--bootstrap_impl=script`. This fixes invocations using non-sandboxed
+ test execution with `--enable_runfiles=false --build_runfile_manifests=true`.
+ ([#2186](https://github.com/bazelbuild/rules_python/issues/2186)).
+
### Added
* (rules) Executables provide {obj}`PyExecutableInfo`, which contains
diff --git a/python/private/stage1_bootstrap_template.sh b/python/private/stage1_bootstrap_template.sh
index 959e7ba..e7e418c 100644
--- a/python/private/stage1_bootstrap_template.sh
+++ b/python/private/stage1_bootstrap_template.sh
@@ -44,10 +44,10 @@
echo "$RUNFILES_DIR"
return 0
elif [[ "${RUNFILES_MANIFEST_FILE:-}" = *".runfiles_manifest" ]]; then
- echo "${RUNFILES_MANIFEST_FILE%%.runfiles_manifest}"
+ echo "${RUNFILES_MANIFEST_FILE%%.runfiles_manifest}.runfiles"
return 0
elif [[ "${RUNFILES_MANIFEST_FILE:-}" = *".runfiles/MANIFEST" ]]; then
- echo "${RUNFILES_MANIFEST_FILE%%.runfiles/MANIFEST}"
+ echo "${RUNFILES_MANIFEST_FILE%%.runfiles/MANIFEST}.runfiles"
return 0
fi
@@ -57,7 +57,6 @@
if [[ "$stub_filename" != /* ]]; then
stub_filename="$PWD/$stub_filename"
fi
-
while true; do
module_space="${stub_filename}.runfiles"
if [[ -d "$module_space" ]]; then
diff --git a/tests/bootstrap_impls/run_binary_zip_no_test.sh b/tests/bootstrap_impls/run_binary_zip_no_test.sh
index 2ee69f3..c45cae5 100755
--- a/tests/bootstrap_impls/run_binary_zip_no_test.sh
+++ b/tests/bootstrap_impls/run_binary_zip_no_test.sh
@@ -29,15 +29,46 @@
echo "Unable to locate test binary: $BIN_RLOCATION"
exit 1
fi
-actual=$($bin 2>&1)
-# How we detect if a zip file was executed from depends on which bootstrap
-# is used.
-# bootstrap_impl=script outputs RULES_PYTHON_ZIP_DIR=<somepath>
-# bootstrap_impl=system_python outputs file:.*Bazel.runfiles
-expected_pattern="Hello"
-if ! (echo "$actual" | grep "$expected_pattern" ) >/dev/null; then
- echo "expected output to match: $expected_pattern"
- echo "but got:\n$actual"
+function test_invocation() {
+ actual=$($bin)
+ # How we detect if a zip file was executed from depends on which bootstrap
+ # is used.
+ # bootstrap_impl=script outputs RULES_PYTHON_ZIP_DIR=<somepath>
+ # bootstrap_impl=system_python outputs file:.*Bazel.runfiles
+ expected_pattern="Hello"
+ if ! (echo "$actual" | grep "$expected_pattern" ) >/dev/null; then
+ echo "Test case failed: $1"
+ echo "expected output to match: $expected_pattern"
+ echo "but got:\n$actual"
+ exit 1
+ fi
+}
+
+# Test invocation with RUNFILES_DIR set
+unset RUNFILES_MANIFEST_FILE
+if [[ ! -e "$RUNFILES_DIR" ]]; then
+ echo "Runfiles doesn't exist: $RUNFILES_DIR"
exit 1
fi
+test_invocation "using RUNFILES_DIR"
+
+
+orig_runfiles_dir="$RUNFILES_DIR"
+unset RUNFILES_DIR
+
+# Test invocation using manifest within runfiles directory (output manifest)
+# NOTE: this file may not actually exist in our test, but that's OK; the
+# bootstrap just uses the path to find the runfiles directory.
+export RUNFILES_MANIFEST_FILE="$orig_runfiles_dir/MANIFEST"
+test_invocation "using RUNFILES_MANIFEST_FILE with output manifest"
+
+# Test invocation using manifest outside runfiles (input manifest)
+# NOTE: this file may not actually exist in our test, but that's OK; the
+# bootstrap just uses the path to find the runfiles directory.
+export RUNFILES_MANIFEST_FILE="${orig_runfiles_dir%%.runfiles}.runfiles_manifest"
+test_invocation "using RUNFILES_MANIFEST_FILE with input manifest"
+
+# Test invocation without any runfiles env vars set
+unset RUNFILES_MANIFEST_FILE
+test_invocation "using no runfiles env vars"
