From 33cf8ae5844aca08633444fbdc36f57f1606fdd6 Mon Sep 17 00:00:00 2001
From: Oliver Lee <oliverzlee@gmail.com>
Date: Fri, 28 Jun 2024 13:10:03 -0700
Subject: [PATCH] fix in-place formatting with older clang-format versions

Change-Id: I0a98b8625c05b962fa1bcc7d8a00f2291ee8efbc
---
 .github/workflows/check.yml         | 32 +++++++++++++++++++++++++----
 README.md                           |  5 ++---
 defs.bzl                            | 24 ++++++++++++++--------
 example/default/.bazelrc            | 10 +++++++--
 example/format-binary/.bazelrc      | 15 +++++++++++---
 example/format-binary/BUILD.bazel   |  5 -----
 example/format-config/.bazelrc      | 15 +++++++++++---
 example/format-config/.clang-format |  2 +-
 example/format-config/BUILD.bazel   |  2 +-
 example/format-ignore/.bazelrc      | 17 +++++++++++----
 example/format-ignore/BUILD.bazel   |  7 +------
 11 files changed, 93 insertions(+), 41 deletions(-)

diff --git a/.github/workflows/check.yml b/.github/workflows/check.yml
index a7b2b7f..c552a27 100644
--- a/.github/workflows/check.yml
+++ b/.github/workflows/check.yml
@@ -12,6 +12,7 @@ jobs:
   check-format:
     runs-on: ubuntu-latest
     strategy:
+      fail-fast: false
       matrix:
         include:
           - example: default
@@ -19,17 +20,40 @@ jobs:
           - example: format-binary
             errors: 2
           - example: format-config
-            errors: 0
+            errors: 1
           - example: format-ignore
             errors: 0
     steps:
       - uses: actions/checkout@v4
-      - run: |
+      - shell: bash
+        run: |
           cd example/${{ matrix.example }}
 
           bazel build \
             --config=clang-format \
             --color=yes \
-            //... 2>&1 | tee log
+            //... 2>&1 | tee log || true
+
+          rc="${PIPESTATUS[0]}"
+
+          [[ ${{ matrix.errors }} -eq 0 ]] && [[ "$rc" -eq 0 ]] || [[ "$rc" -ne 0 ]]
+          [[ ${{ matrix.errors }} -eq $(cat log | grep -c "Wclang-format-violations") ]]
+
+  fix-format:
+    runs-on: ubuntu-latest
+    strategy:
+      fail-fast: false
+      matrix:
+        include:
+          - example: default
+          - example: format-binary
+          - example: format-config
+          - example: format-ignore
+    steps:
+      - uses: actions/checkout@v4
+      - run: |
+          cd example/${{ matrix.example }}
+
+          bazel build --config=clang-format-fix
 
-          test ${{ matrix.errors }} -eq $(cat log | grep -c "Wclang-format-violations")
+          bazel build --config=clang-format
diff --git a/README.md b/README.md
index 3ec367c..8d17407 100644
--- a/README.md
+++ b/README.md
@@ -52,7 +52,6 @@ bazel build //... --config=clang-format-fix
 This will use `clang-format` in your `PATH` and `.clang-format` defined in this
 repo.
 
-
 ### using a hermetic toolchain
 
 <details><summary></summary>
