Merge pull request sysprog21#598 from ChinYikMing/shfmt

Add shell script formatter

Author: jserv

.ci/boot-linux-prepare.sh

@@ -5,10 +5,19 @@
 check_platform
 
 VBLK_IMG=build/disk.img
-which dd >/dev/null 2>&1 || { echo "Error: dd not found"; exit 1; }
-which mkfs.ext4 >/dev/null 2>&1 || which $(brew --prefix e2fsprogs)/sbin/mkfs.ext4 >/dev/null 2>&1 || \
-	{ echo "Error: mkfs.ext4 not found"; exit 1; }
-which 7z >/dev/null 2>&1 || { echo "Error: 7z not found"; exit 1; }
+which dd > /dev/null 2>&1 || {
+    echo "Error: dd not found"
+    exit 1
+}
+which mkfs.ext4 > /dev/null 2>&1 || which $(brew --prefix e2fsprogs)/sbin/mkfs.ext4 > /dev/null 2>&1 \
+    || {
+        echo "Error: mkfs.ext4 not found"
+        exit 1
+    }
+which 7z > /dev/null 2>&1 || {
+    echo "Error: 7z not found"
+    exit 1
+}
 
 ACTION=$1
 
@@ -23,11 +32,11 @@ case "$ACTION" in
                 mkfs.ext4 ${VBLK_IMG}
                 BLK_DEV=$(losetup -f)
                 losetup ${BLK_DEV} ${VBLK_IMG}
-            ;;
+                ;;
             Darwin)
                 $(brew --prefix e2fsprogs)/sbin/mkfs.ext4 ${VBLK_IMG}
                 BLK_DEV=$(hdiutil attach -nomount ${VBLK_IMG})
-            ;;
+                ;;
         esac
 
         # On Linux, ${VBLK_IMG} will be created by root and owned by root:root.
@@ -48,10 +57,10 @@ case "$ACTION" in
         case "${OS_TYPE}" in
             Linux)
                 losetup -d ${BLK_DEV}
-            ;;
+                ;;
             Darwin)
                 hdiutil detach ${BLK_DEV}
-            ;;
+                ;;
         esac
 
         # delete disk image

.ci/boot-linux.sh

@@ -4,12 +4,14 @@
 
 check_platform
 
