Use our own script for buildifier checks (#5745)
This actually prints out the diff and only checks modified files.
Tested:
`./scripts/buildifer.sh "$(git rev-list --max-parents=0 HEAD)"`
AKA run against all files. Fixed some lint in iree_flatcc.bzl
Workflow passes for this PR.
Workflow fails with helpful message for test commit with bad formatting:
https://github.com/google/iree/runs/2505445488
Workflow fails with helpful message for test commit with lint:
https://github.com/google/iree/runs/2505463446
diff --git a/.github/workflows/lint.yml b/.github/workflows/lint.yml
index 5914b2b..c06ae53 100644
--- a/.github/workflows/lint.yml
+++ b/.github/workflows/lint.yml
@@ -37,19 +37,26 @@
steps:
- name: Checking out repository
uses: actions/checkout@v2
- - name: Running Buildifier
- # TODO(gcmn): Look into only running on changed files.
- uses: thompsonja/bazel-buildifier@v0.2.1
- with:
- excludes: "*/third_party/*"
- # For compatibility with Google's internal source repository, we still
- # use this.
- warnings: "-native-cc,-native-py"
+ - 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: Downloading Buildifier
+ run: |
+ mkdir -p "${HOME?}/bin"
+ wget https://github.com/bazelbuild/buildtools/releases/download/4.0.1/buildifier -O "${HOME?}/bin/buildifier"
+ chmod +x "${HOME?}/bin/buildifier"
+ echo "${HOME?}/bin" >> $GITHUB_PATH
+ - name: Running buildifier on changed files
+ run: |
+ # Fail if the script fails (unfixable lint warnings) or it creates a
+ # diff (fixable lint warnings or formatting).
+ EXIT_CODE=0
+ ./scripts/run_buildifier.sh || EXIT_CODE=1
+ git diff --exit-code
+ exit "${EXIT_CODE?}"
yapf:
runs-on: ubuntu-18.04
- env:
- BASE_REF: ${{ github.base_ref }}
steps:
- name: Checking out repository
uses: actions/checkout@v2
@@ -57,24 +64,22 @@
uses: actions/setup-python@v2
- name: Fetching Base Branch
# We have to explicitly fetch the base branch as well
- run: git fetch --no-tags --prune --depth=1 origin "${BASE_REF?}:${BASE_REF?}"
+ run: git fetch --no-tags --prune --depth=1 origin "${GITHUB_BASE_REF?}:${GITHUB_BASE_REF?}"
- name: Install yapf
run: |
python3 -m pip install yapf==0.30.0
- name: Run format_diff.py with yapf
run: |
- git diff -U0 "${BASE_REF?}" | python3 third_party/format_diff/format_diff.py yapf -i
+ git diff -U0 "${GITHUB_BASE_REF?}" | python3 third_party/format_diff/format_diff.py yapf -i
git diff --exit-code
- name: Instructions for fixing the above linting errors
if: ${{ failure() }}
run: |
printf "You can fix the lint errors above by running\n"
- printf " git diff -U0 "${BASE_REF?}" | python3 third_party/format_diff/format_diff.py yapf -i\n"
+ printf " git diff -U0 "${GITHUB_BASE_REF?}" | python3 third_party/format_diff/format_diff.py yapf -i\n"
pytype:
runs-on: ubuntu-18.04
- env:
- BASE_REF: ${{ github.base_ref }}
steps:
- name: Checking out repository
uses: actions/checkout@v2
@@ -85,16 +90,14 @@
python-version: '3.8'
- name: Fetching Base Branch
# We have to explicitly fetch the base branch as well
- run: git fetch --no-tags --prune --depth=1 origin "${BASE_REF?}:${BASE_REF?}"
+ run: git fetch --no-tags --prune --depth=1 origin "${GITHUB_BASE_REF?}:${GITHUB_BASE_REF?}"
- name: Install pytype
run: python3 -m pip install pytype
- name: Run pytype on changed files
- run: ./build_tools/pytype/check_diff.sh "${BASE_REF?}"
+ run: ./build_tools/pytype/check_diff.sh "${GITHUB_BASE_REF?}"
clang-format:
runs-on: ubuntu-18.04
- env:
- BASE_REF: ${{ github.base_ref }}
steps:
- name: Installing dependencies
run: |
@@ -106,10 +109,10 @@
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 "${BASE_REF?}:${BASE_REF?}"
+ 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 "${BASE_REF?}" --binary=clang-format-9 --style=file
+ /tmp/git-clang-format "${GITHUB_BASE_REF?}" --binary=clang-format-9 --style=file
git diff --exit-code
submodules:
@@ -122,16 +125,14 @@
tabs:
runs-on: ubuntu-18.04
- env:
- BASE_REF: ${{ github.base_ref }}
steps:
- 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 "${BASE_REF?}:${BASE_REF?}"
+ run: git fetch --no-tags --prune --depth=1 origin "${GITHUB_BASE_REF?}:${GITHUB_BASE_REF?}"
- name: Checking tabs
- run: ./scripts/check_tabs.sh "${BASE_REF?}"
+ run: ./scripts/check_tabs.sh "${GITHUB_BASE_REF?}"
yamllint:
runs-on: ubuntu-18.04
diff --git a/build_tools/bazel/iree_flatcc.bzl b/build_tools/bazel/iree_flatcc.bzl
index 6d35299..faacdcb 100644
--- a/build_tools/bazel/iree_flatcc.bzl
+++ b/build_tools/bazel/iree_flatcc.bzl
@@ -32,14 +32,14 @@
outs = []
for arg in flags:
if arg == "--reader":
- outs += ["%s_reader.h" % (out_stem)]
+ outs.append("%s_reader.h" % (out_stem))
if arg == "--builder":
- outs += ["%s_builder.h" % (out_stem)]
+ outs.append("%s_builder.h" % (out_stem))
if arg == "--verifier":
- outs += ["%s_verifier.h" % (out_stem)]
+ outs.append("%s_verifier.h" % (out_stem))
if arg == "--json":
- outs += ["%s_json_parser.h" % (out_stem)]
- outs += ["%s_json_printer.h" % (out_stem)]
+ outs.append("%s_json_parser.h" % (out_stem))
+ outs.append("%s_json_printer.h" % (out_stem))
native.genrule(
name = name + "_gen",
diff --git a/scripts/run_buildifier.sh b/scripts/run_buildifier.sh
new file mode 100755
index 0000000..f0b9dda
--- /dev/null
+++ b/scripts/run_buildifier.sh
@@ -0,0 +1,60 @@
+#!/bin/bash
+
+# Copyright 2021 Google LLC
+#
+# 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
+#
+# https://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.
+
+# Checks buildifier formatting and lint in files modified vs the specified
+# reference commit (default "main")
+# See https://github.com/bazelbuild/buildtools/tree/master/buildifier
+
+set -o pipefail
+
+BASE_REF="${1:-main}"
+
+declare -a included_files_patterns=(
+ "/BUILD$"
+ "\.bazel$"
+ "/WORKSPACE$"
+ "\.bzl$"
+)
+
+declare -a excluded_files_patterns=(
+ "/third_party/"
+ "^third_party/"
+)
+
+# Join on |
+included_files_pattern="$(IFS="|" ; echo "${included_files_patterns[*]?}")"
+excluded_files_pattern="$(IFS="|" ; echo "${excluded_files_patterns[*]?}")"
+
+readarray -t files < <(\
+ (git diff --name-only --diff-filter=d "${BASE_REF}" || kill $$) \
+ | grep -E "${included_files_pattern?}" \
+ | grep -v -E "${excluded_files_pattern?}")
+
+if [ ${#files[@]} -eq 0 ]; then
+ echo "No Bazel files changed"
+ exit 0
+fi
+
+echo "Fixing formatting with Buildifier"
+buildifier "${files[@]}"
+
+echo "Fixing automatically fixable lint with Buildifier"
+buildifier -lint=fix "${files[@]}"
+
+echo "Checking complicated lint with Buildifier"
+# Sometimes people don't write docstrings and I am not going to make that
+# blocking.
+buildifier -lint=warn -warnings=-module-docstring,-function-docstring "${files[@]}"
