{"thread":{"id":"32473","subject":"[PATCH 1/4] hooks: Add function to check if a hook exists","startedAt":"2012-12-28T22:57:28Z","lastAt":"2013-01-15T03:12:11Z","messageCount":22,"participants":["Aaron Schrab","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":4},"messages":[{"id":"205617","messageId":"1356735452-21667-1-git-send-email-aaron@schrab.com","threadId":"32473","inReplyTo":null,"subject":"[PATCH 0/4] pre-push hook support","fromName":"Aaron Schrab","fromEmail":"aaron@schrab.com","sentAt":"2012-12-28T22:57:28Z","receivedAt":"2012-12-28T22:57:28Z","isPatch":true,"sender":{"key":"aaron@schrab.com","avatar":"https://avatars.githubusercontent.com/u/39620?v=4"},"body":"There have been at least a couple of submissions to add support for a\npre-push hook, which were rejected at least partially because they didn't\nprovide enough information to a hook script for it to determine what was\nto be pushed any better than a separate wrapper around the 'git push'\ncommand would be able to do. In this series I attempt to address that\nproblem.\n\nThe first two patches in this series do a little bit of refactoring in\norder to make it easier to call hooks with a variable number of arguments.\n\nThe third patch actually adds support for calling a pre-push hook. If it\nexists, it will be called with the name and URL of the destination remote\n(if a named remote isn't being used, the URL will be supplied for both)\nfollowed by another argument for each ref being pushed; these arguments\ntake the form:\n\n :::\n\nThis should provide enough information for a script to easily determine\nthe set of commits that is being pushed, and thus make a decision if that\nshould be allowed.\n\nThe final patch adds a sample pre-push hook script which will deny\nattempts to push commits that are marked as a work in progress.\n\nAaron Schrab (4):\n hooks: Add function to check if a hook exists\n hooks: support variable number of parameters\n push: Add support for pre-push hooks\n Add sample pre-push hook script\n\n Documentation/githooks.txt | 28 ++++++++\n builtin/push.c | 1 +\n run-command.c | 35 ++++++---\n run-command.h | 3 +\n t/t5571-pre-push-hook.sh | 145 ++++++++++++++++++++++++++++++++++++++\n templates/hooks--pre-push.sample | 63 +++++++++++++++++\n transport.c | 25 +++++++\n transport.h | 1 +\n 8 files changed, 292 insertions(+), 9 deletions(-)\n create mode 100755 t/t5571-pre-push-hook.sh\n create mode 100644 templates/hooks--pre-push.sample\n\n-- \n1.7.10.4\n"},{"id":"205616","messageId":"1356735452-21667-2-git-send-email-aaron@schrab.com","threadId":"32473","inReplyTo":"1356735452-21667-1-git-send-email-aaron@schrab.com","subject":"[PATCH 1/4] hooks: Add function to check if a hook exists","fromName":"Aaron Schrab","fromEmail":"aaron@schrab.com","sentAt":"2012-12-28T22:57:29Z","receivedAt":"2012-12-28T22:57:29Z","isPatch":true,"sender":{"key":"aaron@schrab.com","avatar":"https://avatars.githubusercontent.com/u/39620?v=4"},"body":"Create find_hook() function to determine if a given hook exists and is\nexecutable. If it is the path to the script will be returned, otherwise\nNULL is returned.\n\nThis is in support for an upcoming run_hook_argv() function which will\nexpect the full path to the hook script as the first element in the\nargv_array. This also makes it simple for places that can use a hook to\ncheck if a hook exists before doing, possibly lengthy, setup work which\nwould be pointless if no such hook is present.\n\nThe returned value is left as a static value from get_pathname() rather\nthan a duplicate because it is anticipated that the return value will\neither be used as a boolean, or immediately added to an argv_array list\nwhich would result in it being duplicated at that point.\n\nSigned-off-by: Aaron Schrab \n---\n run-command.c | 15 +++++++++++++--\n run-command.h | 1 +\n 2 files changed, 14 insertions(+), 2 deletions(-)\n\ndiff --git a/run-command.c b/run-command.c\nindex 3b982e4..49c8fa0 100644\n--- a/run-command.c\n+++ b/run-command.c\n@@ -735,6 +735,15 @@ int finish_async(struct async *async)\n #endif\n }\n \n+char *find_hook(const char *name)\n+{\n+\tchar *path = git_path(\"hooks/%s\", name);\n+\tif (access(path, X_OK) < 0)\n+\t\tpath = NULL;\n+\n+\treturn path;\n+}\n+\n int run_hook(const char *index_file, const char *name, ...)\n {\n \tstruct child_process hook;\n@@ -744,11 +753,13 @@ int run_hook(const char *index_file, const char *name, ...)\n \tva_list args;\n \tint ret;\n \n-\tif (access(git_path(\"hooks/%s\", name), X_OK) < 0)\n+\tp = find_hook(name);\n+\tif (!p)\n \t\treturn 0;\n \n+\targv_array_push(&argv, p);\n+\n \tva_start(args, name);\n-\targv_array_push(&argv, git_path(\"hooks/%s\", name));\n \twhile ((p = va_arg(args, const char *)))\n \t\targv_array_push(&argv, p);\n \tva_end(args);\ndiff --git a/run-command.h b/run-command.h\nindex 850c638..221ce33 100644\n--- a/run-command.h\n+++ b/run-command.h\n@@ -45,6 +45,7 @@ int start_command(struct child_process *);\n int finish_command(struct child_process *);\n int run_command(struct child_process *);\n \n+extern char *find_hook(const char *name);\n extern int run_hook(const char *index_file, const char *name, ...);\n \n #define RUN_COMMAND_NO_STDIN 1\n-- \n1.7.10.4\n"},{"id":"205618","messageId":"1356735452-21667-3-git-send-email-aaron@schrab.com","threadId":"32473","inReplyTo":"1356735452-21667-1-git-send-email-aaron@schrab.com","subject":"[PATCH 2/4] hooks: support variable number of parameters","fromName":"Aaron Schrab","fromEmail":"aaron@schrab.com","sentAt":"2012-12-28T22:57:30Z","receivedAt":"2012-12-28T22:57:30Z","isPatch":true,"sender":{"key":"aaron@schrab.com","avatar":"https://avatars.githubusercontent.com/u/39620?v=4"},"body":"Define the run_hook_argv() function to allow hooks to be created where\nthe number of parameters to be passed is variable. The existing\nrun_hook() function uses stdarg to allow it to receive a variable number\nof arguments, but the number of arguments that a given caller is passing\nis fixed at compile time. This function will allow the caller of a hook\nto determine the number of arguments to pass when preparing to call the\nhook.\n\nThe first use of this function will be for a pre-push hook which will\nadd an argument for every reference which is to be pushed.\n\nSigned-off-by: Aaron Schrab \n---\n run-command.c | 20 +++++++++++++-------\n run-command.h | 2 ++\n 2 files changed, 15 insertions(+), 7 deletions(-)\n\ndiff --git a/run-command.c b/run-command.c\nindex 49c8fa0..e07202b 100644\n--- a/run-command.c\n+++ b/run-command.c\n@@ -2,7 +2,6 @@\n #include \"run-command.h\"\n #include \"exec_cmd.h\"\n #include \"sigchain.h\"\n-#include \"argv-array.h\"\n \n #ifndef SHELL_PATH\n # define SHELL_PATH \"/bin/sh\"\n@@ -746,10 +745,8 @@ char *find_hook(const char *name)\n \n int run_hook(const char *index_file, const char *name, ...)\n {\n-\tstruct child_process hook;\n \tstruct argv_array argv = ARGV_ARRAY_INIT;\n-\tconst char *p, *env[2];\n-\tchar index[PATH_MAX];\n+\tconst char *p;\n \tva_list args;\n \tint ret;\n \n@@ -764,6 +761,17 @@ int run_hook(const char *index_file, const char *name, ...)\n \t\targv_array_push(&argv, p);\n \tva_end(args);\n \n+\tret = run_hook_argv(index_file, argv);\n+\targv_array_clear(&argv);\n+\treturn ret;\n+}\n+\n+int run_hook_argv(const char *index_file, struct argv_array argv)\n+{\n+\tstruct child_process hook;\n+\tchar index[PATH_MAX];\n+\tconst char *env[2];\n+\n \tmemset(&hook, 0, sizeof(hook));\n \thook.argv = argv.argv;\n \thook.no_stdin = 1;\n@@ -775,7 +783,5 @@ int run_hook(const char *index_file, const char *name, ...)\n \t\thook.env = env;\n \t}\n \n-\tret = run_command(&hook);\n-\targv_array_clear(&argv);\n-\treturn ret;\n+\treturn run_command(&hook);\n }\ndiff --git a/run-command.h b/run-command.h\nindex 221ce33..12faa5b 100644\n--- a/run-command.h\n+++ b/run-command.h\n@@ -1,6 +1,7 @@\n #ifndef RUN_COMMAND_H\n #define RUN_COMMAND_H\n \n+#include \"argv-array.h\"\n #ifndef NO_PTHREADS\n #include \n #endif\n@@ -47,6 +48,7 @@ int run_command(struct child_process *);\n \n extern char *find_hook(const char *name);\n extern int run_hook(const char *index_file, const char *name, ...);\n+extern int run_hook_argv(const char *index_file, struct argv_array);\n \n #define RUN_COMMAND_NO_STDIN 1\n #define RUN_GIT_CMD\t 2\t/*If this is to be git sub-command */\n-- \n1.7.10.4\n"},{"id":"205620","messageId":"1356735452-21667-4-git-send-email-aaron@schrab.com","threadId":"32473","inReplyTo":"1356735452-21667-1-git-send-email-aaron@schrab.com","subject":"[PATCH 3/4] push: Add support for pre-push hooks","fromName":"Aaron Schrab","fromEmail":"aaron@schrab.com","sentAt":"2012-12-28T22:57:31Z","receivedAt":"2012-12-28T22:57:31Z","isPatch":true,"sender":{"key":"aaron@schrab.com","avatar":"https://avatars.githubusercontent.com/u/39620?v=4"},"body":"Add support for a pre-push hook which can be used to determine if the\nset of refs to be pushed is suitable for the target repository. The\nhook should be supplied with:\n\n 1. name of the remote being used, or the URL if not using a named\n remote\n 2. the URL to which we're pushing\n 3. descriptions of what references are to be pushed\n\nEach reference to be pushed should be described in a separate parameter\nto the hook script in the form:\n\n :::\n\nThis will allow the script to determine if the push is acceptable based\non the target repository and branch(es), the commits which are to be\npushed, and even the source branches in some cases.\n\nSigned-off-by: Aaron Schrab \n---\n Documentation/githooks.txt | 28 +++++++++\n builtin/push.c | 1 +\n t/t5571-pre-push-hook.sh | 145 ++++++++++++++++++++++++++++++++++++++++++++\n transport.c | 25 ++++++++\n transport.h | 1 +\n 5 files changed, 200 insertions(+)\n create mode 100755 t/t5571-pre-push-hook.sh\n\ndiff --git a/Documentation/githooks.txt b/Documentation/githooks.txt\nindex b9003fe..e9539bb 100644\n--- a/Documentation/githooks.txt\n+++ b/Documentation/githooks.txt\n@@ -176,6 +176,34 @@ save and restore any form of metadata associated with the working tree\n (eg: permissions/ownership, ACLS, etc). See contrib/hooks/setgitperms.perl\n for an example of how to do this.\n \n+pre-push\n+~~~~~~~~\n+\n+This hook is called by 'git push' and can be used to prevent a push from\n+taking place. The hook is called with a variable number of parameters.\n+\n+The first parameters provide the name and location of the destination\n+remote, if a named remote is not being used both values will be the same.\n+\n+Remaining parameters provide information about the commits which are to be\n+pushed and the ref names being used. These arguments take the form:\n+\n+ :::\n+\n+For instance, if the command +git push origin master:foreign+ were run the\n+hook would be called with a third arugment similar to:\n+\n+ refs/heads/master:67890:refs/heads/foreign:12345\n+\n+although the full, 40-character SHA1s would be supplied. If the foreign ref\n+does not yet exist the `` will be 40 `0`. If a ref is to be\n+deleted, the `` will be supplied as `(delete)` and the `` will be 40 `0`. If the local commit was specified by something other\n+than a name which could be expanded (such as `HEAD~`, or a SHA1) it will be\n+supplied as it was originally given.\n+\n+If this hook exits with a non-zero status, 'git push' will abort.\n+\n [[pre-receive]]\n pre-receive\n ~~~~~~~~~~~\ndiff --git a/builtin/push.c b/builtin/push.c\nindex db9ba30..c33fb9b 100644\n--- a/builtin/push.c\n+++ b/builtin/push.c\n@@ -399,6 +399,7 @@ int cmd_push(int argc, const char **argv, const char *prefix)\n \t\tOPT_BOOL(0, \"progress\", &progress, N_(\"force progress reporting\")),\n \t\tOPT_BIT(0, \"prune\", &flags, N_(\"prune locally removed refs\"),\n \t\t\tTRANSPORT_PUSH_PRUNE),\n+\t\tOPT_BIT(0, \"no-verify\", &flags, N_(\"bypass pre-push hook\"), TRANSPORT_PUSH_NO_HOOK),\n \t\tOPT_END()\n \t};\n \ndiff --git a/t/t5571-pre-push-hook.sh b/t/t5571-pre-push-hook.sh\nnew file mode 100755\nindex 0000000..5444c9b\n--- /dev/null\n+++ b/t/t5571-pre-push-hook.sh\n@@ -0,0 +1,145 @@\n+#!/bin/sh\n+\n+test_description='check pre-push hooks'\n+. ./test-lib.sh\n+\n+# Setup hook that always succeeds\n+HOOKDIR=\"$(git rev-parse --git-dir)/hooks\"\n+HOOK=\"$HOOKDIR/pre-push\"\n+mkdir -p \"$HOOKDIR\"\n+cat >\"$HOOK\" <\"$HOOK\" <\"$HOOK\" <<'EOF'\n+#!/bin/sh -ex\n+test \"$#\" = 3\n+test \"$1\" = parent1\n+test \"$2\" = repo1\n+test \"$3\" = \"refs/heads/master:$COMMIT2:refs/heads/foreign:$COMMIT1\"\n+EOF\n+\n+test_expect_success 'push with hook' '\n+\tgit push parent1 master:foreign\n+'\n+\n+test_expect_success 'add a branch' '\n+\tgit checkout -b other &&\n+\ttest_commit three\n+'\n+\n+COMMIT3=\"$(git rev-parse HEAD)\"\n+export COMMIT3\n+\n+cat >\"$HOOK\" <<'EOF'\n+#!/bin/sh -ex\n+test \"$#\" = 4\n+test \"$1\" = parent1\n+test \"$2\" = repo1\n+test \"$3\" = \"refs/heads/other:$COMMIT3:refs/heads/foreign:$COMMIT2\"\n+test \"$4\" = \"refs/heads/master:$COMMIT2:refs/heads/new:$_z40\"\n+EOF\n+\n+test_expect_success 'push multiple refs' '\n+\tgit push parent1 other:foreign master:new\n+'\n+\n+test_expect_success 'add a branch with an upstream' '\n+\tgit checkout -t -b tracking parent1/foreign &&\n+\ttest_commit four\n+'\n+COMMIT4=\"$(git rev-parse HEAD)\"\n+export COMMIT4\n+\n+cat >\"$HOOK\" <<'EOF'\n+#!/bin/sh -ex\n+test \"$#\" = 3\n+test \"$1\" = parent1\n+test \"$2\" = repo1\n+test \"$3\" = \"refs/heads/tracking:$COMMIT4:refs/heads/foreign:$COMMIT3\"\n+EOF\n+\n+test_expect_success 'push to upstream branch' '\n+\tgit push &&\n+\tgit checkout other\n+'\n+\n+cat >\"$HOOK\" <<'EOF'\n+#!/bin/sh -ex\n+test \"$#\" = 3\n+test \"$1\" = parent1\n+test \"$2\" = repo1\n+test \"$3\" = \"(delete):$_z40:refs/heads/new:$COMMIT2\"\n+EOF\n+\n+test_expect_success 'push deletion' '\n+\tgit push parent1 :new\n+'\n+\n+cat >\"$HOOK\" <<'EOF'\n+#!/bin/sh -ex\n+test \"$#\" = 3\n+test \"$1\" = repo2\n+test \"$2\" = repo2\n+test \"$3\" = \"refs/heads/other:$COMMIT3:refs/heads/new:$_z40\"\n+EOF\n+\n+test_expect_success 'push to URL' '\n+\tgit init --bare repo2 &&\n+\tgit push repo2 other:new\n+'\n+\n+ABBR=$(expr substr $COMMIT3 1 8)\n+export ABBR\n+\n+cat >\"$HOOK\" <<'EOF'\n+#!/bin/sh -ex\n+test \"$#\" = 4\n+test \"$3\" = \"HEAD~:$COMMIT2:refs/heads/commitish:$_z40\"\n+test \"$4\" = \"$ABBR:$COMMIT3:refs/heads/sha:$_z40\"\n+EOF\n+\n+test_expect_success 'push commit' '\n+\tgit push parent1 HEAD~:refs/heads/commitish $ABBR:refs/heads/sha\n+'\n+\n+cat >\"$HOOK\" <<'EOF'\n+#!/bin/sh -ex\n+test \"$#\" = 3\n+test \"$3\" = \"refs/tags/one:$COMMIT1:refs/tags/tagpush:$_z40\"\n+EOF\n+\n+test_expect_success 'push tag' '\n+\tgit push parent1 one:tagpush\n+'\n+\n+test_done\ndiff --git a/transport.c b/transport.c\nindex 9932f40..b0c9a15 100644\n--- a/transport.c\n+++ b/transport.c\n@@ -1052,6 +1052,7 @@ int transport_push(struct transport *transport,\n \t\tint porcelain = flags & TRANSPORT_PUSH_PORCELAIN;\n \t\tint pretend = flags & TRANSPORT_PUSH_DRY_RUN;\n \t\tint push_ret, ret, err;\n+\t\tchar *hook;\n \n \t\tif (flags & TRANSPORT_PUSH_ALL)\n \t\t\tmatch_flags |= MATCH_REFS_ALL;\n@@ -1069,6 +1070,30 @@ int transport_push(struct transport *transport,\n \t\t\tflags & TRANSPORT_PUSH_MIRROR,\n \t\t\tflags & TRANSPORT_PUSH_FORCE);\n \n+\t\tif (!(flags & TRANSPORT_PUSH_NO_HOOK) && (hook = find_hook(\"pre-push\"))) {\n+\t\t\tstruct ref *r;\n+\t\t\tstruct argv_array argv = ARGV_ARRAY_INIT;\n+\n+\t\t\targv_array_push(&argv, hook);\n+\t\t\targv_array_push(&argv, transport->remote->name);\n+\t\t\targv_array_push(&argv, transport->url);\n+\n+\t\t\tfor (r = remote_refs; r; r = r->next) {\n+\t\t\t\tif (!r->peer_ref) continue;\n+\t\t\t\tif (r->status == REF_STATUS_REJECT_NONFASTFORWARD) continue;\n+\t\t\t\tif (r->status == REF_STATUS_UPTODATE) continue;\n+\n+\t\t\t\targv_array_pushf(&argv, \"%s:%s:%s:%s\",\n+\t\t\t\t\tr->peer_ref->name, sha1_to_hex(r->new_sha1),\n+\t\t\t\t\tr->name, sha1_to_hex(r->old_sha1));\n+\t\t\t}\n+\n+\t\t\tret = run_hook_argv(NULL, argv);\n+\t\t\targv_array_clear(&argv);\n+\t\t\tif (ret)\n+\t\t\t\treturn -1;\n+\t\t}\n+\n \t\tif ((flags & TRANSPORT_RECURSE_SUBMODULES_ON_DEMAND) && !is_bare_repository()) {\n \t\t\tstruct ref *ref = remote_refs;\n \t\t\tfor (; ref; ref = ref->next)\ndiff --git a/transport.h b/transport.h\nindex 4a61c0c..3bc9863 100644\n--- a/transport.h\n+++ b/transport.h\n@@ -104,6 +104,7 @@ struct transport {\n #define TRANSPORT_RECURSE_SUBMODULES_CHECK 64\n #define TRANSPORT_PUSH_PRUNE 128\n #define TRANSPORT_RECURSE_SUBMODULES_ON_DEMAND 256\n+#define TRANSPORT_PUSH_NO_HOOK 512\n \n #define TRANSPORT_SUMMARY_WIDTH (2 * DEFAULT_ABBREV + 3)\n #define TRANSPORT_SUMMARY(x) (int)(TRANSPORT_SUMMARY_WIDTH + strlen(x) - gettext_width(x)), (x)\n-- \n1.7.10.4\n"},{"id":"205619","messageId":"1356735452-21667-5-git-send-email-aaron@schrab.com","threadId":"32473","inReplyTo":"1356735452-21667-1-git-send-email-aaron@schrab.com","subject":"[PATCH 4/4] Add sample pre-push hook script","fromName":"Aaron Schrab","fromEmail":"aaron@schrab.com","sentAt":"2012-12-28T22:57:32Z","receivedAt":"2012-12-28T22:57:32Z","isPatch":true,"sender":{"key":"aaron@schrab.com","avatar":"https://avatars.githubusercontent.com/u/39620?v=4"},"body":"Create a sample of a script for a pre-push hook. The main purpose is to\nillustrate how a script may parse the parameters which are supplied to\nsuch a hook. The script may also be useful to some people as-is for\navoiding to push commits which are marked as a work in progress.\n\nSigned-off-by: Aaron Schrab \n---\n templates/hooks--pre-push.sample | 63 ++++++++++++++++++++++++++++++++++++++\n 1 file changed, 63 insertions(+)\n create mode 100644 templates/hooks--pre-push.sample\n\ndiff --git a/templates/hooks--pre-push.sample b/templates/hooks--pre-push.sample\nnew file mode 100644\nindex 0000000..1d3b4a3\n--- /dev/null\n+++ b/templates/hooks--pre-push.sample\n@@ -0,0 +1,63 @@\n+#!/bin/sh\n+\n+# An example hook script to verify what is about to be pushed.\n+# Called by \"git push\" after it has checked the remote status, but before\n+# anything has been pushed. If this script exits with a non-zero status\n+# nothing will be pushed.\n+#\n+# This hook is called with the following parameters:\n+#\n+# $1 -- Name of the remote to which the push is being done\n+# $2 -- URL to which the push is being done\n+#\n+# If pushing without using a named remote those arguments will be equal.\n+#\n+# Further arguments provide information about the commits which are being\n+# pushed in the form:\n+#\n+# :::\n+#\n+# This sample shows how to prevent push of commits where the log\n+# message starts with \"WIP\" (work in progress).\n+\n+remote=\"$1\"\n+url=\"$2\"\n+shift 2\n+\n+z40=0000000000000000000000000000000000000000\n+\n+old_ifs=\"$IFS\"\n+for to_push in \"$@\"\n+do\n+\t# Split the value into its parts\n+\tIFS=:\n+\tset -- $to_push\n+\tIFS=\"$old_ifs\"\n+\n+\tlocal_ref=\"$1\"\n+\tlocal_sha=\"$2\"\n+\tremote_ref=\"$3\"\n+\tremote_sha=\"$4\"\n+\n+\tif [ \"$local_sha\" = $z40 ]\n+\tthen\n+\t\trange=''\n+\t\t# Handle deletes\n+\telse\n+\t\tif [ \"$remote_sha\" = $z40 ]\n+\t\tthen\n+\t\t\trange=\"$local_sha\"\n+\t\telse\n+\t\t\trange=\"$remote_sha..$local_sha\"\n+\t\tfi\n+\n+\t\tcommit=`git rev-list -n 1 --grep '^WIP' \"$range\"`\n+\t\tif [ -n \"$commit\" ]\n+\t\tthen\n+\t\t\techo \"Found WIP commit in $local_ref, not pushing\"\n+\t\t\texit 1\n+\t\tfi\n+\tfi\n+done\n+\n+exit 0\n-- \n1.7.10.4\n"},{"id":"205630","messageId":"7v1ue9hb06.fsf@alter.siamese.dyndns.org","threadId":"32473","inReplyTo":"1356735452-21667-1-git-send-email-aaron@schrab.com","subject":"Re: [PATCH 0/4] pre-push hook support","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-12-29T02:01:45Z","receivedAt":"2012-12-29T02:01:45Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Aaron Schrab writes:\n\n> There have been at least a couple of submissions to add support for a\n> pre-push hook, which were rejected at least partially because they didn't\n> provide enough information to a hook script for it to determine what was\n> to be pushed any better than a separate wrapper around the 'git push'\n> command would be able to do. In this series I attempt to address that\n> problem.\n>\n> The first two patches in this series do a little bit of refactoring in\n> order to make it easier to call hooks with a variable number of arguments.\n>\n> The third patch actually adds support for calling a pre-push hook. If it\n> exists, it will be called with the name and URL of the destination remote\n> (if a named remote isn't being used, the URL will be supplied for both)\n> followed by another argument for each ref being pushed; these arguments\n> take the form:\n>\n> :::\n\nOne lesson we learned long time ago while doing hooks is to avoid\nunbound number of command line arguments and instead feed them from\nthe standard input. I think this should do the same.\n\n> This should provide enough information for a script to easily determine\n> the set of commits that is being pushed, and thus make a decision if that\n> should be allowed.\n\nHow does the hook communicate its decision to the calling Git?\n\nWill it be \"all-or-none\", or \"I'll allow these but not those\"?\n"},{"id":"205631","messageId":"7vwqw1fw5a.fsf@alter.siamese.dyndns.org","threadId":"32473","inReplyTo":"1356735452-21667-2-git-send-email-aaron@schrab.com","subject":"Re: [PATCH 1/4] hooks: Add function to check if a hook exists","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-12-29T02:08:01Z","receivedAt":"2012-12-29T02:08:01Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Aaron Schrab writes:\n\n> Create find_hook() function to determine if a given hook exists and is\n> executable. If it is the path to the script will be returned, otherwise\n> NULL is returned.\n\nSounds like a sensible thing to do. To make sure the API is also\nsensible, all the existing hooks should be updated to use this API,\nno?\n\n> This is in support for an upcoming run_hook_argv() function which will\n> expect the full path to the hook script as the first element in the\n> argv_array. \n\nThere is currently a public function called run_hook() that squats\non the good name with a kludgy API that is too specific to using\nseparate index file. Back when it was a private helper in the\nimplementation of \"git commit\", it was perfectly fine, but it was\nexported without giving much thought on the API.\n\nIf you are introducing a new run_hook_* function, give it a generic\nenough API that lets all the existing hook callers to use it. I\nwould imagine that the API requirement may be modelled after\nrun_command() API so that we can pass argv[] and tweak the hook's\nenviron[], as well as feeding its stdin and possibly reading from\nits stdout. That would be very useful.\n"},{"id":"205653","messageId":"20121229145025.GA3789@pug.qqx.org","threadId":"32473","inReplyTo":"7v1ue9hb06.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH 0/4] pre-push hook support","fromName":"Aaron Schrab","fromEmail":"aaron@schrab.com","sentAt":"2012-12-29T14:50:25Z","receivedAt":"2012-12-29T14:50:25Z","isPatch":true,"sender":{"key":"aaron@schrab.com","avatar":"https://avatars.githubusercontent.com/u/39620?v=4"},"body":"At 18:01 -0800 28 Dec 2012, Junio C Hamano wrote:\n>One lesson we learned long time ago while doing hooks is to avoid\n>unbound number of command line arguments and instead feed them from\n>the standard input. I think this should do the same.\n\nGood point. I had been trying to keep the interface for this hook as \nclose as possible to the ones for other client-side hooks on the theory \nthat less development effort may go into those than for server-side \nhooks. But thinking on that more I certainly see that this could easily \nrun into limits on argument length on some systems; especially when it's \nlikely that each of those arguments is likely to be over 100 bytes long.\n\nI'll work on an updated version which sends the variable length \ninformation over a pipe, using the command-line arguments only to pass \nthe remote name and URL.\n\n>How does the hook communicate its decision to the calling Git?\n>\n>Will it be \"all-or-none\", or \"I'll allow these but not those\"?\n\nCurrently it just uses the exit code to communicate that back, so it's \nall-or-none. I think I'll keep that in the updated version as well.\n\nA future enhancement could modify the protocol to support reading from \nthe hook's stdout the names of remote refs which are to be rejected, I \nthink that just having the option for all-or-nothing is a good starting \npoint.\n"},{"id":"205654","messageId":"20121229145032.GB3789@pug.qqx.org","threadId":"32473","inReplyTo":"7vwqw1fw5a.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH 1/4] hooks: Add function to check if a hook exists","fromName":"Aaron Schrab","fromEmail":"aaron@schrab.com","sentAt":"2012-12-29T14:50:32Z","receivedAt":"2012-12-29T14:50:32Z","isPatch":true,"sender":{"key":"aaron@schrab.com","avatar":"https://avatars.githubusercontent.com/u/39620?v=4"},"body":"At 18:08 -0800 28 Dec 2012, Junio C Hamano wrote:\n>Aaron Schrab writes:\n>\n>> Create find_hook() function to determine if a given hook exists and is\n>> executable. If it is the path to the script will be returned, otherwise\n>> NULL is returned.\n>\n>Sounds like a sensible thing to do. To make sure the API is also\n>sensible, all the existing hooks should be updated to use this API,\n>no?\n\nI'd been trying to keep the changes limited. I'll see about modifying \nthe existing places that run hooks in v2 of the series.\n\n>> This is in support for an upcoming run_hook_argv() function which will\n>> expect the full path to the hook script as the first element in the\n>> argv_array.\n>\n>There is currently a public function called run_hook() that squats\n>on the good name with a kludgy API that is too specific to using\n>separate index file. Back when it was a private helper in the\n>implementation of \"git commit\", it was perfectly fine, but it was\n>exported without giving much thought on the API.\n>\n>If you are introducing a new run_hook_* function, give it a generic\n>enough API that lets all the existing hook callers to use it. I\n>would imagine that the API requirement may be modelled after\n>run_command() API so that we can pass argv[] and tweak the hook's\n>environ[], as well as feeding its stdin and possibly reading from\n>its stdout. That would be very useful.\n\nI think the attraction of the run_hook() API is its simplicity. It's \ncurrently a fairly thin wrapper around the run_command() API. I suspect \nthat if the run_hook() API were made generic enough to support all of \nthe existing hook callers it would greatly complicate the existing calls \nto run_hook() while not providing much benefit to hook callers which \ncan't currently use it beyond what run_command() offers.\n\nSince I'm going to be changing the interface for this hook in v2 of the \nseries so that it will be more complicated than can be readily addressed \nwith the run_hook() API (and will have use a fixed number of arguments \nanyway) I'll be dropping the run_hook_argv() function.\n"},{"id":"205656","messageId":"7vlicgg5z2.fsf@alter.siamese.dyndns.org","threadId":"32473","inReplyTo":"20121229145025.GA3789@pug.qqx.org","subject":"Re: [PATCH 0/4] pre-push hook support","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-12-29T16:48:01Z","receivedAt":"2012-12-29T16:48:01Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Aaron Schrab writes:\n\n> At 18:01 -0800 28 Dec 2012, Junio C Hamano wrote:\n>>Will it be \"all-or-none\", or \"I'll allow these but not those\"?\n>\n> Currently it just uses the exit code to communicate that back, so it's\n> all-or-none. I think I'll keep that in the updated version as well.\n\nThanks; that sounds sensible.\n"},{"id":"205657","messageId":"7vhan4g5ny.fsf@alter.siamese.dyndns.org","threadId":"32473","inReplyTo":"20121229145032.GB3789@pug.qqx.org","subject":"Re: [PATCH 1/4] hooks: Add function to check if a hook exists","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-12-29T16:54:41Z","receivedAt":"2012-12-29T16:54:41Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Aaron Schrab writes:\n\n> Since I'm going to be changing the interface for this hook in v2 of\n> the series so that it will be more complicated than can be readily\n> addressed with the run_hook() API (and will have use a fixed number of\n> arguments anyway) I'll be dropping the run_hook_argv() function.\n\nJust to make sure there is no misunderstanding (sorry for sending\nthe message without finishing it with this clarification at the end\nin the first place). I didn't mean that converting all of the\nexisting callers must come earlier than introducing a new hook\ninvoker.\n\nI just wanted to make sure that we are aware that we are adding to\nour technical debt, if we are adding another that is also\nspecialized; as the proposed interface looked sufficiently generic,\nit would be the ideal one to make _other_ ones thin wrappers around\nit to unify the various codepaths.\n\nThanks.\n"},{"id":"206644","messageId":"1358054224-7710-1-git-send-email-aaron@schrab.com","threadId":"32473","inReplyTo":"1356735452-21667-1-git-send-email-aaron@schrab.com","subject":"[PATCH v2 0/3] pre-push hook support","fromName":"Aaron Schrab","fromEmail":"aaron@schrab.com","sentAt":"2013-01-13T05:17:01Z","receivedAt":"2013-01-13T05:17:01Z","isPatch":true,"sender":{"key":"aaron@schrab.com","avatar":"https://avatars.githubusercontent.com/u/39620?v=4"},"body":"Main changes since the initial version:\n\n * The first patch converts the existing hook callers to use the new\n find_hook() function.\n * Information about what is to be pushed is now sent over a pipe rather\n than passed as command-line parameters.\n\nAaron Schrab (3):\n hooks: Add function to check if a hook exists\n push: Add support for pre-push hooks\n Add sample pre-push hook script\n\n Documentation/githooks.txt | 29 +++++++++\n builtin/commit.c | 6 +-\n builtin/push.c | 1 +\n builtin/receive-pack.c | 25 ++++----\n run-command.c | 15 ++++-\n run-command.h | 1 +\n t/t5571-pre-push-hook.sh | 129 +++++++++++++++++++++++++++++++++++++++\n templates/hooks--pre-push.sample | 53 ++++++++++++++++\n transport.c | 60 ++++++++++++++++++\n transport.h | 1 +\n 10 files changed, 300 insertions(+), 20 deletions(-)\n create mode 100755 t/t5571-pre-push-hook.sh\n create mode 100644 templates/hooks--pre-push.sample\n\n-- \n1.8.1.340.g425b78d\n"},{"id":"206645","messageId":"1358054224-7710-2-git-send-email-aaron@schrab.com","threadId":"32473","inReplyTo":"1358054224-7710-1-git-send-email-aaron@schrab.com","subject":"[PATCH v2 1/3] hooks: Add function to check if a hook exists","fromName":"Aaron Schrab","fromEmail":"aaron@schrab.com","sentAt":"2013-01-13T05:17:02Z","receivedAt":"2013-01-13T05:17:02Z","isPatch":true,"sender":{"key":"aaron@schrab.com","avatar":"https://avatars.githubusercontent.com/u/39620?v=4"},"body":"Create find_hook() function to determine if a given hook exists and is\nexecutable. If it is, the path to the script will be returned,\notherwise NULL is returned.\n\nThis encapsulates the tests that are used to check for the existence of\na hook in one place, making it easier to modify those checks if that is\nfound to be necessary. This also makes it simple for places that can\nuse a hook to check if a hook exists before doing, possibly lengthy,\nsetup work which would be pointless if no such hook is present.\n\nThe returned value is left as a static value from get_pathname() rather\nthan a duplicate because it is anticipated that the return value will\neither be used as a boolean, immediately added to an argv_array list\nwhich would result in it being duplicated at that point, or used to\nactually run the command without much intervening work. Callers which\nneed to hold onto the returned value for a longer time are expected to\nduplicate the return value themselves.\n\nSigned-off-by: Aaron Schrab \n---\n builtin/commit.c | 6 ++----\n builtin/receive-pack.c | 25 +++++++++++--------------\n run-command.c | 15 +++++++++++++--\n run-command.h | 1 +\n 4 files changed, 27 insertions(+), 20 deletions(-)\n\ndiff --git a/builtin/commit.c b/builtin/commit.c\nindex d6dd3df..65d08d2 100644\n--- a/builtin/commit.c\n+++ b/builtin/commit.c\n@@ -1327,8 +1327,6 @@ static int git_commit_config(const char *k, const char *v, void *cb)\n \treturn git_status_config(k, v, s);\n }\n \n-static const char post_rewrite_hook[] = \"hooks/post-rewrite\";\n-\n static int run_rewrite_hook(const unsigned char *oldsha1,\n \t\t\t const unsigned char *newsha1)\n {\n@@ -1339,10 +1337,10 @@ static int run_rewrite_hook(const unsigned char *oldsha1,\n \tint code;\n \tsize_t n;\n \n-\tif (access(git_path(post_rewrite_hook), X_OK) < 0)\n+\targv[0] = find_hook(\"post-rewrite\");\n+\tif (!argv[0])\n \t\treturn 0;\n \n-\targv[0] = git_path(post_rewrite_hook);\n \targv[1] = \"amend\";\n \targv[2] = NULL;\n \ndiff --git a/builtin/receive-pack.c b/builtin/receive-pack.c\nindex ff781fe..e8878de 100644\n--- a/builtin/receive-pack.c\n+++ b/builtin/receive-pack.c\n@@ -182,9 +182,6 @@ struct command {\n \tchar ref_name[FLEX_ARRAY]; /* more */\n };\n \n-static const char pre_receive_hook[] = \"hooks/pre-receive\";\n-static const char post_receive_hook[] = \"hooks/post-receive\";\n-\n static void rp_error(const char *err, ...) __attribute__((format (printf, 1, 2)));\n static void rp_warning(const char *err, ...) __attribute__((format (printf, 1, 2)));\n \n@@ -242,10 +239,10 @@ static int run_and_feed_hook(const char *hook_name, feed_fn feed, void *feed_sta\n \tconst char *argv[2];\n \tint code;\n \n-\tif (access(hook_name, X_OK) < 0)\n+\targv[0] = find_hook(hook_name);\n+\tif (!argv[0])\n \t\treturn 0;\n \n-\targv[0] = hook_name;\n \targv[1] = NULL;\n \n \tmemset(&proc, 0, sizeof(proc));\n@@ -331,15 +328,14 @@ static int run_receive_hook(struct command *commands, const char *hook_name,\n \n static int run_update_hook(struct command *cmd)\n {\n-\tstatic const char update_hook[] = \"hooks/update\";\n \tconst char *argv[5];\n \tstruct child_process proc;\n \tint code;\n \n-\tif (access(update_hook, X_OK) < 0)\n+\targv[0] = find_hook(\"update\");\n+\tif (!argv[0])\n \t\treturn 0;\n \n-\targv[0] = update_hook;\n \targv[1] = cmd->ref_name;\n \targv[2] = sha1_to_hex(cmd->old_sha1);\n \targv[3] = sha1_to_hex(cmd->new_sha1);\n@@ -532,24 +528,25 @@ static const char *update(struct command *cmd)\n \t}\n }\n \n-static char update_post_hook[] = \"hooks/post-update\";\n-\n static void run_update_post_hook(struct command *commands)\n {\n \tstruct command *cmd;\n \tint argc;\n \tconst char **argv;\n \tstruct child_process proc;\n+\tchar *hook;\n \n+\thook = find_hook(\"post-update\");\n \tfor (argc = 0, cmd = commands; cmd; cmd = cmd->next) {\n \t\tif (cmd->error_string || cmd->did_not_exist)\n \t\t\tcontinue;\n \t\targc++;\n \t}\n-\tif (!argc || access(update_post_hook, X_OK) < 0)\n+\tif (!argc || !hook)\n \t\treturn;\n+\n \targv = xmalloc(sizeof(*argv) * (2 + argc));\n-\targv[0] = update_post_hook;\n+\targv[0] = hook;\n \n \tfor (argc = 1, cmd = commands; cmd; cmd = cmd->next) {\n \t\tchar *p;\n@@ -704,7 +701,7 @@ static void execute_commands(struct command *commands, const char *unpacker_erro\n \t\t\t\t 0, &cmd))\n \t\tset_connectivity_errors(commands);\n \n-\tif (run_receive_hook(commands, pre_receive_hook, 0)) {\n+\tif (run_receive_hook(commands, \"pre-receive\", 0)) {\n \t\tfor (cmd = commands; cmd; cmd = cmd->next) {\n \t\t\tif (!cmd->error_string)\n \t\t\t\tcmd->error_string = \"pre-receive hook declined\";\n@@ -994,7 +991,7 @@ int cmd_receive_pack(int argc, const char **argv, const char *prefix)\n \t\t\tunlink_or_warn(pack_lockfile);\n \t\tif (report_status)\n \t\t\treport(commands, unpack_status);\n-\t\trun_receive_hook(commands, post_receive_hook, 1);\n+\t\trun_receive_hook(commands, \"post-receive\", 1);\n \t\trun_update_post_hook(commands);\n \t\tif (auto_gc) {\n \t\t\tconst char *argv_gc_auto[] = {\ndiff --git a/run-command.c b/run-command.c\nindex 0471219..12d4ddb 100644\n--- a/run-command.c\n+++ b/run-command.c\n@@ -735,6 +735,15 @@ int finish_async(struct async *async)\n #endif\n }\n \n+char *find_hook(const char *name)\n+{\n+\tchar *path = git_path(\"hooks/%s\", name);\n+\tif (access(path, X_OK) < 0)\n+\t\tpath = NULL;\n+\n+\treturn path;\n+}\n+\n int run_hook(const char *index_file, const char *name, ...)\n {\n \tstruct child_process hook;\n@@ -744,11 +753,13 @@ int run_hook(const char *index_file, const char *name, ...)\n \tva_list args;\n \tint ret;\n \n-\tif (access(git_path(\"hooks/%s\", name), X_OK) < 0)\n+\tp = find_hook(name);\n+\tif (!p)\n \t\treturn 0;\n \n+\targv_array_push(&argv, p);\n+\n \tva_start(args, name);\n-\targv_array_push(&argv, git_path(\"hooks/%s\", name));\n \twhile ((p = va_arg(args, const char *)))\n \t\targv_array_push(&argv, p);\n \tva_end(args);\ndiff --git a/run-command.h b/run-command.h\nindex 850c638..221ce33 100644\n--- a/run-command.h\n+++ b/run-command.h\n@@ -45,6 +45,7 @@ int start_command(struct child_process *);\n int finish_command(struct child_process *);\n int run_command(struct child_process *);\n \n+extern char *find_hook(const char *name);\n extern int run_hook(const char *index_file, const char *name, ...);\n \n #define RUN_COMMAND_NO_STDIN 1\n-- \n1.8.1.340.g425b78d\n"},{"id":"206646","messageId":"1358054224-7710-3-git-send-email-aaron@schrab.com","threadId":"32473","inReplyTo":"1358054224-7710-1-git-send-email-aaron@schrab.com","subject":"[PATCH v2 2/3] push: Add support for pre-push hooks","fromName":"Aaron Schrab","fromEmail":"aaron@schrab.com","sentAt":"2013-01-13T05:17:03Z","receivedAt":"2013-01-13T05:17:03Z","isPatch":true,"sender":{"key":"aaron@schrab.com","avatar":"https://avatars.githubusercontent.com/u/39620?v=4"},"body":"Add support for a pre-push hook which can be used to determine if the\nset of refs to be pushed is suitable for the target repository. The\nhook is run with two arguments specifying the name and location of the\ndestination repository.\n\nInformation about what is to be pushed is provided by sending lines of\nthe following form to the hook's standard input:\n\n SP SP SP LF\n\nIf the hook exits with a non-zero status, the push will be aborted.\n\nThis will allow the script to determine if the push is acceptable based\non the target repository and branch(es), the commits which are to be\npushed, and even the source branches in some cases.\n\nSigned-off-by: Aaron Schrab \n---\n Documentation/githooks.txt | 29 ++++++++++\n builtin/push.c | 1 +\n t/t5571-pre-push-hook.sh | 129 +++++++++++++++++++++++++++++++++++++++++++++\n transport.c | 60 +++++++++++++++++++++\n transport.h | 1 +\n 5 files changed, 220 insertions(+)\n create mode 100755 t/t5571-pre-push-hook.sh\n\ndiff --git a/Documentation/githooks.txt b/Documentation/githooks.txt\nindex b9003fe..d839233 100644\n--- a/Documentation/githooks.txt\n+++ b/Documentation/githooks.txt\n@@ -176,6 +176,35 @@ save and restore any form of metadata associated with the working tree\n (eg: permissions/ownership, ACLS, etc). See contrib/hooks/setgitperms.perl\n for an example of how to do this.\n \n+pre-push\n+~~~~~~~~\n+\n+This hook is called by 'git push' and can be used to prevent a push from taking\n+place. The hook is called with two parameters which provide the name and\n+location of the destination remote, if a named remote is not being used both\n+values will be the same.\n+\n+Information about what is to be pushed is provided on the hook's standard\n+input with lines of the form:\n+\n+ SP SP SP LF\n+\n+For instance, if the command +git push origin master:foreign+ were run the\n+hook would receive a line like the following:\n+\n+ refs/heads/master 67890 refs/heads/foreign 12345\n+\n+although the full, 40-character SHA1s would be supplied. If the foreign ref\n+does not yet exist the `` will be 40 `0`. If a ref is to be\n+deleted, the `` will be supplied as `(delete)` and the `` will be 40 `0`. If the local commit was specified by something other\n+than a name which could be expanded (such as `HEAD~`, or a SHA1) it will be\n+supplied as it was originally given.\n+\n+If this hook exits with a non-zero status, 'git push' will abort without\n+pushing anything. Information about why the push is rejected may be sent\n+to the user by writing to standard error.\n+\n [[pre-receive]]\n pre-receive\n ~~~~~~~~~~~\ndiff --git a/builtin/push.c b/builtin/push.c\nindex 8491e43..b158028 100644\n--- a/builtin/push.c\n+++ b/builtin/push.c\n@@ -407,6 +407,7 @@ int cmd_push(int argc, const char **argv, const char *prefix)\n \t\tOPT_BOOL(0, \"progress\", &progress, N_(\"force progress reporting\")),\n \t\tOPT_BIT(0, \"prune\", &flags, N_(\"prune locally removed refs\"),\n \t\t\tTRANSPORT_PUSH_PRUNE),\n+\t\tOPT_BIT(0, \"no-verify\", &flags, N_(\"bypass pre-push hook\"), TRANSPORT_PUSH_NO_HOOK),\n \t\tOPT_END()\n \t};\n \ndiff --git a/t/t5571-pre-push-hook.sh b/t/t5571-pre-push-hook.sh\nnew file mode 100755\nindex 0000000..d68fed7\n--- /dev/null\n+++ b/t/t5571-pre-push-hook.sh\n@@ -0,0 +1,129 @@\n+#!/bin/sh\n+\n+test_description='check pre-push hooks'\n+. ./test-lib.sh\n+\n+# Setup hook that always succeeds\n+HOOKDIR=\"$(git rev-parse --git-dir)/hooks\"\n+HOOK=\"$HOOKDIR/pre-push\"\n+mkdir -p \"$HOOKDIR\"\n+write_script \"$HOOK\" <actual\n+echo \"$2\" >>actual\n+cat >>actual\n+EOF\n+\n+cat >expected <expected <expected <expected <expected <expected\n+\tnr=1000\n+\twhile test $nr -lt 2000\n+\tdo\n+\t\tnr=$(( $nr + 1 ))\n+\t\tgit branch b/$nr $COMMIT3\n+\t\techo \"refs/heads/b/$nr $COMMIT3 refs/heads/b/$nr $_z40\" >>expected\n+\tdone\n+\n+\ttest_expect_success 'push many refs' '\n+\t\tgit push parent1 \"refs/heads/b/*:refs/heads/b/*\" &&\n+\t\tdiff expected actual\n+\t'\n+fi\n+\n+test_done\ndiff --git a/transport.c b/transport.c\nindex 2673d27..0750a5f 100644\n--- a/transport.c\n+++ b/transport.c\n@@ -1034,6 +1034,62 @@ static void die_with_unpushed_submodules(struct string_list *needs_pushing)\n \tdie(\"Aborting.\");\n }\n \n+static int run_pre_push_hook(struct transport *transport,\n+\t\t\t struct ref *remote_refs)\n+{\n+\tint ret = 0, x;\n+\tstruct ref *r;\n+\tstruct child_process proc;\n+\tstruct strbuf buf;\n+\tconst char *argv[4];\n+\n+\tif (!(argv[0] = find_hook(\"pre-push\")))\n+\t\treturn 0;\n+\n+\targv[1] = transport->remote->name;\n+\targv[2] = transport->url;\n+\targv[3] = NULL;\n+\n+\tmemset(&proc, 0, sizeof(proc));\n+\tproc.argv = argv;\n+\tproc.in = -1;\n+\n+\tif (start_command(&proc)) {\n+\t\tfinish_command(&proc);\n+\t\treturn -1;\n+\t}\n+\n+\tstrbuf_init(&buf, 256);\n+\n+\tfor (r = remote_refs; r; r = r->next) {\n+\t\tif (!r->peer_ref) continue;\n+\t\tif (r->status == REF_STATUS_REJECT_NONFASTFORWARD) continue;\n+\t\tif (r->status == REF_STATUS_UPTODATE) continue;\n+\n+\t\tstrbuf_reset(&buf);\n+\t\tstrbuf_addf( &buf, \"%s %s %s %s\\n\",\n+\t\t\t r->peer_ref->name, sha1_to_hex(r->new_sha1),\n+\t\t\t r->name, sha1_to_hex(r->old_sha1));\n+\n+\t\tif (write_in_full(proc.in, buf.buf, buf.len) != buf.len) {\n+\t\t\tret = -1;\n+\t\t\tbreak;\n+\t\t}\n+\t}\n+\n+\tstrbuf_release(&buf);\n+\n+\tx = close(proc.in);\n+\tif (!ret)\n+\t\tret = x;\n+\n+\tx = finish_command(&proc);\n+\tif (!ret)\n+\t\tret = x;\n+\n+\treturn ret;\n+}\n+\n int transport_push(struct transport *transport,\n \t\t int refspec_nr, const char **refspec, int flags,\n \t\t unsigned int *reject_reasons)\n@@ -1074,6 +1130,10 @@ int transport_push(struct transport *transport,\n \t\t\tflags & TRANSPORT_PUSH_MIRROR,\n \t\t\tflags & TRANSPORT_PUSH_FORCE);\n \n+\t\tif (!(flags & TRANSPORT_PUSH_NO_HOOK))\n+\t\t\tif (run_pre_push_hook(transport, remote_refs))\n+\t\t\t\treturn -1;\n+\n \t\tif ((flags & TRANSPORT_RECURSE_SUBMODULES_ON_DEMAND) && !is_bare_repository()) {\n \t\t\tstruct ref *ref = remote_refs;\n \t\t\tfor (; ref; ref = ref->next)\ndiff --git a/transport.h b/transport.h\nindex bfd2df5..ac5a9f5 100644\n--- a/transport.h\n+++ b/transport.h\n@@ -104,6 +104,7 @@ struct transport {\n #define TRANSPORT_RECURSE_SUBMODULES_CHECK 64\n #define TRANSPORT_PUSH_PRUNE 128\n #define TRANSPORT_RECURSE_SUBMODULES_ON_DEMAND 256\n+#define TRANSPORT_PUSH_NO_HOOK 512\n \n #define TRANSPORT_SUMMARY_WIDTH (2 * DEFAULT_ABBREV + 3)\n #define TRANSPORT_SUMMARY(x) (int)(TRANSPORT_SUMMARY_WIDTH + strlen(x) - gettext_width(x)), (x)\n-- \n1.8.1.340.g425b78d\n"},{"id":"206647","messageId":"1358054224-7710-4-git-send-email-aaron@schrab.com","threadId":"32473","inReplyTo":"1358054224-7710-1-git-send-email-aaron@schrab.com","subject":"[PATCH v2 3/3] Add sample pre-push hook script","fromName":"Aaron Schrab","fromEmail":"aaron@schrab.com","sentAt":"2013-01-13T05:17:04Z","receivedAt":"2013-01-13T05:17:04Z","isPatch":true,"sender":{"key":"aaron@schrab.com","avatar":"https://avatars.githubusercontent.com/u/39620?v=4"},"body":"Create a sample of a script for a pre-push hook. The main purpose is to\nillustrate how a script may parse the information which is supplied to\nsuch a hook. The script may also be useful to some people as-is for\navoiding to push commits which are marked as a work in progress.\n\nSigned-off-by: Aaron Schrab \n---\n templates/hooks--pre-push.sample | 53 ++++++++++++++++++++++++++++++++++++++++\n 1 file changed, 53 insertions(+)\n create mode 100644 templates/hooks--pre-push.sample\n\ndiff --git a/templates/hooks--pre-push.sample b/templates/hooks--pre-push.sample\nnew file mode 100644\nindex 0000000..15ab6d8\n--- /dev/null\n+++ b/templates/hooks--pre-push.sample\n@@ -0,0 +1,53 @@\n+#!/bin/sh\n+\n+# An example hook script to verify what is about to be pushed. Called by \"git\n+# push\" after it has checked the remote status, but before anything has been\n+# pushed. If this script exits with a non-zero status nothing will be pushed.\n+#\n+# This hook is called with the following parameters:\n+#\n+# $1 -- Name of the remote to which the push is being done\n+# $2 -- URL to which the push is being done\n+#\n+# If pushing without using a named remote those arguments will be equal.\n+#\n+# Information about the commits which are being pushed is supplied as lines to\n+# the standard input in the form:\n+#\n+# \n+#\n+# This sample shows how to prevent push of commits where the log message starts\n+# with \"WIP\" (work in progress).\n+\n+remote=\"$1\"\n+url=\"$2\"\n+\n+z40=0000000000000000000000000000000000000000\n+\n+IFS=' '\n+while read local_ref local_sha remote_ref remote_sha\n+do\n+\tif [ \"$local_sha\" = $z40 ]\n+\tthen\n+\t\t# Handle delete\n+\telse\n+\t\tif [ \"$remote_sha\" = $z40 ]\n+\t\tthen\n+\t\t\t# New branch, examine all commits\n+\t\t\trange=\"$local_sha\"\n+\t\telse\n+\t\t\t# Update to existing branch, examine new commits\n+\t\t\trange=\"$remote_sha..$local_sha\"\n+\t\tfi\n+\n+\t\t# Check for WIP commit\n+\t\tcommit=`git rev-list -n 1 --grep '^WIP' \"$range\"`\n+\t\tif [ -n \"$commit\" ]\n+\t\tthen\n+\t\t\techo \"Found WIP commit in $local_ref, not pushing\"\n+\t\t\texit 1\n+\t\tfi\n+\tfi\n+done\n+\n+exit 0\n-- \n1.8.1.340.g425b78d\n"},{"id":"206813","messageId":"7vsj63ekab.fsf@alter.siamese.dyndns.org","threadId":"32473","inReplyTo":"1358054224-7710-3-git-send-email-aaron@schrab.com","subject":"Re: [PATCH v2 2/3] push: Add support for pre-push hooks","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-01-14T17:39:24Z","receivedAt":"2013-01-14T17:39:24Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Aaron Schrab writes:\n\n> Add support for a pre-push hook which can be used to determine if the\n> set of refs to be pushed is suitable for the target repository. The\n> hook is run with two arguments specifying the name and location of the\n> destination repository.\n>\n> Information about what is to be pushed is provided by sending lines of\n> the following form to the hook's standard input:\n>\n> SP SP SP LF\n>\n> If the hook exits with a non-zero status, the push will be aborted.\n>\n> This will allow the script to determine if the push is acceptable based\n> on the target repository and branch(es), the commits which are to be\n> pushed, and even the source branches in some cases.\n>\n> Signed-off-by: Aaron Schrab \n> ---\n> Documentation/githooks.txt | 29 ++++++++++\n> builtin/push.c | 1 +\n> t/t5571-pre-push-hook.sh | 129 +++++++++++++++++++++++++++++++++++++++++++++\n> transport.c | 60 +++++++++++++++++++++\n> transport.h | 1 +\n> 5 files changed, 220 insertions(+)\n> create mode 100755 t/t5571-pre-push-hook.sh\n>\n> diff --git a/Documentation/githooks.txt b/Documentation/githooks.txt\n> index b9003fe..d839233 100644\n> --- a/Documentation/githooks.txt\n> +++ b/Documentation/githooks.txt\n> @@ -176,6 +176,35 @@ save and restore any form of metadata associated with the working tree\n> (eg: permissions/ownership, ACLS, etc). See contrib/hooks/setgitperms.perl\n> for an example of how to do this.\n> \n> +pre-push\n> +~~~~~~~~\n> +\n> +This hook is called by 'git push' and can be used to prevent a push from taking\n> +place. The hook is called with two parameters which provide the name and\n> +location of the destination remote, if a named remote is not being used both\n> +values will be the same.\n> +\n> +Information about what is to be pushed is provided on the hook's standard\n> +input with lines of the form:\n> +\n> + SP SP SP LF\n> +\n> +For instance, if the command +git push origin master:foreign+ were run the\n\nJust being curious, but why use +monospace text+ here? Most of the\nnew text use `monospace text literally` instead in this patch.\n\n> +hook would receive a line like the following:\n> +\n> + refs/heads/master 67890 refs/heads/foreign 12345\n> +\n> +although the full, 40-character SHA1s would be supplied.\n\nPerhaps ellipses are called for here?\n\n refs/heads/master 67890... refs/heads/foreign 12345...\n\n (the above abbreviates full 40-hexdigits for illustration purposes only)\n\n> +If the foreign ref\n> +does not yet exist the `` will be 40 `0`. If a ref is to be\n> +deleted, the `` will be supplied as `(delete)` and the ` +SHA1>` will be 40 `0`. If the local commit was specified by something other\n> +than a name which could be expanded (such as `HEAD~`, or a SHA1) it will be\n> +supplied as it was originally given.\n> +\n> +If this hook exits with a non-zero status, 'git push' will abort without\n> +pushing anything. Information about why the push is rejected may be sent\n> +to the user by writing to standard error.\n\ns/standard error/& of the hook/; perhaps? It is unclear who does\nthe writing and it can be misunderstood that git-push will write to\nstandard error upon seeing your hook that silently exits.\n\n> diff --git a/builtin/push.c b/builtin/push.c\n> index 8491e43..b158028 100644\n> --- a/builtin/push.c\n> +++ b/builtin/push.c\n> @@ -407,6 +407,7 @@ int cmd_push(int argc, const char **argv, const char *prefix)\n> \t\tOPT_BOOL(0, \"progress\", &progress, N_(\"force progress reporting\")),\n> \t\tOPT_BIT(0, \"prune\", &flags, N_(\"prune locally removed refs\"),\n> \t\t\tTRANSPORT_PUSH_PRUNE),\n> +\t\tOPT_BIT(0, \"no-verify\", &flags, N_(\"bypass pre-push hook\"), TRANSPORT_PUSH_NO_HOOK),\n> \t\tOPT_END()\n> \t};\n\nSo to countermand this, you have to say --no-no-verify? Wouldn't it\nbe more natural to introduce a --verify option that turns the bit\non, which automatically gives you --no-verify to turn it off? A\nbit in a flag word can be initialized to true before the flag word\nis given to the parse_options() machinery to make the field default\nto true, no?\n"},{"id":"206814","messageId":"7vobgrek5m.fsf@alter.siamese.dyndns.org","threadId":"32473","inReplyTo":"1358054224-7710-4-git-send-email-aaron@schrab.com","subject":"Re: [PATCH v2 3/3] Add sample pre-push hook script","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-01-14T17:42:13Z","receivedAt":"2013-01-14T17:42:13Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Aaron Schrab writes:\n\n> Create a sample of a script for a pre-push hook. The main purpose is to\n> illustrate how a script may parse the information which is supplied to\n> such a hook. The script may also be useful to some people as-is for\n> avoiding to push commits which are marked as a work in progress.\n>\n> Signed-off-by: Aaron Schrab \n> ---\n> templates/hooks--pre-push.sample | 53 ++++++++++++++++++++++++++++++++++++++++\n> 1 file changed, 53 insertions(+)\n> create mode 100644 templates/hooks--pre-push.sample\n>\n> diff --git a/templates/hooks--pre-push.sample b/templates/hooks--pre-push.sample\n> new file mode 100644\n> index 0000000..15ab6d8\n> --- /dev/null\n> +++ b/templates/hooks--pre-push.sample\n> @@ -0,0 +1,53 @@\n> +#!/bin/sh\n> +\n> +# An example hook script to verify what is about to be pushed. Called by \"git\n> +# push\" after it has checked the remote status, but before anything has been\n> +# pushed. If this script exits with a non-zero status nothing will be pushed.\n> +#\n> +# This hook is called with the following parameters:\n> +#\n> +# $1 -- Name of the remote to which the push is being done\n> +# $2 -- URL to which the push is being done\n> +#\n> +# If pushing without using a named remote those arguments will be equal.\n> +#\n> +# Information about the commits which are being pushed is supplied as lines to\n> +# the standard input in the form:\n> +#\n> +# \n> +#\n> +# This sample shows how to prevent push of commits where the log message starts\n> +# with \"WIP\" (work in progress).\n\nAn example for a plausible use case is nice to have. I would prefer\nto see any new shell script to follow the Git style, though.\n\n> +remote=\"$1\"\n> +url=\"$2\"\n> +\n> +z40=0000000000000000000000000000000000000000\n> +\n> +IFS=' '\n> +while read local_ref local_sha remote_ref remote_sha\n> +do\n> +\tif [ \"$local_sha\" = $z40 ]\n> +\tthen\n> +\t\t# Handle delete\n\n ... by doing what?\n\n> +\telse\n> +\t\tif [ \"$remote_sha\" = $z40 ]\n> +\t\tthen\n> +\t\t\t# New branch, examine all commits\n> +\t\t\trange=\"$local_sha\"\n> +\t\telse\n> +\t\t\t# Update to existing branch, examine new commits\n> +\t\t\trange=\"$remote_sha..$local_sha\"\n> +\t\tfi\n> +\n> +\t\t# Check for WIP commit\n> +\t\tcommit=`git rev-list -n 1 --grep '^WIP' \"$range\"`\n> +\t\tif [ -n \"$commit\" ]\n> +\t\tthen\n> +\t\t\techo \"Found WIP commit in $local_ref, not pushing\"\n> +\t\t\texit 1\n> +\t\tfi\n> +\tfi\n> +done\n> +\n> +exit 0\n"},{"id":"206815","messageId":"7vk3rfek51.fsf@alter.siamese.dyndns.org","threadId":"32473","inReplyTo":"1358054224-7710-1-git-send-email-aaron@schrab.com","subject":"Re: [PATCH v2 0/3] pre-push hook support","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-01-14T17:42:34Z","receivedAt":"2013-01-14T17:42:34Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Aaron Schrab writes:\n\n> Main changes since the initial version:\n>\n> * The first patch converts the existing hook callers to use the new\n> find_hook() function.\n> * Information about what is to be pushed is now sent over a pipe rather\n> than passed as command-line parameters.\n>\n> Aaron Schrab (3):\n> hooks: Add function to check if a hook exists\n> push: Add support for pre-push hooks\n> Add sample pre-push hook script\n\nGetting much nicer. Thanks.\n"},{"id":"206849","messageId":"7va9sb8jg7.fsf@alter.siamese.dyndns.org","threadId":"32473","inReplyTo":"7vk3rfek51.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH v2 0/3] pre-push hook support","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-01-14T22:54:00Z","receivedAt":"2013-01-14T22:54:00Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Junio C Hamano writes:\n\n> Aaron Schrab writes:\n>\n>> Main changes since the initial version:\n>>\n>> * The first patch converts the existing hook callers to use the new\n>> find_hook() function.\n>> * Information about what is to be pushed is now sent over a pipe rather\n>> than passed as command-line parameters.\n>>\n>> Aaron Schrab (3):\n>> hooks: Add function to check if a hook exists\n>> push: Add support for pre-push hooks\n>> Add sample pre-push hook script\n>\n> Getting much nicer. Thanks.\n\nHmph, t5571 seems to be flaky in that it sometimes fails but passes\nwhen run again. Something timing dependent is going on???\n"},{"id":"206857","messageId":"7vpq1770pr.fsf@alter.siamese.dyndns.org","threadId":"32473","inReplyTo":"7va9sb8jg7.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH v2 0/3] pre-push hook support","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-01-15T00:24:00Z","receivedAt":"2013-01-15T00:24:00Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Junio C Hamano writes:\n\n> Junio C Hamano writes:\n>\n>> Aaron Schrab writes:\n>>\n>>> Main changes since the initial version:\n>>>\n>>> * The first patch converts the existing hook callers to use the new\n>>> find_hook() function.\n>>> * Information about what is to be pushed is now sent over a pipe rather\n>>> than passed as command-line parameters.\n>>>\n>>> Aaron Schrab (3):\n>>> hooks: Add function to check if a hook exists\n>>> push: Add support for pre-push hooks\n>>> Add sample pre-push hook script\n>>\n>> Getting much nicer. Thanks.\n>\n> Hmph, t5571 seems to be flaky in that it sometimes fails but passes\n> when run again. Something timing dependent is going on???\n\nWith this patch applied, repeatedly try to\n\n - make sure \"foreign\" ref does not exist; and\n - attempt pushing the HEAD:foreign to create the \"foreign\" ref\n\nuntil it fails, I can get it stop before the output scrolls off of\nmy 114 line terminal. Then when I revert the changes to transport.[ch]\nand builtin/push.c in this series, the test will keep going.\n\nWait. The sample hook used in the test _is_ fed some input but it\nexits without reading any. What happens when we fork it, and it\ncompletes execution before we even have a chance to feed a single\nbyte? Wont' we get a sigpipe and die?\n\nYup, I think that is what is missing from run_pre_push_hook()\nimplementation.\n\ndiff --git a/t/t5571-pre-push-hook.sh b/t/t5571-pre-push-hook.sh\nindex d68fed7..050318b 100755\n--- a/t/t5571-pre-push-hook.sh\n+++ b/t/t5571-pre-push-hook.sh\n@@ -16,8 +16,15 @@ test_expect_success 'setup' '\n \tgit init --bare repo1 &&\n \tgit remote add parent1 repo1 &&\n \ttest_commit one &&\n-\tgit push parent1 HEAD:foreign\n+\twhile :\n+\tdo\n+\t\tgit push parent1 :refs/heads/foreign &&\n+\t\tgit push parent1 HEAD:foreign || break\n+\tdone\n '\n+\n+exit\n+\n write_script \"$HOOK\" < writes:\n\n> t/t5571-pre-push-hook.sh | 129 +++++++++++++++++++++++++++++++++++++++++++++\n> diff --git a/t/t5571-pre-push-hook.sh b/t/t5571-pre-push-hook.sh\n> new file mode 100755\n> index 0000000..d68fed7\n> --- /dev/null\n> +++ b/t/t5571-pre-push-hook.sh\n> @@ -0,0 +1,129 @@\n> +#!/bin/sh\n> +\n> +test_description='check pre-push hooks'\n> +. ./test-lib.sh\n> +\n> +# Setup hook that always succeeds\n> +HOOKDIR=\"$(git rev-parse --git-dir)/hooks\"\n> +HOOK=\"$HOOKDIR/pre-push\"\n> +mkdir -p \"$HOOKDIR\"\n> +write_script \"$HOOK\" < +exit 0\n> +EOF\n\nAs this script is expected to read from the pipe, if this exits\nbefore the parent has a chance to write to the pipe, the parent can\nbe killed with sigpipe.\n\nAt least the attached patch is necessary.\n\nIn the longer term, we may want to discuss what should happen when\nthe hook exited without even reading what we fed. My gut feeling is\nthat we can still trust its exit status (a hook that was badly coded\nso it wanted to read from us and use that information to decide but\nsomehow died before fully reading from us is not likely to exit with\nzero status, so we wouldn't diagnosing breakage as a success), but\nthere may be downsides for being that lax.\n\nIf we decide we want to be lax, then the call site of this hook and\nthe pre-receive hook (is there any other \"take info from the\nstandard input\" hook?) need to be modified so that they ignore\nsigpipe, I think.\n\nThere was a related discussion around this issue about a year ago.\n\nhttp://thread.gmane.org/gmane.comp.version-control.git/180346/focus=186291\n\n\n t/t5571-pre-push-hook.sh | 3 +++\n 1 file changed, 3 insertions(+)\n\ndiff --git a/t/t5571-pre-push-hook.sh b/t/t5571-pre-push-hook.sh\nindex d68fed7..577d252 100755\n--- a/t/t5571-pre-push-hook.sh\n+++ b/t/t5571-pre-push-hook.sh\n@@ -8,6 +8,7 @@ HOOKDIR=\"$(git rev-parse --git-dir)/hooks\"\n HOOK=\"$HOOKDIR/pre-push\"\n mkdir -p \"$HOOKDIR\"\n write_script \"$HOOK\" </dev/null\n exit 0\n EOF\n \n@@ -19,6 +20,7 @@ test_expect_success 'setup' '\n \tgit push parent1 HEAD:foreign\n '\n write_script \"$HOOK\" </dev/null\n exit 1\n EOF\n \n@@ -38,6 +40,7 @@ COMMIT2=\"$(git rev-parse HEAD)\"\n export COMMIT2\n \n write_script \"$HOOK\" <<'EOF'\n+cat >/dev/null\n echo \"$1\" >actual\n echo \"$2\" >>actual\n cat >>actual\n"},{"id":"206881","messageId":"7vtxqj5ed0.fsf@alter.siamese.dyndns.org","threadId":"32473","inReplyTo":"7vip6z7056.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH v2 2/3] push: Add support for pre-push hooks","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-01-15T03:12:11Z","receivedAt":"2013-01-15T03:12:11Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Junio C Hamano writes:\n\n> At least the attached patch is necessary.\n\nSorry, but the last hunk (see below) is not. It breaks the hook.\n\n> In the longer term, we may want to discuss what should happen when\n> the hook exited without even reading what we fed. My gut feeling is\n> that we can still trust its exit status (a hook that was badly coded\n> so it wanted to read from us and use that information to decide but\n> somehow died before fully reading from us is not likely to exit with\n> zero status, so we wouldn't diagnosing breakage as a success), but\n> there may be downsides for being that lax.\n>\n> If we decide we want to be lax, then the call site of this hook and\n> the pre-receive hook (is there any other \"take info from the\n> standard input\" hook?) need to be modified so that they ignore\n> sigpipe, I think.\n>\n> There was a related discussion around this issue about a year ago.\n>\n> http://thread.gmane.org/gmane.comp.version-control.git/180346/focus=186291\n> ...\n>\n\n> @@ -38,6 +40,7 @@ COMMIT2=\"$(git rev-parse HEAD)\"\n> export COMMIT2\n> \n> write_script \"$HOOK\" <<'EOF'\n> +cat >/dev/null\n> echo \"$1\" >actual\n> echo \"$2\" >>actual\n> cat >>actual\n\nAs this one wants to keep the incoming data to \"actual\", we do not\nwant the extra \"cat\" to slurp everything in. Sorry for not being\ncareful.\n"}]}