diff --git a/.bazelrc.deleted_packages b/.bazelrc.deleted_packages index aca014bb56..db42040b5c 100644 --- a/.bazelrc.deleted_packages +++ b/.bazelrc.deleted_packages @@ -7,6 +7,7 @@ common --deleted_packages=examples/bzlmod/entry_points/tests common --deleted_packages=examples/bzlmod/libs/my_lib common --deleted_packages=examples/bzlmod/other_module common --deleted_packages=examples/bzlmod/other_module/other_module/pkg +common --deleted_packages=examples/bzlmod/other_module/other_module/rule_builder common --deleted_packages=examples/bzlmod/patches common --deleted_packages=examples/bzlmod/runfiles common --deleted_packages=examples/bzlmod/tests diff --git a/examples/bzlmod/other_module/other_module/rule_builder/BUILD.bazel b/examples/bzlmod/other_module/other_module/rule_builder/BUILD.bazel new file mode 100644 index 0000000000..40f66b3322 --- /dev/null +++ b/examples/bzlmod/other_module/other_module/rule_builder/BUILD.bazel @@ -0,0 +1,13 @@ +load(":rule.bzl", "custom_py_binary") + +# This target's mere existence is the regression test: analyzing it exercises +# the implicit attr defaults (build_data_writer, debugger_if_target_config, +# uncachable_version_file) that py_binary_rule_builder() bundles from +# rules_python's private package, from a rule() call made in this external +# module. +custom_py_binary( + name = "app", + srcs = ["app.py"], + main = "app.py", + visibility = ["//visibility:public"], +) diff --git a/examples/bzlmod/other_module/other_module/rule_builder/app.py b/examples/bzlmod/other_module/other_module/rule_builder/app.py new file mode 100644 index 0000000000..db51494ed6 --- /dev/null +++ b/examples/bzlmod/other_module/other_module/rule_builder/app.py @@ -0,0 +1 @@ +print("hello from a rule built via py_binary_rule_builder()") diff --git a/examples/bzlmod/other_module/other_module/rule_builder/rule.bzl b/examples/bzlmod/other_module/other_module/rule_builder/rule.bzl new file mode 100644 index 0000000000..7b1505987a --- /dev/null +++ b/examples/bzlmod/other_module/other_module/rule_builder/rule.bzl @@ -0,0 +1,17 @@ +"""A minimal custom py_binary built via the executables rule builder API. + +Regression coverage for https://github.com/bazel-contrib/rules_python/pull/3919: +py_binary_rule_builder()'s implicit attr defaults (build_data_writer, +debugger_if_target_config, uncachable_version_file) previously had no +visibility outside of rules_python, so any external module (like this one) +constructing a rule via the builder failed at analysis time with a +visibility error. +""" + +load("@rules_python//python/api:executables.bzl", "executables") + +def _make_rule(): + builder = executables.py_binary_rule_builder() + return builder.build() + +custom_py_binary = _make_rule() diff --git a/examples/bzlmod/tests/other_module/BUILD.bazel b/examples/bzlmod/tests/other_module/BUILD.bazel index 24231e651a..f4756e7c3e 100644 --- a/examples/bzlmod/tests/other_module/BUILD.bazel +++ b/examples/bzlmod/tests/other_module/BUILD.bazel @@ -14,6 +14,17 @@ build_test( ], ) +# Regression test for https://github.com/bazel-contrib/rules_python/pull/3919: py_binary_rule_builder()'s implicit +# attr defaults previously had no visibility outside of rules_python, so +# building this target (defined via the builder from an external module) +# failed at analysis time with a visibility error. +build_test( + name = "other_module_rule_builder_build_test", + targets = [ + "@our_other_module//other_module/rule_builder:app", + ], +) + py_test( name = "other_module_import_test", srcs = ["other_module_import_test.py"], diff --git a/news/3919.fixed.md b/news/3919.fixed.md new file mode 100644 index 0000000000..0e45d3ce9c --- /dev/null +++ b/news/3919.fixed.md @@ -0,0 +1,3 @@ +Fixed `py_binary_rule_builder()` / `py_test_rule_builder()` (from `python/api/executables.bzl`) +failing at analysis time with a visibility error when used to construct a custom rule from an +external module. diff --git a/python/private/BUILD.bazel b/python/private/BUILD.bazel index e4d06d4a89..60667103e6 100644 --- a/python/private/BUILD.bazel +++ b/python/private/BUILD.bazel @@ -69,10 +69,16 @@ alias( "@platforms//os:windows": ":build_data_writer.ps1", "//conditions:default": ":build_data_writer.sh", }), + # Not actually public. Only public because it's an implicit dependency of + # rules built via py_binary_rule_builder() / py_test_rule_builder(). + visibility = ["//visibility:public"], ) define_uncachable_version_file( name = "uncachable_version_file", + # Not actually public. Only public because it's an implicit dependency of + # rules built via py_binary_rule_builder() / py_test_rule_builder(). + visibility = ["//visibility:public"], ) # Needed to define bzl_library targets for docgen. (We don't define the @@ -159,6 +165,9 @@ alias( ":is_bazel_config_mode_target": "//python/config_settings:debugger", "//conditions:default": "//python/private:empty", }), + # Not actually public. Only public because it's an implicit dependency of + # rules built via py_binary_rule_builder() / py_test_rule_builder(). + visibility = ["//visibility:public"], ) bazel_config_mode(name = "bazel_config_mode") @@ -208,10 +217,18 @@ current_interpreter_executable( py_library( name = "empty", + # Not actually public. Only public because it's the resolved default of + # debugger_if_target_config, an implicit dependency of rules built via + # py_binary_rule_builder() / py_test_rule_builder(). + visibility = ["//visibility:public"], ) sentinel( name = "sentinel", + # Not actually public. Only public because it's the resolved default of + # uncachable_version_file, an implicit dependency of rules built via + # py_binary_rule_builder() / py_test_rule_builder(). + visibility = ["//visibility:public"], ) py_binary( diff --git a/python/private/uncachable_version_file.bzl b/python/private/uncachable_version_file.bzl index 9b1d65a469..0f58df8043 100644 --- a/python/private/uncachable_version_file.bzl +++ b/python/private/uncachable_version_file.bzl @@ -28,12 +28,13 @@ actions depending on this file will always re-run. implementation = _uncachable_version_file_impl, ) -def define_uncachable_version_file(name): +def define_uncachable_version_file(name, visibility = None): native.alias( name = name, actual = select({ ":stamp_detect": ":uncachable_version_file_impl", "//conditions:default": ":sentinel", }), + visibility = visibility, ) uncachable_version_file(name = "uncachable_version_file_impl")