Re: [PATCH] Add support for a 'pre-push' hook

[Michael Haggerty <mhagger@alum.mit.edu>]

On 11/16/2012 09:30 PM, Junio C Hamano wrote:
> Aske Olsson <askeolsson@gmail.com> writes:
> 
>> If the script .git/hooks/pre-push exists and is executable it will be
>> called before a `git push` command, and when the script exits with a
>> non-zero status the push will be aborted.
>> The hook can be overridden by passing the '--no-verify' option to
>> `git push`.
>>
>> The pre-push hook is usefull to run tests etc. before push. Or to make
>> sure that if a binary solution like git-media, git-annex or git-bin is
>> used the binaries are uploaded before the push, so when others do a
>> fetch the binaries will be available already. This also reduces the
>> need for introducing extra (git) commands to e.g. sync binaries.
>>
>> Signed-off-by: Aske Olsson <askeolsson@gmail.com>
>> ---
>> ...
>> +[[pre-push]]
>> +pre-push
>> +~~~~~~~~
>> +
>> +This hook is invoked by 'git push' and can be bypassed with the
>> +`--no-verify` option. It takes no parameter, and is invoked before
>> +the push happens.
>> +Exiting with a non-zero status from this script causes 'git push'
>> +to abort.
>> ...
>> + if (!no_verify && run_hook(NULL, "pre-push")) {
>> + die(_("pre-push hook failed: exiting\n"));
>> + }
> 
> NAK, at least in the current form.  At least the system should tell
> the hook where it is pushing and what is being pushed.

I agree.

> Besides, there are five valid reasons to add a new hook to the
> system, but your version of pre-push does not satisfy any of them:
> 
>      http://thread.gmane.org/gmane.comp.version-control.git/94111/focus=71069

Here I disagree.  I think it satisfies (1):

>  (1) A hook that countermands the normal decision made by the
>      underlying command.  Examples of this class are the update
>      hook and the pre-commit hook.

pre-push would be very similar in spirit to pre-commit: pre-commit is a
filter that can prevent a "bad" commit from getting into the local
repository; pre-push is similarly a filter between the local repo and
remote repositories.

I also think it satisfies (2) and/or (5b):

>  (2) A hook that operates on data generated after the command
>      starts to run.  [...]

>  (5) [...]  Another example is the post-checkout
>      hook that gets information that is otherwise harder to get
>      (namely, if it was a branch checkout or file checkout --
>      you can figure it out by examining the command line but
>      that already is part of the processing git-checkout does
>      anyway, so no need to force duplicating that code in the
>      userland).

It would not be trivial for a wrapper script to figure out what branches
and commits are about to be pushed.  But "git push" could tell the hook
script what branches are to be pushed.  And if the pre-push hook is run
after negotiation between client and server of what commits need to be
transfered, the hook could also be provided that information and use it
to figure out which commits it should vet.


Although a pre-receive script on the remote repository could do most of
the same things as a pre-push script, the latter would sometimes have
advantages because it is run "client-side":

* When the user doesn't have the ability to change the pre-receive
script on the server (think public git hosting services).

* For user-specific actions that are not wanted by all users pushing to
the same server (for example, maybe I add the string "WIP" to commit
messages for commits that are not ready to be pushed; my pre-push script
could verify that I don't push such a commit by accident).

* Preventing "secret" info (password files, proprietary branches) from
being pushed.  Even if the remote repo were taught to reject them, they
would have already traversed the internet.

Michael

-- 
Michael Haggerty
mhagger@alum.mit.edu
http://softwareswirl.blogspot.com/