From e393cc55aabee371bc00ec438fed2ebea1201eae Mon Sep 17 00:00:00 2001 From: Frank Portman Date: Sun, 25 Oct 2026 17:24:56 -0400 Subject: [PATCH] feat: add PyInfo.direct_implicit_pyc_files and PyExecutableInfo.venv_runfiles PyInfo had direct counterparts for every pyc field except the implicit pyc files, and PyExecutableInfo had the venv's interpreter runfiles but not the application's. Tools that only see providers, like a packager's aspect, need both to tell a target's own files apart from what it inherits. Also pulls a few internals out so they can be reused without change in behavior: py_info_runfiles_files() for the files a binary takes from a dependency's PyInfo, get_runtime_files() for the runtime files, and get_package_and_version() taking the data files instead of the ctx. Split out of #4254. --- news/3757-provider-fields.added.md | 2 + python/private/common.bzl | 23 ++++++ python/private/py_executable.bzl | 71 ++++++++++--------- python/private/py_executable_info.bzl | 10 +++ python/private/py_info.bzl | 19 +++++ python/private/py_library.bzl | 16 ++++- .../precompile/precompile_tests.bzl | 24 +++++++ tests/base_rules/py_executable_base_tests.bzl | 23 ++++++ tests/base_rules/py_info/py_info_tests.bzl | 11 +++ tests/support/py_executable_info_subject.bzl | 8 +++ tests/support/py_info_subject.bzl | 11 +++ 11 files changed, 182 insertions(+), 36 deletions(-) create mode 100644 news/3757-provider-fields.added.md diff --git a/news/3757-provider-fields.added.md b/news/3757-provider-fields.added.md new file mode 100644 index 0000000000..f854306a4e --- /dev/null +++ b/news/3757-provider-fields.added.md @@ -0,0 +1,2 @@ +(rules) Added {obj}`PyInfo.direct_implicit_pyc_files` and +{obj}`PyExecutableInfo.venv_runfiles`. diff --git a/python/private/common.bzl b/python/private/common.bzl index 0a2d8a115b..28f33edf90 100644 --- a/python/private/common.bzl +++ b/python/private/common.bzl @@ -283,6 +283,28 @@ def get_imports(ctx): result.append(import_path) return result +def py_info_runfiles_files(info, pyc_collection_enabled): + """The files a binary puts into its runfiles for a dependency's PyInfo. + + Args: + info: {type}`PyInfo | BuiltinPyInfo` of a dependency. + pyc_collection_enabled: {type}`bool` whether the binary includes + implicit pyc files instead of their sources. + + Returns: + {type}`list[depset[File]]` + """ + files = [info.transitive_sources] + + # BuiltinPyInfo doesn't have the pyc fields + if hasattr(info, "transitive_pyc_files"): + files.append(info.transitive_pyc_files) + if pyc_collection_enabled: + files.append(info.transitive_implicit_pyc_files) + else: + files.append(info.transitive_implicit_pyc_source_files) + return files + def collect_runfiles(ctx, files = depset()): """Collects the necessary files from the rule's context. @@ -392,6 +414,7 @@ def create_py_info( py_info.transitive_original_sources.add(original_sources) py_info.transitive_pyc_files.add(required_pyc_files) py_info.transitive_pyi_files.add(ctx.files.pyi_srcs) + py_info.direct_implicit_pyc_files.add(implicit_pyc_files) py_info.transitive_implicit_pyc_files.add(implicit_pyc_files) py_info.transitive_implicit_pyc_source_files.add(implicit_pyc_source_files) py_info.imports.add(imports) diff --git a/python/private/py_executable.bzl b/python/private/py_executable.bzl index 1ee926edde..e8240da894 100644 --- a/python/private/py_executable.bzl +++ b/python/private/py_executable.bzl @@ -55,6 +55,7 @@ load( "is_bool", "is_windows_platform", "maybe_create_repo_mapping", + "py_info_runfiles_files", "relative_path", "runfiles_root_path", ) @@ -364,11 +365,8 @@ def _create_executable( venv = venv, build_data_file = runfiles_details.build_data_file, ) - extra_runfiles = ctx.runfiles( - [stage2_bootstrap] + ( - venv.files_without_interpreter if venv else [] - ), - ).merge(venv.lib_runfiles) + venv_runfiles = ctx.runfiles(venv.files_without_interpreter).merge(venv.lib_runfiles) + extra_runfiles = ctx.runfiles([stage2_bootstrap]).merge(venv_runfiles) zip_main = _create_zip_main( ctx, stage2_bootstrap = stage2_bootstrap, @@ -377,6 +375,7 @@ def _create_executable( ) else: stage2_bootstrap = None + venv_runfiles = None extra_runfiles = ctx.runfiles() zip_main = ctx.actions.declare_file(base_executable_name + ".temp", sibling = executable) _create_stage1_bootstrap( @@ -495,9 +494,8 @@ WARNING: Target: {} app_runfiles = builders.RunfilesBuilder() app_runfiles.add(runfiles_details.app_runfiles) - if venv: - app_runfiles.add(venv.files_without_interpreter) - app_runfiles.add(venv.lib_runfiles) + if venv_runfiles: + app_runfiles.add(venv_runfiles) # The interpreter is added this late in the process so that it isn't # added to the zipped files. @@ -524,6 +522,8 @@ WARNING: Target: {} venv_python_exe = venv.interpreter if venv else None, # runfiles|None; runfiles in the venv for the interpreter venv_interpreter_runfiles = venv.interpreter_runfiles if venv else None, + # runfiles|None; runfiles in the venv for the application + venv_runfiles = venv_runfiles, # depset[ExplicitSymlink]|None; symlinks that should be created venv_interpreter_symlinks = venv.interpreter_symlinks if venv else None, ) @@ -1317,6 +1317,7 @@ def py_executable_base_impl(ctx, *, semantics, is_test, inherited_environment = venv_interpreter_runfiles = exec_result.venv_interpreter_runfiles, venv_interpreter_symlinks = exec_result.venv_interpreter_symlinks, venv_python_exe = exec_result.venv_python_exe, + venv_runfiles = exec_result.venv_runfiles, ) _add_provider_py_runtime_info(providers, runtime_details) _add_provider_py_cc_link_params_info(providers, cc_details.cc_info_for_propagating) @@ -1444,19 +1445,7 @@ def _get_runtime_details(ctx): fail("'python_path' must be an absolute path or a name to be resolved from the system PATH (e.g., 'python', 'python3').") effective_runtime = _maybe_get_runtime_from_ctx(ctx) - - direct = [] # List of files - transitive = [] # List of depsets - if effective_runtime.interpreter: - direct.append(effective_runtime.interpreter) - transitive.append(effective_runtime.files) - - if ctx.configuration.coverage_enabled: - if effective_runtime.coverage_tool: - direct.append(effective_runtime.coverage_tool) - if effective_runtime.coverage_files: - transitive.append(effective_runtime.coverage_files) - runtime_files = depset(direct = direct, transitive = transitive) + runtime_files = get_runtime_files(ctx, effective_runtime) executable_interpreter_path = _get_interpreter_path( ctx, @@ -1478,6 +1467,29 @@ def _get_runtime_details(ctx): runfiles = ctx.runfiles(transitive_files = runtime_files), ) +def get_runtime_files(ctx, runtime): + """Returns the runtime files an executable puts into its runfiles. + + Args: + ctx: {type}`ctx` the rule or aspect ctx. + runtime: {type}`PyRuntimeInfo` the runtime of the executable. + + Returns: + {type}`depset[File]` + """ + direct = [] # List of files + transitive = [] # List of depsets + if runtime.interpreter: + direct.append(runtime.interpreter) + transitive.append(runtime.files) + + if ctx.configuration.coverage_enabled: + if runtime.coverage_tool: + direct.append(runtime.coverage_tool) + if runtime.coverage_files: + transitive.append(runtime.coverage_files) + return depset(direct = direct, transitive = transitive) + def _maybe_get_runtime_from_ctx(ctx): """Finds the PyRuntimeInfo from the toolchain or attribute, if available. @@ -1556,17 +1568,7 @@ def _get_base_runfiles_for_binary( if not (PyInfo in dep or (BuiltinPyInfo != None and BuiltinPyInfo in dep)): continue info = dep[PyInfo] if PyInfo in dep else dep[BuiltinPyInfo] - app_runfiles.files.add(info.transitive_sources) - - # Everything past this won't work with BuiltinPyInfo - if not hasattr(info, "transitive_pyc_files"): - continue - - app_runfiles.files.add(info.transitive_pyc_files) - if pyc_collection_enabled: - app_runfiles.files.add(info.transitive_implicit_pyc_files) - else: - app_runfiles.files.add(info.transitive_implicit_pyc_source_files) + app_runfiles.files.add(py_info_runfiles_files(info, pyc_collection_enabled)) app_runfiles.runfiles.append(collect_runfiles(ctx)) if extra_deps: @@ -2017,7 +2019,8 @@ def _add_provider_py_executable_info( venv_app_symlinks, venv_interpreter_runfiles, venv_interpreter_symlinks, - venv_python_exe): + venv_python_exe, + venv_runfiles): """Adds the PyExecutableInfo provider. Args: @@ -2034,6 +2037,7 @@ def _add_provider_py_executable_info( venv_interpreter_runfiles: runfiles; runfiles specific to the interpreter for the venv. venv_interpreter_symlinks: depset[ExplicitSymlink]; interpreter-specific symlinks to create for the venv. venv_python_exe: File; the python executable in the venv. + venv_runfiles: runfiles; the venv's files for the application. """ providers.append(PyExecutableInfo( app_runfiles = app_runfiles, @@ -2047,6 +2051,7 @@ def _add_provider_py_executable_info( venv_interpreter_runfiles = venv_interpreter_runfiles, venv_interpreter_symlinks = venv_interpreter_symlinks, venv_python_exe = venv_python_exe, + venv_runfiles = venv_runfiles, )) def _add_provider_py_runtime_info(providers, runtime_details): diff --git a/python/private/py_executable_info.bzl b/python/private/py_executable_info.bzl index 7693fa950f..2a6ccfe2a4 100644 --- a/python/private/py_executable_info.bzl +++ b/python/private/py_executable_info.bzl @@ -119,6 +119,16 @@ mode is not enabled. :::{versionadded} 1.9.0 ::: +""", + "venv_runfiles": """ +:type: runfiles | None + +Runfiles that are specific to the application within the venv: its `site-packages` +files and the symlinks to the application's dependencies. A subset of +{obj}`app_runfiles`. May be None if venv mode is not enabled. + +:::{versionadded} VERSION_NEXT_FEATURE +::: """, }, ) diff --git a/python/private/py_info.bzl b/python/private/py_info.bzl index 79483ab2f5..4bda5601ef 100644 --- a/python/private/py_info.bzl +++ b/python/private/py_info.bzl @@ -369,6 +369,7 @@ def _PyInfo_init( has_py3_only_sources = False, direct_pyc_files = depset(), transitive_pyc_files = depset(), + direct_implicit_pyc_files = depset(), transitive_implicit_pyc_files = depset(), transitive_implicit_pyc_source_files = depset(), direct_original_sources = depset(), @@ -389,6 +390,7 @@ def _PyInfo_init( _check_arg_type("direct_pyc_files", "depset", direct_pyc_files) _check_arg_type("transitive_pyc_files", "depset", transitive_pyc_files) + _check_arg_type("direct_implicit_pyc_files", "depset", direct_implicit_pyc_files) _check_arg_type("transitive_implicit_pyc_files", "depset", transitive_pyc_files) _check_arg_type("transitive_implicit_pyc_source_files", "depset", transitive_pyc_files) @@ -400,6 +402,7 @@ def _PyInfo_init( if type_checking_info != None: _check_arg_type("type_checking_info", "struct", type_checking_info) return { + "direct_implicit_pyc_files": direct_implicit_pyc_files, "direct_original_sources": direct_original_sources, "direct_pyc_files": direct_pyc_files, "direct_pyi_files": direct_pyi_files, @@ -425,6 +428,15 @@ the {obj}`PyCommonApi` utilities. """, init = _PyInfo_init, fields = { + "direct_implicit_pyc_files": """ +:type: depset[File] + +The automatically generated pyc files that are directly provided by the target. +The direct counterpart of {obj}`transitive_implicit_pyc_files`. + +::::{versionadded} VERSION_NEXT_FEATURE +:::: +""", "direct_original_sources": """ :type: depset[File] @@ -602,6 +614,10 @@ def _PyInfoBuilder_typedef(): To create an instance, use {obj}`py_common.get()` and call `PyInfoBuilder()` + :::{field} direct_implicit_pyc_files + :type: DepsetBuilder[File] + ::: + :::{field} direct_original_sources :type: DepsetBuilder[File] ::: @@ -666,6 +682,7 @@ def _new_raw_py_info_builder(type_checking_info = None): add_venv_symlink = lambda *a, **k: _PyInfoBuilder_add_venv_symlink(self, *a, **k), build = lambda *a, **k: _PyInfoBuilder_build(self, *a, **k), build_builtin_py_info = lambda *a, **k: _PyInfoBuilder_build_builtin_py_info(self, *a, **k), + direct_implicit_pyc_files = builders.DepsetBuilder(), direct_original_sources = builders.DepsetBuilder(), direct_pyc_files = builders.DepsetBuilder(), direct_pyi_files = builders.DepsetBuilder(), @@ -854,6 +871,7 @@ def _merge_py_info_fields(self, transitive, *, direct = []): # BuiltinPyInfo doesn't have this field if hasattr(info, "direct_pyc_files"): self.direct_original_sources.add(info.direct_original_sources) + self.direct_implicit_pyc_files.add(info.direct_implicit_pyc_files) self.direct_pyc_files.add(info.direct_pyc_files) self.direct_pyi_files.add(info.direct_pyi_files) @@ -993,6 +1011,7 @@ def _build_py_info_fields(self, type_checking_info = None): imports = self.imports.build(), transitive_sources = self.transitive_sources.build(), uses_shared_libraries = self._uses_shared_libraries[0], + direct_implicit_pyc_files = self.direct_implicit_pyc_files.build(), direct_original_sources = self.direct_original_sources.build(), direct_pyc_files = self.direct_pyc_files.build(), direct_pyi_files = self.direct_pyi_files.build(), diff --git a/python/private/py_library.bzl b/python/private/py_library.bzl index 6282b71ee1..9b1e064853 100644 --- a/python/private/py_library.bzl +++ b/python/private/py_library.bzl @@ -223,14 +223,24 @@ Source files are no longer added to the runfiles directly. ::: """ -def _get_package_and_version(ctx): +def get_package_and_version(data_files): """Return package name and version If the package comes from PyPI then it will have a `.dist-info` as part of `data`, which allows us to get the name of the package and its version. + + Args: + data_files: {type}`list[File]` the files in the library's `data` + attribute. + + Returns: + {type}`tuple[str | None, str | None]` the distribution name, lowercased + with runs of `-`, `_` and `.` turned into `_` (e.g. `google_cloud_storage`), + and the normalized version; `(None, None)` without a + `.dist-info/METADATA` file. """ dist_info_metadata = None - for d in ctx.files.data: + for d in data_files: # work on case insensitive FSes if d.basename.lower() != "metadata": continue @@ -257,7 +267,7 @@ def _get_imports_and_venv_symlinks(ctx): imports = depset() venv_symlinks = [] if VenvsSitePackages.is_enabled(ctx): - package, version_str = _get_package_and_version(ctx) + package, version_str = get_package_and_version(ctx.files.data) # NOTE: Already a list, but buildifier thinks its a depset and # adds to_list() calls later. diff --git a/tests/base_rules/precompile/precompile_tests.bzl b/tests/base_rules/precompile/precompile_tests.bzl index 63865a30e6..ef1ea3c1ca 100644 --- a/tests/base_rules/precompile/precompile_tests.bzl +++ b/tests/base_rules/precompile/precompile_tests.bzl @@ -137,6 +137,30 @@ def _test_precompile_enabled_py_library_common_impl(env, target): "{package}/__pycache__/lib.fakepy-45.pyc", ]) +def _test_precompile_flag_enabled_py_library_implicit_pyc(name): + rt_util.helper_target( + py_library, + name = name + "_subject", + srcs = ["lib.py"], + ) + analysis_test( + name = name, + impl = _test_precompile_flag_enabled_py_library_implicit_pyc_impl, + target = name + "_subject", + config_settings = _COMMON_CONFIG_SETTINGS | { + labels.PRECOMPILE: "enabled", + }, + ) + +def _test_precompile_flag_enabled_py_library_implicit_pyc_impl(env, target): + py_info = env.expect.that_target(target).provider(PyInfo, factory = py_info_subject) + py_info.direct_pyc_files().contains_exactly([]) + py_info.direct_implicit_pyc_files().contains_exactly([ + "{package}/__pycache__/lib.fakepy-45.pyc", + ]) + +_tests.append(_test_precompile_flag_enabled_py_library_implicit_pyc) + def _test_precompile_enabled_py_library_add_to_runfiles_disabled(name): _test_precompile_enabled_py_library_setup( name = name, diff --git a/tests/base_rules/py_executable_base_tests.bzl b/tests/base_rules/py_executable_base_tests.bzl index 34b922a6fb..0caa7ff112 100644 --- a/tests/base_rules/py_executable_base_tests.bzl +++ b/tests/base_rules/py_executable_base_tests.bzl @@ -693,6 +693,29 @@ def _test_venv_output_prefix_with_path_separators_impl(env, targets): _tests.append(_test_venv_output_prefix_with_path_separators) +def _test_venv_runfiles(name, config): + rt_util.helper_target( + config.rule, + name = name + "_subject", + srcs = [name + "_main.py"], + main = name + "_main.py", + ) + analysis_test( + name = name, + impl = _test_venv_runfiles_impl, + target = name + "_subject", + ) + +def _test_venv_runfiles_impl(env, target): + venv_runfiles = env.expect.that_target(target).provider( + PyExecutableInfo, + factory = PyExecutableInfoSubject.new, + ).venv_runfiles() + venv_runfiles.contains_predicate(matching.str_matches("*_subject.venv/*site-packages/bazel.pth")) + venv_runfiles.not_contains_predicate(matching.str_endswith("_main.py")) + +_tests.append(_test_venv_runfiles) + def _test_windows_target_with_path_separators(name, config): rt_util.helper_target( config.rule, diff --git a/tests/base_rules/py_info/py_info_tests.bzl b/tests/base_rules/py_info/py_info_tests.bzl index 1aa7ff2429..087b115eab 100644 --- a/tests/base_rules/py_info/py_info_tests.bzl +++ b/tests/base_rules/py_info/py_info_tests.bzl @@ -23,6 +23,7 @@ load("//tests/support:py_info_subject.bzl", "py_info_subject") def _provide_py_info_impl(ctx): kwargs = { + "direct_implicit_pyc_files": depset(ctx.files.direct_implicit_pyc_files), "direct_original_sources": depset(ctx.files.direct_original_sources), "direct_pyc_files": depset(ctx.files.direct_pyc_files), "direct_pyi_files": depset(ctx.files.direct_pyi_files), @@ -58,6 +59,7 @@ def _provide_py_info_impl(ctx): provide_py_info = rule( implementation = _provide_py_info_impl, attrs = { + "direct_implicit_pyc_files": attr.label_list(allow_files = True), "direct_original_sources": attr.label_list(allow_files = True), "direct_pyc_files": attr.label_list(allow_files = True), "direct_pyi_files": attr.label_list(allow_files = True), @@ -120,6 +122,7 @@ def _test_py_info_builder(name): "trans-original.py", "direct.pyi", "trans.pyi", + "direct-implicit.pyc", ], ) @@ -132,6 +135,7 @@ def _test_py_info_builder(name): name = py_info_name, transitive_sources = ["py{}-trans.py".format(n)], direct_pyc_files = ["py{}-direct.pyc".format(n)], + direct_implicit_pyc_files = ["py{}-direct-implicit.pyc".format(n)], imports = ["py{}import".format(n)], transitive_pyc_files = ["py{}-trans.pyc".format(n)], direct_original_sources = ["py{}-original-direct.py".format(n)], @@ -156,9 +160,11 @@ def _test_py_info_builder_impl(env, targets): trans_original_py, direct_pyi, trans_pyi, + direct_implicit_pyc, ) = targets.misc[DefaultInfo].files.to_list() builder = PyInfoBuilder.new() builder.direct_pyc_files.add(direct_pyc) + builder.direct_implicit_pyc_files.add(direct_implicit_pyc) builder.direct_original_sources.add(original_py) builder.direct_pyi_files.add(direct_pyi) builder.merge_has_py2_only_sources(True) @@ -216,6 +222,11 @@ def _test_py_info_builder_impl(env, targets): "tests/base_rules/py_info/py4-direct.pyc", "tests/base_rules/py_info/py6-direct.pyc", ]) + subject.direct_implicit_pyc_files().contains_exactly([ + "tests/base_rules/py_info/direct-implicit.pyc", + "tests/base_rules/py_info/py4-direct-implicit.pyc", + "tests/base_rules/py_info/py6-direct-implicit.pyc", + ]) subject.transitive_pyc_files().contains_exactly([ "tests/base_rules/py_info/trans.pyc", "tests/base_rules/py_info/py1-trans.pyc", diff --git a/tests/support/py_executable_info_subject.bzl b/tests/support/py_executable_info_subject.bzl index 97216eceff..6bedd8d0fe 100644 --- a/tests/support/py_executable_info_subject.bzl +++ b/tests/support/py_executable_info_subject.bzl @@ -35,6 +35,7 @@ def _py_executable_info_subject_new(info, *, meta): interpreter_path = lambda *a, **k: _py_executable_info_subject_interpreter_path(self, *a, **k), main = lambda *a, **k: _py_executable_info_subject_main(self, *a, **k), runfiles_without_exe = lambda *a, **k: _py_executable_info_subject_runfiles_without_exe(self, *a, **k), + venv_runfiles = lambda *a, **k: _py_executable_info_subject_venv_runfiles(self, *a, **k), # go/keep-sorted end ) self = struct( @@ -64,6 +65,13 @@ def _py_executable_info_subject_runfiles_without_exe(self): meta = self.meta.derive("runfiles_without_exe()"), ) +def _py_executable_info_subject_venv_runfiles(self): + """Returns a subject for `PyExecutableInfo.venv_runfiles`.""" + return subjects.runfiles( + self.actual.venv_runfiles, + meta = self.meta.derive("venv_runfiles()"), + ) + # buildifier: disable=name-conventions PyExecutableInfoSubject = struct( new = _py_executable_info_subject_new, diff --git a/tests/support/py_info_subject.bzl b/tests/support/py_info_subject.bzl index f526f5d563..c45848ae1a 100644 --- a/tests/support/py_info_subject.bzl +++ b/tests/support/py_info_subject.bzl @@ -19,6 +19,7 @@ def _new_py_info_subject(info, *, meta): # buildifier: disable=uninitialized public = struct( # go/keep-sorted start + direct_implicit_pyc_files = lambda *a, **k: _py_info_subject_direct_implicit_pyc_files(self, *a, **k), direct_original_sources = lambda *a, **k: _py_info_subject_direct_original_sources(self, *a, **k), direct_pyc_files = lambda *a, **k: _py_info_subject_direct_pyc_files(self, *a, **k), direct_pyi_files = lambda *a, **k: _py_info_subject_direct_pyi_files(self, *a, **k), @@ -53,6 +54,16 @@ def py_info_subject(info, *, meta): """ return _new_py_info_subject(info, meta = meta) +def _py_info_subject_direct_implicit_pyc_files(self): + """Returns a `DepsetFileSubject` for the `direct_implicit_pyc_files` attribute. + + Method: PyInfoSubject.direct_implicit_pyc_files + """ + return subjects.depset_file( + self.actual.direct_implicit_pyc_files, + meta = self.meta.derive("direct_implicit_pyc_files()"), + ) + def _py_info_subject_direct_original_sources(self): """Returns a `DepsetFileSubject` for the `direct_original_sources` attribute. """