Skip to content

Commit f676656

Browse files
rickeylevrickeylev
authored
pystar: support builtin providers for compatibility (bazel-contrib#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 bazel-contrib#1069
1 parent cde1b52 commit f676656

11 files changed

Lines changed: 106 additions & 47 deletions

File tree

‎python/defs.bzl‎

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,7 @@
1515

1616
load("@bazel_tools//tools/python:srcs_version.bzl", _find_requirements = "find_requirements")
1717
load("//python:py_binary.bzl", _py_binary = "py_binary")
18-
load("//python:py_info.bzl", internal_PyInfo = "PyInfo")
18+
load("//python:py_info.bzl", _PyInfo = "PyInfo")
1919
load("//python:py_library.bzl", _py_library = "py_library")
2020
load("//python:py_runtime.bzl", _py_runtime = "py_runtime")
2121
load("//python:py_runtime_info.bzl", internal_PyRuntimeInfo = "PyRuntimeInfo")
@@ -26,9 +26,7 @@ load(":py_import.bzl", _py_import = "py_import")
2626

2727
# Patching placeholder: end of loads
2828

29-
# Exports of native-defined providers.
30-
31-
PyInfo = internal_PyInfo
29+
PyInfo = _PyInfo
3230

3331
PyRuntimeInfo = internal_PyRuntimeInfo
3432

‎python/private/BUILD.bazel‎

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -169,8 +169,7 @@ bzl_library(
169169
name = "reexports_bzl",
170170
srcs = ["reexports.bzl"],
171171
visibility = [
172-
"//docs:__pkg__",
173-
"//python:__pkg__",
172+
"//:__subpackages__",
174173
],
175174
deps = [":bazel_tools_bzl"],
176175
)

‎python/private/common/BUILD.bazel‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,7 @@ bzl_library(
3131
":providers_bzl",
3232
":py_internal_bzl",
3333
":semantics_bzl",
34+
"//python/private:reexports_bzl",
3435
],
3536
)
3637

@@ -59,6 +60,7 @@ bzl_library(
5960
":providers_bzl",
6061
":py_internal_bzl",
6162
":semantics_bzl",
63+
"//python/private:reexports_bzl",
6264
],
6365
)
6466

‎python/private/common/attributes.bzl‎

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@
1313
# limitations under the License.
1414
"""Attributes for Python rules."""
1515

