From 071b86d4e86cb3374bb6ee6b0fb3bdfd455d2150 Mon Sep 17 00:00:00 2001 From: Steven Casagrande Date: Fri, 19 Jul 2024 14:38:51 -0400 Subject: [PATCH 1/7] Fix finding runfiles dir --- python/private/stage1_bootstrap_template.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/python/private/stage1_bootstrap_template.sh b/python/private/stage1_bootstrap_template.sh index 959e7babe6..e621602045 100644 --- a/python/private/stage1_bootstrap_template.sh +++ b/python/private/stage1_bootstrap_template.sh @@ -44,7 +44,7 @@ else echo "$RUNFILES_DIR" return 0 elif [[ "${RUNFILES_MANIFEST_FILE:-}" = *".runfiles_manifest" ]]; then - echo "${RUNFILES_MANIFEST_FILE%%.runfiles_manifest}" + echo "${RUNFILES_MANIFEST_FILE%%.runfiles_manifest}.runfiles" return 0 elif [[ "${RUNFILES_MANIFEST_FILE:-}" = *".runfiles/MANIFEST" ]]; then echo "${RUNFILES_MANIFEST_FILE%%.runfiles/MANIFEST}" From 42967f4cc6091c052614552d8a99947282a45e4a Mon Sep 17 00:00:00 2001 From: Steven Casagrande Date: Thu, 5 Sep 2024 10:11:13 -0400 Subject: [PATCH 2/7] Expand fix to cover additional case --- python/private/stage1_bootstrap_template.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/python/private/stage1_bootstrap_template.sh b/python/private/stage1_bootstrap_template.sh index e621602045..bb29828386 100644 --- a/python/private/stage1_bootstrap_template.sh +++ b/python/private/stage1_bootstrap_template.sh @@ -47,7 +47,7 @@ else echo "${RUNFILES_MANIFEST_FILE%%.runfiles_manifest}.runfiles" return 0 elif [[ "${RUNFILES_MANIFEST_FILE:-}" = *".runfiles/MANIFEST" ]]; then - echo "${RUNFILES_MANIFEST_FILE%%.runfiles/MANIFEST}" + echo "${RUNFILES_MANIFEST_FILE%%.runfiles/MANIFEST}.runfiles" return 0 fi From 5f1230c533d56b3cf15bf8347da12ecbdd10b331 Mon Sep 17 00:00:00 2001 From: Richard Levasseur Date: Thu, 5 Sep 2024 09:11:53 -0700 Subject: [PATCH 3/7] add test --- python/private/stage1_bootstrap_template.sh | 1 - .../bootstrap_impls/run_binary_zip_no_test.sh | 50 +++++++++++++++---- 2 files changed, 40 insertions(+), 11 deletions(-) diff --git a/python/private/stage1_bootstrap_template.sh b/python/private/stage1_bootstrap_template.sh index bb29828386..e7e418cafb 100644 --- a/python/private/stage1_bootstrap_template.sh +++ b/python/private/stage1_bootstrap_template.sh @@ -57,7 +57,6 @@ else if [[ "$stub_filename" != /* ]]; then stub_filename="$PWD/$stub_filename" fi - while true; do module_space="${stub_filename}.runfiles" if [[ -d "$module_space" ]]; then diff --git a/tests/bootstrap_impls/run_binary_zip_no_test.sh b/tests/bootstrap_impls/run_binary_zip_no_test.sh index 2ee69f3f66..05a3db2d75 100755 --- a/tests/bootstrap_impls/run_binary_zip_no_test.sh +++ b/tests/bootstrap_impls/run_binary_zip_no_test.sh @@ -29,15 +29,45 @@ if [[ -z "$bin" ]]; then echo "Unable to locate test binary: $BIN_RLOCATION" exit 1 fi -actual=$($bin 2>&1) - -# How we detect if a zip file was executed from depends on which bootstrap -# is used. -# bootstrap_impl=script outputs RULES_PYTHON_ZIP_DIR= -# bootstrap_impl=system_python outputs file:.*Bazel.runfiles -expected_pattern="Hello" -if ! (echo "$actual" | grep "$expected_pattern" ) >/dev/null; then - echo "expected output to match: $expected_pattern" - echo "but got:\n$actual" + +function test_invocation() { + actual=$($bin) + # How we detect if a zip file was executed from depends on which bootstrap + # is used. + # bootstrap_impl=script outputs RULES_PYTHON_ZIP_DIR= + # bootstrap_impl=system_python outputs file:.*Bazel.runfiles + expected_pattern="Hello" + if ! (echo "$actual" | grep "$expected_pattern" ) >/dev/null; then + echo "expected output to match: $expected_pattern" + echo "but got:\n$actual" + exit 1 + fi +} + +# Test invocation with RUNFILES_DIR set +unset RUNFILES_MANIFEST_FILE +if [[ ! -e "$RUNFILES_DIR" ]]; then + echo "Runfiles doesn't exist: $RUNFILES_DIR" exit 1 fi +test_invocation + + +orig_runfiles_dir="$RUNFILES_DIR" +unset RUNFILES_DIR + +# Test invocation using manifest within runfiles directory (output manifest) +# Note that this file may not actually exist in our test, but that's OK; the +# bootstrap just uses the path to find the runfiles directory. +export RUNFILES_MANIFEST_FILE="$orig_runfiles_dir/MANIFEST" +test_invocation + +# Test invocation using manifest outside runfiles (input manifest) +# Note that this file may not actually exist in our test, but that's OK; the +# bootstrap just uses the path to find the runfiles directory. +export RUNFILES_MANIFEST_FILE="${orig_runfiles_dir%%.runfiles}.runfiles_manifest" +test_invocation + +# Test invocation without any runfiles env vars set +unset RUNFILES_MANIFEST_FILE +test_invocation From fa27b7fa94037eac4baca2ca5c958dd8eecda91e Mon Sep 17 00:00:00 2001 From: Richard Levasseur Date: Thu, 5 Sep 2024 09:13:18 -0700 Subject: [PATCH 4/7] fixup! add test --- tests/bootstrap_impls/run_binary_zip_no_test.sh | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/bootstrap_impls/run_binary_zip_no_test.sh b/tests/bootstrap_impls/run_binary_zip_no_test.sh index 05a3db2d75..cf2e8b43b4 100755 --- a/tests/bootstrap_impls/run_binary_zip_no_test.sh +++ b/tests/bootstrap_impls/run_binary_zip_no_test.sh @@ -57,13 +57,13 @@ orig_runfiles_dir="$RUNFILES_DIR" unset RUNFILES_DIR # Test invocation using manifest within runfiles directory (output manifest) -# Note that this file may not actually exist in our test, but that's OK; the +# NOTE: this file may not actually exist in our test, but that's OK; the # bootstrap just uses the path to find the runfiles directory. export RUNFILES_MANIFEST_FILE="$orig_runfiles_dir/MANIFEST" test_invocation # Test invocation using manifest outside runfiles (input manifest) -# Note that this file may not actually exist in our test, but that's OK; the +# NOTE: this file may not actually exist in our test, but that's OK; the # bootstrap just uses the path to find the runfiles directory. export RUNFILES_MANIFEST_FILE="${orig_runfiles_dir%%.runfiles}.runfiles_manifest" test_invocation From c23a174c09e967afe6d3bb26aea148ca739e73a5 Mon Sep 17 00:00:00 2001 From: Richard Levasseur Date: Thu, 5 Sep 2024 09:16:15 -0700 Subject: [PATCH 5/7] fixup! fixup! add test --- tests/bootstrap_impls/run_binary_zip_no_test.sh | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/tests/bootstrap_impls/run_binary_zip_no_test.sh b/tests/bootstrap_impls/run_binary_zip_no_test.sh index cf2e8b43b4..c45cae54cd 100755 --- a/tests/bootstrap_impls/run_binary_zip_no_test.sh +++ b/tests/bootstrap_impls/run_binary_zip_no_test.sh @@ -38,6 +38,7 @@ function test_invocation() { # bootstrap_impl=system_python outputs file:.*Bazel.runfiles expected_pattern="Hello" if ! (echo "$actual" | grep "$expected_pattern" ) >/dev/null; then + echo "Test case failed: $1" echo "expected output to match: $expected_pattern" echo "but got:\n$actual" exit 1 @@ -50,7 +51,7 @@ if [[ ! -e "$RUNFILES_DIR" ]]; then echo "Runfiles doesn't exist: $RUNFILES_DIR" exit 1 fi -test_invocation +test_invocation "using RUNFILES_DIR" orig_runfiles_dir="$RUNFILES_DIR" @@ -60,14 +61,14 @@ unset RUNFILES_DIR # NOTE: this file may not actually exist in our test, but that's OK; the # bootstrap just uses the path to find the runfiles directory. export RUNFILES_MANIFEST_FILE="$orig_runfiles_dir/MANIFEST" -test_invocation +test_invocation "using RUNFILES_MANIFEST_FILE with output manifest" # Test invocation using manifest outside runfiles (input manifest) # NOTE: this file may not actually exist in our test, but that's OK; the # bootstrap just uses the path to find the runfiles directory. export RUNFILES_MANIFEST_FILE="${orig_runfiles_dir%%.runfiles}.runfiles_manifest" -test_invocation +test_invocation "using RUNFILES_MANIFEST_FILE with input manifest" # Test invocation without any runfiles env vars set unset RUNFILES_MANIFEST_FILE -test_invocation +test_invocation "using no runfiles env vars" From 5888ed393e813a2d2d9705a1fd1d696e7e189842 Mon Sep 17 00:00:00 2001 From: Richard Levasseur Date: Thu, 5 Sep 2024 09:54:16 -0700 Subject: [PATCH 6/7] update changelog --- CHANGELOG.md | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 6a901bb236..40d1a507aa 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -44,6 +44,10 @@ A brief description of the categories of changes: stage2 bootstrap template. * (bzlmod) Properly handle relative path URLs in parse_simpleapi_html.bzl * (gazelle) Correctly resolve deps that have top-level module overlap with a gazelle_python.yaml dep module +* (rules) Make `RUNFILES_MANIFEST_FILE`-based invocations work when used with + {obj}`--bootstrap_impl=script`. + ([#2186](https://github.com/bazelbuild/rules_python/issues/2186)). + ### Added * (rules) Executables provide {obj}`PyExecutableInfo`, which contains From 53862d9635b27fa9c2073d318a0819d3ec625c8d Mon Sep 17 00:00:00 2001 From: Richard Levasseur Date: Thu, 5 Sep 2024 09:55:25 -0700 Subject: [PATCH 7/7] fixup! update changelog --- CHANGELOG.md | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 40d1a507aa..e468e2bd91 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -45,7 +45,8 @@ A brief description of the categories of changes: * (bzlmod) Properly handle relative path URLs in parse_simpleapi_html.bzl * (gazelle) Correctly resolve deps that have top-level module overlap with a gazelle_python.yaml dep module * (rules) Make `RUNFILES_MANIFEST_FILE`-based invocations work when used with - {obj}`--bootstrap_impl=script`. + {obj}`--bootstrap_impl=script`. This fixes invocations using non-sandboxed + test execution with `--enable_runfiles=false --build_runfile_manifests=true`. ([#2186](https://github.com/bazelbuild/rules_python/issues/2186)).