@@ -98,7 +97,7 @@ filegroup(
 # //:.bazelrc
 
 build:clang-format-base --output_groups=report
-build:clang-format-base --@bazel_clang_format//:config=//:clang-format-config # <-----
+build:clang-format-base --@bazel_clang_format//:config=//:clang-format-config
 ...
 ```
 
@@ -126,7 +125,7 @@ filegroup(
 # //:.bazelrc
 
 build:clang-format-base --output_groups=report
-build:clang-format-base --@bazel_clang_format//:ignore=//:clang-format-ignore# <-----
+build:clang-format-base --@bazel_clang_format//:ignore=//:clang-format-ignore
 ...
 ```
 
diff --git a/defs.bzl b/defs.bzl
index 059cc8b..b9ff02d 100644
--- a/defs.bzl
+++ b/defs.bzl
@@ -26,15 +26,21 @@ def _do_format(ctx, f, format_options, execution_reqs):
 set -euo pipefail
 
 test -e .clang-format || ln -s -f {config} .clang-format
-{binary} {format_options} {infile}
+
+# https://github.com/llvm/llvm-project/issues/46336
+# although newer versions of clang-format (e.g. 18.1.4) *do* appear to work
+# with symlinks
+#
+{binary} {format_options} $(readlink --canonicalize {infile})
+
 touch {outfile}
 """.format(
-    config = ctx.file._config.path,
-    binary = binary.path if binary else "clang-format",
-    format_options = " ".join(format_options),
-    infile = f.path,
-    outfile = out.path,
-),
+            config = ctx.file._config.path,
+            binary = binary.path if binary else "clang-format",
+            format_options = " ".join(format_options),
+            infile = f.path,
+            outfile = out.path,
+        ),
         mnemonic = "ClangFormat",
         progress_message = "Formatting {}".format(f.short_path),
         execution_requirements = execution_reqs,
@@ -57,8 +63,8 @@ def _clang_format_aspect_impl(format_options, execution_requirements):
                 execution_requirements,
             )
             for f in (
-                    _source_files_in(ctx, "srcs") +
-                    _source_files_in(ctx, "hdrs")
+                _source_files_in(ctx, "srcs") +
+                _source_files_in(ctx, "hdrs")
             )
         ]
 
diff --git a/example/default/.bazelrc b/example/default/.bazelrc
index dd82396..9efa438 100644
--- a/example/default/.bazelrc
+++ b/example/default/.bazelrc
@@ -1,2 +1,8 @@
- build:clang-format --aspects @bazel_clang_format//:defs.bzl%check_aspect
- build:clang-format --output_groups=report
+common --enable_bzlmod=false
+
+build:clang-format --aspects @bazel_clang_format//:defs.bzl%check_aspect
+build:clang-format --output_groups=report
+
+build:clang-format-fix --aspects @bazel_clang_format//:defs.bzl%fix_aspect
+build:clang-format-fix --output_groups=report
+build:clang-format-fix --use_action_cache=false
diff --git a/example/format-binary/.bazelrc b/example/format-binary/.bazelrc
index 62311bf..9112e07 100644
--- a/example/format-binary/.bazelrc
+++ b/example/format-binary/.bazelrc
@@ -1,3 +1,12 @@
- build:clang-format --aspects @bazel_clang_format//:defs.bzl%check_aspect
- build:clang-format --@bazel_clang_format//:binary=@llvm14//:clang-format
- build:clang-format --output_groups=report
+common --enable_bzlmod=false
+
+build:clang-format-base --@bazel_clang_format//:binary=@llvm14//:clang-format
+build:clang-format-base --output_groups=report
+build:clang-format-base --keep_going
+
+build:clang-format --config=clang-format-base
+build:clang-format --aspects @bazel_clang_format//:defs.bzl%check_aspect
+
+build:clang-format-fix --config=clang-format-base
+build:clang-format-fix --aspects @bazel_clang_format//:defs.bzl%fix_aspect
+build:clang-format-fix --use_action_cache=false
diff --git a/example/format-binary/BUILD.bazel b/example/format-binary/BUILD.bazel
index 5b65355..2938ae1 100644
--- a/example/format-binary/BUILD.bazel
+++ b/example/format-binary/BUILD.bazel
@@ -2,8 +2,3 @@ cc_binary(
     name = "main",
     srcs = ["main.cpp"],
 )
-
-filegroup(
-    name = "clang_format_config",
-    srcs = [".clang-format"],
-)
diff --git a/example/format-config/.bazelrc b/example/format-config/.bazelrc
index 28818f1..4f936e8 100644
--- a/example/format-config/.bazelrc
+++ b/example/format-config/.bazelrc
@@ -1,3 +1,12 @@
- build:clang-format --aspects @bazel_clang_format//:defs.bzl%check_aspect
- build:clang-format --@bazel_clang_format//:config=//:clang_format_config
- build:clang-format --output_groups=report
+common --enable_bzlmod=false
+
+build:clang-format-base --@bazel_clang_format//:config=//:clang-format-config
+build:clang-format-base --output_groups=report
+build:clang-format-base --keep_going
+
+build:clang-format --config=clang-format-base
+build:clang-format --aspects @bazel_clang_format//:defs.bzl%check_aspect
+
+build:clang-format-fix --config=clang-format-base
+build:clang-format-fix --aspects @bazel_clang_format//:defs.bzl%fix_aspect
+build:clang-format-fix --use_action_cache=false
diff --git a/example/format-config/.clang-format b/example/format-config/.clang-format
index 48661d1..efd7c71 100644
--- a/example/format-config/.clang-format
+++ b/example/format-config/.clang-format
@@ -28,7 +28,7 @@ DerivePointerAlignment: 'false'
 FixNamespaceComments: 'true'
 IncludeBlocks: Regroup
 IndentCaseLabels: 'true'
-IndentWidth: '4'
+IndentWidth: '2'
 Language: Cpp
 MaxEmptyLinesToKeep: '1'
 NamespaceIndentation: None
diff --git a/example/format-config/BUILD.bazel b/example/format-config/BUILD.bazel
index 5b65355..d30cd5d 100644
--- a/example/format-config/BUILD.bazel
+++ b/example/format-config/BUILD.bazel
@@ -4,6 +4,6 @@ cc_binary(
 )
 
 filegroup(
-    name = "clang_format_config",
+    name = "clang-format-config",
     srcs = [".clang-format"],
 )
diff --git a/example/format-ignore/.bazelrc b/example/format-ignore/.bazelrc
index b40a639..79e5991 100644
--- a/example/format-ignore/.bazelrc
+++ b/example/format-ignore/.bazelrc
@@ -1,4 +1,13 @@
- build:clang-format --aspects @bazel_clang_format//:defs.bzl%check_aspect
- build:clang-format --@bazel_clang_format//:binary=@llvm14//:clang-format
- build:clang-format --@bazel_clang_format//:ignore=//:clang_format_ignore
- build:clang-format --output_groups=report
+common --enable_bzlmod=false
+
+build:clang-format-base --@bazel_clang_format//:binary=@llvm14//:clang-format
+build:clang-format-base --@bazel_clang_format//:ignore=//:clang-format-ignore
+build:clang-format-base --output_groups=report
+build:clang-format-base --keep_going
+
+build:clang-format --config=clang-format-base
+build:clang-format --aspects @bazel_clang_format//:defs.bzl%check_aspect
+
+build:clang-format-fix --config=clang-format-base
+build:clang-format-fix --aspects @bazel_clang_format//:defs.bzl%fix_aspect
+build:clang-format-fix --use_action_cache=false
diff --git a/example/format-ignore/BUILD.bazel b/example/format-ignore/BUILD.bazel
index 2d2ecf6..defefdf 100644
--- a/example/format-ignore/BUILD.bazel
+++ b/example/format-ignore/BUILD.bazel
@@ -4,12 +4,7 @@ cc_binary(
 )
 
 filegroup(
-    name = "clang_format_config",
-    srcs = [".clang-format"],
-)
-
-filegroup(
-    name = "clang_format_ignore",
+    name = "clang-format-ignore",
     srcs = [
         ":main",
     ],