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.shdiff --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?