Skip to content

Commit 18380a2

Browse files
rickeylevrickeylev
authored
refactor: add builders to make creating PyInfo/depset/runfiles easier (bazel-contrib#2251)
This add some builders "classes" to make constructing PyInfo, depsets, and runfiles a bit cleaner. Previously, building such objects required creating multiple local variables for each arg/attribute of the object (e.g. list_of_files and list_of_depsets for building a depset). Keeping track of all the variables was verbose and tedious. To make it easier, encapsulate that bookkeeping in some objects. Accumulating parts is now much easier: builders have an `add()` method that looks at the input and stores it in the appropriate place. This builders will be exposed to users in an upcoming PR to make building and merging PyInfo objects easier. Work towards bazel-contrib#1647
1 parent 9a98e8c commit 18380a2

10 files changed

Lines changed: 728 additions & 79 deletions

File tree

‎python/private/BUILD.bazel‎

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -69,6 +69,14 @@ bzl_library(
6969
],
7070
)
7171

72+
bzl_library(
73+
name = "builders_bzl",
74+
srcs = ["builders.bzl"],
75+
deps = [
76+
"@bazel_skylib//lib:types",
77+
],
78+
)
79+
7280
bzl_library(
7381
name = "bzlmod_enabled_bzl",
7482
srcs = ["bzlmod_enabled.bzl"],
@@ -270,8 +278,10 @@ bzl_library(
270278
name = "py_info_bzl",
271279
srcs = ["py_info.bzl"],
272280
deps = [
281+
":builders_bzl",
273282
":reexports_bzl",
274283
":util_bzl",
284+
"@rules_python_internal//:rules_python_config_bzl",
275285
],
276286
)
277287

‎python/private/builders.bzl‎

Lines changed: 190 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,190 @@
1+
# Copyright 2024 The Bazel Authors. All rights reserved.
2+
#
3+
# Licensed under the Apache License, Version 2.0 (the "License");
4+
# you may not use this file except in compliance with the License.
5+
# You may obtain a copy of the License at
6+
#
7+
# http://www.apache.org/licenses/LICENSE-2.0
8+
#
9+
# Unless required by applicable law or agreed to in writing, software
10+
# distributed under the License is distributed on an "AS IS" BASIS,
11+
# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
12+
# See the License for the specific language governing permissions and
13+
# limitations under the License.
14+
"""Builders to make building complex objects easier."""
15+
16+
load("@bazel_skylib//lib:types.bzl", "types")
17+
18+
def _DepsetBuilder():
19+
"""Create a builder for a depset."""
20+
21+
# buildifier: disable=uninitialized
22+
self = struct(
23+
_order = [None],
24+
add = lambda *a, **k: _DepsetBuilder_add(self, *a, **k),
25+
build = lambda *a, **k: _DepsetBuilder_build(self, *a, **k),
26+
direct = [],
27+
get_order = lambda *a, **k: _DepsetBuilder_get_order(self, *a, **k),
28+
set_order = lambda *a, **k: _DepsetBuilder_set_order(self, *a, **k),
29+
transitive = [],
30+
)
31+
return self
32+
33+
def _DepsetBuilder_add(self, *values):
34+
"""Add value to the depset.
35+
36+
Args:
37+
self: {type}`DepsetBuilder` implicitly added.
38+
*values: {type}`depset | list | object` Values to add to the depset.
39+
The values can be a depset, the non-depset value to add, or
40+
a list of such values to add.
41+
42+
Returns:
43+
{type}`DepsetBuilder`
44+
"""
45+
for value in values:
46+
if types.is_list(value):
47+
for sub_value in value:
48+
if types.is_depset(sub_value):
49+
self.transitive.append(sub_value)
50+
else:
51+
self.direct.append(sub_value)
52+
elif types.is_depset(value):
53+
self.transitive.append(value)
54+
else:
55+
self.direct.append(value)
56+
return self
57+
58+
def _DepsetBuilder_set_order(self, order):
59+
"""Sets the order to use.
60+
61+
Args:
62+
self: {type}`DepsetBuilder` implicitly added.
63+
order: {type}`str` One of the {obj}`depset` `order` values.
64+
65+
Returns:
66+
{type}`DepsetBuilder`
67+
"""
68+
self._order[0] = order
69+
return self
70+
71+
def _DepsetBuilder_get_order(self):
72+
"""Gets the depset order that will be used.
73+
74+
Args:
75+
self: {type}`DepsetBuilder` implicitly added.
76+
77+
Returns:
78+
{type}`str | None` If not previously set, `None` is returned.
79+
"""
80+
return self._order[0]
81+
82+
def _DepsetBuilder_build(self):
83+
"""Creates a {obj}`depset` from the accumulated values.
84+
85+
Args:
86+
self: {type}`DepsetBuilder` implicitly added.
87+
88+
Returns:
89+
{type}`depset`
90+
"""
91+
if not self.direct and len(self.transitive) == 1 and self._order[0] == None:
92+
return self.transitive[0]
93+
else:
94+
kwargs = {}
95+
if self._order[0] != None:
96+
kwargs["order"] = self._order[0]
97+
return depset(direct = self.direct, transitive = self.transitive, **kwargs)
98+
99+
def _RunfilesBuilder():
100+
"""Creates a `RunfilesBuilder`.
101+
102+
Returns:
103+
{type}`RunfilesBuilder`
104+
"""
105+
106+
# buildifier: disable=uninitialized
107+
self = struct(
108+
add = lambda *a, **k: _RunfilesBuilder_add(self, *a, **k),
109+
add_targets = lambda *a, **k: _RunfilesBuilder_add_targets(self, *a, **k),
110+
build = lambda *a, **k: _RunfilesBuilder_build(self, *a, **k),
111+
files = _DepsetBuilder(),
112+
root_symlinks = {},
113+
runfiles = [],
114+
symlinks = {},
115+
)
116+
return self
117+
118+
def _RunfilesBuilder_add(self, *values):
119+
"""Adds a value to the runfiles.
120+
121+
Args:
122+
self: {type}`RunfilesBuilder` implicitly added.
123+
*values: {type}`File | runfiles | list[File] | depset[File] | list[runfiles]`
124+
The values to add.
125+
126+
Returns:
127+
{type}`RunfilesBuilder`
128+
"""
129+
for value in values:
130+
if types.is_list(value):
131+
for sub_value in value:
132+
_RunfilesBuilder_add_internal(self, sub_value)
133+
else:
134+
_RunfilesBuilder_add_internal(self, value)
135+
return self
136+
137+
def _RunfilesBuilder_add_targets(self, targets):
138+
"""Adds runfiles from targets
139+
140+
Args:
141+
self: {type}`RunfilesBuilder` implicitly added.
142+
targets: {type}`list[Target]` targets whose default runfiles
143+
to add.
144+
145+
Returns:
146+
{type}`RunfilesBuilder`
147+
"""
148+
for t in targets:
149+
self.runfiles.append(t[DefaultInfo].default_runfiles)
150+
return self
151+
152+
def _RunfilesBuilder_add_internal(self, value):
153+
if _is_file(value):
154+
self.files.add(value)
155+
elif types.is_depset(value):
156+
self.files.add(value)
157+
elif _is_runfiles(value):
158+
self.runfiles.append(value)
159+
else:
160+
fail("Unhandled value: type {}: {}".format(type(value), value))
161+
162+
def _RunfilesBuilder_build(self, ctx, **kwargs):
163+
"""Creates a {obj}`runfiles` from the accumulated values.
164+
165+
Args:
166+
self: {type}`RunfilesBuilder` implicitly added.
167+
ctx: {type}`ctx` The rule context to use to create the runfiles object.
168+
**kwargs: additional args to pass along to {obj}`ctx.runfiles`.
169+
170+
Returns:
171+
{type}`runfiles`
172+
"""
173+
return ctx.runfiles(
174+
transitive_files = self.files.build(),
175+
symlinks = self.symlinks,
176+
root_symlinks = self.root_symlinks,
177+
**kwargs
178+
).merge_all(self.runfiles)
179+
180+
# Skylib's types module doesn't have is_file, so roll our own
181+
def _is_file(value):
182+
return type(value) == "File"
183+
184+
def _is_runfiles(value):
185+
return type(value) == "runfiles"
186+
187+
builders = struct(
188+
DepsetBuilder = _DepsetBuilder,
189+
RunfilesBuilder = _RunfilesBuilder,
190+
)

‎python/private/common/common.bzl‎

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

16-
load("//python/private:py_info.bzl", "PyInfo")
16+
load("//python/private:py_info.bzl", "PyInfo", "PyInfoBuilder")
1717
load("//python/private:reexports.bzl", "BuiltinPyInfo")
1818
load(":cc_helper.bzl", "cc_helper")
1919
load(":py_internal.bzl", "py_internal")
@@ -282,7 +282,7 @@ def collect_imports(ctx, semantics):
282282
if BuiltinPyInfo in dep
283283
])
284284

