Skip to content

fix: make executable implicit defaults public so other modules can use rule builders#3919

Merged
rickeylev merged 2 commits into
bazel-contrib:mainfrom
chicagotrading:fix/private-targets-visibility-for-rule-builders
Jul 12, 2026
Merged

fix: make executable implicit defaults public so other modules can use rule builders#3919
rickeylev merged 2 commits into
bazel-contrib:mainfrom
chicagotrading:fix/private-targets-visibility-for-rule-builders

Conversation

@13steinj

@13steinj 13steinj commented Jul 11, 2026

Copy link
Copy Markdown
Contributor

create_executable_rule_builder() (used by py_binary_rule_builder() /
py_test_rule_builder(), the public python/api/executables.bzl API)
bundles three implicit attrs whose defaults point at private targets
under //python/private: build_data_writer,
debugger_if_target_config, and uncachable_version_file. These
targets had no explicit visibility, so they inherited the package's
default_visibility (//:__subpackages__), which only covers packages
inside the rules_python repo itself.

Because the builder API is designed to let external modules call
rule() themselves (via builder.build()), Bazel checks visibility of
these attr defaults from the calling module's package, not from
rules_python's. Any external repo constructing a rule via
py_binary_rule_builder() / py_test_rule_builder() therefore fails at
analysis time with a visibility error.

This has been broken since the builder API's introduction; there's prior
art in this same file for exactly this situation (e.g.
stage1_bootstrap_template, among others)

build_data_writer, debugger_if_target_config, and uncachable_version_file
are added as implicit attrs to all py_binary/py_test targets via
EXECUTABLE_ATTRS in 1.9.0. When an external package uses
py_binary_rule_builder() to construct a custom rule, Bazel checks
visibility of those attr defaults from the external package. These
targets inherited the package default (//python/private:__subpackages__)
making them inaccessible, causing a visibility error at analysis time.

Set visibility = ["//visibility:public"] on the three targets.
@13steinj 13steinj force-pushed the fix/private-targets-visibility-for-rule-builders branch from a3f1a05 to dd3f1f5 Compare July 11, 2026 02:39
@13steinj 13steinj marked this pull request as ready for review July 11, 2026 02:39

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Code Review

This pull request resolves a visibility issue where implicit attribute defaults used by py_binary_rule_builder() and py_test_rule_builder() were private to rules_python, causing analysis failures for external modules. The changes expose these targets as public and add regression tests. The review feedback suggests forwarding the visibility parameter to uncachable_version_file_impl to prevent failures when stamping is enabled, and cleaning up placeholder TODO(pr-number) comments in the test files.

Important

The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.

Comment thread python/private/uncachable_version_file.bzl
Comment thread examples/bzlmod/other_module/other_module/rule_builder/rule.bzl
Comment thread examples/bzlmod/tests/other_module/BUILD.bazel
@rickeylev rickeylev changed the title fix: expose private EXECUTABLE_ATTRS defaults to external rule builders fix: make executable implicit defaults public so other modules can use rule builders Jul 11, 2026
@rickeylev

Copy link
Copy Markdown
Collaborator

This looks great. Thanks for the fix!

@rickeylev rickeylev enabled auto-merge July 11, 2026 17:11
@rickeylev rickeylev added this pull request to the merge queue Jul 12, 2026
Merged via the queue into bazel-contrib:main with commit 19ffd21 Jul 12, 2026
6 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants