All of lore.kernel.org
 help / color / mirror / Atom feed
From: Stefan Beller <sbeller@google.com>
To: git@vger.kernel.org, dwwang@google.com
Cc: Stefan Beller <sbeller@google.com>
Subject: [PATCH 1/4] push options: {pre,post}-receive hook learns about push options
Date: Wed, 29 Jun 2016 17:59:48 -0700	[thread overview]
Message-ID: <20160630005951.7408-2-sbeller@google.com> (raw)
In-Reply-To: <20160630005951.7408-1-sbeller@google.com>

The environment variable GIT_PUSH_OPTION_FILE is set to the push options
separated by new line.

The code is not executed as the push options are set to NULL, nor is the
new capability advertised.

The rationale for keeping the actual options inside a file instead of
putting them directly into an environment variable has multiple reasons:

1) After a discussion about environment variables and shells, we may not
want to put user data into an environment variable (see [1] for example).

2) If a user passes multiple push options, we need to pass them to the
hooks in a way, the hook can separate them. This could be done via
multiple environment variables (e.g. have GIT_PUSH_OPTIONS_COUNT and
GIT_PUSH_OPTIONS_{0,1,2,...} set), or put it all in one environment
variable and choose an appropriate separator. That is hard to parse
in both ways. For now we'll just put it in a file separated by new line,
such that the hook scripts can pickup the variables as

    while read option ; do
        process_push_option() $option
    done < $GIT_PUSH_OPTION_FILE

3) environment variables are messed with in the run-command API depending
on the occurrence of a '=' to set or unset the variable. Having multiple
'=' is ok, such that we could have it set to a "key=value" pair.

4) When putting the options in a file, we need to take care of the race
condition of multiple clients pushing. That is actually rather easy.

5) We only inject new lines as separator into the file, so it is possible
to transmit binaries ("attach an image to a code review").

[1] 'Shellshock' https://lwn.net/Articles/614218/

Signed-off-by: Stefan Beller <sbeller@google.com>
---
 Documentation/githooks.txt |  4 ++++
 builtin/receive-pack.c     | 41 ++++++++++++++++++++++++++++-------------
 2 files changed, 32 insertions(+), 13 deletions(-)

diff --git a/Documentation/githooks.txt b/Documentation/githooks.txt
index d82e912..dc80574 100644
--- a/Documentation/githooks.txt
+++ b/Documentation/githooks.txt
@@ -247,6 +247,8 @@ Both standard output and standard error output are forwarded to
 'git send-pack' on the other end, so you can simply `echo` messages
 for the user.
 
+The push options are available in the variable GIT_PUSH_OPTION_FILE.
+
 [[update]]
 update
 ~~~~~~
@@ -322,6 +324,8 @@ a sample script `post-receive-email` provided in the `contrib/hooks`
 directory in Git distribution, which implements sending commit
 emails.
 
+The push options are available in the variable GIT_PUSH_OPTION_FILE.
+
 [[post-update]]
 post-update
 ~~~~~~~~~~~
diff --git a/builtin/receive-pack.c b/builtin/receive-pack.c
index 15c323a..0da6852 100644
--- a/builtin/receive-pack.c
+++ b/builtin/receive-pack.c
@@ -550,8 +550,16 @@ static void prepare_push_cert_sha1(struct child_process *proc)
 	}
 }
 
+struct receive_hook_feed_state {
+	struct command *cmd;
+	int skip_broken;
+	struct strbuf buf;
+	const char *push_options_file;
+};
+
 typedef int (*feed_fn)(void *, const char **, size_t *);
