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
diff --git a/python/defs.bzl b/python/defs.bzl
index 3fb6b5b..bd89f5b 100644
--- a/python/defs.bzl
+++ b/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 @@
# Patching placeholder: end of loads
-# Exports of native-defined providers.
-
-PyInfo = internal_PyInfo
+PyInfo = _PyInfo
PyRuntimeInfo = internal_PyRuntimeInfo
diff --git a/python/private/BUILD.bazel b/python/private/BUILD.bazel
index d5b170e..04c7af6 100644
--- a/python/private/BUILD.bazel
+++ b/python/private/BUILD.bazel
@@ -169,8 +169,7 @@
name = "reexports_bzl",
srcs = ["reexports.bzl"],
visibility = [
- "//docs:__pkg__",
- "//python:__pkg__",
+ "//:__subpackages__",
],
deps = [":bazel_tools_bzl"],
)
diff --git a/python/private/common/BUILD.bazel b/python/private/common/BUILD.bazel
index f20e682..b180e3d 100644
--- a/python/private/common/BUILD.bazel
+++ b/python/private/common/BUILD.bazel
@@ -31,6 +31,7 @@
":providers_bzl",
":py_internal_bzl",
":semantics_bzl",
+ "//python/private:reexports_bzl",
],
)
@@ -59,6 +60,7 @@
":providers_bzl",
":py_internal_bzl",
":semantics_bzl",
+ "//python/private:reexports_bzl",
],
)
diff --git a/python/private/common/attributes.bzl b/python/private/common/attributes.bzl
index b1c54a0..b26d02c 100644
--- a/python/private/common/attributes.bzl
+++ b/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 @@
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,
diff --git a/python/private/common/common.bzl b/python/private/common/common.bzl
index 84b2aa5..75c117f 100644
--- a/python/private/common/common.bzl
+++ b/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 @@
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 @@
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 @@
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 @@
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 @@
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(
diff --git a/python/private/common/py_executable.bzl b/python/private/common/py_executable.bzl
index d188b3a..a24bad6 100644
--- a/python/private/common/py_executable.bzl
+++ b/python/private/common/py_executable.bzl
@@ -765,7 +765,7 @@
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 @@
)
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(
diff --git a/python/private/common/py_library.bzl b/python/private/common/py_library.bzl
index 8d09c51..28ee7bf 100644
--- a/python/private/common/py_library.bzl
+++ b/python/private/common/py_library.bzl
@@ -61,7 +61,7 @@
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 @@
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 = {}),
diff --git a/python/private/reexports.bzl b/python/private/reexports.bzl
index a300a20..af5b394 100644
--- a/python/private/reexports.bzl
+++ b/python/private/reexports.bzl
@@ -12,20 +12,14 @@
# 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
@@ -33,15 +27,14 @@
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
diff --git a/python/py_info.bzl b/python/py_info.bzl
index cbf145d..0af35ac 100644
--- a/python/py_info.bzl
+++ b/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
diff --git a/tests/base_rules/base_tests.bzl b/tests/base_rules/base_tests.bzl
index 99a35f9..fb95c15 100644
--- a/tests/base_rules/base_tests.bzl
+++ b/tests/base_rules/base_tests.bzl
@@ -17,17 +17,37 @@
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 @@
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")
+ )
-_tests.append(_test_consumes_provider)
+ 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_py_info_propagation)
def _test_requires_provider(name, config):
rt_util.helper_target(
diff --git a/tests/base_rules/py_info_subject.bzl b/tests/base_rules/py_info_subject.bzl
index 20185e5..b23308c 100644
--- a/tests/base_rules/py_info_subject.bzl
+++ b/tests/base_rules/py_info_subject.bzl
@@ -70,7 +70,7 @@
Method: PyInfoSubject.imports
"""
return subjects.collection(
- self.actual.imports,
+ self.actual.imports.to_list(),
meta = self.meta.derive("imports()"),
)
