diff --git a/CHANGELOG.md b/CHANGELOG.md index 189e46f3f8..82d58a9c07 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -67,6 +67,7 @@ END_UNRELEASED_TEMPLATE * (zipapp) Resolve issue passing through compression settings in `py_zippapp_binary` targets ([#3646](https://github.com/bazel-contrib/rules_python/issues/3646)). +* (bootstrap) Fixed incorrect runfiles path construction in bootstrap scripts when binary is defined in another bazel module {#v0-0-0-added} ### Added diff --git a/examples/bzlmod/other_module/other_module/pkg/BUILD.bazel b/examples/bzlmod/other_module/other_module/pkg/BUILD.bazel index 53344c708a..318822d888 100644 --- a/examples/bzlmod/other_module/other_module/pkg/BUILD.bazel +++ b/examples/bzlmod/other_module/other_module/pkg/BUILD.bazel @@ -1,5 +1,6 @@ load("@rules_python//python:py_binary.bzl", "py_binary") load("@rules_python//python:py_library.bzl", "py_library") +load("@rules_python//python/zipapp:py_zipapp_binary.bzl", "py_zipapp_binary") py_library( name = "lib", @@ -26,4 +27,13 @@ py_binary( ], ) +# This is used for regression testing runfiles paths in submodules. +# https://github.com/bazel-contrib/rules_python/issues/3563. +py_zipapp_binary( + name = "bin_zipapp", + testonly = True, + binary = ":bin", + visibility = ["//visibility:public"], +) + exports_files(["data/data.txt"]) diff --git a/examples/bzlmod/tests/other_module/BUILD.bazel b/examples/bzlmod/tests/other_module/BUILD.bazel index 1bd8a900a9..1e657c8431 100644 --- a/examples/bzlmod/tests/other_module/BUILD.bazel +++ b/examples/bzlmod/tests/other_module/BUILD.bazel @@ -5,6 +5,7 @@ # in the root module. load("@bazel_skylib//rules:build_test.bzl", "build_test") +load("@rules_python//python:py_test.bzl", "py_test") build_test( name = "other_module_bin_build_test", @@ -12,3 +13,10 @@ build_test( "@our_other_module//other_module/pkg:bin", ], ) + +py_test( + name = "other_module_import_test", + srcs = ["other_module_import_test.py"], + data = ["@our_other_module//other_module/pkg:bin_zipapp"], + env = {"ZIPAPP_PATH": "$(location @our_other_module//other_module/pkg:bin_zipapp)"}, +) diff --git a/examples/bzlmod/tests/other_module/other_module_import_test.py b/examples/bzlmod/tests/other_module/other_module_import_test.py new file mode 100644 index 0000000000..6b92a853e0 --- /dev/null +++ b/examples/bzlmod/tests/other_module/other_module_import_test.py @@ -0,0 +1,22 @@ +"""Regression test for https://github.com/bazel-contrib/rules_python/issues/3563""" +import os +import subprocess +import sys + +def main(): + # The rlocation path for the bin_zipapp. It is in the "our_other_module" repository. + zipapp_path = os.environ.get("ZIPAPP_PATH") + print(f"Running bin_zipapp at: {zipapp_path}") + + result = subprocess.run([zipapp_path], capture_output=True, text=True) + print("--- bin_zippapp stdout ---") + print(result.stdout) + print("--- bin_zippapp stderr ---") + print(result.stderr) + + if result.returncode != 0: + print(f"bin_zippapp failed with return code {result.returncode}") + sys.exit(result.returncode) + +if __name__ == "__main__": + main() diff --git a/python/private/py_executable.bzl b/python/private/py_executable.bzl index 284aea6bff..495c7dddcf 100644 --- a/python/private/py_executable.bzl +++ b/python/private/py_executable.bzl @@ -510,10 +510,7 @@ def _create_zip_main(ctx, *, stage2_bootstrap, runtime_details, venv): substitutions = { "%python_binary%": python_binary, "%python_binary_actual%": python_binary_actual, - "%stage2_bootstrap%": "{}/{}".format( - ctx.workspace_name, - stage2_bootstrap.short_path, - ), + "%stage2_bootstrap%": runfiles_root_path(ctx, stage2_bootstrap.short_path), "%workspace_name%": ctx.workspace_name, }, ) @@ -616,7 +613,7 @@ def _create_venv(ctx, output_prefix, imports, runtime_details, add_runfiles_root "%add_runfiles_root_to_sys_path%": add_runfiles_root_to_sys_path, "%coverage_tool%": _get_coverage_tool_runfiles_path(ctx, runtime), "%import_all%": "True" if read_possibly_native_flag(ctx, "python_import_all_repositories") else "False", - "%site_init_runfiles_path%": "{}/{}".format(ctx.workspace_name, site_init.short_path), + "%site_init_runfiles_path%": runfiles_root_path(ctx, site_init.short_path), "%workspace_name%": ctx.workspace_name, }, computed_substitutions = computed_subs, @@ -671,10 +668,7 @@ def _get_coverage_tool_runfiles_path(ctx, runtime): if (ctx.configuration.coverage_enabled and runtime and runtime.coverage_tool): - return "{}/{}".format( - ctx.workspace_name, - runtime.coverage_tool.short_path, - ) + return runfiles_root_path(ctx, runtime.coverage_tool.short_path) else: return "" @@ -777,10 +771,7 @@ def _create_stage1_bootstrap( if (ctx.configuration.coverage_enabled and runtime and runtime.coverage_tool): - coverage_tool_runfiles_path = "{}/{}".format( - ctx.workspace_name, - runtime.coverage_tool.short_path, - ) + coverage_tool_runfiles_path = runfiles_root_path(ctx, runtime.coverage_tool.short_path) else: coverage_tool_runfiles_path = "" if runtime: @@ -793,7 +784,7 @@ def _create_stage1_bootstrap( subs["%coverage_tool%"] = coverage_tool_runfiles_path subs["%import_all%"] = ("True" if read_possibly_native_flag(ctx, "python_import_all_repositories") else "False") subs["%imports%"] = ":".join(imports.to_list()) - subs["%main%"] = "{}/{}".format(ctx.workspace_name, main_py.short_path) + subs["%main%"] = runfiles_root_path(ctx, main_py.short_path) ctx.actions.expand_template( template = template,