[PATCH] add a 'pre-push' hook

This commit adds support for a 'pre-push' hook that can be called before
a `git push` command.

It takes no arguments currently, but if the .git/hooks/pre-push script
exists and is executable, it will be called before the 'git push' command
and will stop the push process if it does not exit with a 0 status.

This hook can be overridden by passing in the --no-verify or -n option to
git push.  Documentation and tests have been updated to reflect the change.

Signed-off-by: Scott Chacon <redacted>
---
 Documentation/git-push.txt |   11 +++-
 builtin-push.c             |   27 +++++++++-
 t/t5550-pre-push-hook.sh   |  132 ++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 168 insertions(+), 2 deletions(-)
 create mode 100644 t/t5550-pre-push-hook.sh

diff --git a/Documentation/git-push.txt b/Documentation/git-push.txt
index 45c9643..2b504b3 100644
--- a/Documentation/git-push.txt
+++ b/Documentation/git-push.txt

@@ -10,7 +10,7 @@ SYNOPSIS
 --------
 [verse]
 'git push' [--all] [--dry-run] [--tags] [--receive-pack=<git-receive-pack>]
-          [--repo=all] [-f | --force] [-v | --verbose]
+          [--repo=all] [-f | --force] [-v | --verbose] [-n | --no-verify]
          [<repository> <refspec>...]

 DESCRIPTION

@@ -111,6 +111,10 @@ nor in any Push line of the corresponding remotes

file---see below).
       transfer spends extra cycles to minimize the number of
       objects to be sent and meant to be used on slower connection.

+--no-verify::
+       This option bypasses the pre-push hook.
+       See also linkgit:githooks[5].
+
 -v::
 --verbose::
       Run verbosely.

@@ -193,6 +197,11 @@ git push origin master:refs/heads/experimental::
       needed to create a new branch or tag in the remote repository when
       the local name and the remote name are different; otherwise,
       the ref name on its own will work.
+
+HOOKS
+-----
+This command can run the `pre-push` hook.
+See linkgit:githooks[5] for more information.

 Author
 ------

diff --git a/builtin-push.c b/builtin-push.c
index c1ed68d..f63de9f 100644
--- a/builtin-push.c
+++ b/builtin-push.c

@@ -10,11 +10,12 @@
 #include "parse-options.h"

 static const char * const push_usage[] = {
-       "git push [--all | --mirror] [--dry-run] [--tags]

[--receive-pack=<git-receive-pack>] [--repo=all] [-f | --force] [-v]
[<repository> <refspec>...]",
+       "git push [--all | --mirror] [--dry-run] [--tags]
[--receive-pack=<git-receive-pack>] [--repo=all] [-f | --force] [-n |
--no-verify] [-v] [<repository> <refspec>...]",
       NULL,
 };

 static int thin;
+static int skiphook;
 static const char *receivepack;

 static const char **refspec;

@@ -98,6 +99,24 @@ static int do_push(const char *repo, int flags)
       return !!errs;
 }

+static int run_hook(const char *name)
+{
+       struct child_process hook;
+       const char *argv[1];
+
+       argv[0] = git_path("hooks/%s", name);
+
+       if (access(argv[0], X_OK) < 0)
+               return 0;
+
+       memset(&hook, 0, sizeof(hook));
+       hook.argv = argv;
+       hook.no_stdin = 1;
+       hook.stdout_to_stderr = 1;
+
+       return run_command(&hook);
+}
+
 int cmd_push(int argc, const char **argv, const char *prefix)
 {
       int flags = 0;

@@ -115,6 +134,7 @@ int cmd_push(int argc, const char **argv, const

char *prefix)
               OPT_BIT( 0 , "dry-run", &flags, "dry run",
TRANSPORT_PUSH_DRY_RUN),
               OPT_BIT('f', "force", &flags, "force updates",
TRANSPORT_PUSH_FORCE),
               OPT_BOOLEAN( 0 , "thin", &thin, "use thin pack"),
+               OPT_BOOLEAN('n', "no-verify", &skiphook, "skip pre-push hook"),
               OPT_STRING( 0 , "receive-pack", &receivepack,
"receive-pack", "receive pack program"),
               OPT_STRING( 0 , "exec", &receivepack, "receive-pack",
"receive pack program"),
               OPT_END()

@@ -130,6 +150,11 @@ int cmd_push(int argc, const char **argv, const

char *prefix)
               set_refspecs(argv + 1, argc - 1);
       }

