Skip to content

Commit beed670

Browse files
committed
And now for something completely different...
Experimenting with a different approach
1 parent c51d29a commit beed670

File tree

2 files changed

+129
-53
lines changed

2 files changed

+129
-53
lines changed

refresh.template.py

+8-10
Original file line numberDiff line numberDiff line change
@@ -1418,10 +1418,11 @@ def main():
14181418
# the compile commands for the targets defined for those collections.
14191419
compile_command_sets = collections.defaultdict(list)
14201420

1421-
for (target, flags) in target_flag_pairs:
1421+
for target_key, target_data in target_flag_pairs:
1422+
target, flags = target_data
14221423
commands = list(_get_commands(target, flags))
14231424
# If the target is assigned to any collections, put the compile commands in the compile command sets for those collections.
1424-
collections_for_target = [collection_name for target_name, collection_name in target_collections.items() if target == target_name]
1425+
collections_for_target = [collection_name for collection_name, target_keys in target_collections.items() if target_key in target_keys]
14251426
for collection_name in collections_for_target:
14261427
compile_command_sets[collection_name].extend(commands)
14271428
# Also put them into the main file.
@@ -1437,24 +1438,21 @@ def main():
14371438
root_dir.mkdir(parents=True)
14381439

14391440
# Chain output into compile_commands.json
1440-
for target_name in compile_command_sets:
1441+
for collection in compile_command_sets:
14411442
# If the target doesn't have a specified file name, put it into the "catch all"
14421443
# compilation database.
1443-
if target_name == '__all__':
1444+
if collection == '__all__':
14441445
file_path = root_dir / "compile_commands.json"
14451446
# Otherwise, write the database to the specific target file.
14461447
else:
1447-
target_dir = root_dir / target_name
1448+
target_dir = root_dir / collection
14481449
target_dir.mkdir(exist_ok=True)
14491450
file_path = target_dir / "compile_commands.json"
14501451

