grml · zeha · Nov 11, 2023 · Nov 12, 2023 · Nov 12, 2023
diff --git a/.github/workflows/pr-review.yml b/.github/workflows/pr-review.yml
@@ -0,0 +1,40 @@
+# PR Review workflows.
+# The intention is for these to only find *new* issues.
+
+name: pr-review
+on:
+  pull_request:
+jobs:
+
+  shellcheck-code:
+    name: shellcheck grml-debootstrap
+    runs-on: ubuntu-latest
+    steps:
+      - uses: actions/checkout@v4
+      - name: shellcheck
+        uses: reviewdog/action-shellcheck@v1
+        with:
+          github_token: ${{ secrets.github_token }}
+          reporter: github-pr-review
+          path: "."
+          pattern: |
+            chroot-script
+            grml-debootstrap
+            config
+            tests/shellcheck-stub-debootstrap-variables
+          check_all_files_with_shebangs: "false"
+
+  shellcheck-tests:
+    name: shellcheck test scripts
+    runs-on: ubuntu-latest
+    steps:
+      - uses: actions/checkout@v4
+      - name: shellcheck
+        uses: reviewdog/action-shellcheck@v1
+        with:
+          github_token: ${{ secrets.github_token }}
+          reporter: github-pr-review
+          path: "."
+          pattern: |
+            tests/*
+          check_all_files_with_shebangs: "false"
diff --git a/chroot-script b/chroot-script
@@ -8,6 +8,7 @@
 # GRML_CHROOT_SCRIPT_MARKER - do not remove this line unless you want to keep
 # this script as /bin/chroot-script on your new installed system
 ################################################################################
+# shellcheck disable=SC2317  # shellcheck has trouble understanding the code flow in this file
 
 # error_handler {{{
 if [ "$REPORT_TRAP_ERR" = "yes" ] || [ "$FAIL_TRAP_ERR" = "yes" ]; then
@@ -18,9 +19,9 @@ if [ "$REPORT_TRAP_ERR" = "yes" ] || [ "$FAIL_TRAP_ERR" = "yes" ]; then
 fi
 # }}}
 
-# shellcheck disable=SC1091
+# shellcheck source=config
 . /etc/debootstrap/config    || exit 1
-# shellcheck disable=SC1091
+# shellcheck source=tests/shellcheck-stub-debootstrap-variables
 . /etc/debootstrap/variables || exit 1
 
 [ -r /proc/1 ] || mount -t proc none /proc
@@ -343,8 +344,8 @@ get_kernel_version() {
 
   for KPREFIX in "" "2.6-" ; do  # iterate through the kernel prefixes,
                                  # currently "" and "2.6-"
-    if package_exists linux-image-${KPREFIX}${KARCH} ; then
-      echo ${KPREFIX}${KARCH}
+    if package_exists "linux-image-${KPREFIX}${KARCH}" ; then
+      echo "${KPREFIX}${KARCH}"
       return 0
     fi
 
@@ -405,6 +406,7 @@ passwords()
   fi
 
   if [ -n "$ROOTPASSWORD" ] ; then
+     # shellcheck disable=SC2086
      echo root:"$ROOTPASSWORD" | chpasswd $CHPASSWD_OPTION
      export ROOTPASSWORD=''
   else
@@ -427,6 +429,7 @@ passwords()
          a='1'
          b='2'
        else
+         # shellcheck disable=SC2086
          echo root:"$a" | chpasswd $CHPASSWD_OPTION
          unset a
          unset b
@@ -614,8 +617,10 @@ initrd() {
      echo "Generating initrd."
      if [ "$INITRD_GENERATOR" = 'dracut' ] ; then
          DEBIAN_FRONTEND=$DEBIAN_FRONTEND $APTINSTALL dracut
+         # shellcheck disable=SC2086
          dracut --no-hostonly --kver "$KERNELVER" --fstab --add-fstab /etc/fstab --force --reproducible $INITRD_GENERATOR_OPTS
      else
+         # shellcheck disable=SC2086
          update-initramfs -c -t -k "$KERNELVER" $INITRD_GENERATOR_OPTS
      fi
   fi
@@ -711,9 +716,9 @@ grub_install() {
   echo "Setting ${GRUB_PACKAGE} debconf configuration for install device to $GRUB"
   echo "${GRUB_PACKAGE} ${GRUB_PACKAGE}/install_devices multiselect ${grub_device}" | debconf-set-selections
 
-  if ! dpkg --list ${GRUB_PACKAGE} 2>/dev/null | grep -q '^ii' ; then
+  if ! dpkg --list "${GRUB_PACKAGE}" 2>/dev/null | grep -q '^ii' ; then
     echo "Notice: grub option set but no ${GRUB_PACKAGE} package, installing it therefore."
-    DEBIAN_FRONTEND=$DEBIAN_FRONTEND $APTINSTALL ${GRUB_PACKAGE}
+    DEBIAN_FRONTEND=$DEBIAN_FRONTEND $APTINSTALL "${GRUB_PACKAGE}"
   fi
 
   if ! [ -x "$(command -v grub-install)" ] ; then

diff --git a/config b/config
@@ -4,6 +4,7 @@
 # Bug-Reports:   see https://grml.org/bugs/
 # License:       This file is licensed under the GPL v2 or any later version.
 ################################################################################
+# shellcheck shell=sh
 
 ################################################################################
 # Important: adjust this file if you want to execute grml-debootstrap

diff --git a/grml-debootstrap b/grml-debootstrap
@@ -5,6 +5,7 @@
 # Bug-Reports:   see https://grml.org/bugs/
 # License:       This file is licensed under the GPL v2+
 ################################################################################
+# shellcheck disable=SC2001,SC2181
 
 # error_handler {{{
 [ -n "$REPORT_TRAP_ERR" ] || REPORT_TRAP_ERR='no'
@@ -42,7 +43,7 @@
 # variables {{{
 PN="$(basename "$0")"
 if [[ -d "$(dirname "$(command -v "$0")")"/.git ]]; then
-  VERSION="$(git --git-dir $(dirname "$(command -v "$0")")/.git describe | sed 's|^v||')"
+  VERSION="$(git --git-dir "$(dirname "$(command -v "$0")")"/.git describe | sed 's|^v||')"
 else
   VERSION="$(dpkg-query --show --showformat='${Version}' "$PN")"
 fi
@@ -806,7 +807,7 @@
 prompt_for_release()
 {
  [ -n "$RELEASE" ] && DEFAULT_RELEASE="$RELEASE" || DEFAULT_RELEASE='bullseye'
  RELEASE="$(dialog --stdout --title "${PN}" --default-item $DEFAULT_RELEASE --menu \
            "Please enter the Debian release you would like to use for installation:" \
            0 50 8 \
            buster   Debian/10 \
@@ -861,7 +862,7 @@
 {
  [ -n "$ISO" ] && DEFAULT_MIRROR='local' || DEFAULT_MIRROR='net'

  CHOOSE_MIRROR=$(dialog --stdout --title "$PN" --default-item $DEFAULT_MIRROR \
          --menu "Where do you want to install from?" 0 0 0 \
            net   "install via network (downloading from mirror)" \
            local "install from local directory/mirror"
@@ -872,13 +873,13 @@
     [ -n "$MIRROR" ] || MIRROR='http://deb.debian.org/debian'
     MIRROR="$(dialog --stdout --title "${PN}" --inputbox \
               "Please enter Debian mirror you would like to use for installing packages." \
               0 0 $MIRROR)"
     [ $? -eq 0 ] || bailout
  else # CHOOSE_MIRROR == local
     [ -n "$ISO" ] || ISO='/mnt/mirror'
     ISO="$(dialog --stdout --title "${PN}" --inputbox \
               "Please enter directory name you would like to use for installing packages." \
               0 0 $ISO)"
     [ $? -eq 0 ] || bailout
  fi
 }
@@ -1857,12 +1858,12 @@
  [ -n "$TUNE2FS" ]             && echo "TUNE2FS='$(sed "s,','\\\\'',g" <<<"${TUNE2FS}")'"                         >> "$CHROOT_VARIABLES"
  [ -n "$VMSIZE" ]              && echo "VMSIZE='$(sed "s,','\\\\'',g" <<<"${VMSIZE}")'"                           >> "$CHROOT_VARIABLES"

  cp $VERBOSE "${CONFFILES}"/chroot-script "${MNTPOINT}"/bin/chroot-script
  chmod 755 "${MNTPOINT}"/bin/chroot-script
  [ -d "$MNTPOINT"/etc/debootstrap/ ] || mkdir "$MNTPOINT"/etc/debootstrap/

  # make sure we have our files for later use via chroot-script
  cp $VERBOSE "${CONFFILES}/config"           "${MNTPOINT}"/etc/debootstrap/
  # make sure we adjust the configuration variables accordingly:
  sed -i "s#RELEASE=.*#RELEASE=\"$RELEASE\"#" "${MNTPOINT}"/etc/debootstrap/config
  sed -i "s#TARGET=.*#TARGET=\"$TARGET\"#"    "${MNTPOINT}"/etc/debootstrap/config
@@ -1881,24 +1882,24 @@
      PACKAGES_FILE="packages-arm64"
    fi

    cp $VERBOSE "${_opt_packages:-$CONFFILES/$PACKAGES_FILE}" \
      "${MNTPOINT}/etc/debootstrap/${PACKAGES_FILE}"
  fi

  # debconf preseeding:
  _opt_debconf=${_opt_debconf:-$CONFFILES/debconf-selections}
  [ -f "${_opt_debconf}" ] && [ "$DEBCONF" = 'yes' ] && \
    cp $VERBOSE "${_opt_debconf}" "${MNTPOINT}"/etc/debootstrap/debconf-selections

  # copy scripts that should be executed inside the chroot:
  _opt_chroot_scripts=${_opt_chroot_scripts:-$CONFFILES/chroot-scripts/}
  [ -d "$_opt_chroot_scripts" ] && [ "$CHROOT_SCRIPTS" = 'yes' ] && {
    mkdir -p "${MNTPOINT}"/etc/debootstrap/chroot-scripts
    cp -a $VERBOSE "${_opt_chroot_scripts}"/* "${MNTPOINT}"/etc/debootstrap/chroot-scripts/
  }

  # notice: do NOT use $CHROOT_VARIABLES inside chroot but statically file instead!
  cp $VERBOSE "${CHROOT_VARIABLES}" "${MNTPOINT}"/etc/debootstrap/variables

  cp $VERBOSE -a -L "${CONFFILES}"/extrapackages/ "${MNTPOINT}"/etc/debootstrap/

@@ -1966,11 +1967,13 @@
     einfo "Installing default /etc/network/interfaces as requested via --defaultinterfaces options."
     mkdir -p "${MNTPOINT}/etc/network"
     echo "$DEFAULT_INTERFACES" > "${MNTPOINT}/etc/network/interfaces"
+    # shellcheck disable=SC2320
     eend $?
   elif [ -n "$VIRTUAL" ] ; then
     einfo "Setting up Virtual Machine, installing default /etc/network/interfaces"
     mkdir -p "${MNTPOINT}/etc/network"
     echo "$DEFAULT_INTERFACES" > "${MNTPOINT}/etc/network/interfaces"
+    # shellcheck disable=SC2320
     eend $?
   elif [ -r /etc/network/interfaces ] ; then
     einfo "Copying /etc/network/interfaces from host to target system"
@@ -1981,6 +1984,7 @@
     ewarn "Couldn't read /etc/network/interfaces, installing default /etc/network/interfaces"
     mkdir -p "${MNTPOINT}/etc/network"
     echo "$DEFAULT_INTERFACES" > "${MNTPOINT}/etc/network/interfaces"
+    # shellcheck disable=SC2320
     eend $?
   fi
 

diff --git a/tests/shellcheck-stub-debootstrap-variables b/tests/shellcheck-stub-debootstrap-variables
@@ -0,0 +1,7 @@
+# Stub data file for shellcheck runs.
+# shellcheck shell=sh
+# shellcheck disable=SC2034  # The point of our whole existence is to conflict with SC2034.
+
+# ARCH is defaulted in grml-debootstrap, so it is never empty.
+ARCH=amd64
+