Merge pull request #1879 from larsewi/linting

ENT-12601: Added workflow to lint shell scripts

Author: larsewi

.github/workflows/shellcheck.yml

@@ -0,0 +1,24 @@
+name: shellcheck
+
+on:
+  push:
+    branches: [master]
+  pull_request:
+    branches: [master]
+
+jobs:
+  check:
+    runs-on: ubuntu-latest
+    permissions:
+      contents: read
+
+    steps:
+      - uses: actions/checkout@v4
+      - name: Install dependencies
+        run: sudo apt install shellcheck
+      - name: Lint sources with shellcheck
+        run: |
+          # Recursively find all shell scripts in the build-scripts directory with a shebang
+          grep -Erl '^(#!/(bin|usr/bin)/(env )?(sh|bash))' build-scripts/ | while read -r file; do
+              shellcheck --external-sources --source-path=build-scripts "$file"
+          done

build-scripts/bootstrap-tarballs

@@ -112,9 +112,7 @@ run_and_print_on_failure ./configure
 echo "$(basename "$0"): Debug: Running make dist on masterfiles repository..."
 run_and_print_on_failure make dist # source tarball
 echo "$(basename "$0"): Debug: Running make tar-package on masterfiles repository..."
-run_and_print_on_failure make tar-package # package tarball (containing all
-# files as if they were installed
-# under "prefix".)
+run_and_print_on_failure make tar-package # package tarball (containing all files as if they were installed under "prefix".)
 mv cfengine-masterfiles*.tar.gz "$BASEDIR"/output/tarballs/
 echo "$(basename "$0"): Debug: Running make distclean on masterfiles repository..."
 run_and_print_on_failure make distclean

build-scripts/build-environment-check

@@ -1,6 +1,6 @@
 #!/bin/sh
 
-. $(dirname "$0")/functions
+. "$(dirname "$0")"/functions
 . detect-environment
 . compile-options
 
@@ -45,17 +45,17 @@ DEP_LIST="$DEP_LIST rsync gcc make sudo wget"
 
 RET=0
 for unwanted in $UNWANTED_DEPS; do
-    if query_pkg $unwanted; then
-        echo "ERROR Found unwanted package:" $unwanted
+    if query_pkg "$unwanted"; then
+        echo "ERROR Found unwanted package: $unwanted"
         RET=1
     fi
 done
 
 for dep in $DEP_LIST; do
-    if query_pkg $dep; then
+    if query_pkg "$dep"; then
         true
     else
-        echo "ERROR Missing system package:" $dep
+        echo "ERROR Missing system package: $dep"
         RET=1
     fi
 done

build-scripts/clean-results

@@ -1,6 +1,6 @@
 #!/bin/sh -ex
 
-. $(dirname "$0")/functions
+. "$(dirname "$0")"/functions
 . version
 
 # This script is designed to clean up old directories, specifically keeping the

build-scripts/compile

@@ -34,24 +34,24 @@ nova)
 esac
 
 echo "$(basename "$0"): Debug: Running make in core repo..."
-run_and_print_on_failure $MAKE -C "$BASEDIR"/core -k
+run_and_print_on_failure "$MAKE" -C "$BASEDIR"/core -k
 echo "$(basename "$0"): Debug: Running make install in core repo..."
-run_and_print_on_failure $MAKE -C "$BASEDIR"/core install DESTDIR="$BASEDIR"/cfengine/dist
+run_and_print_on_failure "$MAKE" -C "$BASEDIR"/core install DESTDIR="$BASEDIR"/cfengine/dist
 
 if [ "$NOVA" = yes ]; then
     echo "$(basename "$0"): Debug: Running make in enterprise repo..."
-    run_and_print_on_failure $MAKE -C "$BASEDIR"/enterprise -k
+    run_and_print_on_failure "$MAKE" -C "$BASEDIR"/enterprise -k
     echo "$(basename "$0"): Debug: Running make install in enterprise repo..."
-    run_and_print_on_failure $MAKE -C "$BASEDIR"/enterprise install DESTDIR="$BASEDIR"/cfengine/dist
+    run_and_print_on_failure "$MAKE" -C "$BASEDIR"/enterprise install DESTDIR="$BASEDIR"/cfengine/dist
     if [ "$ROLE" = hub ]; then
         echo "$(basename "$0"): Debug: Running make in nova repo..."
-        run_and_print_on_failure $MAKE -C "$BASEDIR"/nova -k
+        run_and_print_on_failure "$MAKE" -C "$BASEDIR"/nova -k
         echo "$(basename "$0"): Debug: Running make install in nova repo..."
-        run_and_print_on_failure $MAKE -C "$BASEDIR"/nova install DESTDIR="$BASEDIR"/cfengine/dist
+        run_and_print_on_failure "$MAKE" -C "$BASEDIR"/nova install DESTDIR="$BASEDIR"/cfengine/dist
         echo "$(basename "$0"): Debug: Running make install in masterfiles repo..."
-        run_and_print_on_failure $MAKE -C "$BASEDIR"/masterfiles install DESTDIR="$BASEDIR"/cfengine/dist
+        run_and_print_on_failure "$MAKE" -C "$BASEDIR"/masterfiles install DESTDIR="$BASEDIR"/cfengine/dist
     fi
 else
     echo "$(basename "$0"): Debug: Running make install in masterfiles repo..."
