Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -80,6 +80,9 @@ Other changes:
configuration. See the {any}`RULES_PYTHON_PYCACHE_DIR` environment variable
for more information.
([#3643](https://github.com/bazel-contrib/rules_python/issues/3643)).
* (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)).

{#v0-0-0-added}
### Added
Expand Down
10 changes: 10 additions & 0 deletions examples/bzlmod/other_module/other_module/pkg/BUILD.bazel
Original file line number Diff line number Diff line change
@@ -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",
Expand All @@ -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"])
14 changes: 14 additions & 0 deletions examples/bzlmod/tests/other_module/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -5,10 +5,24 @@
# 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",
targets = [
"@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)"},
# For now, skip this test on Windows because it fails for reasons
# other than the code path being tested.
target_compatible_with = select({
"@platforms//os:windows": ["@platforms//:incompatible"],
"//conditions:default": [],
}),
)
22 changes: 22 additions & 0 deletions examples/bzlmod/tests/other_module/other_module_import_test.py
Original file line number Diff line number Diff line change
@@ -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()
19 changes: 5 additions & 14 deletions python/private/py_executable.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -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,
},
)
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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 ""

Expand Down Expand Up @@ -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:
Expand All @@ -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,
Expand Down