Skip to content

Commit c65f26f

Browse files
nasamuffingitster
authored andcommitted
receive-pack: convert receive hooks to hook API
This converts the last remaining hooks to the new hook API, for the same benefits as the previous conversions (no need to toggle signals, manage custom struct child_process, call find_hook(), prepares for specifyinig hooks via configs, etc.). I noticed a performance degradation when processing large amounts of hook input with just 1 line per callback, due to run-command's poll loop, therefore I batched 500 lines per callback, to ensure similar pipe throughput as before and to avoid hook child waiting on stdin. 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>
1 parent 0bbaf36 commit c65f26f

File tree

1 file changed

+93
-119
lines changed

1 file changed

+93
-119
lines changed

builtin/receive-pack.c

Lines changed: 93 additions & 119 deletions
Original file line numberDiff line numberDiff line change
@@ -749,7 +749,7 @@ static int check_cert_push_options(const struct string_list *push_options)
749749
return retval;
750750
}
751751

752-
static void prepare_push_cert_sha1(struct child_process *proc)
752+
static void prepare_push_cert_sha1(struct run_hooks_opt *opt)
753753
{
754754
static int already_done;
755755

@@ -775,23 +775,23 @@ static void prepare_push_cert_sha1(struct child_process *proc)
775775
nonce_status = check_nonce(sigcheck.payload);
776776
}
777777
if (!is_null_oid(&push_cert_oid)) {
778-
strvec_pushf(&proc->env, "GIT_PUSH_CERT=%s",
778+
strvec_pushf(&opt->env, "GIT_PUSH_CERT=%s",
779779
oid_to_hex(&push_cert_oid));
780-
strvec_pushf(&proc->env, "GIT_PUSH_CERT_SIGNER=%s",
780+
strvec_pushf(&opt->env, "GIT_PUSH_CERT_SIGNER=%s",
781781
sigcheck.signer ? sigcheck.signer : "");
782-
strvec_pushf(&proc->env, "GIT_PUSH_CERT_KEY=%s",
782+
strvec_pushf(&opt->env, "GIT_PUSH_CERT_KEY=%s",
783783
sigcheck.key ? sigcheck.key : "");
784-
strvec_pushf(&proc->env, "GIT_PUSH_CERT_STATUS=%c",
784+
strvec_pushf(&opt->env, "GIT_PUSH_CERT_STATUS=%c",
785785
sigcheck.result);
786786
if (push_cert_nonce) {
787-
strvec_pushf(&proc->env,
787+
strvec_pushf(&opt->env,
788788
"GIT_PUSH_CERT_NONCE=%s",
789789
push_cert_nonce);
790-
strvec_pushf(&proc->env,
790+
strvec_pushf(&opt->env,
791791
"GIT_PUSH_CERT_NONCE_STATUS=%s",
792792
nonce_status);
793793
if (nonce_status == NONCE_SLOP)
794-
strvec_pushf(&proc->env,
794+
strvec_pushf(&opt->env,
795795
"GIT_PUSH_CERT_NONCE_SLOP=%ld",
796796
nonce_stamp_slop);
797797
}
@@ -803,119 +803,64 @@ struct receive_hook_feed_state {
803803
struct ref_push_report *report;
804804
int skip_broken;
805805
struct strbuf buf;
806-
const struct string_list *push_options;
807806
};
808807

809-
typedef int (*feed_fn)(void *, const char **, size_t *);
810-
static int run_and_feed_hook(const char *hook_name, feed_fn feed,
811-
struct receive_hook_feed_state *feed_state)
808+
static int feed_receive_hook_cb(int hook_stdin_fd, void *pp_cb UNUSED, void *pp_task_cb)
812809
{
813-
struct child_process proc = CHILD_PROCESS_INIT;
814-
struct async muxer;
815-
int code;
816-
const char *hook_path = find_hook(the_repository, hook_name);
810+
struct receive_hook_feed_state *state = pp_task_cb;
811+
struct command *cmd = state->cmd;
812+
unsigned int lines_batch_size = 500;
817813

818-
if (!hook_path)
819-
return 0;
814+
strbuf_reset(&state->buf);
820815

821-
strvec_push(&proc.args, hook_path);
822-
proc.in = -1;
823-
proc.stdout_to_stderr = 1;
824-
proc.trace2_hook_name = hook_name;
825-
826-
if (feed_state->push_options) {
827-
size_t i;
828-
for (i = 0; i < feed_state->push_options->nr; i++)
829-
strvec_pushf(&proc.env,
830-
"GIT_PUSH_OPTION_%"PRIuMAX"=%s",
831-
(uintmax_t)i,
832-
feed_state->push_options->items[i].string);
833-
strvec_pushf(&proc.env, "GIT_PUSH_OPTION_COUNT=%"PRIuMAX"",
834-
(uintmax_t)feed_state->push_options->nr);
835-
} else
836-
strvec_pushf(&proc.env, "GIT_PUSH_OPTION_COUNT");
816+
/* batch lines to avoid going through run-command's poll loop for each line */
817+
for (unsigned int i = 0; i < lines_batch_size; i++) {
818+
while (cmd &&
819+
state->skip_broken && (cmd->error_string || cmd->did_not_exist))
820+
cmd = cmd->next;
837821

838-
if (tmp_objdir)
839-
strvec_pushv(&proc.env, tmp_objdir_env(tmp_objdir));
822+
if (!cmd)
823+
break; /* no more commands left */
840824

841-
if (use_sideband) {
842-
memset(&muxer, 0, sizeof(muxer));
843-
muxer.proc = copy_to_sideband;
844-
muxer.in = -1;
845-
code = start_async(&muxer);
846-
if (code)
847-
return code;
848-
proc.err = muxer.in;
849-
}
825+
if (!state->report)
826+
state->report = cmd->report;
850827

851-
prepare_push_cert_sha1(&proc);
828+
if (state->report) {
829+
struct object_id *old_oid;
830+
struct object_id *new_oid;
831+
const char *ref_name;
852832

853-
code = start_command(&proc);
854-
if (code) {
855-
if (use_sideband)
856-
finish_async(&muxer);
857-
return code;
858-
}
833+
old_oid = state->report->old_oid ? state->report->old_oid : &cmd->old_oid;
834+
new_oid = state->report->new_oid ? state->report->new_oid : &cmd->new_oid;
835+
ref_name = state->report->ref_name ? state->report->ref_name : cmd->ref_name;
859836

860-
sigchain_push(SIGPIPE, SIG_IGN);
837+
strbuf_addf(&state->buf, "%s %s %s\n",
838+
oid_to_hex(old_oid), oid_to_hex(new_oid),
839+
ref_name);
861840

862-
while (1) {
863-
const char *buf;
864-
size_t n;
865-
if (feed(feed_state, &buf, &n))
866-
break;
867-
if (write_in_full(proc.in, buf, n) < 0)
868-
break;
841+
state->report = state->report->next;
842+
if (!state->report)
843+
cmd = cmd->next;
844+
} else {
845+
strbuf_addf(&state->buf, "%s %s %s\n",
846+
oid_to_hex(&cmd->old_oid), oid_to_hex(&cmd->new_oid),
847+
cmd->ref_name);
848+
cmd = cmd->next;
849+
}
869850
}
870-
close(proc.in);
871-
if (use_sideband)
872-
finish_async(&muxer);
873851

874-
sigchain_pop(SIGPIPE);
852+
state->cmd = cmd;
875853

876-
return finish_command(&proc);
877-
}
878-
879-
static int feed_receive_hook(void *state_, const char **bufp, size_t *sizep)
880-
{
881-
struct receive_hook_feed_state *state = state_;
882-
struct command *cmd = state->cmd;
883-
884-
while (cmd &&
885-
state->skip_broken && (cmd->error_string || cmd->did_not_exist))
886-
cmd = cmd->next;
887-
if (!cmd)
888-
return -1; /* EOF */
889-
if (!bufp)
890-
return 0; /* OK, can feed something. */
891-
strbuf_reset(&state->buf);
892-
if (!state->report)
893-
state->report = cmd->report;
894-
if (state->report) {
895-
struct object_id *old_oid;
896-
struct object_id *new_oid;
897-
const char *ref_name;
898-
899-
old_oid = state->report->old_oid ? state->report->old_oid : &cmd->old_oid;
900-
new_oid = state->report->new_oid ? state->report->new_oid : &cmd->new_oid;
901-
ref_name = state->report->ref_name ? state->report->ref_name : cmd->ref_name;
902-
strbuf_addf(&state->buf, "%s %s %s\n",
903-
oid_to_hex(old_oid), oid_to_hex(new_oid),
904-
ref_name);
905-
state->report = state->report->next;
906-
if (!state->report)
907-
state->cmd = cmd->next;
908-
} else {
909-
strbuf_addf(&state->buf, "%s %s %s\n",
910-
oid_to_hex(&cmd->old_oid), oid_to_hex(&cmd->new_oid),
911-
cmd->ref_name);
912-
state->cmd = cmd->next;
913-
}
914-
if (bufp) {
915-
*bufp = state->buf.buf;
916-
*sizep = state->buf.len;
854+
if (state->buf.len > 0) {
855+
int ret = write_in_full(hook_stdin_fd, state->buf.buf, state->buf.len);
856+
if (ret < 0) {
857+
if (errno == EPIPE)
858+
return 1; /* child closed pipe */
859+
return ret;
860+
}
917861
}
918-
return 0;
862+
863+
return state->cmd ? 0 : 1; /* 0 = more to come, 1 = EOF */
919864
}
920865

921866
static void hook_output_to_sideband(struct strbuf *output, void *cb_data UNUSED)
@@ -933,20 +878,49 @@ static int run_receive_hook(struct command *commands,
933878
int skip_broken,
934879
const struct string_list *push_options)
935880
{
936-
struct receive_hook_feed_state state;
937-
int status;
938-
939-
strbuf_init(&state.buf, 0);
940-
state.cmd = commands;
941-
state.skip_broken = skip_broken;
942-
state.report = NULL;
943-
if (feed_receive_hook(&state, NULL, NULL))
881+
struct run_hooks_opt opt = RUN_HOOKS_OPT_INIT;
882+
struct command *iter = commands;
883+
struct receive_hook_feed_state feed_state;
884+
int ret;
885+
886+
/* if there are no valid commands, don't invoke the hook at all. */
887+
while (iter && skip_broken && (iter->error_string || iter->did_not_exist))
888+
iter = iter->next;
889+
if (!iter)
944890
return 0;
945-
state.cmd = commands;
946-
state.push_options = push_options;
947-
status = run_and_feed_hook(hook_name, feed_receive_hook, &state);
948-
strbuf_release(&state.buf);
949-
return status;
891+
892+
if (push_options) {
893+
for (int i = 0; i < push_options->nr; i++)
894+
strvec_pushf(&opt.env, "GIT_PUSH_OPTION_%d=%s", i,
895+
push_options->items[i].string);
896+
strvec_pushf(&opt.env, "GIT_PUSH_OPTION_COUNT=%"PRIuMAX"",
897+
(uintmax_t)push_options->nr);
898+
} else {
899+
strvec_push(&opt.env, "GIT_PUSH_OPTION_COUNT");
900+
}
901+
902+
if (tmp_objdir)
903+
strvec_pushv(&opt.env, tmp_objdir_env(tmp_objdir));
904+
905+
prepare_push_cert_sha1(&opt);
906+
907+
/* set up sideband printer */
908+
if (use_sideband)
909+
opt.consume_output = hook_output_to_sideband;
910+
911+
/* set up stdin callback */
912+
feed_state.cmd = commands;
913+
feed_state.skip_broken = skip_broken;
914+
feed_state.report = NULL;
915+
strbuf_init(&feed_state.buf, 0);
916+
opt.feed_pipe_cb_data = &feed_state;
917+
opt.feed_pipe = feed_receive_hook_cb;
918+
919+
ret = run_hooks_opt(the_repository, hook_name, &opt);
920+
921+
strbuf_release(&feed_state.buf);
922+
923+
return ret;
950924
}
951925

952926
static int run_update_hook(struct command *cmd)

0 commit comments

Comments
 (0)