16+
load("//python/private:reexports.bzl", "BuiltinPyInfo")
1617
load(":common.bzl", "union_attrs")
1718
load(":providers.bzl", "PyInfo")
1819
load(":py_internal.bzl", "py_internal")
@@ -127,7 +128,11 @@ COMMON_ATTRS = union_attrs(
127128
PY_SRCS_ATTRS = union_attrs(
128129
{
129130
"deps": attr.label_list(
130-
providers = [[PyInfo], [_CcInfo]],
131+
providers = [
132+
[PyInfo],
133+
[_CcInfo],
134+
[BuiltinPyInfo],
135+
],
131136
# TODO(b/228692666): Google-specific; remove these allowances once
132137
# the depot is cleaned up.
133138
allow_rules = DEPS_ATTR_ALLOW_RULES,

‎python/private/common/common.bzl‎

Lines changed: 20 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@
1313
# limitations under the License.
1414
"""Various things common to Bazel and Google rule implementations."""
1515

16+
load("//python/private:reexports.bzl", "BuiltinPyInfo")
1617
load(":cc_helper.bzl", "cc_helper")
1718
load(":providers.bzl", "PyInfo")
1819
load(":py_internal.bzl", "py_internal")
@@ -265,6 +266,10 @@ def collect_imports(ctx, semantics):
265266
dep[PyInfo].imports
266267
for dep in ctx.attr.deps
267268
if PyInfo in dep
269+
] + [
270+
dep[BuiltinPyInfo].imports
271+
for dep in ctx.attr.deps
272+
if BuiltinPyInfo in dep
268273
])
269274

270275
def collect_runfiles(ctx, files):
@@ -355,8 +360,8 @@ def create_py_info(ctx, *, direct_sources, imports):
355360
transitive_sources_files = [] # list of Files
356361
for target in ctx.attr.deps:
357362
# PyInfo may not be present e.g. cc_library rules.
358-
if PyInfo in target:
359-
info = target[PyInfo]
363+
if PyInfo in target or BuiltinPyInfo in target:
364+
info = _get_py_info(target)
360365
transitive_sources_depsets.append(info.transitive_sources)
361366
uses_shared_libraries = uses_shared_libraries or info.uses_shared_libraries
362367
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):
384389
for target in ctx.attr.data:
385390
# TODO(b/234730058): Remove checking for PyInfo in data once depot
386391
# cleaned up.
387-
if PyInfo in target:
388-
info = target[PyInfo]
392+
if PyInfo in target or BuiltinPyInfo in target:
393+
info = _get_py_info(target)
389394
uses_shared_libraries = info.uses_shared_libraries
390395
else:
391396
files = target.files.to_list()
@@ -396,9 +401,7 @@ def create_py_info(ctx, *, direct_sources, imports):
396401
if uses_shared_libraries:
397402
break
398403

399-
# TODO(b/203567235): Set `uses_shared_libraries` field, though the Bazel
400-
# docs indicate it's unused in Bazel and may be removed.
401-
py_info = PyInfo(
404+
py_info_kwargs = dict(
402405
transitive_sources = depset(
403406
transitive = [deps_transitive_sources, direct_sources],
404407
),
@@ -410,7 +413,16 @@ def create_py_info(ctx, *, direct_sources, imports):
410413
has_py3_only_sources = has_py3_only_sources,
411414
uses_shared_libraries = uses_shared_libraries,
412415
)
413-
return py_info, deps_transitive_sources
416+
417+
# TODO(b/203567235): Set `uses_shared_libraries` field, though the Bazel
418+
# docs indicate it's unused in Bazel and may be removed.
419+
py_info = PyInfo(**py_info_kwargs)
420+
builtin_py_info = BuiltinPyInfo(**py_info_kwargs)
421+
422+
return py_info, deps_transitive_sources, builtin_py_info
423+
424+
def _get_py_info(target):
425+
return target[PyInfo] if PyInfo in target else target[BuiltinPyInfo]
414426

415427
def create_instrumented_files_info(ctx):
416428
return _coverage_common.instrumented_files_info(

‎python/private/common/py_executable.bzl‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -765,7 +765,7 @@ def _create_providers(
765765
PyCcLinkParamsProvider(cc_info = cc_info),
766766
)
767767

768-
py_info, deps_transitive_sources = create_py_info(
768+
py_info, deps_transitive_sources, builtin_py_info = create_py_info(
769769
ctx,
770770
direct_sources = depset(direct_sources),
771771
imports = imports,
@@ -780,6 +780,7 @@ def _create_providers(
780780
)
781781

782782
providers.append(py_info)
783+
providers.append(builtin_py_info)
783784
providers.append(create_output_group_info(py_info.transitive_sources, output_groups))
784785

785786
extra_legacy_providers, extra_providers = semantics.get_extra_providers(

‎python/private/common/py_library.bzl‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -61,7 +61,7 @@ def py_library_impl(ctx, *, semantics):
6161
runfiles = collect_runfiles(ctx = ctx, files = output_sources)
6262

6363
cc_info = semantics.get_cc_info_for_library(ctx)
64-
py_info, deps_transitive_sources = create_py_info(
64+
py_info, deps_transitive_sources, builtins_py_info = create_py_info(
6565
ctx,
6666
direct_sources = depset(direct_sources),
6767
imports = collect_imports(ctx, semantics),
@@ -78,6 +78,7 @@ def py_library_impl(ctx, *, semantics):
7878
return [
7979
DefaultInfo(files = output_sources, runfiles = runfiles),
8080
py_info,
81+
builtins_py_info,
8182
create_instrumented_files_info(ctx),
8283
PyCcLinkParamsProvider(cc_info = cc_info),
8384
create_output_group_info(py_info.transitive_sources, extra_groups = {}),

‎python/private/reexports.bzl‎

Lines changed: 8 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -12,36 +12,29 @@
1212
# See the License for the specific language governing permissions and
1313
# limitations under the License.
1414

15-
"""Internal re-exports of built-in symbols.
15+
"""Internal re-exports of builtin symbols.
1616
17-
Currently the definitions here are re-exports of the native rules, "blessed" to
18-
work under `--incompatible_load_python_rules_from_bzl`. As the native rules get
19-
migrated to Starlark, their implementations will be removed from here.
17+
We want to use both the PyInfo defined by builtins and the one defined by
18+
rules_python. Because the builtin symbol is going away, the rules_python
19+
PyInfo symbol is given preference. Unfortunately, that masks the builtin,
20+
so we have to rebind it to another name and load it to make it available again.
2021
21-
We want to re-export a built-in symbol as if it were defined in a Starlark
22-
file, so that users can for instance do:
23-
24-
```
25-
load("@rules_python//python:defs.bzl", "PyInfo")
26-
```
27-
28-
Unfortunately, we can't just write in defs.bzl
22+
Unfortunately, we can't just write:
2923
3024
```
3125
PyInfo = PyInfo
3226
```
3327
3428
because the declaration of module-level symbol `PyInfo` makes the builtin
3529
inaccessible. So instead we access the builtin here and export it under a
36-
different name. Then we can load it from defs.bzl and export it there under
37-
the original name.
30+
different name. Then we can load it from elsewhere.
3831
"""
3932

4033
# Don't use underscore prefix, since that would make the symbol local to this
4134
# file only. Use a non-conventional name to emphasize that this is not a public
4235
# symbol.
4336
# buildifier: disable=name-conventions
44-
internal_PyInfo = PyInfo
37+
BuiltinPyInfo = PyInfo
4538

4639
# buildifier: disable=name-conventions
4740
internal_PyRuntimeInfo = PyRuntimeInfo

‎python/py_info.bzl‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,7 @@
1515
"""Public entry point for PyInfo."""
1616

1717
load("@rules_python_internal//:rules_python_config.bzl", "config")
18-
load("//python/private:reexports.bzl", "internal_PyInfo")
18+
load("//python/private:reexports.bzl", "BuiltinPyInfo")
1919
load("//python/private/common:providers.bzl", _starlark_PyInfo = "PyInfo")
2020

21-
PyInfo = _starlark_PyInfo if config.enable_pystar else internal_PyInfo
21+
PyInfo = _starlark_PyInfo if config.enable_pystar else BuiltinPyInfo

‎tests/base_rules/base_tests.bzl‎

Lines changed: 60 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -17,17 +17,37 @@ load("@rules_testing//lib:analysis_test.bzl", "analysis_test")
1717
load("@rules_testing//lib:truth.bzl", "matching")
1818
load("@rules_testing//lib:util.bzl", "PREVENT_IMPLICIT_BUILDING_TAGS", rt_util = "util")
1919
load("//python:defs.bzl", "PyInfo")
20+
load("//python/private:reexports.bzl", "BuiltinPyInfo") # buildifier: disable=bzl-visibility
2021
load("//tests/base_rules:py_info_subject.bzl", "py_info_subject")
2122
load("//tests/base_rules:util.bzl", pt_util = "util")
2223

2324
_tests = []
2425

26+
_PRODUCES_PY_INFO_ATTRS = {
27+
"imports": attr.string_list(),
28+
"srcs": attr.label_list(allow_files = True),
29+
}
30+
31+
def _create_py_info(ctx, provider_type):
32+
return [provider_type(
33+
transitive_sources = depset(ctx.files.srcs),
34+
imports = depset(ctx.attr.imports),
35+
)]
36+
37+
def _produces_builtin_py_info_impl(ctx):
38+
return _create_py_info(ctx, BuiltinPyInfo)
39+
40+
_produces_builtin_py_info = rule(
41+
implementation = _produces_builtin_py_info_impl,
42+
attrs = _PRODUCES_PY_INFO_ATTRS,
43+
)
44+
2545
def _produces_py_info_impl(ctx):
26-
return [PyInfo(transitive_sources = depset(ctx.files.srcs))]
46+
return _create_py_info(ctx, BuiltinPyInfo)
2747

2848
_produces_py_info = rule(
2949
implementation = _produces_py_info_impl,
30-
attrs = {"srcs": attr.label_list(allow_files = True)},
50+
attrs = _PRODUCES_PY_INFO_ATTRS,
3151
)
3252

3353
def _not_produces_py_info_impl(ctx):
@@ -38,30 +58,58 @@ _not_produces_py_info = rule(
3858
implementation = _not_produces_py_info_impl,
3959
)
4060

41-
def _test_consumes_provider(name, config):
61+
def _py_info_propagation_setup(name, config, produce_py_info_rule, test_impl):
4262
rt_util.helper_target(
4363
config.base_test_rule,
4464
name = name + "_subject",
45-
deps = [name + "_produces_py_info"],
65+
deps = [name + "_produces_builtin_py_info"],
4666
)
4767
rt_util.helper_target(
48-
_produces_py_info,
49-
name = name + "_produces_py_info",
68+
produce_py_info_rule,
69+
name = name + "_produces_builtin_py_info",
5070
srcs = [rt_util.empty_file(name + "_produce.py")],
71+
imports = ["custom-import"],
5172
)
5273
analysis_test(
5374
name = name,
5475
target = name + "_subject",
55-
impl = _test_consumes_provider_impl,
76+
impl = test_impl,
5677
)
5778

58-
def _test_consumes_provider_impl(env, target):
59-
env.expect.that_target(target).provider(
60-
PyInfo,
79+
def _py_info_propagation_test_impl(env, target, provider_type):
80+
info = env.expect.that_target(target).provider(
81+
provider_type,
6182
factory = py_info_subject,
62-
).transitive_sources().contains("{package}/{test_name}_produce.py")
83+
)
84+
85+
info.transitive_sources().contains("{package}/{test_name}_produce.py")
86+
info.imports().contains("custom-import")
87+
88+
def _test_py_info_propagation_builtin(name, config):
89+
_py_info_propagation_setup(
90+
name,
91+
config,
92+
_produces_builtin_py_info,
93+
_test_py_info_propagation_builtin_impl,
94+
)
95+
96+
def _test_py_info_propagation_builtin_impl(env, target):
97+
_py_info_propagation_test_impl(env, target, BuiltinPyInfo)
98+
99+
_tests.append(_test_py_info_propagation_builtin)
100+
101+
def _test_py_info_propagation(name, config):
102+
_py_info_propagation_setup(
103+
name,
104+
config,
105+
_produces_py_info,
106+
_test_py_info_propagation_impl,
107+
)
108+
109+
def _test_py_info_propagation_impl(env, target):
110+
_py_info_propagation_test_impl(env, target, PyInfo)
63111

64-
_tests.append(_test_consumes_provider)
112+
_tests.append(_test_py_info_propagation)
65113

66114
def _test_requires_provider(name, config):
67115
rt_util.helper_target(

0 commit comments

Comments
 (0)