launchpad-reviewers team mailing list archive

[Colin Watson <mp+425317@xxxxxxxxxxxxxxxxxx>]

Colin Watson has proposed merging ~cjwatson/launchpad:shellcheck into launchpad:master.

Commit message:
Enforce shellcheck in pre-commit

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)

For more details, see:
https://code.launchpad.net/~cjwatson/launchpad/+git/launchpad/+merge/425317

Fix an assorted collection of minor `shellcheck` complaints, and enforce this in `pre-commit` so that they don't come back.
-- 
Your team Launchpad code reviewers is requested to review the proposed merge of ~cjwatson/launchpad:shellcheck into launchpad:master.

diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml
index dbcf289..03f333b 100644
--- a/.pre-commit-config.yaml
+++ b/.pre-commit-config.yaml
@@ -97,3 +97,7 @@ repos:
     -   id: lp-lint-doctest
         args: [--allow-option-flag, IGNORE_EXCEPTION_MODULE_IN_PYTHON2]
         exclude: ^doc/.*
+-   repo: https://github.com/shellcheck-py/shellcheck-py
+    rev: v0.8.0.4
+    hooks:
+    -   id: shellcheck
diff --git a/cronscripts/publishing/cron.base-ppa.sh b/cronscripts/publishing/cron.base-ppa.sh
index f9d8d70..8066618 100644
--- a/cronscripts/publishing/cron.base-ppa.sh
+++ b/cronscripts/publishing/cron.base-ppa.sh
@@ -1,11 +1,13 @@
+# shellcheck shell=bash
 # Copyright 2009 Canonical Ltd.  This software is licensed under the
 # GNU Affero General Public License version 3 (see the file LICENSE).
 
 # Initial setup for PPA cronscripts.
 
 # DO NOT set LPCONFIG here, it should come from the crontab or the shell.
-# Define common variables.
+# Define common variables (also used by cron.daily-ppa).
 PPAROOT=/srv/launchpad.net/ppa-archive
+# shellcheck disable=SC2034
 P3AROOT=/srv/launchpad.net/private-ppa-archive
 LOCKFILE=$PPAROOT/.lock
 # Default lockfile options, retry once if it's locked.
diff --git a/cronscripts/publishing/cron.daily-ppa b/cronscripts/publishing/cron.daily-ppa
index e95937e..5edec8d 100755
--- a/cronscripts/publishing/cron.daily-ppa
+++ b/cronscripts/publishing/cron.daily-ppa
@@ -10,8 +10,9 @@ set -x
 LOCKFILEOPTIONS="-r-1"
 
 # Variables, lockfile and exit handler for PPA scripts.
+# shellcheck source-path=SCRIPTDIR
 source "$(dirname "$0")/cron.base-ppa.sh"
 
 # Clear out empty and thus redundant dirs.
-find "$PPAROOT" -type d -empty | xargs -r rmdir
-find "$P3AROOT" -type d -empty | xargs -r rmdir
+find "$PPAROOT" -type d -empty -exec rmdir {} +
+find "$P3AROOT" -type d -empty -exec rmdir {} +
diff --git a/cronscripts/publishing/cron.publish-copy-archives b/cronscripts/publishing/cron.publish-copy-archives
index 5c35e56..c21826e 100755
--- a/cronscripts/publishing/cron.publish-copy-archives
+++ b/cronscripts/publishing/cron.publish-copy-archives
@@ -21,8 +21,6 @@ set -u
 # Informational -- this *MUST* match the database.
 ARCHIVEROOT=/srv/launchpad.net/rebuild-test/ubuntu
 DISTSROOT=$ARCHIVEROOT/dists
-OVERRIDEROOT=$ARCHIVEROOT/../ubuntu-overrides
-INDICES=$ARCHIVEROOT/indices
 PRODUCTION_CONFIG=ftpmaster-publish
 
 if [ "$LPCONFIG" = "$PRODUCTION_CONFIG" ]; then