285-
def collect_runfiles(ctx, files):
285+
def collect_runfiles(ctx, files = depset()):
286286
"""Collects the necessary files from the rule's context.
287287
288288
This presumes the ctx is for a py_binary, py_test, or py_library rule.
@@ -364,84 +364,50 @@ def create_py_info(ctx, *, direct_sources, direct_pyc_files, imports):
364364
transitive sources collected from dependencies (the latter is only
365365
necessary for deprecated extra actions support).
366366
"""
367-
uses_shared_libraries = False
368-
has_py2_only_sources = ctx.attr.srcs_version in ("PY2", "PY2ONLY")
369-
has_py3_only_sources = ctx.attr.srcs_version in ("PY3", "PY3ONLY")
370-
transitive_sources_depsets = [] # list of depsets
371-
transitive_sources_files = [] # list of Files
372-
transitive_pyc_depsets = [direct_pyc_files] # list of depsets
367+
368+
py_info = PyInfoBuilder()
369+
py_info.direct_pyc_files.add(direct_pyc_files)
370+
py_info.transitive_pyc_files.add(direct_pyc_files)
371+
py_info.imports.add(imports)
372+
py_info.merge_has_py2_only_sources(ctx.attr.srcs_version in ("PY2", "PY2ONLY"))
373+
py_info.merge_has_py3_only_sources(ctx.attr.srcs_version in ("PY3", "PY3ONLY"))
374+
373375
for target in ctx.attr.deps:
374376
# PyInfo may not be present e.g. cc_library rules.
375377
if PyInfo in target or BuiltinPyInfo in target:
376-
info = _get_py_info(target)
377-
transitive_sources_depsets.append(info.transitive_sources)
378-
uses_shared_libraries = uses_shared_libraries or info.uses_shared_libraries
379-
has_py2_only_sources = has_py2_only_sources or info.has_py2_only_sources
380-
has_py3_only_sources = has_py3_only_sources or info.has_py3_only_sources
381-
382-
# BuiltinPyInfo doesn't have this field.
383-
if hasattr(info, "transitive_pyc_files"):
384-
transitive_pyc_depsets.append(info.transitive_pyc_files)
378+
py_info.merge(_get_py_info(target))
385379
else:
386380
# TODO(b/228692666): Remove this once non-PyInfo targets are no
387381
# longer supported in `deps`.
388382
files = target.files.to_list()
389383
for f in files:
390384
if f.extension == "py":
391-
transitive_sources_files.append(f)
392-
uses_shared_libraries = (
393-
uses_shared_libraries or
394-
cc_helper.is_valid_shared_library_artifact(f)
395-
)
396-
deps_transitive_sources = depset(
397-
direct = transitive_sources_files,
398-
transitive = transitive_sources_depsets,
399-
)
385+
py_info.transitive_sources.add(f)
386+
py_info.merge_uses_shared_libraries(cc_helper.is_valid_shared_library_artifact(f))
387+
388+
deps_transitive_sources = py_info.transitive_sources.build()
389+
py_info.transitive_sources.add(direct_sources)
400390

