pystar: support builtin providers for compatibility (#1573)

This makes the rules_python Starlark implementation accept and return
the builtin providers. This allows depending on, and being depended on
by, the builtin rules, which enables the two rule sets to interoperate
better.

Work towards #1069

python/defs.bzl

@@ -15,7 +15,7 @@
 
 load("@bazel_tools//tools/python:srcs_version.bzl", _find_requirements = "find_requirements")
 load("//python:py_binary.bzl", _py_binary = "py_binary")
-load("//python:py_info.bzl", internal_PyInfo = "PyInfo")
+load("//python:py_info.bzl", _PyInfo = "PyInfo")
 load("//python:py_library.bzl", _py_library = "py_library")
 load("//python:py_runtime.bzl", _py_runtime = "py_runtime")
 load("//python:py_runtime_info.bzl", internal_PyRuntimeInfo = "PyRuntimeInfo")
@@ -26,9 +26,7 @@ load(":py_import.bzl", _py_import = "py_import")
 
 # Patching placeholder: end of loads
 
-# Exports of native-defined providers.
-
-PyInfo = internal_PyInfo
+PyInfo = _PyInfo
 
 PyRuntimeInfo = internal_PyRuntimeInfo
 

python/private/BUILD.bazel

@@ -169,8 +169,7 @@ bzl_library(
     name = "reexports_bzl",
     srcs = ["reexports.bzl"],
     visibility = [
-        "//docs:__pkg__",
-        "//python:__pkg__",
+        "//:__subpackages__",
     ],
     deps = [":bazel_tools_bzl"],
 )

python/private/common/BUILD.bazel

@@ -31,6 +31,7 @@ bzl_library(
         ":providers_bzl",
         ":py_internal_bzl",
         ":semantics_bzl",
+        "//python/private:reexports_bzl",
     ],
 )
 
@@ -59,6 +60,7 @@ bzl_library(
         ":providers_bzl",
         ":py_internal_bzl",
         ":semantics_bzl",
+        "//python/private:reexports_bzl",
     ],
 )
 

python/private/common/attributes.bzl

@@ -13,6 +13,7 @@
 # limitations under the License.
 """Attributes for Python rules."""
 
+load("//python/private:reexports.bzl", "BuiltinPyInfo")
 load(":common.bzl", "union_attrs")
 load(":providers.bzl", "PyInfo")
 load(":py_internal.bzl", "py_internal")
