Merge branch 'lint' into 'main'

Improve linting flow

See merge request repositories/verilator-infrastructure-bugpoint!23

.ci.yml

@@ -8,8 +8,7 @@ test-and-lint:
   image: "verilator/verilator:v5.016"
   tags: ['ace-x86_64']
   script:
-    - apt-get -qqy update && apt-get -qqy --no-install-recommends install cmake build-essential git ca-certificates python3 clang-format-14 mold ninja-build
+    - apt-get -qqy update && apt-get -qqy --no-install-recommends install cmake build-essential git ca-certificates python3 clang-format-14 mold ninja-build shellcheck
     - cmake -B build -DCMAKE_BUILD_TYPE=RelWithDebInfo -G Ninja -DCMAKE_EXE_LINKER_FLAGS="-fuse-ld=mold" -DCMAKE_SHARED_LINKER_FLAGS="-fuse-ld=mold"
     - cmake --build build -j"$(nproc)"
-    - PATH="$PWD/build:$PWD/scripts:$PATH" && make -j"$(nproc)" -O -k -C tests/ --no-print-directory
-    - .ci-scripts/cpp-fmt-check source/*.cpp source/*.hpp
+    - PATH="$PWD/build:$PWD/scripts:$PATH" && make -j"$(nproc)" -O -k --no-print-directory -f check.mk

.github/workflows/test.yml

@@ -12,14 +12,12 @@ jobs:
     steps:
       - uses: actions/checkout@v4
       - name: Install deps
-        run: apt-get -qqy update && apt-get -qqy --no-install-recommends install cmake build-essential git ca-certificates python3 clang-format-14
+        run: apt-get -qqy update && apt-get -qqy --no-install-recommends install cmake build-essential git ca-certificates python3 clang-format-14 shellcheck
       - name: Build
         run: |
           cmake -B build -DCMAKE_BUILD_TYPE=RelWithDebInfo
           cmake --build build -j"$(nproc)"
-      - name: Run tests
+      - name: Run tests and linters
         run: |
           PATH="$PWD/build:$PWD/scripts:$PATH"
-          make -j"$(nproc)" -O -k -C tests/ --no-print-directory
-      - name: Check formatting
-        run: .ci-scripts/cpp-fmt-check source/*.cpp source/*.hpp
+          make -j"$(nproc)" -O -k --no-print-directory -f check.mk

README.md

@@ -78,6 +78,21 @@ The script attempts to:
 
 This script works on a best-effort basis, and it is expected that the result will require some manual adjustments.
 
-## Tests
+## Testing and linting
 
-In order to launch tests, go to the `tests/` directory and run `make`.
+`make`, `clang-format`, `shellcheck` and `verilator` are prerequisites for testing and linting.
+
+To run all tests, linters and format-checkers, invoke:
+```
+make -f check.mk
+```
+
+in project root. To auto-apply linter/formatter fixes, run:
+```
+make -f check.mk autofix
+```
+
+Golden files used in tests can be regenerated using:
+```
+GOLDEN=1 make -f check.mk test
+```

check.mk

@@ -0,0 +1,43 @@
+SHELL := /bin/bash
+
+.PHONY: test_and_lint_dont_abort_after_err
+test_and_lint_dont_abort_after_err:
+	@$(MAKE) -f check.mk -k test_and_lint
+
+.PHONY: test_and_lint
+test_and_lint: test lint
+
+.PHONY: autofix
+autofix: autofmt shellcheck_autofix
+
+.PHONY: autofmt
+autofmt:
+	clang-format-14 -i source/*.cpp source/*.hpp
+
+.PHONY: test
+test:
+	@$(MAKE) -C tests
+
+.PHONY: lint
+lint: fmt_check shellcheck
+
+.PHONY: fmt_check
+fmt_check:
+	for i in source/*.cpp source/*.hpp; do \
+	  diff "$$i" <(clang-format-14 "$$i")  --label "original $$i" --label "formatted $$i" --color=always -u; \
+	done
+	@echo # blank line for consistency
+
+
+SCRIPTS=scripts/* tests/*.sh tests/run_test \
+	examples/caliptra_vcd/sv-bugpoint-check.sh \
+	examples/caliptra_verilation_err/sv-bugpoint-check.sh
+
+.PHONY: shellcheck
+shellcheck:
+	shellcheck $(SCRIPTS) --color
+
+.PHONY: shellcheck
+shellcheck_autofix:
+	shellcheck $(SCRIPTS) -f diff --color || exit 0
+	shellcheck $(SCRIPTS) -f diff | git apply

examples/caliptra_vcd/sv-bugpoint-check.sh

@@ -29,6 +29,7 @@ mkfifo sim.vcd
 trap cleanup EXIT INT HUP TERM
 
 timeout 3 ./obj_dir/Vcaliptra_top_tb &
+# shellcheck disable=SC2016
 # extract all changes to timer1_timeout_period[0] and timer1_timeout_period[1] before timestamp "40"
 timeout 3 awk '
 /var wire .* timer1_timeout_period\[/ { ID_TO_NAME[$4]=$5 }

tests/run_test

@@ -5,22 +5,19 @@
 printf "TEST: %s\n" "$1"
 
 actual_file=out/"$1"/sv-bugpoint-minimized.sv
-[ -e $actual_file ] && rm $actual_file # to not get fake PASS if bugpoint did not overwrite old result
+[ -e "$actual_file" ] && rm "$actual_file" # to not get fake PASS if bugpoint did not overwrite old result
 golden_file=golden/"$1"/sv-bugpoint-minimized.sv
 
-bugpoint_msg=$(sv-bugpoint out/"$1" "$2" "$3" --force 2>&1)
-if [ $? -ne 0 ]; then
-  printf "%s\n" "$bugpoint_msg" >&2; exit 1
-fi
+bugpoint_msg=$(sv-bugpoint out/"$1" "$2" "$3" --force 2>&1) || (printf "%s\n" "$bugpoint_msg" >&2; exit 1)
 
-diff $golden_file $actual_file --color=always >&2
+diff "$golden_file" "$actual_file" --color=always >&2
 
 EXIT_CODE=$?
 
 [ "$EXIT_CODE" -eq 0 ] && printf "PASSED\n\n" || printf "FAILED\n\n"
 if [ -n "$GOLDEN" ]; then
-  mkdir -p "$(dirname $golden_file)"
-  cp $actual_file $golden_file
+  mkdir -p "$(dirname "$golden_file")"
+  cp "$actual_file" "$golden_file"
 fi
 
 exit $EXIT_CODE