-
-
Notifications
You must be signed in to change notification settings - Fork 717
feat(rules): emit RunfilesGroupInfo from py_binary, py_test, and py_runtime
#3962
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.
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
base: main
Are you sure you want to change the base?
Changes from all commits
75ab18b
1d1dcc2
fcba61a
c37ba6d
e481a9f
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,10 @@ | ||
| (rules) Added experimental support for the `RunfilesGroupInfo` provider from | ||
| [`rules_runfiles_group`](https://github.com/bazel-contrib/rules_runfiles_group). | ||
| When enabled (via `--@rules_runfiles_group//runfiles_group:enabled` or | ||
|
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Posted by Fable 5.1 on behalf of @FrankPortman requesting a Claude review. minor / docs. Advertises |
||
| {obj}`--@rules_python//python/config_settings:runfiles_groups`), `py_binary` | ||
|
Comment on lines
+3
to
+4
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think we should advertise only one - the |
||
| and `py_test` split their runfiles into named groups (interpreter plus | ||
| stdlib, one group per dependency, the binary-specific venv, and the binary's | ||
| own code) so packaging rules can build layered container images with a | ||
| shared interpreter layer and one layer per package | ||
| ([#3757](https://github.com/bazel-contrib/rules_python/issues/3757)). | ||
| Requires Bazel 9+. | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -41,6 +41,28 @@ config = struct( | |
| ) | ||
| """ | ||
|
|
||
| # rules_runfiles_group isn't in Bazel 8's hardcoded list of repositories that | ||
| # are exempt from autoloads, so loading its bzl files from rules_python creates | ||
| # a load cycle there (https://github.com/bazelbuild/bazel/issues/23043). This | ||
| # repo is on that list, so the load is routed through here and stubbed out on | ||
| # Bazel versions with autoloads. | ||
| _RUNFILES_GROUPS_SHIM_TEMPLATE = """ | ||
| load("@rules_runfiles_group//runfiles_group:lib.bzl", _runfiles_groups = "runfiles_groups") | ||
| load("@rules_runfiles_group//runfiles_group:providers.bzl", _RunfilesGroupInfo = "RunfilesGroupInfo") | ||
|
|
||
| RUNFILES_GROUPS_AVAILABLE = True | ||
| runfiles_groups = _runfiles_groups | ||
| RunfilesGroupInfo = _RunfilesGroupInfo | ||
| ENABLED_FLAG_LABEL = "@rules_runfiles_group//runfiles_group:enabled" | ||
| """ | ||
|
|
||
| _RUNFILES_GROUPS_SHIM_UNAVAILABLE_TEMPLATE = """ | ||
| RUNFILES_GROUPS_AVAILABLE = False | ||
| runfiles_groups = None | ||
| RunfilesGroupInfo = None | ||
| ENABLED_FLAG_LABEL = "//python:none" | ||
| """ | ||
|
|
||
| ROOT_BUILD_TEMPLATE = """ | ||
| load("@bazel_skylib//:bzl_library.bzl", "bzl_library") | ||
|
|
||
|
|
@@ -59,6 +81,12 @@ bzl_library( | |
| name = "rules_python_config", | ||
| srcs = ["rules_python_config.bzl"], | ||
| ) | ||
|
|
||
| bzl_library( | ||
| name = "runfiles_groups_shim", | ||
| srcs = ["runfiles_groups_shim.bzl"], | ||
| deps = [{runfiles_groups_shim_deps}], | ||
| ) | ||
| """ | ||
|
|
||
| _EXTRA_TRANSITIONS_TEMPLATE = """ | ||
|
|
@@ -117,8 +145,17 @@ def _internal_config_repo_impl(rctx): | |
| bazel_10_or_later = str(bazel_major_version > 9), | ||
| )) | ||
|
|
||
| if bazel_major_version >= 9: | ||
|
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Posted by Fable 5.1 on behalf of @FrankPortman requesting a Claude review. minor / hermeticity (acceptable as-is, flagging for the record). The shim is keyed on |
||
| runfiles_groups_shim = _RUNFILES_GROUPS_SHIM_TEMPLATE | ||
| runfiles_groups_shim_deps = '"@rules_runfiles_group//runfiles_group:lib", "@rules_runfiles_group//runfiles_group:providers"' | ||
| else: | ||
| runfiles_groups_shim = _RUNFILES_GROUPS_SHIM_UNAVAILABLE_TEMPLATE | ||
| runfiles_groups_shim_deps = "" | ||
| rctx.file("runfiles_groups_shim.bzl", runfiles_groups_shim) | ||
|
|
||
| rctx.file("BUILD", ROOT_BUILD_TEMPLATE.format( | ||
| visibility = "@rules_python//:__subpackages__", | ||
| runfiles_groups_shim_deps = runfiles_groups_shim_deps, | ||
| )) | ||
|
|
||
| rctx.file( | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -68,6 +68,21 @@ load(":py_internal.bzl", "py_internal") | |
| load(":py_runtime_info.bzl", "DEFAULT_STUB_SHEBANG") | ||
| load(":reexports.bzl", "BuiltinPyInfo", "BuiltinPyRuntimeInfo") | ||
| load(":rule_builders.bzl", "ruleb") | ||
| load( | ||
| ":runfiles_groups.bzl", | ||
| "APP_GROUP", | ||
| "RUNFILES_GROUP_ENABLED_LABEL", | ||
| "app_entry", | ||
| "build_entries_depset", | ||
| "collect_data_entries", | ||
| "collect_dep_entries", | ||
| "collect_src_entries", | ||
| "create_runfiles_group_info", | ||
| "pyc_collection_enabled_by_default", | ||
| "runtime_entry", | ||
| "venv_entry", | ||
| runfiles_groups_enabled = "is_enabled", | ||
| ) | ||
| load(":toolchain_types.bzl", "CC_TOOLCHAIN_TYPE", "EXEC_TOOLS_TOOLCHAIN_TYPE", "LAUNCHER_MAKER_TOOLCHAIN_TYPE", TOOLCHAIN_TYPE = "TARGET_TOOLCHAIN_TYPE") | ||
| load(":transition_labels.bzl", "TRANSITION_LABELS") | ||
| load(":venv_runfiles.bzl", "create_venv_app_files") | ||
|
|
@@ -91,6 +106,8 @@ EXECUTABLE_ATTRS = dicts.add( | |
| "_default_to_explicit_init_py_flag": attr.label(default = "//python/config_settings:incompatible_default_to_explicit_init_py"), | ||
| "_python_import_all_repositories_flag": attr.label(default = "//python/config_settings:experimental_python_import_all_repositories"), | ||
| "_python_path_flag": attr.label(default = "//python/config_settings:python_path"), | ||
| "_runfiles_group_enabled": attr.label(default = RUNFILES_GROUP_ENABLED_LABEL), | ||
| "_runfiles_groups_flag": attr.label(default = labels.RUNFILES_GROUPS), | ||
| }, | ||
| { | ||
| "interpreter_args": lambda: attrb.StringList( | ||
|
|
@@ -519,6 +536,11 @@ WARNING: Target: {} | |
| # depset[ExplicitSymlink]None; symlinks that should be created in | ||
| # the venv to augment app_runfiles | ||
| venv_app_symlinks = venv.lib_symlinks if venv else None, | ||
| # runfiles|None; the non-interpreter venv files: site-packages | ||
| # symlinks and small generated files (pth, site init, pyvenv.cfg). | ||
| venv_files_runfiles = ( | ||
|
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Posted by Fable 5.1 on behalf of @FrankPortman requesting a Claude review. minor / performance. |
||
| ctx.runfiles(venv.files_without_interpreter).merge(venv.lib_runfiles) if venv else None | ||
| ), | ||
| # File|None; the venv `bin/python3` file, if any. | ||
| venv_python_exe = venv.interpreter if venv else None, | ||
| # runfiles|None; runfiles in the venv for the interpreter | ||
|
|
@@ -1322,8 +1344,119 @@ def py_executable_base_impl(ctx, *, semantics, is_test, inherited_environment = | |
| _maybe_add_test_main_validation(ctx, main_py_source, output_groups) | ||
| _add_provider_output_group_info(providers, py_info, output_groups) | ||
|
|
||
| if runfiles_groups_enabled(ctx): | ||
| providers.append(_create_runfiles_groups( | ||
| ctx, | ||
| executable = executable, | ||
| runtime_details = runtime_details, | ||
| runfiles_details = runfiles_details, | ||
| exec_result = exec_result, | ||
| cc_details = cc_details, | ||
| native_deps_details = native_deps_details, | ||
| extra_deps = extra_deps, | ||
| required_py_files = required_py_files, | ||
| required_pyc_files = required_pyc_files, | ||
| implicit_pyc_files = implicit_pyc_files, | ||
| implicit_pyc_source_files = implicit_pyc_source_files, | ||
| )) | ||
|
|
||
| return providers | ||
|
|
||
| def _create_runfiles_groups( | ||
|
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Posted by Fable 5.1 on behalf of @FrankPortman requesting a Claude review. major / structure. Groups are produced by re-enumerating every runfiles contribution (required files, pyc variant, Suggested direction: build the runtime/venv/app/dep pieces once in |
||
| ctx, | ||
| *, | ||
| executable, | ||
| runtime_details, | ||
| runfiles_details, | ||
| exec_result, | ||
| cc_details, | ||
| native_deps_details, | ||
| extra_deps, | ||
| required_py_files, | ||
| required_pyc_files, | ||
| implicit_pyc_files, | ||
| implicit_pyc_source_files): | ||
| """Creates the public RunfilesGroupInfo provider. | ||
|
|
||
| The union of all groups equals `DefaultInfo.default_runfiles` exactly; | ||
| packaging rules depend on that invariant. See runfiles_groups.bzl for | ||
| how the groups fit together. | ||
| """ | ||
| pyc_collection_enabled = PycCollectionAttr.is_pyc_collection_enabled(ctx) | ||
|
|
||
| # Dependencies compute their entries against the configuration | ||
| # (see pyc_collection_enabled_by_default). If this binary's | ||
| # `pyc_collection` attribute overrides that, the dependency-provided | ||
| # entries don't match what this binary adds to its runfiles, so fall | ||
| # back to coarse grouping. The same applies to the deprecated implicit | ||
| # `__init__.py` creation, whose synthesized empty files only exist in | ||
| # `app_runfiles`. | ||
| fine_grained = ( | ||
| pyc_collection_enabled == pyc_collection_enabled_by_default(ctx) and | ||
| not _should_create_init_files(ctx) | ||
| ) | ||
|
|
||
| collected = [] | ||
| if fine_grained: | ||
| dep_entries = collect_dep_entries( | ||
| ctx, | ||
| ctx.attr.deps + extra_deps, | ||
| pyc_collection_enabled = pyc_collection_enabled, | ||
| ) | ||
| src_entries = collect_src_entries(ctx.attr.srcs) | ||
| data_entries = collect_data_entries(ctx.attr.data) | ||
| collected = [dep_entries, src_entries, data_entries] | ||
|
|
||
| app_files = builders.DepsetBuilder() | ||
| app_files.add(required_py_files) | ||
| app_files.add(required_pyc_files) | ||
| if pyc_collection_enabled: | ||
| app_files.add(implicit_pyc_files) | ||
| else: | ||
| app_files.add(implicit_pyc_source_files) | ||
| app_files.add(data_entries.own_files) | ||
| app_files.add(exec_result.extra_default_outputs) | ||
| app_files.add(executable) | ||
| if exec_result.stage2_bootstrap: | ||
| app_files.add(exec_result.stage2_bootstrap) | ||
| if runfiles_details.build_data_file: | ||
| app_files.add(runfiles_details.build_data_file) | ||
| app_runfiles = ctx.runfiles(transitive_files = app_files.build()) | ||
| app_runfiles = app_runfiles.merge_all([ | ||
| cc_details.extra_runfiles, | ||
| native_deps_details.runfiles, | ||
| ]) | ||
| else: | ||
| # `app_runfiles` is everything except the runtime and venv: | ||
| # the binary's own sources, all dependencies, and data. | ||
| coarse_files = builders.DepsetBuilder() | ||
| coarse_files.add(exec_result.extra_default_outputs) | ||
| coarse_files.add(executable) | ||
| if exec_result.stage2_bootstrap: | ||
| coarse_files.add(exec_result.stage2_bootstrap) | ||
| if runfiles_details.build_data_file: | ||
| coarse_files.add(runfiles_details.build_data_file) | ||
| app_runfiles = ctx.runfiles(transitive_files = coarse_files.build()) | ||
| app_runfiles = app_runfiles.merge(runfiles_details.app_runfiles) | ||
|
|
||
| direct = [app_entry(app_runfiles)] | ||
|
|
||
| runtime_runfiles = runtime_details.runfiles | ||
| if exec_result.venv_interpreter_runfiles: | ||
| runtime_runfiles = runtime_runfiles.merge( | ||
| exec_result.venv_interpreter_runfiles, | ||
| ) | ||
| direct.append(runtime_entry(runtime_runfiles)) | ||
|
|
||
| if exec_result.venv_files_runfiles: | ||
| direct.append(venv_entry(exec_result.venv_files_runfiles)) | ||
|
|
||
| collected.append(struct(direct = direct, transitive = [])) | ||
| return create_runfiles_group_info( | ||
| build_entries_depset(*collected), | ||
| executable_group = APP_GROUP, | ||
| ) | ||
|
|
||
| def _maybe_add_test_main_validation(ctx, main_py, output_groups): | ||
| """Adds a validation action that checks the test main actually runs tests. | ||
|
|
||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Posted by Fable 5.1 on behalf of @FrankPortman requesting a Claude review.
minor / docs. "That exactness has two exceptions" is inaccurate: the union contract still holds in coarse mode (verified); what degrades is granularity. Reword to "falls back to coarse grouping". Relatedly, the
collect_data_entriesdocstring's "Known approximations" list describes a hypothetical gap while omitting the real one (default outputs ofdatarule targets).