-    run_and_print_on_failure $MAKE -C "$BASEDIR"/masterfiles install DESTDIR="$BASEDIR"/cfengine/dist
+    run_and_print_on_failure "$MAKE" -C "$BASEDIR"/masterfiles install DESTDIR="$BASEDIR"/cfengine/dist
 fi

build-scripts/compile-options

@@ -5,8 +5,8 @@
 
 # Autodect PROJECT if not set
 
-if [ x"$PROJECT" = x ]; then
-    case x"$JOB_NAME" in
+if [ -z "$PROJECT" ]; then
+    case "$JOB_NAME" in
     *-community-*) PROJECT=community ;;
     *-enterprise-*) PROJECT=nova ;;
     *-hub-*) PROJECT=nova ;;
@@ -17,8 +17,8 @@ fi
 # If still not set, then either we are running outside Jenkins, or this
 # is not a main "build" type job (it could be the bootstrap job).
 # Do directory-based auto-detection.
-if [ x"$PROJECT" = x ]; then
-    if [ -d $BASEDIR/nova ]; then
+if [ -z "$PROJECT" ]; then
+    if [ -d "$BASEDIR"/nova ]; then
         PROJECT=nova
     else
         PROJECT=community
@@ -31,7 +31,7 @@ export PROJECT
 # When running manually, you can just export this variable.
 # It's a flag: if it's set to 1 - then we use system OpenSSL.
 # Otherwise, we build it.
-if [ x"$SYSTEM_SSL" = x ]; then
+if [ -z "$SYSTEM_SSL" ]; then
     # We don't bundle OpenSSL on some redhat-derived systems due to incompatability with libpam and our openssl.
     _OS_MAJOR_VERSION="$(echo "$OS_VERSION" | cut -d. -f1)"
     if [ "$OS" = "rhel" ] && expr "$_OS_MAJOR_VERSION" ">=" "8" >/dev/null; then
@@ -43,6 +43,9 @@ if [ x"$SYSTEM_SSL" = x ]; then
         fi
     fi
     # Detect using system ssl when running a Jenkins job
+    # shellcheck disable=SC2154
+    # > label is referenced but not assigned.
+    # This file is sourced by other scripts. label is assigned elsewhere.
     if expr x"$label" ":" ".*systemssl" >/dev/null; then
         SYSTEM_SSL=1
     fi
@@ -82,7 +85,7 @@ solaris)
 esac
 
 # When we don't bundle OpenSSL, then we need to pull it from /usr/lib64.
-if [ x"$SYSTEM_SSL" = x1 ]; then
+if [ "$SYSTEM_SSL" = 1 ]; then
     LDFLAGS="$LDFLAGS -L/usr/lib64"
 fi
 export LDFLAGS
@@ -96,8 +99,11 @@ EMBEDDED_DB="lmdb"
 
 ############### Fill in build dependencies in DEPS variable ################
 
+# shellcheck disable=SC2034
+# > DEPS appears unused. Verify use (or export if used externally).
+# This file is sourced by other scripts that uses it
 DEPS=
-[ $OS_FAMILY = mingw ] && var_append DEPS "pthreads-w32 libgnurx"
+[ "$OS_FAMILY" = mingw ] && var_append DEPS "pthreads-w32 libgnurx"
 
 # libgcc_s.so is needed before we compile any other dependency
 #             on some platforms!
@@ -107,7 +113,7 @@ esac
 
 var_append DEPS "$EMBEDDED_DB pcre2"
 
-if ! [ x"$SYSTEM_SSL" = x1 ]; then
+if [ "$SYSTEM_SSL" != 1 ]; then
     # FIXME: Why do we need zlib?
     # ANSWER: Openssl uses it optionally, TODO DISABLE
     var_append DEPS "zlib openssl"
@@ -156,7 +162,7 @@ agent) ROLE=agent ;;
 hub) ROLE=hub ;;
 *)
     # Not running under Jenkins?
-    if [ x"$JENKINS_SERVER_COOKIE" = x ]; then
+    if [ -z "$JENKINS_SERVER_COOKIE" ]; then
         case "$PROJECT-$ARCH-$OS-${OS_VERSION}" in
         community-*) ROLE=agent ;;
         # We do not support 32 bits hubs anymore
@@ -220,7 +226,12 @@ esac
 # unit files for it?
 case "$OS_FAMILY" in
 linux) WITH_SYSTEMD=yes ;;
-*) WITH_SYSTEMD=no ;;
+*)
+    # shellcheck disable=SC2034
+    # > DEPS appears unused. Verify use (or export if used externally).
+    # This file is sourced by other scripts that uses it.
+    WITH_SYSTEMD=no
+    ;;
 esac
 
 case "$OS_FAMILY" in

build-scripts/detect-environment

@@ -17,6 +17,9 @@ fi
 # compilation targets. However, if CROSS_TARGET is already set in the
 # environment it will take precedence.
 detect_cross_target() {
+    # shellcheck disable=SC2154
+    # > label is referenced but not assigned.
+    # This file is sourced by other scripts. label is assigned elsewhere.
     case "$label" in
     *_x86_64_mingw*)
         CROSS_TARGET=${CROSS_TARGET:-x64-mingw}