reference-transaction: use hook API instead of run-command

Convert the reference-transaction hook to the new hook API,
so it doesn't need to set up a struct child_process, call
find_hook or toggle the pipe signals.

The stdin feed callback is processing one ref update per
call. I haven't noticed any performance degradation due
to this, however we can batch as many we want in each call,
to ensure a good pipe throughtput (i.e. the child does not
wait after stdin).

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

diff --git a/refs.c b/refs.c
index 965381367e0e53914040d77bea12975354c8b854..e0b060224ab1438ed4f554a20bffc903c9842011 100644 (file)
--- a/refs.c
+++ b/refs.c
@@ -2405,68 +2405,72 @@ static int ref_update_reject_duplicates(struct string_list *refnames,
        return 0;
 }
 
-static int run_transaction_hook(struct ref_transaction *transaction,
-                               const char *state)
+struct transaction_feed_cb_data {
+       size_t index;
+       struct strbuf buf;
+};
+
+static int transaction_hook_feed_stdin(int hook_stdin_fd, void *pp_cb, void *pp_task_cb)
 {
-       struct child_process proc = CHILD_PROCESS_INIT;
-       struct strbuf buf = STRBUF_INIT;
-       const char *hook;
-       int ret = 0;
+       struct hook_cb_data *hook_cb = pp_cb;
+       struct ref_transaction *transaction = hook_cb->options->feed_pipe_ctx;
+       struct transaction_feed_cb_data *feed_cb_data = pp_task_cb;
+       struct strbuf *buf = &feed_cb_data->buf;
+       struct ref_update *update;
+       size_t i = feed_cb_data->index++;
+       int ret;
 
-       hook = find_hook(transaction->ref_store->repo, "reference-transaction");
-       if (!hook)
-               return ret;
+       if (i >= transaction->nr)
+               return 1; /* No more refs to process */
 
-       strvec_pushl(&proc.args, hook, state, NULL);
-       proc.in = -1;
-       proc.stdout_to_stderr = 1;
-       proc.trace2_hook_name = "reference-transaction";
+       update = transaction->updates[i];
 
-       ret = start_command(&proc);
-       if (ret)
-               return ret;
+       if (update->flags & REF_LOG_ONLY)
+               return 0;
 
-       sigchain_push(SIGPIPE, SIG_IGN);
+       strbuf_reset(buf);
 
-       for (size_t i = 0; i < transaction->nr; i++) {
-               struct ref_update *update = transaction->updates[i];
+       if (!(update->flags & REF_HAVE_OLD))
+               strbuf_addf(buf, "%s ", oid_to_hex(null_oid(the_hash_algo)));
+       else if (update->old_target)
+               strbuf_addf(buf, "ref:%s ", update->old_target);
+       else
+               strbuf_addf(buf, "%s ", oid_to_hex(&update->old_oid));
 
-               if (update->flags & REF_LOG_ONLY)
-                       continue;
+       if (!(update->flags & REF_HAVE_NEW))
+               strbuf_addf(buf, "%s ", oid_to_hex(null_oid(the_hash_algo)));
+       else if (update->new_target)
+               strbuf_addf(buf, "ref:%s ", update->new_target);
+       else
+               strbuf_addf(buf, "%s ", oid_to_hex(&update->new_oid));
 
-               strbuf_reset(&buf);
+       strbuf_addf(buf, "%s\n", update->refname);
 
-               if (!(update->flags & REF_HAVE_OLD))
-                       strbuf_addf(&buf, "%s ", oid_to_hex(null_oid(the_hash_algo)));
-               else if (update->old_target)
-                       strbuf_addf(&buf, "ref:%s ", update->old_target);
-               else
-                       strbuf_addf(&buf, "%s ", oid_to_hex(&update->old_oid));
+       ret = write_in_full(hook_stdin_fd, buf->buf, buf->len);
+       if (ret < 0 && errno != EPIPE)
+               return ret;
 
-               if (!(update->flags & REF_HAVE_NEW))
-                       strbuf_addf(&buf, "%s ", oid_to_hex(null_oid(the_hash_algo)));
-               else if (update->new_target)
-                       strbuf_addf(&buf, "ref:%s ", update->new_target);
-               else
-                       strbuf_addf(&buf, "%s ", oid_to_hex(&update->new_oid));
+       return 0; /* no more input to feed */
+}
+
+static int run_transaction_hook(struct ref_transaction *transaction,
+                               const char *state)
+{
+       struct run_hooks_opt opt = RUN_HOOKS_OPT_INIT;
+       struct transaction_feed_cb_data feed_ctx = { 0 };
+       int ret = 0;
 
-               strbuf_addf(&buf, "%s\n", update->refname);
+       strvec_push(&opt.args, state);
 
-               if (write_in_full(proc.in, buf.buf, buf.len) < 0) {
-                       if (errno != EPIPE) {
-                               /* Don't leak errno outside this API */
-                               errno = 0;
-                               ret = -1;
-                       }
-                       break;
-               }
-       }
+       opt.feed_pipe = transaction_hook_feed_stdin;
+       opt.feed_pipe_ctx = transaction;
+       opt.feed_pipe_cb_data = &feed_ctx;
 
-       close(proc.in);
-       sigchain_pop(SIGPIPE);
-       strbuf_release(&buf);
+       strbuf_init(&feed_ctx.buf, 0);
+
+       ret = run_hooks_opt(transaction->ref_store->repo, "reference-transaction", &opt);
 
-       ret |= finish_command(&proc);
+       strbuf_release(&feed_ctx.buf);
        return ret;
 }