[tor-commits] [tor/master] scripts/git: Add check_cocci_parse.sh to the pre-commit hook

[teor at torproject.org]

commit b9a2286765ff2c6dcd51254669480982962982e8
Author: teor <teor at torproject.org>
Date:   Wed Oct 23 15:43:27 2019 +1000

    scripts/git: Add check_cocci_parse.sh to the pre-commit hook
    
    But only check modified files, which dramatically speeds up
    check_cocci_parse.sh.
    
    Part of 31919.
---
 scripts/git/pre-commit.git-hook | 41 ++++++++++++++++++++++++++++++++
 scripts/git/pre-push.git-hook   | 52 +++++++++++++++++++++++++++--------------
 2 files changed, 76 insertions(+), 17 deletions(-)

diff --git a/scripts/git/pre-commit.git-hook b/scripts/git/pre-commit.git-hook
index 1c381ec60..7437bc085 100755
--- a/scripts/git/pre-commit.git-hook
+++ b/scripts/git/pre-commit.git-hook
@@ -19,6 +19,7 @@ fi
 
 if [ -d src/lib ]; then
     # This is the layout in 0.3.5
+    # There are two copies of this list in this file
     perl scripts/maint/checkSpace.pl -C \
          src/lib/*/*.[ch] \
          src/core/*/*.[ch] \
@@ -57,3 +58,43 @@ fi
 if [ -e scripts/maint/checkShellScripts.sh ]; then
     scripts/maint/checkShellScripts.sh
 fi
+
+if [ -e scripts/coccinelle/check_cocci_parse.sh ]; then
+
+    if [ $# -eq 0 ]; then
+        # When called in pre-commit, check the files modified in this commit
+        CHECK_FILTER="git diff --cached --name-only --diff-filter=ACMR"
+        # Use the owned tor source layout from 0.3.5 to filter the changed
+        # files.
+        # (There are two copies of this list in this file.)
+        CHECK_FILES="$($CHECK_FILTER \
+                          src/lib/*/*.[ch] \
+                          src/core/*/*.[ch] \
+                          src/feature/*/*.[ch] \
+                          src/app/*/*.[ch] \
+                          src/test/*.[ch] \
+                          src/test/*/*.[ch] \
+                          src/tools/*.[ch] \
+                      )"
+    fi
+
+    if [ "${CHECK_FILES:-$*}" ]; then
+        printf "Modified files:\n%s\n" "${CHECK_FILES:-$*}"
+
+        # Run a verbose cocci parse check on the changed files
+        # (spatch is slow, so we don't want to check all the files.)
+        if [ $# -eq 0 ]; then
+            # pre-commit: use $CHECK_FILES, fails on spaces in file names
+            #
+            # We want word splitting here, because file names are space
+            # separated
+            # shellcheck disable=SC2086
+            VERBOSE=1 scripts/coccinelle/check_cocci_parse.sh \
+                      $CHECK_FILES
+        else
+            # pre-push: use "$@" to preserve spaces in arguments
+            VERBOSE=1 scripts/coccinelle/check_cocci_parse.sh \
+                      "$@"
+        fi
+    fi
+fi
diff --git a/scripts/git/pre-push.git-hook b/scripts/git/pre-push.git-hook
index f4504c421..6a44e93dd 100755
--- a/scripts/git/pre-push.git-hook
+++ b/scripts/git/pre-push.git-hook
@@ -16,32 +16,21 @@
 # The following sample script was used as starting point:
 # https://github.com/git/git/blob/master/templates/hooks--pre-push.sample
 
+# Are you adding a new check to the git hooks?
+#  - Common checks belong in the pre-commit hook
+#  - Push-only checks belong in the pre-push hook
+
 echo "Running pre-push hook"
 
 z40=0000000000000000000000000000000000000000
 
 upstream_name=${TOR_UPSTREAM_REMOTE_NAME:-"upstream"}
 
-# Are you adding a new check to the git hooks?
-#  - Common checks belong in the pre-commit hook
-#  - Push-only checks belong in the pre-push hook
-#
-# Call the pre-commit hook for the common checks, if it is executable.
 workdir=$(git rev-parse --show-toplevel)
-if [ -x "$workdir/.git/hooks/pre-commit" ]; then
-  if ! "$workdir"/.git/hooks/pre-commit; then
-    exit 1
-  fi
-fi
 
 remote="$1"
-
 remote_name=$(git remote --verbose | grep "$2" | awk '{print $1}' | head -n 1)
 
-if [[ "$remote_name" != "$upstream_name" ]]; then
-  echo "Not pushing to upstream - refraining from further checks"
-  exit 0
-fi
 
 ref_is_upstream_branch() {
         if [ "$1" == "refs/heads/master" ] ||
@@ -62,13 +51,42 @@ do
 	else
 		if [ "$remote_sha" = $z40 ]
 		then
-			# New branch, examine all commits
-			range="$local_sha"
+			# New branch, examine commits not in master
+			range="master...$local_sha"
 		else
 			# Update to existing branch, examine new commits
 			range="$remote_sha..$local_sha"
 		fi
 
+        # Call the pre-commit hook for the common checks, if it is executable.
+        # Use the owned tor source layout from 0.3.5 to filter the changed
+        # files.
+        if [ -x "$workdir/scripts/git/pre-commit.git-hook" ]; then
+            CHECK_FILES="$(git diff --name-only --diff-filter=ACMR \
+                               "$range" \
+                               src/lib/*/*.[ch] \
+                               src/core/*/*.[ch] \
+                               src/feature/*/*.[ch] \
+                               src/app/*/*.[ch] \
+                               src/test/*.[ch] \
+                               src/test/*/*.[ch] \
+                               src/tools/*.[ch] \
+                          )"
+
+            # We want word splitting here, because file names are space
+            # separated
+            # shellcheck disable=SC2086
+            if ! "$workdir/"scripts/git/pre-commit.git-hook $CHECK_FILES; then
+                exit 1
+            fi
+        fi
+
+        if [[ "$remote_name" != "$upstream_name" ]]; then
+            echo "Not pushing to upstream - refraining from further checks"
+            continue
+        fi
+
+
                 if (ref_is_upstream_branch "$local_ref" == 0 ||
                         ref_is_upstream_branch "$remote_ref"  == 0) &&
                         [ "$local_ref" != "$remote_ref" ]