CI improvements (vesoft-inc#304)

* Fix bashism * Report Gherkin diff * Add PHONY target for tests check-and-diff * Improve clang-format-diff handling Use GITHUB_BASE_REF because a PR can easily have more than one commit: https://docs.github.com/en/actions/learn-github-actions/environment-variables#default-environment-variables Switch CLANG_HOME handling to support debian/ubuntu installed clang-format or a user's installation at some other location. Install clang-format-10 if it isn't installed in CI (this is mostly for nektos/act where images might be thinner than the GitHub standard). * Get the base commit In order to see what changes this PR is making, we need to get the base commit. * Use actions/checkout to get base and head Co-authored-by: Josh Soref <[email protected]> Co-authored-by: Yee <[email protected]> Co-authored-by: kyle.cao <[email protected]> Co-authored-by: Josh Soref <[email protected]> Co-authored-by: Josh Soref <[email protected]> Co-authored-by: Yee <[email protected]> Co-authored-by: kyle.cao <[email protected]>
yixinglu · Nov 23, 2021 · 1bd92ad · 1bd92ad
1 parent e516637
commit 1bd92ad
Show file tree

Hide file tree

Showing 3 changed files with 44 additions and 11 deletions.
diff --git a/.github/workflows/pull_request.yml b/.github/workflows/pull_request.yml
@@ -23,24 +23,30 @@ jobs:
     steps:
       - uses: actions/checkout@v2
         with:
-          fetch-depth: 2
+          ref: ${{ github.event.pull_request.base.sha }}
+      - uses: actions/checkout@v2
+        with:
+          clean: false
       - name: Check License Header
         uses: apache/skywalking-eyes@main
+      - name: Ensure clang-format-10 is available
+        run: |
+          command -v clang-format-10 > /dev/null || (apt-get update && apt-get install -y clang-format-10)
       - name: Cpplint
         run: |
           ln -snf $PWD/.linters/cpp/hooks/pre-commit.sh $PWD/.linters/cpp/pre-commit.sh
-          .linters/cpp/pre-commit.sh $(git --no-pager diff --diff-filter=d --name-only HEAD^ HEAD)
+          .linters/cpp/pre-commit.sh $(git --no-pager diff --diff-filter=d --name-only ${{ github.event.pull_request.base.sha }} HEAD)
       - name: Format check
         run: |
-          res=$(git diff -U0 --no-color HEAD^ | /usr/share/clang/clang-format-10/clang-format-diff.py -p1)
-          [[ ! -z "$res" ]] && exit 1 || true
+          git diff -U0 --no-color ${{ github.event.pull_request.base.sha }} HEAD | /usr/share/clang/clang-format-10/clang-format-diff.py -p1 | tee /tmp/.clang-format-diff
+          [ -s /tmp/.clang-format-diff ] && exit 1 || true
       - uses: actions/setup-python@v2
         with:
           python-version: 3.7
       - name: Prepare Gherkin exec environ
         run: make init-all -C tests
       - name: Check Gherkin feature format
-        run: make check -C tests
+        run: make check-and-diff -C tests
 
   build:
     name: build

diff --git a/.linters/cpp/hooks/pre-commit.sh b/.linters/cpp/hooks/pre-commit.sh
@@ -56,12 +56,34 @@ fi
 
 echo "Performing C++ code format check..."
 
-CLANG_HOME=/opt/vesoft/toolset/clang/10.0.0/
+CLANG_FALLBACK=/opt/vesoft/toolset/clang/10.0.0/
+if [ -z "$CLANG_HOME" ] && [ -d "$CLANG_FALLBACK" ]; then
+    CLANG_HOME=$CLANG_FALLBACK
+fi
+
+CLANG_FORMAT=$(command -v clang-format-10)
+if [ -z "$CLANG_FORMAT" ]; then
+    CLANG_FORMAT=$CLANG_HOME/bin/clang-format
+fi
+
+CLANG_FORMAT_DIFF=$(command -v clang-format-diff-10)
+if [ -z "$CLANG_FORMAT_DIFF" ]; then
+    CLANG_FORMAT_DIFF=$CLANG_HOME/share/clang/clang-format-diff.py
+fi
 
-if [ ! -d "$CLANG_HOME" ]; then
-    echo "The $CLANG_HOME directory is not found, and the source changes cannot be automatically formatted."
+if [ -z "$CLANG_FORMAT" ] || [ -z "$CLANG_FORMAT_DIFF" ]; then
+    if [ ! -d "$CLANG_HOME" ]; then
+        echo "The $CLANG_HOME directory was not found."
+    fi
+    if [ -z "$CLANG_FORMAT" ]; then
+        echo "Could not find clang-format"
+    fi
+    if [ -z "$CLANG_FORMAT_DIFF" ]; then
+        echo "Could not find clang-format-diff"
+    fi
+    echo "source changes cannot be automatically formatted."
     exit 0
 fi
 
-git diff -U0 --no-color --staged | $CLANG_HOME/share/clang/clang-format-diff.py -i -p1 -binary $CLANG_HOME/bin/clang-format
+git diff -U0 --no-color --staged | "$CLANG_FORMAT_DIFF" -i -p1 -binary "$CLANG_FORMAT"
 git add $CHECK_FILES
diff --git a/tests/Makefile b/tests/Makefile
@@ -2,7 +2,7 @@
 #
 # This source code is licensed under Apache 2.0 License.
 
-.PHONY: fmt check init init-all clean test tck fail up down
+.PHONY: fmt check check-and-diff init init-all clean test tck fail up down
 
 PYPI_MIRROR = https://mirrors.aliyun.com/pypi/simple/
 # PYPI_MIRROR = http://pypi.mirrors.ustc.edu.cn/simple --trusted-host pypi.mirrors.ustc.edu.cn
@@ -37,7 +37,7 @@ install-nebula-py: install-deps
 	rm -rf $(CURR_DIR)/nebula-python
 
 gherkin-fmt: install-deps
-	@if [[ $(PY_VERSION) -lt 7 ]]; then echo 'Python version must >= 3.7'; exit 1; fi
+	@if [ $(PY_VERSION) -lt 7 ]; then echo 'Python version must >= 3.7'; exit 1; fi
 	pip3 install --user poetry
 	git clone --branch master https://github.com/OneContainer/reformat-gherkin $(CURR_DIR)/reformat-gherkin
 	cd $(CURR_DIR)/reformat-gherkin && python3 -m poetry build
@@ -55,6 +55,11 @@ fmt:
 check:
 	@find $(CURR_DIR)/tck/ -type f -iname "*.feature" -print | xargs $(gherkin_fmt) --check
 
+check-and-diff:
+	@(find $(CURR_DIR)/tck/ -type f -iname '*.feature' -print | xargs $(gherkin_fmt)) 2>&1 | tee .gherkin_fmt
+	@git diff
+	@tail -1 .gherkin_fmt | grep -qv ,
+
 up: clean
 	@mkdir -p $(CURR_DIR)/.pytest
 	$(run_test) --cmd=start \