Add pre-commit config and replace many lint checks with it. (iree-org…

…#17538) This adds a [pre-commit](https://pre-commit.com/) configuration file and migrates checks from our previous `lint.yml` and `lint.sh` files to using these new hooks, making progress on iree-org#17430. Each hook is versioned and is automatically installed by pre-commit when used. * Documentation updates to https://iree.dev/developers/general/contributing/#coding-style-guidelines will follow after some soak time (I haven't tested the actual pre-commit git hook portion of the tooling yet, for example). TLDR: ```bash # setup (or using your package manager of choice) py -m pip install --user pipx py -m pipx ensurepath pipx install pre-commit # run manually on changed files pre-commit run # run manually on all files pre-commit run --all-files # install git hook scripts pre-commit install ``` * A few checks are disabled for now. * `end-of-file-fixer` and `trailing-whitespace` are new checks for this project so I want to give developers some time to learn pre-commit workflows before adding them. * Other checks like `generated_cmake_files` and `buildifier` are still using the existing scripts while I figure out how to cleanly migrate them. Sample runs: * Success: https://github.com/iree-org/iree/actions/runs/9324252254/job/25669139815?pr=17538#step:4:54 * One error: https://github.com/iree-org/iree/actions/runs/9324187058/job/25668919747?pr=17538#step:4:85 skip-ci: test lint job only
nod-ai · Jun 3, 2024 · 778d00d · 778d00d
1 parent b54d17e
commit 778d00d
Show file tree

Hide file tree

Showing 6 changed files with 123 additions and 269 deletions.
diff --git a/.github/workflows/lint.yml b/.github/workflows/lint.yml
@@ -14,18 +14,15 @@ permissions:
   contents: read
 
 jobs:
-  bazel_to_cmake:
+  pre-commit:
     runs-on: ubuntu-20.04
     steps:
       - name: Checking out repository
         uses: actions/checkout@8f4b7f84864484a7bf31766abe9204da3cbe65b3 # v3.5.0
-      - name: Running bazel_to_cmake
-        run: ./build_tools/bazel_to_cmake/bazel_to_cmake.py --verbosity=2
-      - name: Checking Diff
-        run: |
-          # Make sure to pick up new files that have been added.
-          git add -A
-          git diff HEAD --exit-code
+      - name: Setting up python
+        uses: actions/setup-python@d27e3f3d7c64b4bbf8e4abfb9b63b83e846e0435 # v4.5.0
+      - name: Running pre-commit
+        uses: pre-commit/action@2c7b3805fd2a0fd8c1884dcaebf91fc102a13ecd # v3.0.1
 
   buildifier:
     runs-on: ubuntu-20.04
@@ -50,31 +47,6 @@ jobs:
           git diff --exit-code
           exit "${EXIT_CODE?}"
 
-  black:
-    runs-on: ubuntu-20.04
-    steps:
-      - name: Checking out repository
-        uses: actions/checkout@8f4b7f84864484a7bf31766abe9204da3cbe65b3 # v3.5.0
-      - name: Setting up python
-        uses: actions/setup-python@d27e3f3d7c64b4bbf8e4abfb9b63b83e846e0435 # v4.5.0
-      - 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: Install black
-        run: |
-          python3 -m pip install black==23.3
-      - name: Check if modified files are formatted
-        run: |
-          # The filter lowercase `d` means to exclude deleted files.
-          git diff "${GITHUB_BASE_REF?}" --name-only --diff-filter=d \
-            -- '*.py' ':!third_party' \
-            | xargs --no-run-if-empty black --check --diff --verbose
-      - name: Instructions for fixing the above linting errors
-        if: failure()
-        run: |
-          printf "You can fix formatting by running 'black' on the modified python files:\n"
-          printf "  git diff ${GITHUB_BASE_REF?} --name-only -- '*.py' ':!third_party' | xargs black\n"
-
   pytype:
     runs-on: ubuntu-20.04
     steps:
@@ -90,66 +62,6 @@ jobs:
       - name: Run pytype on changed files
         run: ./build_tools/pytype/check_diff.sh "${GITHUB_BASE_REF?}"
 
-  clang-format:
-    runs-on: ubuntu-24.04
-    steps:
-      - name: Installing dependencies
-        run: |
-          wget https://raw.githubusercontent.com/llvm/llvm-project/main/clang/tools/clang-format/git-clang-format -O /tmp/git-clang-format
-          chmod +x /tmp/git-clang-format
-      - name: Checking out repository
-        uses: actions/checkout@8f4b7f84864484a7bf31766abe9204da3cbe65b3 # v3.5.0
-      - 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?}"
-      # See which versions are available for the GitHub-hosted runner here:
-      # https://github.com/actions/runner-images/blob/main/images/ubuntu/Ubuntu2404-Readme.md#language-and-runtime
-      - name: Running clang-format on changed source files
-        run: |
-          clang-format-18 --version
-          /tmp/git-clang-format "${GITHUB_BASE_REF?}" --binary=clang-format-18 --style=file || true
-          git diff --exit-code
-
-  tabs:
-    runs-on: ubuntu-20.04
-    steps:
-      - name: Checking out repository
-        uses: actions/checkout@8f4b7f84864484a7bf31766abe9204da3cbe65b3 # v3.5.0
-      - 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: Checking tabs
-        run: ./build_tools/scripts/check_tabs.sh "${GITHUB_BASE_REF?}"
-
-  yamllint:
-    runs-on: ubuntu-20.04
-    steps:
-      - name: Checking out repository
-        uses: actions/checkout@8f4b7f84864484a7bf31766abe9204da3cbe65b3 # v3.5.0
-      - 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: yamllint
-        run: ./build_tools/scripts/run_yamllint.sh "${GITHUB_BASE_REF?}"
-
-  markdownlint:
-    runs-on: ubuntu-20.04
-    steps:
-      - name: Checking out repository
-        uses: actions/checkout@8f4b7f84864484a7bf31766abe9204da3cbe65b3 # v3.5.0
-      - name: Downloading markdownlint
-        run: npm install -g markdownlint-cli@0.41.0
-      - name: Running markdownlint
-        run: ./build_tools/scripts/run_markdownlint.sh
-
-  path_lengths:
-    runs-on: ubuntu-20.04
-    steps:
-      - name: Checking out repository
-        uses: actions/checkout@8f4b7f84864484a7bf31766abe9204da3cbe65b3 # v3.5.0
-      - name: Running check_path_lengths
-        run: ./build_tools/scripts/check_path_lengths.py
-
   generated_cmake_files:
     runs-on: ubuntu-20.04
     steps:
