Skip to content

Commit

Permalink
avoid discarding analysis cache when formatting
Browse files Browse the repository at this point in the history
Change-Id: Idbc465e5e9aaad7d061b3e6bfdd42bad9020d8a5
  • Loading branch information
oliverlee committed Jun 26, 2024
1 parent 649b2c1 commit 231df59
Show file tree
Hide file tree
Showing 4 changed files with 53 additions and 75 deletions.
7 changes: 1 addition & 6 deletions BUILD.bazel
Original file line number Diff line number Diff line change
@@ -1,12 +1,7 @@
load("//:defs.bzl", "bool_flag", "clang_format_update")
load("//:defs.bzl", "clang_format_update")

package(default_visibility = ["//visibility:public"])

bool_flag(
name = "dry_run",
build_setting_default = True,
)

# Your binary should be a `*_binary`, *NOT* filegroup
filegroup(
name = "default_binary",
Expand Down
80 changes: 35 additions & 45 deletions defs.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -2,26 +2,6 @@
clang-format aspect
"""

# Avoid the need to bring in bazel-skylib as a dependency
# https://github.com/bazelbuild/bazel-skylib/blob/main/docs/common_settings_doc.md
BuildSettingInfo = provider(
doc = "Contains the value of a build setting.",
fields = {
"value": "The value of the build setting in the current configuration. " +
"This value may come from the command line or an upstream transition, " +
"or else it will be the build setting's default.",
},
)

def _impl(ctx):
return BuildSettingInfo(value = ctx.build_setting_value)

bool_flag = rule(
implementation = _impl,
build_setting = config.bool(flag = True),
doc = "A bool-typed build setting that can be set on the command line",
)

def _source_files_in(ctx, attr):
if not hasattr(ctx.rule.attr, attr):
return []
Expand All @@ -32,9 +12,8 @@ def _source_files_in(ctx, attr):

return [f for f in files if f.is_source]

def _check_format(ctx, package, f):
def _check_format(ctx, package, f, update):
binary = ctx.attr._binary.files_to_run.executable
dry_run = ctx.attr._dry_run[BuildSettingInfo].value

out = ctx.actions.declare_file(
"{name}.clang_format".format(
Expand All @@ -43,40 +22,51 @@ def _check_format(ctx, package, f):
),
)

ctx.actions.run(
progress_message = "Formatting {}".format(f.short_path)

ctx.actions.run_shell(
inputs = [ctx.file._config] + ([binary] if binary else []) + [f],
outputs = [out],
executable = ctx.executable._wrapper,
arguments = [
binary.path if binary else "clang-format",
ctx.file._config.path,
f.path,
out.path,
] + (["--dry-run"] if dry_run else [""]),
command = """
set -euo pipefail
test -e .clang-format || ln -s -f {config} .clang-format
{binary} --color=true --Werror {options} {infile} > {outfile}
""".format(
config = ctx.file._config.path,
binary = binary.path if binary else "clang-format",
infile = f.path,
outfile = out.path,
options = "" if update else "--dry-run",
),
mnemonic = "ClangFormat",
progress_message = progress_message if update else None,
)

return out

def _clang_format_aspect_impl(target, ctx):
ignored = {f.owner: "" for f in ctx.attr._ignore.files.to_list()}
def _clang_format_aspect_impl(update):
def impl(target, ctx):
ignored = {f.owner: "" for f in ctx.attr._ignore.files.to_list()}

if target.label in ignored.keys():
return [OutputGroupInfo(report = depset([]))]

if target.label in ignored.keys():
return [OutputGroupInfo(report = depset([]))]
outputs = [
_check_format(ctx, target.label.package, f, update)
for f in (
_source_files_in(ctx, "srcs") +
_source_files_in(ctx, "hdrs")
)
]

outputs = [
_check_format(ctx, target.label.package, f)
for f in (
_source_files_in(ctx, "srcs") +
_source_files_in(ctx, "hdrs")
)
]
return [OutputGroupInfo(report = depset(outputs))]

return [OutputGroupInfo(report = depset(outputs))]
return impl

def make_clang_format_aspect(binary = None, config = None, ignore = None):
def make_clang_format_aspect(binary = None, config = None, ignore = None, update = False):
return aspect(
implementation = _clang_format_aspect_impl,
implementation = _clang_format_aspect_impl(update),
fragments = ["cpp"],
attrs = {
"_wrapper": attr.label(
Expand All @@ -95,13 +85,13 @@ def make_clang_format_aspect(binary = None, config = None, ignore = None):
"_ignore": attr.label(
default = Label(ignore or "//:ignore"),
),
"_dry_run": attr.label(default = Label("//:dry_run")),
},
required_providers = [CcInfo],
toolchains = ["@bazel_tools//tools/cpp:toolchain_type"],
)

clang_format_aspect = make_clang_format_aspect()
clang_format_update_aspect = make_clang_format_aspect(update=True)

def _clang_format_update_impl(ctx):
update_format = ctx.actions.declare_file(
Expand Down
29 changes: 17 additions & 12 deletions update.template.sh
Original file line number Diff line number Diff line change
Expand Up @@ -17,27 +17,33 @@ function bazel_bin
bazel=$(bazel_bin)

bazel_query=("$bazel" query \
--noshow_progress \
"--ui_event_filters=-info" \
"--color=yes")

bazel_format=("$bazel" build \
--noshow_progress \
"--ui_event_filters=-info,-stdout" \
bazel_update=("$bazel" build \
"--color=yes" \
"--aspects=@@WORKSPACE@//:defs.bzl%clang_format_aspect" \
"--aspects=@@WORKSPACE@//:defs.bzl%clang_format_update_aspect" \
"--@@WORKSPACE@//:binary=@BINARY@" \
"--@@WORKSPACE@//:config=@CONFIG@" \
"--@@WORKSPACE@//:ignore=@IGNORE@" \
"--output_groups=report" \
--keep_going \
--verbose_failures)

bazel_check=("$bazel" build \
"--color=yes" \
"--aspects=@@WORKSPACE@//:defs.bzl%clang_format_aspect" \
"--@@WORKSPACE@//:binary=@BINARY@" \
"--@@WORKSPACE@//:config=@CONFIG@" \
"--@@WORKSPACE@//:ignore=@IGNORE@" \
"--output_groups=report" \
--keep_going \
--verbose_failures)

function stale
{
format_args=("$@")

result=$("${bazel_format[@]}" \
result=$("${bazel_check[@]}" \
--check_up_to_date \
"${format_args[@]}" 2>&1 || true)

Expand All @@ -63,7 +69,6 @@ function update
mv "$generated" "$1"
}


cd "$BUILD_WORKSPACE_DIRECTORY"

"$bazel" build @BINARY@
Expand All @@ -89,21 +94,21 @@ fi
# use bazel to generate the formatted files in a separate
# directory in case the user is overriding .clang-format
if [[ ${#files[@]} -ne 0 ]]; then
"${bazel_format[@]}" --compile_one_dependency --@@WORKSPACE@//:dry_run=False --remote_download_outputs=toplevel "${files[@]}"
"${bazel_update[@]}" --compile_one_dependency --remote_download_outputs=toplevel "${files[@]}"
fi

# format all header only libs
if [[ ${#header_files[@]} -ne 0 ]]; then
"${bazel_format[@]}" --@@WORKSPACE@//:dry_run=False --remote_download_outputs=toplevel "${header_libraries[@]}"
"${bazel_update[@]}" --remote_download_outputs=toplevel "${header_libraries[@]}"
fi

export -f update
printf '%s\n' "${files[@]}" "${header_files[@]}" | xargs -P 0 -n 1 -I {} /bin/bash -c 'update {}'

# run format check to cache success
if [[ ${#files[@]} -ne 0 ]]; then
"${bazel_format[@]}" --compile_one_dependency "${files[@]}"
"${bazel_check[@]}" --compile_one_dependency "${files[@]}"
fi
if [[ ${#header_files[@]} -ne 0 ]]; then
"${bazel_format[@]}" "${header_libraries[@]}"
"${bazel_check[@]}" "${header_libraries[@]}"
fi
12 changes: 0 additions & 12 deletions wrapper.sh

This file was deleted.

0 comments on commit 231df59

Please sign in to comment.