Skip to content

fix: manual runfiles path construction when using submodules #3636

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
faximan wants to merge 2 commits into
base: main
Choose a base branch
from
+26 −14
Open

fix: manual runfiles path construction when using submodules #3636

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
34e4d28
Fix manual runfiles path construction
faximan Feb 24, 2026
eb6ac47
Add entry to CHANGELOG
faximan Feb 24, 2026
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
21 changes: 21 additions & 0 deletions CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,27 @@ 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-removed}
### Removed
* Nothing removed.

{#v0-0-0-changed}
### Changed
* Nothing changed.

{#v0-0-0-fixed}
### Fixed
* (bootstrap) Fixed incorrect runfiles path construction in bootstrap scripts when binary is defined in another bazel module

{#v0-0-0-added}
### Added
* Nothing added.

{#v1-9-0}
## [1.9.0] - 2026-02-21

Expand Down
19 changes: 5 additions & 14 deletions python/private/py_executable.bzl
View file
Open in desktop
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(
Copy link
Collaborator

@aignas aignas Feb 25, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would it be possible to add a test somewhere? For example examples/bzlmod has submodules in it which could ensure that we can still import things.

@rickeylev Any other ideas how we could do this?

Copy link
Author

@faximan faximan Feb 25, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I tried and failed to write a test here tests the specific fix in this PR. As I'm very unfamiliar with this code base in general, I am afraid I'd need support to land that.

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