@@ -164,16 +76,3 @@ jobs:
           # pick up new files that have been added.
           git add -A
           git diff HEAD --exit-code
-
-  build_file_names:
-    runs-on: ubuntu-20.04
-    steps:
-      - name: Checking out repository
-        uses: actions/checkout@8f4b7f84864484a7bf31766abe9204da3cbe65b3 # v3.5.0
-      - name: Check BUILD files are not named BUILD (prefer BUILD.bazel)
-        run: exit $(git ls-files '**/BUILD' | wc -l)
-      - name: Instructions for fixing the above linting error
-        if: failure()
-        run: |
-          echo "failure: found files named BUILD. Please rename the following files to BUILD.bazel:"
-          git ls-files '**/BUILD'
diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml
@@ -0,0 +1,116 @@
+# Pre-commit (https://pre-commit.com) configuration for assorted lint checks.
+#
+# See https://pre-commit.com/hooks.html for more hooks.
+
+exclude: "third_party/"
+
+repos:
+
+-   repo: https://github.com/pre-commit/pre-commit-hooks
+    rev: v3.2.0
+    hooks:
+    -   id: check-merge-conflict
+
+    -   id: check-yaml
+        # * Extensions can't be included in the mkdocs schema, so skip checking
+        #   https://github.com/squidfunk/mkdocs-material/issues/6378
+        # * clang-format files use `---` to split for multiple languages,
+        #   resulting in errors like `expected a single document in the stream`
+        exclude: "mkdocs.yml|.clang-format"
+
+    # TODO(scotttodd): enable once enough contributors are familiar with pre-commit
+    # -   id: end-of-file-fixer
+    #     exclude_types: ["image", "jupyter"]
+
+    # TODO(scotttodd): enable once enough contributors are familiar with pre-commit
+    # -   id: trailing-whitespace
+    #     exclude_types: ["image", "jupyter"]
+
+-   repo: https://github.com/psf/black
+    rev: 23.3.0
+    hooks:
+    -   id: black
+        name: Run Black to format Python files
+
+-   repo: https://github.com/pre-commit/mirrors-clang-format
+    # Loosely track the most recent versions in
+    #   * Runner images: https://github.com/actions/runner-images/
+    #   * Editor extensions: https://github.com/microsoft/vscode-cpptools
+    rev: v18.1.3
+    hooks:
+    -   id: clang-format
+        name: Run clang-format on C/C++/etc. files
+        exclude_types: ["jupyter"]
+
+-   repo: https://github.com/igorshubovych/markdownlint-cli
+    rev: v0.41.0
+    hooks:
+    -   id: markdownlint
+        name: Run markdownlint on .md files
+        args: ["--config", "docs/.markdownlint.yml"]
+        files: "docs/website/.*.md"
+        exclude: "mlir-dialects/!(index).md"
+
+-   repo: https://github.com/Lucas-C/pre-commit-hooks
+    rev: v1.5.5
+    hooks:
+    -   id: forbid-tabs
+        exclude: ".gitmodules|Makefile"
+
+-   repo: https://github.com/jlebar/pre-commit-hooks.git
+    rev: f2d115a052860b09b2888b4f104be614bf3b4779
+    hooks:
+      # TODO(scotttodd): Download as needed
+      #   https://github.com/jlebar/pre-commit-hooks/issues/3
+      # TODO(scotttodd): enable when this works on Windows
+      #   https://github.com/bazelbuild/buildtools/issues/914
+      #   could try `git add . --renormalize`
+      # -   id: bazel-buildifier
+
+      -   id: do-not-submit
+
+-   repo: local
+    hooks:
+    -   id: bazel_to_cmake
+        name: Run bazel_to_cmake.py
+        language: python
+        entry: ./build_tools/bazel_to_cmake/bazel_to_cmake.py
+        # TODO(scotttodd): run on BUILD.bazel/CMakeLists.txt files individually
+        always_run: true
+        pass_filenames: false
+
+    # TODO(scotttodd): replace with an official pre-commit hook?
+    #   See https://github.com/pre-commit/pre-commit-hooks/issues/760
+    -   id: check_path_lengths
+        name: Run check_path_lengths.py
+        language: python
+        entry: ./build_tools/scripts/check_path_lengths.py
+        always_run: true
+        pass_filenames: false
+
+    -   id: build_file_names
+        name: Check Bazel file names
+        entry: Files should be named BUILD.bazel instead of BUILD
+        language: fail
+        files: "BUILD$"
+
+    # TODO(scotttodd): enable these checks when they work on Windows
+    #   the generator scripts use \ on Windows instead of /
+
+    # -   id: generate_cmake_e2e_test_artifacts_suite
+    #     name: Run generate_cmake_e2e_test_artifacts_suite.py
+    #     language: python
+    #     entry: ./build_tools/testing/generate_cmake_e2e_test_artifacts_suite.py
+    #     args: ["--output_dir", "./tests/e2e/test_artifacts"]
+    #     # TODO(scotttodd): run only on relevant files
+    #     always_run: true
+    #     pass_filenames: false
+
+    # -   id: generate_cmake_e2e_model_tests
+    #     name: Run generate_cmake_e2e_model_tests.py
+    #     language: python
+    #     entry: ./build_tools/testing/generate_cmake_e2e_model_tests.py
+    #     args: ["--output", "./tests/e2e/stablehlo_models/generated_e2e_model_tests.cmake"]
+    #     # TODO(scotttodd): run only on relevant files
+    #     always_run: true
+    #     pass_filenames: false
diff --git a/build_tools/scripts/check_tabs.sh b/build_tools/scripts/check_tabs.sh
diff --git a/build_tools/scripts/lint.sh b/build_tools/scripts/lint.sh
@@ -44,51 +44,21 @@ if [[ $? -ne 0 ]]; then
   exit 1
 fi
 
