Merge ~cjwatson/launchpad:shellcheck into launchpad:master

[Colin Watson]

Enforce shellcheck in pre-commit

Fix an assorted collection of minor `shellcheck` complaints, and enforce this in `pre-commit` so that they don't come back.

[Jürgen Gmach (jugmac00)]

You have added great insight in the commit messages why we do things differently as one might expect, and especially as the linter expects.

Could you please add the reasoning also to the source code as a comment?

Probably nobody except you and maybe William would know something like "However, this structure is deliberate: we use it when migrating between PostgreSQL versions. Disable the warning."

While `git blame` makes it easy to view the current commit message, after some time and changes it is pretty cumbersome to dig out relevant information.

review: Approve

[Colin Watson (cjwatson)]

I think it was only relevant to a couple of the most recent commits in this branch, but sure - I amended in a couple of extra comments.

[Otto Co-Pilot (otto-copilot)]

Merging failed
https://jenkins.ols.canonical.com/online-services/job/launchpad/2613/

diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml
index a1c7bb0..1297eef 100644
--- a/.pre-commit-config.yaml
+++ b/.pre-commit-config.yaml
@@ -100,3 +100,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..0065021 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  # not used here, but used by cron.daily-ppa
 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..e78931e 100755
--- a/utilities/launchpad-database-setup
+++ b/utilities/launchpad-database-setup
@@ -21,6 +21,9 @@ fi
 # initial Launchpad setup on an otherwise unconfigured postgresql instance
 
 pgversion=
+# This loop contains multiple versions when we are in the process of
+# migrating between PostgreSQL versions.
+# 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