401391
# We only look at data to calculate uses_shared_libraries, if it's already
402392
# true, then we don't need to waste time looping over it.
403-
if not uses_shared_libraries:
393+
if not py_info.get_uses_shared_libraries():
404394
# Similar to the above, except we only calculate uses_shared_libraries
405395
for target in ctx.attr.data:
406396
# TODO(b/234730058): Remove checking for PyInfo in data once depot
407397
# cleaned up.
408398
if PyInfo in target or BuiltinPyInfo in target:
409399
info = _get_py_info(target)
410-
uses_shared_libraries = info.uses_shared_libraries
400+
py_info.merge_uses_shared_libraries(info.uses_shared_libraries)
411401
else:
412402
files = target.files.to_list()
413403
for f in files:
414-
uses_shared_libraries = cc_helper.is_valid_shared_library_artifact(f)
415-
if uses_shared_libraries:
404+
py_info.merge_uses_shared_libraries(cc_helper.is_valid_shared_library_artifact(f))
405+
if py_info.get_uses_shared_libraries():
416406
break
417-
if uses_shared_libraries:
407+
if py_info.get_uses_shared_libraries():
418408
break
419409

420-
py_info_kwargs = dict(
421-
transitive_sources = depset(
422-
transitive = [deps_transitive_sources, direct_sources],
423-
),
424-
imports = imports,
425-
# NOTE: This isn't strictly correct, but with Python 2 gone,
426-
# the srcs_version logic is largely defunct, so shouldn't matter in
427-
# practice.
428-
has_py2_only_sources = has_py2_only_sources,
429-
has_py3_only_sources = has_py3_only_sources,
430-
uses_shared_libraries = uses_shared_libraries,
431-
direct_pyc_files = direct_pyc_files,
432-
transitive_pyc_files = depset(transitive = transitive_pyc_depsets),
433-
)
434-
435-
# TODO(b/203567235): Set `uses_shared_libraries` field, though the Bazel
436-
# docs indicate it's unused in Bazel and may be removed.
437-
py_info = PyInfo(**py_info_kwargs)
438-
439-
# Remove args that BuiltinPyInfo doesn't support
440-
py_info_kwargs.pop("direct_pyc_files")
441-
py_info_kwargs.pop("transitive_pyc_files")
442-
builtin_py_info = BuiltinPyInfo(**py_info_kwargs)
443-
444-
return py_info, deps_transitive_sources, builtin_py_info
410+
return py_info.build(), deps_transitive_sources, py_info.build_builtin_py_info()
445411

446412
def _get_py_info(target):
447413
return target[PyInfo] if PyInfo in target else target[BuiltinPyInfo]

0 commit comments

Comments
 (0)