-echo "***** Bazel -> CMake *****"
-./build_tools/bazel_to_cmake/bazel_to_cmake.py
-git add -A
-git diff HEAD --exit-code
+echo "***** pre-commit *****"
+pre-commit run
 
 echo "***** buildifier *****"
 ${scripts_dir}/run_buildifier.sh
 git diff --exit-code
 
-echo "***** black *****"
-# The filter lowercase `d` means to exclude deleted files.
-git diff main --name-only --diff-filter=d -- '*.py' ':!third_party' \
-  | xargs -r black
-
 echo "***** pytype *****"
 ./build_tools/pytype/check_diff.sh
 
-echo "***** clang-format *****"
-git-clang-format --style=file main
-git diff --exit-code
-
-echo "***** tabs *****"
-"${scripts_dir}/check_tabs.sh"
-
-echo "***** yamllint *****"
-"${scripts_dir}/run_yamllint.sh"
-
-echo "***** markdownlint *****"
-"${scripts_dir}/run_markdownlint.sh"
-
-echo "***** Path Lengths *****"
-./build_tools/scripts/check_path_lengths.py
-
 echo "***** Generates CMake files *****"
 ./build_tools/scripts/generate_cmake_files.sh
 git add -A
 git diff HEAD --exit-code
 
-echo "***** Check BUILD files are not named BUILD (prefer BUILD.bazel) *****"
-if [[ $(git ls-files '**/BUILD') ]]; then
-  echo "failure: found files named BUILD. Please rename the following files to BUILD.bazel:"
-  git ls-files '**/BUILD'
-  (exit 1)
-fi
-
 if (( "${FINAL_RET}" != 0 )); then
   echo "Encountered failures when running: '${FAILING_CMD}'. Check error" \
        "messages and changes to the working directory and git index (which" \