From 232498faa9f9979ad62fa9ef2ddad92605cc1dfc Mon Sep 17 00:00:00 2001
From: Kevin Gleason <gleasonk@google.com>
Date: Fri, 23 Sep 2022 11:27:40 -0400
Subject: [PATCH] Refactor the lint action into a build script, temporarily
 update .clang-format (#183)

Two changes in this PR:

1) Refactor the lint action to be a locally runnable bash script. This should give _more_ predictable results in terms of addressing CI failures.

2) Update `.clang-format` to use Google style in the short term. More on this rationale below.

Background on all this is that my editor formatted a large chunk of a file which caused the lint action to fail remotely, and took me quite some time to roll back the individual formatting changes, since google styling appears to prefer the most common spacing of `Type* var` vs `Type *var` in a file as the proper formatting.

This change will allow local qualification / fixing, as well as provide a stop-gap on editor formatting issues while #15 is discussed. While I agree that we should resolve #15, I think more discussion is required, and there is unnecessary friction in the current setup/CI of clang-format in the current repo that should be addressed. Happy to drive the formatting conversation in the near future, but think this should be in place in the short term.

GH Actions run testing that it errors properly in CI: https://github.com/GleasonK/stablehlo/actions/runs/3106772722/jobs/5034066123

Closes #75
---
 .clang-format                                 |  2 +-
 .github/workflows/lint.yml                    | 16 +++---
 .../github_actions/lint_clang_format.sh       | 50 +++++++++++++++++++
 3 files changed, 57 insertions(+), 11 deletions(-)
 create mode 100755 build_tools/github_actions/lint_clang_format.sh

diff --git a/.clang-format b/.clang-format
index a74fda4b673..76f650220c7 100644
--- a/.clang-format
+++ b/.clang-format
@@ -1,2 +1,2 @@
-BasedOnStyle: LLVM
+BasedOnStyle: Google
 AlwaysBreakTemplateDeclarations: Yes
diff --git a/.github/workflows/lint.yml b/.github/workflows/lint.yml
index edf18025084..8ca71ba4d08 100644
--- a/.github/workflows/lint.yml
+++ b/.github/workflows/lint.yml
@@ -2,7 +2,9 @@
 
 name: Lint
 
-on: [pull_request]
+on: 
+  pull_request:
+    paths-ignore: ['**.md', 'docs/**']
 
 jobs:
   clang-format:
@@ -10,16 +12,10 @@ jobs:
     if: "github.event_name == 'pull_request'"
     runs-on: ubuntu-latest
     steps:
-      - name: Installing dependencies
-        run: |
-          wget https://raw.githubusercontent.com/llvm-mirror/clang/master/tools/clang-format/git-clang-format -O /tmp/git-clang-format
-          chmod +x /tmp/git-clang-format
       - name: Checking out repository
         uses: actions/checkout@v2
-      - name: Fetching Base Branch
-        # We have to explicitly fetch the base branch as well
-        run: git fetch --no-tags --prune --depth=1 origin "${GITHUB_BASE_REF?}:${GITHUB_BASE_REF?}"
+
       - name: Running clang-format on changed source files
         run: |
-          /tmp/git-clang-format "${GITHUB_BASE_REF?}" --style=google
-          git diff --exit-code
+          git fetch --no-tags --prune --depth=1 origin "${GITHUB_BASE_REF?}:${GITHUB_BASE_REF?}"
+          ./build_tools/github_actions/lint_clang_format.sh -b "${GITHUB_BASE_REF}"
diff --git a/build_tools/github_actions/lint_clang_format.sh b/build_tools/github_actions/lint_clang_format.sh
new file mode 100755
index 00000000000..d0d50744f45
--- /dev/null
+++ b/build_tools/github_actions/lint_clang_format.sh
@@ -0,0 +1,50 @@
+# Copyright 2022 The StableHLO Authors.
+# Licensed under the Apache License, Version 2.0 (the "License");
+# you may not use this file except in compliance with the License.
+# You may obtain a copy of the License at
+#
+#     http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+
+print_usage() {
+  echo "Usage: $0 [-fd]"
+  echo "    -f           Auto-fix clang-format issues."
+  echo "    -b <branch>  Base branch name, default to origin/main."
+}
+
+FORMAT_MODE='validate'
+BASE_BRANCH="$(git merge-base HEAD origin/main)"
+while getopts 'fb:' flag; do
+  case "${flag}" in
+    f) FORMAT_MODE="fix" ;;
+    b) BASE_BRANCH="$OPTARG" ;;
+    *) print_usage
+       exit 1 ;;
+  esac
+done
+shift $(( OPTIND - 1 ))
+
+if [[ $# -ne 0 ]] ; then
+  print_usage
+  exit 1
+fi
+
+echo "Gathering changed files..."
+CHANGED_FILES=$(git diff --name-only HEAD $BASE_BRANCH | grep '.*\.h\|.*\.cpp' | xargs)
+if [[ -z "$CHANGED_FILES" ]]; then
+  echo "No files to format."
+  exit 0
+fi
+
+echo "Running clang-format [mode=$FORMAT_MODE]..."
+echo "  Files: $CHANGED_FILES"
+if [[ $FORMAT_MODE == 'fix' ]]; then
+  clang-format --style=google -i $CHANGED_FILES
+else
+  clang-format --style=google --dry-run --Werror $CHANGED_FILES
+fi