many: make shell scripts shellcheck-clean #3369

Merged
merged 5 commits into from May 24, 2017
View
@@ -33,6 +33,10 @@ Add `$GOPATH/bin` to your `PATH`, so you can run the go programs you install:
PATH="$PATH:$GOPATH/bin"
+(note `$GOPATH` can actually point to multiple locations, like `$PATH`, so if
+your `$GOPATH` is more complex than a single entry you'll need to adjust the
+above).
+
### Getting the snapd sources
The easiest way to get the source for `snapd` is to use the `go get` command.
View
@@ -11,7 +11,7 @@ subdirs = snap-confine snap-discard-ns system-shutdown libsnap-confine-private
# Run check-syntax when checking
# TODO: conver those to autotools-style tests later
-check: check-syntax-c check-syntax-sh check-unit-tests
+check: check-syntax-c check-unit-tests
# Force particular coding style on all source and header files.
.PHONY: check-syntax-c
@@ -25,15 +25,6 @@ check-syntax-c:
diff -Naur "$$f" "$$out" || exit 1; \
done;
-# Check for common shell errors
-.PHONY: check-syntax-sh
-check-syntax-sh: $(filter-out %.log %.trs,$(wildcard $(srcdir)/snap-confine/tests/test_*)) snap-confine/tests/common.sh
-if HAVE_SHELLCHECK
- $(HAVE_SHELLCHECK) -x --format=gcc $^
-else
- echo "shellcheck is not installed, skipping (non-critical) syntax test"
-endif
-
.PHONY: check-unit-tests
if WITH_UNIT_TESTS
check-unit-tests: snap-confine/unit-tests system-shutdown/unit-tests libsnap-confine-private/unit-tests
View
@@ -16,6 +16,7 @@ autoreconf -i -f
# Configure the build
extra_opts=
+# shellcheck disable=SC1091
. /etc/os-release
case "$ID" in
arch)
@@ -47,4 +48,5 @@ case "$ID" in
esac
echo "Configuring with: $extra_opts"
+# shellcheck disable=SC2086
@zyga

zyga May 24, 2017

Contributor

Wow, thank you, I never knew how to disable those :)

./configure --enable-maintainer-mode --prefix=/usr $extra_opts
View
@@ -178,10 +178,6 @@ AC_PATH_PROGS([HAVE_RST2MAN],[rst2man rst2man.py])
AS_IF([test "x$HAVE_RST2MAN" = "x"], [AC_MSG_WARN(["cannot find the rst2man tool, install python-docutils or similar"])])
AM_CONDITIONAL([HAVE_RST2MAN], [test "x${HAVE_RST2MAN}" != "x"])
-AC_PATH_PROG([HAVE_SHELLCHECK],[shellcheck])
-AM_CONDITIONAL([HAVE_SHELLCHECK], [test "x${HAVE_SHELLCHECK}" != "x"])
-AS_IF([test "x$HAVE_SHELLCHECK" = "x"], [AC_MSG_WARN(["cannot find the shellcheck tool, will not run syntax checks for shell code"])])
-
AC_PATH_PROG([HAVE_VALGRIND],[valgrind])
AM_CONDITIONAL([HAVE_VALGRIND], [test "x${HAVE_VALGRIND}" != "x"])
AS_IF([test "x$HAVE_VALGRIND" = "x"], [AC_MSG_WARN(["cannot find the valgrind tool, will not run unit tests through valgrind"])])
@@ -15,7 +15,7 @@ MAJMIN="$4"
[ -n "$DEVPATH" ] || { echo "no devpath given" >&2; exit 1; }
[ -n "$MAJMIN" ] || { echo "no major/minor given" >&2; exit 0; }
-APPNAME=`echo $APPNAME | tr '_' '.'`
@mvo5

mvo5 May 23, 2017

Collaborator

❤️

@zyga

zyga May 24, 2017

Contributor

Oh, this script slows down execution of snap-confine. We should re-factor this a little!

