diff --git a/Documentation/config/hook.adoc b/Documentation/config/hook.adoc new file mode 100644 index 0000000000..9e78f26439 --- /dev/null +++ b/Documentation/config/hook.adoc @@ -0,0 +1,24 @@ +hook..command:: + The command to execute for `hook.`. `` + is a unique name that identifies this hook. The hook events that + trigger the command are configured with `hook..event`. + The value can be an executable path or a shell oneliner. If more than + one value is specified for the same ``, only the last + value parsed is used. See linkgit:git-hook[1]. + +hook..event:: + The hook events that trigger `hook.`. The value is the + name of a hook event, like "pre-commit" or "update". (See + linkgit:githooks[5] for a complete list of hook events.) On the + specified event, the associated `hook..command` is executed. + This is a multi-valued key. To run `hook.` on multiple + events, specify the key more than once. An empty value resets + the list of events, clearing any previously defined events for + `hook.`. See linkgit:git-hook[1]. + +hook..enabled:: + Whether the hook `hook.` is enabled. Defaults to `true`. + Set to `false` to disable the hook without removing its + configuration. This is particularly useful when a hook is defined + in a system or global config file and needs to be disabled for a + specific repository. See linkgit:git-hook[1]. diff --git a/Documentation/git-hook.adoc b/Documentation/git-hook.adoc index f6cc72d2ca..4d4e728327 100644 --- a/Documentation/git-hook.adoc +++ b/Documentation/git-hook.adoc @@ -9,6 +9,7 @@ SYNOPSIS -------- [verse] 'git hook' run [--ignore-missing] [--to-stdin=] [-- ] +'git hook' list [-z] [--show-scope] DESCRIPTION ----------- @@ -16,18 +17,107 @@ DESCRIPTION A command interface for running git hooks (see linkgit:githooks[5]), for use by other scripted git commands. +This command parses the default configuration files for sets of configs like +so: + + [hook "linter"] + event = pre-commit + command = ~/bin/linter --cpp20 + +In this example, `[hook "linter"]` represents one script - `~/bin/linter +--cpp20` - which can be shared by many repos, and even by many hook events, if +appropriate. + +To add an unrelated hook which runs on a different event, for example a +spell-checker for your commit messages, you would write a configuration like so: + + [hook "linter"] + event = pre-commit + command = ~/bin/linter --cpp20 + [hook "spellcheck"] + event = commit-msg + command = ~/bin/spellchecker + +With this config, when you run 'git commit', first `~/bin/linter --cpp20` will +have a chance to check your files to be committed (during the `pre-commit` hook +event`), and then `~/bin/spellchecker` will have a chance to check your commit +message (during the `commit-msg` hook event). + +Commands are run in the order Git encounters their associated +`hook..event` configs during the configuration parse (see +linkgit:git-config[1]). Although multiple `hook.linter.event` configs can be +added, only one `hook.linter.command` event is valid - Git uses "last-one-wins" +to determine which command to run. + +So if you wanted your linter to run when you commit as well as when you push, +you would configure it like so: + + [hook "linter"] + event = pre-commit + event = pre-push + command = ~/bin/linter --cpp20 + +With this config, `~/bin/linter --cpp20` would be run by Git before a commit is +generated (during `pre-commit`) as well as before a push is performed (during +`pre-push`). + +And if you wanted to run your linter as well as a secret-leak detector during +only the "pre-commit" hook event, you would configure it instead like so: + + [hook "linter"] + event = pre-commit + command = ~/bin/linter --cpp20 + [hook "no-leaks"] + event = pre-commit + command = ~/bin/leak-detector + +With this config, before a commit is generated (during `pre-commit`), Git would +first start `~/bin/linter --cpp20` and second start `~/bin/leak-detector`. It +would evaluate the output of each when deciding whether to proceed with the +commit. + +For a full list of hook events which you can set your `hook..event` to, +and how hooks are invoked during those events, see linkgit:githooks[5]. + +Git will ignore any `hook..event` that specifies an event it doesn't +recognize. This is intended so that tools which wrap Git can use the hook +infrastructure to run their own hooks; see "WRAPPERS" for more guidance. + +In general, when instructions suggest adding a script to +`.git/hooks/`, you can specify it in the config instead by running: + +---- +git config set hook..command +git config set --append hook..event +---- + +This way you can share the script between multiple repos. That is, `cp +~/my-script.sh ~/project/.git/hooks/pre-commit` would become: + +---- +git config set hook.my-script.command ~/my-script.sh +git config set --append hook.my-script.event pre-commit +---- + SUBCOMMANDS ----------- run:: - Run the `` hook. See linkgit:githooks[5] for - supported hook names. + Runs hooks configured for ``, in the order they are + discovered during the config parse. The default `` from + the hookdir is run last. See linkgit:githooks[5] for supported + hook names. + Any positional arguments to the hook should be passed after a mandatory `--` (or `--end-of-options`, see linkgit:gitcli[7]). See linkgit:githooks[5] for arguments hooks might expect (if any). +list [-z] [--show-scope]:: + Print a list of hooks which will be run on `` event. If no + hooks are configured for that event, print a warning and return 1. + Use `-z` to terminate output lines with NUL instead of newlines. + OPTIONS ------- @@ -41,6 +131,54 @@ OPTIONS tools that want to do a blind one-shot run of a hook that may or may not be present. +-z:: + Terminate "list" output lines with NUL instead of newlines. + +--show-scope:: + For "list"; print the config scope (e.g. `local`, `global`, `system`) + in parentheses after the friendly name of each configured hook, to show + where it was defined. Traditional hooks from the hookdir are unaffected. + +WRAPPERS +-------- + +`git hook run` has been designed to make it easy for tools which wrap Git to +configure and execute hooks using the Git hook infrastructure. It is possible to +provide arguments and stdin via the command line, as well as specifying parallel +or series execution if the user has provided multiple hooks. + +Assuming your wrapper wants to support a hook named "mywrapper-start-tests", you +can have your users specify their hooks like so: + + [hook "setup-test-dashboard"] + event = mywrapper-start-tests + command = ~/mywrapper/setup-dashboard.py --tap + +Then, in your 'mywrapper' tool, you can invoke any users' configured hooks by +running: + +---- +git hook run mywrapper-start-tests \ + # providing something to stdin + --stdin some-tempfile-123 \ + # execute hooks in serial + # plus some arguments of your own... + -- \ + --testname bar \ + baz +---- + +Take care to name your wrapper's hook events in a way which is unlikely to +overlap with Git's native hooks (see linkgit:githooks[5]) - a hook event named +`mywrappertool-validate-commit` is much less likely to be added to native Git +than a hook event named `validate-commit`. If Git begins to use a hook event +named the same thing as your wrapper hook, it may invoke your users' hooks in +unintended and unsupported ways. + +CONFIGURATION +------------- +include::config/hook.adoc[] + SEE ALSO -------- linkgit:githooks[5] diff --git a/builtin/hook.c b/builtin/hook.c index 7afec380d2..ff446948fa 100644 --- a/builtin/hook.c +++ b/builtin/hook.c @@ -5,13 +5,15 @@ #include "gettext.h" #include "hook.h" #include "parse-options.h" -#include "strvec.h" #define BUILTIN_HOOK_RUN_USAGE \ N_("git hook run [--ignore-missing] [--to-stdin=] [-- ]") +#define BUILTIN_HOOK_LIST_USAGE \ + N_("git hook list [-z] [--show-scope] ") static const char * const builtin_hook_usage[] = { BUILTIN_HOOK_RUN_USAGE, + BUILTIN_HOOK_LIST_USAGE, NULL }; @@ -20,6 +22,81 @@ static const char * const builtin_hook_run_usage[] = { NULL }; +static int list(int argc, const char **argv, const char *prefix, + struct repository *repo) +{ + static const char *const builtin_hook_list_usage[] = { + BUILTIN_HOOK_LIST_USAGE, + NULL + }; + struct string_list *head; + struct string_list_item *item; + const char *hookname = NULL; + int line_terminator = '\n'; + int show_scope = 0; + int ret = 0; + + struct option list_options[] = { + OPT_SET_INT('z', NULL, &line_terminator, + N_("use NUL as line terminator"), '\0'), + OPT_BOOL(0, "show-scope", &show_scope, + N_("show the config scope that defined each hook")), + OPT_END(), + }; + + argc = parse_options(argc, argv, prefix, list_options, + builtin_hook_list_usage, 0); + + /* + * The only unnamed argument provided should be the hook-name; if we add + * arguments later they probably should be caught by parse_options. + */ + if (argc != 1) + usage_msg_opt(_("you must specify a hook event name to list."), + builtin_hook_list_usage, list_options); + + hookname = argv[0]; + + head = list_hooks(repo, hookname, NULL); + + if (!head->nr) { + warning(_("no hooks found for event '%s'"), hookname); + ret = 1; /* no hooks found */ + goto cleanup; + } + + for_each_string_list_item(item, head) { + struct hook *h = item->util; + + switch (h->kind) { + case HOOK_TRADITIONAL: + printf("%s%c", _("hook from hookdir"), line_terminator); + break; + case HOOK_CONFIGURED: { + const char *name = h->u.configured.friendly_name; + const char *scope = show_scope ? + config_scope_name(h->u.configured.scope) : NULL; + if (scope) + printf("%s (%s%s)%c", name, scope, + h->u.configured.disabled ? ", disabled" : "", + line_terminator); + else if (h->u.configured.disabled) + printf("%s (disabled)%c", name, line_terminator); + else + printf("%s%c", name, line_terminator); + break; + } + default: + BUG("unknown hook kind"); + } + } + +cleanup: + string_list_clear_func(head, hook_free); + free(head); + return ret; +} + static int run(int argc, const char **argv, const char *prefix, struct repository *repo UNUSED) { @@ -77,6 +154,7 @@ int cmd_hook(int argc, parse_opt_subcommand_fn *fn = NULL; struct option builtin_hook_options[] = { OPT_SUBCOMMAND("run", &fn, run), + OPT_SUBCOMMAND("list", &fn, list), OPT_END(), }; diff --git a/builtin/receive-pack.c b/builtin/receive-pack.c index 4c0112b4bc..6066cbae8e 100644 --- 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) @@ -749,7 +791,7 @@ static int check_cert_push_options(const struct string_list *push_options) return retval; } -static void prepare_push_cert_sha1(struct child_process *proc) +static void prepare_push_cert_sha1(struct run_hooks_opt *opt) { static int already_done; @@ -775,23 +817,23 @@ static void prepare_push_cert_sha1(struct child_process *proc) nonce_status = check_nonce(sigcheck.payload); } if (!is_null_oid(&push_cert_oid)) { - strvec_pushf(&proc->env, "GIT_PUSH_CERT=%s", + strvec_pushf(&opt->env, "GIT_PUSH_CERT=%s", oid_to_hex(&push_cert_oid)); - strvec_pushf(&proc->env, "GIT_PUSH_CERT_SIGNER=%s", + strvec_pushf(&opt->env, "GIT_PUSH_CERT_SIGNER=%s", sigcheck.signer ? sigcheck.signer : ""); - strvec_pushf(&proc->env, "GIT_PUSH_CERT_KEY=%s", + strvec_pushf(&opt->env, "GIT_PUSH_CERT_KEY=%s", sigcheck.key ? sigcheck.key : ""); - strvec_pushf(&proc->env, "GIT_PUSH_CERT_STATUS=%c", + strvec_pushf(&opt->env, "GIT_PUSH_CERT_STATUS=%c", sigcheck.result); if (push_cert_nonce) { - strvec_pushf(&proc->env, + strvec_pushf(&opt->env, "GIT_PUSH_CERT_NONCE=%s", push_cert_nonce); - strvec_pushf(&proc->env, + strvec_pushf(&opt->env, "GIT_PUSH_CERT_NONCE_STATUS=%s", nonce_status); if (nonce_status == NONCE_SLOP) - strvec_pushf(&proc->env, + strvec_pushf(&opt->env, "GIT_PUSH_CERT_NONCE_SLOP=%ld", nonce_stamp_slop); } @@ -803,94 +845,25 @@ struct receive_hook_feed_state { struct ref_push_report *report; int skip_broken; struct strbuf buf; - const struct string_list *push_options; }; -typedef int (*feed_fn)(void *, const char **, size_t *); -static int run_and_feed_hook(const char *hook_name, feed_fn feed, - struct receive_hook_feed_state *feed_state) +static int feed_receive_hook_cb(int hook_stdin_fd, void *pp_cb UNUSED, void *pp_task_cb) { - struct child_process proc = CHILD_PROCESS_INIT; - struct async muxer; - int code; - const char *hook_path = find_hook(the_repository, hook_name); - - if (!hook_path) - return 0; - - strvec_push(&proc.args, hook_path); - proc.in = -1; - proc.stdout_to_stderr = 1; - proc.trace2_hook_name = hook_name; - - if (feed_state->push_options) { - size_t i; - for (i = 0; i < feed_state->push_options->nr; i++) - strvec_pushf(&proc.env, - "GIT_PUSH_OPTION_%"PRIuMAX"=%s", - (uintmax_t)i, - feed_state->push_options->items[i].string); - strvec_pushf(&proc.env, "GIT_PUSH_OPTION_COUNT=%"PRIuMAX"", - (uintmax_t)feed_state->push_options->nr); - } else - strvec_pushf(&proc.env, "GIT_PUSH_OPTION_COUNT"); - - if (tmp_objdir) - strvec_pushv(&proc.env, tmp_objdir_env(tmp_objdir)); - - if (use_sideband) { - memset(&muxer, 0, sizeof(muxer)); - muxer.proc = copy_to_sideband; - muxer.in = -1; - code = start_async(&muxer); - if (code) - return code; - proc.err = muxer.in; - } - - prepare_push_cert_sha1(&proc); - - code = start_command(&proc); - if (code) { - if (use_sideband) - finish_async(&muxer); - return code; - } - - sigchain_push(SIGPIPE, SIG_IGN); - - while (1) { - const char *buf; - size_t n; - if (feed(feed_state, &buf, &n)) - break; - if (write_in_full(proc.in, buf, n) < 0) - break; - } - close(proc.in); - if (use_sideband) - finish_async(&muxer); - - sigchain_pop(SIGPIPE); - - return finish_command(&proc); -} - -static int feed_receive_hook(void *state_, const char **bufp, size_t *sizep) -{ - struct receive_hook_feed_state *state = state_; + struct receive_hook_feed_state *state = pp_task_cb; struct command *cmd = state->cmd; + strbuf_reset(&state->buf); + while (cmd && state->skip_broken && (cmd->error_string || cmd->did_not_exist)) cmd = cmd->next; + if (!cmd) - return -1; /* EOF */ - if (!bufp) - return 0; /* OK, can feed something. */ - strbuf_reset(&state->buf); + return 1; /* no more commands left */ + if (!state->report) state->report = cmd->report; + if (state->report) { struct object_id *old_oid; struct object_id *new_oid; @@ -899,23 +872,54 @@ static int feed_receive_hook(void *state_, const char **bufp, size_t *sizep) old_oid = state->report->old_oid ? state->report->old_oid : &cmd->old_oid; new_oid = state->report->new_oid ? state->report->new_oid : &cmd->new_oid; ref_name = state->report->ref_name ? state->report->ref_name : cmd->ref_name; + strbuf_addf(&state->buf, "%s %s %s\n", oid_to_hex(old_oid), oid_to_hex(new_oid), ref_name); + state->report = state->report->next; if (!state->report) - state->cmd = cmd->next; + cmd = cmd->next; } else { strbuf_addf(&state->buf, "%s %s %s\n", oid_to_hex(&cmd->old_oid), oid_to_hex(&cmd->new_oid), cmd->ref_name); - state->cmd = cmd->next; + cmd = cmd->next; } - if (bufp) { - *bufp = state->buf.buf; - *sizep = state->buf.len; + + state->cmd = cmd; + + if (state->buf.len > 0) { + int ret = write_in_full(hook_stdin_fd, state->buf.buf, state->buf.len); + if (ret < 0) { + if (errno == EPIPE) + return 1; /* child closed pipe */ + return ret; + } } - return 0; + + return state->cmd ? 0 : 1; /* 0 = more to come, 1 = EOF */ +} + +static void *receive_hook_feed_state_alloc(void *feed_pipe_ctx) +{ + struct receive_hook_feed_state *init_state = feed_pipe_ctx; + struct receive_hook_feed_state *data; + CALLOC_ARRAY(data, 1); + data->report = init_state->report; + data->cmd = init_state->cmd; + data->skip_broken = init_state->skip_broken; + strbuf_init(&data->buf, 0); + return data; +} + +static void receive_hook_feed_state_free(void *data) +{ + struct receive_hook_feed_state *d = data; + if (!d) + return; + strbuf_release(&d->buf); + free(d); } static int run_receive_hook(struct command *commands, @@ -923,47 +927,82 @@ static int run_receive_hook(struct command *commands, int skip_broken, const struct string_list *push_options) { - struct receive_hook_feed_state state; - int status; + struct run_hooks_opt opt = RUN_HOOKS_OPT_INIT; + struct command *iter = commands; + struct receive_hook_feed_state feed_init_state = { + .cmd = commands, + .skip_broken = skip_broken, + .buf = STRBUF_INIT, + }; + struct async sideband_async; + int sideband_async_started = 0; + int saved_stderr = -1; + int ret; - strbuf_init(&state.buf, 0); - state.cmd = commands; - state.skip_broken = skip_broken; - state.report = NULL; - if (feed_receive_hook(&state, NULL, NULL)) + if (!hook_exists(the_repository, hook_name)) return 0; - state.cmd = commands; - state.push_options = push_options; - status = run_and_feed_hook(hook_name, feed_receive_hook, &state); - strbuf_release(&state.buf); - return status; + + /* if there are no valid commands, don't invoke the hook at all. */ + while (iter && skip_broken && (iter->error_string || iter->did_not_exist)) + iter = iter->next; + if (!iter) + return 0; + + if (push_options) { + for (int i = 0; i < push_options->nr; i++) + strvec_pushf(&opt.env, "GIT_PUSH_OPTION_%d=%s", i, + push_options->items[i].string); + strvec_pushf(&opt.env, "GIT_PUSH_OPTION_COUNT=%"PRIuMAX"", + (uintmax_t)push_options->nr); + } else { + strvec_push(&opt.env, "GIT_PUSH_OPTION_COUNT"); + } + + if (tmp_objdir) + strvec_pushv(&opt.env, tmp_objdir_env(tmp_objdir)); + + prepare_push_cert_sha1(&opt); + + prepare_sideband_async(&sideband_async, &saved_stderr, &sideband_async_started); + + /* set up stdin callback */ + opt.feed_pipe_ctx = &feed_init_state; + opt.feed_pipe = feed_receive_hook_cb; + opt.feed_pipe_cb_data_alloc = receive_hook_feed_state_alloc; + opt.feed_pipe_cb_data_free = receive_hook_feed_state_free; + + ret = run_hooks_opt(the_repository, hook_name, &opt); + + finish_sideband_async(&sideband_async, saved_stderr, sideband_async_started); + + return ret; } static int run_update_hook(struct command *cmd) { - struct child_process proc = CHILD_PROCESS_INIT; + static const char hook_name[] = "update"; + 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) + if (!hook_exists(the_repository, hook_name)) return 0; - 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)); + strvec_pushl(&opt.args, + cmd->ref_name, + oid_to_hex(&cmd->old_oid), + oid_to_hex(&cmd->new_oid), + NULL); - proc.no_stdin = 1; - proc.stdout_to_stderr = 1; - proc.err = use_sideband ? -1 : 0; - proc.trace2_hook_name = "update"; + prepare_sideband_async(&sideband_async, &saved_stderr, &sideband_async_started); - code = start_command(&proc); - if (code) - return code; - if (use_sideband) - copy_to_sideband(proc.err, -1, NULL); - return finish_command(&proc); + code = run_hooks_opt(the_repository, hook_name, &opt); + + finish_sideband_async(&sideband_async, saved_stderr, sideband_async_started); + + return code; } static struct command *find_command_by_refname(struct command *list, @@ -1639,34 +1678,29 @@ out: static void run_update_post_hook(struct command *commands) { + static const char hook_name[] = "post-update"; + 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; + int sideband_async_started = 0; + int saved_stderr = -1; - hook = find_hook(the_repository, "post-update"); - if (!hook) + if (!hook_exists(the_repository, hook_name)) return; 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, hook_name, &opt); + + finish_sideband_async(&sideband_async, saved_stderr, sideband_async_started); } static void check_aliased_update_internal(struct command *cmd, diff --git a/git.c b/git.c index 744cb6527e..6480ff8373 100644 --- a/git.c +++ b/git.c @@ -587,7 +587,7 @@ static struct cmd_struct commands[] = { { "hash-object", cmd_hash_object }, { "help", cmd_help }, { "history", cmd_history, RUN_SETUP }, - { "hook", cmd_hook, RUN_SETUP }, + { "hook", cmd_hook, RUN_SETUP_GENTLY }, { "index-pack", cmd_index_pack, RUN_SETUP_GENTLY | NO_PARSEOPT }, { "init", cmd_init_db }, { "init-db", cmd_init_db }, diff --git a/hook.c b/hook.c index b3de1048bf..4f4f060156 100644 --- a/hook.c +++ b/hook.c @@ -4,9 +4,11 @@ #include "gettext.h" #include "hook.h" #include "path.h" +#include "parse.h" #include "run-command.h" #include "config.h" #include "strbuf.h" +#include "strmap.h" #include "environment.h" #include "setup.h" @@ -16,6 +18,9 @@ const char *find_hook(struct repository *r, const char *name) int found_hook; + if (!r || !r->gitdir) + return NULL; + repo_git_path_replace(r, &path, "hooks/%s", name); found_hook = access(path.buf, X_OK) >= 0; #ifdef STRIP_EXTENSION @@ -47,42 +52,440 @@ const char *find_hook(struct repository *r, const char *name) return path.buf; } +/* + * Frees a struct hook stored as the util pointer of a string_list_item. + * Suitable for use as a string_list_clear_func_t callback. + */ +void hook_free(void *p, const char *str UNUSED) +{ + struct hook *h = p; + + if (!h) + return; + + if (h->kind == HOOK_TRADITIONAL) { + free((void *)h->u.traditional.path); + } else if (h->kind == HOOK_CONFIGURED) { + free((void *)h->u.configured.friendly_name); + free((void *)h->u.configured.command); + } + + if (h->data_free && h->feed_pipe_cb_data) + h->data_free(h->feed_pipe_cb_data); + + free(h); +} + +/* Helper to detect and add default "traditional" hooks from the hookdir. */ +static void list_hooks_add_default(struct repository *r, const char *hookname, + struct string_list *hook_list, + struct run_hooks_opt *options) +{ + const char *hook_path = find_hook(r, hookname); + struct hook *h; + + if (!hook_path) + return; + + CALLOC_ARRAY(h, 1); + + /* + * If the hook is to run in a specific dir, a relative path can + * become invalid in that dir, so convert to an absolute path. + */ + if (options && options->dir) + hook_path = absolute_path(hook_path); + + /* + * Setup per-hook internal state callback data. + * When provided, the alloc/free callbacks are always provided + * together, so use them to alloc/free the internal hook state. + */ + if (options && options->feed_pipe_cb_data_alloc) { + h->feed_pipe_cb_data = options->feed_pipe_cb_data_alloc(options->feed_pipe_ctx); + h->data_free = options->feed_pipe_cb_data_free; + } + + h->kind = HOOK_TRADITIONAL; + h->u.traditional.path = xstrdup(hook_path); + + string_list_append(hook_list, hook_path)->util = h; +} + +/* + * Cache entry stored as the .util pointer of string_list items inside the + * hook config cache. + */ +struct hook_config_cache_entry { + char *command; + enum config_scope scope; + int disabled; +}; + +/* + * Callback struct to collect all hook.* keys in a single config pass. + * commands: friendly-name to command map. + * event_hooks: event-name to list of friendly-names map. + * disabled_hooks: set of friendly-names with hook..enabled = false. + */ +struct hook_all_config_cb { + struct strmap commands; + struct strmap event_hooks; + struct string_list disabled_hooks; +}; + +/* repo_config() callback that collects all hook.* configuration in one pass. */ +static int hook_config_lookup_all(const char *key, const char *value, + const struct config_context *ctx, + void *cb_data) +{ + struct hook_all_config_cb *data = cb_data; + const char *name, *subkey; + char *hook_name; + size_t name_len = 0; + + if (parse_config_key(key, "hook", &name, &name_len, &subkey)) + return 0; + + if (!value) + return config_error_nonbool(key); + + /* Extract name, ensuring it is null-terminated. */ + hook_name = xmemdupz(name, name_len); + + if (!strcmp(subkey, "event")) { + if (!*value) { + /* Empty values reset previous events for this hook. */ + struct hashmap_iter iter; + struct strmap_entry *e; + + strmap_for_each_entry(&data->event_hooks, &iter, e) + unsorted_string_list_remove(e->value, hook_name, 0); + } else { + struct string_list *hooks = + strmap_get(&data->event_hooks, value); + + if (!hooks) { + CALLOC_ARRAY(hooks, 1); + string_list_init_dup(hooks); + strmap_put(&data->event_hooks, value, hooks); + } + + /* Re-insert if necessary to preserve last-seen order. */ + unsorted_string_list_remove(hooks, hook_name, 0); + + if (!ctx->kvi) + BUG("hook config callback called without key-value info"); + + /* + * Stash the config scope in the util pointer for + * later retrieval in build_hook_config_map(). This + * intermediate struct is transient and never leaves + * that function, so we pack the enum value into the + * pointer rather than heap-allocating a wrapper. + */ + string_list_append(hooks, hook_name)->util = + (void *)(uintptr_t)ctx->kvi->scope; + } + } else if (!strcmp(subkey, "command")) { + /* Store command overwriting the old value */ + char *old = strmap_put(&data->commands, hook_name, + xstrdup(value)); + free(old); + } else if (!strcmp(subkey, "enabled")) { + switch (git_parse_maybe_bool(value)) { + case 0: /* disabled */ + if (!unsorted_string_list_lookup(&data->disabled_hooks, + hook_name)) + string_list_append(&data->disabled_hooks, + hook_name); + break; + case 1: /* enabled: undo a prior disabled entry */ + unsorted_string_list_remove(&data->disabled_hooks, + hook_name, 0); + break; + default: + break; /* ignore unrecognised values */ + } + } + + free(hook_name); + return 0; +} + +/* + * The hook config cache maps each hook event name to a string_list where + * every item's string is the hook's friendly-name and its util pointer is + * a hook_config_cache_entry. All strings are owned by the map. + * + * Disabled hooks are kept in the cache with entry->disabled set, so that + * "git hook list" can display them. Hooks missing a command are filtered + * out at build time; if a disabled hook has no command it is silently + * skipped rather than triggering a fatal error. + */ +void hook_cache_clear(struct hook_config_cache *cache) +{ + struct hashmap_iter iter; + struct strmap_entry *e; + + strmap_for_each_entry(&cache->hooks, &iter, e) { + struct string_list *hooks = e->value; + for (size_t i = 0; i < hooks->nr; i++) { + struct hook_config_cache_entry *entry = hooks->items[i].util; + free(entry->command); + free(entry); + } + string_list_clear(hooks, 0); + free(hooks); + } + strmap_clear(&cache->hooks, 0); +} + +/* Populate `cache` with the complete hook configuration */ +static void build_hook_config_map(struct repository *r, + struct hook_config_cache *cache) +{ + struct hook_all_config_cb cb_data; + struct hashmap_iter iter; + struct strmap_entry *e; + + strmap_init(&cb_data.commands); + strmap_init(&cb_data.event_hooks); + string_list_init_dup(&cb_data.disabled_hooks); + + /* Parse all configs in one run. */ + repo_config(r, hook_config_lookup_all, &cb_data); + + /* Construct the cache from parsed configs. */ + strmap_for_each_entry(&cb_data.event_hooks, &iter, e) { + struct string_list *hook_names = e->value; + struct string_list *hooks; + CALLOC_ARRAY(hooks, 1); + + string_list_init_dup(hooks); + + for (size_t i = 0; i < hook_names->nr; i++) { + const char *hname = hook_names->items[i].string; + enum config_scope scope = + (enum config_scope)(uintptr_t)hook_names->items[i].util; + struct hook_config_cache_entry *entry; + char *command; + + int is_disabled = + !!unsorted_string_list_lookup( + &cb_data.disabled_hooks, hname); + + command = strmap_get(&cb_data.commands, hname); + if (!command) { + if (is_disabled) + warning(_("disabled hook '%s' has no " + "command configured"), hname); + else + die(_("'hook.%s.command' must be configured or " + "'hook.%s.event' must be removed;" + " aborting."), hname, hname); + } + + /* util stores a cache entry; owned by the cache. */ + CALLOC_ARRAY(entry, 1); + entry->command = command ? xstrdup(command) : NULL; + entry->scope = scope; + entry->disabled = is_disabled; + string_list_append(hooks, hname)->util = entry; + } + + strmap_put(&cache->hooks, e->key, hooks); + } + + strmap_clear(&cb_data.commands, 1); + string_list_clear(&cb_data.disabled_hooks, 0); + strmap_for_each_entry(&cb_data.event_hooks, &iter, e) { + string_list_clear(e->value, 0); + free(e->value); + } + strmap_clear(&cb_data.event_hooks, 0); +} + +/* + * Return the hook config cache for `r`, populating it first if needed. + * + * Out-of-repo calls (r->gitdir == NULL) allocate and return a temporary + * cache; the caller is responsible for freeing it with + * hook_cache_clear() + free(). + */ +static struct hook_config_cache *get_hook_config_cache(struct repository *r) +{ + struct hook_config_cache *cache = NULL; + + if (r && r->gitdir) { + /* + * For in-repo calls, the cache is stored in r->hook_config_cache, + * so repeated invocations don't parse the configs; allocate + * it just once on the first call. + */ + if (!r->hook_config_cache) { + CALLOC_ARRAY(r->hook_config_cache, 1); + strmap_init(&r->hook_config_cache->hooks); + build_hook_config_map(r, r->hook_config_cache); + } + cache = r->hook_config_cache; + } else { + /* + * Out-of-repo calls (no gitdir) allocate and return a temporary + * cache which gets freed immediately by the caller. + */ + CALLOC_ARRAY(cache, 1); + strmap_init(&cache->hooks); + build_hook_config_map(r, cache); + } + + return cache; +} + +static void list_hooks_add_configured(struct repository *r, + const char *hookname, + struct string_list *list, + struct run_hooks_opt *options) +{ + struct hook_config_cache *cache = get_hook_config_cache(r); + struct string_list *configured_hooks = strmap_get(&cache->hooks, hookname); + + /* Iterate through configured hooks and initialize internal states */ + for (size_t i = 0; configured_hooks && i < configured_hooks->nr; i++) { + const char *friendly_name = configured_hooks->items[i].string; + struct hook_config_cache_entry *entry = configured_hooks->items[i].util; + struct hook *hook; + CALLOC_ARRAY(hook, 1); + + /* + * When provided, the alloc/free callbacks are always provided + * together, so use them to alloc/free the internal hook state. + */ + if (options && options->feed_pipe_cb_data_alloc) { + hook->feed_pipe_cb_data = + options->feed_pipe_cb_data_alloc( + options->feed_pipe_ctx); + hook->data_free = options->feed_pipe_cb_data_free; + } + + hook->kind = HOOK_CONFIGURED; + hook->u.configured.friendly_name = xstrdup(friendly_name); + hook->u.configured.command = + entry->command ? xstrdup(entry->command) : NULL; + hook->u.configured.scope = entry->scope; + hook->u.configured.disabled = entry->disabled; + + string_list_append(list, friendly_name)->util = hook; + } + + /* + * Cleanup temporary cache for out-of-repo calls since they can't be + * stored persistently. Next out-of-repo calls will have to re-parse. + */ + if (!r || !r->gitdir) { + hook_cache_clear(cache); + free(cache); + } +} + +struct string_list *list_hooks(struct repository *r, const char *hookname, + struct run_hooks_opt *options) +{ + struct string_list *hook_head; + + if (!hookname) + BUG("null hookname was provided to hook_list()!"); + + CALLOC_ARRAY(hook_head, 1); + string_list_init_dup(hook_head); + + /* Add hooks from the config, e.g. hook.myhook.event = pre-commit */ + list_hooks_add_configured(r, hookname, hook_head, options); + + /* Add the default "traditional" hooks from hookdir. */ + list_hooks_add_default(r, hookname, hook_head, options); + + return hook_head; +} + int hook_exists(struct repository *r, const char *name) { - return !!find_hook(r, name); + struct string_list *hooks = list_hooks(r, name, NULL); + int exists = 0; + + for (size_t i = 0; i < hooks->nr; i++) { + struct hook *h = hooks->items[i].util; + if (h->kind == HOOK_TRADITIONAL || + !h->u.configured.disabled) { + exists = 1; + break; + } + } + string_list_clear_func(hooks, hook_free); + free(hooks); + return exists; } static int pick_next_hook(struct child_process *cp, struct strbuf *out UNUSED, void *pp_cb, - void **pp_task_cb UNUSED) + void **pp_task_cb) { struct hook_cb_data *hook_cb = pp_cb; - const char *hook_path = hook_cb->hook_path; + struct string_list *hook_list = hook_cb->hook_command_list; + struct hook *h; - if (!hook_path) - return 0; + do { + if (hook_cb->hook_to_run_index >= hook_list->nr) + return 0; + h = hook_list->items[hook_cb->hook_to_run_index++].util; + } while (h->kind == HOOK_CONFIGURED && h->u.configured.disabled); cp->no_stdin = 1; strvec_pushv(&cp->env, hook_cb->options->env.v); + + if (hook_cb->options->path_to_stdin && hook_cb->options->feed_pipe) + BUG("options path_to_stdin and feed_pipe are mutually exclusive"); + /* reopen the file for stdin; run_command closes it. */ if (hook_cb->options->path_to_stdin) { cp->no_stdin = 0; cp->in = xopen(hook_cb->options->path_to_stdin, O_RDONLY); } - cp->stdout_to_stderr = 1; + + if (hook_cb->options->feed_pipe) { + cp->no_stdin = 0; + /* start_command() will allocate a pipe / stdin fd for us */ + cp->in = -1; + } + + cp->stdout_to_stderr = hook_cb->options->stdout_to_stderr; cp->trace2_hook_name = hook_cb->hook_name; cp->dir = hook_cb->options->dir; - strvec_push(&cp->args, hook_path); + /* Add hook exec paths or commands */ + if (h->kind == HOOK_TRADITIONAL) { + strvec_push(&cp->args, h->u.traditional.path); + } else if (h->kind == HOOK_CONFIGURED) { + /* to enable oneliners, let config-specified hooks run in shell. */ + cp->use_shell = true; + if (!h->u.configured.command) + BUG("non-disabled HOOK_CONFIGURED hook has no command"); + strvec_push(&cp->args, h->u.configured.command); + } else { + BUG("unknown hook kind"); + } + + if (!cp->args.nr) + BUG("hook must have at least one command or exec path"); + strvec_pushv(&cp->args, hook_cb->options->args.v); /* - * This pick_next_hook() will be called again, we're only - * running one hook, so indicate that no more work will be - * done. + * Provide per-hook internal state via task_cb for easy access, so + * hook callbacks don't have to go through hook_cb->options. */ - hook_cb->hook_path = NULL; + *pp_task_cb = h->feed_pipe_cb_data; return 1; } @@ -123,23 +526,22 @@ static void run_hooks_opt_clear(struct run_hooks_opt *options) int run_hooks_opt(struct repository *r, const char *hook_name, struct run_hooks_opt *options) { - struct strbuf abs_path = STRBUF_INIT; struct hook_cb_data cb_data = { .rc = 0, .hook_name = hook_name, .options = options, }; - const char *const hook_path = find_hook(r, hook_name); int ret = 0; const struct run_process_parallel_opts opts = { .tr2_category = "hook", .tr2_label = hook_name, - .processes = 1, - .ungroup = 1, + .processes = options->jobs, + .ungroup = options->jobs == 1, .get_next_task = pick_next_hook, .start_failure = notify_start_failure, + .feed_pipe = options->feed_pipe, .task_finished = notify_hook_finished, .data = &cb_data, @@ -148,27 +550,34 @@ int run_hooks_opt(struct repository *r, const char *hook_name, if (!options) BUG("a struct run_hooks_opt must be provided to run_hooks"); + if (options->path_to_stdin && options->feed_pipe) + BUG("options path_to_stdin and feed_pipe are mutually exclusive"); + + if (!options->jobs) + BUG("run_hooks_opt must be called with options.jobs >= 1"); + + /* + * Ensure cb_data copy and free functions are either provided together, + * or neither one is provided. + */ + if (!options->feed_pipe_cb_data_alloc != !options->feed_pipe_cb_data_free) + BUG("feed_pipe_cb_data_alloc and feed_pipe_cb_data_free must be set together"); + if (options->invoked_hook) *options->invoked_hook = 0; - if (!hook_path && !options->error_if_missing) + cb_data.hook_command_list = list_hooks(r, hook_name, options); + if (!cb_data.hook_command_list->nr) { + if (options->error_if_missing) + ret = error("cannot find a hook named %s", hook_name); goto cleanup; - - if (!hook_path) { - ret = error("cannot find a hook named %s", hook_name); - goto cleanup; - } - - cb_data.hook_path = hook_path; - if (options->dir) { - strbuf_add_absolute_path(&abs_path, hook_path); - cb_data.hook_path = abs_path.buf; } run_processes_parallel(&opts); ret = cb_data.rc; cleanup: - strbuf_release(&abs_path); + string_list_clear_func(cb_data.hook_command_list, hook_free); + free(cb_data.hook_command_list); run_hooks_opt_clear(options); return ret; } diff --git a/hook.h b/hook.h index 11863fa734..0432df963f 100644 --- a/hook.h +++ b/hook.h @@ -1,9 +1,62 @@ #ifndef HOOK_H #define HOOK_H +#include "config.h" #include "strvec.h" +#include "run-command.h" +#include "string-list.h" +#include "strmap.h" struct repository; +typedef void (*hook_data_free_fn)(void *data); +typedef void *(*hook_data_alloc_fn)(void *init_ctx); + +/** + * Represents a hook command to be run. + * Hooks can be: + * 1. "traditional" (found in the hooks directory) + * 2. "configured" (defined in Git's configuration via hook..event). + * The 'kind' field determines which part of the union 'u' is valid. + */ +struct hook { + enum { + HOOK_TRADITIONAL, + HOOK_CONFIGURED, + } kind; + union { + struct { + const char *path; + } traditional; + struct { + const char *friendly_name; + const char *command; + enum config_scope scope; + int disabled; + } configured; + } u; + + /** + * Opaque data pointer used to keep internal state across callback calls. + * + * It can be accessed directly via the third hook callback arg: + * struct ... *state = pp_task_cb; + * + * The caller is responsible for managing the memory for this data by + * providing alloc/free callbacks to `run_hooks_opt`. + * + * Only useful when using `run_hooks_opt.feed_pipe`, otherwise ignore it. + */ + void *feed_pipe_cb_data; + + /** + * Callback to free `feed_pipe_cb_data`. + * + * It is called automatically and points to the `feed_pipe_cb_data_free` + * provided via the `run_hook_opt` parameter. + */ + hook_data_free_fn data_free; +}; + struct run_hooks_opt { /* Environment vars to be set for each hook */ @@ -15,6 +68,14 @@ struct run_hooks_opt /* Emit an error if the hook is missing */ unsigned int error_if_missing:1; + /** + * Number of processes to parallelize across. + * + * If > 1, output will be buffered and de-interleaved (ungroup=0). + * If == 1, output will be real-time (ungroup=1). + */ + unsigned int jobs; + /** * An optional initial working directory for the hook, * translates to "struct child_process"'s "dir" member. @@ -33,26 +94,129 @@ struct run_hooks_opt */ int *invoked_hook; + /** + * Send the hook's stdout to stderr. + * + * This is the default behavior for all hooks except pre-push, + * which has separate stdout and stderr streams for backwards + * compatibility reasons. + */ + unsigned int stdout_to_stderr:1; + /** * Path to file which should be piped to stdin for each hook. */ const char *path_to_stdin; + + /** + * Callback used to incrementally feed a child hook stdin pipe. + * + * Useful especially if a hook consumes large quantities of data + * (e.g. a list of all refs in a client push), so feeding it via + * in-memory strings or slurping to/from files is inefficient. + * While the callback allows piecemeal writing, it can also be + * used for smaller inputs, where it gets called only once. + * + * Add hook callback initalization context to `feed_pipe_ctx`. + * Add hook callback internal state to `feed_pipe_cb_data`. + * + */ + feed_pipe_fn feed_pipe; + + /** + * Opaque data pointer used to pass context to `feed_pipe_fn`. + * + * It can be accessed via the second callback arg 'pp_cb': + * ((struct hook_cb_data *) pp_cb)->hook_cb->options->feed_pipe_ctx; + * + * The caller is responsible for managing the memory for this data. + * Only useful when using `run_hooks_opt.feed_pipe`, otherwise ignore it. + */ + void *feed_pipe_ctx; + + /** + * Some hooks need to create a fresh `feed_pipe_cb_data` internal state, + * so they can keep track of progress without affecting one another. + * + * If provided, this function will be called to alloc & initialize the + * `feed_pipe_cb_data` for each hook. + * + * The `feed_pipe_ctx` pointer can be used to pass initialization data. + */ + hook_data_alloc_fn feed_pipe_cb_data_alloc; + + /** + * Called to free the memory initialized by `feed_pipe_cb_data_alloc`. + * + * Must always be provided when `feed_pipe_cb_data_alloc` is provided. + */ + hook_data_free_fn feed_pipe_cb_data_free; }; #define RUN_HOOKS_OPT_INIT { \ .env = STRVEC_INIT, \ .args = STRVEC_INIT, \ + .stdout_to_stderr = 1, \ + .jobs = 1, \ } struct hook_cb_data { /* rc reflects the cumulative failure state */ int rc; const char *hook_name; - const char *hook_path; + + /** + * A list of hook commands/paths to run for the 'hook_name' event. + * + * The 'string' member of each item holds the path (for traditional hooks) + * or the unique friendly-name for hooks specified in configs. + * The 'util' member of each item points to the corresponding struct hook. + */ + struct string_list *hook_command_list; + + /* Iterator/cursor for the above list, pointing to the next hook to run. */ + size_t hook_to_run_index; + struct run_hooks_opt *options; }; -/* +/** + * Provides a list of hook commands to run for the 'hookname' event. + * + * This function consolidates hooks from two sources: + * 1. The config-based hooks (not yet implemented). + * 2. The "traditional" hook found in the repository hooks directory + * (e.g., .git/hooks/pre-commit). + * + * The list is ordered by execution priority. + * + * The caller is responsible for freeing the memory of the returned list + * using string_list_clear() and free(). + */ +struct string_list *list_hooks(struct repository *r, const char *hookname, + struct run_hooks_opt *options); + +/** + * Frees a struct hook stored as the util pointer of a string_list_item. + * Suitable for use as a string_list_clear_func_t callback. + */ +void hook_free(void *p, const char *str UNUSED); + +/** + * Persistent cache for hook configuration, stored on `struct repository`. + * Populated lazily on first hook use and freed by repo_clear(). + */ +struct hook_config_cache { + struct strmap hooks; /* maps event name -> string_list of hooks */ +}; + +/** + * Frees the hook configuration cache stored in `struct repository`. + * Called by repo_clear(). + */ +void hook_cache_clear(struct hook_config_cache *cache); + +/** * Returns the path to the hook file, or NULL if the hook is missing * or disabled. Note that this points to static storage that will be * overwritten by further calls to find_hook and run_hook_*. diff --git a/refs.c b/refs.c index 0ad1da990d..bd91c5c882 100644 --- a/refs.c +++ b/refs.c @@ -16,7 +16,6 @@ #include "iterator.h" #include "refs.h" #include "refs/refs-internal.h" -#include "run-command.h" #include "hook.h" #include "object-name.h" #include "odb.h" @@ -27,7 +26,6 @@ #include "strvec.h" #include "repo-settings.h" #include "setup.h" -#include "sigchain.h" #include "date.h" #include "commit.h" #include "wildmatch.h" @@ -2551,68 +2549,87 @@ static int ref_update_reject_duplicates(struct string_list *refnames, return 0; } +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 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; + + if (i >= transaction->nr) + return 1; /* No more refs to process */ + + update = transaction->updates[i]; + + if (update->flags & REF_LOG_ONLY) + return 0; + + strbuf_reset(buf); + + 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_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_addf(buf, "%s\n", update->refname); + + ret = write_in_full(hook_stdin_fd, buf->buf, buf->len); + if (ret < 0 && errno != EPIPE) + return ret; + + return 0; /* no more input to feed */ +} + +static void *transaction_feed_cb_data_alloc(void *feed_pipe_ctx UNUSED) +{ + struct transaction_feed_cb_data *data; + CALLOC_ARRAY(data, 1); + strbuf_init(&data->buf, 0); + data->index = 0; + return data; +} + +static void transaction_feed_cb_data_free(void *data) +{ + struct transaction_feed_cb_data *d = data; + if (!d) + return; + strbuf_release(&d->buf); + free(d); +} + static int run_transaction_hook(struct ref_transaction *transaction, const char *state) { - struct child_process proc = CHILD_PROCESS_INIT; - struct strbuf buf = STRBUF_INIT; - const char *hook; + struct run_hooks_opt opt = RUN_HOOKS_OPT_INIT; int ret = 0; - hook = find_hook(transaction->ref_store->repo, "reference-transaction"); - if (!hook) - return ret; + strvec_push(&opt.args, state); - strvec_pushl(&proc.args, hook, state, NULL); - proc.in = -1; - proc.stdout_to_stderr = 1; - proc.trace2_hook_name = "reference-transaction"; + opt.feed_pipe = transaction_hook_feed_stdin; + opt.feed_pipe_ctx = transaction; + opt.feed_pipe_cb_data_alloc = transaction_feed_cb_data_alloc; + opt.feed_pipe_cb_data_free = transaction_feed_cb_data_free; - ret = start_command(&proc); - if (ret) - return ret; + ret = run_hooks_opt(transaction->ref_store->repo, "reference-transaction", &opt); - sigchain_push(SIGPIPE, SIG_IGN); - - for (size_t i = 0; i < transaction->nr; i++) { - struct ref_update *update = transaction->updates[i]; - - if (update->flags & REF_LOG_ONLY) - continue; - - strbuf_reset(&buf); - - 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_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_addf(&buf, "%s\n", update->refname); - - 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; - } - } - - close(proc.in); - sigchain_pop(SIGPIPE); - strbuf_release(&buf); - - ret |= finish_command(&proc); return ret; } diff --git a/repository.c b/repository.c index e7fa42c14f..8717a1693a 100644 --- a/repository.c +++ b/repository.c @@ -1,6 +1,7 @@ #include "git-compat-util.h" #include "abspath.h" #include "repository.h" +#include "hook.h" #include "odb.h" #include "config.h" #include "object.h" @@ -413,6 +414,11 @@ void repo_clear(struct repository *repo) FREE_AND_NULL(repo->index); } + if (repo->hook_config_cache) { + hook_cache_clear(repo->hook_config_cache); + FREE_AND_NULL(repo->hook_config_cache); + } + if (repo->promisor_remote_config) { promisor_remote_clear(repo->promisor_remote_config); FREE_AND_NULL(repo->promisor_remote_config); diff --git a/repository.h b/repository.h index 9ad6520c37..8d6e0b3dd2 100644 --- a/repository.h +++ b/repository.h @@ -12,6 +12,7 @@ struct lock_file; struct pathspec; struct object_database; struct submodule_cache; +struct hook_config_cache; struct promisor_remote_config; struct remote_state; @@ -166,6 +167,12 @@ struct repository { /* True if commit-graph has been disabled within this process. */ int commit_graph_disabled; + /* + * Lazily-populated cache mapping hook event names to configured hooks. + * NULL until first hook use. + */ + struct hook_config_cache *hook_config_cache; + /* Configurations related to promisor remotes. */ char *repository_format_partial_clone; struct promisor_remote_config *promisor_remote_config; diff --git a/run-command.c b/run-command.c index 438a290d30..b27064ef57 100644 --- a/run-command.c +++ b/run-command.c @@ -1478,15 +1478,40 @@ enum child_state { GIT_CP_WAIT_CLEANUP, }; +struct parallel_child { + enum child_state state; + struct child_process process; + struct strbuf err; + void *data; +}; + +static int child_is_working(const struct parallel_child *pp_child) +{ + return pp_child->state == GIT_CP_WORKING; +} + +static int child_is_ready_for_cleanup(const struct parallel_child *pp_child) +{ + return child_is_working(pp_child) && !pp_child->process.in; +} + +static int child_is_receiving_input(const struct parallel_child *pp_child) +{ + return child_is_working(pp_child) && pp_child->process.in > 0; +} +static int child_is_sending_output(const struct parallel_child *pp_child) +{ + /* + * all pp children which buffer output through run_command via ungroup=0 + * redirect stdout to stderr, so we just need to check process.err. + */ + return child_is_working(pp_child) && pp_child->process.err > 0; +} + struct parallel_processes { size_t nr_processes; - struct { - enum child_state state; - struct child_process process; - struct strbuf err; - void *data; - } *children; + struct parallel_child *children; /* * The struct pollfd is logically part of *children, * but the system call expects it as its own array. @@ -1509,7 +1534,7 @@ static void kill_children(const struct parallel_processes *pp, int signo) { for (size_t i = 0; i < opts->processes; i++) - if (pp->children[i].state == GIT_CP_WORKING) + if (child_is_working(&pp->children[i])) kill(pp->children[i].process.pid, signo); } @@ -1545,7 +1570,7 @@ static void pp_init(struct parallel_processes *pp, CALLOC_ARRAY(pp->children, n); if (!opts->ungroup) - CALLOC_ARRAY(pp->pfd, n); + CALLOC_ARRAY(pp->pfd, n * 2); for (size_t i = 0; i < n; i++) { strbuf_init(&pp->children[i].err, 0); @@ -1652,21 +1677,101 @@ static int pp_start_one(struct parallel_processes *pp, return 0; } -static void pp_buffer_stderr(struct parallel_processes *pp, - const struct run_process_parallel_opts *opts, - int output_timeout) +static void pp_buffer_stdin(struct parallel_processes *pp, + const struct run_process_parallel_opts *opts) { - while (poll(pp->pfd, opts->processes, output_timeout) < 0) { + /* Buffer stdin for each pipe. */ + for (size_t i = 0; i < opts->processes; i++) { + struct child_process *proc = &pp->children[i].process; + int ret; + + if (!child_is_receiving_input(&pp->children[i])) + continue; + + /* + * child input is provided via path_to_stdin when the feed_pipe cb is + * missing, so we just signal an EOF. + */ + if (!opts->feed_pipe) { + close(proc->in); + proc->in = 0; + continue; + } + + /** + * Feed the pipe: + * ret < 0 means error + * ret == 0 means there is more data to be fed + * ret > 0 means feeding finished + */ + ret = opts->feed_pipe(proc->in, opts->data, pp->children[i].data); + if (ret < 0) + die_errno("feed_pipe"); + + if (ret) { + close(proc->in); + proc->in = 0; + } + } +} + +static void pp_buffer_io(struct parallel_processes *pp, + const struct run_process_parallel_opts *opts, + int timeout) +{ + /* for each potential child slot, prepare two pollfd entries */ + for (size_t i = 0; i < opts->processes; i++) { + if (child_is_sending_output(&pp->children[i])) { + pp->pfd[2*i].fd = pp->children[i].process.err; + pp->pfd[2*i].events = POLLIN | POLLHUP; + } else { + pp->pfd[2*i].fd = -1; + } + + if (child_is_receiving_input(&pp->children[i])) { + pp->pfd[2*i+1].fd = pp->children[i].process.in; + pp->pfd[2*i+1].events = POLLOUT; + } else { + pp->pfd[2*i+1].fd = -1; + } + } + + while (poll(pp->pfd, opts->processes * 2, timeout) < 0) { if (errno == EINTR) continue; pp_cleanup(pp, opts); die_errno("poll"); } - /* Buffer output from all pipes. */ for (size_t i = 0; i < opts->processes; i++) { - if (pp->children[i].state == GIT_CP_WORKING && - pp->pfd[i].revents & (POLLIN | POLLHUP)) { + /* Handle input feeding (stdin) */ + if (pp->pfd[2*i+1].revents & (POLLOUT | POLLHUP | POLLERR)) { + if (opts->feed_pipe) { + int ret = opts->feed_pipe(pp->children[i].process.in, + opts->data, + pp->children[i].data); + if (ret < 0) + die_errno("feed_pipe"); + if (ret) { + /* done feeding */ + close(pp->children[i].process.in); + pp->children[i].process.in = 0; + } + } else { + /* + * No feed_pipe means there is nothing to do, so + * close the fd. Child input can be fed by other + * methods, such as opts->path_to_stdin which + * slurps a file via dup2, so clean up here. + */ + close(pp->children[i].process.in); + pp->children[i].process.in = 0; + } + } + + /* Handle output reading (stderr) */ + if (child_is_working(&pp->children[i]) && + pp->pfd[2*i].revents & (POLLIN | POLLHUP)) { int n = strbuf_read_once(&pp->children[i].err, pp->children[i].process.err, 0); if (n == 0) { @@ -1683,7 +1788,7 @@ static void pp_output(const struct parallel_processes *pp) { size_t i = pp->output_owner; - if (pp->children[i].state == GIT_CP_WORKING && + if (child_is_working(&pp->children[i]) && pp->children[i].err.len) { strbuf_write(&pp->children[i].err, stderr); strbuf_reset(&pp->children[i].err); @@ -1722,6 +1827,7 @@ static int pp_collect_finished(struct parallel_processes *pp, pp->children[i].state = GIT_CP_FREE; if (pp->pfd) pp->pfd[i].fd = -1; + pp->children[i].process.in = 0; child_process_init(&pp->children[i].process); if (opts->ungroup) { @@ -1748,7 +1854,7 @@ static int pp_collect_finished(struct parallel_processes *pp, * running process time. */ for (i = 0; i < n; i++) - if (pp->children[(pp->output_owner + i) % n].state == GIT_CP_WORKING) + if (child_is_working(&pp->children[(pp->output_owner + i) % n])) break; pp->output_owner = (pp->output_owner + i) % n; } @@ -1756,10 +1862,25 @@ static int pp_collect_finished(struct parallel_processes *pp, return result; } +static void pp_handle_child_IO(struct parallel_processes *pp, + const struct run_process_parallel_opts *opts, + int timeout) +{ + if (opts->ungroup) { + pp_buffer_stdin(pp, opts); + for (size_t i = 0; i < opts->processes; i++) + if (child_is_ready_for_cleanup(&pp->children[i])) + pp->children[i].state = GIT_CP_WAIT_CLEANUP; + } else { + pp_buffer_io(pp, opts, timeout); + pp_output(pp); + } +} + void run_processes_parallel(const struct run_process_parallel_opts *opts) { int i, code; - int output_timeout = 100; + int timeout = 100; int spawn_cap = 4; struct parallel_processes_for_signal pp_sig; struct parallel_processes pp = { @@ -1775,6 +1896,13 @@ void run_processes_parallel(const struct run_process_parallel_opts *opts) "max:%"PRIuMAX, (uintmax_t)opts->processes); + /* + * Child tasks might receive input via stdin, terminating early (or not), so + * ignore the default SIGPIPE which gets handled by each feed_pipe_fn which + * actually writes the data to children stdin fds. + */ + sigchain_push(SIGPIPE, SIG_IGN); + pp_init(&pp, opts, &pp_sig); while (1) { for (i = 0; @@ -1792,13 +1920,7 @@ void run_processes_parallel(const struct run_process_parallel_opts *opts) } if (!pp.nr_processes) break; - if (opts->ungroup) { - for (size_t i = 0; i < opts->processes; i++) - pp.children[i].state = GIT_CP_WAIT_CLEANUP; - } else { - pp_buffer_stderr(&pp, opts, output_timeout); - pp_output(&pp); - } + pp_handle_child_IO(&pp, opts, timeout); code = pp_collect_finished(&pp, opts); if (code) { pp.shutdown = 1; @@ -1809,6 +1931,8 @@ void run_processes_parallel(const struct run_process_parallel_opts *opts) pp_cleanup(&pp, opts); + sigchain_pop(SIGPIPE); + if (do_trace2) trace2_region_leave(tr2_category, tr2_label, NULL); } diff --git a/run-command.h b/run-command.h index 0df25e445f..e1ca965b5b 100644 --- a/run-command.h +++ b/run-command.h @@ -420,6 +420,21 @@ typedef int (*start_failure_fn)(struct strbuf *out, void *pp_cb, void *pp_task_cb); +/** + * This callback is repeatedly called on every child process who requests + * start_command() to create a pipe by setting child_process.in < 0. + * + * pp_cb is the callback cookie as passed into run_processes_parallel, and + * pp_task_cb is the callback cookie as passed into get_next_task_fn. + * + * Returns < 0 for error + * Returns == 0 when there is more data to be fed (will be called again) + * Returns > 0 when finished (child closed fd or no more data to be fed) + */ +typedef int (*feed_pipe_fn)(int child_in, + void *pp_cb, + void *pp_task_cb); + /** * This callback is called on every child process that finished processing. * @@ -473,6 +488,12 @@ struct run_process_parallel_opts */ start_failure_fn start_failure; + /* + * feed_pipe: see feed_pipe_fn() above. This can be NULL to omit any + * special handling. + */ + feed_pipe_fn feed_pipe; + /** * task_finished: See task_finished_fn() above. This can be * NULL to omit any special handling. diff --git a/sequencer.c b/sequencer.c index a3eb39bb25..aafd0bc959 100644 --- a/sequencer.c +++ b/sequencer.c @@ -1292,32 +1292,40 @@ int update_head_with_reflog(const struct commit *old_head, return ret; } +static int pipe_from_strbuf(int hook_stdin_fd, void *pp_cb, void *pp_task_cb UNUSED) +{ + struct hook_cb_data *hook_cb = pp_cb; + struct strbuf *to_pipe = hook_cb->options->feed_pipe_ctx; + int ret; + + if (!to_pipe) + BUG("pipe_from_strbuf called without feed_pipe_ctx"); + + ret = write_in_full(hook_stdin_fd, to_pipe->buf, to_pipe->len); + if (ret < 0 && errno != EPIPE) + return ret; + + return 1; /* done writing */ +} + static int run_rewrite_hook(const struct object_id *oldoid, const struct object_id *newoid) { - struct child_process proc = CHILD_PROCESS_INIT; + struct run_hooks_opt opt = RUN_HOOKS_OPT_INIT; int code; struct strbuf sb = STRBUF_INIT; - const char *hook_path = find_hook(the_repository, "post-rewrite"); - if (!hook_path) - return 0; - - strvec_pushl(&proc.args, hook_path, "amend", NULL); - proc.in = -1; - proc.stdout_to_stderr = 1; - proc.trace2_hook_name = "post-rewrite"; - - code = start_command(&proc); - if (code) - return code; strbuf_addf(&sb, "%s %s\n", oid_to_hex(oldoid), oid_to_hex(newoid)); - sigchain_push(SIGPIPE, SIG_IGN); - write_in_full(proc.in, sb.buf, sb.len); - close(proc.in); + + opt.feed_pipe_ctx = &sb; + opt.feed_pipe = pipe_from_strbuf; + + strvec_push(&opt.args, "amend"); + + code = run_hooks_opt(the_repository, "post-rewrite", &opt); + strbuf_release(&sb); - sigchain_pop(SIGPIPE); - return finish_command(&proc); + return code; } void commit_post_rewrite(struct repository *r, diff --git a/string-list.c b/string-list.c index fffa2ad4b6..d260b873c8 100644 --- a/string-list.c +++ b/string-list.c @@ -281,6 +281,15 @@ void unsorted_string_list_delete_item(struct string_list *list, int i, int free_ list->nr--; } +void unsorted_string_list_remove(struct string_list *list, const char *str, + int free_util) +{ + struct string_list_item *item = unsorted_string_list_lookup(list, str); + if (item) + unsorted_string_list_delete_item(list, item - list->items, + free_util); +} + /* * append a substring [p..end] to list; return number of things it * appended to the list. diff --git a/string-list.h b/string-list.h index 3ad862a187..b86ee7c099 100644 --- a/string-list.h +++ b/string-list.h @@ -265,6 +265,14 @@ struct string_list_item *unsorted_string_list_lookup(struct string_list *list, */ void unsorted_string_list_delete_item(struct string_list *list, int i, int free_util); +/** + * Remove the first item matching `str` from an unsorted string_list. + * No-op if `str` is not found. If `free_util` is non-zero, the `util` + * pointer of the removed item is freed before deletion. + */ +void unsorted_string_list_remove(struct string_list *list, const char *str, + int free_util); + /** * Split string into substrings on characters in `delim` and append the * substrings to `list`. The input string is not modified. diff --git a/t/helper/test-run-command.c b/t/helper/test-run-command.c index 3719f23cc2..4a56456894 100644 --- a/t/helper/test-run-command.c +++ b/t/helper/test-run-command.c @@ -23,19 +23,26 @@ static int number_callbacks; static int parallel_next(struct child_process *cp, struct strbuf *err, void *cb, - void **task_cb UNUSED) + void **task_cb) { struct child_process *d = cb; if (number_callbacks >= 4) return 0; strvec_pushv(&cp->args, d->args.v); + cp->in = d->in; + cp->no_stdin = d->no_stdin; if (err) strbuf_addstr(err, "preloaded output of a child\n"); else fprintf(stderr, "preloaded output of a child\n"); number_callbacks++; + + /* test_stdin callback will use this to count remaining lines */ + *task_cb = xmalloc(sizeof(int)); + *(int*)(*task_cb) = 2; + return 1; } @@ -54,15 +61,48 @@ static int no_job(struct child_process *cp UNUSED, static int task_finished(int result UNUSED, struct strbuf *err, void *pp_cb UNUSED, - void *pp_task_cb UNUSED) + void *pp_task_cb) { if (err) strbuf_addstr(err, "asking for a quick stop\n"); else fprintf(stderr, "asking for a quick stop\n"); + + FREE_AND_NULL(pp_task_cb); + return 1; } +static int task_finished_quiet(int result UNUSED, + struct strbuf *err UNUSED, + void *pp_cb UNUSED, + void *pp_task_cb) +{ + FREE_AND_NULL(pp_task_cb); + return 0; +} + +static int test_stdin_pipe_feed(int hook_stdin_fd, void *cb UNUSED, void *task_cb) +{ + int *lines_remaining = task_cb; + + if (*lines_remaining) { + struct strbuf buf = STRBUF_INIT; + strbuf_addf(&buf, "sample stdin %d\n", --(*lines_remaining)); + if (write_in_full(hook_stdin_fd, buf.buf, buf.len) < 0) { + if (errno == EPIPE) { + /* child closed stdin, nothing more to do */ + strbuf_release(&buf); + return 1; + } + die_errno("write"); + } + strbuf_release(&buf); + } + + return !(*lines_remaining); +} + struct testsuite { struct string_list tests, failed; int next; @@ -157,6 +197,7 @@ static int testsuite(int argc, const char **argv) struct run_process_parallel_opts opts = { .get_next_task = next_test, .start_failure = test_failed, + .feed_pipe = test_stdin_pipe_feed, .task_finished = test_finished, .data = &suite, }; @@ -460,12 +501,19 @@ int cmd__run_command(int argc, const char **argv) if (!strcmp(argv[1], "run-command-parallel")) { opts.get_next_task = parallel_next; + opts.task_finished = task_finished_quiet; } else if (!strcmp(argv[1], "run-command-abort")) { opts.get_next_task = parallel_next; opts.task_finished = task_finished; } else if (!strcmp(argv[1], "run-command-no-jobs")) { opts.get_next_task = no_job; opts.task_finished = task_finished; + } else if (!strcmp(argv[1], "run-command-stdin")) { + proc.in = -1; + proc.no_stdin = 0; + opts.get_next_task = parallel_next; + opts.task_finished = task_finished_quiet; + opts.feed_pipe = test_stdin_pipe_feed; } else { ret = 1; fprintf(stderr, "check usage\n"); diff --git a/t/t0061-run-command.sh b/t/t0061-run-command.sh index 76d4936a87..2f77fde0d9 100755 --- a/t/t0061-run-command.sh +++ b/t/t0061-run-command.sh @@ -164,6 +164,37 @@ test_expect_success 'run_command runs ungrouped in parallel with more tasks than test_line_count = 4 err ' +test_expect_success 'run_command listens to stdin' ' + cat >expect <<-\EOF && + preloaded output of a child + listening for stdin: + sample stdin 1 + sample stdin 0 + preloaded output of a child + listening for stdin: + sample stdin 1 + sample stdin 0 + preloaded output of a child + listening for stdin: + sample stdin 1 + sample stdin 0 + preloaded output of a child + listening for stdin: + sample stdin 1 + sample stdin 0 + EOF + + write_script stdin-script <<-\EOF && + echo "listening for stdin:" + while read line + do + echo "$line" + done + EOF + test-tool run-command run-command-stdin 2 ./stdin-script 2>actual && + test_cmp expect actual +' + cat >expect <<-EOF preloaded output of a child asking for a quick stop diff --git a/t/t1800-hook.sh b/t/t1800-hook.sh index 4feaf0d7be..cbf7d2bf80 100755 --- a/t/t1800-hook.sh +++ b/t/t1800-hook.sh @@ -1,18 +1,78 @@ #!/bin/sh -test_description='git-hook command' +test_description='git-hook command and config-managed multihooks' . ./test-lib.sh . "$TEST_DIRECTORY"/lib-terminal.sh +setup_hooks () { + test_config hook.ghi.command "/path/ghi" + test_config hook.ghi.event pre-commit --add + test_config hook.ghi.event test-hook --add + test_config_global hook.def.command "/path/def" + test_config_global hook.def.event pre-commit --add +} + +setup_hookdir () { + mkdir .git/hooks + write_script .git/hooks/pre-commit <<-EOF + echo \"Legacy Hook\" + EOF + test_when_finished rm -rf .git/hooks +} + test_expect_success 'git hook usage' ' test_expect_code 129 git hook && test_expect_code 129 git hook run && test_expect_code 129 git hook run -h && test_expect_code 129 git hook run --unknown 2>err && + test_expect_code 129 git hook list && + test_expect_code 129 git hook list -h && grep "unknown option" err ' +test_expect_success 'git hook list: nonexistent hook' ' + cat >stderr.expect <<-\EOF && + warning: no hooks found for event '\''test-hook'\'' + EOF + test_expect_code 1 git hook list test-hook 2>stderr.actual && + test_cmp stderr.expect stderr.actual +' + +test_expect_success 'git hook list: traditional hook from hookdir' ' + test_hook test-hook <<-EOF && + echo Test hook + EOF + + cat >expect <<-\EOF && + hook from hookdir + EOF + git hook list test-hook >actual && + test_cmp expect actual +' + +test_expect_success 'git hook list: configured hook' ' + test_config hook.myhook.command "echo Hello" && + test_config hook.myhook.event test-hook --add && + + echo "myhook" >expect && + git hook list test-hook >actual && + test_cmp expect actual +' + +test_expect_success 'git hook list: -z shows NUL-terminated output' ' + test_hook test-hook <<-EOF && + echo Test hook + EOF + test_config hook.myhook.command "echo Hello" && + test_config hook.myhook.event test-hook --add && + + printf "myhookQhook from hookdirQ" >expect && + git hook list -z test-hook >actual.raw && + nul_to_q actual && + test_cmp expect actual +' + test_expect_success 'git hook run: nonexistent hook' ' cat >stderr.expect <<-\EOF && error: cannot find a hook named test-hook @@ -83,12 +143,18 @@ test_expect_success 'git hook run -- pass arguments' ' test_cmp expect actual ' -test_expect_success 'git hook run -- out-of-repo runs excluded' ' - test_hook test-hook <<-EOF && - echo Test hook - EOF +test_expect_success 'git hook run: out-of-repo runs execute global hooks' ' + test_config_global hook.global-hook.event test-hook --add && + test_config_global hook.global-hook.command "echo no repo no problems" --add && - nongit test_must_fail git hook run test-hook + echo "global-hook" >expect && + nongit git hook list test-hook >actual && + test_cmp expect actual && + + echo "no repo no problems" >expect && + + nongit git hook run test-hook 2>actual && + test_cmp expect actual ' test_expect_success 'git -c core.hooksPath= hook run' ' @@ -150,6 +216,244 @@ test_expect_success TTY 'git commit: stdout and stderr are connected to a TTY' ' test_hook_tty commit -m"B.new" ' +test_expect_success 'git hook list orders by config order' ' + setup_hooks && + + cat >expected <<-\EOF && + def + ghi + EOF + + git hook list pre-commit >actual && + test_cmp expected actual +' + +test_expect_success 'git hook list reorders on duplicate event declarations' ' + setup_hooks && + + # 'def' is usually configured globally; move it to the end by + # configuring it locally. + test_config hook.def.event "pre-commit" --add && + + cat >expected <<-\EOF && + ghi + def + EOF + + git hook list pre-commit >actual && + test_cmp expected actual +' + +test_expect_success 'git hook list: empty event value resets events' ' + setup_hooks && + + # ghi is configured for pre-commit; reset it with an empty value + test_config hook.ghi.event "" --add && + + # only def should remain for pre-commit + echo "def" >expected && + git hook list pre-commit >actual && + test_cmp expected actual +' + +test_expect_success 'hook can be configured for multiple events' ' + setup_hooks && + + # 'ghi' should be included in both 'pre-commit' and 'test-hook' + git hook list pre-commit >actual && + grep "ghi" actual && + git hook list test-hook >actual && + grep "ghi" actual +' + +test_expect_success 'git hook list shows hooks from the hookdir' ' + setup_hookdir && + + cat >expected <<-\EOF && + hook from hookdir + EOF + + git hook list pre-commit >actual && + test_cmp expected actual +' + +test_expect_success 'inline hook definitions execute oneliners' ' + test_config hook.oneliner.event "pre-commit" && + test_config hook.oneliner.command "echo \"Hello World\"" && + + echo "Hello World" >expected && + + # hooks are run with stdout_to_stderr = 1 + git hook run pre-commit 2>actual && + test_cmp expected actual +' + +test_expect_success 'inline hook definitions resolve paths' ' + write_script sample-hook.sh <<-\EOF && + echo \"Sample Hook\" + EOF + + test_when_finished "rm sample-hook.sh" && + + test_config hook.sample-hook.event pre-commit && + test_config hook.sample-hook.command "\"$(pwd)/sample-hook.sh\"" && + + echo \"Sample Hook\" >expected && + + # hooks are run with stdout_to_stderr = 1 + git hook run pre-commit 2>actual && + test_cmp expected actual +' + +test_expect_success 'hookdir hook included in git hook run' ' + setup_hookdir && + + echo \"Legacy Hook\" >expected && + + # hooks are run with stdout_to_stderr = 1 + git hook run pre-commit 2>actual && + test_cmp expected actual +' + +test_expect_success 'stdin to multiple hooks' ' + test_config hook.stdin-a.event "test-hook" && + test_config hook.stdin-a.command "xargs -P1 -I% echo a%" && + test_config hook.stdin-b.event "test-hook" && + test_config hook.stdin-b.command "xargs -P1 -I% echo b%" && + + cat >input <<-\EOF && + 1 + 2 + 3 + EOF + + cat >expected <<-\EOF && + a1 + a2 + a3 + b1 + b2 + b3 + EOF + + git hook run --to-stdin=input test-hook 2>actual && + test_cmp expected actual +' + +test_expect_success 'rejects hooks with no commands configured' ' + test_config hook.broken.event "test-hook" && + test_must_fail git hook list test-hook 2>actual && + test_grep "hook.broken.command" actual && + test_must_fail git hook run test-hook 2>actual && + test_grep "hook.broken.command" actual +' + +test_expect_success 'disabled hook is not run' ' + test_config hook.skipped.event "test-hook" && + test_config hook.skipped.command "echo \"Should not run\"" && + test_config hook.skipped.enabled false && + + git hook run --ignore-missing test-hook 2>actual && + test_must_be_empty actual +' + +test_expect_success 'disabled hook with no command warns' ' + test_config hook.nocommand.event "pre-commit" && + test_config hook.nocommand.enabled false && + + git hook list pre-commit 2>actual && + test_grep "disabled hook.*nocommand.*no command configured" actual +' + +test_expect_success 'disabled hook appears as disabled in git hook list' ' + test_config hook.active.event "pre-commit" && + test_config hook.active.command "echo active" && + test_config hook.inactive.event "pre-commit" && + test_config hook.inactive.command "echo inactive" && + test_config hook.inactive.enabled false && + + git hook list pre-commit >actual && + test_grep "^active$" actual && + test_grep "^inactive (disabled)$" actual +' + +test_expect_success 'disabled hook shows scope with --show-scope' ' + test_config hook.myhook.event "pre-commit" && + test_config hook.myhook.command "echo hi" && + test_config hook.myhook.enabled false && + + git hook list --show-scope pre-commit >actual && + test_grep "myhook (local, disabled)" actual +' + +test_expect_success 'disabled configured hook is not reported as existing by hook_exists' ' + test_when_finished "rm -f git-bugreport-hook-exists-test.txt" && + test_config hook.linter.event "pre-commit" && + test_config hook.linter.command "echo lint" && + test_config hook.linter.enabled false && + + git bugreport -s hook-exists-test && + test_grep ! "pre-commit" git-bugreport-hook-exists-test.txt +' + +test_expect_success 'globally disabled hook can be re-enabled locally' ' + test_config_global hook.global-hook.event "test-hook" && + test_config_global hook.global-hook.command "echo \"global-hook ran\"" && + test_config_global hook.global-hook.enabled false && + test_config hook.global-hook.enabled true && + + echo "global-hook ran" >expected && + git hook run test-hook 2>actual && + test_cmp expected actual +' + +test_expect_success 'configured hooks run before hookdir hook' ' + setup_hookdir && + test_config hook.first.event "pre-commit" && + test_config hook.first.command "echo first" && + test_config hook.second.event "pre-commit" && + test_config hook.second.command "echo second" && + + cat >expected <<-\EOF && + first + second + hook from hookdir + EOF + + git hook list pre-commit >actual && + test_cmp expected actual && + + # "Legacy Hook" is the output of the hookdir pre-commit script + # written by setup_hookdir() above. + cat >expected <<-\EOF && + first + second + "Legacy Hook" + EOF + + git hook run pre-commit 2>actual && + test_cmp expected actual +' + +test_expect_success 'git hook list --show-scope shows config scope' ' + test_config_global hook.global-hook.command "echo global" && + test_config_global hook.global-hook.event test-hook --add && + test_config hook.local-hook.command "echo local" && + test_config hook.local-hook.event test-hook --add && + + cat >expected <<-\EOF && + global-hook (global) + local-hook (local) + EOF + git hook list --show-scope test-hook >actual && + test_cmp expected actual && + + # without --show-scope the scope must not appear + git hook list test-hook >actual && + test_grep ! "(global)" actual && + test_grep ! "(local)" actual +' + test_expect_success 'git hook run a hook with a bad shebang' ' test_when_finished "rm -rf bad-hooks" && mkdir bad-hooks && @@ -167,6 +471,7 @@ test_expect_success 'git hook run a hook with a bad shebang' ' ' test_expect_success 'stdin to hooks' ' + mkdir -p .git/hooks && write_script .git/hooks/test-hook <<-\EOF && echo BEGIN stdin cat @@ -184,4 +489,141 @@ test_expect_success 'stdin to hooks' ' test_cmp expect actual ' +check_stdout_separate_from_stderr () { + for hook in "$@" + do + # Ensure hook's stdout is only in stdout, not stderr + test_grep "Hook $hook stdout" stdout.actual || return 1 + test_grep ! "Hook $hook stdout" stderr.actual || return 1 + + # Ensure hook's stderr is only in stderr, not stdout + test_grep "Hook $hook stderr" stderr.actual || return 1 + test_grep ! "Hook $hook stderr" stdout.actual || return 1 + done +} + +check_stdout_merged_to_stderr () { + for hook in "$@" + do + # Ensure hook's stdout is only in stderr, not stdout + test_grep "Hook $hook stdout" stderr.actual || return 1 + test_grep ! "Hook $hook stdout" stdout.actual || return 1 + + # Ensure hook's stderr is only in stderr, not stdout + test_grep "Hook $hook stderr" stderr.actual || return 1 + test_grep ! "Hook $hook stderr" stdout.actual || return 1 + done +} + +setup_hooks () { + for hook in "$@" + do + test_hook $hook <<-EOF + echo >&1 Hook $hook stdout + echo >&2 Hook $hook stderr + EOF + done +} + +test_expect_success 'client hooks: pre-push expects separate stdout and stderr' ' + test_when_finished "rm -f stdout.actual stderr.actual" && + git init --bare remote && + git remote add origin remote && + test_commit A && + setup_hooks pre-push && + git push origin HEAD:main >stdout.actual 2>stderr.actual && + check_stdout_separate_from_stderr pre-push +' + +test_expect_success 'client hooks: commit hooks expect stdout redirected to stderr' ' + hooks="pre-commit prepare-commit-msg \ + commit-msg post-commit \ + reference-transaction" && + setup_hooks $hooks && + test_when_finished "rm -f stdout.actual stderr.actual" && + git checkout -B main && + git checkout -b branch-a && + test_commit commit-on-branch-a && + git commit --allow-empty -m "Test" >stdout.actual 2>stderr.actual && + check_stdout_merged_to_stderr $hooks +' + +test_expect_success 'client hooks: checkout hooks expect stdout redirected to stderr' ' + setup_hooks post-checkout reference-transaction && + test_when_finished "rm -f stdout.actual stderr.actual" && + git checkout -b new-branch main >stdout.actual 2>stderr.actual && + check_stdout_merged_to_stderr post-checkout reference-transaction +' + +test_expect_success 'client hooks: merge hooks expect stdout redirected to stderr' ' + setup_hooks pre-merge-commit post-merge reference-transaction && + test_when_finished "rm -f stdout.actual stderr.actual" && + test_commit new-branch-commit && + git merge --no-ff branch-a >stdout.actual 2>stderr.actual && + check_stdout_merged_to_stderr pre-merge-commit post-merge reference-transaction +' + +test_expect_success 'client hooks: post-rewrite hooks expect stdout redirected to stderr' ' + setup_hooks post-rewrite reference-transaction && + test_when_finished "rm -f stdout.actual stderr.actual" && + git commit --amend --allow-empty --no-edit >stdout.actual 2>stderr.actual && + check_stdout_merged_to_stderr post-rewrite reference-transaction +' + +test_expect_success 'client hooks: applypatch hooks expect stdout redirected to stderr' ' + setup_hooks applypatch-msg pre-applypatch post-applypatch && + test_when_finished "rm -f stdout.actual stderr.actual" && + git checkout -b branch-b main && + test_commit branch-b && + git format-patch -1 --stdout >patch && + git checkout -b branch-c main && + git am patch >stdout.actual 2>stderr.actual && + check_stdout_merged_to_stderr applypatch-msg pre-applypatch post-applypatch +' + +test_expect_success 'client hooks: rebase hooks expect stdout redirected to stderr' ' + setup_hooks pre-rebase && + test_when_finished "rm -f stdout.actual stderr.actual" && + git checkout -b branch-d main && + test_commit branch-d && + git checkout main && + test_commit diverge-main && + git checkout branch-d && + git rebase main >stdout.actual 2>stderr.actual && + check_stdout_merged_to_stderr pre-rebase +' + +test_expect_success 'client hooks: post-index-change expects stdout redirected to stderr' ' + setup_hooks post-index-change && + test_when_finished "rm -f stdout.actual stderr.actual" && + oid=$(git hash-object -w --stdin stdout.actual 2>stderr.actual && + check_stdout_merged_to_stderr post-index-change +' + +test_expect_success 'server hooks expect stdout redirected to stderr' ' + test_when_finished "rm -f stdout.actual stderr.actual" && + git init --bare remote-server && + git remote add origin-server remote-server && + cd remote-server && + setup_hooks pre-receive update post-receive post-update && + cd .. && + git push origin-server HEAD:new-branch >stdout.actual 2>stderr.actual && + check_stdout_merged_to_stderr pre-receive update post-receive post-update +' + +test_expect_success 'server push-to-checkout hook expects stdout redirected to stderr' ' + test_when_finished "rm -f stdout.actual stderr.actual" && + git init server && + git -C server checkout -b main && + test_config -C server receive.denyCurrentBranch updateInstead && + git remote add origin-server-2 server && + cd server && + setup_hooks push-to-checkout && + cd .. && + git push origin-server-2 HEAD:main >stdout.actual 2>stderr.actual && + check_stdout_merged_to_stderr push-to-checkout +' + test_done diff --git a/transport.c b/transport.c index 5784a699e3..56a4015389 100644 --- a/transport.c +++ b/transport.c @@ -1317,65 +1317,86 @@ static void die_with_unpushed_submodules(struct string_list *needs_pushing) die(_("Aborting.")); } +struct feed_pre_push_hook_data { + struct strbuf buf; + const struct ref *refs; +}; + +static int pre_push_hook_feed_stdin(int hook_stdin_fd, void *pp_cb UNUSED, void *pp_task_cb) +{ + struct feed_pre_push_hook_data *data = pp_task_cb; + const struct ref *r = data->refs; + int ret = 0; + + if (!r) + return 1; /* no more refs */ + + data->refs = r->next; + + switch (r->status) { + case REF_STATUS_REJECT_NONFASTFORWARD: + case REF_STATUS_REJECT_REMOTE_UPDATED: + case REF_STATUS_REJECT_STALE: + case REF_STATUS_UPTODATE: + return 0; /* skip refs which won't be pushed */ + default: + break; + } + + if (!r->peer_ref) + return 0; + + strbuf_reset(&data->buf); + strbuf_addf(&data->buf, "%s %s %s %s\n", + r->peer_ref->name, oid_to_hex(&r->new_oid), + r->name, oid_to_hex(&r->old_oid)); + + ret = write_in_full(hook_stdin_fd, data->buf.buf, data->buf.len); + if (ret < 0 && errno != EPIPE) + return ret; /* We do not mind if a hook does not read all refs. */ + + return 0; +} + +static void *pre_push_hook_data_alloc(void *feed_pipe_ctx) +{ + struct feed_pre_push_hook_data *data; + CALLOC_ARRAY(data, 1); + strbuf_init(&data->buf, 0); + data->refs = (struct ref *)feed_pipe_ctx; + return data; +} + +static void pre_push_hook_data_free(void *data) +{ + struct feed_pre_push_hook_data *d = data; + if (!d) + return; + strbuf_release(&d->buf); + free(d); +} + static int run_pre_push_hook(struct transport *transport, struct ref *remote_refs) { - int ret = 0, x; - struct ref *r; - struct child_process proc = CHILD_PROCESS_INIT; - struct strbuf buf; - const char *hook_path = find_hook(the_repository, "pre-push"); + struct run_hooks_opt opt = RUN_HOOKS_OPT_INIT; + int ret = 0; - if (!hook_path) - return 0; + strvec_push(&opt.args, transport->remote->name); + strvec_push(&opt.args, transport->url); - strvec_push(&proc.args, hook_path); - strvec_push(&proc.args, transport->remote->name); - strvec_push(&proc.args, transport->url); + opt.feed_pipe = pre_push_hook_feed_stdin; + opt.feed_pipe_ctx = remote_refs; + opt.feed_pipe_cb_data_alloc = pre_push_hook_data_alloc; + opt.feed_pipe_cb_data_free = pre_push_hook_data_free; - proc.in = -1; - proc.trace2_hook_name = "pre-push"; + /* + * pre-push hooks expect stdout & stderr to be separate, so don't merge + * them to keep backwards compatibility with existing hooks. + */ + opt.stdout_to_stderr = 0; - if (start_command(&proc)) { - finish_command(&proc); - return -1; - } - - sigchain_push(SIGPIPE, SIG_IGN); - - strbuf_init(&buf, 256); - - for (r = remote_refs; r; r = r->next) { - if (!r->peer_ref) continue; - if (r->status == REF_STATUS_REJECT_NONFASTFORWARD) continue; - if (r->status == REF_STATUS_REJECT_STALE) continue; - if (r->status == REF_STATUS_REJECT_REMOTE_UPDATED) continue; - if (r->status == REF_STATUS_UPTODATE) continue; - - strbuf_reset(&buf); - strbuf_addf( &buf, "%s %s %s %s\n", - r->peer_ref->name, oid_to_hex(&r->new_oid), - r->name, oid_to_hex(&r->old_oid)); - - if (write_in_full(proc.in, buf.buf, buf.len) < 0) { - /* We do not mind if a hook does not read all refs. */ - if (errno != EPIPE) - ret = -1; - break; - } - } - - strbuf_release(&buf); - - x = close(proc.in); - if (!ret) - ret = x; - - sigchain_pop(SIGPIPE); - - x = finish_command(&proc); - if (!ret) - ret = x; + ret = run_hooks_opt(the_repository, "pre-push", &opt); return ret; }