Thread (1 message) 1 message, 1 author, 2016-06-15

Re: [PATCH v2 2/3] push: Add support for pre-push hooks

From: Junio C Hamano <hidden>
Date: 2016-06-15 22:55:45

Aaron Schrab [off-list ref] writes:
quoted hunk
Add support for a pre-push hook which can be used to determine if the
set of refs to be pushed is suitable for the target repository.  The
hook is run with two arguments specifying the name and location of the
destination repository.

Information about what is to be pushed is provided by sending lines of
the following form to the hook's standard input:

  <local ref> SP <local sha1> SP <remote ref> SP <remote sha1> LF

If the hook exits with a non-zero status, the push will be aborted.

This will allow the script to determine if the push is acceptable based
on the target repository and branch(es), the commits which are to be
pushed, and even the source branches in some cases.

Signed-off-by: Aaron Schrab <redacted>
---
 Documentation/githooks.txt |  29 ++++++++++
 builtin/push.c             |   1 +
 t/t5571-pre-push-hook.sh   | 129 +++++++++++++++++++++++++++++++++++++++++++++
 transport.c                |  60 +++++++++++++++++++++
 transport.h                |   1 +
 5 files changed, 220 insertions(+)
 create mode 100755 t/t5571-pre-push-hook.sh
diff --git a/Documentation/githooks.txt b/Documentation/githooks.txt
index b9003fe..d839233 100644
--- a/Documentation/githooks.txt
+++ b/Documentation/githooks.txt
@@ -176,6 +176,35 @@ save and restore any form of metadata associated with the working tree
 (eg: permissions/ownership, ACLS, etc).  See contrib/hooks/setgitperms.perl
 for an example of how to do this.
 
+pre-push
+~~~~~~~~
+
+This hook is called by 'git push' and can be used to prevent a push from taking
+place.  The hook is called with two parameters which provide the name and
+location of the destination remote, if a named remote is not being used both
+values will be the same.
+
+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
+
+For instance, if the command +git push origin master:foreign+ were run the
Just being curious, but why use +monospace text+ here?  Most of the
new text use `monospace text literally` instead in this patch.
+hook would receive a line like the following:
+
+  refs/heads/master 67890 refs/heads/foreign 12345
+
+although the full, 40-character SHA1s would be supplied.
Perhaps ellipses are called for here?

    refs/heads/master 67890... refs/heads/foreign 12345...

 (the above abbreviates full 40-hexdigits for illustration purposes only)
+If the foreign ref
+does not yet exist the `<remote SHA1>` will be 40 `0`.  If a ref is to be
+deleted, the `<local ref>` will be supplied as `(delete)` and the `<local
+SHA1>` will be 40 `0`.  If the local commit was specified by something other
+than a name which could be expanded (such as `HEAD~`, or a SHA1) it will be
+supplied as it was originally given.
+
+If this hook exits with a non-zero status, 'git push' will abort without
+pushing anything.  Information about why the push is rejected may be sent
+to the user by writing to standard error.
s/standard error/& of the hook/; perhaps?  It is unclear who does
the writing and it can be misunderstood that git-push will write to
standard error upon seeing your hook that silently exits.
quoted hunk
diff --git a/builtin/push.c b/builtin/push.c
index 8491e43..b158028 100644
--- a/builtin/push.c
+++ b/builtin/push.c
@@ -407,6 +407,7 @@ int cmd_push(int argc, const char **argv, const char *prefix)
 		OPT_BOOL(0, "progress", &progress, N_("force progress reporting")),
 		OPT_BIT(0, "prune", &flags, N_("prune locally removed refs"),
 			TRANSPORT_PUSH_PRUNE),
+		OPT_BIT(0, "no-verify", &flags, N_("bypass pre-push hook"), TRANSPORT_PUSH_NO_HOOK),
 		OPT_END()
 	};
So to countermand this, you have to say --no-no-verify?  Wouldn't it
be more natural to introduce a --verify option that turns the bit
on, which automatically gives you --no-verify to turn it off?  A
bit in a flag word can be initialized to true before the flag word
is given to the parse_options() machinery to make the field default
to true, no?
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help