fix: make bootstrap_impl=script compute correct directory when RUNFIL…

…ES_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 bazelbuild/bazel#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 #2186

---------

Co-authored-by: Richard Levasseur <rlevasseur@google.com>

CHANGELOG.md

@@ -44,6 +44,11 @@ A brief description of the categories of changes:
   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

python/private/stage1_bootstrap_template.sh

@@ -44,10 +44,10 @@ else
       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 @@ else
     if [[ "$stub_filename" != /* ]]; then
       stub_filename="$PWD/$stub_filename"
     fi
-
     while true; do
       module_space="${stub_filename}.runfiles"
       if [[ -d "$module_space" ]]; then

tests/bootstrap_impls/run_binary_zip_no_test.sh

@@ -29,15 +29,46 @@ if [[ -z "$bin" ]]; then
   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"