@@ -127,7 +128,11 @@ COMMON_ATTRS = union_attrs(
 PY_SRCS_ATTRS = union_attrs(
     {
         "deps": attr.label_list(
-            providers = [[PyInfo], [_CcInfo]],
+            providers = [
+                [PyInfo],
+                [_CcInfo],
+                [BuiltinPyInfo],
+            ],
             # TODO(b/228692666): Google-specific; remove these allowances once
             # the depot is cleaned up.
             allow_rules = DEPS_ATTR_ALLOW_RULES,

python/private/common/common.bzl

@@ -13,6 +13,7 @@
 # limitations under the License.
 """Various things common to Bazel and Google rule implementations."""
 
+load("//python/private:reexports.bzl", "BuiltinPyInfo")
 load(":cc_helper.bzl", "cc_helper")
 load(":providers.bzl", "PyInfo")
 load(":py_internal.bzl", "py_internal")
@@ -265,6 +266,10 @@ def collect_imports(ctx, semantics):
         dep[PyInfo].imports
         for dep in ctx.attr.deps
         if PyInfo in dep
+    ] + [
+        dep[BuiltinPyInfo].imports
+        for dep in ctx.attr.deps
+        if BuiltinPyInfo in dep
     ])
 
 def collect_runfiles(ctx, files):
@@ -355,8 +360,8 @@ def create_py_info(ctx, *, direct_sources, imports):
     transitive_sources_files = []  # list of Files
     for target in ctx.attr.deps:
         # PyInfo may not be present e.g. cc_library rules.
-        if PyInfo in target:
-            info = target[PyInfo]
+        if PyInfo in target or BuiltinPyInfo in target:
+            info = _get_py_info(target)
             transitive_sources_depsets.append(info.transitive_sources)
             uses_shared_libraries = uses_shared_libraries or info.uses_shared_libraries
             has_py2_only_sources = has_py2_only_sources or info.has_py2_only_sources
@@ -384,8 +389,8 @@ def create_py_info(ctx, *, direct_sources, imports):
         for target in ctx.attr.data:
             # TODO(b/234730058): Remove checking for PyInfo in data once depot
             # cleaned up.
-            if PyInfo in target:
-                info = target[PyInfo]
+            if PyInfo in target or BuiltinPyInfo in target:
+                info = _get_py_info(target)
                 uses_shared_libraries = info.uses_shared_libraries
             else:
                 files = target.files.to_list()
@@ -396,9 +401,7 @@ def create_py_info(ctx, *, direct_sources, imports):
             if uses_shared_libraries:
                 break
 
-    # TODO(b/203567235): Set `uses_shared_libraries` field, though the Bazel
-    # docs indicate it's unused in Bazel and may be removed.
-    py_info = PyInfo(
+    py_info_kwargs = dict(
         transitive_sources = depset(
             transitive = [deps_transitive_sources, direct_sources],
         ),
@@ -410,7 +413,16 @@ def create_py_info(ctx, *, direct_sources, imports):
         has_py3_only_sources = has_py3_only_sources,
         uses_shared_libraries = uses_shared_libraries,
     )
-    return py_info, deps_transitive_sources
+
+    # TODO(b/203567235): Set `uses_shared_libraries` field, though the Bazel
+    # docs indicate it's unused in Bazel and may be removed.
+    py_info = PyInfo(**py_info_kwargs)
+    builtin_py_info = BuiltinPyInfo(**py_info_kwargs)
+
+    return py_info, deps_transitive_sources, builtin_py_info
+
+def _get_py_info(target):
+    return target[PyInfo] if PyInfo in target else target[BuiltinPyInfo]
 
 def create_instrumented_files_info(ctx):
     return _coverage_common.instrumented_files_info(

python/private/common/py_executable.bzl

@@ -765,7 +765,7 @@ def _create_providers(
             PyCcLinkParamsProvider(cc_info = cc_info),
         )
 
-    py_info, deps_transitive_sources = create_py_info(
+    py_info, deps_transitive_sources, builtin_py_info = create_py_info(
         ctx,
         direct_sources = depset(direct_sources),
         imports = imports,
@@ -780,6 +780,7 @@ def _create_providers(
         )
 
     providers.append(py_info)
+    providers.append(builtin_py_info)
     providers.append(create_output_group_info(py_info.transitive_sources, output_groups))
 
     extra_legacy_providers, extra_providers = semantics.get_extra_providers(

python/private/common/py_library.bzl

@@ -61,7 +61,7 @@ def py_library_impl(ctx, *, semantics):
     runfiles = collect_runfiles(ctx = ctx, files = output_sources)
 
     cc_info = semantics.get_cc_info_for_library(ctx)
-    py_info, deps_transitive_sources = create_py_info(
+    py_info, deps_transitive_sources, builtins_py_info = create_py_info(
         ctx,
         direct_sources = depset(direct_sources),
         imports = collect_imports(ctx, semantics),
@@ -78,6 +78,7 @@ def py_library_impl(ctx, *, semantics):
     return [
         DefaultInfo(files = output_sources, runfiles = runfiles),
         py_info,
+        builtins_py_info,
         create_instrumented_files_info(ctx),
         PyCcLinkParamsProvider(cc_info = cc_info),
         create_output_group_info(py_info.transitive_sources, extra_groups = {}),

python/private/reexports.bzl

@@ -12,36 +12,29 @@
 # See the License for the specific language governing permissions and
 # limitations under the License.
 
-"""Internal re-exports of built-in symbols.
+"""Internal re-exports of builtin symbols.
 
-Currently the definitions here are re-exports of the native rules, "blessed" to
-work under `--incompatible_load_python_rules_from_bzl`. As the native rules get
-migrated to Starlark, their implementations will be removed from here.
+We want to use both the PyInfo defined by builtins and the one defined by
+rules_python. Because the builtin symbol is going away, the rules_python
+PyInfo symbol is given preference. Unfortunately, that masks the builtin,
+so we have to rebind it to another name and load it to make it available again.
 
-We want to re-export a built-in symbol as if it were defined in a Starlark
-file, so that users can for instance do:
-
-```
-load("@rules_python//python:defs.bzl", "PyInfo")
-```
-
-Unfortunately, we can't just write in defs.bzl
+Unfortunately, we can't just write:
 
 ```
 PyInfo = PyInfo
 ```
 
 because the declaration of module-level symbol `PyInfo` makes the builtin
 inaccessible. So instead we access the builtin here and export it under a
-different name. Then we can load it from defs.bzl and export it there under
-the original name.
+different name. Then we can load it from elsewhere.
 """
 
 # Don't use underscore prefix, since that would make the symbol local to this
 # file only. Use a non-conventional name to emphasize that this is not a public
 # symbol.
 # buildifier: disable=name-conventions
-internal_PyInfo = PyInfo
+BuiltinPyInfo = PyInfo
 
 # buildifier: disable=name-conventions
 internal_PyRuntimeInfo = PyRuntimeInfo

python/py_info.bzl

@@ -15,7 +15,7 @@
 """Public entry point for PyInfo."""
 
 load("@rules_python_internal//:rules_python_config.bzl", "config")
-load("//python/private:reexports.bzl", "internal_PyInfo")
+load("//python/private:reexports.bzl", "BuiltinPyInfo")
 load("//python/private/common:providers.bzl", _starlark_PyInfo = "PyInfo")
 
-PyInfo = _starlark_PyInfo if config.enable_pystar else internal_PyInfo
+PyInfo = _starlark_PyInfo if config.enable_pystar else BuiltinPyInfo

tests/base_rules/base_tests.bzl

@@ -17,17 +17,37 @@ load("@rules_testing//lib:analysis_test.bzl", "analysis_test")
 load("@rules_testing//lib:truth.bzl", "matching")
 load("@rules_testing//lib:util.bzl", "PREVENT_IMPLICIT_BUILDING_TAGS", rt_util = "util")
 load("//python:defs.bzl", "PyInfo")
+load("//python/private:reexports.bzl", "BuiltinPyInfo")  # buildifier: disable=bzl-visibility
 load("//tests/base_rules:py_info_subject.bzl", "py_info_subject")
 load("//tests/base_rules:util.bzl", pt_util = "util")
 
 _tests = []
 
+_PRODUCES_PY_INFO_ATTRS = {
+    "imports": attr.string_list(),
+    "srcs": attr.label_list(allow_files = True),
+}
+
+def _create_py_info(ctx, provider_type):
+    return [provider_type(
+        transitive_sources = depset(ctx.files.srcs),
+        imports = depset(ctx.attr.imports),
+    )]
+
+def _produces_builtin_py_info_impl(ctx):
+    return _create_py_info(ctx, BuiltinPyInfo)
+
+_produces_builtin_py_info = rule(
+    implementation = _produces_builtin_py_info_impl,
+    attrs = _PRODUCES_PY_INFO_ATTRS,
+)
+
 def _produces_py_info_impl(ctx):
-    return [PyInfo(transitive_sources = depset(ctx.files.srcs))]
+    return _create_py_info(ctx, BuiltinPyInfo)
 
 _produces_py_info = rule(
     implementation = _produces_py_info_impl,
-    attrs = {"srcs": attr.label_list(allow_files = True)},
+    attrs = _PRODUCES_PY_INFO_ATTRS,
 )
 
 def _not_produces_py_info_impl(ctx):
@@ -38,30 +58,58 @@ _not_produces_py_info = rule(
     implementation = _not_produces_py_info_impl,
 )
 
-def _test_consumes_provider(name, config):
+def _py_info_propagation_setup(name, config, produce_py_info_rule, test_impl):
     rt_util.helper_target(
         config.base_test_rule,
         name = name + "_subject",
-        deps = [name + "_produces_py_info"],
+        deps = [name + "_produces_builtin_py_info"],
     )
     rt_util.helper_target(
-        _produces_py_info,
-        name = name + "_produces_py_info",
+        produce_py_info_rule,
+        name = name + "_produces_builtin_py_info",
         srcs = [rt_util.empty_file(name + "_produce.py")],
+        imports = ["custom-import"],
     )
     analysis_test(
         name = name,
         target = name + "_subject",
-        impl = _test_consumes_provider_impl,
+        impl = test_impl,
     )
 
-def _test_consumes_provider_impl(env, target):
-    env.expect.that_target(target).provider(
-        PyInfo,
+def _py_info_propagation_test_impl(env, target, provider_type):
+    info = env.expect.that_target(target).provider(
+        provider_type,
         factory = py_info_subject,
-    ).transitive_sources().contains("{package}/{test_name}_produce.py")
+    )
+
+    info.transitive_sources().contains("{package}/{test_name}_produce.py")
+    info.imports().contains("custom-import")
+
+def _test_py_info_propagation_builtin(name, config):
+    _py_info_propagation_setup(
+        name,
+        config,
+        _produces_builtin_py_info,
+        _test_py_info_propagation_builtin_impl,
+    )
+
+def _test_py_info_propagation_builtin_impl(env, target):
+    _py_info_propagation_test_impl(env, target, BuiltinPyInfo)
+
+_tests.append(_test_py_info_propagation_builtin)
+
+def _test_py_info_propagation(name, config):
+    _py_info_propagation_setup(
+        name,
+        config,
+        _produces_py_info,
+        _test_py_info_propagation_impl,
+    )
+
+def _test_py_info_propagation_impl(env, target):
+    _py_info_propagation_test_impl(env, target, PyInfo)
 
-_tests.append(_test_consumes_provider)
+_tests.append(_test_py_info_propagation)
 
 def _test_requires_provider(name, config):
     rt_util.helper_target(

tests/base_rules/py_info_subject.bzl

@@ -70,7 +70,7 @@ def _py_info_subject_imports(self):
     Method: PyInfoSubject.imports
     """
     return subjects.collection(
-        self.actual.imports,
+        self.actual.imports.to_list(),
         meta = self.meta.derive("imports()"),
     )