[PATCH for comments] configure: install a whitespace checking pre-commit hook for developers
Ralph Böhme
slow at samba.org
Wed Apr 11 09:59:45 UTC 2018
On Wed, Apr 11, 2018 at 10:04:32AM +0200, Stefan Metzmacher wrote:
> Am 11.04.2018 um 08:24 schrieb Ralph Böhme via samba-technical:
> > On Wed, Apr 11, 2018 at 07:48:43AM +0200, Ralph Böhme wrote:
> >> This has several acks and no nack, so this version could be pushed if it passes
> >> a final review. Thanks!
> >
> > just noticed that this version will fail if you git rebase to a version of the
> > tree that doesn't have the patchset but the git-hook is in place.
> >
> > Attached patchset has an additional check in the hook:
> >
> > +if [ ! -f ${gitdir}/script/git-hooks/pre-commit-script ] ; then
> > + exit 0
> > +fi
> > +
>
> Why is unset GIT_LITERAL_PATHSPECS not needed in 'pre-commit-script',
??? I guess you meant "why is it needed in 'pre-commit-script'" ? It's needed
there because we want to remove it from the environment that later gets passed
to any other shell script.
> but not in 'pre-commit-hook'?
Because git rev-parse doesn't suffer from the issue described in the issue
tracker.
Being in the script and not in the installed hook, we can later revisit this
workaroung for the magit issue.
> I'd also prefer to have 'set -e' and 'set -u' in (at least)
> pre-commit-hook, before it gets install widely.
Added to both hook and script.
Updated patch attached, please push if happy. Thanks!
-slow
--
Ralph Boehme, Samba Team https://samba.org/
Samba Developer, SerNet GmbH https://sernet.de/en/samba/
GPG Key Fingerprint: FAE2 C608 8A24 2520 51C5
59E4 AA1E 9B71 2639 9E46
-------------- next part --------------
From 2f25f9ca51eaac9e98907bc6c49624ea2dd85d9d Mon Sep 17 00:00:00 2001
From: Ralph Boehme <slow at samba.org>
Date: Tue, 10 Apr 2018 13:04:27 +0200
Subject: [PATCH 1/2] Add a wrapper script as git pre-commit hook
When developer mode is enabled, the wrapper script
"script/git-hooks/pre-commit-hook" gets installed as
.git/hooks/pre-commit
and calls "script/git-hooks/pre-commit-script".
This way we can later modify the "script/git-hooks/pre-commit-script"
without the need to ever change the installed commit hook itself.
Signed-off-by: Ralph Boehme <slow at samba.org>
Reviewed-by: Martin Schwenke <martin at meltin.net>
Reviewed-by: Jeremy Allison <jra at samba.org>
---
script/git-hooks/pre-commit-hook | 17 +++++++++++++++++
script/git-hooks/pre-commit-script | 17 +++++++++++++++++
wscript | 9 ++++++++-
3 files changed, 42 insertions(+), 1 deletion(-)
create mode 100755 script/git-hooks/pre-commit-hook
create mode 100755 script/git-hooks/pre-commit-script
diff --git a/script/git-hooks/pre-commit-hook b/script/git-hooks/pre-commit-hook
new file mode 100755
index 00000000000..3f51254c2cf
--- /dev/null
+++ b/script/git-hooks/pre-commit-hook
@@ -0,0 +1,17 @@
+#!/bin/sh
+
+set -eu
+
+gitdir=$(git rev-parse --show-toplevel)
+if [ $? -ne 0 ] ; then
+ echo "git rev-parse --show-toplevel failed"
+ exit 1
+fi
+
+if [ ! -f ${gitdir}/script/git-hooks/pre-commit-script ] ; then
+ exit 0
+fi
+
+${gitdir}/script/git-hooks/pre-commit-script || exit $?
+
+exit 0
diff --git a/script/git-hooks/pre-commit-script b/script/git-hooks/pre-commit-script
new file mode 100755
index 00000000000..22ebecec970
--- /dev/null
+++ b/script/git-hooks/pre-commit-script
@@ -0,0 +1,17 @@
+#!/bin/sh
+
+set -eu
+
+#
+# make emacs/magit work, cf
+# https://github.com/magit/magit/issues/3419
+#
+unset GIT_LITERAL_PATHSPECS
+
+gitdir=$(git rev-parse --show-toplevel)
+if [ $? -ne 0 ] ; then
+ echo "git rev-parse --show-toplevel failed"
+ exit 1
+fi
+
+exit 0
diff --git a/wscript b/wscript
index 0985aa94867..9c42f9a6ef6 100644
--- a/wscript
+++ b/wscript
@@ -10,7 +10,7 @@ import sys, os, tempfile
sys.path.insert(0, srcdir+"/buildtools/wafsamba")
import wafsamba, Options, samba_dist, samba_git, Scripting, Utils, samba_version
import Logs, samba_utils
-
+import shutil
samba_dist.DIST_DIRS('.')
samba_dist.DIST_BLACKLIST('.gitignore .bzrignore source4/selftest/provisions')
@@ -105,6 +105,13 @@ default_prefix = Options.default_prefix = '/usr/local/samba'
if Options.options.developer:
conf.ADD_CFLAGS('-DDEVELOPER -DDEBUG_PASSWORD')
conf.env.DEVELOPER = True
+ # if we are in a git tree without a pre-commit hook, install a
+ # simple default.
+ pre_commit_hook = os.path.join(srcdir, '.git/hooks/pre-commit')
+ if (os.path.isdir(os.path.dirname(pre_commit_hook)) and
+ not os.path.exists(pre_commit_hook)):
+ shutil.copy(os.path.join(srcdir, 'script/git-hooks/pre-commit-hook'),
+ pre_commit_hook)
conf.ADD_EXTRA_INCLUDES('#include/public #source4 #lib #source4/lib #source4/include #include #lib/replace')
--
2.13.6
From 033ee91de639cdaba58920e83b415f389a522835 Mon Sep 17 00:00:00 2001
From: Ralph Boehme <slow at samba.org>
Date: Tue, 10 Apr 2018 13:19:09 +0200
Subject: [PATCH 2/2] script/git-hooks: add check-trailing-whitespace
Signed-off-by: Ralph Boehme <slow at samba.org>
Reviewed-by: Martin Schwenke <martin at meltin.net>
Reviewed-by: Jeremy Allison <jra at samba.org>
---
script/git-hooks/check-trailing-whitespace | 17 +++++++++++++++++
script/git-hooks/pre-commit-script | 2 ++
2 files changed, 19 insertions(+)
create mode 100755 script/git-hooks/check-trailing-whitespace
diff --git a/script/git-hooks/check-trailing-whitespace b/script/git-hooks/check-trailing-whitespace
new file mode 100755
index 00000000000..5303f1fcefa
--- /dev/null
+++ b/script/git-hooks/check-trailing-whitespace
@@ -0,0 +1,17 @@
+#!/bin/sh
+
+git diff-index --cached --check HEAD -- :/*.[ch] :/*.p[ylm]
+
+if [ $? != 0 ]; then
+ echo
+ echo "The commit failed because it seems to introduce trailing whitespace"
+ echo "into C, Perl, or Python code."
+ echo
+ echo "If you are sure you want to do this, repeat the commit with the "
+ echo "--no-verify, like this:"
+ echo
+ echo " git commit --no-verify"
+ exit 1
+fi
+
+exit 0
diff --git a/script/git-hooks/pre-commit-script b/script/git-hooks/pre-commit-script
index 22ebecec970..015a553fb45 100755
--- a/script/git-hooks/pre-commit-script
+++ b/script/git-hooks/pre-commit-script
@@ -14,4 +14,6 @@ if [ $? -ne 0 ] ; then
exit 1
fi
+${gitdir}/script/git-hooks/check-trailing-whitespace || exit $?
+
exit 0
--
2.13.6
More information about the samba-technical
mailing list