From d14573a62371fd6d21ed801bc89e2f3ebdb98b2b Mon Sep 17 00:00:00 2001 From: teor Date: Fri, 30 Aug 2019 22:25:14 +1000 Subject: [PATCH 1/4] Makefile: Explicitly list the subdirectories that contain scripts Part of 30967. --- Makefile.am | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/Makefile.am b/Makefile.am index 5cc4305cb6..25c6562c10 100644 --- a/Makefile.am +++ b/Makefile.am @@ -239,9 +239,10 @@ test: all $(top_builddir)/src/test/test shellcheck: - # Only use shellcheck if it is present + # Only use shellcheck if shellcheck is installed + # Check the directories that contain scripts that we can fix if command -v shellcheck; then \ - find "$(top_srcdir)" -name "*.sh" -not -path "$(top_srcdir)/src/ext/*" -not -path "$(top_srcdir)/src/rust/registry/*" -exec shellcheck {} +; \ + find "$(top_srcdir)" -name "*.sh" -path "$(top_srcdir)/contrib/*" -path "$(top_srcdir)/doc/*" -path "$(top_srcdir)/scripts/*" -path "$(top_srcdir)/src/*" -not -path "$(top_srcdir)/src/ext/*" -not -path "$(top_srcdir)/src/rust/registry/*" -exec shellcheck {} +; \ if [ -d "$(top_srcdir)/scripts/test" ]; then \ shellcheck $(top_srcdir)/scripts/test/cov-diff $(top_srcdir)/scripts/test/coverage; \ fi; \ From 5c2941e69f3e444664d6426f42cb52cb885800ee Mon Sep 17 00:00:00 2001 From: teor Date: Fri, 30 Aug 2019 22:54:05 +1000 Subject: [PATCH 2/4] shellcheck: Add shellcheck to the pre-commit hook * Move the shellcheck script from the Makefile to its own script file * Reformat the shellcheck script so it's easier to read and modify * Call the shellcheck script from the pre-commit hook Fixes bug 30967; not in any released version of Tor. --- Makefile.am | 18 +-------- scripts/git/pre-commit.git-hook | 4 ++ scripts/maint/checkShellScripts.sh | 64 ++++++++++++++++++++++++++++++ 3 files changed, 69 insertions(+), 17 deletions(-) create mode 100755 scripts/maint/checkShellScripts.sh diff --git a/Makefile.am b/Makefile.am index 25c6562c10..4022995fb9 100644 --- a/Makefile.am +++ b/Makefile.am @@ -239,23 +239,7 @@ test: all $(top_builddir)/src/test/test shellcheck: - # Only use shellcheck if shellcheck is installed - # Check the directories that contain scripts that we can fix - if command -v shellcheck; then \ - find "$(top_srcdir)" -name "*.sh" -path "$(top_srcdir)/contrib/*" -path "$(top_srcdir)/doc/*" -path "$(top_srcdir)/scripts/*" -path "$(top_srcdir)/src/*" -not -path "$(top_srcdir)/src/ext/*" -not -path "$(top_srcdir)/src/rust/registry/*" -exec shellcheck {} +; \ - if [ -d "$(top_srcdir)/scripts/test" ]; then \ - shellcheck $(top_srcdir)/scripts/test/cov-diff $(top_srcdir)/scripts/test/coverage; \ - fi; \ - if [ -e "$(top_srcdir)/contrib/dirauth-tools/nagios-check-tor-authority-cert" ]; then \ - shellcheck "$(top_srcdir)/contrib/dirauth-tools/nagios-check-tor-authority-cert"; \ - fi; \ - if [ -e "$(top_srcdir)/contrib/client-tools/torify" ]; then \ - shellcheck "$(top_srcdir)/contrib/client-tools/torify"; \ - fi; \ - if [ -d "$(top_srcdir)/scripts/git" ]; then \ - shellcheck $(top_srcdir)/scripts/git/*.git-hook; \ - fi; \ - fi + $(top_srcdir)/scripts/maint/checkShellScripts.sh check-local: check-spaces check-changes check-includes check-best-practices shellcheck diff --git a/scripts/git/pre-commit.git-hook b/scripts/git/pre-commit.git-hook index b2a1847a2b..1c381ec60a 100755 --- a/scripts/git/pre-commit.git-hook +++ b/scripts/git/pre-commit.git-hook @@ -53,3 +53,7 @@ if [ -e "${PT_DIR}/practracker.py" ]; then fi fi fi + +if [ -e scripts/maint/checkShellScripts.sh ]; then + scripts/maint/checkShellScripts.sh +fi diff --git a/scripts/maint/checkShellScripts.sh b/scripts/maint/checkShellScripts.sh new file mode 100755 index 0000000000..02d1275c48 --- /dev/null +++ b/scripts/maint/checkShellScripts.sh @@ -0,0 +1,64 @@ +#!/usr/bin/env bash +# +# Copyright (c) 2019 The Tor Project, Inc. +# See LICENSE for license information +# +# checkShellScripts.sh +# -------------------- +# If shellcheck is installed, check all the shell scripts that we can fix. + +set -e + +# Only run this script if shellcheck is installed +# command echoes the path to shellcheck, which is a useful diagnostic log +if ! command -v shellcheck; then + printf "%s: Install shellcheck to check shell scripts.\\n" "$0" + exit 0 +fi + +# Some platforms don't have realpath +if command -v realpath ; then + HERE=$(dirname "$(realpath "$0")") +else + HERE=$(dirname "$0") + if [ ! -d "$HERE" ]; then + HERE=$(dirname "$PWD/$0") + fi +fi +TOPLEVEL=$(dirname "$(dirname "$HERE")") + +# Check we actually have a tor/src directory +if [ ! -d "$TOPLEVEL/src" ]; then + printf "Error: Couldn't find src directory in expected location: %s\\n" \ + "$TOPLEVEL/src" +fi + +# Check *.sh scripts, but ignore the ones that we can't fix +find "$TOPLEVEL" \ + -name "*.sh" \ + -path "$TOPLEVEL/contrib/*" \ + -path "$TOPLEVEL/doc/*" \ + -path "$TOPLEVEL/scripts/*" \ + -path "$TOPLEVEL/src/*" \ + -not -path "$TOPLEVEL/src/ext/*" \ + -not -path "$TOPLEVEL/src/rust/registry/*" \ + -exec shellcheck {} + + +# Check scripts that aren't named *.sh +if [ -d "$TOPLEVEL/scripts/test" ]; then + shellcheck \ + "$TOPLEVEL/scripts/test/cov-diff" \ + "$TOPLEVEL/scripts/test/coverage" +fi +if [ -e \ + "$TOPLEVEL/contrib/dirauth-tools/nagios-check-tor-authority-cert" \ + ]; then + shellcheck \ + "$TOPLEVEL/contrib/dirauth-tools/nagios-check-tor-authority-cert" +fi +if [ -e "$TOPLEVEL/contrib/client-tools/torify" ]; then + shellcheck "$TOPLEVEL/contrib/client-tools/torify" +fi +if [ -d "$TOPLEVEL/scripts/git" ]; then + shellcheck "$TOPLEVEL/scripts/git/"*.git-hook +fi From 4b1d2ba9798b0255d9566a642dffd09467e321df Mon Sep 17 00:00:00 2001 From: teor Date: Fri, 30 Aug 2019 23:03:50 +1000 Subject: [PATCH 3/4] changes: file for 30967 --- changes/ticket30967 | 6 ++++++ 1 file changed, 6 insertions(+) create mode 100644 changes/ticket30967 diff --git a/changes/ticket30967 b/changes/ticket30967 new file mode 100644 index 0000000000..5fe9c980b6 --- /dev/null +++ b/changes/ticket30967 @@ -0,0 +1,6 @@ + o Testing: + - When checking shell scripts, ignore any user-created directories. + Closes ticket 30967. + o Minor features (git scripts): + - Call the shellcheck script from the pre-commit hook. + Closes ticket 30967. From 4f762bc41d534012e2eaf31e2357c1149053c9c8 Mon Sep 17 00:00:00 2001 From: teor Date: Fri, 30 Aug 2019 23:52:03 +1000 Subject: [PATCH 4/4] Makefile: include checkShellScripts.sh in EXTRA_DIST Part of 30967. --- Makefile.am | 1 + 1 file changed, 1 insertion(+) diff --git a/Makefile.am b/Makefile.am index 4022995fb9..a7d1cbe8e6 100644 --- a/Makefile.am +++ b/Makefile.am @@ -168,6 +168,7 @@ EXTRA_DIST+= \ ReleaseNotes \ scripts/maint/checkIncludes.py \ scripts/maint/checkSpace.pl \ + scripts/maint/checkShellScripts.sh \ scripts/maint/practracker/README \ scripts/maint/practracker/exceptions.txt \ scripts/maint/practracker/includes.py \