Skip to content

Commit

Permalink
Change enable_swift to exclude_swift and default to False
Browse files Browse the repository at this point in the history
  • Loading branch information
xinzhengzhang committed Dec 5, 2022
1 parent 968686b commit 047daf9
Show file tree
Hide file tree
Showing 2 changed files with 5 additions and 10 deletions.
7 changes: 1 addition & 6 deletions refresh.template.py
Original file line number Diff line number Diff line change
Expand Up @@ -917,19 +917,14 @@ def _get_commands(target: str, flags: str):
Try adding them as flags in your refresh_compile_commands rather than targets.
In a moment, Bazel will likely fail to parse.""")

support_mnemonics = ["Objc", "Cpp"]
if {enable_swift}:
support_mnemonics += ["Swift"]
mnemonics_string = '|'.join(support_mnemonics)

# First, query Bazel's C-family compile actions for that configured target
aquery_args = [
'bazel',
'aquery',
# Aquery docs if you need em: https://docs.bazel.build/versions/master/aquery.html
# Aquery output proto reference: https://github.com/bazelbuild/bazel/blob/master/src/main/protobuf/analysis_v2.proto
# One bummer, not described in the docs, is that aquery filters over *all* actions for a given target, rather than just those that would be run by a build to produce a given output. This mostly isn't a problem, but can sometimes surface extra, unnecessary, misconfigured actions. Chris has emailed the authors to discuss and filed an issue so anyone reading this could track it: https://github.com/bazelbuild/bazel/issues/14156.
f"mnemonic('({mnemonics_string})Compile',deps({target}))",
f"mnemonic('(Objc|Cpp{swift_mnemonic_string})Compile',deps({target}))",
# We switched to jsonproto instead of proto because of https://github.com/bazelbuild/bazel/issues/13404. We could change back when fixed--reverting most of the commit that added this line and tweaking the build file to depend on the target in that issue. That said, it's kinda nice to be free of the dependency, unless (OPTIMNOTE) jsonproto becomes a performance bottleneck compated to binary protos.
'--output=jsonproto',
# We'll disable artifact output for efficiency, since it's large and we don't use them. Small win timewise, but dramatically less json output from aquery.
Expand Down
8 changes: 4 additions & 4 deletions refresh_compile_commands.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,7 @@ def refresh_compile_commands(
targets = None,
exclude_headers = None,
exclude_external_sources = False,
enable_swift = False,
exclude_swift = False,
**kwargs): # For the other common attributes. Tags, compatible_with, etc. https://docs.bazel.build/versions/main/be/common-definitions.html#common-attributes.
# Convert the various, acceptable target shorthands into the dictionary format
# 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.
Expand All @@ -80,7 +80,7 @@ def refresh_compile_commands(

# Generate runnable python script from template
script_name = name + ".py"
_expand_template(name = script_name, labels_to_flags = targets, exclude_headers = exclude_headers, exclude_external_sources = exclude_external_sources, enable_swift = enable_swift, **kwargs)
_expand_template(name = script_name, labels_to_flags = targets, exclude_headers = exclude_headers, exclude_external_sources = exclude_external_sources, exclude_swift = exclude_swift, **kwargs)
native.py_binary(name = name, srcs = [script_name], **kwargs)

def _expand_template_impl(ctx):
Expand All @@ -96,7 +96,7 @@ def _expand_template_impl(ctx):
" {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
"{exclude_headers}": '"' + str(ctx.attr.exclude_headers) + '"',
"{exclude_external_sources}": str(ctx.attr.exclude_external_sources),
"{enable_swift}": str(ctx.attr.enable_swift),
"{swift_mnemonic_string}": ("" if ctx.attr.exclude_swift else "|Swift") # Note there is a '|' before Swift
},
)
return DefaultInfo(files = depset([script]))
Expand All @@ -106,7 +106,7 @@ _expand_template = rule(
"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.
"exclude_external_sources": attr.bool(default = False),
"exclude_headers": attr.string(values = ["all", "external", ""]), # "" needed only for compatibility with Bazel < 3.6.0
"enable_swift": attr.bool(default = False),
"exclude_swift": attr.bool(default = False),
"_script_template": attr.label(allow_single_file = True, default = "refresh.template.py"),
"_cc_toolchain": attr.label(default = "@bazel_tools//tools/cpp:current_cc_toolchain"), # For Windows INCLUDE. If this were eliminated, for example by the resolution of https://github.com/clangd/clangd/issues/123, we'd be able to just use a macro and skylib's expand_template rule: https://github.com/bazelbuild/bazel-skylib/pull/330
},
Expand Down

0 comments on commit 047daf9

Please sign in to comment.