+       if (!skiphook && run_hook("pre-push")) {
+               fprintf(stderr, "pre-push script failed: exiting\n");
+               return 128;
+       }
+
       rc = do_push(repo, flags);
       if (rc == -1)
               usage_with_options(push_usage, options);

diff --git a/t/t5550-pre-push-hook.sh b/t/t5550-pre-push-hook.sh
new file mode 100644
index 0000000..f3c9cce
--- /dev/null
+++ b/t/t5550-pre-push-hook.sh

@@ -0,0 +1,132 @@
+#!/bin/sh
+
+test_description='pre-push hook'
+
+. ./test-lib.sh
+
+D=`pwd`
+
+mk_repo_pair () {
+       rm -rf master mirror &&
+       mkdir mirror &&
+       (
+               cd mirror &&
+               git init
+       ) &&
+       mkdir master &&
+       (
+               cd master &&
+               git init &&
+               git remote add $1 up ../mirror
+       )
+}
+
+test_expect_success 'with no hook' '
+       mk_repo_pair &&
+       (
+               cd master &&
+               echo one >foo && git add foo && git commit -m one &&
+               git push --mirror up
+       )
+'
+
+test_expect_success '--no-verify with no hook' '
+       mk_repo_pair &&
+       (
+               cd master &&
+               echo one >foo && git add foo && git commit -m one &&
+               git push --no-verify --mirror up
+       )
+'
+
+# now install hook that always succeeds
+HOOKDIR="master/.git/hooks"
+HOOK="$HOOKDIR/pre-push"
+mk_hook_exec () {
+       mkdir -p "$HOOKDIR"
+cat > "$HOOK" <<EOF
+#!/bin/sh
+exit 0
+EOF
+       chmod +x "$HOOK"
+}
+
+test_expect_success 'with succeeding hook' '
+       mk_repo_pair &&
+       (
+               mk_hook_exec &&
+               cd master &&
+               echo one >foo && git add foo && git commit -m one &&
+               git push --mirror up
+       )
+'
+
+test_expect_success '--no-verify with succeeding hook' '
+       mk_repo_pair &&
+       (
+               mk_hook_exec &&
+               cd master &&
+               echo one >foo && git add foo && git commit -m one &&
+               git push --no-verify --mirror up
+       )
+'
+
+# now a hook that fails
+mk_hook_fail () {
+cat > "$HOOK" <<EOF
+#!/bin/sh
+echo 'test run'
+exit 1
+EOF
+       chmod +x "$HOOK"
+}
+
+test_expect_success 'with failing hook' '
+       mk_repo_pair &&
+       (
+               mk_hook_fail &&
+               cd master &&
+               echo one >foo && git add foo && git commit -m one &&
+               test_must_fail git push --mirror up
+       )
+'
+
+test_expect_success '--no-verify with failing hook' '
+       mk_repo_pair &&
+       (
+               mk_hook_fail &&
+               cd master &&
+               echo one >foo && git add foo && git commit -m one &&
+               git push --no-verify --mirror up
+       )
+'
+
+chmod -x "$HOOK"
+mk_hook_no_exec () {
+cat > "$HOOK" <<EOF
+#!/bin/sh
+echo 'test run'
+exit 0
+EOF
+}
+
+test_expect_success 'with non-executable hook' '
+       mk_repo_pair &&
+       (
+               mk_hook_no_exec &&
+               cd master &&
+               echo one >foo && git add foo && git commit -m one &&
+               git push --mirror up
+       )
+'
+
+test_expect_success '--no-verify with non-executable hook' '
+       mk_repo_pair &&
+       (
+               mk_hook_no_exec &&
+               cd master &&
+               echo one >foo && git add foo && git commit -m one &&
+               git push --no-verify --mirror up
+       )
+'
+test_done
--

1.6.0.GIT

On Tue, Aug 19, 2008 at 11:55:27AM -0700, Scott Chacon wrote:

This commit adds support for a 'pre-push' hook that can be called before
a `git push` command.

