fix(bootstrap): handle when runfiles env vars don't point to current binary's runfiles (#3192)
The stage1 bootstrap script had a bug in the find_runfiles_root
function where it would unconditionally use the RUNFILES_DIR et al
environment variables if they were set.
This failed in a particular nested context: an outer binary
calling an inner binary when the inner binary isn't a data
dependency of the outer binary (i.e. the outer doesn't contain
the inner in runfiles). This would cause the inner binary to
incorrectly resolve its runfiles, leading to failures. Such
a case can occur if a genrule calls the outer binary, which has
the inner binary passed as an arg.
This change adds a check to validate that the script's entry point
exists within the inherited RUNFILES_DIR before using it. If the
entry point is not found, it proceeds with other runfiles discovery
methods. This matches the system_python runfiles discovery logic.
Fixes https://github.com/bazel-contrib/rules_python/issues/3187
diff --git a/CHANGELOG.md b/CHANGELOG.md
index fc3d7bb..03eccf8 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -47,6 +47,29 @@
END_UNRELEASED_TEMPLATE
-->
+{#v0-0-0}
+## Unreleased
+
+[0.0.0]: https://github.com/bazel-contrib/rules_python/releases/tag/0.0.0
+
+{#v0-0-0-changed}
+### Changed
+* Nothing changed.
+
+{#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)).
+
+{#v0-0-0-added}
+### Added
+* Nothing added.
+
+{#v0-0-0-removed}
+### Removed
+* Nothing removed.
+
{#1-6-0}
## [1.6.0] - 2025-08-23
@@ -102,7 +125,7 @@
name.
* (pypi) The selection of the whls has been changed and should no longer result
in ambiguous select matches ({gh-issue}`2759`) and should be much more efficient
- when running `bazel query` due to fewer repositories being included
+ when running `bazel query` due to fewer repositories being included
({gh-issue}`2849`).
* Multi-line python imports (e.g. with escaped newlines) are now correctly processed by Gazelle.
* (toolchains) `local_runtime_repo` works with multiarch Debian with Python 3.8
diff --git a/python/private/python_bootstrap_template.txt b/python/private/python_bootstrap_template.txt
index 62ded87..495a52c 100644
--- a/python/private/python_bootstrap_template.txt
+++ b/python/private/python_bootstrap_template.txt
@@ -516,6 +516,9 @@
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%)
diff --git a/python/private/stage1_bootstrap_template.sh b/python/private/stage1_bootstrap_template.sh
index 9927d4f..a984344 100644
--- a/python/private/stage1_bootstrap_template.sh
+++ b/python/private/stage1_bootstrap_template.sh
@@ -61,14 +61,20 @@
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 @@
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"
diff --git a/tests/bootstrap_impls/bin_calls_bin/BUILD.bazel b/tests/bootstrap_impls/bin_calls_bin/BUILD.bazel
new file mode 100644
index 0000000..02835fb
--- /dev/null
+++ b/tests/bootstrap_impls/bin_calls_bin/BUILD.bazel
@@ -0,0 +1,86 @@
+load("@rules_shell//shell:sh_test.bzl", "sh_test")
+load("//tests/support:py_reconfig.bzl", "py_reconfig_binary")
+load("//tests/support:support.bzl", "NOT_WINDOWS", "SUPPORTS_BOOTSTRAP_SCRIPT")
+
+# =====
+# bootstrap_impl=system_python testing
+# =====
+py_reconfig_binary(
+ name = "outer_bootstrap_system_python",
+ srcs = ["outer.py"],
+ bootstrap_impl = "system_python",
+ main = "outer.py",
+ tags = ["manual"],
+)
+
+py_reconfig_binary(
+ name = "inner_bootstrap_system_python",
+ srcs = ["inner.py"],
+ bootstrap_impl = "system_python",
+ main = "inner.py",
+ tags = ["manual"],
+)
+
+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) > $@",
+ tags = ["manual"],
+ 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",
+ ],
+ # The way verify_system_python.sh loads verify.sh doesn't work
+ # with Windows for some annoying reason. Just skip windows for now;
+ # the logic being test isn't OS-specific, so this should be fine.
+ target_compatible_with = NOT_WINDOWS,
+)
+
+# =====
+# bootstrap_impl=script testing
+# =====
+py_reconfig_binary(
+ name = "inner_bootstrap_script",
+ srcs = ["inner.py"],
+ bootstrap_impl = "script",
+ main = "inner.py",
+ tags = ["manual"],
+)
+
+py_reconfig_binary(
+ name = "outer_bootstrap_script",
+ srcs = ["outer.py"],
+ bootstrap_impl = "script",
+ main = "outer.py",
+ tags = ["manual"],
+)
+
+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) > $@",
+ tags = ["manual"],
+ 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",
+ ],
+ target_compatible_with = SUPPORTS_BOOTSTRAP_SCRIPT,
+)
diff --git a/tests/bootstrap_impls/bin_calls_bin/inner.py b/tests/bootstrap_impls/bin_calls_bin/inner.py
new file mode 100644
index 0000000..e67b31d
--- /dev/null
+++ b/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}'")
diff --git a/tests/bootstrap_impls/bin_calls_bin/outer.py b/tests/bootstrap_impls/bin_calls_bin/outer.py
new file mode 100644
index 0000000..19dac06
--- /dev/null
+++ b/tests/bootstrap_impls/bin_calls_bin/outer.py
@@ -0,0 +1,18 @@
+import os
+import subprocess
+import sys
+
+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)
diff --git a/tests/bootstrap_impls/bin_calls_bin/verify.sh b/tests/bootstrap_impls/bin_calls_bin/verify.sh
new file mode 100755
index 0000000..433704e
--- /dev/null
+++ b/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."
+}
diff --git a/tests/bootstrap_impls/bin_calls_bin/verify_script_python.sh b/tests/bootstrap_impls/bin_calls_bin/verify_script_python.sh
new file mode 100755
index 0000000..012daee
--- /dev/null
+++ b/tests/bootstrap_impls/bin_calls_bin/verify_script_python.sh
@@ -0,0 +1,5 @@
+#!/bin/bash
+set -euo pipefail
+
+source "$(dirname "$0")/verify.sh"
+verify_output "$(dirname "$0")/outer_calls_inner_script_python.out"
diff --git a/tests/bootstrap_impls/bin_calls_bin/verify_system_python.sh b/tests/bootstrap_impls/bin_calls_bin/verify_system_python.sh
new file mode 100755
index 0000000..460769f
--- /dev/null
+++ b/tests/bootstrap_impls/bin_calls_bin/verify_system_python.sh
@@ -0,0 +1,5 @@
+#!/bin/bash
+set -euo pipefail
+
+source "$(dirname "$0")/verify.sh"
+verify_output "$(dirname "$0")/outer_calls_inner_system_python.out"
diff --git a/tests/support/support.bzl b/tests/support/support.bzl
index adb8e75..f869462 100644
--- a/tests/support/support.bzl
+++ b/tests/support/support.bzl
@@ -54,3 +54,8 @@
"@platforms//os:windows": ["@platforms//:incompatible"],
"//conditions:default": [],
}) if BZLMOD_ENABLED else ["@platforms//:incompatible"]
+
+NOT_WINDOWS = select({
+ "@platforms//os:windows": ["@platforms//:incompatible"],
+ "//conditions:default": [],
+})
