feat: add PyInfo.direct_implicit_pyc_files and PyExecutableInfo.venv_runfiles (#4341) `PyInfo` had a direct counterpart for every pyc field except the implicit pyc files, and `PyExecutableInfo` exposed 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, with no change in behavior: `py_info_runfiles_files()` for the files a binary takes from a dependency's PyInfo, `get_runtime_files()` for the runtime's files, and `get_package_and_version()` taking the data files instead of the ctx. Split out of #4254, which uses all of these.
diff --git a/news/3757-provider-fields.added.md b/news/3757-provider-fields.added.md new file mode 100644 index 0000000..f854306 --- /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 0a2d8a1..28f33ed 100644 --- a/python/private/common.bzl +++ b/python/private/common.bzl
@@ -283,6 +283,28 @@ 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 @@ 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 1ee926e..e8240da 100644 --- a/python/private/py_executable.bzl +++ b/python/private/py_executable.bzl
@@ -55,6 +55,7 @@ "is_bool", "is_windows_platform", "maybe_create_repo_mapping", + "py_info_runfiles_files", "relative_path", "runfiles_root_path", ) @@ -364,11 +365,8 @@ 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 @@ ) 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 @@ 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 @@ 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 @@ 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 @@ 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 @@ 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 @@ 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 @@ 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 @@ 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 @@ 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 7693fa9..2a6ccfe 100644 --- a/python/private/py_executable_info.bzl +++ b/python/private/py_executable_info.bzl
@@ -120,5 +120,15 @@ :::{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 79483ab..4bda560 100644 --- a/python/private/py_info.bzl +++ b/python/private/py_info.bzl
@@ -369,6 +369,7 @@ 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 @@ _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 @@ 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 @@ """, 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 @@ 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 @@ 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 @@ # 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 @@ 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 6282b71..9b1e064 100644 --- a/python/private/py_library.bzl +++ b/python/private/py_library.bzl
@@ -223,14 +223,24 @@ ::: """ -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 @@ 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 63865a3..ef1ea3c 100644 --- a/tests/base_rules/precompile/precompile_tests.bzl +++ b/tests/base_rules/precompile/precompile_tests.bzl
@@ -137,6 +137,30 @@ "{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 34b922a..0caa7ff 100644 --- a/tests/base_rules/py_executable_base_tests.bzl +++ b/tests/base_rules/py_executable_base_tests.bzl
@@ -693,6 +693,29 @@ _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 1aa7ff2..087b115 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 @@ 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 @@ 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 @@ "trans-original.py", "direct.pyi", "trans.pyi", + "direct-implicit.pyc", ], ) @@ -132,6 +135,7 @@ 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 @@ 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 @@ "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 97216ec..6bedd8d 100644 --- a/tests/support/py_executable_info_subject.bzl +++ b/tests/support/py_executable_info_subject.bzl
@@ -35,6 +35,7 @@ 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 @@ 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 f526f5d..c45848a 100644 --- a/tests/support/py_info_subject.bzl +++ b/tests/support/py_info_subject.bzl
@@ -19,6 +19,7 @@ # 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 @@ """ 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. """