diff --git a/cronscripts/publishing/cron.publish-ppa b/cronscripts/publishing/cron.publish-ppa
index 7c93542..fe11a32 100755
--- a/cronscripts/publishing/cron.publish-ppa
+++ b/cronscripts/publishing/cron.publish-ppa
@@ -6,6 +6,7 @@
 set -x
 
 # Variables, lockfile and exit handler for PPA scripts.
+# shellcheck source-path=SCRIPTDIR
 source "$(dirname "$0")/cron.base-ppa.sh"
 
 LPCURRENT="$(dirname "$0")/../.."
diff --git a/utilities/launchpad-database-setup b/utilities/launchpad-database-setup
index 729f941..7c29dd6 100755
--- a/utilities/launchpad-database-setup
+++ b/utilities/launchpad-database-setup
@@ -21,6 +21,7 @@ fi
 # initial Launchpad setup on an otherwise unconfigured postgresql instance
 
 pgversion=
+# shellcheck disable=SC2043
 for try_pgversion in 10
 do
   if sudo grep -qs "^auto" /etc/postgresql/$try_pgversion/main/start.conf; then
diff --git a/utilities/rocketfuel-get b/utilities/rocketfuel-get
index c41416e..678ccb6 100755
--- a/utilities/rocketfuel-get
+++ b/utilities/rocketfuel-get
@@ -11,7 +11,7 @@
 set -eu
 
 # A rough measure of how much stuff we can do in parallel.
-CPU_COUNT="$(egrep -c '^processor\b' /proc/cpuinfo)"
+CPU_COUNT="$(grep -Ec '^processor\b' /proc/cpuinfo)"
 
 # Helper function to run a child process, indenting stdout to aid
 # readability.
@@ -22,6 +22,7 @@ run-child() {
 # Load local settings.
 if [ -e "$HOME/.rocketfuel-env.sh" ]
 then
+    # shellcheck disable=SC1091
     source "$HOME/.rocketfuel-env.sh"
 else
     echo "Please run rocketfuel-setup first." >&2
diff --git a/utilities/rocketfuel-setup b/utilities/rocketfuel-setup
index ab29385..53c236e 100755
--- a/utilities/rocketfuel-setup
+++ b/utilities/rocketfuel-setup
@@ -8,7 +8,7 @@
 # as utilities/rocketfuel-setup
 
 # load up Ubuntu release details so we know which repos to enable
-source /etc/lsb-release
+DISTRIB_CODENAME="$(lsb_release -sc)"
 DO_WORKSPACE=1
 INSTALL_OPTS=""
 getopt_output="$(getopt -o '' -l no-workspace,lpusername:,assume-yes -- "$@")" || exit 1
@@ -35,9 +35,9 @@ done
 
 if [ -z "$lpusername" ]; then
   # Establish LP username
-  whoami=`whoami`
-  printf "What is your Launchpad username? [$whoami] "
-  read lpusername
+  whoami=$(whoami)
+  printf "What is your Launchpad username? [%s] " "$whoami"
+  read -r lpusername
   if [ -z "$lpusername" ]; then
     lpusername=${whoami}
   fi
@@ -61,7 +61,6 @@ if ! grep -q "^127.0.0.88" /etc/hosts; then
   echo "launchpad.test added to /etc/hosts"
 fi
 
-declare -a hostnames
 hostnames=$(cat <<EOF
     answers.launchpad.test
     archive.launchpad.test
@@ -224,6 +223,7 @@ LP_SOURCEDEPS_PATH=\$(eval echo \${LP_SOURCEDEPS_PATH})
 " > "$HOME/.rocketfuel-env.sh"
 fi
 
+# shellcheck disable=SC1091
 if ! source "$HOME/.rocketfuel-env.sh"; then
     echo "Something went wrong with rocketfuel-setup!"
     exit 1