[SSSD] [PATCH] Add a client-side hook to prevent pushes without Reviewed-By
Jakub Hrozek
jhrozek at redhat.com
Wed Sep 30 07:22:43 UTC 2015
On Tue, Sep 29, 2015 at 08:28:30AM +0200, Lukas Slebodnik wrote:
> On (28/09/15 14:19), Jakub Hrozek wrote:
> >Hi,
> >
> >to activate this hook, copy it from contrib to .git/hooks and make sure
> >the executable flag is on. Attempting to push a commit without
> >Reviewed-By will then trigger an error.
> >
> Good idea.
>
> >If we want to be truly strict about not pushing commits without a RB
> >tag, then we need a server-side hook.
> As you wish.
>
>
> >From 391666fda49fef9ac003d192e7ae8a3c2b00e113 Mon Sep 17 00:00:00 2001
> >From: Jakub Hrozek <jhrozek at redhat.com>
> >Date: Mon, 28 Sep 2015 13:46:39 +0200
> >Subject: [PATCH] contrib: Add a pre-push hook to warn about commits without
> > Reviewed-By
> >
> >---
> > contrib/git/pre-push | 62 ++++++++++++++++++++++++++++++++++++++++++++++++++++
> > 1 file changed, 62 insertions(+)
> > create mode 100755 contrib/git/pre-push
> >
> >diff --git a/contrib/git/pre-push b/contrib/git/pre-push
> Initially, I thought it does not work because I tested with
> creating a new branch
> git push origin master:new_branch
> However it works after trying to push another patch to this branch.
>
> There are few pep8 warning
> sh$ pep8 contrib/git/pre-push
> contrib/git/pre-push:10:1: E302 expected 2 blank lines, found 1
> contrib/git/pre-push:11:13: E201 whitespace after '['
> contrib/git/pre-push:11:70: E202 whitespace before ']'
> contrib/git/pre-push:14:13: E201 whitespace after '['
> contrib/git/pre-push:14:74: E202 whitespace before ']'
> contrib/git/pre-push:16:1: E302 expected 2 blank lines, found 1
> contrib/git/pre-push:17:13: E201 whitespace after '['
> contrib/git/pre-push:17:54: E202 whitespace before ']'
> contrib/git/pre-push:22:1: E302 expected 2 blank lines, found 1
> contrib/git/pre-push:31:1: E302 expected 2 blank lines, found 1
> contrib/git/pre-push:39:1: E302 expected 2 blank lines, found 1
pep8 should be happy about the attached patch.
>
> It would be also good to add small howto from mail.
> "copy it from contrib to .git/hooks and make sure
> the executable flag is on" to the commit message or better to
> README into directory contrib/git/
OK, Added.
>
> and you can also mention hint about .git-commit-template
I tried to add something, hope it makes sense.
-------------- next part --------------
>From 66df6b096bcb3044431b1d65cb426a1f1fb1f37c Mon Sep 17 00:00:00 2001
From: Jakub Hrozek <jhrozek at redhat.com>
Date: Mon, 28 Sep 2015 13:46:39 +0200
Subject: [PATCH] contrib: Add a pre-push hook to warn about commits without
Reviewed-By
---
contrib/git/pre-push | 73 ++++++++++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 73 insertions(+)
create mode 100755 contrib/git/pre-push
diff --git a/contrib/git/pre-push b/contrib/git/pre-push
new file mode 100755
index 0000000000000000000000000000000000000000..d05202dfdd2f59a885b843e25e89878d985909e4
--- /dev/null
+++ b/contrib/git/pre-push
@@ -0,0 +1,73 @@
+#!/usr/bin/env python
+
+# A git pre-push hook that declines commits that don't contain a Reviewed-By:
+# tag. The tag must be present on the beginning of the line. To activate, copy
+# to $GIT_DIR/hooks/pre-push and make sure the executable flag is on.
+
+# The commit message should also be based on .git-commit-template, although
+# that is just best practice and not enforced
+
+import sys
+import re
+import subprocess
+
+
+def get_all_commits(ref_from, ref_to):
+ args = ['git', 'rev-list', '{:s}..{:s}'.format(ref_from, ref_to)]
+ p = subprocess.Popen(args, stdout=subprocess.PIPE, stderr=subprocess.PIPE)
+ out, err = p.communicate()
+ return [commit.strip() for commit in out.split('\n') if commit != '']
+
+
+def commit_message(commit_hash):
+ args = ['git', 'cat-file', 'commit', commit_hash]
+ p = subprocess.Popen(args, stdout=subprocess.PIPE, stderr=subprocess.PIPE)
+ out, err = p.communicate()
+ return out
+
+
+def commit_has_rb(commit):
+ msg = commit_message(commit)
+ for l in msg.split('\n'):
+ has_rb = re.search('^Reviewed-by:', l)
+ if has_rb:
+ return True
+
+ return False
+
+
+def report_commit(commit_hash):
+ print("Commit {:s} does not have Reviewed-By!".format(commit_hash))
+ print("Full message:\n======")
+ print("{:s}".format(commit_message(commit_hash)))
+ print("======")
+
+
+# man 5 githooks says:
+# Information about what is to be pushed is provided on the hook's
+# standard input with lines of the form:
+# <local ref> SP <local sha1> SP <remote ref> SP <remote sha1> LF
+def check_push(hook_input):
+ ref_to = hook_input.split()[1][:6]
+ ref_from = hook_input.split()[3][:6]
+ commit_list = get_all_commits(ref_from, ref_to)
+
+ no_rb_list = []
+ for commit in commit_list:
+ if not commit_has_rb(commit):
+ no_rb_list.append(commit)
+
+ return no_rb_list
+
+# Don't warn when pushing to personal repositories, only origin
+remote = sys.argv[1]
+if remote != 'origin':
+ sys.exit(0)
+
+for hook_input in sys.stdin.readlines():
+ no_rb_list = check_push(hook_input)
+
+ if len(no_rb_list) > 0:
+ for offender in no_rb_list:
+ report_commit(offender)
+ sys.exit(1)
--
2.4.3
More information about the sssd-devel
mailing list