[PATCH 0/3] Propagating push options to remote hooks

[PATCH 0/3] Propagating push options to remote hooks

[Dennis Kaarsemaker]

I have a few pre-receive hooks that are meant to catch mistakes. They are
fairly strict, as the mistakes it catches can have some serious bad effects.
However, sometimes they get it wrong (and can't really get it right) and it
would be really useful to override them.

Currently I do this by parseing the commit message, looking for 'Force: true',
but it would be very useful if --force were propagated to the hook. Obviously,
making --force skip all remote hooks would be a very bad way of doing this.
Hooks should decide whether --force is respected or not.

Instead of that, we can pass options to receive-pack using a new capability,
and receive-pack can make it available to hooks in their environment. That way
we don't change behaviour of existing hooks and each hook can decide for itself
whether it respects these options.

The initial implementation only passes on --quiet and --force. I've been
thinking of allowing the user of push to specify arbitrary values, but don't
see the value of that yet. It would be easy to add though.

Dennis Kaarsemaker (3):
  connect.[ch]: make parse_feature_value non-static
  receive-pack: add a capability for hook options
  send-pack: propagate --force and --quiet to remote hooks

 Documentation/technical/protocol-capabilities.txt |  9 ++++++
 builtin/receive-pack.c                            | 19 ++++++++++--
 connect.c                                         |  3 +-
 connect.h                                         |  1 +
 send-pack.c                                       | 10 ++++++
 t/t5544-push-hook-options.sh                      | 37 +++++++++++++++++++++++
 6 files changed, 75 insertions(+), 4 deletions(-)
 create mode 100755 t/t5544-push-hook-options.sh

-- 
2.7.0-91-gf04ef09

[PATCH 1/3] connect.[ch]: make parse_feature_value non-static

[Dennis Kaarsemaker]

We'll need it in the next patch.

Signed-off-by: Dennis Kaarsemaker <git@vger.kernel.org>
---
 connect.c | 3 +--
 connect.h | 1 +
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/connect.c b/connect.c
index fd7ffe1..9e64b0b 100644
--- a/connect.c
+++ b/connect.c
@@ -12,7 +12,6 @@
 #include "transport.h"
 
 static char *server_capabilities;
-static const char *parse_feature_value(const char *, const char *, int *);
 
 static int check_ref(const char *name, unsigned int flags)
 {
@@ -179,7 +178,7 @@ struct ref **get_remote_heads(int in, char *src_buf, size_t src_len,
 	return list;
 }
 
-static const char *parse_feature_value(const char *feature_list, const char *feature, int *lenp)
+const char *parse_feature_value(const char *feature_list, const char *feature, int *lenp)
 {
 	int len;
 
diff --git a/connect.h b/connect.h
index c41a685..7daf702 100644
--- a/connect.h
+++ b/connect.h
@@ -9,6 +9,7 @@ extern int git_connection_is_socket(struct child_process *conn);
 extern int server_supports(const char *feature);
 extern int parse_feature_request(const char *features, const char *feature);
 extern const char *server_feature_value(const char *feature, int *len_ret);
+extern const char *parse_feature_value(const char *, const char *, int *);
 extern int url_is_local_not_ssh(const char *url);
 
 #endif
-- 
2.7.0-91-gf04ef09

[PATCH 2/3] receive-pack: add a capability for hook options

[Dennis Kaarsemaker]

Allow the client to specify options to influence the behaviour of hooks
run by receive-pack. This can be used to e.g. tell hooks to be quiet or
verbose, or to ignore errors.

These options are passed on to the hooks in the environment variable
GIT_HOOK_OPTIONS, which hooks can choose to respect to or ignore. The
default hooks do not respect these options.

Signed-off-by: Dennis Kaarsemaker <git@vger.kernel.org>
---
 Documentation/technical/protocol-capabilities.txt |  9 +++++++++
 builtin/receive-pack.c                            | 19 +++++++++++++++++--
 2 files changed, 26 insertions(+), 2 deletions(-)

diff --git a/Documentation/technical/protocol-capabilities.txt b/Documentation/technical/protocol-capabilities.txt
index eaab6b4..dea47e6 100644
--- a/Documentation/technical/protocol-capabilities.txt
+++ b/Documentation/technical/protocol-capabilities.txt
@@ -275,3 +275,12 @@ to accept a signed push certificate, and asks the <nonce> to be
 included in the push certificate.  A send-pack client MUST NOT
 send a push-cert packet unless the receive-pack server advertises
 this capability.
+
+hook-options
+------------
+
+The receive-pack server that advertises this capability can accept hook
+options in the capabilities sent by the client. The hook options string is
+a string of characters in the set [0-9a-zA-Z,=_] and is made available to
+all hooks executed by the receive pack process as environment variable
+GIT_HOOK_OPTIONS
diff --git a/builtin/receive-pack.c b/builtin/receive-pack.c
index f2d6761..120d9b3 100644
--- a/builtin/receive-pack.c
+++ b/builtin/receive-pack.c
@@ -73,6 +73,8 @@ static long nonce_stamp_slop;
 static unsigned long nonce_stamp_slop_limit;
 static struct ref_transaction *transaction;
 
+static const char *hook_options;
+
 static enum deny_action parse_deny_action(const char *var, const char *value)
 {
 	if (value) {
@@ -201,7 +203,7 @@ static void show_ref(const char *path, const unsigned char *sha1)
 		struct strbuf cap = STRBUF_INIT;
 
 		strbuf_addstr(&cap,
-			      "report-status delete-refs side-band-64k quiet");
+			      "report-status delete-refs side-band-64k quiet hook-options");
 		if (advertise_atomic_push)
 			strbuf_addstr(&cap, " atomic");
 		if (prefer_ofs_delta)
@@ -561,6 +563,9 @@ static int run_and_feed_hook(const char *hook_name, feed_fn feed, void *feed_sta
 
 	argv[1] = NULL;
 
+	if (hook_options)
+		argv_array_pushf(&proc.env_array, "GIT_HOOK_OPTIONS=%s", hook_options);
+
 	proc.argv = argv;
 	proc.in = -1;
 	proc.stdout_to_stderr = 1;
@@ -663,6 +668,9 @@ static int run_update_hook(struct command *cmd)
 	argv[3] = sha1_to_hex(cmd->new_sha1);
 	argv[4] = NULL;
 
+	if (hook_options)
+		argv_array_pushf(&proc.env_array, "GIT_HOOK_OPTIONS=%s", hook_options);
+
 	proc.no_stdin = 1;
 	proc.stdout_to_stderr = 1;
 	proc.err = use_sideband ? -1 : 0;
@@ -1055,6 +1063,9 @@ static void run_update_post_hook(struct command *commands)
 	}
 	argv[argc] = NULL;
 
+	if (hook_options)
+		argv_array_pushf(&proc.env_array, "GIT_HOOK_OPTIONS=%s", hook_options);
+
 	proc.no_stdin = 1;
 	proc.stdout_to_stderr = 1;
 	proc.err = use_sideband ? -1 : 0;
@@ -1415,7 +1426,8 @@ static struct command *read_head_info(struct sha1_array *shallow)
 	struct command **p = &commands;
 	for (;;) {
 		char *line;
-		int len, linelen;
+		const char *feature;
+		int len, linelen, featurelen;
 
 		line = packet_read_line(0, &len);
 		if (!line)
@@ -1442,6 +1454,9 @@ static struct command *read_head_info(struct sha1_array *shallow)
 			if (advertise_atomic_push
 			    && parse_feature_request(feature_list, "atomic"))
 				use_atomic = 1;
+			if ((feature =
+				parse_feature_value(feature_list, "hook-options", &featurelen)))
+				hook_options = xmemdupz(feature, featurelen);
 		}
 
 		if (!strcmp(line, "push-cert")) {
-- 
2.7.0-91-gf04ef09

[PATCH 3/3] send-pack: propagate --force and --quiet to remote hooks

[Dennis Kaarsemaker]

When a server supports hook options, we send it options for quiet and
force if the user used push --force/--quiet.

Signed-off-by: Dennis Kaarsemaker <git@vger.kernel.org>
---
 send-pack.c                  | 10 ++++++++++
 t/t5544-push-hook-options.sh | 37 +++++++++++++++++++++++++++++++++++++
 2 files changed, 47 insertions(+)
 create mode 100755 t/t5544-push-hook-options.sh

diff --git a/send-pack.c b/send-pack.c
index 047bd18..5630327 100644
--- a/send-pack.c
+++ b/send-pack.c
@@ -371,6 +371,8 @@ int send_pack(struct send_pack_args *args,
 	int agent_supported = 0;
 	int use_atomic = 0;
 	int atomic_supported = 0;
+	int hook_options_supported = 0;
+	int hook_options_seen = 0;
 	unsigned cmds_sent = 0;
 	int ret;
 	struct async demux;
@@ -393,6 +395,8 @@ int send_pack(struct send_pack_args *args,
 		args->use_thin_pack = 0;
 	if (server_supports("atomic"))
 		atomic_supported = 1;
+	if (server_supports("hook-options"))
+		hook_options_supported = 1;
 
 	if (args->push_cert != SEND_PACK_PUSH_CERT_NEVER) {
 		int len;
@@ -429,6 +433,12 @@ int send_pack(struct send_pack_args *args,
 		strbuf_addstr(&cap_buf, " atomic");
 	if (agent_supported)
 		strbuf_addf(&cap_buf, " agent=%s", git_user_agent_sanitized());
+	if (hook_options_supported) {
+		if (args->quiet)
+			strbuf_addf(&cap_buf, "%squiet", hook_options_seen++ ? "," : " hook-options=");
+		if (args->force_update)
+			strbuf_addf(&cap_buf, "%sforce", hook_options_seen++ ? "," : " hook-options=");
+	}
 
 	/*
 	 * NEEDSWORK: why does delete-refs have to be so specific to
diff --git a/t/t5544-push-hook-options.sh b/t/t5544-push-hook-options.sh
new file mode 100755
index 0000000..6d52ad1
--- /dev/null
+++ b/t/t5544-push-hook-options.sh
@@ -0,0 +1,37 @@
+#!/bin/sh
+
+test_description='pushing to a repository with hook options'
+
+. ./test-lib.sh
+
+test_expect_success 'hook options are passed on to hooks' '
+	git init --bare remote.git &&
+	write_script remote.git/hooks/post-receive <<-\EOF &&
+	echo "post-receive-hook"
+	echo $GIT_HOOK_OPTIONS
+	EOF
+	(
+		test_commit one &&
+		git push remote/ master:test 2> actual &&
+		test_commit two &&
+		git push --quiet remote/ master:test 2>> actual &&
+		test_commit three &&
+		git push --force remote/ master:test 2>> actual &&
+		test_commit four &&
+		git push --quiet --force remote/ master:test 2>> actual
+	) &&
+	sed -ne "s/remote: \([^ ]*\).*/\1/p" -i actual &&
+	cat > expected <<-\EOF &&
+	post-receive-hook
+
+	post-receive-hook
+	quiet
+	post-receive-hook
+	force
+	post-receive-hook
+	quiet,force
+	EOF
+	test_cmp expected actual
+'
+
+test_done
-- 
2.7.0-91-gf04ef09