diff --git a/CHANGELOG.md b/CHANGELOG.md index 39f0223e4a..7734a45fe4 100644 --- a/CHANGELOG.md +++ b/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: diff --git a/python/private/python_bootstrap_template.txt b/python/private/python_bootstrap_template.txt index 4efd46690a..51c9013e9d 100644 --- a/python/private/python_bootstrap_template.txt +++ b/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 diff --git a/python/private/stage2_bootstrap_template.py b/python/private/stage2_bootstrap_template.py index c03a4a2e62..3c2d6807cc 100644 --- a/python/private/stage2_bootstrap_template.py +++ b/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) diff --git a/tests/bootstrap_impls/bin_calls_bin/BUILD.bazel b/tests/bootstrap_impls/bin_calls_bin/BUILD.bazel index 1822ccd08e..b5ba68f987 100644 --- a/tests/bootstrap_impls/bin_calls_bin/BUILD.bazel +++ b/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( diff --git a/tests/bootstrap_impls/bin_calls_bin/inner.py b/tests/bootstrap_impls/bin_calls_bin/inner.py index 96409eb8f8..6fef455a84 100644 --- a/tests/bootstrap_impls/bin_calls_bin/inner.py +++ b/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}'") diff --git a/tests/bootstrap_impls/bin_calls_bin/inner_lib.py b/tests/bootstrap_impls/bin_calls_bin/inner_lib.py new file mode 100644 index 0000000000..97efbb1565 --- /dev/null +++ b/tests/bootstrap_impls/bin_calls_bin/inner_lib.py @@ -0,0 +1,3 @@ +# Rather than having a completely empty file... +def confirm(): + return "success" \ No newline at end of file diff --git a/tests/bootstrap_impls/bin_calls_bin/outer.py b/tests/bootstrap_impls/bin_calls_bin/outer.py index f995c24ff6..a4432dff59 100644 --- a/tests/bootstrap_impls/bin_calls_bin/outer.py +++ b/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) diff --git a/tests/bootstrap_impls/bin_calls_bin/verify.sh b/tests/bootstrap_impls/bin_calls_bin/verify.sh index 9a9a17c6c6..bbe4252cc1 100755 --- a/tests/bootstrap_impls/bin_calls_bin/verify.sh +++ b/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." }