-function cleanup {
+function cleanup
+{
     sleep 1
     pkill -9 rv32emu
 }
 
-function ASSERT {
+function ASSERT
+{
     $*
     local RES=$?
     if [ ${RES} -ne 0 ]; then
@@ -30,45 +32,45 @@ OPTS_BASE+=" -i build/linux-image/rootfs.cpio"
 
 TEST_OPTIONS=("base (${OPTS_BASE})")
 EXPECT_CMDS=('
-expect "buildroot login:" { send "root\n" } timeout { exit 1 }
-expect "# " { send "uname -a\n" } timeout { exit 2 }
-expect "riscv32 GNU/Linux" { send "\x01"; send "x" } timeout { exit 3 }
+    expect "buildroot login:" { send "root\n" } timeout { exit 1 }
+    expect "# " { send "uname -a\n" } timeout { exit 2 }
+    expect "riscv32 GNU/Linux" { send "\x01"; send "x" } timeout { exit 3 }
 ')
 
 COLOR_G='\e[32;01m' # Green
 COLOR_R='\e[31;01m' # Red
 COLOR_Y='\e[33;01m' # Yellow
-COLOR_N='\e[0m' # No color
+COLOR_N='\e[0m'     # No color
 
-MESSAGES=("${COLOR_G}OK!" \
-     "${COLOR_R}Fail to boot" \
-     "${COLOR_R}Fail to login" \
-     "${COLOR_R}Fail to run commands" \
-     "${COLOR_R}Fail to find emu.txt in ${VBLK_IMG}"\
+MESSAGES=("${COLOR_G}OK!"
+    "${COLOR_R}Fail to boot"
+    "${COLOR_R}Fail to login"
+    "${COLOR_R}Fail to run commands"
+    "${COLOR_R}Fail to find emu.txt in ${VBLK_IMG}"
 )
 
 if [ "${ENABLE_VBLK}" -eq "1" ]; then
     # Read-only
     TEST_OPTIONS+=("${OPTS_BASE} -x vblk:${VBLK_IMG},readonly")
     EXPECT_CMDS+=('
-    expect "buildroot login:" { send "root\n" } timeout { exit 1 }
-    expect "# " { send "uname -a\n" } timeout { exit 2 }
-    expect "riscv32 GNU/Linux" { send "mkdir mnt && mount /dev/vda mnt\n" } timeout { exit 3 }
-    expect "# " { send "echo rv32emu > mnt/emu.txt\n" } timeout { exit 3 }
-    expect -ex "-sh: can'\''t create mnt/emu.txt: Read-only file system" {} timeout { exit 3 }
-    expect "# " { send "\x01"; send "x" } timeout { exit 3 }
+        expect "buildroot login:" { send "root\n" } timeout { exit 1 }
+        expect "# " { send "uname -a\n" } timeout { exit 2 }
+        expect "riscv32 GNU/Linux" { send "mkdir mnt && mount /dev/vda mnt\n" } timeout { exit 3 }
+        expect "# " { send "echo rv32emu > mnt/emu.txt\n" } timeout { exit 3 }
+        expect -ex "-sh: can'\''t create mnt/emu.txt: Read-only file system" {} timeout { exit 3 }
+        expect "# " { send "\x01"; send "x" } timeout { exit 3 }
     ')
 
     # Read-write using disk image
     TEST_OPTIONS+=("${OPTS_BASE} -x vblk:${VBLK_IMG}")
     VBLK_EXPECT_CMDS='
-     expect "buildroot login:" { send "root\n" } timeout { exit 1 }
-     expect "# " { send "uname -a\n" } timeout { exit 2 }
-     expect "riscv32 GNU/Linux" { send "mkdir mnt && mount /dev/vda mnt\n" } timeout { exit 3 }
-     expect "# " { send "echo rv32emu > mnt/emu.txt\n" } timeout { exit 3 }
-     expect "# " { send "sync\n" } timeout { exit 3 }
-     expect "# " { send "umount mnt\n" } timeout { exit 3 }
-     expect "# " { send "\x01"; send "x" } timeout { exit 3 }
+        expect "buildroot login:" { send "root\n" } timeout { exit 1 }
+        expect "# " { send "uname -a\n" } timeout { exit 2 }
+        expect "riscv32 GNU/Linux" { send "mkdir mnt && mount /dev/vda mnt\n" } timeout { exit 3 }
+        expect "# " { send "echo rv32emu > mnt/emu.txt\n" } timeout { exit 3 }
+        expect "# " { send "sync\n" } timeout { exit 3 }
+        expect "# " { send "umount mnt\n" } timeout { exit 3 }
+        expect "# " { send "\x01"; send "x" } timeout { exit 3 }
     '
     EXPECT_CMDS+=("${VBLK_EXPECT_CMDS}")
 
@@ -87,7 +89,7 @@ for i in "${!TEST_OPTIONS[@]}"; do
     fi
     RUN_LINUX="build/rv32emu ${OPTS}"
 
-    ASSERT expect <<-DONE
+    ASSERT expect <<- DONE
 	set timeout ${TIMEOUT}
 	spawn ${RUN_LINUX}
 	${EXPECT_CMDS[$i]}
@@ -100,7 +102,7 @@ for i in "${!TEST_OPTIONS[@]}"; do
     if [[ "${TEST_OPTIONS[$i]}" =~ vblk ]]; then
         # read-only test first, so the emu.txt definitely does not exist, skipping the check
         if [[ ! "${TEST_OPTIONS[$i]}" =~ readonly ]]; then
-            7z l ${VBLK_IMG} | grep emu.txt >/dev/null 2>&1 || ret=4
+            7z l ${VBLK_IMG} | grep emu.txt > /dev/null 2>&1 || ret=4
         fi
         printf "Virtio-blk Test: [ ${MESSAGES[$ret]}${COLOR_N} ]\n"
     fi

.ci/check-format.sh

@@ -1,14 +1,24 @@
 #!/usr/bin/env bash
 
-set -e -u -o pipefail
+# The -e is not set because we want to get all the mismatch format at once
 
-SOURCES=$(find $(git rev-parse --show-toplevel) | egrep "\.(c|cxx|cpp|h|hpp)\$")
+set -u -o pipefail
 
 set -x
 
-for file in ${SOURCES};
-do
+REPO_ROOT="$(git rev-parse --show-toplevel)"
+
+C_SOURCES=$(find "${REPO_ROOT}" | egrep "\.(c|cxx|cpp|h|hpp)$")
+for file in ${C_SOURCES}; do
     clang-format-18 ${file} > expected-format
     diff -u -p --label="${file}" --label="expected coding style" ${file} expected-format
 done
-exit $(clang-format-18 --output-replacements-xml ${SOURCES} | egrep -c "</replacement>")
+C_MISMATCH_LINE_CNT=$(clang-format-18 --output-replacements-xml ${C_SOURCES} | egrep -c "</replacement>")
+
+SH_SOURCES=$(find "${REPO_ROOT}" | egrep "\.sh$")
+for file in ${SH_SOURCES}; do
+    shfmt -d "${file}"
+done
+SH_MISMATCH_FILE_CNT=$(shfmt -l ${SH_SOURCES})
+
+exit $((C_MISMATCH_LINE_CNT + SH_MISMATCH_FILE_CNT))

.ci/common.sh

@@ -6,8 +6,8 @@ OS_TYPE=$(uname -s)
 check_platform()
 {
     case "${MACHINE_TYPE}/${OS_TYPE}" in
-        x86_64/Linux | aarch64/Linux | arm64/Darwin)
-            ;;
+        x86_64/Linux | aarch64/Linux | arm64/Darwin) ;;
+
         *)
             echo "Unsupported platform: ${MACHINE_TYPE}/${OS_TYPE}"
             exit 1
@@ -16,7 +16,7 @@ check_platform()
 }
 
 if [[ "${OS_TYPE}" == "Linux" ]]; then
-  PARALLEL=-j$(nproc)
+    PARALLEL=-j$(nproc)
 else
-  PARALLEL=-j$(sysctl -n hw.logicalcpu)
+    PARALLEL=-j$(sysctl -n hw.logicalcpu)
 fi

.ci/riscv-tests.sh

@@ -18,7 +18,7 @@ make distclean
 # for this purpose, although the emulator can run all selected benchmarks with
 # much smaller memory mapping regions.
 make ENABLE_ARCH_TEST=1 ENABLE_EXT_M=1 ENABLE_EXT_A=1 ENABLE_EXT_F=1 ENABLE_EXT_C=1 \
-     ENABLE_Zicsr=1 ENABLE_Zifencei=1 ENABLE_FULL4G=1 $PARALLEL
+    ENABLE_Zicsr=1 ENABLE_Zifencei=1 ENABLE_FULL4G=1 $PARALLEL
 make arch-test RISCV_DEVICE=IMAFCZicsrZifencei hw_data_misaligned_support=1 $PARALLEL || exit 1
 make arch-test RISCV_DEVICE=FCZicsr hw_data_misaligned_support=1 $PARALLEL || exit 1
 make arch-test RISCV_DEVICE=IMZbaZbbZbcZbs hw_data_misaligned_support=1 $PARALLEL || exit 1
@@ -33,6 +33,6 @@ make arch-test RISCV_DEVICE=E $PARALLEL || exit 1
 # the hardware misalignment (hw_data_misaligned_support) option.
 make distclean
 make ENABLE_ARCH_TEST=1 ENABLE_JIT=1 ENABLE_T2C=0 \
-     ENABLE_EXT_M=1 ENABLE_EXT_A=1 ENABLE_EXT_F=1 ENABLE_EXT_C=1 \
-     ENABLE_Zicsr=1 ENABLE_Zifencei=1 ENABLE_FULL4G=1 $PARALLEL
+    ENABLE_EXT_M=1 ENABLE_EXT_A=1 ENABLE_EXT_F=1 ENABLE_EXT_C=1 \
+    ENABLE_Zicsr=1 ENABLE_Zifencei=1 ENABLE_FULL4G=1 $PARALLEL
 make arch-test RISCV_DEVICE=IMC hw_data_misaligned_support=0 $PARALLEL || exit 1

.editorconfig

@@ -0,0 +1,14 @@
+# Top-level EditorConfig file
+root = true
+
+# Shell script-specific settings
+[*.sh]
+indent_style = space
+indent_size = 4
+end_of_line = lf
+trim_trailing_whitespace = true
+insert_final_newline = true
+function_next_line = true
+switch_case_indent = true
+space_redirects = true
+binary_next_line = true

.github/workflows/main.yml

@@ -505,7 +505,7 @@ jobs:
     - uses: actions/checkout@v4
     - name: coding convention
       run: |
-            sudo apt-get install -q=2 clang-format-18
+            sudo apt-get install -q=2 clang-format-18 shfmt
             .ci/check-newline.sh
             .ci/check-format.sh
       shell: bash

CONTRIBUTING.md

@@ -38,16 +38,32 @@ However, participation requires adherence to fundamental ground rules:
 * Code must strictly adhere to the established C coding style (refer to the guidelines below).
   While there is some flexibility in basic style, it is crucial to stick to the current coding standards.
   Complex algorithmic constructs without proper comments will not be accepted.
+* Shell scripts must be formatted before submission. Use consistent flags across the project to ensure uniform formatting.
 * External pull requests should include thorough documentation in the pull request comments for consideration.
 * When composing documentation, code comments, and other materials in English,
   please adhere to the American English (`en_US`) dialect.
   This variant should be considered the standard for all documentation efforts.
   For instance, opt for "initialize" over "initialise" and "color" rather than "colour".
 
-Software requirement: [clang-format](https://clang.llvm.org/docs/ClangFormat.html) version 18 or later.
+Software requirement:
+* [clang-format](https://clang.llvm.org/docs/ClangFormat.html) version 18 or later.
+* [shfmt](https://github.com/mvdan/sh).
 
-This repository consistently contains an up-to-date `.clang-format` file with rules that match the explained ones.
-For maintaining a uniform coding style, execute the command `clang-format -i *.{c,h}`.
+This repository consistently contains an up-to-date `.clang-format` file with rules that match the explained ones and uses shell script formatting supported by `shfmt`.
+For maintaining a uniform coding style, execute the command `clang-format -i *.{c,h}` and `shfmt -w $(find . -type f -name "*.sh")`.
+
+## Coding Style for Shell Script
+
+Shell scripts must be clean, consistent, and portable. The following `shfmt` rules(check `.editorconfig` file) are enforced project-wide:
+* Use spaces for indentation.
+* Indent with 4 spaces.
+* Use Unix-style line endings (LF).
+* Remove trailing whitespace at the end of lines.
+* Ensure the file ends with a newline.
+* Place the opening brace of a function on the next line.
+* Indent `case` statements within `switch` blocks.
+* Add spaces around redirection operators (e.g., `>`, `>>`).
+* Place binary operators (e.g., `&&`, `|`) on the next line when breaking lines.
 
 ## Coding Style for Modern C
 
@@ -877,22 +893,22 @@ Author: Jim Huang <[email protected]>
 Date:   Mon Feb 24 13:08:32 2025 +0800
 
     Introduce CPU architecture filtering in scheduler
-    
+
     In environments with mixed CPU architectures, it is crucial to ensure
     that an instance runs only on a host with a compatible CPU
     type—preventing, for example, a RISC-V instance from being scheduled on
     an Arm host.
-    
+
     This new scheduler filter enforces that requirement by comparing an
     instance's architecture against the host's allowed architectures. For
     the libvirt driver, the host's guest capabilities are queried, and the
     permitted architectures are recorded in the permitted_instances_types
     list within the host's cpu_info dictionary.
-    
+
     The filter systematically excludes hosts that do not support the
     instance's CPU architecture. Additionally, RISC-V has been added to the
     set of acceptable architectures for scheduling.
-    
+
     Note that the CPU architecture filter is disabled by default.
 ```
 

tests/common.sh

@@ -1,4 +1,4 @@
-function fail()                                                                                                                                               
+function fail()
 {
     echo "*** Fail"
     exit 1

tests/dhrystone.sh

@@ -25,8 +25,7 @@ function run_dhrystone()
 sanity_check
 # Run Dhrystone benchmark and collect DMIPS values
 dmips_values=()
-for ((i=1; i<=$N_RUNS; i++))
-do
+for ((i = 1; i <= $N_RUNS; i++)); do
     echo "Running Dhrystone benchmark - Run #$i"
     dmips=$(run_dhrystone)
     exit_code=$?
@@ -48,7 +47,7 @@ fi
 
 deviation=0
 for dmips in "${sorted_dmips[@]}"; do
-    if (( $(echo "$dmips > $median" | bc -l) )); then
+    if (($(echo "$dmips > $median" | bc -l))); then
         diff=$(echo "$dmips - $median" | bc -l)
     else
         diff=$(echo "$median - $dmips" | bc -l)
@@ -60,15 +59,14 @@ mad=$(echo "scale=2; $deviation / $num_dmips" | bc -l)
 
 # Filter outliers based on MAD
 filtered_dmips=()
-for dmips in "${sorted_dmips[@]}"
-do
-    if (( $(echo "$dmips > 0" | bc -l) )); then
-        if (( $(echo "$dmips > $median" | bc -l) )); then
+for dmips in "${sorted_dmips[@]}"; do
+    if (($(echo "$dmips > 0" | bc -l))); then
+        if (($(echo "$dmips > $median" | bc -l))); then
             diff=$(echo "$dmips - $median" | bc -l)
         else
             diff=$(echo "$median - $dmips" | bc -l)
         fi
-        if (( $(echo "$diff <= $mad * 2" | bc -l) )); then
+        if (($(echo "$diff <= $mad * 2" | bc -l))); then
             filtered_dmips+=("$dmips")
         fi
     fi
@@ -83,8 +81,7 @@ echo -n "" > $benchmark_output
 num_filtered=${#filtered_dmips[@]}
 if ((num_filtered > 0)); then
     total_dmips=0
-    for dmips in "${filtered_dmips[@]}"
-    do
+    for dmips in "${filtered_dmips[@]}"; do
         total_dmips=$(echo "scale=2; $total_dmips + $dmips" | bc -l)
     done