From 150941ca5a6fc2e2ca7a4182a289c1bf4a37b990 Mon Sep 17 00:00:00 2001 From: Richard Levasseur Date: Wed, 27 Nov 2024 22:48:49 -0800 Subject: [PATCH] feat: add PyInfo fields for storing original sources of a target --- CHANGELOG.md | 4 + python/private/common.bzl | 4 + python/private/py_executable.bzl | 5 + python/private/py_info.bzl | 131 ++++++++++++++++++++- python/private/py_library.bzl | 1 + tests/base_rules/py_info/py_info_tests.bzl | 66 ++++++++++- tests/support/py_info_subject.bzl | 38 ++++++ 7 files changed, 245 insertions(+), 4 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index eb4bcfa8da..f1e265f3b6 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -95,6 +95,10 @@ Unreleased changes template. * 3.11.11 * 3.12.8 * 3.13.1 +* (providers) {obj}`PyInfo` has new fields to aid static analysis tools: + {obj}`direct_original_sources`, {obj}`direct_pyi_files`, + {obj}`transitive_original_sources`, {obj}`transitive_pyi_files`. NOTE: these + are not yet fully populated by `py_library` [20241206]: https://github.com/astral-sh/python-build-standalone/releases/tag/20241206 diff --git a/python/private/common.bzl b/python/private/common.bzl index 9c285f97bc..9cc61ed47c 100644 --- a/python/private/common.bzl +++ b/python/private/common.bzl @@ -408,6 +408,7 @@ def collect_runfiles(ctx, files = depset()): def create_py_info( ctx, *, + original_sources, required_py_files, required_pyc_files, implicit_pyc_files, @@ -417,6 +418,7 @@ def create_py_info( Args: ctx: rule ctx. + original_sources: `depset[File]`; the original input sources from `srcs` required_py_files: `depset[File]`; the direct, `.py` sources for the target that **must** be included by downstream targets. This should only be Python source files. It should not include pyc files. @@ -437,6 +439,8 @@ def create_py_info( """ py_info = PyInfoBuilder() + py_info.direct_original_sources.add(original_sources) + py_info.transitive_original_sources.add(original_sources) py_info.direct_pyc_files.add(required_pyc_files) py_info.transitive_pyc_files.add(required_pyc_files) py_info.transitive_implicit_pyc_files.add(implicit_pyc_files) diff --git a/python/private/py_executable.bzl b/python/private/py_executable.bzl index 40c74100f2..20af98da9d 100644 --- a/python/private/py_executable.bzl +++ b/python/private/py_executable.bzl @@ -985,6 +985,7 @@ def py_executable_base_impl(ctx, *, semantics, is_test, inherited_environment = runfiles_details = runfiles_details, main_py = main_py, imports = imports, + original_sources = direct_sources, required_py_files = required_py_files, required_pyc_files = required_pyc_files, implicit_pyc_files = implicit_pyc_files, @@ -1548,6 +1549,7 @@ def _create_providers( ctx, executable, main_py, + original_sources, required_py_files, required_pyc_files, implicit_pyc_files, @@ -1566,6 +1568,8 @@ def _create_providers( ctx: The rule ctx. executable: File; the target's executable file. main_py: File; the main .py entry point. + original_sources: `depset[File]` the direct `.py` sources for the + target that were the original input sources. required_py_files: `depset[File]` the direct, `.py` sources for the target that **must** be included by downstream targets. This should only be Python source files. It should not include pyc files. @@ -1649,6 +1653,7 @@ def _create_providers( py_info, deps_transitive_sources, builtin_py_info = create_py_info( ctx, + original_sources = original_sources, required_py_files = required_py_files, required_pyc_files = required_pyc_files, implicit_pyc_files = implicit_pyc_files, diff --git a/python/private/py_info.bzl b/python/private/py_info.bzl index 4b2b8888c9..1e791868b5 100644 --- a/python/private/py_info.bzl +++ b/python/private/py_info.bzl @@ -38,7 +38,11 @@ def _PyInfo_init( direct_pyc_files = depset(), transitive_pyc_files = depset(), transitive_implicit_pyc_files = depset(), - transitive_implicit_pyc_source_files = depset()): + transitive_implicit_pyc_source_files = depset(), + direct_original_sources = depset(), + transitive_original_sources = depset(), + direct_pyi_files = depset(), + transitive_pyi_files = depset()): _check_arg_type("transitive_sources", "depset", transitive_sources) # Verify it's postorder compatible, but retain is original ordering. @@ -53,14 +57,24 @@ def _PyInfo_init( _check_arg_type("transitive_implicit_pyc_files", "depset", transitive_pyc_files) _check_arg_type("transitive_implicit_pyc_source_files", "depset", transitive_pyc_files) + + _check_arg_type("direct_original_sources", "depset", direct_original_sources) + _check_arg_type("transitive_original_sources", "depset", transitive_original_sources) + + _check_arg_type("direct_pyi_files", "depset", direct_pyi_files) + _check_arg_type("transitive_pyi_files", "depset", transitive_pyi_files) return { + "direct_original_sources": direct_original_sources, "direct_pyc_files": direct_pyc_files, + "direct_pyi_files": direct_pyi_files, "has_py2_only_sources": has_py2_only_sources, "has_py3_only_sources": has_py2_only_sources, "imports": imports, "transitive_implicit_pyc_files": transitive_implicit_pyc_files, "transitive_implicit_pyc_source_files": transitive_implicit_pyc_source_files, + "transitive_original_sources": transitive_original_sources, "transitive_pyc_files": transitive_pyc_files, + "transitive_pyi_files": transitive_pyi_files, "transitive_sources": transitive_sources, "uses_shared_libraries": uses_shared_libraries, } @@ -69,6 +83,18 @@ PyInfo, _unused_raw_py_info_ctor = define_bazel_6_provider( doc = "Encapsulates information provided by the Python rules.", init = _PyInfo_init, fields = { + "direct_original_sources": """ +:type: depset[File] + +The `.py` source files (if any) that are considered directly provided by +the target. This field is intended so that static analysis tools can recover the +original Python source files, regardless of any build settings (e.g. +precompiling), so they can analyze source code. The values are typically the +`.py` files in the `srcs` attribute (or equivalent). + +::::{versionadded} 1.1.0 +:::: +""", "direct_pyc_files": """ :type: depset[File] @@ -78,6 +104,21 @@ by the target and **must be included**. These files usually come from, e.g., a library setting {attr}`precompile=enabled` to forcibly enable precompiling for itself. Downstream binaries are expected to always include these files, as the originating target expects them to exist. +""", + "direct_pyi_files": """ +:type: depset[File] + +Type definition files (usually `.pyi` files) for the Python modules provided by +this target. Usually they describe the source files listed in +`direct_original_sources`. This field is primarily for static analysis tools. + +:::{note} +This may contain implementation-specific file types specific to a particular +type checker. +::: + +::::{versionadded} 1.1.0 +:::: """, "has_py2_only_sources": """ :type: bool @@ -116,6 +157,21 @@ then {obj}`transitive_implicit_pyc_files` should be included instead. ::::{versionadded} 0.37.0 :::: +""", + "transitive_original_sources": """ +:type: depset[File] + +The transitive set of `.py` source files (if any) that are considered the +original sources for this target and its transitive dependencies. This field is +intended so that static analysis tools can recover the original Python source +files, regardless of any build settings (e.g. precompiling), so they can analyze +source code. The values are typically the `.py` files in the `srcs` attribute +(or equivalent). + +This is superset of `direct_original_sources`. + +::::{versionadded} 1.1.0 +:::: """, "transitive_pyc_files": """ :type: depset[File] @@ -125,6 +181,22 @@ The transitive set of precompiled files that must be included. These files usually come from, e.g., a library setting {attr}`precompile=enabled` to forcibly enable precompiling for itself. Downstream binaries are expected to always include these files, as the originating target expects them to exist. +""", + "transitive_pyi_files": """ +:type: depset[File] + +The transitive set of type definition files (usually `.pyi` files) for the +Python modules for this target and its transitive dependencies. this target. +Usually they describe the source files listed in `transitive_original_sources`. +This field is primarily for static analysis tools. + +:::{note} +This may contain implementation-specific file types specific to a particular +type checker. +::: + +::::{versionadded} 1.1.0 +:::: """, "transitive_sources": """\ :type: depset[File] @@ -165,7 +237,9 @@ def PyInfoBuilder(): _uses_shared_libraries = [False], 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_original_sources = builders.DepsetBuilder(), direct_pyc_files = builders.DepsetBuilder(), + direct_pyi_files = builders.DepsetBuilder(), get_has_py2_only_sources = lambda *a, **k: _PyInfoBuilder_get_has_py2_only_sources(self, *a, **k), get_has_py3_only_sources = lambda *a, **k: _PyInfoBuilder_get_has_py3_only_sources(self, *a, **k), get_uses_shared_libraries = lambda *a, **k: _PyInfoBuilder_get_uses_shared_libraries(self, *a, **k), @@ -182,7 +256,9 @@ def PyInfoBuilder(): set_uses_shared_libraries = lambda *a, **k: _PyInfoBuilder_set_uses_shared_libraries(self, *a, **k), transitive_implicit_pyc_files = builders.DepsetBuilder(), transitive_implicit_pyc_source_files = builders.DepsetBuilder(), + transitive_original_sources = builders.DepsetBuilder(), transitive_pyc_files = builders.DepsetBuilder(), + transitive_pyi_files = builders.DepsetBuilder(), transitive_sources = builders.DepsetBuilder(), ) return self @@ -221,13 +297,37 @@ def _PyInfoBuilder_set_uses_shared_libraries(self, value): return self def _PyInfoBuilder_merge(self, *infos, direct = []): + """Merge other PyInfos into this PyInfo. + + Args: + *infos: {type}`PyInfo` objects to merge in, but only merge in their + information into this object's transitive fields. + direct: {type}`list[PyInfo]` objects to merge in, but also merge their + direct fields into this object's direct fields. + + Returns: + {type}`PyInfoBuilder` the current object + """ return self.merge_all(list(infos), direct = direct) def _PyInfoBuilder_merge_all(self, transitive, *, direct = []): + """Merge other PyInfos into this PyInfo. + + Args: + *infos: {type}`list[PyInfo]` objects to merge in, but only merge in their + information into this object's transitive fields. + direct: {type}`list[PyInfo]` objects to merge in, but also merge their + direct fields into this object's direct fields. + + Returns: + {type}`PyInfoBuilder` the current object + """ for info in direct: # BuiltinPyInfo doesn't have this field if hasattr(info, "direct_pyc_files"): + self.direct_original_sources.add(info.direct_original_sources) self.direct_pyc_files.add(info.direct_pyc_files) + self.direct_pyi_files.add(info.direct_pyi_files) for info in direct + transitive: self.imports.add(info.imports) @@ -240,11 +340,23 @@ def _PyInfoBuilder_merge_all(self, transitive, *, direct = []): if hasattr(info, "transitive_pyc_files"): self.transitive_implicit_pyc_files.add(info.transitive_implicit_pyc_files) self.transitive_implicit_pyc_source_files.add(info.transitive_implicit_pyc_source_files) + self.transitive_original_sources.add(info.transitive_original_sources) self.transitive_pyc_files.add(info.transitive_pyc_files) + self.transitive_pyi_files.add(info.transitive_pyi_files) return self def _PyInfoBuilder_merge_target(self, target): + """Merge a target's Python information in this object. + + Args: + target: {type}`Target` targets that provide PyInfo, or other relevant + providers, will be merged into this object. If a target doesn't provide + any relevant providers, it is ignored. + + Returns: + {type}`PyInfoBuilder` the current object. + """ if PyInfo in target: self.merge(target[PyInfo]) elif BuiltinPyInfo != None and BuiltinPyInfo in target: @@ -252,6 +364,17 @@ def _PyInfoBuilder_merge_target(self, target): return self def _PyInfoBuilder_merge_targets(self, targets): + """Merge multiple targets into this object. + + Args: + targets: {type}`list[Target]` + targets that provide PyInfo, or other relevant + providers, will be merged into this object. If a target doesn't provide + any relevant providers, it is ignored. + + Returns: + {type}`PyInfoBuilder` the current object. + """ for t in targets: self.merge_target(t) return self @@ -259,10 +382,14 @@ def _PyInfoBuilder_merge_targets(self, targets): def _PyInfoBuilder_build(self): if config.enable_pystar: kwargs = dict( + direct_original_sources = self.direct_original_sources.build(), direct_pyc_files = self.direct_pyc_files.build(), - transitive_pyc_files = self.transitive_pyc_files.build(), + direct_pyi_files = self.direct_pyi_files.build(), transitive_implicit_pyc_files = self.transitive_implicit_pyc_files.build(), transitive_implicit_pyc_source_files = self.transitive_implicit_pyc_source_files.build(), + transitive_original_sources = self.transitive_original_sources.build(), + transitive_pyc_files = self.transitive_pyc_files.build(), + transitive_pyi_files = self.transitive_pyi_files.build(), ) else: kwargs = {} diff --git a/python/private/py_library.bzl b/python/private/py_library.bzl index 6a65038e8a..350ea35aa6 100644 --- a/python/private/py_library.bzl +++ b/python/private/py_library.bzl @@ -102,6 +102,7 @@ def py_library_impl(ctx, *, semantics): cc_info = semantics.get_cc_info_for_library(ctx) py_info, deps_transitive_sources, builtins_py_info = create_py_info( ctx, + original_sources = direct_sources, required_py_files = required_py_files, required_pyc_files = required_pyc_files, implicit_pyc_files = implicit_pyc_files, diff --git a/tests/base_rules/py_info/py_info_tests.bzl b/tests/base_rules/py_info/py_info_tests.bzl index 4067a59d24..e160e704de 100644 --- a/tests/base_rules/py_info/py_info_tests.bzl +++ b/tests/base_rules/py_info/py_info_tests.bzl @@ -24,9 +24,13 @@ load("//tests/support:py_info_subject.bzl", "py_info_subject") def _provide_py_info_impl(ctx): kwargs = { + "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), "imports": depset(ctx.attr.imports), + "transitive_original_sources": depset(ctx.files.transitive_original_sources), "transitive_pyc_files": depset(ctx.files.transitive_pyc_files), + "transitive_pyi_files": depset(ctx.files.transitive_pyi_files), "transitive_sources": depset(ctx.files.transitive_sources), } if ctx.attr.has_py2_only_sources != -1: @@ -56,11 +60,15 @@ def _provide_py_info_impl(ctx): provide_py_info = rule( implementation = _provide_py_info_impl, attrs = { + "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), "has_py2_only_sources": attr.int(default = -1), "has_py3_only_sources": attr.int(default = -1), "imports": attr.string_list(), + "transitive_original_sources": attr.label_list(allow_files = True), "transitive_pyc_files": attr.label_list(allow_files = True), + "transitive_pyi_files": attr.label_list(allow_files = True), "transitive_sources": attr.label_list(allow_files = True), }, ) @@ -109,7 +117,15 @@ def _test_py_info_builder(name): rt_util.helper_target( native.filegroup, name = name + "_misc", - srcs = ["trans.py", "direct.pyc", "trans.pyc"], + srcs = [ + "trans.py", + "direct.pyc", + "trans.pyc", + "original.py", + "trans-original.py", + "direct.pyi", + "trans.pyi", + ], ) py_info_targets = {} @@ -123,6 +139,10 @@ def _test_py_info_builder(name): direct_pyc_files = ["py{}-direct.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)], + transitive_original_sources = ["py{}-original-trans.py".format(n)], + direct_pyi_files = ["py{}-direct.pyi".format(n)], + transitive_pyi_files = ["py{}-trans.pyi".format(n)], ) analysis_test( name = name, @@ -133,13 +153,25 @@ def _test_py_info_builder(name): ) def _test_py_info_builder_impl(env, targets): - trans, direct_pyc, trans_pyc = targets.misc[DefaultInfo].files.to_list() + ( + trans, + direct_pyc, + trans_pyc, + original_py, + trans_original_py, + direct_pyi, + trans_pyi, + ) = targets.misc[DefaultInfo].files.to_list() builder = PyInfoBuilder() builder.direct_pyc_files.add(direct_pyc) + builder.direct_original_sources.add(original_py) + builder.direct_pyi_files.add(direct_pyi) builder.merge_has_py2_only_sources(True) builder.merge_has_py3_only_sources(True) builder.imports.add("import-path") builder.transitive_pyc_files.add(trans_pyc) + builder.transitive_pyi_files.add(trans_pyi) + builder.transitive_original_sources.add(trans_original_py) builder.transitive_sources.add(trans) builder.merge_uses_shared_libraries(True) @@ -174,6 +206,8 @@ def _test_py_info_builder_impl(env, targets): "py5import", "py6import", ]) + + # Checks for non-Bazel builtin PyInfo if hasattr(actual, "direct_pyc_files"): subject.direct_pyc_files().contains_exactly([ "tests/base_rules/py_info/direct.pyc", @@ -189,6 +223,34 @@ def _test_py_info_builder_impl(env, targets): "tests/base_rules/py_info/py5-trans.pyc", "tests/base_rules/py_info/py6-trans.pyc", ]) + subject.direct_original_sources().contains_exactly([ + "tests/base_rules/py_info/original.py", + "tests/base_rules/py_info/py4-original-direct.py", + "tests/base_rules/py_info/py6-original-direct.py", + ]) + subject.transitive_original_sources().contains_exactly([ + "tests/base_rules/py_info/trans-original.py", + "tests/base_rules/py_info/py1-original-trans.py", + "tests/base_rules/py_info/py2-original-trans.py", + "tests/base_rules/py_info/py3-original-trans.py", + "tests/base_rules/py_info/py4-original-trans.py", + "tests/base_rules/py_info/py5-original-trans.py", + "tests/base_rules/py_info/py6-original-trans.py", + ]) + subject.direct_pyi_files().contains_exactly([ + "tests/base_rules/py_info/direct.pyi", + "tests/base_rules/py_info/py4-direct.pyi", + "tests/base_rules/py_info/py6-direct.pyi", + ]) + subject.transitive_pyi_files().contains_exactly([ + "tests/base_rules/py_info/trans.pyi", + "tests/base_rules/py_info/py1-trans.pyi", + "tests/base_rules/py_info/py2-trans.pyi", + "tests/base_rules/py_info/py3-trans.pyi", + "tests/base_rules/py_info/py4-trans.pyi", + "tests/base_rules/py_info/py5-trans.pyi", + "tests/base_rules/py_info/py6-trans.pyi", + ]) check(builder.build()) if BuiltinPyInfo != None: diff --git a/tests/support/py_info_subject.bzl b/tests/support/py_info_subject.bzl index bfed0b335d..9122eaa9fd 100644 --- a/tests/support/py_info_subject.bzl +++ b/tests/support/py_info_subject.bzl @@ -31,11 +31,15 @@ def py_info_subject(info, *, meta): # buildifier: disable=uninitialized public = struct( # go/keep-sorted start + 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), has_py2_only_sources = lambda *a, **k: _py_info_subject_has_py2_only_sources(self, *a, **k), has_py3_only_sources = lambda *a, **k: _py_info_subject_has_py3_only_sources(self, *a, **k), imports = lambda *a, **k: _py_info_subject_imports(self, *a, **k), + transitive_original_sources = lambda *a, **k: _py_info_subject_transitive_original_sources(self, *a, **k), transitive_pyc_files = lambda *a, **k: _py_info_subject_transitive_pyc_files(self, *a, **k), + transitive_pyi_files = lambda *a, **k: _py_info_subject_transitive_pyi_files(self, *a, **k), transitive_sources = lambda *a, **k: _py_info_subject_transitive_sources(self, *a, **k), uses_shared_libraries = lambda *a, **k: _py_info_subject_uses_shared_libraries(self, *a, **k), # go/keep-sorted end @@ -46,6 +50,14 @@ def py_info_subject(info, *, meta): ) return public +def _py_info_subject_direct_original_sources(self): + """Returns a `DepsetFileSubject` for the `direct_original_sources` attribute. + """ + return subjects.depset_file( + self.actual.direct_original_sources, + meta = self.meta.derive("direct_original_sources()"), + ) + def _py_info_subject_direct_pyc_files(self): """Returns a `DepsetFileSubject` for the `direct_pyc_files` attribute. @@ -56,6 +68,14 @@ def _py_info_subject_direct_pyc_files(self): meta = self.meta.derive("direct_pyc_files()"), ) +def _py_info_subject_direct_pyi_files(self): + """Returns a `DepsetFileSubject` for the `direct_pyi_files` attribute. + """ + return subjects.depset_file( + self.actual.direct_pyi_files, + meta = self.meta.derive("direct_pyi_files()"), + ) + def _py_info_subject_has_py2_only_sources(self): """Returns a `BoolSubject` for the `has_py2_only_sources` attribute. @@ -86,6 +106,16 @@ def _py_info_subject_imports(self): meta = self.meta.derive("imports()"), ) +def _py_info_subject_transitive_original_sources(self): + """Returns a `DepsetFileSubject` for the `transitive_original_sources` attribute. + + Method: PyInfoSubject.transitive_original_sources + """ + return subjects.depset_file( + self.actual.transitive_original_sources, + meta = self.meta.derive("transitive_original_sources()"), + ) + def _py_info_subject_transitive_pyc_files(self): """Returns a `DepsetFileSubject` for the `transitive_pyc_files` attribute. @@ -96,6 +126,14 @@ def _py_info_subject_transitive_pyc_files(self): meta = self.meta.derive("transitive_pyc_files()"), ) +def _py_info_subject_transitive_pyi_files(self): + """Returns a `DepsetFileSubject` for the `transitive_pyi_files` attribute. + """ + return subjects.depset_file( + self.actual.transitive_pyi_files, + meta = self.meta.derive("transitive_pyi_files()"), + ) + def _py_info_subject_transitive_sources(self): """Returns a `DepsetFileSubject` for the `transitive_sources` attribute.