fix(bootstrap): handle when runfiles env vars don't point to current binary's runfiles

CHANGELOG.md

@@ -86,6 +86,9 @@ END_UNRELEASED_TEMPLATE
 
 {#v0-0-0-fixed}
 ### Fixed
+* (bootstrap) The stage1 bootstrap script now correctly handles nested `RUNFILES_DIR`
+  environments, fixing issues where a `py_binary` calls another `py_binary`
+  ([#3187](https://github.com/bazel-contrib/rules_python/issues/3187)).
 * (pypi) Fixes an issue where builds using a `bazel vendor` vendor directory
   would fail if the constraints file contained environment markers. Fixes
   [#2996](https://github.com/bazel-contrib/rules_python/issues/2996).

python/private/python_bootstrap_template.txt

@@ -513,6 +513,9 @@ def Main():
     module_space = FindModuleSpace(main_rel_path)
     delete_module_space = False
 
+  if os.environ.get("RULES_PYTHON_TESTING_TELL_MODULE_SPACE"):
+    new_env["RULES_PYTHON_TESTING_MODULE_SPACE"] = module_space
+
   python_imports = '%imports%'
   python_path_entries = CreatePythonPathEntries(python_imports, module_space)
   python_path_entries += GetRepositoriesImports(module_space, %import_all%)

python/private/stage1_bootstrap_template.sh

@@ -61,14 +61,20 @@ if [[ "$IS_ZIPFILE" == "1" ]]; then
 
 else
   function find_runfiles_root() {
+    local maybe_root=""
     if [[ -n "${RUNFILES_DIR:-}" ]]; then
-      echo "$RUNFILES_DIR"
-      return 0
+      maybe_root="$RUNFILES_DIR"
     elif [[ "${RUNFILES_MANIFEST_FILE:-}" = *".runfiles_manifest" ]]; then
-      echo "${RUNFILES_MANIFEST_FILE%%.runfiles_manifest}.runfiles"
-      return 0
+      maybe_root="${RUNFILES_MANIFEST_FILE%%.runfiles_manifest}.runfiles"
     elif [[ "${RUNFILES_MANIFEST_FILE:-}" = *".runfiles/MANIFEST" ]]; then
-      echo "${RUNFILES_MANIFEST_FILE%%.runfiles/MANIFEST}.runfiles"
+      maybe_root="${RUNFILES_MANIFEST_FILE%%.runfiles/MANIFEST}.runfiles"
+    fi
+
+    # The RUNFILES_DIR et al variables may misreport the runfiles directory
+    # if an outer binary invokes this binary when it isn't a data dependency.
+    # e.g. a genrule calls `bazel-bin/outer --inner=bazel-bin/inner`
+    if [[ -n "$maybe_root" && -e "$maybe_root/$STAGE2_BOOTSTRAP" ]]; then
+      echo "$maybe_root"
       return 0
     fi
 
@@ -99,6 +105,9 @@ else
   RUNFILES_DIR=$(find_runfiles_root $0)
 fi
 
+if [[ -n "$RULES_PYTHON_TESTING_TELL_MODULE_SPACE" ]]; then
+  export RULES_PYTHON_TESTING_MODULE_SPACE="$RUNFILES_DIR"
+fi
 
 function find_python_interpreter() {
   runfiles_root="$1"

tests/bootstrap_impls/bin_calls_bin/BUILD.bazel

@@ -0,0 +1,74 @@
+load("@rules_shell//shell:sh_test.bzl", "sh_test")
+load("//tests/support:py_reconfig.bzl", "py_reconfig_binary")
+
+# =====
+# bootstrap_impl=system_python testing
+# =====
+py_reconfig_binary(
+    name = "outer_bootstrap_system_python",
+    srcs = ["outer.py"],
+    bootstrap_impl = "system_python",
+    main = "outer.py",
+)
+
+py_reconfig_binary(
+    name = "inner_bootstrap_system_python",
+    srcs = ["inner.py"],
+    bootstrap_impl = "system_python",
+    main = "inner.py",
+)
+
+genrule(
+    name = "outer_calls_inner_system_python",
+    outs = ["outer_calls_inner_system_python.out"],
+    cmd = "RULES_PYTHON_TESTING_TELL_MODULE_SPACE=1 $(location :outer_bootstrap_system_python) $(location :inner_bootstrap_system_python) > $@",
+    tools = [
+        ":inner_bootstrap_system_python",
+        ":outer_bootstrap_system_python",
+    ],
+)
+
+sh_test(
+    name = "bootstrap_system_python_test",
+    srcs = ["verify_system_python.sh"],
+    data = [
+        "verify.sh",
+        ":outer_calls_inner_system_python",
+    ],
+)
+
+# =====
+# bootstrap_impl=script testing
+# =====
+py_reconfig_binary(
+    name = "inner_bootstrap_script",
+    srcs = ["inner.py"],
+    bootstrap_impl = "script",
+    main = "inner.py",
+)
+
+py_reconfig_binary(
+    name = "outer_bootstrap_script",
+    srcs = ["outer.py"],
+    bootstrap_impl = "script",
+    main = "outer.py",
+)
+
+genrule(
+    name = "outer_calls_inner_script_python",
+    outs = ["outer_calls_inner_script_python.out"],
+    cmd = "RULES_PYTHON_TESTING_TELL_MODULE_SPACE=1 $(location :outer_bootstrap_script) $(location :inner_bootstrap_script) > $@",
+    tools = [
+        ":inner_bootstrap_script",
+        ":outer_bootstrap_script",
+    ],
+)
+
+sh_test(
+    name = "bootstrap_script_python_test",
+    srcs = ["verify_script_python.sh"],
+    data = [
+        "verify.sh",
+        ":outer_calls_inner_script_python",
+    ],
+)

tests/bootstrap_impls/bin_calls_bin/inner.py

@@ -0,0 +1,4 @@
+import os
+
+module_space = os.environ.get("RULES_PYTHON_TESTING_MODULE_SPACE")
+print(f"inner: RULES_PYTHON_TESTING_MODULE_SPACE='{module_space}'")

tests/bootstrap_impls/bin_calls_bin/outer.py

@@ -0,0 +1,18 @@
+import subprocess
+import sys
+import os
+
+if __name__ == "__main__":
+    module_space = os.environ.get("RULES_PYTHON_TESTING_MODULE_SPACE")
+    print(f"outer: RULES_PYTHON_TESTING_MODULE_SPACE='{module_space}'")
+
+    inner_binary_path = sys.argv[1]
+    result = subprocess.run(
+        [inner_binary_path],
+        capture_output=True,
+        text=True,
+        check=True,
+    )
+    print(result.stdout, end="")
+    if result.stderr:
+        print(result.stderr, end="", file=sys.stderr)

tests/bootstrap_impls/bin_calls_bin/verify.sh

@@ -0,0 +1,32 @@
+#!/bin/bash
+set -euo pipefail
+
+verify_output() {
+  local OUTPUT_FILE=$1
+
+  # Extract the RULES_PYTHON_TESTING_MODULE_SPACE values
+  local OUTER_MODULE_SPACE=$(grep "outer: RULES_PYTHON_TESTING_MODULE_SPACE" "$OUTPUT_FILE" | sed "s/outer: RULES_PYTHON_TESTING_MODULE_SPACE='\(.*\)'/\1/")
+  local INNER_MODULE_SPACE=$(grep "inner: RULES_PYTHON_TESTING_MODULE_SPACE" "$OUTPUT_FILE" | sed "s/inner: RULES_PYTHON_TESTING_MODULE_SPACE='\(.*\)'/\1/")
+
+  echo "Outer module space: $OUTER_MODULE_SPACE"
+  echo "Inner module space: $INNER_MODULE_SPACE"
+
+  # Check 1: The two values are different
+  if [ "$OUTER_MODULE_SPACE" == "$INNER_MODULE_SPACE" ]; then
+    echo "Error: Outer and Inner module spaces are the same."
+    exit 1
+  fi
+
+  # Check 2: Inner is not a subdirectory of Outer
+  case "$INNER_MODULE_SPACE" in
+    "$OUTER_MODULE_SPACE"/*)
+      echo "Error: Inner module space is a subdirectory of Outer's."
+      exit 1
+      ;;
+    *)
+      # This is the success case
+      ;;
+  esac
+
+  echo "Verification successful."
+}

tests/bootstrap_impls/bin_calls_bin/verify_script_python.sh

@@ -0,0 +1,4 @@
+#!/bin/bash
+set -euo pipefail
+source "$(dirname "$0")/verify.sh"
+verify_output "$(dirname "$0")/outer_calls_inner_script_python.out"

tests/bootstrap_impls/bin_calls_bin/verify_system_python.sh

@@ -0,0 +1,4 @@
+#!/bin/bash
+set -euo pipefail
+source "$(dirname "$0")/verify.sh"
+verify_output "$(dirname "$0")/outer_calls_inner_system_python.out"