{"thread":{"id":"41292","subject":"[PATCH 0/3] Propagating push options to remote hooks","startedAt":"2016-01-30T18:28:07Z","lastAt":"2016-01-30T18:28:10Z","messageCount":4,"participants":["Dennis Kaarsemaker"],"isPatch":true,"patchVersion":1,"patchTotal":3},"messages":[{"id":"277095","messageId":"1454178490-17873-1-git-send-email-dennis@kaarsemaker.net","threadId":"41292","inReplyTo":null,"subject":"[PATCH 0/3] Propagating push options to remote hooks","fromName":"Dennis Kaarsemaker","fromEmail":"dennis@kaarsemaker.net","sentAt":"2016-01-30T18:28:07Z","receivedAt":"2016-01-30T18:28:07Z","isPatch":true,"sender":{"key":"dennis@kaarsemaker.net","avatar":"https://avatars.githubusercontent.com/u/200649?v=4"},"body":"I have a few pre-receive hooks that are meant to catch mistakes. They are\nfairly strict, as the mistakes it catches can have some serious bad effects.\nHowever, sometimes they get it wrong (and can't really get it right) and it\nwould be really useful to override them.\n\nCurrently I do this by parseing the commit message, looking for 'Force: true',\nbut it would be very useful if --force were propagated to the hook. Obviously,\nmaking --force skip all remote hooks would be a very bad way of doing this.\nHooks should decide whether --force is respected or not.\n\nInstead of that, we can pass options to receive-pack using a new capability,\nand receive-pack can make it available to hooks in their environment. That way\nwe don't change behaviour of existing hooks and each hook can decide for itself\nwhether it respects these options.\n\nThe initial implementation only passes on --quiet and --force. I've been\nthinking of allowing the user of push to specify arbitrary values, but don't\nsee the value of that yet. It would be easy to add though.\n\nDennis Kaarsemaker (3):\n connect.[ch]: make parse_feature_value non-static\n receive-pack: add a capability for hook options\n send-pack: propagate --force and --quiet to remote hooks\n\n Documentation/technical/protocol-capabilities.txt | 9 ++++++\n builtin/receive-pack.c | 19 ++++++++++--\n connect.c | 3 +-\n connect.h | 1 +\n send-pack.c | 10 ++++++\n t/t5544-push-hook-options.sh | 37 +++++++++++++++++++++++\n 6 files changed, 75 insertions(+), 4 deletions(-)\n create mode 100755 t/t5544-push-hook-options.sh\n\n-- \n2.7.0-91-gf04ef09\n"},{"id":"277096","messageId":"1454178490-17873-2-git-send-email-dennis@kaarsemaker.net","threadId":"41292","inReplyTo":"1454178490-17873-1-git-send-email-dennis@kaarsemaker.net","subject":"[PATCH 1/3] connect.[ch]: make parse_feature_value non-static","fromName":"Dennis Kaarsemaker","fromEmail":"dennis@kaarsemaker.net","sentAt":"2016-01-30T18:28:08Z","receivedAt":"2016-01-30T18:28:08Z","isPatch":true,"sender":{"key":"dennis@kaarsemaker.net","avatar":"https://avatars.githubusercontent.com/u/200649?v=4"},"body":"We'll need it in the next patch.\n\nSigned-off-by: Dennis Kaarsemaker \n---\n connect.c | 3 +--\n connect.h | 1 +\n 2 files changed, 2 insertions(+), 2 deletions(-)\n\ndiff --git a/connect.c b/connect.c\nindex fd7ffe1..9e64b0b 100644\n--- a/connect.c\n+++ b/connect.c\n@@ -12,7 +12,6 @@\n #include \"transport.h\"\n \n static char *server_capabilities;\n-static const char *parse_feature_value(const char *, const char *, int *);\n \n static int check_ref(const char *name, unsigned int flags)\n {\n@@ -179,7 +178,7 @@ struct ref **get_remote_heads(int in, char *src_buf, size_t src_len,\n \treturn list;\n }\n \n-static const char *parse_feature_value(const char *feature_list, const char *feature, int *lenp)\n+const char *parse_feature_value(const char *feature_list, const char *feature, int *lenp)\n {\n \tint len;\n \ndiff --git a/connect.h b/connect.h\nindex c41a685..7daf702 100644\n--- a/connect.h\n+++ b/connect.h\n@@ -9,6 +9,7 @@ extern int git_connection_is_socket(struct child_process *conn);\n extern int server_supports(const char *feature);\n extern int parse_feature_request(const char *features, const char *feature);\n extern const char *server_feature_value(const char *feature, int *len_ret);\n+extern const char *parse_feature_value(const char *, const char *, int *);\n extern int url_is_local_not_ssh(const char *url);\n \n #endif\n-- \n2.7.0-91-gf04ef09\n"},{"id":"277098","messageId":"1454178490-17873-3-git-send-email-dennis@kaarsemaker.net","threadId":"41292","inReplyTo":"1454178490-17873-1-git-send-email-dennis@kaarsemaker.net","subject":"[PATCH 2/3] receive-pack: add a capability for hook options","fromName":"Dennis Kaarsemaker","fromEmail":"dennis@kaarsemaker.net","sentAt":"2016-01-30T18:28:09Z","receivedAt":"2016-01-30T18:28:09Z","isPatch":true,"sender":{"key":"dennis@kaarsemaker.net","avatar":"https://avatars.githubusercontent.com/u/200649?v=4"},"body":"Allow the client to specify options to influence the behaviour of hooks\nrun by receive-pack. This can be used to e.g. tell hooks to be quiet or\nverbose, or to ignore errors.\n\nThese options are passed on to the hooks in the environment variable\nGIT_HOOK_OPTIONS, which hooks can choose to respect to or ignore. The\ndefault hooks do not respect these options.\n\nSigned-off-by: Dennis Kaarsemaker \n---\n Documentation/technical/protocol-capabilities.txt | 9 +++++++++\n builtin/receive-pack.c | 19 +++++++++++++++++--\n 2 files changed, 26 insertions(+), 2 deletions(-)\n\ndiff --git a/Documentation/technical/protocol-capabilities.txt b/Documentation/technical/protocol-capabilities.txt\nindex eaab6b4..dea47e6 100644\n--- a/Documentation/technical/protocol-capabilities.txt\n+++ b/Documentation/technical/protocol-capabilities.txt\n@@ -275,3 +275,12 @@ to accept a signed push certificate, and asks the to be\n included in the push certificate. A send-pack client MUST NOT\n send a push-cert packet unless the receive-pack server advertises\n this capability.\n+\n+hook-options\n+------------\n+\n+The receive-pack server that advertises this capability can accept hook\n+options in the capabilities sent by the client. The hook options string is\n+a string of characters in the set [0-9a-zA-Z,=_] and is made available to\n+all hooks executed by the receive pack process as environment variable\n+GIT_HOOK_OPTIONS\ndiff --git a/builtin/receive-pack.c b/builtin/receive-pack.c\nindex f2d6761..120d9b3 100644\n--- a/builtin/receive-pack.c\n+++ b/builtin/receive-pack.c\n@@ -73,6 +73,8 @@ static long nonce_stamp_slop;\n static unsigned long nonce_stamp_slop_limit;\n static struct ref_transaction *transaction;\n \n+static const char *hook_options;\n+\n static enum deny_action parse_deny_action(const char *var, const char *value)\n {\n \tif (value) {\n@@ -201,7 +203,7 @@ static void show_ref(const char *path, const unsigned char *sha1)\n \t\tstruct strbuf cap = STRBUF_INIT;\n \n \t\tstrbuf_addstr(&cap,\n-\t\t\t \"report-status delete-refs side-band-64k quiet\");\n+\t\t\t \"report-status delete-refs side-band-64k quiet hook-options\");\n \t\tif (advertise_atomic_push)\n \t\t\tstrbuf_addstr(&cap, \" atomic\");\n \t\tif (prefer_ofs_delta)\n@@ -561,6 +563,9 @@ static int run_and_feed_hook(const char *hook_name, feed_fn feed, void *feed_sta\n \n \targv[1] = NULL;\n \n+\tif (hook_options)\n+\t\targv_array_pushf(&proc.env_array, \"GIT_HOOK_OPTIONS=%s\", hook_options);\n+\n \tproc.argv = argv;\n \tproc.in = -1;\n \tproc.stdout_to_stderr = 1;\n@@ -663,6 +668,9 @@ static int run_update_hook(struct command *cmd)\n \targv[3] = sha1_to_hex(cmd->new_sha1);\n \targv[4] = NULL;\n \n+\tif (hook_options)\n+\t\targv_array_pushf(&proc.env_array, \"GIT_HOOK_OPTIONS=%s\", hook_options);\n+\n \tproc.no_stdin = 1;\n \tproc.stdout_to_stderr = 1;\n \tproc.err = use_sideband ? -1 : 0;\n@@ -1055,6 +1063,9 @@ static void run_update_post_hook(struct command *commands)\n \t}\n \targv[argc] = NULL;\n \n+\tif (hook_options)\n+\t\targv_array_pushf(&proc.env_array, \"GIT_HOOK_OPTIONS=%s\", hook_options);\n+\n \tproc.no_stdin = 1;\n \tproc.stdout_to_stderr = 1;\n \tproc.err = use_sideband ? -1 : 0;\n@@ -1415,7 +1426,8 @@ static struct command *read_head_info(struct sha1_array *shallow)\n \tstruct command **p = &commands;\n \tfor (;;) {\n \t\tchar *line;\n-\t\tint len, linelen;\n+\t\tconst char *feature;\n+\t\tint len, linelen, featurelen;\n \n \t\tline = packet_read_line(0, &len);\n \t\tif (!line)\n@@ -1442,6 +1454,9 @@ static struct command *read_head_info(struct sha1_array *shallow)\n \t\t\tif (advertise_atomic_push\n \t\t\t && parse_feature_request(feature_list, \"atomic\"))\n \t\t\t\tuse_atomic = 1;\n+\t\t\tif ((feature =\n+\t\t\t\tparse_feature_value(feature_list, \"hook-options\", &featurelen)))\n+\t\t\t\thook_options = xmemdupz(feature, featurelen);\n \t\t}\n \n \t\tif (!strcmp(line, \"push-cert\")) {\n-- \n2.7.0-91-gf04ef09\n"},{"id":"277097","messageId":"1454178490-17873-4-git-send-email-dennis@kaarsemaker.net","threadId":"41292","inReplyTo":"1454178490-17873-1-git-send-email-dennis@kaarsemaker.net","subject":"[PATCH 3/3] send-pack: propagate --force and --quiet to remote hooks","fromName":"Dennis Kaarsemaker","fromEmail":"dennis@kaarsemaker.net","sentAt":"2016-01-30T18:28:10Z","receivedAt":"2016-01-30T18:28:10Z","isPatch":true,"sender":{"key":"dennis@kaarsemaker.net","avatar":"https://avatars.githubusercontent.com/u/200649?v=4"},"body":"When a server supports hook options, we send it options for quiet and\nforce if the user used push --force/--quiet.\n\nSigned-off-by: Dennis Kaarsemaker \n---\n send-pack.c | 10 ++++++++++\n t/t5544-push-hook-options.sh | 37 +++++++++++++++++++++++++++++++++++++\n 2 files changed, 47 insertions(+)\n create mode 100755 t/t5544-push-hook-options.sh\n\ndiff --git a/send-pack.c b/send-pack.c\nindex 047bd18..5630327 100644\n--- a/send-pack.c\n+++ b/send-pack.c\n@@ -371,6 +371,8 @@ int send_pack(struct send_pack_args *args,\n \tint agent_supported = 0;\n \tint use_atomic = 0;\n \tint atomic_supported = 0;\n+\tint hook_options_supported = 0;\n+\tint hook_options_seen = 0;\n \tunsigned cmds_sent = 0;\n \tint ret;\n \tstruct async demux;\n@@ -393,6 +395,8 @@ int send_pack(struct send_pack_args *args,\n \t\targs->use_thin_pack = 0;\n \tif (server_supports(\"atomic\"))\n \t\tatomic_supported = 1;\n+\tif (server_supports(\"hook-options\"))\n+\t\thook_options_supported = 1;\n \n \tif (args->push_cert != SEND_PACK_PUSH_CERT_NEVER) {\n \t\tint len;\n@@ -429,6 +433,12 @@ int send_pack(struct send_pack_args *args,\n \t\tstrbuf_addstr(&cap_buf, \" atomic\");\n \tif (agent_supported)\n \t\tstrbuf_addf(&cap_buf, \" agent=%s\", git_user_agent_sanitized());\n+\tif (hook_options_supported) {\n+\t\tif (args->quiet)\n+\t\t\tstrbuf_addf(&cap_buf, \"%squiet\", hook_options_seen++ ? \",\" : \" hook-options=\");\n+\t\tif (args->force_update)\n+\t\t\tstrbuf_addf(&cap_buf, \"%sforce\", hook_options_seen++ ? \",\" : \" hook-options=\");\n+\t}\n \n \t/*\n \t * NEEDSWORK: why does delete-refs have to be so specific to\ndiff --git a/t/t5544-push-hook-options.sh b/t/t5544-push-hook-options.sh\nnew file mode 100755\nindex 0000000..6d52ad1\n--- /dev/null\n+++ b/t/t5544-push-hook-options.sh\n@@ -0,0 +1,37 @@\n+#!/bin/sh\n+\n+test_description='pushing to a repository with hook options'\n+\n+. ./test-lib.sh\n+\n+test_expect_success 'hook options are passed on to hooks' '\n+\tgit init --bare remote.git &&\n+\twrite_script remote.git/hooks/post-receive <<-\\EOF &&\n+\techo \"post-receive-hook\"\n+\techo $GIT_HOOK_OPTIONS\n+\tEOF\n+\t(\n+\t\ttest_commit one &&\n+\t\tgit push remote/ master:test 2> actual &&\n+\t\ttest_commit two &&\n+\t\tgit push --quiet remote/ master:test 2>> actual &&\n+\t\ttest_commit three &&\n+\t\tgit push --force remote/ master:test 2>> actual &&\n+\t\ttest_commit four &&\n+\t\tgit push --quiet --force remote/ master:test 2>> actual\n+\t) &&\n+\tsed -ne \"s/remote: \\([^ ]*\\).*/\\1/p\" -i actual &&\n+\tcat > expected <<-\\EOF &&\n+\tpost-receive-hook\n+\n+\tpost-receive-hook\n+\tquiet\n+\tpost-receive-hook\n+\tforce\n+\tpost-receive-hook\n+\tquiet,force\n+\tEOF\n+\ttest_cmp expected actual\n+'\n+\n+test_done\n-- \n2.7.0-91-gf04ef09\n"}]}