It takes no arguments currently, but if the .git/hooks/pre-push script
exists and is executable, it will be called before the 'git push' command
and will stop the push process if it does not exit with a 0 status.

This hook can be overridden by passing in the --no-verify or -n option to
git push.  Documentation and tests have been updated to reflect the change.

Would you care to describe what this is useful for (either in
documentation, to help potential users, or at least in the commit log,
so we know there is a need that is not otherwise fulfilled)?

-Peff

If the patch is acceptable, I will update the githooks doc with more
information, but we would like this so that you could add a hook that
runs your automated tests before a push would go through.

Scott

On Tue, Aug 19, 2008 at 11:58 AM, Jeff King [off-list ref] wrote:

On Tue, Aug 19, 2008 at 11:55:27AM -0700, Scott Chacon wrote:

quoted

This commit adds support for a 'pre-push' hook that can be called before
a `git push` command.

It takes no arguments currently, but if the .git/hooks/pre-push script
exists and is executable, it will be called before the 'git push' command
and will stop the push process if it does not exit with a 0 status.

This hook can be overridden by passing in the --no-verify or -n option to
git push.  Documentation and tests have been updated to reflect the change.

Would you care to describe what this is useful for (either in
documentation, to help potential users, or at least in the commit log,
so we know there is a need that is not otherwise fulfilled)?

-Peff

On Tue, Aug 19, 2008 at 12:00:38PM -0700, Scott Chacon wrote:

If the patch is acceptable, I will update the githooks doc with more
information, but we would like this so that you could add a hook that
runs your automated tests before a push would go through.

I think the common wisdom has been that such tests should be done on the
_receiving_ end, since that makes a more trustworthy enforcement point.
E.g., I know that crap can't get into my central repo because a hook
checks everything coming in. But if a developer has turned off his
pre-push hook (or accidentally failed to enable it), he can still send
crap.

One other argument I have seen is that, to prevent the proliferation of
hooks, the rule is not to add a hook that could just as easily be done
as a sequence of commands. IOW, what's wrong with

  run_my_automated_tests && git push

?

Off the top of my head, I guess the response to those two arguments
would be:

 - sometimes the receiving end isn't set up to run tests, which means it
   is more reasonable to do it on the sending side

 - it's more convenient to just type "git push" than to remember "tests
   && git push", so this reduces the chances of contributors
   accidentally pushing crap

-Peff

Jeff King [off-list ref] wrote:

I think the common wisdom has been that such tests should be done on the
_receiving_ end, since that makes a more trustworthy enforcement point.
E.g., I know that crap can't get into my central repo because a hook
checks everything coming in. But if a developer has turned off his
pre-push hook (or accidentally failed to enable it), he can still send
crap.

One other argument I have seen is that, to prevent the proliferation of
hooks, the rule is not to add a hook that could just as easily be done
as a sequence of commands. IOW, what's wrong with

  run_my_automated_tests && git push

Yup, I agree completely.

Why not just setup an alias:

	git config alias.send '! run_my_tests && git push "$@"'

and retrain your fingers to use "git send ..."?
 
-- 
Shawn.

On Tue, Aug 19, 2008 at 12:59 PM, Shawn O. Pearce [off-list ref] wrote:

Jeff King [off-list ref] wrote:

quoted

I think the common wisdom has been that such tests should be done on the
_receiving_ end, since that makes a more trustworthy enforcement point.
E.g., I know that crap can't get into my central repo because a hook
checks everything coming in. But if a developer has turned off his
pre-push hook (or accidentally failed to enable it), he can still send
crap.

One other argument I have seen is that, to prevent the proliferation of
hooks, the rule is not to add a hook that could just as easily be done
as a sequence of commands. IOW, what's wrong with

  run_my_automated_tests && git push

Yup, I agree completely.

Why not just setup an alias:

       git config alias.send '! run_my_tests && git push "$@"'

and retrain your fingers to use "git send ..."?

--
Shawn.

Sorry, but couldn't this argument be made about any of the hooks run
after manual operations?  ie: pre-commit, pre-applypatch, commit-msg,
post-commit, post-applypatch?  I mean, couldn't you do :

git config alias.docommit '! do_pre_commit && git commit ...' ?

I thought the point of these kind of hooks was to make stuff like this
automatic and easy to standardize for a project, so people working on
a dozen git repos don't have to remember all the aliases they set up
in each one.

