receive-pack: convert update hooks to new API

The hook API avoids creating a custom struct child_process and other
internal hook plumbing (e.g. calling find_hook()) and prepares for
the specification of hooks via configs or running parallel hooks.

Execution is still sequential through the run_hooks_opt .jobs == 1,
which is the unchanged default for all hooks.

When use_sideband==1, the async thread redirects the hook outputs to
sideband 2, otherwise it is not used and the hooks write directly to
the fds inherited from the main parent process.

When .jobs == 1, run-command's poll loop is avoided entirely via the
ungroup=1 option like before (this was Jeff's suggestion), achieving
the same real-time output performance.

When running in parallel, run-command with ungroup=0 will capture
and de-interleave the output of each hook, then write to the parent
stderr which is redirected via dup2 to the sideband thread, so that
each parallel hook output is presented clearly to the client.

Signed-off-by: Emily Shaffer <emilyshaffer@google.com>
Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Helped-by: Jeff King <peff@peff.net>
Signed-off-by: Adrian Ratiu <adrian.ratiu@collabora.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

diff --git a/builtin/receive-pack.c b/builtin/receive-pack.c
index 9c491746168a6f0674328b5c4175477034a555c7..f554dac1ef9b03c8341b0e20e3d35f1cb9362dfa 100644 (file)
--- a/builtin/receive-pack.c
+++ b/builtin/receive-pack.c
@@ -561,6 +561,48 @@ static int copy_to_sideband(int in, int out UNUSED, void *arg UNUSED)
        return 0;
 }
 
+/*
+ * Start an async thread which redirects hook stderr over the sideband.
+ * The original stderr fd is saved to `saved_stderr` and STDERR_FILENO is
+ * redirected to the async's input pipe.
+ */
+static void prepare_sideband_async(struct async *sideband_async, int *saved_stderr, int *started)
+{
+       *started = 0;
+
+       if (!use_sideband)
+               return;
+
+       memset(sideband_async, 0, sizeof(*sideband_async));
+       sideband_async->proc = copy_to_sideband;
+       sideband_async->in = -1;
+
+       if (!start_async(sideband_async)) {
+               *started = 1;
+               *saved_stderr = dup(STDERR_FILENO);
+               if (*saved_stderr >= 0)
+                       dup2(sideband_async->in, STDERR_FILENO);
+               close(sideband_async->in);
+       }
+}
+
+/*
+ * Restore the original stderr and wait for the async sideband thread to finish.
+ */
+static void finish_sideband_async(struct async *sideband_async, int saved_stderr, int started)
+{
+       if (!use_sideband)
+               return;
+
+       if (saved_stderr >= 0) {
+               dup2(saved_stderr, STDERR_FILENO);
+               close(saved_stderr);
+       }
+
+       if (started)
+               finish_async(sideband_async);
+}
+
 static void hmac_hash(unsigned char *out,
                      const char *key_in, size_t key_len,
                      const char *text, size_t text_len)
@@ -941,29 +983,25 @@ static int run_receive_hook(struct command *commands,
 
 static int run_update_hook(struct command *cmd)
 {
-       struct child_process proc = CHILD_PROCESS_INIT;
+       struct run_hooks_opt opt = RUN_HOOKS_OPT_INIT;
+       struct async sideband_async;
+       int sideband_async_started = 0;
+       int saved_stderr = -1;
        int code;
-       const char *hook_path = find_hook(the_repository, "update");
 
-       if (!hook_path)
-               return 0;
+       strvec_pushl(&opt.args,
+                    cmd->ref_name,
+                    oid_to_hex(&cmd->old_oid),
+                    oid_to_hex(&cmd->new_oid),
+                    NULL);
 
-       strvec_push(&proc.args, hook_path);
-       strvec_push(&proc.args, cmd->ref_name);
-       strvec_push(&proc.args, oid_to_hex(&cmd->old_oid));
-       strvec_push(&proc.args, oid_to_hex(&cmd->new_oid));
+       prepare_sideband_async(&sideband_async, &saved_stderr, &sideband_async_started);
 
-       proc.no_stdin = 1;
-       proc.stdout_to_stderr = 1;
-       proc.err = use_sideband ? -1 : 0;
-       proc.trace2_hook_name = "update";
+       code = run_hooks_opt(the_repository, "update", &opt);
 
-       code = start_command(&proc);
-       if (code)
-               return code;
-       if (use_sideband)
-               copy_to_sideband(proc.err, -1, NULL);
-       return finish_command(&proc);
+       finish_sideband_async(&sideband_async, saved_stderr, sideband_async_started);
+
+       return code;
 }
 
 static struct command *find_command_by_refname(struct command *list,
@@ -1639,34 +1677,25 @@ out:
 
 static void run_update_post_hook(struct command *commands)
 {
+       struct run_hooks_opt opt = RUN_HOOKS_OPT_INIT;
+       struct async sideband_async;
        struct command *cmd;
-       struct child_process proc = CHILD_PROCESS_INIT;
-       const char *hook;
-
-       hook = find_hook(the_repository, "post-update");
-       if (!hook)
-               return;
+       int sideband_async_started = 0;
+       int saved_stderr = -1;
 
        for (cmd = commands; cmd; cmd = cmd->next) {
                if (cmd->error_string || cmd->did_not_exist)
                        continue;
-               if (!proc.args.nr)
-                       strvec_push(&proc.args, hook);
-               strvec_push(&proc.args, cmd->ref_name);
+               strvec_push(&opt.args, cmd->ref_name);
        }
-       if (!proc.args.nr)
+       if (!opt.args.nr)
                return;
 
-       proc.no_stdin = 1;
-       proc.stdout_to_stderr = 1;
-       proc.err = use_sideband ? -1 : 0;
-       proc.trace2_hook_name = "post-update";
+       prepare_sideband_async(&sideband_async, &saved_stderr, &sideband_async_started);
 
-       if (!start_command(&proc)) {
-               if (use_sideband)
-                       copy_to_sideband(proc.err, -1, NULL);
-               finish_command(&proc);
-       }
+       run_hooks_opt(the_repository, "post-update", &opt);
+
+       finish_sideband_async(&sideband_async, saved_stderr, sideband_async_started);
 }
 
 static void check_aliased_update_internal(struct command *cmd,