After much delay and $DAYJOB, here is v9. - Addressed nits in reviews on v8 - sendemail-validate hook becomes non-parallelized; updated to use Ævar's updated system_or_die() function - changed strbuf to char* in hooks_list - Attempted to do so in run_command's stdout callback, but this made length protection difficult, so stuck with strbuf there. - test_i18ncmp -> test_cmp - Stop doing i18n lego in run_hooks() - Checked that run_hooks_opt_init() is always separated by a space from variable decl blocks - Checked for early returns which may skip run_hooks_opt_clear(); this resulted in minimizing the scope of run_hooks_opt in most places - Got rid of native-hooks.txt. It was a nice idea, but not attached to a large and slow series like this one. - In traces, log the name of the hook (e.g. "pre-commit") instead of the name of the executable (e.g. "/home/emily/check-for-debug-strings"); the executable name is tracelogged as part of argv anyways, and we want to be able to tell which hook was responsible for invoking the executable in question. Thanks. - Emily Emily Shaffer (37): doc: propose hooks managed by the config hook: introduce git-hook subcommand hook: include hookdir hook in list hook: teach hook.runHookDir hook: implement hookcmd.<name>.skip parse-options: parse into strvec hook: add 'run' subcommand hook: introduce hook_exists() hook: support passing stdin to hooks run-command: allow stdin for run_processes_parallel hook: allow parallel hook execution hook: allow specifying working directory for hooks run-command: add stdin callback for parallelization hook: provide stdin by string_list or callback run-command: allow capturing of collated output hooks: allow callers to capture output commit: use config-based hooks am: convert applypatch hooks to use config merge: use config-based hooks for post-merge hook gc: use hook library for pre-auto-gc hook rebase: teach pre-rebase to use hook.h read-cache: convert post-index-change hook to use config receive-pack: convert push-to-checkout hook to hook.h git-p4: use 'git hook' to run hooks hooks: convert 'post-checkout' hook to hook library hook: convert 'post-rewrite' hook to config transport: convert pre-push hook to use config reference-transaction: look for hooks in config receive-pack: convert 'update' hook to hook.h proc-receive: acquire hook list from hook.h post-update: use hook.h library receive-pack: convert receive hooks to hook.h bugreport: use hook_exists instead of find_hook git-send-email: use 'git hook run' for 'sendemail-validate' run-command: stop thinking about hooks doc: clarify fsmonitor-watchman specification docs: link githooks and git-hook manpages .gitignore | 1 + Documentation/Makefile | 1 + Documentation/config/hook.txt | 27 + Documentation/git-hook.txt | 162 ++++++ Documentation/githooks.txt | 77 ++- Documentation/technical/api-parse-options.txt | 7 + .../technical/config-based-hooks.txt | 369 +++++++++++++ Makefile | 2 + builtin.h | 1 + builtin/am.c | 39 +- builtin/bugreport.c | 4 +- builtin/checkout.c | 19 +- builtin/clone.c | 8 +- builtin/commit.c | 11 +- builtin/fetch.c | 1 + builtin/gc.c | 9 +- builtin/hook.c | 190 +++++++ builtin/merge.c | 15 +- builtin/rebase.c | 10 +- builtin/receive-pack.c | 326 ++++++------ builtin/submodule--helper.c | 2 +- builtin/worktree.c | 32 +- command-list.txt | 1 + commit.c | 22 +- commit.h | 3 +- git-p4.py | 67 +-- git-send-email.perl | 26 +- git.c | 1 + hook.c | 483 ++++++++++++++++++ hook.h | 139 +++++ parse-options-cb.c | 16 + parse-options.h | 4 + read-cache.c | 13 +- refs.c | 43 +- reset.c | 17 +- run-command.c | 156 +++--- run-command.h | 55 +- sequencer.c | 92 ++-- submodule.c | 1 + t/helper/test-parse-options.c | 6 + t/helper/test-run-command.c | 46 +- t/t0040-parse-options.sh | 27 + t/t0061-run-command.sh | 37 ++ t/t1360-config-based-hooks.sh | 329 ++++++++++++ t/t1416-ref-transaction-hooks.sh | 12 +- t/t5411/test-0015-too-many-hooks-error.sh | 47 ++ ...3-pre-commit-and-pre-merge-commit-hooks.sh | 17 +- t/t9001-send-email.sh | 13 +- transport.c | 58 +-- 49 files changed, 2505 insertions(+), 539 deletions(-) create mode 100644 Documentation/config/hook.txt create mode 100644 Documentation/git-hook.txt create mode 100644 Documentation/technical/config-based-hooks.txt create mode 100644 builtin/hook.c create mode 100644 hook.c create mode 100644 hook.h create mode 100755 t/t1360-config-based-hooks.sh create mode 100644 t/t5411/test-0015-too-many-hooks-error.sh 1: 85b99369f1 ! 1: d2b7ee8317 doc: propose hooks managed by the config @@ Commit message Signed-off-by: Emily Shaffer <emilyshaffer@xxxxxxxxxx> + + ## Notes ## + Since v6, checked for inconsistencies with implementation and added lots of + caveats about whether 'git hook add' and 'git hook edit' will ever materialize. + + Hopefully this reflects reality now; please review accordingly. + + Since v6, checked for inconsistencies with implementation and added lots of + caveats about whether 'git hook add' and 'git hook edit' will ever materialize. + + Hopefully this reflects reality now; please review accordingly. + + Since v4, addressed comments from Jonathan Tan about wording. However, I have + not addressed AEvar's comments or done a full re-review of this document. + I wanted to get the rest of the series out for initial review first. + + - Emily + + Since v4, addressed comments from Jonathan Tan about wording. + ## Documentation/Makefile ## @@ Documentation/Makefile: SP_ARTICLES += $(API_DOCS) TECH_DOCS += MyFirstContribution 2: 1d19f1477c < -: ---------- hook: scaffolding for git-hook subcommand 3: c125c63880 ! 2: 112a809f02 hook: add list command @@ Metadata Author: Emily Shaffer <emilyshaffer@xxxxxxxxxx> ## Commit message ## - hook: add list command + hook: introduce git-hook subcommand - Teach 'git hook list <hookname>', which checks the known configs in + Add a new subcommand, git-hook, which will be used to ease config-based + hook management. This command will handle parsing configs to compose a + list of hooks to run for a given event, as well as adding or modifying + hook configs in an interactive fashion. + + Start with 'git hook list <hookname>', which checks the known configs in order to create an ordered list of hooks to run on a given hook event. Multiple commands can be specified for a given hook by providing @@ Commit message Signed-off-by: Emily Shaffer <emilyshaffer@xxxxxxxxxx> + + ## Notes ## + Since v4, mainly changed to RUN_SETUP_GENTLY so that 'git hook list' can + be executed outside of a repo. + + ## .gitignore ## +@@ + /git-grep + /git-hash-object + /git-help ++/git-hook + /git-http-backend + /git-http-fetch + /git-http-push + ## Documentation/config/hook.txt (new) ## @@ +hook.<command>.command:: @@ Documentation/config/hook.txt (new) + as a command. This can be an executable on your device or a oneliner for + your shell. See linkgit:git-hook[1]. - ## Documentation/git-hook.txt ## -@@ Documentation/git-hook.txt: git-hook - Manage configured hooks - SYNOPSIS - -------- - [verse] --'git hook' + ## Documentation/git-hook.txt (new) ## +@@ ++git-hook(1) ++=========== ++ ++NAME ++---- ++git-hook - Manage configured hooks ++ ++SYNOPSIS ++-------- ++[verse] +'git hook' list <hook-name> - - DESCRIPTION - ----------- --A placeholder command. Later, you will be able to list, add, and modify hooks --with this command. ++ ++DESCRIPTION ++----------- +You can list configured hooks with this command. Later, you will be able to run, +add, and modify hooks with this command. + @@ Documentation/git-hook.txt: git-hook - Manage configured hooks +CONFIGURATION +------------- +include::config/hook.txt[] - - GIT - --- ++ ++GIT ++--- ++Part of the linkgit:git[1] suite ## Makefile ## @@ Makefile: LIB_OBJS += hash-lookup.o @@ Makefile: LIB_OBJS += hash-lookup.o LIB_OBJS += ident.o LIB_OBJS += json-writer.o LIB_OBJS += kwset.o +@@ Makefile: BUILTIN_OBJS += builtin/get-tar-commit-id.o + BUILTIN_OBJS += builtin/grep.o + BUILTIN_OBJS += builtin/hash-object.o + BUILTIN_OBJS += builtin/help.o ++BUILTIN_OBJS += builtin/hook.o + BUILTIN_OBJS += builtin/index-pack.o + BUILTIN_OBJS += builtin/init-db.o + BUILTIN_OBJS += builtin/interpret-trailers.o - ## builtin/hook.c ## + ## builtin.h ## +@@ builtin.h: int cmd_get_tar_commit_id(int argc, const char **argv, const char *prefix); + int cmd_grep(int argc, const char **argv, const char *prefix); + int cmd_hash_object(int argc, const char **argv, const char *prefix); + int cmd_help(int argc, const char **argv, const char *prefix); ++int cmd_hook(int argc, const char **argv, const char *prefix); + int cmd_index_pack(int argc, const char **argv, const char *prefix); + int cmd_init_db(int argc, const char **argv, const char *prefix); + int cmd_interpret_trailers(int argc, const char **argv, const char *prefix); + + ## builtin/hook.c (new) ## @@ - #include "cache.h" -- - #include "builtin.h" ++#include "cache.h" ++#include "builtin.h" +#include "config.h" +#include "hook.h" - #include "parse-options.h" ++#include "parse-options.h" +#include "strbuf.h" - - static const char * const builtin_hook_usage[] = { -- N_("git hook"), ++ ++static const char * const builtin_hook_usage[] = { + N_("git hook list <hookname>"), - NULL - }; - --int cmd_hook(int argc, const char **argv, const char *prefix) ++ NULL ++}; ++ +static int list(int argc, const char **argv, const char *prefix) - { -- struct option builtin_hook_options[] = { ++{ + struct list_head *head, *pos; -+ struct strbuf hookname = STRBUF_INIT; ++ const char *hookname = NULL; + + struct option list_options[] = { - OPT_END(), - }; - -- argc = parse_options(argc, argv, prefix, builtin_hook_options, ++ OPT_END(), ++ }; ++ + argc = parse_options(argc, argv, prefix, list_options, - builtin_hook_usage, 0); - ++ builtin_hook_usage, 0); ++ + if (argc < 1) { + usage_msg_opt(_("You must specify a hook event name to list."), + builtin_hook_usage, list_options); + } + -+ strbuf_addstr(&hookname, argv[0]); ++ hookname = argv[0]; + -+ head = hook_list(&hookname); ++ head = hook_list(hookname); + + if (list_empty(head)) { + printf(_("no commands configured for hook '%s'\n"), -+ hookname.buf); -+ strbuf_release(&hookname); ++ hookname); + return 0; + } + @@ builtin/hook.c + } + + clear_hook_list(head); -+ strbuf_release(&hookname); + - return 0; - } ++ return 0; ++} + +int cmd_hook(int argc, const char **argv, const char *prefix) +{ @@ builtin/hook.c + usage_with_options(builtin_hook_usage, builtin_hook_options); +} + ## command-list.txt ## +@@ command-list.txt: git-grep mainporcelain info + git-gui mainporcelain + git-hash-object plumbingmanipulators + git-help ancillaryinterrogators complete ++git-hook mainporcelain + git-http-backend synchingrepositories + git-http-fetch synchelpers + git-http-push synchelpers + + ## git.c ## +@@ git.c: static struct cmd_struct commands[] = { + { "grep", cmd_grep, RUN_SETUP_GENTLY }, + { "hash-object", cmd_hash_object }, + { "help", cmd_help }, ++ { "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 }, + ## hook.c (new) ## @@ +#include "cache.h" @@ hook.c (new) + return 0; +} + -+struct list_head* hook_list(const struct strbuf* hookname) ++struct list_head* hook_list(const char* hookname) +{ + struct strbuf hook_key = STRBUF_INIT; + struct list_head *hook_head = xmalloc(sizeof(struct list_head)); @@ hook.c (new) + if (!hookname) + return NULL; + -+ strbuf_addf(&hook_key, "hook.%s.command", hookname->buf); ++ strbuf_addf(&hook_key, "hook.%s.command", hookname); + + git_config(hook_config_lookup, &cb_data); + @@ hook.h (new) + * Provides a linked list of 'struct hook' detailing commands which should run + * in response to the 'hookname' event, in execution order. + */ -+struct list_head* hook_list(const struct strbuf *hookname); ++struct list_head* hook_list(const char *hookname); + +/* Free memory associated with a 'struct hook' */ +void free_hook(struct hook *ptr); +/* Empties the list at 'head', calling 'free_hook()' on each entry */ +void clear_hook_list(struct list_head *head); - ## t/t1360-config-based-hooks.sh ## -@@ t/t1360-config-based-hooks.sh: test_description='config-managed multihooks, including git-hook command' - - . ./test-lib.sh - --test_expect_success 'git hook command does not crash' ' -- git hook + ## t/t1360-config-based-hooks.sh (new) ## +@@ ++#!/bin/bash ++ ++test_description='config-managed multihooks, including git-hook command' ++ ++. ./test-lib.sh ++ +ROOT= +if test_have_prereq MINGW +then @@ t/t1360-config-based-hooks.sh: test_description='config-managed multihooks, incl + + git hook list pre-commit >actual && + test_cmp expected actual - ' - - test_done ++' ++ ++test_done 4: 0b8cd46ff9 ! 3: 3114306368 hook: include hookdir hook in list @@ builtin/hook.c: static int list(int argc, const char **argv, const char *prefix) list_for_each(pos, head) { struct hook *item = list_entry(pos, struct hook, list); - if (item) +- printf("%s: %s\n", +- config_scope_name(item->origin), + item = list_entry(pos, struct hook, list); + if (item) { -+ /* Don't translate 'hookdir' - it matches the config */ - printf("%s: %s\n", -- config_scope_name(item->origin), ++ /* ++ * TRANSLATORS: "<config scope>: <path>". Both fields ++ * should be left untranslated; config scope matches the ++ * output of 'git config --show-scope'. Marked for ++ * translation to provide better RTL support later. ++ */ ++ printf(_("%s: %s\n"), + (item->from_hookdir + ? "hookdir" + : config_scope_name(item->origin)), @@ hook.c: static void append_or_move_hook(struct list_head *head, const char *comm } /* re-set the scope so we show where an override was specified */ -@@ hook.c: struct list_head* hook_list(const struct strbuf* hookname) +@@ hook.c: struct list_head* hook_list(const char* hookname) git_config(hook_config_lookup, &cb_data); + if (have_git_dir()) { -+ const char *legacy_hook_path = find_hook(hookname->buf); ++ const char *legacy_hook_path = find_hook(hookname); + + /* Unconditionally add legacy hook, but annotate it. */ + if (legacy_hook_path) { 5: 05c503fbe1 ! 4: 681013c32a hook: teach hook.runHookDir @@ Commit message list'. Later on, though, we will pay attention to this enum when running the hooks. + + ## Notes ## + Since v7, tidied up the behavior of the HOOK_UNKNOWN flag and added a test to + enforce it - now it matches the design doc much better. + + Also, thanks Jonathan Tan for pointing out that the commit message made no sense + and was targeted for a different change. Rewrote the commit message now. + + Plus, added HOOK_ERROR flag per Junio and Jonathan Nieder. + + Newly split into its own commit since v4, and taking place much sooner. + + An unfortunate side effect of adding this support *before* the + hook.runHookDir support is that the labels on the list are not clear - + because we aren't yet flagging which hooks are from the hookdir versus + the config. I suppose we could move the addition of that field to the + struct hook up to this patch, but it didn't make a lot of sense to me to + do it just for cosmetic purposes. + + Since v7, tidied up the behavior of the HOOK_UNKNOWN flag and added a test to + enforce it - now it matches the design doc much better. + + Also, thanks Jonathan Tan for pointing out that the commit message made no sense + and was targeted for a different change. Rewrote the commit message now. + + Newly split into its own commit since v4, and taking place much sooner. + + An unfortunate side effect of adding this support *before* the + hook.runHookDir support is that the labels on the list are not clear - + because we aren't yet flagging which hooks are from the hookdir versus + the config. I suppose we could move the addition of that field to the + struct hook up to this patch, but it didn't make a lot of sense to me to + do it just for cosmetic purposes. + ## Documentation/config/hook.txt ## @@ Documentation/config/hook.txt: hookcmd.<name>.command:: A command to execute during a hook for which <name> has been specified @@ builtin/hook.c: static const char * const builtin_hook_usage[] = { static int list(int argc, const char **argv, const char *prefix) { struct list_head *head, *pos; - struct strbuf hookname = STRBUF_INIT; + const char *hookname = NULL; + struct strbuf hookdir_annotation = STRBUF_INIT; struct option list_options[] = { OPT_END(), @@ builtin/hook.c: static int list(int argc, const char **argv, const char *prefix) - return 0; - } - -+ switch (should_run_hookdir) { -+ case HOOKDIR_NO: -+ strbuf_addstr(&hookdir_annotation, _(" (will not run)")); -+ break; -+ case HOOKDIR_ERROR: -+ strbuf_addstr(&hookdir_annotation, _(" (will error and not run)")); -+ break; -+ case HOOKDIR_INTERACTIVE: -+ strbuf_addstr(&hookdir_annotation, _(" (will prompt)")); -+ break; -+ case HOOKDIR_WARN: -+ strbuf_addstr(&hookdir_annotation, _(" (will warn but run)")); -+ break; -+ case HOOKDIR_YES: -+ /* -+ * The default behavior should agree with -+ * hook.c:configured_hookdir_opt(). HOOKDIR_UNKNOWN should just -+ * do the default behavior. -+ */ -+ case HOOKDIR_UNKNOWN: -+ default: -+ break; -+ } -+ - list_for_each(pos, head) { struct hook *item = list_entry(pos, struct hook, list); item = list_entry(pos, struct hook, list); if (item) { - /* Don't translate 'hookdir' - it matches the config */ -- printf("%s: %s\n", -+ printf("%s: %s%s\n", - (item->from_hookdir - ? "hookdir" - : config_scope_name(item->origin)), +- /* +- * TRANSLATORS: "<config scope>: <path>". Both fields +- * should be left untranslated; config scope matches the +- * output of 'git config --show-scope'. Marked for +- * translation to provide better RTL support later. +- */ +- printf(_("%s: %s\n"), +- (item->from_hookdir +- ? "hookdir" +- : config_scope_name(item->origin)), - item->command.buf); -+ item->command.buf, -+ (item->from_hookdir -+ ? hookdir_annotation.buf -+ : "")); ++ if (item->from_hookdir) { ++ /* ++ * TRANSLATORS: do not translate 'hookdir' as ++ * it matches the config setting. ++ */ ++ switch (should_run_hookdir) { ++ case HOOKDIR_NO: ++ printf(_("hookdir: %s (will not run)\n"), ++ item->command.buf); ++ break; ++ case HOOKDIR_ERROR: ++ printf(_("hookdir: %s (will error and not run)\n"), ++ item->command.buf); ++ break; ++ case HOOKDIR_INTERACTIVE: ++ printf(_("hookdir: %s (will prompt)\n"), ++ item->command.buf); ++ break; ++ case HOOKDIR_WARN: ++ printf(_("hookdir: %s (will warn but run)\n"), ++ item->command.buf); ++ break; ++ case HOOKDIR_YES: ++ /* ++ * The default behavior should agree with ++ * hook.c:configured_hookdir_opt(). HOOKDIR_UNKNOWN should just ++ * do the default behavior. ++ */ ++ case HOOKDIR_UNKNOWN: ++ default: ++ printf(_("hookdir: %s\n"), ++ item->command.buf); ++ break; ++ } ++ } else { ++ /* ++ * TRANSLATORS: "<config scope>: <path>". Both fields ++ * should be left untranslated; config scope matches the ++ * output of 'git config --show-scope'. Marked for ++ * translation to provide better RTL support later. ++ */ ++ printf(_("%s: %s\n"), ++ config_scope_name(item->origin), ++ item->command.buf); ++ } } } clear_hook_list(head); + strbuf_release(&hookdir_annotation); - strbuf_release(&hookname); return 0; -@@ builtin/hook.c: static int list(int argc, const char **argv, const char *prefix) + } int cmd_hook(int argc, const char **argv, const char *prefix) { @@ builtin/hook.c: static int list(int argc, const char **argv, const char *prefix) + else if (!strcmp(run_hookdir, "interactive")) + should_run_hookdir = HOOKDIR_INTERACTIVE; + else ++ /* ++ * TRANSLATORS: leave "yes/warn/interactive/no" ++ * untranslated; the strings are compared literally. ++ */ + die(_("'%s' is not a valid option for --run-hookdir " + "(yes, warn, interactive, no)"), run_hookdir); + else @@ hook.c: static int hook_config_lookup(const char *key, const char *value, void * + return HOOKDIR_UNKNOWN; +} + - struct list_head* hook_list(const struct strbuf* hookname) + struct list_head* hook_list(const char* hookname) { struct strbuf hook_key = STRBUF_INIT; ## hook.h ## @@ hook.h: struct hook { */ - struct list_head* hook_list(const struct strbuf *hookname); + struct list_head* hook_list(const char *hookname); +enum hookdir_opt +{ @@ hook.h: struct hook { + * command line arguments. + */ +enum hookdir_opt configured_hookdir_opt(void); ++ ++/* ++ * Provides the hookdir_opt specified in the config without consulting any ++ * command line arguments. ++ */ ++enum hookdir_opt configured_hookdir_opt(void); + /* Free memory associated with a 'struct hook' */ void free_hook(struct hook *ptr); @@ t/t1360-config-based-hooks.sh: test_expect_success 'git hook list shows hooks fr + + git hook list pre-commit >actual && + # the hookdir annotation is translated -+ test_i18ncmp expected actual ++ test_cmp expected actual +' + +test_expect_success 'hook.runHookDir = error is respected by list' ' @@ t/t1360-config-based-hooks.sh: test_expect_success 'git hook list shows hooks fr + + git hook list pre-commit >actual && + # the hookdir annotation is translated -+ test_i18ncmp expected actual ++ test_cmp expected actual +' + +test_expect_success 'hook.runHookDir = warn is respected by list' ' @@ t/t1360-config-based-hooks.sh: test_expect_success 'git hook list shows hooks fr + + git hook list pre-commit >actual && + # the hookdir annotation is translated -+ test_i18ncmp expected actual ++ test_cmp expected actual +' + + @@ t/t1360-config-based-hooks.sh: test_expect_success 'git hook list shows hooks fr + + git hook list pre-commit >actual && + # the hookdir annotation is translated -+ test_i18ncmp expected actual ++ test_cmp expected actual +' + +test_expect_success 'hook.runHookDir is tolerant to unknown values' ' @@ t/t1360-config-based-hooks.sh: test_expect_success 'git hook list shows hooks fr + + git hook list pre-commit >actual && + # the hookdir annotation is translated -+ test_i18ncmp expected actual ++ test_cmp expected actual +' + test_done 6: e86025853a ! 5: 0a4b9f27b3 hook: implement hookcmd.<name>.skip @@ hook.c: static int hook_config_lookup(const char *key, const char *value, void * ## t/t1360-config-based-hooks.sh ## @@ t/t1360-config-based-hooks.sh: test_expect_success 'hook.runHookDir = warn is respected by list' ' - test_i18ncmp expected actual + test_cmp expected actual ' +test_expect_success 'git hook list removes skipped hookcmd' ' @@ t/t1360-config-based-hooks.sh: test_expect_success 'hook.runHookDir = warn is re + EOF + + git hook list pre-commit >actual && -+ test_i18ncmp expected actual ++ test_cmp expected actual +' + +test_expect_success 'git hook list ignores skip referring to unused hookcmd' ' @@ t/t1360-config-based-hooks.sh: test_expect_success 'hook.runHookDir = warn is re + EOF + + git hook list pre-commit >actual && -+ test_i18ncmp expected actual ++ test_cmp expected actual +' + +test_expect_success 'git hook list removes skipped inlined hook' ' 7: 6e10593d75 ! 6: 2ad4f44d08 parse-options: parse into strvec @@ Commit message Signed-off-by: Emily Shaffer <emilyshaffer@xxxxxxxxxx> + + ## Notes ## + Since v7, updated the reference doc to make the intended usage for OPT_STRVEC + more clear. + + Since v4, fixed one or two more places where I missed the argv_array->strvec + rename. + ## Documentation/technical/api-parse-options.txt ## @@ Documentation/technical/api-parse-options.txt: There are some macros to easily define options: The string argument is stored as an element in `string_list`. @@ parse-options.h: int parse_opt_commits(const struct option *, const char *, int) int parse_opt_noop_cb(const struct option *, const char *, int); enum parse_opt_result parse_opt_unknown_cb(struct parse_opt_ctx_t *ctx, const struct option *, + + ## t/helper/test-parse-options.c ## +@@ + #include "cache.h" + #include "parse-options.h" + #include "string-list.h" ++#include "strvec.h" + #include "trace2.h" + + static int boolean = 0; +@@ t/helper/test-parse-options.c: static char *string = NULL; + static char *file = NULL; + static int ambiguous; + static struct string_list list = STRING_LIST_INIT_NODUP; ++static struct strvec vector = STRVEC_INIT; + + static struct { + int called; +@@ t/helper/test-parse-options.c: int cmd__parse_options(int argc, const char **argv) + OPT_STRING('o', NULL, &string, "str", "get another string"), + OPT_NOOP_NOARG(0, "obsolete"), + OPT_STRING_LIST(0, "list", &list, "str", "add str to list"), ++ OPT_STRVEC(0, "vector", &vector, "str", "add str to strvec"), + OPT_GROUP("Magic arguments"), + OPT_ARGUMENT("quux", NULL, "means --quux"), + OPT_NUMBER_CALLBACK(&integer, "set integer to NUM", +@@ t/helper/test-parse-options.c: int cmd__parse_options(int argc, const char **argv) + for (i = 0; i < list.nr; i++) + show(&expect, &ret, "list: %s", list.items[i].string); + ++ for (i = 0; i < vector.nr; i++) ++ show(&expect, &ret, "vector: %s", vector.v[i]); ++ + for (i = 0; i < argc; i++) + show(&expect, &ret, "arg %02d: %s", i, argv[i]); + + + ## t/t0040-parse-options.sh ## +@@ t/t0040-parse-options.sh: String options + --st <st> get another string (pervert ordering) + -o <str> get another string + --list <str> add str to list ++ --vector <str> add str to strvec + + Magic arguments + --quux means --quux +@@ t/t0040-parse-options.sh: test_expect_success '--no-list resets list' ' + test_cmp expect output + ' + ++cat >expect <<\EOF ++boolean: 0 ++integer: 0 ++magnitude: 0 ++timestamp: 0 ++string: (not set) ++abbrev: 7 ++verbose: -1 ++quiet: 0 ++dry run: no ++file: (not set) ++vector: foo ++vector: bar ++vector: baz ++EOF ++test_expect_success '--vector keeps list of strings' ' ++ test-tool parse-options --vector foo --vector=bar --vector=baz >output && ++ test_cmp expect output ++' ++ ++test_expect_success '--no-vector resets list' ' ++ test-tool parse-options --vector=other --vector=irrelevant --vector=options \ ++ --no-vector --vector=foo --vector=bar --vector=baz >output && ++ test_cmp expect output ++' ++ + test_expect_success 'multiple quiet levels' ' + test-tool parse-options --expect="quiet: 3" -q -q -q + ' 8: 0dc9284057 ! 7: 27dd8e3edf hook: add 'run' subcommand @@ Commit message Signed-off-by: Emily Shaffer <emilyshaffer@xxxxxxxxxx> + + ## Notes ## + Since v7, added support for "error" hook.runHookDir setting. + + Since v4, updated the docs, and did less local application of single + quotes. In order for hookdir hooks to run successfully with a space in + the path, though, they must not be run with 'sh -c'. So we can treat the + hookdir hooks specially, and warn users via doc about special + considerations for configured hooks with spaces in their path. + ## Documentation/git-hook.txt ## @@ Documentation/git-hook.txt: SYNOPSIS -------- @@ hook.c void free_hook(struct hook *ptr) { +- if (ptr) { ++ if (ptr) + strbuf_release(&ptr->command); +- free(ptr); +- } ++ free(ptr); + } + + static struct hook * find_hook_by_command(struct list_head *head, const char *command) @@ hook.c: enum hookdir_opt configured_hookdir_opt(void) return HOOKDIR_UNKNOWN; } @@ hook.c: enum hookdir_opt configured_hookdir_opt(void) + + switch (cfg) + { -+ case HOOKDIR_ERROR: -+ fprintf(stderr, _("Skipping legacy hook at '%s'\n"), -+ path); -+ /* FALLTHROUGH */ -+ case HOOKDIR_NO: -+ return 0; -+ case HOOKDIR_WARN: -+ fprintf(stderr, _("Running legacy hook at '%s'\n"), -+ path); -+ return 1; -+ case HOOKDIR_INTERACTIVE: -+ do { -+ /* -+ * TRANSLATORS: Make sure to include [Y] and [n] -+ * in your translation. Only English input is -+ * accepted. Default option is "yes". -+ */ -+ fprintf(stderr, _("Run '%s'? [Yn] "), path); -+ git_read_line_interactively(&prompt); -+ strbuf_tolower(&prompt); -+ if (starts_with(prompt.buf, "n")) { -+ strbuf_release(&prompt); -+ return 0; -+ } else if (starts_with(prompt.buf, "y")) { -+ strbuf_release(&prompt); -+ return 1; -+ } -+ /* otherwise, we didn't understand the input */ -+ } while (prompt.len); /* an empty reply means "Yes" */ -+ strbuf_release(&prompt); -+ return 1; -+ /* -+ * HOOKDIR_UNKNOWN should match the default behavior, but let's -+ * give a heads up to the user. -+ */ -+ case HOOKDIR_UNKNOWN: -+ fprintf(stderr, -+ _("Unrecognized value for 'hook.runHookDir'. " -+ "Is there a typo? ")); -+ /* FALLTHROUGH */ -+ case HOOKDIR_YES: -+ default: -+ return 1; ++ case HOOKDIR_ERROR: ++ fprintf(stderr, _("Skipping legacy hook at '%s'\n"), ++ path); ++ /* FALLTHROUGH */ ++ case HOOKDIR_NO: ++ return 0; ++ case HOOKDIR_WARN: ++ fprintf(stderr, _("Running legacy hook at '%s'\n"), ++ path); ++ return 1; ++ case HOOKDIR_INTERACTIVE: ++ do { ++ /* ++ * TRANSLATORS: Make sure to include [Y] and [n] ++ * in your translation. Only English input is ++ * accepted. Default option is "yes". ++ */ ++ fprintf(stderr, _("Run '%s'? [Y/n] "), path); ++ git_read_line_interactively(&prompt); ++ /* ++ * In case of prompt = '' - that is, user hit enter, ++ * saying "yes I want the default" - strncasecmp will ++ * return 0 regardless. So list the default first. ++ * ++ * Case insensitively, accept "y", "ye", or "yes" as ++ * "yes"; accept "n" or "no" as "no". ++ */ ++ if (!strncasecmp(prompt.buf, "yes", prompt.len)) { ++ strbuf_release(&prompt); ++ return 1; ++ } else if (!strncasecmp(prompt.buf, "no", prompt.len)) { ++ strbuf_release(&prompt); ++ return 0; ++ } ++ /* otherwise, we didn't understand the input */ ++ } while (prompt.len); /* an empty reply means default (yes) */ ++ return 1; ++ /* ++ * HOOKDIR_UNKNOWN should match the default behavior, but let's ++ * give a heads up to the user. ++ */ ++ case HOOKDIR_UNKNOWN: ++ fprintf(stderr, ++ _("Unrecognized value for 'hook.runHookDir'. " ++ "Is there a typo? ")); ++ /* FALLTHROUGH */ ++ case HOOKDIR_YES: ++ default: ++ return 1; + } +} + - struct list_head* hook_list(const struct strbuf* hookname) + struct list_head* hook_list(const char* hookname) { struct strbuf hook_key = STRBUF_INIT; -@@ hook.c: struct list_head* hook_list(const struct strbuf* hookname) +@@ hook.c: struct list_head* hook_list(const char* hookname) strbuf_release(&hook_key); return hook_head; } @@ hook.c: struct list_head* hook_list(const struct strbuf* hookname) + strvec_clear(&o->args); +} + -+static void prepare_hook_cp(struct hook *hook, struct run_hooks_opt *options, ++static void prepare_hook_cp(const char *hookname, struct hook *hook, ++ struct run_hooks_opt *options, + struct child_process *cp) +{ + if (!hook) @@ hook.c: struct list_head* hook_list(const struct strbuf* hookname) + cp->no_stdin = 1; + cp->env = options->env.v; + cp->stdout_to_stderr = 1; -+ cp->trace2_hook_name = hook->command.buf; ++ cp->trace2_hook_name = hookname; + + /* + * Commands from the config could be oneliners, but we know @@ hook.c: struct list_head* hook_list(const struct strbuf* hookname) + +int run_hooks(const char *hookname, struct run_hooks_opt *options) +{ -+ struct strbuf hookname_str = STRBUF_INIT; + struct list_head *to_run, *pos = NULL, *tmp = NULL; + int rc = 0; + + if (!options) + BUG("a struct run_hooks_opt must be provided to run_hooks"); + -+ strbuf_addstr(&hookname_str, hookname); -+ -+ to_run = hook_list(&hookname_str); ++ to_run = hook_list(hookname); + + list_for_each_safe(pos, tmp, to_run) { + struct child_process hook_proc = CHILD_PROCESS_INIT; @@ hook.c: struct list_head* hook_list(const struct strbuf* hookname) + !should_include_hookdir(hook->command.buf, options->run_hookdir)) + continue; + -+ prepare_hook_cp(hook, options, &hook_proc); ++ prepare_hook_cp(hookname, hook, options, &hook_proc); + + rc |= run_command(&hook_proc); + } @@ hook.h: enum hookdir_opt +void run_hooks_opt_init(struct run_hooks_opt *o); +void run_hooks_opt_clear(struct run_hooks_opt *o); + -+/* + /* +- * Provides the hookdir_opt specified in the config without consulting any +- * command line arguments. + * Runs all hooks associated to the 'hookname' event in order. Each hook will be + * passed 'env' and 'args'. -+ */ + */ +-enum hookdir_opt configured_hookdir_opt(void); +int run_hooks(const char *hookname, struct run_hooks_opt *options); -+ + /* Free memory associated with a 'struct hook' */ void free_hook(struct hook *ptr); - /* Empties the list at 'head', calling 'free_hook()' on each entry */ ## t/t1360-config-based-hooks.sh ## @@ t/t1360-config-based-hooks.sh: test_expect_success 'hook.runHookDir = no is respected by list' ' git hook list pre-commit >actual && # the hookdir annotation is translated -- test_i18ncmp expected actual -+ test_i18ncmp expected actual && +- test_cmp expected actual ++ test_cmp expected actual && + + git hook run pre-commit 2>actual && + test_must_be_empty actual @@ t/t1360-config-based-hooks.sh: test_expect_success 'hook.runHookDir = error is r git hook list pre-commit >actual && # the hookdir annotation is translated -+ test_i18ncmp expected actual && ++ test_cmp expected actual && + + cat >expected <<-EOF && + Skipping legacy hook at '\''$(pwd)/.git/hooks/pre-commit'\'' + EOF + + git hook run pre-commit 2>actual && - test_i18ncmp expected actual + test_cmp expected actual ' @@ t/t1360-config-based-hooks.sh: test_expect_success 'hook.runHookDir = warn is respected by list' ' git hook list pre-commit >actual && # the hookdir annotation is translated -+ test_i18ncmp expected actual && ++ test_cmp expected actual && + + cat >expected <<-EOF && + Running legacy hook at '\''$(pwd)/.git/hooks/pre-commit'\'' @@ t/t1360-config-based-hooks.sh: test_expect_success 'hook.runHookDir = warn is re + EOF + + git hook run pre-commit 2>actual && - test_i18ncmp expected actual + test_cmp expected actual ' @@ t/t1360-config-based-hooks.sh: test_expect_success 'git hook list removes skipped inlined hook' ' @@ t/t1360-config-based-hooks.sh: test_expect_success 'hook.runHookDir = interactiv git hook list pre-commit >actual && # the hookdir annotation is translated -- test_i18ncmp expected actual -+ test_i18ncmp expected actual && ++ test_cmp expected actual && + + test_write_lines n | git hook run pre-commit 2>actual && + ! grep "Legacy Hook" actual && @@ t/t1360-config-based-hooks.sh: test_expect_success 'hook.runHookDir = interactiv + test_cmp expected actual +' + ++test_expect_success 'git hook run can pass args and env vars' ' ++ write_script sample-hook.sh <<-\EOF && ++ echo $1 ++ echo $2 ++ echo $TEST_ENV_1 ++ echo $TEST_ENV_2 ++ EOF ++ ++ test_config hook.pre-commit.command "\"$(pwd)/sample-hook.sh\"" && ++ ++ cat >expected <<-EOF && ++ arg1 ++ arg2 ++ env1 ++ env2 ++ EOF ++ ++ git hook run --arg arg1 \ ++ --env TEST_ENV_1=env1 \ ++ -a arg2 \ ++ -e TEST_ENV_2=env2 \ ++ pre-commit 2>actual && ++ + test_cmp expected actual + ' + +test_expect_success 'hookdir hook included in git hook run' ' + setup_hookdir && + @@ t/t1360-config-based-hooks.sh: test_expect_success 'hook.runHookDir = interactiv + setup_hooks && + + nongit test_must_fail git hook run pre-commit - ' - ++' ++ test_expect_success 'hook.runHookDir is tolerant to unknown values' ' + setup_hookdir && + 9: 92c67ed9da ! 8: 46975c11c8 hook: introduce hook_exists() @@ hook.c: void run_hooks_opt_init(struct run_hooks_opt *o) strvec_clear(&o->env); ## hook.h ## -@@ hook.h: struct list_head* hook_list(const struct strbuf *hookname); +@@ hook.h: struct list_head* hook_list(const char *hookname); enum hookdir_opt { 10: 9b3bb0b655 ! 9: e11f9e28a3 hook: support passing stdin to hooks @@ hook.c: void run_hooks_opt_init(struct run_hooks_opt *o) o->run_hookdir = configured_hookdir_opt(); } -@@ hook.c: static void prepare_hook_cp(struct hook *hook, struct run_hooks_opt *options, +@@ hook.c: static void prepare_hook_cp(const char *hookname, struct hook *hook, if (!hook) return; @@ hook.c: static void prepare_hook_cp(struct hook *hook, struct run_hooks_opt *opt + cp->env = options->env.v; cp->stdout_to_stderr = 1; - cp->trace2_hook_name = hook->command.buf; + cp->trace2_hook_name = hookname; ## hook.h ## @@ hook.h: struct run_hooks_opt @@ hook.h: int hook_exists(const char *hookname, enum hookdir_opt should_run_hookdi ## t/t1360-config-based-hooks.sh ## @@ t/t1360-config-based-hooks.sh: test_expect_success 'hook.runHookDir is tolerant to unknown values' ' - test_i18ncmp expected actual + test_cmp expected actual ' +test_expect_success 'stdin to multiple hooks' ' 11: 9933985e78 = 10: 3ceb4156fd run-command: allow stdin for run_processes_parallel 12: 43caafe656 ! 11: 93a47f5242 hook: allow parallel hook execution @@ Commit message Signed-off-by: Emily Shaffer <emilyshaffer@xxxxxxxxxx> + + ## Notes ## + Per AEvar's request - parallel hook execution on day zero. + + In most ways run_processes_parallel() worked great for me - but it didn't + have great support for hooks where we pipe to and from. I had to add this + support later in the series. + + Since I modified an existing and in-use library I'd appreciate a keen look on + these patches. + + - Emily + ## Documentation/config/hook.txt ## @@ Documentation/config/hook.txt: hook.runHookDir:: Controls how hooks contained in your hookdir are executed. Can be any of @@ hook.c: enum hookdir_opt configured_hookdir_opt(void) static int should_include_hookdir(const char *path, enum hookdir_opt cfg) { struct strbuf prompt = STRBUF_INIT; -@@ hook.c: struct list_head* hook_list(const struct strbuf* hookname) +@@ hook.c: struct list_head* hook_list(const char* hookname) return hook_head; } @@ hook.c: void run_hooks_opt_clear(struct run_hooks_opt *o) strvec_clear(&o->args); } --static void prepare_hook_cp(struct hook *hook, struct run_hooks_opt *options, +-static void prepare_hook_cp(const char *hookname, struct hook *hook, +- struct run_hooks_opt *options, - struct child_process *cp) +static int pick_next_hook(struct child_process *cp, + struct strbuf *out, @@ hook.c: void run_hooks_opt_clear(struct run_hooks_opt *o) - cp->env = options->env.v; + cp->env = hook_cb->options->env.v; cp->stdout_to_stderr = 1; - cp->trace2_hook_name = hook->command.buf; +- cp->trace2_hook_name = hookname; ++ cp->trace2_hook_name = hook_cb->hookname; -@@ hook.c: static void prepare_hook_cp(struct hook *hook, struct run_hooks_opt *options, + /* + * Commands from the config could be oneliners, but we know +@@ hook.c: static void prepare_hook_cp(const char *hookname, struct hook *hook, * add passed-in argv, without expanding - let the user get back * exactly what they put in */ @@ hook.c: static void prepare_hook_cp(struct hook *hook, struct run_hooks_opt *opt int run_hooks(const char *hookname, struct run_hooks_opt *options) { - struct strbuf hookname_str = STRBUF_INIT; struct list_head *to_run, *pos = NULL, *tmp = NULL; - int rc = 0; -+ struct hook_cb_data cb_data = { 0, NULL, NULL, options }; ++ struct hook_cb_data cb_data = { 0, hookname, NULL, NULL, options }; if (!options) BUG("a struct run_hooks_opt must be provided to run_hooks"); @@ hook.c: int run_hooks(const char *hookname, struct run_hooks_opt *options) - to_run = hook_list(&hookname_str); + to_run = hook_list(hookname); list_for_each_safe(pos, tmp, to_run) { - struct child_process hook_proc = CHILD_PROCESS_INIT; @@ hook.c: int run_hooks(const char *hookname, struct run_hooks_opt *options) + if (list_empty(to_run)) + return 0; -- prepare_hook_cp(hook, options, &hook_proc); +- prepare_hook_cp(hookname, hook, options, &hook_proc); + cb_data.head = to_run; + cb_data.run_me = list_entry(to_run->next, struct hook, list); @@ hook.h: struct run_hooks_opt + */ +struct hook_cb_data { + int rc; ++ const char *hookname; + struct list_head *head; + struct hook *run_me; + struct run_hooks_opt *options; 13: 2e189a7566 ! 12: 7f8c886d3f hook: allow specifying working directory for hooks @@ Commit message Signed-off-by: Emily Shaffer <emilyshaffer@xxxxxxxxxx> + + ## Notes ## + Needed later for "post-checkout" conversion. + ## hook.c ## @@ hook.c: void run_hooks_opt_init_sync(struct run_hooks_opt *o) o->path_to_stdin = NULL; @@ hook.c: void run_hooks_opt_init_sync(struct run_hooks_opt *o) @@ hook.c: static int pick_next_hook(struct child_process *cp, cp->env = hook_cb->options->env.v; cp->stdout_to_stderr = 1; - cp->trace2_hook_name = hook->command.buf; + cp->trace2_hook_name = hook_cb->hookname; + cp->dir = hook_cb->options->dir; /* 14: 07899ad346 = 13: 8930baa9db run-command: add stdin callback for parallelization 15: d3f18e433f ! 14: d0f362591a hook: provide stdin by string_list or callback @@ Commit message Signed-off-by: Emily Shaffer <emilyshaffer@xxxxxxxxxx> ## hook.c ## -@@ hook.c: void free_hook(struct hook *ptr) +@@ + + void free_hook(struct hook *ptr) { - if (ptr) { +- if (ptr) ++ if (ptr) { strbuf_release(&ptr->command); + free(ptr->feed_pipe_cb_data); - free(ptr); - } ++ } + free(ptr); } + @@ hook.c: static void append_or_move_hook(struct list_head *head, const char *command) strbuf_init(&to_add->command, 0); strbuf_addstr(&to_add->command, command); @@ hook.c: int run_hooks(const char *hookname, struct run_hooks_opt *options) + if (options->path_to_stdin && options->feed_pipe) + BUG("choose only one method to populate stdin"); + - strbuf_addstr(&hookname_str, hookname); + to_run = hook_list(hookname); - to_run = hook_list(&hookname_str); + list_for_each_safe(pos, tmp, to_run) { @@ hook.c: int run_hooks(const char *hookname, struct run_hooks_opt *options) run_processes_parallel_tr2(options->jobs, pick_next_hook, 16: 417c3f054e ! 15: 83bbb405a5 run-command: allow capturing of collated output @@ Commit message Signed-off-by: Emily Shaffer <emilyshaffer@xxxxxxxxxx> + + ## Notes ## + Originally when writing this patch I attempted to use a pipe in memory - + but managing its lifetime was actually pretty tricky, and I found I could + achieve the same thing with less code by doing it this way. Critique welcome, + including "no, you really need to do it with a pipe". + ## builtin/fetch.c ## @@ builtin/fetch.c: static int fetch_multiple(struct string_list *list, int max_children) result = run_processes_parallel_tr2(max_children, 17: c0f3471bd1 ! 16: 73ed5de54c hooks: allow callers to capture output @@ Commit message Signed-off-by: Emily Shaffer <emilyshaffer@xxxxxxxxxx> + + ## Notes ## + You can see this in practice in the conversions for some of the push hooks, + like 'receive-pack'. + ## hook.c ## @@ hook.c: void run_hooks_opt_init_sync(struct run_hooks_opt *o) o->dir = NULL; 18: 13446e4273 = 17: 900c4d8963 commit: use config-based hooks 19: 9380c43180 ! 18: a562120e22 am: convert applypatch hooks to use config @@ builtin/am.c: static void am_destroy(const struct am_state *state) { int ret; + struct run_hooks_opt opt; ++ + run_hooks_opt_init_sync(&opt); assert(state->msg); @@ builtin/am.c: static void do_commit(const struct am_state *state) const char *reflog_msg, *author, *committer = NULL; struct strbuf sb = STRBUF_INIT; + struct run_hooks_opt hook_opt; ++ + run_hooks_opt_init_async(&hook_opt); - if (run_hook_le(NULL, "pre-applypatch", NULL)) -+ if (run_hooks("pre-applypatch", &hook_opt)) ++ if (run_hooks("pre-applypatch", &hook_opt)) { ++ run_hooks_opt_clear(&hook_opt); exit(1); ++ } ++ ++ run_hooks_opt_clear(&hook_opt); if (write_cache_as_tree(&tree, 0, NULL)) + die(_("git write-tree failed to write a tree")); @@ builtin/am.c: static void do_commit(const struct am_state *state) fclose(fp); } - run_hook_le(NULL, "post-applypatch", NULL); ++ run_hooks_opt_init_async(&hook_opt); + run_hooks("post-applypatch", &hook_opt); + run_hooks_opt_clear(&hook_opt); 20: 316a605606 ! 19: e841ed4384 merge: use config-based hooks for post-merge hook @@ builtin/merge.c: static void finish(struct commit *head_commit, struct strbuf reflog_message = STRBUF_INIT; + struct run_hooks_opt opt; const struct object_id *head = &head_commit->object.oid; -+ run_hooks_opt_init_async(&opt); if (!msg) - strbuf_addstr(&reflog_message, getenv("GIT_REFLOG_ACTION")); @@ builtin/merge.c: static void finish(struct commit *head_commit, } /* Run a post-merge hook */ - run_hook_le(NULL, "post-merge", squash ? "1" : "0", NULL); ++ run_hooks_opt_init_async(&opt); + strvec_push(&opt.args, squash ? "1" : "0"); + run_hooks("post-merge", &opt); + run_hooks_opt_clear(&opt); 21: a5132f14b3 ! 20: 7e99398f7d gc: use hook library for pre-auto-gc hook @@ builtin/gc.c: static void add_repack_incremental_option(void) static int need_to_gc(void) { + struct run_hooks_opt hook_opt; -+ run_hooks_opt_init_async(&hook_opt); ++ /* * Setting gc.auto to 0 or negative can disable the * automatic gc. @@ builtin/gc.c: static int need_to_gc(void) return 0; - if (run_hook_le(NULL, "pre-auto-gc", NULL)) -+ if (run_hooks("pre-auto-gc", &hook_opt)) ++ run_hooks_opt_init_async(&hook_opt); ++ if (run_hooks("pre-auto-gc", &hook_opt)) { ++ run_hooks_opt_clear(&hook_opt); return 0; ++ } ++ run_hooks_opt_clear(&hook_opt); return 1; } + 22: 01f1331cc9 ! 21: 5423217ef2 rebase: teach pre-rebase to use hook.h @@ builtin/rebase.c: int cmd_rebase(int argc, const char **argv, const char *prefix struct option builtin_rebase_options[] = { OPT_STRING(0, "onto", &options.onto_name, N_("revision"), -@@ builtin/rebase.c: int cmd_rebase(int argc, const char **argv, const char *prefix) - }; - int i; - -+ run_hooks_opt_init_async(&hook_opt); -+ - if (argc == 2 && !strcmp(argv[1], "-h")) - usage_with_options(builtin_rebase_usage, - builtin_rebase_options); @@ builtin/rebase.c: int cmd_rebase(int argc, const char **argv, const char *prefix) } /* If a hook exists, give it a chance to interrupt*/ ++ run_hooks_opt_init_async(&hook_opt); + strvec_pushl(&hook_opt.args, options.upstream_arg, argc ? argv[0] : NULL, NULL); if (!ok_to_skip_pre_rebase && - run_hook_le(NULL, "pre-rebase", options.upstream_arg, - argc ? argv[0] : NULL, NULL)) -+ run_hooks("pre-rebase", &hook_opt)) ++ run_hooks("pre-rebase", &hook_opt)) { ++ run_hooks_opt_clear(&hook_opt); die(_("The pre-rebase hook refused to rebase.")); ++ } ++ run_hooks_opt_clear(&hook_opt); if (options.flags & REBASE_DIFFSTAT) { -@@ builtin/rebase.c: int cmd_rebase(int argc, const char **argv, const char *prefix) - ret = !!run_specific_rebase(&options, action); - - cleanup: -+ run_hooks_opt_clear(&hook_opt); - strbuf_release(&buf); - strbuf_release(&revisions); - free(options.head_name); + struct diff_options opts; 23: 85a7721adc ! 22: 1c0c5ad129 read-cache: convert post-index-change hook to use config @@ Documentation/githooks.txt: and "0" meaning they were not. ## read-cache.c ## @@ - #include "fsmonitor.h" #include "thread-utils.h" #include "progress.h" + #include "sparse-index.h" +#include "hook.h" ++>>>>>>> 9524a9d29d (read-cache: convert post-index-change hook to use config) /* Mask for the name length in ce_flags in the on-disk index */ @@ read-cache.c: static int do_write_locked_index(struct index_state *istate, struct lock_file *l - unsigned flags) { int ret; + int was_full = !istate->sparse_index; + struct run_hooks_opt hook_opt; -+ run_hooks_opt_init_async(&hook_opt); - /* - * TODO trace2: replace "the_repository" with the actual repo instance + ret = convert_to_sparse(istate); + @@ read-cache.c: static int do_write_locked_index(struct index_state *istate, struct lock_file *l else ret = close_lock_file_gently(lock); @@ read-cache.c: static int do_write_locked_index(struct index_state *istate, struc - run_hook_le(NULL, "post-index-change", - istate->updated_workdir ? "1" : "0", - istate->updated_skipworktree ? "1" : "0", NULL); ++ run_hooks_opt_init_async(&hook_opt); + strvec_pushl(&hook_opt.args, + istate->updated_workdir ? "1" : "0", + istate->updated_skipworktree ? "1" : "0", 24: 21ec3e1a9d ! 23: 1193e856e6 receive-pack: convert push-to-checkout hook to hook.h @@ builtin/receive-pack.c: static const char *push_to_checkout(unsigned char *hash, const char *work_tree) { + struct run_hooks_opt opt; ++ + run_hooks_opt_init_sync(&opt); + strvec_pushf(env, "GIT_WORK_TREE=%s", absolute_path(work_tree)); 25: e0405e96ad ! 24: 1817b6851b git-p4: use 'git hook' to run hooks @@ Commit message Signed-off-by: Emily Shaffer <emilyshaffer@xxxxxxxxxx> + + ## Notes ## + Maybe there is a better way to do this - I had a hard time getting this to run + locally, and Python is not my forte, so if anybody has a better approach I'd + love to just take that patch instead :) + + Since v6, removed the developer debug print statements.... :X + + Maybe there is a better way to do this - I had a hard time getting this to run + locally, and Python is not my forte, so if anybody has a better approach I'd + love to just take that patch instead :) + ## git-p4.py ## @@ git-p4.py: def decode_path(path): 26: c52578e078 ! 25: b3a354e4a8 hooks: convert 'post-checkout' hook to hook library @@ builtin/checkout.c: struct branch_info { int changed) { - return run_hook_le(NULL, "post-checkout", -- oid_to_hex(old_commit ? &old_commit->object.oid : &null_oid), -- oid_to_hex(new_commit ? &new_commit->object.oid : &null_oid), +- oid_to_hex(old_commit ? &old_commit->object.oid : null_oid()), +- oid_to_hex(new_commit ? &new_commit->object.oid : null_oid()), - changed ? "1" : "0", NULL); + struct run_hooks_opt opt; + int rc; @@ builtin/checkout.c: struct branch_info { a commit exists. */ - + strvec_pushl(&opt.args, -+ oid_to_hex(old_commit ? &old_commit->object.oid : &null_oid), -+ oid_to_hex(new_commit ? &new_commit->object.oid : &null_oid), ++ oid_to_hex(old_commit ? &old_commit->object.oid : null_oid()), ++ oid_to_hex(new_commit ? &new_commit->object.oid : null_oid()), + changed ? "1" : "0", + NULL); + rc = run_hooks("post-checkout", &opt); @@ builtin/clone.c: static int checkout(int submodule_progress) struct tree_desc t; int err = 0; + struct run_hooks_opt hook_opt; -+ run_hooks_opt_init_sync(&hook_opt); if (option_no_checkout) return 0; @@ builtin/clone.c: static int checkout(int submodule_progress) if (write_locked_index(&the_index, &lock_file, COMMIT_LOCK)) die(_("unable to write new index file")); -- err |= run_hook_le(NULL, "post-checkout", oid_to_hex(&null_oid), +- err |= run_hook_le(NULL, "post-checkout", oid_to_hex(null_oid()), - oid_to_hex(&oid), "1", NULL); -+ strvec_pushl(&hook_opt.args, oid_to_hex(&null_oid), oid_to_hex(&oid), "1", NULL); ++ run_hooks_opt_init_sync(&hook_opt); ++ strvec_pushl(&hook_opt.args, oid_to_hex(null_oid()), oid_to_hex(&oid), "1", NULL); + err |= run_hooks("post-checkout", &hook_opt); + run_hooks_opt_clear(&hook_opt); @@ builtin/worktree.c: static int add_worktree(const char *path, const char *refnam - cp.argv = NULL; - cp.trace2_hook_name = "post-checkout"; - strvec_pushl(&cp.args, absolute_path(hook), -- oid_to_hex(&null_oid), +- oid_to_hex(null_oid()), - oid_to_hex(&commit->object.oid), - "1", NULL); - ret = run_command(&cp); - } + struct run_hooks_opt opt; ++ + run_hooks_opt_init_sync(&opt); + + strvec_pushl(&opt.env, "GIT_DIR", "GIT_WORK_TREE", NULL); + strvec_pushl(&opt.args, -+ oid_to_hex(&null_oid), ++ oid_to_hex(null_oid()), + oid_to_hex(&commit->object.oid), + "1", + NULL); @@ builtin/worktree.c: static int add_worktree(const char *path, const char *refnam strvec_clear(&child_env); + ## read-cache.c ## +@@ + #include "progress.h" + #include "sparse-index.h" + #include "hook.h" +->>>>>>> 9524a9d29d (read-cache: convert post-index-change hook to use config) + + /* Mask for the name length in ce_flags in the on-disk index */ + + ## reset.c ## @@ #include "tree-walk.h" @@ reset.c: int reset_head(struct repository *r, struct object_id *oid, const char } - if (run_hook) - run_hook_le(NULL, "post-checkout", -- oid_to_hex(orig ? orig : &null_oid), +- oid_to_hex(orig ? orig : null_oid()), - oid_to_hex(oid), "1", NULL); + if (run_hook) { + struct run_hooks_opt opt; ++ + run_hooks_opt_init_sync(&opt); + strvec_pushl(&opt.args, -+ oid_to_hex(orig ? orig : &null_oid), ++ oid_to_hex(orig ? orig : null_oid()), + oid_to_hex(oid), + "1", + NULL); 27: 316cb6f584 ! 26: 692352f9aa hook: convert 'post-rewrite' hook to config @@ builtin/am.c: static int run_applypatch_msg_hook(struct am_state *state) - const char *hook = find_hook("post-rewrite"); + struct run_hooks_opt opt; int ret; -+ run_hooks_opt_init_async(&opt); - if (!hook) - return 0; -- ++ run_hooks_opt_init_async(&opt); + - strvec_push(&cp.args, hook); - strvec_push(&cp.args, "rebase"); -- -- cp.in = xopen(am_path(state, "rewritten"), O_RDONLY); -- cp.stdout_to_stderr = 1; -- cp.trace2_hook_name = "post-rewrite"; + strvec_push(&opt.args, "rebase"); + opt.path_to_stdin = am_path(state, "rewritten"); -- ret = run_command(&cp); +- cp.in = xopen(am_path(state, "rewritten"), O_RDONLY); +- cp.stdout_to_stderr = 1; +- cp.trace2_hook_name = "post-rewrite"; + ret = run_hooks("post-rewrite", &opt); +- ret = run_command(&cp); +- - close(cp.in); + run_hooks_opt_clear(&opt); return ret; @@ sequencer.c: int update_head_with_reflog(const struct commit *old_head, + struct string_list to_stdin = STRING_LIST_INIT_DUP; int code; - struct strbuf sb = STRBUF_INIT; -+ run_hooks_opt_init_async(&opt); - argv[0] = find_hook("post-rewrite"); - if (!argv[0]) - return 0; -+ strvec_push(&opt.args, "amend"); ++ run_hooks_opt_init_async(&opt); - argv[1] = "amend"; - argv[2] = NULL; @@ sequencer.c: int update_head_with_reflog(const struct commit *old_head, - strbuf_release(&sb); - sigchain_pop(SIGPIPE); - return finish_command(&proc); ++ strvec_push(&opt.args, "amend"); ++ + strbuf_addf(&tmp, + "%s %s", + oid_to_hex(oldoid), @@ sequencer.c: static int pick_commits(struct repository *r, - strvec_push(&child.args, "--for-rewrite=rebase"); + struct child_process notes_cp = CHILD_PROCESS_INIT; + struct run_hooks_opt hook_opt; ++ + run_hooks_opt_init_async(&hook_opt); + + notes_cp.in = open(rebase_path_rewritten_list(), O_RDONLY); 28: 0ab776068d = 27: 964011dfdd transport: convert pre-push hook to use config 29: 601dada804 ! 28: c04822add9 reference-transaction: look for hooks in config @@ t/t1416-ref-transaction-hooks.sh: test_expect_success 'interleaving hook calls s EOF git push ./target-repo.git PRE POST && + + ## transport.c ## +@@ transport.c: static int run_pre_push_hook(struct transport *transport, + struct strbuf tmp = STRBUF_INIT; + struct ref *r; + struct string_list to_stdin = STRING_LIST_INIT_DUP; ++ + run_hooks_opt_init_async(&opt); + + strvec_push(&opt.args, transport->remote->name); 30: d60f2b146e ! 29: ddc6f56bec receive-pack: convert 'update' hook to hook.h @@ builtin/receive-pack.c: static int run_receive_hook(struct command *commands, return status; } --static int run_update_hook(struct command *cmd) +static void hook_output_to_sideband(struct strbuf *output, void *cb_data) - { -- const char *argv[5]; -- struct child_process proc = CHILD_PROCESS_INIT; -- int code; ++{ + int keepalive_active = 0; - -- argv[0] = find_hook("update"); -- if (!argv[0]) -- return 0; ++ + if (keepalive_in_sec <= 0) + use_keepalive = KEEPALIVE_NEVER; + if (use_keepalive == KEEPALIVE_ALWAYS) + keepalive_active = 1; - -- argv[1] = cmd->ref_name; -- argv[2] = oid_to_hex(&cmd->old_oid); -- argv[3] = oid_to_hex(&cmd->new_oid); -- argv[4] = NULL; ++ + /* send a keepalive if there is no data to write */ + if (keepalive_active && !output->len) { + static const char buf[] = "0005\1"; + write_or_die(1, buf, sizeof(buf) - 1); + return; + } - -- proc.no_stdin = 1; -- proc.stdout_to_stderr = 1; -- proc.err = use_sideband ? -1 : 0; -- proc.argv = argv; -- proc.trace2_hook_name = "update"; ++ + if (use_keepalive == KEEPALIVE_AFTER_NUL && !keepalive_active) { + const char *first_null = memchr(output->buf, '\0', output->len); + if (first_null) { @@ builtin/receive-pack.c: static int run_receive_hook(struct command *commands, + send_sideband(1, 2, output->buf, output->len, use_sideband); +} + -+static int run_update_hook(struct command *cmd) -+{ + static int run_update_hook(struct command *cmd) + { +- const char *argv[5]; +- struct child_process proc = CHILD_PROCESS_INIT; + struct run_hooks_opt opt; -+ int code; + int code; + +- argv[0] = find_hook("update"); +- if (!argv[0]) +- return 0; + run_hooks_opt_init_async(&opt); -+ + +- argv[1] = cmd->ref_name; +- argv[2] = oid_to_hex(&cmd->old_oid); +- argv[3] = oid_to_hex(&cmd->new_oid); +- argv[4] = NULL; + 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.argv = argv; +- proc.trace2_hook_name = "update"; +- - code = start_command(&proc); - if (code) - return code; 31: 1e6898670b ! 30: e1e810869f proc-receive: acquire hook list from hook.h @@ builtin/receive-pack.c: static int run_proc_receive_hook(struct command *command - argv[0] = find_hook("proc-receive"); - if (!argv[0]) { -+ struct strbuf hookname = STRBUF_INIT; + struct hook *proc_receive = NULL; + struct list_head *pos, *hooks; + -+ strbuf_addstr(&hookname, "proc-receive"); -+ hooks = hook_list(&hookname); ++ hooks = hook_list("proc-receive"); + + list_for_each(pos, hooks) { + if (proc_receive) { 32: 012e3a7a79 ! 31: b8be5a2288 post-update: use hook.h library @@ builtin/receive-pack.c: static const char *update(struct command *cmd, struct sh struct command *cmd; - struct child_process proc = CHILD_PROCESS_INIT; - const char *hook; -- ++ struct run_hooks_opt opt; + - hook = find_hook("post-update"); - if (!hook) - return; -+ struct run_hooks_opt opt; + run_hooks_opt_init_async(&opt); for (cmd = commands; cmd; cmd = cmd->next) { 33: 2740bcda6d = 32: 1cc1384eae receive-pack: convert receive hooks to hook.h 34: f201f3af5f = 33: 1bb9a3810c bugreport: use hook_exists instead of find_hook 35: 0956a94cc7 ! 34: 3db7bf3b0d git-send-email: use 'git hook run' for 'sendemail-validate' @@ git-send-email.perl: sub unique_email_list { my ($fn, $xfer_encoding) = @_; - if ($repo) { -- my $validate_hook = catfile(catdir($repo->repo_path(), 'hooks'), +- my $validate_hook = catfile($repo->hooks_path(), - 'sendemail-validate'); - my $hook_error; - if (-x $validate_hook) { @@ git-send-email.perl: sub unique_email_list { - chdir($repo->wc_path() or $repo->repo_path()) - or die("chdir: $!"); - local $ENV{"GIT_DIR"} = $repo->repo_path(); -- $hook_error = "rejected by sendemail-validate hook" -- if system($validate_hook, $target); +- $hook_error = system_or_msg([$validate_hook, $target]); - chdir($cwd_save) or die("chdir: $!"); - } -- return $hook_error if $hook_error; +- if ($hook_error) { +- die sprintf(__("fatal: %s: rejected by sendemail-validate hook\n" . +- "%s\n" . +- "warning: no patches were sent\n"), $fn, $hook_error); +- } - } + my $target = abs_path($fn); -+ return "rejected by sendemail-validate hook" -+ if system(("git", "hook", "run", "sendemail-validate", "-a", -+ $target)); ++ ++ system_or_die(["git", "hook", "run", "sendemail-validate", "-j1", "-a", $target], ++ sprintf(__("fatal: %s: rejected by sendemail-validate hook\n" . ++ "warning: no patches were sent\n"), ++ $fn)); # Any long lines will be automatically fixed if we use a suitable transfer # encoding. ## t/t9001-send-email.sh ## +@@ t/t9001-send-email.sh: test_expect_success $PREREQ "--validate respects relative core.hooksPath path" ' + test_path_is_file my-hooks.ran && + cat >expect <<-EOF && + fatal: longline.patch: rejected by sendemail-validate hook +- fatal: command '"'"'$PWD/my-hooks/sendemail-validate'"'"' died with exit code 1 + warning: no patches were sent + EOF + test_cmp expect actual +@@ t/t9001-send-email.sh: test_expect_success $PREREQ "--validate respects absolute core.hooksPath path" ' + test_path_is_file my-hooks.ran && + cat >expect <<-EOF && + fatal: longline.patch: rejected by sendemail-validate hook +- fatal: command '"'"'$PWD/my-hooks/sendemail-validate'"'"' died with exit code 1 + warning: no patches were sent + EOF + test_cmp expect actual @@ t/t9001-send-email.sh: test_expect_success $PREREQ 'invoke hook' ' mkdir -p .git/hooks && 36: 7f05b25392 ! 35: d2a477d9e3 run-command: stop thinking about hooks @@ hook.c: static int should_include_hookdir(const char *path, enum hookdir_opt cfg +} + + - struct list_head* hook_list(const struct strbuf* hookname) + struct list_head* hook_list(const char* hookname) { struct strbuf hook_key = STRBUF_INIT; -@@ hook.c: struct list_head* hook_list(const struct strbuf* hookname) +@@ hook.c: struct list_head* hook_list(const char* hookname) git_config(hook_config_lookup, &cb_data); if (have_git_dir()) { -- const char *legacy_hook_path = find_hook(hookname->buf); -+ const char *legacy_hook_path = find_legacy_hook(hookname->buf); +- const char *legacy_hook_path = find_hook(hookname); ++ const char *legacy_hook_path = find_legacy_hook(hookname); /* Unconditionally add legacy hook, but annotate it. */ if (legacy_hook_path) { 37: e9b1f847f2 < -: ---------- docs: unify githooks and git-hook manpages -: ---------- > 36: 62a3e3b419 doc: clarify fsmonitor-watchman specification -: ---------- > 37: 5c864de1aa docs: link githooks and git-hook manpages -- 2.31.1.818.g46aad6cb9e-goog