GNU bug report logs - #29197
27.0.50; pre-commit checks for new files against "head"

Previous Next

Full log

Message #11 received at 29197 <at> debbugs.gnu.org (full text, mbox):

From: Noam Postavsky <npostavs <at> gmail.com>
To: Stefan Monnier <monnier <at> iro.umontreal.ca>,  29197 <at> debbugs.gnu.org
Subject: Re: bug#29197: 27.0.50; pre-commit checks for new files against "head"
Date: Wed, 14 Feb 2018 20:03:50 -0500

[Message part 1 (text/plain, inline)]

Noam Postavsky <npostavs <at> 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.

[v1-0001-Let-pre-commit-git-hook-check-merged-in-changes-B.patch (text/x-diff, inline)]

From fa94e7452be5ffb4d63ad2a40fc5b9fdeed3fed0 Mon Sep 17 00:00:00 2001
From: Noam Postavsky <npostavs <at> 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

Previous Next

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.