1451-
# This condition is only relevant to __all__. If each specified target has a specified
1452-
# file name, there won't be any compile commands in __all__, and we shouldn't write
1453-
# a file with an empty array.
1454-
if (len(compile_command_sets[target_name]) > 0):
1452+
if (len(compile_command_sets[collection]) > 0):
14551453
with open(file_path, 'w') as output_file:
14561454
json.dump(
1457-
compile_command_sets[target_name],
1455+
compile_command_sets[collection],
14581456
output_file,
14591457
indent=2, # Yay, human readability!
14601458
check_circular=False # For speed.

refresh_compile_commands.bzl

+121-43
Original file line numberDiff line numberDiff line change
@@ -37,12 +37,27 @@ refresh_compile_commands(
3737
# However, if you use clangd and are looking for speed, we strongly recommend you follow the instructions below instead, since clangd is going to regularly infer the wrong commands for headers and give you lots of annoyingly unnecessary red squigglies.
3838
3939
# Need to create separate files for specific targets? Give those targets a name and their compile commands file will be written into a subdirectory with that name.
40-
# target_file_names = {
41-
# "//:host_build": "host",
42-
# "//:target_build": "target",
40+
# target_collections = {
41+
# "host": "//:host_build",
42+
# "target": "//:target_build",
4343
# }
4444
45-
# Need to write compile commands to some directory other than the workspace root? Provide a path relative to the workspace root.
45+
# You can define target collections, sort of like "meta-targets" that contain combination of compile commands from the specified targets.
46+
# This is useful for multi-architecture projects, where you might be building the same files in multiple different ways depending on what architecture or device the code will run on.
47+
# It only makes sense to get code intelligence for a consistent build, so you can produce consistent compile commands with target collections.
48+
# Targets only need to be specified in either `targets` or `target_collections`; there's no need to add them to both.
49+
# target_collections = {
50+
# "host": [
51+
# "//:host_build",
52+
# "//:host_tests",
53+
# ],
54+
# "target": [
55+
# ["//:target_build", "--platform=//:target_device --important_flag"],
56+
# ],
57+
# }
58+
59+
60+
# Need to write compile commands to some directory other than the workspace root? Provide a path relative to the workspace root. This is useful if you use target collections.
4661
# out_dir = ".compile_commands"
4762
4863
# Need things to run faster? [Either for compile_commands.json generation or clangd indexing.]
@@ -76,62 +91,122 @@ def refresh_compile_commands(
7691
exclude_headers = None,
7792
exclude_external_sources = False,
7893
**kwargs): # For the other common attributes. Tags, compatible_with, etc. https://docs.bazel.build/versions/main/be/common-definitions.html#common-attributes.
79-
80-
if not target_collections:
81-
target_collections = {}
8294

83-
target_collections = {
84-
_make_label_absolute(target): collection_name for target, collection_name in target_collections.items()
85-
}
86-
87-
# We want a list of targets specified in `target_collections`, and we want a list of all of the target collections.
88-
# There's a many-to-many relationship between targets and target collections, there can be duplicates in both of those lists.
89-
# We want to de-duplicate those lists, but Starlark doesn't have a set class like Python does, so we do it manually.
90-
target_collection_names = []
91-
target_collection_targets = []
92-
93-
for target, collection_name in target_collections.items():
94-
if not target in target_collection_targets:
95-
target_collection_targets.append(target)
96-
97-
if not collection_name in target_collection_names:
98-
target_collection_names.append(collection_name)
95+
# Given `targets` that may be absent, or be a single string target:
96+
# targets = "//:some_target"
97+
#
98+
# ... or be a list of string targets:
99+
# targets = [
100+
# "//:some_target",
101+
# "//:another_target"
102+
# ]
103+
#
104+
# ... or be a dict of string targets and build flags:
105+
# targets = {
106+
# "//:some_target": "--flag ...",
107+
# "//:another_target": "--arg ..."
108+
# }
109+
#
110+
# And given `target_collections` that may be absent, or have a collection name associated with a list of string targets:
111+
# target_collections = {
112+
# "host": [
113+
# "//:host_target",
114+
# ...
115+
# ],
116+
# "device": [
117+
# "//:device_target",
118+
# ...
119+
# ]
120+
# }
121+
#
122+
# ... or have collection names associated with lists of string targets with build flags:
123+
# target_collections = {
124+
# "host": [
125+
# ("//:host_target", "--flag ..."),
126+
# ("//:test_target", "--platform=//:host_platform"),
127+
# ...
128+
# ],
129+
# "device": [
130+
# ("//:device_target", "--arg ..."),
131+
# ("//:test_target", "--platform=//:device_platform"),
132+
# ...
133+
# ]
134+
# }
135+
#
136+
# ... we want to produce a `string_dict_list` that we can pass into the Python script template to assemble the `target_flag_pairs`.
137+
# A simple dict with target name keys isn't adequate, because we can specify a target more than once with different build flags (see the last example above).
138+
# So we assemble a dict where the keys are unique to each target+flag combination, and the values are a list of strings in this format: [<target>, <flags>]
139+
#
140+
# That takes care of the `target_flag_pairs`, which determines which targets to generate compile commands for.
141+
#
142+
# Associating compile commands with target collections is easier; we pass `target_collections`, but with the target names and flags concatenated in the same way
143+
# as described above. This lets us associate each target+flag combination we generate compile commands for with its membership(s) in target collections.
144+
145+
target_collection_targets_list = []
146+
serialized_target_collections = {}
99147

148+
if target_collections:
149+
# Convert the targets specified in `target_collections` into the format we'll use with `targets`, so we can combine them into one list of targets to generate compile commands for.
150+
for targets_for_collection in target_collections.values():
151+
# You can specify a bare string target if the collection only has one target.
152+
if type(targets_for_collection) != "list":
153+
targets_for_collection = [targets_for_collection]
154+
155+
for target in targets_for_collection:
156+
# The target can either be a plain string target name, or a tuple of a target name and build flags, similar to the format of `targets`.
157+
if type(target) == "list":
158+
target_name, flags = target
159+
else:
160+
target_name = target
161+
flags = ""
100162

163+
target_data = (_make_label_absolute(target_name), flags)
164+
165+
# Targets may appear in multiple collections. We don't want duplicates in the final list, but Starlark doesn't have Python's set class. So we de-duplicate manually.
166+
if target_data not in target_collection_targets_list:
167+
target_collection_targets_list.append(target_data)
168+
169+
# Assemble the association between target collections and their targets.
170+
for collection, targets_for_collection in target_collections.items():
171+
serialized_targets = []
172+
173+
for target in targets_for_collection:
174+
if type(target) == "list":
175+
# If the target has flags, concat them with the target name. That's how we'll associate compile commands with collections on the Python side.
176+
serialized_targets.append("".join(target))
177+
else:
178+
serialized_targets.append(target)
179+
180+
serialized_target_collections[collection] = serialized_targets
181+
182+
target_collection_targets = {"{}{}".format(target, flags): [target, flags] for target, flags in target_collection_targets_list}
101183

102184
# Convert the various, acceptable target shorthands into the dictionary format
103185
# In Python, `type(x) == y` is an antipattern, but [Starlark doesn't support inheritance](https://bazel.build/rules/language), so `isinstance` doesn't exist, and this is the correct way to switch on type.
104-
if not targets and len(target_collection_targets) == 0: # Default to all targets in main workspace
105-
targets = {"@//...": ""}
106-
if not targets: # In this case, targets were defined only in `target_collections`
107-
targets = {target: "" for target in target_collection_targets}
186+
if not targets and not target_collections: # Default to all targets in main workspace
187+
targets = {"@//...": ["@//...", ""]}
188+
elif not targets: # In this case, targets were defined only in `target_collections`
189+
targets = target_collection_targets
108190
elif type(targets) == "select": # Allow select: https://bazel.build/reference/be/functions#select
109191
# Pass select() to _expand_template to make it work
110192
# see https://bazel.build/docs/configurable-attributes#faq-select-macro
111193
pass
112194
elif type(targets) == "list": # Allow specifying a list of targets w/o arguments
113-
# The reason we want the list of target collection targets is that if you specify a target in `target_collections`, you don't have to redundantly specify it in `targets`, *unless* you want to specify build flags too.
114-
# So we want to cull the list of target collection targets down to only those that *aren't* specified in `targets`.
115-
unique_target_collection_targets = [target for target in target_collection_targets if target not in targets]
116-
targets = {target: "" for target in (targets + unique_target_collection_targets)}
195+
absolute_targets = [_make_label_absolute(target) for target in targets]
196+
targets = {target: [target, ""] for target in absolute_targets} | target_collection_targets
117197
elif type(targets) != "dict": # Assume they've supplied a single string/label and wrap it
118-
unique_target_collection_targets = [target for target in target_collection_targets if target not in targets]
119-
targets = {target: "" for target in ([targets] + unique_target_collection_targets)}
198+
targets = {targets: [targets, ""]} | target_collection_targets
120199
else: # Assume that they've provided a dict of targets with flags
121-
unique_target_collection_targets = [target for target in target_collection_targets if target not in targets]
122-
targets = targets | {target: "" for target in target_collection_targets}
123-
124-
targets = {
125-
_make_label_absolute(target): flags for target, flags in targets.items()
126-
}
200+
absolute_targets = {_make_label_absolute(target): flags for target, flags in targets.items()}
201+
targets = {"{}{}".format(target, flags): [target, flags] for target, flags in absolute_targets.items()} | target_collection_targets
127202

128203
# Create a wrapper script that prints a helpful error message if the python version is too old, generated from check_python_version.template.py
129204
version_checker_script_name = name + ".check_python_version.py"
130205
_check_python_version(name = version_checker_script_name, to_run = name)
131206

132207
# Generate the core, runnable python script from refresh.template.py
133208
script_name = name + ".py"
134-
_expand_template(name = script_name, labels_to_flags = targets, labels_to_collections = target_collections, out_dir = out_dir, exclude_headers = exclude_headers, exclude_external_sources = exclude_external_sources, **kwargs)
209+
_expand_template(name = script_name, labels_to_flags = targets, labels_to_collections = serialized_target_collections, out_dir = out_dir, exclude_headers = exclude_headers, exclude_external_sources = exclude_external_sources, **kwargs)
135210

136211
# Combine them so the wrapper calls the main script
137212
native.py_binary(
@@ -147,6 +222,9 @@ def _make_label_absolute(label):
147222
# Make any package-relative labels absolute
148223
return label if label.startswith("/") or label.startswith("@") else "{}//{}:{}".format(native.repository_name(), native.package_name(), label.removeprefix(":"))
149224

225+
def _expand_target_collection_targets(targets):
226+
return "[\n" + "\n".join([" '{}',".format(target) for target in targets]) + "\n ]"
227+
150228
def _expand_template_impl(ctx):
151229
"""Inject targets of interest--and other settings--into refresh.template.py, and set it up to be run."""
152230
script = ctx.actions.declare_file(ctx.attr.name)
@@ -157,7 +235,7 @@ def _expand_template_impl(ctx):
157235
substitutions = {
158236
# Note, don't delete whitespace. Correctly doing multiline indenting.
159237
" {target_flag_pairs}": "\n".join([" {},".format(pair) for pair in ctx.attr.labels_to_flags.items()]),
160-
" {target_collections}": "\n".join([" '{}': '{}',".format(target, collection_name) for (target, collection_name) in ctx.attr.labels_to_collections.items()]),
238+
" {target_collections}": "\n".join([" '{}': {},".format(collection_name, _expand_target_collection_targets(targets)) for collection_name, targets in ctx.attr.labels_to_collections.items()]),
161239
" {windows_default_include_paths}": "\n".join([" %r," % path for path in find_cpp_toolchain(ctx).built_in_include_directories]), # find_cpp_toolchain is from https://docs.bazel.build/versions/main/integrating-with-rules-cc.html
162240
"{out_dir}": repr(ctx.attr.out_dir),
163241
"{exclude_headers}": repr(ctx.attr.exclude_headers),
@@ -169,8 +247,8 @@ def _expand_template_impl(ctx):
169247

170248
_expand_template = rule(
171249
attrs = {
172-
"labels_to_flags": attr.string_dict(mandatory = True), # string keys instead of label_keyed because Bazel doesn't support parsing wildcard target patterns (..., *, :all) in BUILD attributes.
173-
"labels_to_collections": attr.string_dict(),
250+
"labels_to_flags": attr.string_list_dict(mandatory = True), # string keys instead of label_keyed because Bazel doesn't support parsing wildcard target patterns (..., *, :all) in BUILD attributes.
251+
"labels_to_collections": attr.string_list_dict(),
174252
"out_dir": attr.string(default = "."),
175253
"exclude_external_sources": attr.bool(default = False),
176254
"exclude_headers": attr.string(values = ["all", "external", ""]), # "" needed only for compatibility with Bazel < 3.6.0

0 commit comments

Comments
 (0)