+APPNAME="$( echo "$APPNAME" | tr '_' '.' )"
app_dev_cgroup="/sys/fs/cgroup/devices/$APPNAME"
# check if it's a block or char dev
@@ -1,4 +1,4 @@
-#!/bin/sh
+#!/bin/bash
# This script creates a new release tarball
set -xue
@@ -1,4 +1,4 @@
-#!/bin/sh
+#!/bin/bash
# This script is started by spread to prepare the execution environment
set -xue
@@ -9,8 +9,11 @@ test -f configure.ac || ( echo 'this script must be executed from the top-level
top_dir=$(pwd)
# Record the current distribution release data to know what to do
-release_ID="$( . /etc/os-release && echo "${ID:-linux}" )"
-release_VERSION_ID="$( . /etc/os-release && echo "${VERSION_ID:-}" )"
+# shellcheck disable=SC1091
+{
@zyga

zyga May 24, 2017

Contributor

Curious, why do you use { ... } here?

@chipaca

chipaca May 24, 2017

Member

so the #shellcheck disable applies to the whole block

+ release_ID="$( . /etc/os-release && echo "${ID:-linux}" )"
+ release_VERSION_ID="$( . /etc/os-release && echo "${VERSION_ID:-}" )"
+}
build_debian_or_ubuntu_package() {
@@ -30,8 +33,11 @@ build_debian_or_ubuntu_package() {
fi
# source the distro specific vars
- . "$top_dir/spread-tests/distros/$release_ID.$release_VERSION_ID"
- . "$top_dir/spread-tests/distros/$release_ID.common"
+ # shellcheck disable=SC1090
+ {
+ . "$top_dir/spread-tests/distros/$release_ID.$release_VERSION_ID"
+ . "$top_dir/spread-tests/distros/$release_ID.common"
+ }
# sanity check, ensure that essential variables were defined
test -n "$distro_packaging_git_branch"
@@ -51,7 +57,7 @@ build_debian_or_ubuntu_package() {
# Install all the build dependencies declared by the package.
apt-get install --quiet -y gdebi-core
- apt-get install --quiet -y $(gdebi --quiet --apt-line ./distro-packaging/debian/control)
+ gdebi --quiet --apt-line ./distro-packaging/debian/control | xargs -r apt-get install --quiet -y
# Generate a new upstream tarball from the current state of the tree
( cd "$top_dir" && spread-tests/release.sh )
@@ -2,7 +2,7 @@
set -e
-# shellcheck source=snap-confine/tests/common.sh
+# shellcheck source=cmd/snap-confine/tests/common.sh
@zyga

zyga May 24, 2017

Contributor

Ohh, so tedious :)

@chipaca

chipaca May 24, 2017

Member

yeah :-/

. "${srcdir:-.}/snap-confine/tests/common.sh"
get_common_syscalls >"$TMP"/tmpl
@@ -2,7 +2,7 @@
set -e
-# shellcheck source=snap-confine/tests/common.sh
+# shellcheck source=cmd/snap-confine/tests/common.sh
. "${srcdir:-.}/snap-confine/tests/common.sh"
get_common_syscalls >"$TMP"/tmpl
@@ -2,7 +2,7 @@
set -e
-# shellcheck source=snap-confine/tests/common.sh
+# shellcheck source=cmd/snap-confine/tests/common.sh
. "${srcdir:-.}/snap-confine/tests/common.sh"
get_common_syscalls >"$TMP"/tmpl
@@ -2,7 +2,7 @@
set -e
-# shellcheck source=snap-confine/tests/common.sh
+# shellcheck source=cmd/snap-confine/tests/common.sh
. "${srcdir:-.}/snap-confine/tests/common.sh"
get_common_syscalls >"$TMP"/tmpl
@@ -2,7 +2,7 @@
set -e
-# shellcheck source=snap-confine/tests/common.sh
+# shellcheck source=cmd/snap-confine/tests/common.sh
. "${srcdir:-.}/snap-confine/tests/common.sh"
get_common_syscalls >"$TMP"/tmpl
@@ -2,7 +2,7 @@
set -e
-# shellcheck source=snap-confine/tests/common.sh
+# shellcheck source=cmd/snap-confine/tests/common.sh
. "${srcdir:-.}/snap-confine/tests/common.sh"
get_common_syscalls >"$TMP"/tmpl
@@ -2,7 +2,7 @@
set -e
-# shellcheck source=snap-confine/tests/common.sh
+# shellcheck source=cmd/snap-confine/tests/common.sh
. "${srcdir:-.}/snap-confine/tests/common.sh"
get_common_syscalls >"$TMP"/tmpl
@@ -2,7 +2,7 @@
set -e
-# shellcheck source=snap-confine/tests/common.sh
+# shellcheck source=cmd/snap-confine/tests/common.sh
. "${srcdir:-.}/snap-confine/tests/common.sh"
get_common_syscalls >"$TMP"/tmpl
@@ -2,7 +2,7 @@
set -e
-# shellcheck source=snap-confine/tests/common.sh
+# shellcheck source=cmd/snap-confine/tests/common.sh
. "${srcdir:-.}/snap-confine/tests/common.sh"
get_common_syscalls >"$TMP"/tmpl
@@ -2,7 +2,7 @@
set -e
-# shellcheck source=snap-confine/tests/common.sh
+# shellcheck source=cmd/snap-confine/tests/common.sh
. "${srcdir:-.}/snap-confine/tests/common.sh"
printf "Test seccomp filter (bad - too long)"
@@ -2,7 +2,7 @@
set -e
-# shellcheck source=snap-confine/tests/common.sh
+# shellcheck source=cmd/snap-confine/tests/common.sh
. "${srcdir:-.}/snap-confine/tests/common.sh"
printf "Test seccomp filter (bad - no trailing newline)"
@@ -2,7 +2,7 @@
set -e
-# shellcheck source=snap-confine/tests/common.sh
+# shellcheck source=cmd/snap-confine/tests/common.sh
. "${srcdir:-.}/snap-confine/tests/common.sh"
cat >"$TMP/snap.name.app" <<EOF
@@ -2,7 +2,7 @@
set -e
-# shellcheck source=snap-confine/tests/common.sh
+# shellcheck source=cmd/snap-confine/tests/common.sh
. "${srcdir:-.}/snap-confine/tests/common.sh"
cat >"$TMP/snap.name.app" <<EOF
@@ -2,7 +2,7 @@
set -e
-# shellcheck source=snap-confine/tests/common.sh
+# shellcheck source=cmd/snap-confine/tests/common.sh
. "${srcdir:-.}/snap-confine/tests/common.sh"
printf "Test that a non-existing profile causes the launcher to not start"
@@ -2,7 +2,7 @@
set -e
-# shellcheck source=snap-confine/tests/common.sh
+# shellcheck source=cmd/snap-confine/tests/common.sh
. "${srcdir:-.}/snap-confine/tests/common.sh"
cat >"$TMP/snap.name.app" <<EOF
@@ -2,7 +2,7 @@
set -e
-# shellcheck source=snap-confine/tests/common.sh
+# shellcheck source=cmd/snap-confine/tests/common.sh
. "${srcdir:-.}/snap-confine/tests/common.sh"
get_common_syscalls >"$TMP/snap.name.app"
@@ -2,7 +2,7 @@
set -e
-# shellcheck source=snap-confine/tests/common.sh
+# shellcheck source=cmd/snap-confine/tests/common.sh
. "${srcdir:-.}/snap-confine/tests/common.sh"
get_common_syscalls >"$TMP"/tmpl
@@ -2,7 +2,7 @@
set -e
-# shellcheck source=snap-confine/tests/common.sh
+# shellcheck source=cmd/snap-confine/tests/common.sh
. "${srcdir:-.}/snap-confine/tests/common.sh"
get_common_syscalls >"$TMP"/tmpl
@@ -2,7 +2,7 @@
set -e
-# shellcheck source=snap-confine/tests/common.sh
+# shellcheck source=cmd/snap-confine/tests/common.sh
. "${srcdir:-.}/snap-confine/tests/common.sh"
get_common_syscalls >"$TMP"/tmpl
@@ -2,7 +2,7 @@
set -e
-# shellcheck source=snap-confine/tests/common.sh
+# shellcheck source=cmd/snap-confine/tests/common.sh
. "${srcdir:-.}/snap-confine/tests/common.sh"
get_common_syscalls >"$TMP"/tmpl
@@ -2,7 +2,7 @@
set -e
-# shellcheck source=snap-confine/tests/common.sh
+# shellcheck source=cmd/snap-confine/tests/common.sh
. "${srcdir:-.}/snap-confine/tests/common.sh"
get_common_syscalls >"$TMP"/tmpl
@@ -2,7 +2,7 @@
set -e
-# shellcheck source=snap-confine/tests/common.sh
+# shellcheck source=cmd/snap-confine/tests/common.sh
. "${srcdir:-.}/snap-confine/tests/common.sh"
get_common_syscalls >"$TMP"/tmpl
@@ -2,7 +2,7 @@
set -e
-# shellcheck source=snap-confine/tests/common.sh
+# shellcheck source=cmd/snap-confine/tests/common.sh
. "${srcdir:-.}/snap-confine/tests/common.sh"
get_common_syscalls >"$TMP"/tmpl
@@ -2,7 +2,7 @@
set -e
-# shellcheck source=snap-confine/tests/common.sh
+# shellcheck source=cmd/snap-confine/tests/common.sh
. "${srcdir:-.}/snap-confine/tests/common.sh"
get_common_syscalls >"$TMP"/tmpl
@@ -2,7 +2,7 @@
set -e
-# shellcheck source=snap-confine/tests/common.sh
+# shellcheck source=cmd/snap-confine/tests/common.sh
. "${srcdir:-.}/snap-confine/tests/common.sh"
cat >"$TMP/snap.name.app" <<EOF
@@ -2,7 +2,7 @@
set -e
-# shellcheck source=snap-confine/tests/common.sh
+# shellcheck source=cmd/snap-confine/tests/common.sh
. "${srcdir:-.}/snap-confine/tests/common.sh"
cat >"$TMP/snap.name.app" <<EOF
@@ -2,7 +2,7 @@
set -e
-# shellcheck source=snap-confine/tests/common.sh
+# shellcheck source=cmd/snap-confine/tests/common.sh
. "${srcdir:-.}/snap-confine/tests/common.sh"
printf "Test appname whitelist"
@@ -40,7 +40,7 @@ _complete_from_snap() {
{
# De-serialize the output of 'snap run --command=complete ...' into the format
# bash expects:
- read -a opts
+ read -r -a opts
# opts is expected to be a series of compopt options
if [[ ${#opts[@]} -gt 0 ]]; then
if [[ "${opts[0]}" == "cannot" ]]; then
@@ -56,7 +56,7 @@ _complete_from_snap() {
done
fi
- read bounced
+ read -r bounced
case "$bounced" in
""|"alias"|"export"|"job"|"variable")
# OK
@@ -67,7 +67,7 @@ _complete_from_snap() {
;;
esac
- read sep
+ read -r sep
if [ -n "$sep" ]; then
# non-blank separator? madness!
return 2
Oops, something went wrong.