Scott

Quoting Scott Chacon [off-list ref]:

On Tue, Aug 19, 2008 at 12:59 PM, Shawn O. Pearce [off-list ref] wrote:

quoted

Jeff King [off-list ref] wrote:

quoted

One other argument I have seen is that, to prevent the proliferation of
hooks, the rule is not to add a hook that could just as easily be done
as a sequence of commands. IOW, what's wrong with

  run_my_automated_tests && git push

Yup, I agree completely.

Why not just setup an alias:

       git config alias.send '! run_my_tests && git push "$@"'

and retrain your fingers to use "git send ..."?

Sorry, but couldn't this argument be made about any of the hooks run
after manual operations?  ie: pre-commit, pre-applypatch, commit-msg,
post-commit, post-applypatch?  I mean, couldn't you do :

git config alias.docommit '! do_pre_commit && git commit ...' ?

I thought the point of these kind of hooks was to make stuff like this
automatic and easy to standardize for a project, so people working on
a dozen git repos don't have to remember all the aliases they set up
in each one.

This topic seems to come up every once in a while.

 http://thread.gmane.org/gmane.comp.version-control.git/70781/focus=71069
 http://thread.gmane.org/gmane.comp.version-control.git/79306/focus=79321

Somebody needs to describe the general rules in SubmittingPatches, perhaps?

I do not understand why Junio said he thinks this pre-push hook is a good idea.  This clearly is "you always would want to do before running a git command" case.

-- 
Nanako Shiraishi
http://ivory.ap.teacup.com/nanako3/

On Tue, Aug 19, 2008 at 2:26 PM, しらいしななこ [off-list ref] wrote:

Quoting Scott Chacon [off-list ref]:

quoted

On Tue, Aug 19, 2008 at 12:59 PM, Shawn O. Pearce [off-list ref] wrote:

quoted

Jeff King [off-list ref] wrote:

quoted

One other argument I have seen is that, to prevent the proliferation of
hooks, the rule is not to add a hook that could just as easily be done
as a sequence of commands. IOW, what's wrong with

  run_my_automated_tests && git push

Yup, I agree completely.

Why not just setup an alias:

       git config alias.send '! run_my_tests && git push "$@"'

and retrain your fingers to use "git send ..."?

Sorry, but couldn't this argument be made about any of the hooks run
after manual operations?  ie: pre-commit, pre-applypatch, commit-msg,
post-commit, post-applypatch?  I mean, couldn't you do :

git config alias.docommit '! do_pre_commit && git commit ...' ?

I thought the point of these kind of hooks was to make stuff like this
automatic and easy to standardize for a project, so people working on
a dozen git repos don't have to remember all the aliases they set up
in each one.

This topic seems to come up every once in a while.

 http://thread.gmane.org/gmane.comp.version-control.git/70781/focus=71069
 http://thread.gmane.org/gmane.comp.version-control.git/79306/focus=79321

Somebody needs to describe the general rules in SubmittingPatches, perhaps?

I do not understand why Junio said he thinks this pre-push hook is a good idea.  This clearly is "you always would want to do before running a git command" case.

--
Nanako Shiraishi
http://ivory.ap.teacup.com/nanako3/

I don't think I understand how this is different than 'pre-commit'
(or, alternatively, how this does not fall under #1 in that list).  If
the script exits non-0, it stops the push, isn't that exactly what
pre-commit does, but with 'push' instead of 'commit'?

Scott

Jeff King wrote:

quoted

If the patch is acceptable, I will update the githooks doc with more
information, but we would like this so that you could add a hook that
runs your automated tests before a push would go through.

I think the common wisdom has been that such tests should be done on the
_receiving_ end, since that makes a more trustworthy enforcement point.

Probably true, but if someone wants to arrange it the other way around,
what harm is there in that?

Sam

On Wed, Aug 20, 2008 at 10:26:48AM +1200, Sam Vilain wrote:

quoted

I think the common wisdom has been that such tests should be done on the
_receiving_ end, since that makes a more trustworthy enforcement point.

Probably true, but if someone wants to arrange it the other way around,
what harm is there in that?

Read the rest of the message you are quoting where I say basically that.

-Peff