diff --git a/CHANGELOG.md b/CHANGELOG.md index fc3d7bbce3..03eccf881e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -47,6 +47,29 @@ BEGIN_UNRELEASED_TEMPLATE 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 @@ END_UNRELEASED_TEMPLATE 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 62ded87337..495a52cfe9 100644 --- a/python/private/python_bootstrap_template.txt +++ b/python/private/python_bootstrap_template.txt @@ -516,6 +516,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%) diff --git a/python/private/stage1_bootstrap_template.sh b/python/private/stage1_bootstrap_template.sh index 9927d4faa7..a984344647 100644 --- a/python/private/stage1_bootstrap_template.sh +++ b/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" 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 0000000000..02835fb77b --- /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 0000000000..e67b31dda3 --- /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 0000000000..19dac06eb7 --- /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 0000000000..433704e9ab --- /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 0000000000..012daee05b --- /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 0000000000..460769fd04 --- /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 adb8e75f71..f8694629c1 100644 --- a/tests/support/support.bzl +++ b/tests/support/support.bzl @@ -54,3 +54,8 @@ SUPPORTS_BZLMOD_UNIXY = select({ "@platforms//os:windows": ["@platforms//:incompatible"], "//conditions:default": [], }) if BZLMOD_ENABLED else ["@platforms//:incompatible"] + +NOT_WINDOWS = select({ + "@platforms//os:windows": ["@platforms//:incompatible"], + "//conditions:default": [], +})