fix(bootstrap) handle when the runfiles env vars are not correct (#3644)

This change addresses an issue that has existed since 1.7.0 where Python
binaries launched as subprocesses could incorrectly inherit and use
runfiles environment variables (`RUNFILES_DIR`,
`RUNFILES_MANIFEST_FILE`) from the parent process.

## Why this change is needed:

When a Python binary spawns another Python binary, the child process
inherits the environment variables. If the parent had runfiles-related
environment variables set, the child would attempt to use the parent's
runfiles tree, which is incorrect and leads to import errors if the
child has different dependencies or a different runfiles layout.

## Behavior Before:

A child Python process would trust and use any existing `RUNFILES_DIR`
and `RUNFILES_MANIFEST_FILE` environment variables. If these were set by
a parent Python process, they would point to the parent's runfiles,
causing the child to fail when trying to load its own resources or
dependencies.

## Behavior After:

The Python bootstrap scripts now include a check to validate the
inherited runfiles environment variables. If the runfiles variables
exist but do not point to the correct location for the current binary,
the bootstrap script will unset both `RUNFILES_DIR` and
`RUNFILES_MANIFEST_FILE` from the environment. This allows the
subsequent fallback logic in the bootstrap process to correctly locate
the runfiles for the current process, ensuring dependencies are resolved
properly. This modification has been applied to both bootstrap template
files.

This change ensures that even nested Python binary calls correctly find
their respective runfiles.

Fixes #3518

---------

Co-authored-by: Ignas Anikevicius <240938+aignas@users.noreply.github.com>

Author: adhoc-bobcat

CHANGELOG.md

@@ -102,6 +102,9 @@ Other changes:
 * (bootstrap) Fixed incorrect runfiles path construction in bootstrap
   scripts when binary is defined in another bazel module
   ([#3563](https://github.com/bazel-contrib/rules_python/issues/3563)).
+* (bootstrap) Resolve `RUNFILES_DIR` inheritance issues, which lead to a child
+  Python binary incorrectly using it's parent's Python binary environment
+  ([#3518](https://github.com/bazel-contrib/rules_python/issues/3518)).
 * (uv) Downloads for versions `>=0.10` work again. In order to fix this we had
   drop support for `powerpc64` platform. People interested in the platform can
   bring it back via the `uv.default` API. Like:

python/private/python_bootstrap_template.txt

@@ -214,6 +214,13 @@ def find_runfiles_root(main_rel_path):
   if runfiles_dir and os.path.exists(os.path.join(runfiles_dir, main_rel_path)):
     return runfiles_dir
 
+  # Clear RUNFILES_DIR & RUNFILES_MANIFEST_FILE since the runfiles dir was
+  # not found. These can be correctly set for a parent Python process, but
+  # inherited by the child, and not correct for it. Later bootstrap code
+  # assumes they're are correct if set.
+  os.environ.pop('RUNFILES_DIR', None)
+  os.environ.pop('RUNFILES_MANIFEST_FILE', None)
+
   stub_filename = sys.argv[0]
   # On Windows, the path may contain both forward and backslashes.
   # Normalize to the OS separator because the regex used later assumes

python/private/stage2_bootstrap_template.py

@@ -185,6 +185,13 @@ def find_runfiles_root(main_rel_path):
     if runfiles_dir and os.path.exists(os.path.join(runfiles_dir, main_rel_path)):
         return runfiles_dir
 
+    # Clear RUNFILES_DIR & RUNFILES_MANIFEST_FILE since the runfiles dir was
+    # not found. These can be correctly set for a parent Python process, but
+    # inherited by the child, and not correct for it. Later bootstrap code
+    # assumes they're are correct if set.
+    os.environ.pop('RUNFILES_DIR', None)
+    os.environ.pop('RUNFILES_MANIFEST_FILE', None)
+
     stub_filename = sys.argv[0]
     if not os.path.isabs(stub_filename):
         stub_filename = os.path.join(os.getcwd(), stub_filename)

tests/bootstrap_impls/bin_calls_bin/BUILD.bazel

@@ -1,7 +1,14 @@
 load("@rules_shell//shell:sh_test.bzl", "sh_test")
+load("//python:py_library.bzl", "py_library")
 load("//tests/support:py_reconfig.bzl", "py_reconfig_binary")
 load("//tests/support:support.bzl", "NOT_WINDOWS", "SUPPORTS_BOOTSTRAP_SCRIPT")
 
+py_library(
+    name = "inner_lib",
+    srcs = ["inner_lib.py"],
+    tags = ["manual"],
+)
+
 # =====
 # bootstrap_impl=system_python testing
 # =====
@@ -19,6 +26,7 @@ py_reconfig_binary(
     bootstrap_impl = "system_python",
     main = "inner.py",
     tags = ["manual"],
+    deps = [":inner_lib"],
 )
 
 genrule(
@@ -54,6 +62,7 @@ py_reconfig_binary(
     bootstrap_impl = "script",
     main = "inner.py",
     tags = ["manual"],
+    deps = [":inner_lib"],
 )
 
 py_reconfig_binary(

tests/bootstrap_impls/bin_calls_bin/inner.py

@@ -1,4 +1,14 @@
 import os
 
 runfiles_root = os.environ.get("RULES_PYTHON_TESTING_RUNFILES_ROOT")
+runfiles_dir = os.environ.get("RUNFILES_DIR")
+runfiles_manifest_file = os.environ.get("RUNFILES_MANIFEST_FILE")
 print(f"inner: RULES_PYTHON_TESTING_RUNFILES_ROOT='{runfiles_root}'")
+print(f"inner: RUNFILES_DIR='{runfiles_dir}'")
+print(f"inner: RUNFILES_MANIFEST_FILE='{runfiles_manifest_file}'")
+
+try:
+    import tests.bootstrap_impls.bin_calls_bin.inner_lib as inner_lib
+    print(f"inner: import_result='{inner_lib.confirm()}'")
+except ImportError as e:
+    print(f"inner: import_result='{e}'")

tests/bootstrap_impls/bin_calls_bin/inner_lib.py

@@ -0,0 +1,3 @@
+# Rather than having a completely empty file...
+def confirm():
+    return "success"

tests/bootstrap_impls/bin_calls_bin/outer.py

@@ -11,8 +11,8 @@
         [inner_binary_path],
         capture_output=True,
         text=True,
-        check=True,
     )
     print(result.stdout, end="")
     if result.stderr:
         print(result.stderr, end="", file=sys.stderr)
+    sys.exit(result.returncode)

tests/bootstrap_impls/bin_calls_bin/verify.sh

@@ -11,6 +11,18 @@ verify_output() {
   echo "Outer runfiles root: $OUTER_RUNFILES_ROOT"
   echo "Inner runfiles root: $INNER_RUNFILES_ROOT"
 
+  # Extract the inner runfiles values
+  local INNER_RUNFILES_DIR=$(grep "inner: RUNFILES_DIR" "$OUTPUT_FILE" | sed "s/inner: RUNFILES_DIR='\(.*\)'/\1/")
+  local INNER_RUNFILES_MANIFEST_FILE=$(grep "inner: RUNFILES_MANIFEST_FILE" "$OUTPUT_FILE" | sed "s/inner: RUNFILES_MANIFEST_FILE='\(.*\)'/\1/")
+
+  echo "Inner runfiles dir: $INNER_RUNFILES_DIR"
+  echo "Inner runfiles manifest file: $INNER_RUNFILES_MANIFEST_FILE"
+
+  # Extract the inner lib import result
+  local INNER_LIB_IMPORT=$(grep "inner: import_result" "$OUTPUT_FILE" | sed "s/inner: import_result='\(.*\)'/\1/")
+  echo "Inner lib import result: $INNER_LIB_IMPORT"
+
+
   # Check 1: The two values are different
   if [ "$OUTER_RUNFILES_ROOT" == "$INNER_RUNFILES_ROOT" ]; then
     echo "Error: Outer and Inner runfiles roots are the same."
@@ -28,5 +40,11 @@ verify_output() {
       ;;
   esac
 
+  # Check 3: inner_lib was imported
+  if [ "$INNER_LIB_IMPORT" != "success" ]; then
+    echo "Error: Inner lib was not successfully imported."
+    exit 1
+  fi
+
   echo "Verification successful."
 }