bug#29197: 27.0.50; pre-commit checks for new files against "head"

[Noam Postavsky <npostavs@gmail.com>]

[-- Attachment #1: Type: text/plain, Size: 819 bytes --]

Noam Postavsky <npostavs@users.sourceforge.net> writes:

>> I think the warning should be improved:
>> - I shouldn't get a warning in the above case, since this commit doesn't
>>   *add* those files (they weren't on origin/master admittedly but they were
>>   already on HEAD).
>
> We could choose which side of the merge to check based on an
> environment var (that was considered in the thread I referenced above,
> but we didn't see much of a use case at the time). Or is it possible
> to check only changes from the merge itself (i.e., in case of conflict
> resolution)?
>
>> - the warning should give me some hint about which file fails the test.
>
> Yeah, that would make sense.

This seems fairly difficult to do while keeping in standard bourne shell
syntax.  Not sure how to catch file names with whitespace.


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: patch --]
[-- Type: text/x-diff, Size: 2187 bytes --]

From fa94e7452be5ffb4d63ad2a40fc5b9fdeed3fed0 Mon Sep 17 00:00:00 2001
From: Noam Postavsky <npostavs@gmail.com>
Date: Wed, 14 Feb 2018 19:58:07 -0500
Subject: [PATCH v1] ; Let pre-commit git hook check merged in changes
 (Bug#29197)

* build-aux/git-hooks/pre-commit: If GIT_MERGE_CHECK_OTHER is 'true',
check changes against the merge target, rather than the current
branch.  Include file name when giving error message about
non-standard characters.
---
 build-aux/git-hooks/pre-commit | 20 ++++++++++++--------
 1 file changed, 12 insertions(+), 8 deletions(-)

diff --git a/build-aux/git-hooks/pre-commit b/build-aux/git-hooks/pre-commit
index 5e42dab233..33d8c91168 100755
--- a/build-aux/git-hooks/pre-commit
+++ b/build-aux/git-hooks/pre-commit
@@ -28,7 +28,7 @@ LC_ALL=
 # When doing a two-way merge, ignore problems that came from the other
 # side of the merge.
 head=HEAD
-if test -r "$GIT_DIR"/MERGE_HEAD; then
+if test -r "$GIT_DIR"/MERGE_HEAD && test "$GIT_MERGE_CHECK_OTHER" != true; then
   merge_heads=`cat "$GIT_DIR"/MERGE_HEAD` || exit
   for merge_head in $merge_heads; do
     case $head in
@@ -42,13 +42,6 @@ head=
 fi
 
 git_diff='git diff --cached --name-only --diff-filter=A'
-ok_chars='\0+[=-=]./0-9A-Z_a-z'
-nbadchars=`$git_diff -z $head | tr -d "$ok_chars" | wc -c`
-
-if test "$nbadchars" -ne 0; then
-  echo "File name does not consist of -+./_ or ASCII letters or digits."
-  exit 1
-fi
 
 for new_name in `$git_diff $head`; do
   case $new_name in
@@ -58,9 +51,20 @@ nbadchars=
     ChangeLog | */ChangeLog)
       echo "$new_name: Please use git commit messages, not ChangeLog files."
       exit 1;;
+    *[^-+./_0-9A-Z_a-z]*)
+      echo "$new_name: File name does not consist of -+./_ or ASCII letters or digits."
+      exit 1;;
   esac
 done
 
+ok_chars='\0+[=-=]./0-9A-Z_a-z'
+nbadchars=`$git_diff -z $head | tr -d "$ok_chars" | wc -c`
+
+if test "$nbadchars" -ne 0; then
+  echo "A file name has whitespace."
+  exit 1
+fi
+
 # The '--check' option of git diff-index makes Git complain if changes
 # introduce whitespace errors.  This can be a pain when editing test
 # files that deliberately contain lines with trailing whitespace.
-- 
2.11.0