-static int run_and_feed_hook(const char *hook_name, feed_fn feed, void *feed_state)
+static int run_and_feed_hook(const char *hook_name, feed_fn feed,
+			     struct receive_hook_feed_state *feed_state)
 {
 	struct child_process proc = CHILD_PROCESS_INIT;
 	struct async muxer;
@@ -567,6 +575,9 @@ static int run_and_feed_hook(const char *hook_name, feed_fn feed, void *feed_sta
 	proc.argv = argv;
 	proc.in = -1;
 	proc.stdout_to_stderr = 1;
+	if (feed_state && feed_state->push_options_file)
+		argv_array_pushf(&proc.env_array, "GIT_PUSH_OPTION_FILE=%s",
+				 feed_state->push_options_file);
 
 	if (use_sideband) {
 		memset(&muxer, 0, sizeof(muxer));
@@ -606,12 +617,6 @@ static int run_and_feed_hook(const char *hook_name, feed_fn feed, void *feed_sta
 	return finish_command(&proc);
 }
 
-struct receive_hook_feed_state {
-	struct command *cmd;
-	int skip_broken;
-	struct strbuf buf;
-};
-
 static int feed_receive_hook(void *state_, const char **bufp, size_t *sizep)
 {
 	struct receive_hook_feed_state *state = state_;
@@ -634,8 +639,10 @@ static int feed_receive_hook(void *state_, const char **bufp, size_t *sizep)
 	return 0;
 }
 
-static int run_receive_hook(struct command *commands, const char *hook_name,
-			    int skip_broken)
+static int run_receive_hook(struct command *commands,
+			    const char *hook_name,
+			    int skip_broken,
+			    const char *push_options_file)
 {
 	struct receive_hook_feed_state state;
 	int status;
@@ -646,6 +653,7 @@ static int run_receive_hook(struct command *commands, const char *hook_name,
 	if (feed_receive_hook(&state, NULL, NULL))
 		return 0;
 	state.cmd = commands;
+	state.push_options_file = push_options_file;
 	status = run_and_feed_hook(hook_name, feed_receive_hook, &state);
 	strbuf_release(&state.buf);
 	return status;
@@ -1316,7 +1324,8 @@ cleanup:
 
 static void execute_commands(struct command *commands,
 			     const char *unpacker_error,
-			     struct shallow_info *si)
+			     struct shallow_info *si,
+			     const char *push_options_file)
 {
 	struct command *cmd;
 	unsigned char sha1[20];
@@ -1335,7 +1344,7 @@ static void execute_commands(struct command *commands,
 
 	reject_updates_to_hidden(commands);
 
-	if (run_receive_hook(commands, "pre-receive", 0)) {
+	if (run_receive_hook(commands, "pre-receive", 0, push_options_file)) {
 		for (cmd = commands; cmd; cmd = cmd->next) {
 			if (!cmd->error_string)
 				cmd->error_string = "pre-receive hook declined";
@@ -1756,6 +1765,7 @@ int cmd_receive_pack(int argc, const char **argv, const char *prefix)
 
 	if ((commands = read_head_info(&shallow)) != NULL) {
 		const char *unpack_status = NULL;
+		const char *push_options_file = NULL;
 
 		prepare_shallow_info(&si, &shallow);
 		if (!si.nr_ours && !si.nr_theirs)
@@ -1764,13 +1774,18 @@ int cmd_receive_pack(int argc, const char **argv, const char *prefix)
 			unpack_status = unpack_with_sideband(&si);
 			update_shallow_info(commands, &si, &ref);
 		}
-		execute_commands(commands, unpack_status, &si);
+		execute_commands(commands, unpack_status, &si,
+				 push_options_file);
 		if (pack_lockfile)
 			unlink_or_warn(pack_lockfile);
 		if (report_status)
 			report(commands, unpack_status);
-		run_receive_hook(commands, "post-receive", 1);
+		run_receive_hook(commands, "post-receive", 1,
+				 push_options_file);
 		run_update_post_hook(commands);
+		if (push_options_file)
+			/* ignore errors */
+			unlink(push_options_file);
 		if (auto_gc) {
 			const char *argv_gc_auto[] = {
 				"gc", "--auto", "--quiet", NULL,
-- 
2.9.0.141.gdd65b60


  reply	other threads:[~2016-06-30  1:28 UTC|newest]

Thread overview: 31+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-06-30  0:59 [RFC PATCHv1 0/4] Push options in C Git Stefan Beller
2016-06-30  0:59 ` Stefan Beller [this message]
2016-07-01  7:14   ` [PATCH 1/4] push options: {pre,post}-receive hook learns about push options Jeff King
2016-07-01 17:20     ` Stefan Beller
2016-07-01 17:59       ` Jeff King
2016-07-01 18:03         ` Junio C Hamano
2016-07-01 18:11           ` Jeff King
2016-07-01 19:25             ` Junio C Hamano
2016-07-01 19:31               ` Stefan Beller
2016-07-01 19:39               ` Jeff King
2016-07-01 19:50                 ` Stefan Beller
2016-07-01 18:40         ` Stefan Beller
2016-06-30  0:59 ` [PATCH 2/4] receive-pack: implement advertising and receiving " Stefan Beller
2016-07-01 17:11   ` Junio C Hamano
2016-07-01 17:24     ` Stefan Beller
2016-06-30  0:59 ` [PATCH 3/4] push: accept " Stefan Beller
2016-06-30  0:59 ` [PATCH 4/4] add a test for " Stefan Beller
2016-07-01  7:09 ` [RFC PATCHv1 0/4] Push options in C Git Jeff King
2016-07-01 17:07   ` Stefan Beller
2016-07-01 17:55     ` Jeff King
2016-07-01 18:25       ` Stefan Beller
2016-07-01 20:01         ` Jeff King
2016-07-07  1:12 [PATCHv3 " Stefan Beller
2016-07-07  1:12 ` [PATCH 1/4] push options: {pre,post}-receive hook learns about push options Stefan Beller
2016-07-07 20:20   ` Junio C Hamano
2016-07-07 21:50     ` Stefan Beller
2016-07-07 21:53       ` Junio C Hamano
2016-07-09  0:31 [PATCHv4 0/4] Push options Stefan Beller
2016-07-09  0:31 ` [PATCH 1/4] push options: {pre,post}-receive hook learns about push options Stefan Beller
2016-07-14 17:39 [PATCHv5 0/4] Push options Stefan Beller
2016-07-14 17:39 ` [PATCH 1/4] push options: {pre,post}-receive hook learns about push options Stefan Beller
2016-07-14 21:49 [PATCHv7 0/4] Push options Stefan Beller
2016-07-14 21:49 ` [PATCH 1/4] push options: {pre,post}-receive hook learns about push options Stefan Beller
2016-07-14 22:46   ` Jeff King
2016-07-14 22:51     ` Stefan Beller

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20160630005951.7408-2-sbeller@google.com \
    --to=sbeller@google.com \
    --cc=dwwang@google.com \
    --cc=git@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.