{"thread":{"id":"56692","subject":"[PATCH 00/13] hook.[ch]: new library to run hooks + simple hook conversion","startedAt":"2021-10-12T13:30:57Z","lastAt":"2021-10-13T13:04:19Z","messageCount":18,"participants":["Ævar Arnfjörð Bjarmason","René Scharfe","Bagas Sanjaya"],"isPatch":true,"patchVersion":1,"patchTotal":13},"messages":[{"id":"438536","messageId":"cover-00.13-00000000000-20211012T131934Z-avarab@gmail.com","threadId":"56692","inReplyTo":null,"subject":"[PATCH 00/13] hook.[ch]: new library to run hooks + simple hook conversion","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2021-10-12T13:30:25Z","receivedAt":"2021-10-12T13:30:57Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"This series is part 2 of an incremental restart of the greater\n\"config-based hooks\" topic by Emily Shaffer that I've been helping\nwith. See [1] for part 1, and [2] (search for \"mark2\") for a summary\nof parts to come.\n\nThis goes on top of the \"ab/config-based-hooks-1\" topic that's already\nin \"next\" and marked for graduation to \"master\".\n\nIn this topic we build upon the skeleton hook.[ch] library and build\ninfrastructure added in part 1 and add a hook running library and new\n\"git hook\" command.\n\nThe new \"git hook\" command is (for now) only for the benefit of\nin-tree non-C programs that need to run hooks, and this series\nconverts git-send-email and git-p4 to use it.\n\nAt the end of the series we remove\n\"run_hook_{le,ve}()\" from run-command.c, as we've migrated all its\ncallers.\n\nWhat we don't do is convert any of the more complex in-tree hooks that\nrequire input on stdin such as \"pre-receive\" and the like, that's in\nlater parts once this lands.\n\nThis series is approximately patch 6-20/36 of the previous 36 patch\nab/config-based-hooks-base topic. A range-diff to that v5[3] is\nincluded below.\n\nThe changes since that v5 are rather trivial, they are:\n\n * Formatting changes to reduce the diff to parts that come after this\n (which Emily & I were juggling at the time), and re-flowing some\n overly long lines.\n\n * The new test is now marked for SANITIZE=leak testing, with the new\n test facility I've added recently (and which just landed in\n \"master\"). They all pass, the new API doesn't leak.\n\n * I rewrote some of the git-send-email.perl code to avoid\n de-duplication and hardcoding (just using intermediate variables).\n\nThings to focus on reviewing:\n\n * This should all be pretty solid and well tested, but the git-p4.py\n part in particular I've never tested for real (not having access to\n p4), and think Emily hasn't either[4].\n\n The relevant patch looks trivially correct to me[5], and I've tried it\n out in the Python REPL. But if any of the CC'd people active in\n git-p4.py development could give it some end-to-end testing that\n would be much appreciated.\n\n * Both Python and Perl code now calls the in-between \"git hook run\"\n command rather than calling hooks directly. Will this behave\n differently due to any special behavior running via a git built-in\n adds?\n\n I vaguely recall a third-party \"git-foo\" program breaking in the\n past when invoked as \"git foo\" but not \"git-foo\", due to git\n squatting on SIGINT, but none of that should be relevant here\n (we're not starting a pager etc.).\n\n1. https://lore.kernel.org/git/cover-v2-0.8-00000000000-20210926T185800Z-avarab@gmail.com/\n2. https://lore.kernel.org/git/875yut8nns.fsf@evledraar.gmail.com/\n3. https://lore.kernel.org/git/cover-v5-00.36-00000000000-20210902T125110Z-avarab@gmail.com/\n4. https://lore.kernel.org/git/20210311021037.3001235-26-emilyshaffer@google.com/\n5. https://lore.kernel.org/git/patch-09.13-69cc447a1e1-20211012T131934Z-avarab@gmail.com\n\nEmily Shaffer (12):\n hook: add 'run' subcommand\n gc: use hook library for pre-auto-gc hook\n rebase: convert pre-rebase to use hook.h\n am: convert applypatch to use hook.h\n hooks: convert 'post-checkout' hook to hook library\n merge: convert post-merge to use hook.h\n send-email: use 'git hook run' for 'sendemail-validate'\n git-p4: use 'git hook' to run hooks\n commit: convert {pre-commit,prepare-commit-msg} hook to hook.h\n read-cache: convert post-index-change to use hook.h\n receive-pack: convert push-to-checkout hook to hook.h\n run-command: remove old run_hook_{le,ve}() hook API\n\nÆvar Arnfjörð Bjarmason (1):\n git hook run: add an --ignore-missing flag\n\n .gitignore | 1 +\n Documentation/git-hook.txt | 46 +++++++++++++\n Documentation/githooks.txt | 4 ++\n Makefile | 1 +\n builtin.h | 1 +\n builtin/am.c | 8 ++-\n builtin/checkout.c | 14 ++--\n builtin/clone.c | 7 +-\n builtin/gc.c | 3 +-\n builtin/hook.c | 96 ++++++++++++++++++++++++++\n builtin/merge.c | 4 +-\n builtin/rebase.c | 8 ++-\n builtin/receive-pack.c | 7 +-\n builtin/worktree.c | 28 ++++----\n command-list.txt | 1 +\n commit.c | 15 +++--\n git-p4.py | 72 ++------------------\n git-send-email.perl | 22 +++---\n git.c | 1 +\n hook.c | 130 +++++++++++++++++++++++++++++++++++\n hook.h | 61 +++++++++++++++++\n read-cache.c | 11 ++-\n reset.c | 14 ++--\n run-command.c | 32 ---------\n run-command.h | 17 -----\n t/t1800-hook.sh | 134 +++++++++++++++++++++++++++++++++++++\n t/t9001-send-email.sh | 4 +-\n 27 files changed, 572 insertions(+), 170 deletions(-)\n create mode 100644 Documentation/git-hook.txt\n create mode 100644 builtin/hook.c\n create mode 100755 t/t1800-hook.sh\n\nRange-diff:\n 1: 72dd1010f5b ! 1: a39c0748d3f hook: add 'run' subcommand\n @@ hook.h\n +\tstruct run_hooks_opt *options;\n +};\n \n - /**\n + /*\n * Returns the path to the hook file, or NULL if the hook is missing\n @@ hook.h: const char *find_hook(const char *name);\n */\n @@ t/t1800-hook.sh (new)\n +\n +test_description='git-hook command'\n +\n ++TEST_PASSES_SANITIZE_LEAK=true\n +. ./test-lib.sh\n +\n +test_expect_success 'git hook usage' '\n 2: 821cc9bf11e ! 2: dbac4204f7b gc: use hook library for pre-auto-gc hook\n @@ hook.c: int run_hooks(const char *hook_name, const char *hook_path,\n +int run_hooks_oneshot(const char *hook_name, struct run_hooks_opt *options)\n +{\n +\tconst char *hook_path;\n -+\tint ret;\n +\tstruct run_hooks_opt hook_opt_scratch = RUN_HOOKS_OPT_INIT;\n ++\tint ret = 0;\n +\n +\tif (!options)\n +\t\toptions = &hook_opt_scratch;\n +\n +\thook_path = find_hook(hook_name);\n -+\tif (!hook_path) {\n -+\t\tret = 0;\n ++\tif (!hook_path)\n +\t\tgoto cleanup;\n -+\t}\n +\n +\tret = run_hooks(hook_name, hook_path, options);\n -+\n +cleanup:\n +\trun_hooks_opt_clear(options);\n +\n 3: d71c90254ea ! 3: ff306debcb8 rebase: convert pre-rebase to use hook.h\n @@ builtin/rebase.c: int cmd_rebase(int argc, const char **argv, const char *prefix\n \t}\n \n \t/* If a hook exists, give it a chance to interrupt*/\n -+\tstrvec_pushl(&hook_opt.args, options.upstream_arg, argc ? argv[0] : NULL, NULL);\n ++\tstrvec_push(&hook_opt.args, options.upstream_arg);\n ++\tif (argc)\n ++\t\tstrvec_push(&hook_opt.args, argv[0]);\n \tif (!ok_to_skip_pre_rebase &&\n -\t run_hook_le(NULL, \"pre-rebase\", options.upstream_arg,\n -\t\t\targc ? argv[0] : NULL, NULL))\n 4: ea3af2ccc4d = 4: b1d529ca485 am: convert applypatch to use hook.h\n 5: fed0b52f88f ! 5: 15d71fc210b hooks: convert 'post-checkout' hook to hook library\n @@ builtin/clone.c: static int checkout(int submodule_progress)\n \n -\terr |= run_hook_le(NULL, \"post-checkout\", oid_to_hex(null_oid()),\n -\t\t\t oid_to_hex(&oid), \"1\", NULL);\n -+\tstrvec_pushl(&hook_opt.args, oid_to_hex(null_oid()), oid_to_hex(&oid), \"1\", NULL);\n ++\tstrvec_pushl(&hook_opt.args, oid_to_hex(null_oid()), oid_to_hex(&oid),\n ++\t\t \"1\", NULL);\n +\terr |= run_hooks_oneshot(\"post-checkout\", &hook_opt);\n \n \tif (!err && (option_recurse_submodules.nr > 0)) {\n 6: 53d8721a0e3 = 6: 08f27f0d6be merge: convert post-merge to use hook.h\n 7: d60827a2856 ! 7: 107c14d740f git hook run: add an --ignore-missing flag\n @@ builtin/hook.c: static int run(int argc, const char **argv, const char *prefix)\n \tconst char *hook_path;\n \tstruct option run_options[] = {\n +\t\tOPT_BOOL(0, \"ignore-missing\", &ignore_missing,\n -+\t\t\t N_(\"exit quietly with a zero exit code if the requested hook cannot be found\")),\n ++\t\t\t N_(\"silently ignore missing requested \")),\n \t\tOPT_END(),\n \t};\n \tint ret;\n 8: d4976a0821f ! 8: 1d30e2dbbe0 send-email: use 'git hook run' for 'sendemail-validate'\n @@ git-send-email.perl: sub validate_patch {\n \t\t\t\tor die(\"chdir: $!\");\n \t\t\tlocal $ENV{\"GIT_DIR\"} = $repo->repo_path();\n -\t\t\t$hook_error = system_or_msg([$validate_hook, $target]);\n -+\t\t\tmy @validate_hook = (\"git\", \"hook\", \"run\", \"--ignore-missing\", $hook_name, \"--\", $target);\n -+\t\t\t$hook_error = system_or_msg(\\@validate_hook, undef,\n -+\t\t\t\t\t\t \"git hook run $hook_name -- \");\n ++\t\t\tmy @cmd = (\"git\", \"hook\", \"run\", \"--ignore-missing\",\n ++\t\t\t\t $hook_name, \"--\");\n ++\t\t\tmy @cmd_msg = (@cmd, \"\");\n ++\t\t\tmy @cmd_run = (@cmd, $target);\n ++\t\t\t$hook_error = system_or_msg(\\@cmd_run, undef, \"@cmd_msg\");\n \t\t\tchdir($cwd_save) or die(\"chdir: $!\");\n \t\t}\n \t\tif ($hook_error) {\n @@ t/t9001-send-email.sh: test_expect_success $PREREQ \"--validate respects relative\n \tcat >expect <<-EOF &&\n \tfatal: longline.patch: rejected by sendemail-validate hook\n -\tfatal: command '\"'\"'my-hooks/sendemail-validate'\"'\"' died with exit code 1\n -+\tfatal: command '\"'\"'git hook run sendemail-validate -- '\"'\"' died with exit code 1\n ++\tfatal: command '\"'\"'git hook run --ignore-missing sendemail-validate -- '\"'\"' died with exit code 1\n \twarning: no patches were sent\n \tEOF\n \ttest_cmp expect actual\n @@ t/t9001-send-email.sh: test_expect_success $PREREQ \"--validate respects absolute\n \tcat >expect <<-EOF &&\n \tfatal: longline.patch: rejected by sendemail-validate hook\n -\tfatal: command '\"'\"'$hooks_path/sendemail-validate'\"'\"' died with exit code 1\n -+\tfatal: command '\"'\"'git hook run sendemail-validate -- '\"'\"' died with exit code 1\n ++\tfatal: command '\"'\"'git hook run --ignore-missing sendemail-validate -- '\"'\"' died with exit code 1\n \twarning: no patches were sent\n \tEOF\n \ttest_cmp expect actual\n 9: 99f3dcd1945 = 9: 69cc447a1e1 git-p4: use 'git hook' to run hooks\n10: 509761454e6 = 10: 1c22b2992cf commit: convert {pre-commit,prepare-commit-msg} hook to hook.h\n11: e2c94d95427 ! 11: e762fce32af read-cache: convert post-index-change to use hook.h\n @@ Commit message\n Move the post-index-change hook away from run-command.h to and over to\n the new hook.h library.\n \n - This removes the last direct user of run_hook_ve(), so we can make the\n - function static now. It'll be removed entirely soon.\n + This removes the last direct user of \"run_hook_ve()\" outside of\n + run-command.c (\"run_hook_le()\" still uses it). So we can make the\n + function static now. A subsequent commit will remove this code\n + entirely when \"run_hook_le()\" itself goes away.\n \n Signed-off-by: Emily Shaffer \n Signed-off-by: Ævar Arnfjörð Bjarmason \n @@ read-cache.c: static int do_write_locked_index(struct index_state *istate, struc\n \tint was_full = !istate->sparse_index;\n +\tstruct run_hooks_opt hook_opt = RUN_HOOKS_OPT_INIT;\n \n - \tret = convert_to_sparse(istate);\n + \tret = convert_to_sparse(istate, 0);\n \n @@ read-cache.c: static int do_write_locked_index(struct index_state *istate, struct lock_file *l\n \telse\n12: fa7d0d24ea2 = 12: d63b91196ae receive-pack: convert push-to-checkout hook to hook.h\n13: 428bb5a6792 = 13: fe8996dda3e run-command: remove old run_hook_{le,ve}() hook API\n-- \n2.33.0.1567.g7b23ce7ed9e\n\n"},{"id":"438537","messageId":"patch-01.13-a39c0748d3f-20211012T131934Z-avarab@gmail.com","threadId":"56692","inReplyTo":"cover-00.13-00000000000-20211012T131934Z-avarab@gmail.com","subject":"[PATCH 01/13] hook: add 'run' subcommand","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2021-10-12T13:30:26Z","receivedAt":"2021-10-12T13:31:00Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"From: Emily Shaffer \n\nIn order to enable hooks to be run as an external process, by a\nstandalone Git command, or by tools which wrap Git, provide an external\nmeans to run all configured hook commands for a given hook event.\n\nMost of our hooks require more complex functionality than this, but\nlet's start with the bare minimum required to support our simplest\nhooks.\n\nIn terms of implementation the usage_with_options() and \"goto usage\"\npattern here mirrors that of\nbuiltin/{commit-graph,multi-pack-index}.c.\n\nSigned-off-by: Emily Shaffer \nSigned-off-by: Ævar Arnfjörð Bjarmason \n---\n .gitignore | 1 +\n Documentation/git-hook.txt | 38 +++++++++++\n Documentation/githooks.txt | 4 ++\n Makefile | 1 +\n builtin.h | 1 +\n builtin/hook.c | 90 ++++++++++++++++++++++++++\n command-list.txt | 1 +\n git.c | 1 +\n hook.c | 101 +++++++++++++++++++++++++++++\n hook.h | 39 +++++++++++\n t/t1800-hook.sh | 129 +++++++++++++++++++++++++++++++++++++\n 11 files changed, 406 insertions(+)\n create mode 100644 Documentation/git-hook.txt\n create mode 100644 builtin/hook.c\n create mode 100755 t/t1800-hook.sh\n\ndiff --git a/.gitignore b/.gitignore\nindex 6be9de41ae8..66189ca3cdc 100644\n--- a/.gitignore\n+++ b/.gitignore\n@@ -77,6 +77,7 @@\n /git-grep\n /git-hash-object\n /git-help\n+/git-hook\n /git-http-backend\n /git-http-fetch\n /git-http-push\ndiff --git a/Documentation/git-hook.txt b/Documentation/git-hook.txt\nnew file mode 100644\nindex 00000000000..660d6a992a0\n--- /dev/null\n+++ b/Documentation/git-hook.txt\n@@ -0,0 +1,38 @@\n+git-hook(1)\n+===========\n+\n+NAME\n+----\n+git-hook - run git hooks\n+\n+SYNOPSIS\n+--------\n+[verse]\n+'git hook' run [-- ]\n+\n+DESCRIPTION\n+-----------\n+\n+This command is an interface to git hooks (see linkgit:githooks[5]).\n+Currently it only provides a convenience wrapper for running hooks for\n+use by git itself. In the future it might gain other functionality.\n+\n+SUBCOMMANDS\n+-----------\n+\n+run::\n+\tRun the `` hook. See linkgit:githooks[5] for\n+\tthe hook names we support.\n++\n+Any positional arguments to the hook should be passed after an\n+optional `--` (or `--end-of-options`, see linkgit:gitcli[7]). The\n+arguments (if any) differ by hook name, see linkgit:githooks[5] for\n+what those are.\n+\n+SEE ALSO\n+--------\n+linkgit:githooks[5]\n+\n+GIT\n+---\n+Part of the linkgit:git[1] suite\ndiff --git a/Documentation/githooks.txt b/Documentation/githooks.txt\nindex b51959ff941..a16e62bc8c8 100644\n--- a/Documentation/githooks.txt\n+++ b/Documentation/githooks.txt\n@@ -698,6 +698,10 @@ and \"0\" meaning they were not.\n Only one parameter should be set to \"1\" when the hook runs. The hook\n running passing \"1\", \"1\" should not be possible.\n \n+SEE ALSO\n+--------\n+linkgit:git-hook[1]\n+\n GIT\n ---\n Part of the linkgit:git[1] suite\ndiff --git a/Makefile b/Makefile\nindex 877492386ee..12b481fdabe 100644\n--- a/Makefile\n+++ b/Makefile\n@@ -1108,6 +1108,7 @@ BUILTIN_OBJS += builtin/get-tar-commit-id.o\n BUILTIN_OBJS += builtin/grep.o\n BUILTIN_OBJS += builtin/hash-object.o\n BUILTIN_OBJS += builtin/help.o\n+BUILTIN_OBJS += builtin/hook.o\n BUILTIN_OBJS += builtin/index-pack.o\n BUILTIN_OBJS += builtin/init-db.o\n BUILTIN_OBJS += builtin/interpret-trailers.o\ndiff --git a/builtin.h b/builtin.h\nindex 8a58743ed63..83379f3832c 100644\n--- a/builtin.h\n+++ b/builtin.h\n@@ -164,6 +164,7 @@ int cmd_get_tar_commit_id(int argc, const char **argv, const char *prefix);\n int cmd_grep(int argc, const char **argv, const char *prefix);\n int cmd_hash_object(int argc, const char **argv, const char *prefix);\n int cmd_help(int argc, const char **argv, const char *prefix);\n+int cmd_hook(int argc, const char **argv, const char *prefix);\n int cmd_index_pack(int argc, const char **argv, const char *prefix);\n int cmd_init_db(int argc, const char **argv, const char *prefix);\n int cmd_interpret_trailers(int argc, const char **argv, const char *prefix);\ndiff --git a/builtin/hook.c b/builtin/hook.c\nnew file mode 100644\nindex 00000000000..012a2973b38\n--- /dev/null\n+++ b/builtin/hook.c\n@@ -0,0 +1,90 @@\n+#include \"cache.h\"\n+#include \"builtin.h\"\n+#include \"config.h\"\n+#include \"hook.h\"\n+#include \"parse-options.h\"\n+#include \"strbuf.h\"\n+#include \"strvec.h\"\n+\n+#define BUILTIN_HOOK_RUN_USAGE \\\n+\tN_(\"git hook run [-- ]\")\n+\n+static const char * const builtin_hook_usage[] = {\n+\tBUILTIN_HOOK_RUN_USAGE,\n+\tNULL\n+};\n+\n+static const char * const builtin_hook_run_usage[] = {\n+\tBUILTIN_HOOK_RUN_USAGE,\n+\tNULL\n+};\n+\n+static int run(int argc, const char **argv, const char *prefix)\n+{\n+\tint i;\n+\tstruct run_hooks_opt opt = RUN_HOOKS_OPT_INIT;\n+\tconst char *hook_name;\n+\tconst char *hook_path;\n+\tstruct option run_options[] = {\n+\t\tOPT_END(),\n+\t};\n+\tint ret;\n+\n+\targc = parse_options(argc, argv, prefix, run_options,\n+\t\t\t builtin_hook_run_usage,\n+\t\t\t PARSE_OPT_KEEP_DASHDASH);\n+\n+\tif (!argc)\n+\t\tgoto usage;\n+\n+\t/*\n+\t * Having a -- for \"run\" when providing is\n+\t * mandatory.\n+\t */\n+\tif (argc > 1 && strcmp(argv[1], \"--\") &&\n+\t strcmp(argv[1], \"--end-of-options\"))\n+\t\tgoto usage;\n+\n+\t/* Add our arguments, start after -- */\n+\tfor (i = 2 ; i < argc; i++)\n+\t\tstrvec_push(&opt.args, argv[i]);\n+\n+\t/* Need to take into account core.hooksPath */\n+\tgit_config(git_default_config, NULL);\n+\n+\t/*\n+\t * We are not using a plain run_hooks() because we'd like to\n+\t * detect missing hooks. Let's find it ourselves and call\n+\t * run_hooks() instead.\n+\t */\n+\thook_name = argv[0];\n+\thook_path = find_hook(hook_name);\n+\tif (!hook_path) {\n+\t\terror(\"cannot find a hook named %s\", hook_name);\n+\t\treturn 1;\n+\t}\n+\n+\tret = run_hooks(hook_name, hook_path, &opt);\n+\trun_hooks_opt_clear(&opt);\n+\treturn ret;\n+usage:\n+\tusage_with_options(builtin_hook_run_usage, run_options);\n+}\n+\n+int cmd_hook(int argc, const char **argv, const char *prefix)\n+{\n+\tstruct option builtin_hook_options[] = {\n+\t\tOPT_END(),\n+\t};\n+\n+\targc = parse_options(argc, argv, NULL, builtin_hook_options,\n+\t\t\t builtin_hook_usage, PARSE_OPT_STOP_AT_NON_OPTION);\n+\tif (!argc)\n+\t\tgoto usage;\n+\n+\tif (!strcmp(argv[0], \"run\"))\n+\t\treturn run(argc, argv, prefix);\n+\n+usage:\n+\tusage_with_options(builtin_hook_usage, builtin_hook_options);\n+}\ndiff --git a/command-list.txt b/command-list.txt\nindex a289f09ed6f..9ccd8e5aebe 100644\n--- a/command-list.txt\n+++ b/command-list.txt\n@@ -103,6 +103,7 @@ git-grep mainporcelain info\n git-gui mainporcelain\n git-hash-object plumbingmanipulators\n git-help ancillaryinterrogators complete\n+git-hook mainporcelain\n git-http-backend synchingrepositories\n git-http-fetch synchelpers\n git-http-push synchelpers\ndiff --git a/git.c b/git.c\nindex 60c2784be45..e5891e02021 100644\n--- a/git.c\n+++ b/git.c\n@@ -538,6 +538,7 @@ static struct cmd_struct commands[] = {\n \t{ \"grep\", cmd_grep, RUN_SETUP_GENTLY },\n \t{ \"hash-object\", cmd_hash_object },\n \t{ \"help\", cmd_help },\n+\t{ \"hook\", cmd_hook, RUN_SETUP },\n \t{ \"index-pack\", cmd_index_pack, RUN_SETUP_GENTLY | NO_PARSEOPT },\n \t{ \"init\", cmd_init_db },\n \t{ \"init-db\", cmd_init_db },\ndiff --git a/hook.c b/hook.c\nindex 55e1145a4b7..240270db64e 100644\n--- a/hook.c\n+++ b/hook.c\n@@ -1,6 +1,7 @@\n #include \"cache.h\"\n #include \"hook.h\"\n #include \"run-command.h\"\n+#include \"config.h\"\n \n const char *find_hook(const char *name)\n {\n@@ -40,3 +41,103 @@ int hook_exists(const char *name)\n {\n \treturn !!find_hook(name);\n }\n+\n+void run_hooks_opt_clear(struct run_hooks_opt *o)\n+{\n+\tstrvec_clear(&o->env);\n+\tstrvec_clear(&o->args);\n+}\n+\n+static int pick_next_hook(struct child_process *cp,\n+\t\t\t struct strbuf *out,\n+\t\t\t void *pp_cb,\n+\t\t\t void **pp_task_cb)\n+{\n+\tstruct hook_cb_data *hook_cb = pp_cb;\n+\tstruct hook *run_me = hook_cb->run_me;\n+\n+\tif (!run_me)\n+\t\treturn 0;\n+\n+\tcp->no_stdin = 1;\n+\tcp->env = hook_cb->options->env.v;\n+\tcp->stdout_to_stderr = 1;\n+\tcp->trace2_hook_name = hook_cb->hook_name;\n+\n+\t/* add command */\n+\tstrvec_push(&cp->args, run_me->hook_path);\n+\n+\t/*\n+\t * add passed-in argv, without expanding - let the user get back\n+\t * exactly what they put in\n+\t */\n+\tstrvec_pushv(&cp->args, hook_cb->options->args.v);\n+\n+\t/* Provide context for errors if necessary */\n+\t*pp_task_cb = run_me;\n+\n+\t/*\n+\t * This pick_next_hook() will be called again, we're only\n+\t * running one hook, so indicate that no more work will be\n+\t * done.\n+\t */\n+\thook_cb->run_me = NULL;\n+\n+\treturn 1;\n+}\n+\n+static int notify_start_failure(struct strbuf *out,\n+\t\t\t\tvoid *pp_cb,\n+\t\t\t\tvoid *pp_task_cp)\n+{\n+\tstruct hook_cb_data *hook_cb = pp_cb;\n+\tstruct hook *attempted = pp_task_cp;\n+\n+\thook_cb->rc |= 1;\n+\n+\tstrbuf_addf(out, _(\"Couldn't start hook '%s'\\n\"),\n+\t\t attempted->hook_path);\n+\n+\treturn 1;\n+}\n+\n+static int notify_hook_finished(int result,\n+\t\t\t\tstruct strbuf *out,\n+\t\t\t\tvoid *pp_cb,\n+\t\t\t\tvoid *pp_task_cb)\n+{\n+\tstruct hook_cb_data *hook_cb = pp_cb;\n+\n+\thook_cb->rc |= result;\n+\n+\treturn 0;\n+}\n+\n+int run_hooks(const char *hook_name, const char *hook_path,\n+\t struct run_hooks_opt *options)\n+{\n+\tstruct hook my_hook = {\n+\t\t.hook_path = hook_path,\n+\t};\n+\tstruct hook_cb_data cb_data = {\n+\t\t.rc = 0,\n+\t\t.hook_name = hook_name,\n+\t\t.options = options,\n+\t};\n+\tint jobs = 1;\n+\n+\tif (!options)\n+\t\tBUG(\"a struct run_hooks_opt must be provided to run_hooks\");\n+\n+\tcb_data.run_me = &my_hook;\n+\n+\trun_processes_parallel_tr2(jobs,\n+\t\t\t\t pick_next_hook,\n+\t\t\t\t notify_start_failure,\n+\t\t\t\t notify_hook_finished,\n+\t\t\t\t &cb_data,\n+\t\t\t\t \"hook\",\n+\t\t\t\t hook_name);\n+\n+\treturn cb_data.rc;\n+}\ndiff --git a/hook.h b/hook.h\nindex 6aa36fc7ff9..111c5cf9296 100644\n--- a/hook.h\n+++ b/hook.h\n@@ -1,5 +1,33 @@\n #ifndef HOOK_H\n #define HOOK_H\n+#include \"strvec.h\"\n+\n+struct hook {\n+\t/* The path to the hook */\n+\tconst char *hook_path;\n+};\n+\n+struct run_hooks_opt\n+{\n+\t/* Environment vars to be set for each hook */\n+\tstruct strvec env;\n+\n+\t/* Args to be passed to each hook */\n+\tstruct strvec args;\n+};\n+\n+#define RUN_HOOKS_OPT_INIT { \\\n+\t.env = STRVEC_INIT, \\\n+\t.args = STRVEC_INIT, \\\n+}\n+\n+struct hook_cb_data {\n+\t/* rc reflects the cumulative failure state */\n+\tint rc;\n+\tconst char *hook_name;\n+\tstruct hook *run_me;\n+\tstruct run_hooks_opt *options;\n+};\n \n /*\n * Returns the path to the hook file, or NULL if the hook is missing\n@@ -13,4 +41,15 @@ const char *find_hook(const char *name);\n */\n int hook_exists(const char *hookname);\n \n+/**\n+ * Clear data from an initialized \"struct run_hooks-opt\".\n+ */\n+void run_hooks_opt_clear(struct run_hooks_opt *o);\n+\n+/**\n+ * Takes an already resolved hook found via find_hook() and runs\n+ * it. Does not call run_hooks_opt_clear() for you.\n+ */\n+int run_hooks(const char *hookname, const char *hook_path,\n+\t struct run_hooks_opt *options);\n #endif\ndiff --git a/t/t1800-hook.sh b/t/t1800-hook.sh\nnew file mode 100755\nindex 00000000000..3aea1b105f0\n--- /dev/null\n+++ b/t/t1800-hook.sh\n@@ -0,0 +1,129 @@\n+#!/bin/sh\n+\n+test_description='git-hook command'\n+\n+TEST_PASSES_SANITIZE_LEAK=true\n+. ./test-lib.sh\n+\n+test_expect_success 'git hook usage' '\n+\ttest_expect_code 129 git hook &&\n+\ttest_expect_code 129 git hook run &&\n+\ttest_expect_code 129 git hook run -h &&\n+\ttest_expect_code 129 git hook run --unknown 2>err &&\n+\tgrep \"unknown option\" err\n+'\n+\n+test_expect_success 'git hook run: nonexistent hook' '\n+\tcat >stderr.expect <<-\\EOF &&\n+\terror: cannot find a hook named test-hook\n+\tEOF\n+\ttest_expect_code 1 git hook run test-hook 2>stderr.actual &&\n+\ttest_cmp stderr.expect stderr.actual\n+'\n+\n+test_expect_success 'git hook run: basic' '\n+\twrite_script .git/hooks/test-hook <<-EOF &&\n+\techo Test hook\n+\tEOF\n+\n+\tcat >expect <<-\\EOF &&\n+\tTest hook\n+\tEOF\n+\tgit hook run test-hook 2>actual &&\n+\ttest_cmp expect actual\n+'\n+\n+test_expect_success 'git hook run: stdout and stderr both write to our stderr' '\n+\twrite_script .git/hooks/test-hook <<-EOF &&\n+\techo >&1 Will end up on stderr\n+\techo >&2 Will end up on stderr\n+\tEOF\n+\n+\tcat >stderr.expect <<-\\EOF &&\n+\tWill end up on stderr\n+\tWill end up on stderr\n+\tEOF\n+\tgit hook run test-hook >stdout.actual 2>stderr.actual &&\n+\ttest_cmp stderr.expect stderr.actual &&\n+\ttest_must_be_empty stdout.actual\n+'\n+\n+test_expect_success 'git hook run: exit codes are passed along' '\n+\twrite_script .git/hooks/test-hook <<-EOF &&\n+\texit 1\n+\tEOF\n+\n+\ttest_expect_code 1 git hook run test-hook &&\n+\n+\twrite_script .git/hooks/test-hook <<-EOF &&\n+\texit 2\n+\tEOF\n+\n+\ttest_expect_code 2 git hook run test-hook &&\n+\n+\twrite_script .git/hooks/test-hook <<-EOF &&\n+\texit 128\n+\tEOF\n+\n+\ttest_expect_code 128 git hook run test-hook &&\n+\n+\twrite_script .git/hooks/test-hook <<-EOF &&\n+\texit 129\n+\tEOF\n+\n+\ttest_expect_code 129 git hook run test-hook\n+'\n+\n+test_expect_success 'git hook run arg u ments without -- is not allowed' '\n+\ttest_expect_code 129 git hook run test-hook arg u ments\n+'\n+\n+test_expect_success 'git hook run -- pass arguments' '\n+\twrite_script .git/hooks/test-hook <<-\\EOF &&\n+\techo $1\n+\techo $2\n+\tEOF\n+\n+\tcat >expect <<-EOF &&\n+\targ\n+\tu ments\n+\tEOF\n+\n+\tgit hook run test-hook -- arg \"u ments\" 2>actual &&\n+\ttest_cmp expect actual\n+'\n+\n+test_expect_success 'git hook run -- out-of-repo runs excluded' '\n+\twrite_script .git/hooks/test-hook <<-EOF &&\n+\techo Test hook\n+\tEOF\n+\n+\tnongit test_must_fail git hook run test-hook\n+'\n+\n+test_expect_success 'git -c core.hooksPath= hook run' '\n+\tmkdir my-hooks &&\n+\twrite_script my-hooks/test-hook <<-\\EOF &&\n+\techo Hook ran $1 >>actual\n+\tEOF\n+\n+\tcat >expect <<-\\EOF &&\n+\tTest hook\n+\tHook ran one\n+\tHook ran two\n+\tHook ran three\n+\tHook ran four\n+\tEOF\n+\n+\t# Test various ways of specifying the path. See also\n+\t# t1350-config-hooks-path.sh\n+\t>actual &&\n+\tgit hook run test-hook -- ignored 2>>actual &&\n+\tgit -c core.hooksPath=my-hooks hook run test-hook -- one 2>>actual &&\n+\tgit -c core.hooksPath=my-hooks/ hook run test-hook -- two 2>>actual &&\n+\tgit -c core.hooksPath=\"$PWD/my-hooks\" hook run test-hook -- three 2>>actual &&\n+\tgit -c core.hooksPath=\"$PWD/my-hooks/\" hook run test-hook -- four 2>>actual &&\n+\ttest_cmp expect actual\n+'\n+\n+test_done\n-- \n2.33.0.1567.g7b23ce7ed9e\n\n"},{"id":"438538","messageId":"patch-02.13-dbac4204f7b-20211012T131934Z-avarab@gmail.com","threadId":"56692","inReplyTo":"cover-00.13-00000000000-20211012T131934Z-avarab@gmail.com","subject":"[PATCH 02/13] gc: use hook library for pre-auto-gc hook","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2021-10-12T13:30:27Z","receivedAt":"2021-10-12T13:31:06Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"From: Emily Shaffer \n\nMove the pre-auto-gc hook away from run-command.h to and over to the\nnew hook.h library.\n\nTo do this introduce a simple run_hooks_oneshot() wrapper, we'll be\nusing it extensively for these simple cases of wanting to run a single\nhook under a given name, and having it free the memory we allocate for\nus.\n\nSigned-off-by: Emily Shaffer \nSigned-off-by: Ævar Arnfjörð Bjarmason \n---\n builtin/gc.c | 3 ++-\n hook.c | 20 ++++++++++++++++++++\n hook.h | 13 +++++++++++++\n 3 files changed, 35 insertions(+), 1 deletion(-)\n\ndiff --git a/builtin/gc.c b/builtin/gc.c\nindex 6b3de3dd514..95939e77f53 100644\n--- a/builtin/gc.c\n+++ b/builtin/gc.c\n@@ -32,6 +32,7 @@\n #include \"remote.h\"\n #include \"object-store.h\"\n #include \"exec-cmd.h\"\n+#include \"hook.h\"\n \n #define FAILED_RUN \"failed to run %s\"\n \n@@ -394,7 +395,7 @@ static int need_to_gc(void)\n \telse\n \t\treturn 0;\n \n-\tif (run_hook_le(NULL, \"pre-auto-gc\", NULL))\n+\tif (run_hooks_oneshot(\"pre-auto-gc\", NULL))\n \t\treturn 0;\n \treturn 1;\n }\ndiff --git a/hook.c b/hook.c\nindex 240270db64e..bfdc79e2f1a 100644\n--- a/hook.c\n+++ b/hook.c\n@@ -141,3 +141,23 @@ int run_hooks(const char *hook_name, const char *hook_path,\n \n \treturn cb_data.rc;\n }\n+\n+int run_hooks_oneshot(const char *hook_name, struct run_hooks_opt *options)\n+{\n+\tconst char *hook_path;\n+\tstruct run_hooks_opt hook_opt_scratch = RUN_HOOKS_OPT_INIT;\n+\tint ret = 0;\n+\n+\tif (!options)\n+\t\toptions = &hook_opt_scratch;\n+\n+\thook_path = find_hook(hook_name);\n+\tif (!hook_path)\n+\t\tgoto cleanup;\n+\n+\tret = run_hooks(hook_name, hook_path, options);\n+cleanup:\n+\trun_hooks_opt_clear(options);\n+\n+\treturn ret;\n+}\ndiff --git a/hook.h b/hook.h\nindex 111c5cf9296..a2b8d4fc6bd 100644\n--- a/hook.h\n+++ b/hook.h\n@@ -49,7 +49,20 @@ void run_hooks_opt_clear(struct run_hooks_opt *o);\n /**\n * Takes an already resolved hook found via find_hook() and runs\n * it. Does not call run_hooks_opt_clear() for you.\n+ *\n+ * See run_hooks_oneshot() for the simpler one-shot API.\n */\n int run_hooks(const char *hookname, const char *hook_path,\n \t struct run_hooks_opt *options);\n+\n+/**\n+ * Calls find_hook() on your \"hook_name\" and runs the hooks (if any)\n+ * with run_hooks().\n+ *\n+ * If \"options\" is provided calls run_hooks_opt_clear() on it for\n+ * you. If \"options\" is NULL the default options from\n+ * RUN_HOOKS_OPT_INIT will be used.\n+ */\n+int run_hooks_oneshot(const char *hook_name, struct run_hooks_opt *options);\n+\n #endif\n-- \n2.33.0.1567.g7b23ce7ed9e\n\n"},{"id":"438539","messageId":"patch-03.13-ff306debcb8-20211012T131934Z-avarab@gmail.com","threadId":"56692","inReplyTo":"cover-00.13-00000000000-20211012T131934Z-avarab@gmail.com","subject":"[PATCH 03/13] rebase: convert pre-rebase to use hook.h","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2021-10-12T13:30:28Z","receivedAt":"2021-10-12T13:31:07Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"From: Emily Shaffer \n\nMove the pre-rebase hook away from run-command.h to and over to the\nnew hook.h library.\n\nSigned-off-by: Emily Shaffer \nSigned-off-by: Ævar Arnfjörð Bjarmason \n---\n builtin/rebase.c | 8 ++++++--\n 1 file changed, 6 insertions(+), 2 deletions(-)\n\ndiff --git a/builtin/rebase.c b/builtin/rebase.c\nindex 8c6393f6d78..9a44939d662 100644\n--- a/builtin/rebase.c\n+++ b/builtin/rebase.c\n@@ -28,6 +28,7 @@\n #include \"sequencer.h\"\n #include \"rebase-interactive.h\"\n #include \"reset.h\"\n+#include \"hook.h\"\n \n #define DEFAULT_REFLOG_ACTION \"rebase\"\n \n@@ -1305,6 +1306,7 @@ int cmd_rebase(int argc, const char **argv, const char *prefix)\n \tchar *squash_onto_name = NULL;\n \tint reschedule_failed_exec = -1;\n \tint allow_preemptive_ff = 1;\n+\tstruct run_hooks_opt hook_opt = RUN_HOOKS_OPT_INIT;\n \tstruct option builtin_rebase_options[] = {\n \t\tOPT_STRING(0, \"onto\", &options.onto_name,\n \t\t\t N_(\"revision\"),\n@@ -2021,9 +2023,11 @@ int cmd_rebase(int argc, const char **argv, const char *prefix)\n \t}\n \n \t/* If a hook exists, give it a chance to interrupt*/\n+\tstrvec_push(&hook_opt.args, options.upstream_arg);\n+\tif (argc)\n+\t\tstrvec_push(&hook_opt.args, argv[0]);\n \tif (!ok_to_skip_pre_rebase &&\n-\t run_hook_le(NULL, \"pre-rebase\", options.upstream_arg,\n-\t\t\targc ? argv[0] : NULL, NULL))\n+\t run_hooks_oneshot(\"pre-rebase\", &hook_opt))\n \t\tdie(_(\"The pre-rebase hook refused to rebase.\"));\n \n \tif (options.flags & REBASE_DIFFSTAT) {\n-- \n2.33.0.1567.g7b23ce7ed9e\n\n"},{"id":"438540","messageId":"patch-04.13-b1d529ca485-20211012T131934Z-avarab@gmail.com","threadId":"56692","inReplyTo":"cover-00.13-00000000000-20211012T131934Z-avarab@gmail.com","subject":"[PATCH 04/13] am: convert applypatch to use hook.h","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2021-10-12T13:30:29Z","receivedAt":"2021-10-12T13:31:08Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"From: Emily Shaffer \n\nTeach pre-applypatch, post-applypatch, and applypatch-msg to use the\nhook.h library instead of the run-command.h library.\n\nSigned-off-by: Emily Shaffer \nSigned-off-by: Ævar Arnfjörð Bjarmason \n---\n builtin/am.c | 8 +++++---\n 1 file changed, 5 insertions(+), 3 deletions(-)\n\ndiff --git a/builtin/am.c b/builtin/am.c\nindex 3527945a467..2cfe6451b6f 100644\n--- a/builtin/am.c\n+++ b/builtin/am.c\n@@ -446,9 +446,11 @@ static void am_destroy(const struct am_state *state)\n static int run_applypatch_msg_hook(struct am_state *state)\n {\n \tint ret;\n+\tstruct run_hooks_opt opt = RUN_HOOKS_OPT_INIT;\n \n \tassert(state->msg);\n-\tret = run_hook_le(NULL, \"applypatch-msg\", am_path(state, \"final-commit\"), NULL);\n+\tstrvec_push(&opt.args, am_path(state, \"final-commit\"));\n+\tret = run_hooks_oneshot(\"applypatch-msg\", &opt);\n \n \tif (!ret) {\n \t\tFREE_AND_NULL(state->msg);\n@@ -1609,7 +1611,7 @@ static void do_commit(const struct am_state *state)\n \tconst char *reflog_msg, *author, *committer = NULL;\n \tstruct strbuf sb = STRBUF_INIT;\n \n-\tif (run_hook_le(NULL, \"pre-applypatch\", NULL))\n+\tif (run_hooks_oneshot(\"pre-applypatch\", NULL))\n \t\texit(1);\n \n \tif (write_cache_as_tree(&tree, 0, NULL))\n@@ -1661,7 +1663,7 @@ static void do_commit(const struct am_state *state)\n \t\tfclose(fp);\n \t}\n \n-\trun_hook_le(NULL, \"post-applypatch\", NULL);\n+\trun_hooks_oneshot(\"post-applypatch\", NULL);\n \n \tstrbuf_release(&sb);\n }\n-- \n2.33.0.1567.g7b23ce7ed9e\n\n"},{"id":"438541","messageId":"patch-05.13-15d71fc210b-20211012T131934Z-avarab@gmail.com","threadId":"56692","inReplyTo":"cover-00.13-00000000000-20211012T131934Z-avarab@gmail.com","subject":"[PATCH 05/13] hooks: convert 'post-checkout' hook to hook library","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2021-10-12T13:30:30Z","receivedAt":"2021-10-12T13:31:10Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"From: Emily Shaffer \n\nMove the running of the 'post-checkout' hook away from run-command.h\nto the new hook.h library. For \"worktree\" this requires a change to it\nto run the hooks from a given directory.\n\nWe could strictly speaking skip the \"absolute_path\" flag and just\ncheck if \"dir\" is specified, but let's split them up for clarity, as\nwell as for any future user who'd like to set \"dir\" but not implicitly\nchange the argument to an absolute path.\n\nSigned-off-by: Emily Shaffer \nSigned-off-by: Ævar Arnfjörð Bjarmason \n---\n builtin/checkout.c | 14 +++++++++-----\n builtin/clone.c | 7 +++++--\n builtin/worktree.c | 28 ++++++++++++----------------\n hook.c | 9 +++++++++\n hook.h | 9 +++++++++\n read-cache.c | 1 +\n reset.c | 14 ++++++++++----\n 7 files changed, 55 insertions(+), 27 deletions(-)\n\ndiff --git a/builtin/checkout.c b/builtin/checkout.c\nindex 8c69dcdf72a..555119cc322 100644\n--- a/builtin/checkout.c\n+++ b/builtin/checkout.c\n@@ -9,6 +9,7 @@\n #include \"config.h\"\n #include \"diff.h\"\n #include \"dir.h\"\n+#include \"hook.h\"\n #include \"ll-merge.h\"\n #include \"lockfile.h\"\n #include \"merge-recursive.h\"\n@@ -106,13 +107,16 @@ struct branch_info {\n static int post_checkout_hook(struct commit *old_commit, struct commit *new_commit,\n \t\t\t int changed)\n {\n-\treturn run_hook_le(NULL, \"post-checkout\",\n-\t\t\t oid_to_hex(old_commit ? &old_commit->object.oid : null_oid()),\n-\t\t\t oid_to_hex(new_commit ? &new_commit->object.oid : null_oid()),\n-\t\t\t changed ? \"1\" : \"0\", NULL);\n+\tstruct run_hooks_opt opt = RUN_HOOKS_OPT_INIT;\n+\n \t/* \"new_commit\" can be NULL when checking out from the index before\n \t a commit exists. */\n-\n+\tstrvec_pushl(&opt.args,\n+\t\t oid_to_hex(old_commit ? &old_commit->object.oid : null_oid()),\n+\t\t oid_to_hex(new_commit ? &new_commit->object.oid : null_oid()),\n+\t\t changed ? \"1\" : \"0\",\n+\t\t NULL);\n+\treturn run_hooks_oneshot(\"post-checkout\", &opt);\n }\n \n static int update_some(const struct object_id *oid, struct strbuf *base,\ndiff --git a/builtin/clone.c b/builtin/clone.c\nindex c9d4ca26640..4b356770739 100644\n--- a/builtin/clone.c\n+++ b/builtin/clone.c\n@@ -32,6 +32,7 @@\n #include \"connected.h\"\n #include \"packfile.h\"\n #include \"list-objects-filter-options.h\"\n+#include \"hook.h\"\n \n /*\n * Overall FIXMEs:\n@@ -659,6 +660,7 @@ static int checkout(int submodule_progress)\n \tstruct tree *tree;\n \tstruct tree_desc t;\n \tint err = 0;\n+\tstruct run_hooks_opt hook_opt = RUN_HOOKS_OPT_INIT;\n \n \tif (option_no_checkout)\n \t\treturn 0;\n@@ -704,8 +706,9 @@ static int checkout(int submodule_progress)\n \tif (write_locked_index(&the_index, &lock_file, COMMIT_LOCK))\n \t\tdie(_(\"unable to write new index file\"));\n \n-\terr |= run_hook_le(NULL, \"post-checkout\", oid_to_hex(null_oid()),\n-\t\t\t oid_to_hex(&oid), \"1\", NULL);\n+\tstrvec_pushl(&hook_opt.args, oid_to_hex(null_oid()), oid_to_hex(&oid),\n+\t\t \"1\", NULL);\n+\terr |= run_hooks_oneshot(\"post-checkout\", &hook_opt);\n \n \tif (!err && (option_recurse_submodules.nr > 0)) {\n \t\tstruct strvec args = STRVEC_INIT;\ndiff --git a/builtin/worktree.c b/builtin/worktree.c\nindex d22ece93e1a..330867c19bf 100644\n--- a/builtin/worktree.c\n+++ b/builtin/worktree.c\n@@ -382,22 +382,18 @@ static int add_worktree(const char *path, const char *refname,\n \t * is_junk is cleared, but do return appropriate code when hook fails.\n \t */\n \tif (!ret && opts->checkout) {\n-\t\tconst char *hook = find_hook(\"post-checkout\");\n-\t\tif (hook) {\n-\t\t\tconst char *env[] = { \"GIT_DIR\", \"GIT_WORK_TREE\", NULL };\n-\t\t\tcp.git_cmd = 0;\n-\t\t\tcp.no_stdin = 1;\n-\t\t\tcp.stdout_to_stderr = 1;\n-\t\t\tcp.dir = path;\n-\t\t\tcp.env = env;\n-\t\t\tcp.argv = NULL;\n-\t\t\tcp.trace2_hook_name = \"post-checkout\";\n-\t\t\tstrvec_pushl(&cp.args, absolute_path(hook),\n-\t\t\t\t oid_to_hex(null_oid()),\n-\t\t\t\t oid_to_hex(&commit->object.oid),\n-\t\t\t\t \"1\", NULL);\n-\t\t\tret = run_command(&cp);\n-\t\t}\n+\t\tstruct run_hooks_opt opt = RUN_HOOKS_OPT_INIT;\n+\n+\t\tstrvec_pushl(&opt.env, \"GIT_DIR\", \"GIT_WORK_TREE\", NULL);\n+\t\tstrvec_pushl(&opt.args,\n+\t\t\t oid_to_hex(null_oid()),\n+\t\t\t oid_to_hex(&commit->object.oid),\n+\t\t\t \"1\",\n+\t\t\t NULL);\n+\t\topt.dir = path;\n+\t\topt.absolute_path = 1;\n+\n+\t\tret = run_hooks_oneshot(\"post-checkout\", &opt);\n \t}\n \n \tstrvec_clear(&child_env);\ndiff --git a/hook.c b/hook.c\nindex bfdc79e2f1a..186c249bbec 100644\n--- a/hook.c\n+++ b/hook.c\n@@ -63,6 +63,7 @@ static int pick_next_hook(struct child_process *cp,\n \tcp->env = hook_cb->options->env.v;\n \tcp->stdout_to_stderr = 1;\n \tcp->trace2_hook_name = hook_cb->hook_name;\n+\tcp->dir = hook_cb->options->dir;\n \n \t/* add command */\n \tstrvec_push(&cp->args, run_me->hook_path);\n@@ -116,6 +117,7 @@ static int notify_hook_finished(int result,\n int run_hooks(const char *hook_name, const char *hook_path,\n \t struct run_hooks_opt *options)\n {\n+\tstruct strbuf abs_path = STRBUF_INIT;\n \tstruct hook my_hook = {\n \t\t.hook_path = hook_path,\n \t};\n@@ -129,6 +131,10 @@ int run_hooks(const char *hook_name, const char *hook_path,\n \tif (!options)\n \t\tBUG(\"a struct run_hooks_opt must be provided to run_hooks\");\n \n+\tif (options->absolute_path) {\n+\t\tstrbuf_add_absolute_path(&abs_path, hook_path);\n+\t\tmy_hook.hook_path = abs_path.buf;\n+\t}\n \tcb_data.run_me = &my_hook;\n \n \trun_processes_parallel_tr2(jobs,\n@@ -139,6 +145,9 @@ int run_hooks(const char *hook_name, const char *hook_path,\n \t\t\t\t \"hook\",\n \t\t\t\t hook_name);\n \n+\tif (options->absolute_path)\n+\t\tstrbuf_release(&abs_path);\n+\n \treturn cb_data.rc;\n }\n \ndiff --git a/hook.h b/hook.h\nindex a2b8d4fc6bd..7a858a6beed 100644\n--- a/hook.h\n+++ b/hook.h\n@@ -14,6 +14,15 @@ struct run_hooks_opt\n \n \t/* Args to be passed to each hook */\n \tstruct strvec args;\n+\n+\t/*\n+\t * Resolve and run the \"absolute_path(hook)\" instead of\n+\t * \"hook\". Used for \"git worktree\" hooks\n+\t */\n+\tint absolute_path;\n+\n+\t/* Path to initial working directory for subprocess */\n+\tconst char *dir;\n };\n \n #define RUN_HOOKS_OPT_INIT { \\\ndiff --git a/read-cache.c b/read-cache.c\nindex a78b88a41bf..9773118d078 100644\n--- a/read-cache.c\n+++ b/read-cache.c\n@@ -28,6 +28,7 @@\n #include \"sparse-index.h\"\n #include \"csum-file.h\"\n #include \"promisor-remote.h\"\n+#include \"hook.h\"\n \n /* Mask for the name length in ce_flags in the on-disk index */\n \ndiff --git a/reset.c b/reset.c\nindex 79310ae071b..1237ced8a58 100644\n--- a/reset.c\n+++ b/reset.c\n@@ -7,6 +7,7 @@\n #include \"tree-walk.h\"\n #include \"tree.h\"\n #include \"unpack-trees.h\"\n+#include \"hook.h\"\n \n int reset_head(struct repository *r, struct object_id *oid, const char *action,\n \t const char *switch_to_branch, unsigned flags,\n@@ -125,10 +126,15 @@ int reset_head(struct repository *r, struct object_id *oid, const char *action,\n \t\t\tret = create_symref(\"HEAD\", switch_to_branch,\n \t\t\t\t\t reflog_head);\n \t}\n-\tif (run_hook)\n-\t\trun_hook_le(NULL, \"post-checkout\",\n-\t\t\t oid_to_hex(orig ? orig : null_oid()),\n-\t\t\t oid_to_hex(oid), \"1\", NULL);\n+\tif (run_hook) {\n+\t\tstruct run_hooks_opt opt = RUN_HOOKS_OPT_INIT;\n+\t\tstrvec_pushl(&opt.args,\n+\t\t\t oid_to_hex(orig ? orig : null_oid()),\n+\t\t\t oid_to_hex(oid),\n+\t\t\t \"1\",\n+\t\t\t NULL);\n+\t\trun_hooks_oneshot(\"post-checkout\", &opt);\n+\t}\n \n leave_reset_head:\n \tstrbuf_release(&msg);\n-- \n2.33.0.1567.g7b23ce7ed9e\n\n"},{"id":"438542","messageId":"patch-06.13-08f27f0d6be-20211012T131934Z-avarab@gmail.com","threadId":"56692","inReplyTo":"cover-00.13-00000000000-20211012T131934Z-avarab@gmail.com","subject":"[PATCH 06/13] merge: convert post-merge to use hook.h","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2021-10-12T13:30:31Z","receivedAt":"2021-10-12T13:31:20Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"From: Emily Shaffer \n\nTeach post-merge to use the hook.h library instead of the\nrun-command.h library to run hooks.\n\nSigned-off-by: Emily Shaffer \nSigned-off-by: Ævar Arnfjörð Bjarmason \n---\n builtin/merge.c | 4 +++-\n 1 file changed, 3 insertions(+), 1 deletion(-)\n\ndiff --git a/builtin/merge.c b/builtin/merge.c\nindex 956b6259f21..167d13bd737 100644\n--- a/builtin/merge.c\n+++ b/builtin/merge.c\n@@ -448,6 +448,7 @@ static void finish(struct commit *head_commit,\n \t\t const struct object_id *new_head, const char *msg)\n {\n \tstruct strbuf reflog_message = STRBUF_INIT;\n+\tstruct run_hooks_opt opt = RUN_HOOKS_OPT_INIT;\n \tconst struct object_id *head = &head_commit->object.oid;\n \n \tif (!msg)\n@@ -488,7 +489,8 @@ static void finish(struct commit *head_commit,\n \t}\n \n \t/* Run a post-merge hook */\n-\trun_hook_le(NULL, \"post-merge\", squash ? \"1\" : \"0\", NULL);\n+\tstrvec_push(&opt.args, squash ? \"1\" : \"0\");\n+\trun_hooks_oneshot(\"post-merge\", &opt);\n \n \tapply_autostash(git_path_merge_autostash(the_repository));\n \tstrbuf_release(&reflog_message);\n-- \n2.33.0.1567.g7b23ce7ed9e\n\n"},{"id":"438543","messageId":"patch-09.13-69cc447a1e1-20211012T131934Z-avarab@gmail.com","threadId":"56692","inReplyTo":"cover-00.13-00000000000-20211012T131934Z-avarab@gmail.com","subject":"[PATCH 09/13] git-p4: use 'git hook' to run hooks","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2021-10-12T13:30:34Z","receivedAt":"2021-10-12T13:31:22Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"From: Emily Shaffer \n\nInstead of duplicating the behavior of run-command.h:run_hook_le() in\nPython, we can directly call 'git hook run'. We emulate the existence\ncheck with the --ignore-missing flag.\n\nAs this is the last hook execution in git.git to not go through \"git\nhook run\" or the hook.[ch] library we can now be absolutely sure that\nour assertion in hook.c that only hooks known by the generated (from\ngithooks(5)) hook-list.h are permitted.\n\nSigned-off-by: Emily Shaffer \nSigned-off-by: Ævar Arnfjörð Bjarmason \n---\n git-p4.py | 72 ++++++-------------------------------------------------\n 1 file changed, 7 insertions(+), 65 deletions(-)\n\ndiff --git a/git-p4.py b/git-p4.py\nindex 2b4500226aa..1f24cbf0bca 100755\n--- a/git-p4.py\n+++ b/git-p4.py\n@@ -207,71 +207,13 @@ def decode_path(path):\n return path\n \n def run_git_hook(cmd, param=[]):\n- \"\"\"Execute a hook if the hook exists.\"\"\"\n- if verbose:\n- sys.stderr.write(\"Looking for hook: %s\\n\" % cmd)\n- sys.stderr.flush()\n-\n- hooks_path = gitConfig(\"core.hooksPath\")\n- if len(hooks_path) <= 0:\n- hooks_path = os.path.join(os.environ[\"GIT_DIR\"], \"hooks\")\n-\n- if not isinstance(param, list):\n- param=[param]\n-\n- # resolve hook file name, OS depdenent\n- hook_file = os.path.join(hooks_path, cmd)\n- if platform.system() == 'Windows':\n- if not os.path.isfile(hook_file):\n- # look for the file with an extension\n- files = glob.glob(hook_file + \".*\")\n- if not files:\n- return True\n- files.sort()\n- hook_file = files.pop()\n- while hook_file.upper().endswith(\".SAMPLE\"):\n- # The file is a sample hook. We don't want it\n- if len(files) > 0:\n- hook_file = files.pop()\n- else:\n- return True\n-\n- if not os.path.isfile(hook_file) or not os.access(hook_file, os.X_OK):\n- return True\n-\n- return run_hook_command(hook_file, param) == 0\n-\n-def run_hook_command(cmd, param):\n- \"\"\"Executes a git hook command\n- cmd = the command line file to be executed. This can be\n- a file that is run by OS association.\n-\n- param = a list of parameters to pass to the cmd command\n-\n- On windows, the extension is checked to see if it should\n- be run with the Git for Windows Bash shell. If there\n- is no file extension, the file is deemed a bash shell\n- and will be handed off to sh.exe. Otherwise, Windows\n- will be called with the shell to handle the file assocation.\n-\n- For non Windows operating systems, the file is called\n- as an executable.\n- \"\"\"\n- cli = [cmd] + param\n- use_shell = False\n- if platform.system() == 'Windows':\n- (root,ext) = os.path.splitext(cmd)\n- if ext == \"\":\n- exe_path = os.environ.get(\"EXEPATH\")\n- if exe_path is None:\n- exe_path = \"\"\n- else:\n- exe_path = os.path.join(exe_path, \"bin\")\n- cli = [os.path.join(exe_path, \"SH.EXE\")] + cli\n- else:\n- use_shell = True\n- return subprocess.call(cli, shell=use_shell)\n-\n+ \"\"\"args are specified with -a -a -a \"\"\"\n+ args = ['git', 'hook', 'run', '--ignore-missing', cmd]\n+ if param:\n+ args.append(\"--\")\n+ for p in param:\n+ args.append(p)\n+ return subprocess.call(args) == 0\n \n def write_pipe(c, stdin):\n if verbose:\n-- \n2.33.0.1567.g7b23ce7ed9e\n\n"},{"id":"438544","messageId":"patch-07.13-107c14d740f-20211012T131934Z-avarab@gmail.com","threadId":"56692","inReplyTo":"cover-00.13-00000000000-20211012T131934Z-avarab@gmail.com","subject":"[PATCH 07/13] git hook run: add an --ignore-missing flag","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2021-10-12T13:30:32Z","receivedAt":"2021-10-12T13:31:23Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"For certain one-shot hooks we'd like to optimistically run them, and\nnot complain if they don't exist. This will be used by send-email in a\nsubsequent commit.\n\nSigned-off-by: Ævar Arnfjörð Bjarmason \n---\n Documentation/git-hook.txt | 10 +++++++++-\n builtin/hook.c | 10 ++++++++--\n t/t1800-hook.sh | 5 +++++\n 3 files changed, 22 insertions(+), 3 deletions(-)\n\ndiff --git a/Documentation/git-hook.txt b/Documentation/git-hook.txt\nindex 660d6a992a0..097fb9de63b 100644\n--- a/Documentation/git-hook.txt\n+++ b/Documentation/git-hook.txt\n@@ -8,7 +8,7 @@ git-hook - run git hooks\n SYNOPSIS\n --------\n [verse]\n-'git hook' run [-- ]\n+'git hook' run [--ignore-missing] [-- ]\n \n DESCRIPTION\n -----------\n@@ -29,6 +29,14 @@ optional `--` (or `--end-of-options`, see linkgit:gitcli[7]). The\n arguments (if any) differ by hook name, see linkgit:githooks[5] for\n what those are.\n \n+OPTIONS\n+-------\n+\n+--ignore-missing::\n+\tIgnore any missing hook by quietly returning zero. Used for\n+\ttools that want to do a blind one-shot run of a hook that may\n+\tor may not be present.\n+\n SEE ALSO\n --------\n linkgit:githooks[5]\ndiff --git a/builtin/hook.c b/builtin/hook.c\nindex 012a2973b38..e6b01a7f5c6 100644\n--- a/builtin/hook.c\n+++ b/builtin/hook.c\n@@ -7,7 +7,7 @@\n #include \"strvec.h\"\n \n #define BUILTIN_HOOK_RUN_USAGE \\\n-\tN_(\"git hook run [-- ]\")\n+\tN_(\"git hook run [--ignore-missing] [-- ]\")\n \n static const char * const builtin_hook_usage[] = {\n \tBUILTIN_HOOK_RUN_USAGE,\n@@ -23,9 +23,12 @@ static int run(int argc, const char **argv, const char *prefix)\n {\n \tint i;\n \tstruct run_hooks_opt opt = RUN_HOOKS_OPT_INIT;\n+\tint ignore_missing = 0;\n \tconst char *hook_name;\n \tconst char *hook_path;\n \tstruct option run_options[] = {\n+\t\tOPT_BOOL(0, \"ignore-missing\", &ignore_missing,\n+\t\t\t N_(\"silently ignore missing requested \")),\n \t\tOPT_END(),\n \t};\n \tint ret;\n@@ -55,9 +58,12 @@ static int run(int argc, const char **argv, const char *prefix)\n \t/*\n \t * We are not using a plain run_hooks() because we'd like to\n \t * detect missing hooks. Let's find it ourselves and call\n-\t * run_hooks() instead.\n+\t * run_hooks() instead...\n \t */\n \thook_name = argv[0];\n+\tif (ignore_missing)\n+\t\t/* ... act like a plain run_hooks() under --ignore-missing */\n+\t\treturn run_hooks_oneshot(hook_name, &opt);\n \thook_path = find_hook(hook_name);\n \tif (!hook_path) {\n \t\terror(\"cannot find a hook named %s\", hook_name);\ndiff --git a/t/t1800-hook.sh b/t/t1800-hook.sh\nindex 3aea1b105f0..29718aa9913 100755\n--- a/t/t1800-hook.sh\n+++ b/t/t1800-hook.sh\n@@ -21,6 +21,11 @@ test_expect_success 'git hook run: nonexistent hook' '\n \ttest_cmp stderr.expect stderr.actual\n '\n \n+test_expect_success 'git hook run: nonexistent hook with --ignore-missing' '\n+\tgit hook run --ignore-missing does-not-exist 2>stderr.actual &&\n+\ttest_must_be_empty stderr.actual\n+'\n+\n test_expect_success 'git hook run: basic' '\n \twrite_script .git/hooks/test-hook <<-EOF &&\n \techo Test hook\n-- \n2.33.0.1567.g7b23ce7ed9e\n\n"},{"id":"438545","messageId":"patch-10.13-1c22b2992cf-20211012T131934Z-avarab@gmail.com","threadId":"56692","inReplyTo":"cover-00.13-00000000000-20211012T131934Z-avarab@gmail.com","subject":"[PATCH 10/13] commit: convert {pre-commit,prepare-commit-msg} hook to hook.h","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2021-10-12T13:30:35Z","receivedAt":"2021-10-12T13:31:24Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"From: Emily Shaffer \n\nMove these hooks hook away from run-command.h to and over to the new\nhook.h library.\n\nSigned-off-by: Emily Shaffer \nSigned-off-by: Ævar Arnfjörð Bjarmason \n---\n commit.c | 15 ++++++++-------\n 1 file changed, 8 insertions(+), 7 deletions(-)\n\ndiff --git a/commit.c b/commit.c\nindex 551de4903c1..4e7cbd7585d 100644\n--- a/commit.c\n+++ b/commit.c\n@@ -21,6 +21,7 @@\n #include \"commit-reach.h\"\n #include \"run-command.h\"\n #include \"shallow.h\"\n+#include \"hook.h\"\n \n static struct commit_extra_header *read_commit_extra_header_lines(const char *buf, size_t len, const char **);\n \n@@ -1700,22 +1701,22 @@ size_t ignore_non_trailer(const char *buf, size_t len)\n int run_commit_hook(int editor_is_used, const char *index_file,\n \t\t const char *name, ...)\n {\n-\tstruct strvec hook_env = STRVEC_INIT;\n+\tstruct run_hooks_opt opt = RUN_HOOKS_OPT_INIT;\n \tva_list args;\n-\tint ret;\n+\tconst char *arg;\n \n-\tstrvec_pushf(&hook_env, \"GIT_INDEX_FILE=%s\", index_file);\n+\tstrvec_pushf(&opt.env, \"GIT_INDEX_FILE=%s\", index_file);\n \n \t/*\n \t * Let the hook know that no editor will be launched.\n \t */\n \tif (!editor_is_used)\n-\t\tstrvec_push(&hook_env, \"GIT_EDITOR=:\");\n+\t\tstrvec_push(&opt.env, \"GIT_EDITOR=:\");\n \n \tva_start(args, name);\n-\tret = run_hook_ve(hook_env.v, name, args);\n+\twhile ((arg = va_arg(args, const char *)))\n+\t\tstrvec_push(&opt.args, arg);\n \tva_end(args);\n-\tstrvec_clear(&hook_env);\n \n-\treturn ret;\n+\treturn run_hooks_oneshot(name, &opt);\n }\n-- \n2.33.0.1567.g7b23ce7ed9e\n\n"},{"id":"438546","messageId":"patch-12.13-d63b91196ae-20211012T131934Z-avarab@gmail.com","threadId":"56692","inReplyTo":"cover-00.13-00000000000-20211012T131934Z-avarab@gmail.com","subject":"[PATCH 12/13] receive-pack: convert push-to-checkout hook to hook.h","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2021-10-12T13:30:37Z","receivedAt":"2021-10-12T13:31:24Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"From: Emily Shaffer \n\nMove the push-to-checkout hook away from run-command.h to and over to\nthe new hook.h library.\n\nThis removes the last direct user of run_hook_le(), so we could remove\nthat function now, but let's leave that to a follow-up cleanup commit.\n\nSigned-off-by: Emily Shaffer \nSigned-off-by: Ævar Arnfjörð Bjarmason \n---\n builtin/receive-pack.c | 7 +++++--\n 1 file changed, 5 insertions(+), 2 deletions(-)\n\ndiff --git a/builtin/receive-pack.c b/builtin/receive-pack.c\nindex 25cc0c907e1..22e7a431728 100644\n--- a/builtin/receive-pack.c\n+++ b/builtin/receive-pack.c\n@@ -1435,9 +1435,12 @@ static const char *push_to_checkout(unsigned char *hash,\n \t\t\t\t struct strvec *env,\n \t\t\t\t const char *work_tree)\n {\n+\tstruct run_hooks_opt opt = RUN_HOOKS_OPT_INIT;\n+\n \tstrvec_pushf(env, \"GIT_WORK_TREE=%s\", absolute_path(work_tree));\n-\tif (run_hook_le(env->v, push_to_checkout_hook,\n-\t\t\thash_to_hex(hash), NULL))\n+\tstrvec_pushv(&opt.env, env->v);\n+\tstrvec_push(&opt.args, hash_to_hex(hash));\n+\tif (run_hooks_oneshot(push_to_checkout_hook, &opt))\n \t\treturn \"push-to-checkout hook declined\";\n \telse\n \t\treturn NULL;\n-- \n2.33.0.1567.g7b23ce7ed9e\n\n"},{"id":"438547","messageId":"patch-11.13-e762fce32af-20211012T131934Z-avarab@gmail.com","threadId":"56692","inReplyTo":"cover-00.13-00000000000-20211012T131934Z-avarab@gmail.com","subject":"[PATCH 11/13] read-cache: convert post-index-change to use hook.h","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2021-10-12T13:30:36Z","receivedAt":"2021-10-12T13:31:29Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"From: Emily Shaffer \n\nMove the post-index-change hook away from run-command.h to and over to\nthe new hook.h library.\n\nThis removes the last direct user of \"run_hook_ve()\" outside of\nrun-command.c (\"run_hook_le()\" still uses it). So we can make the\nfunction static now. A subsequent commit will remove this code\nentirely when \"run_hook_le()\" itself goes away.\n\nSigned-off-by: Emily Shaffer \nSigned-off-by: Ævar Arnfjörð Bjarmason \n---\n read-cache.c | 10 +++++++---\n run-command.c | 2 +-\n run-command.h | 1 -\n 3 files changed, 8 insertions(+), 5 deletions(-)\n\ndiff --git a/read-cache.c b/read-cache.c\nindex 9773118d078..84cc3d88196 100644\n--- a/read-cache.c\n+++ b/read-cache.c\n@@ -3087,6 +3087,7 @@ static int do_write_locked_index(struct index_state *istate, struct lock_file *l\n {\n \tint ret;\n \tint was_full = !istate->sparse_index;\n+\tstruct run_hooks_opt hook_opt = RUN_HOOKS_OPT_INIT;\n \n \tret = convert_to_sparse(istate, 0);\n \n@@ -3115,9 +3116,12 @@ static int do_write_locked_index(struct index_state *istate, struct lock_file *l\n \telse\n \t\tret = close_lock_file_gently(lock);\n \n-\trun_hook_le(NULL, \"post-index-change\",\n-\t\t\tistate->updated_workdir ? \"1\" : \"0\",\n-\t\t\tistate->updated_skipworktree ? \"1\" : \"0\", NULL);\n+\tstrvec_pushl(&hook_opt.args,\n+\t\t istate->updated_workdir ? \"1\" : \"0\",\n+\t\t istate->updated_skipworktree ? \"1\" : \"0\",\n+\t\t NULL);\n+\trun_hooks_oneshot(\"post-index-change\", &hook_opt);\n+\n \tistate->updated_workdir = 0;\n \tistate->updated_skipworktree = 0;\n \ndiff --git a/run-command.c b/run-command.c\nindex e4862b8f46d..7a05aa73e3d 100644\n--- a/run-command.c\n+++ b/run-command.c\n@@ -1323,7 +1323,7 @@ int async_with_fork(void)\n #endif\n }\n \n-int run_hook_ve(const char *const *env, const char *name, va_list args)\n+static int run_hook_ve(const char *const *env, const char *name, va_list args)\n {\n \tstruct child_process hook = CHILD_PROCESS_INIT;\n \tconst char *p;\ndiff --git a/run-command.h b/run-command.h\nindex 5e544acf4bc..f06b39c50ee 100644\n--- a/run-command.h\n+++ b/run-command.h\n@@ -239,7 +239,6 @@ int run_command(struct child_process *);\n */\n LAST_ARG_MUST_BE_NULL\n int run_hook_le(const char *const *env, const char *name, ...);\n-int run_hook_ve(const char *const *env, const char *name, va_list args);\n \n /*\n * Trigger an auto-gc\n-- \n2.33.0.1567.g7b23ce7ed9e\n\n"},{"id":"438548","messageId":"patch-13.13-fe8996dda3e-20211012T131934Z-avarab@gmail.com","threadId":"56692","inReplyTo":"cover-00.13-00000000000-20211012T131934Z-avarab@gmail.com","subject":"[PATCH 13/13] run-command: remove old run_hook_{le,ve}() hook API","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2021-10-12T13:30:38Z","receivedAt":"2021-10-12T13:31:29Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"From: Emily Shaffer \n\nThe new hook.h library has replaced all run-command.h hook-related\nfunctionality. So let's delete this dead code.\n\nSigned-off-by: Emily Shaffer \nSigned-off-by: Ævar Arnfjörð Bjarmason \n---\n run-command.c | 32 --------------------------------\n run-command.h | 16 ----------------\n 2 files changed, 48 deletions(-)\n\ndiff --git a/run-command.c b/run-command.c\nindex 7a05aa73e3d..e39ce17d009 100644\n--- a/run-command.c\n+++ b/run-command.c\n@@ -1323,38 +1323,6 @@ int async_with_fork(void)\n #endif\n }\n \n-static int run_hook_ve(const char *const *env, const char *name, va_list args)\n-{\n-\tstruct child_process hook = CHILD_PROCESS_INIT;\n-\tconst char *p;\n-\n-\tp = find_hook(name);\n-\tif (!p)\n-\t\treturn 0;\n-\n-\tstrvec_push(&hook.args, p);\n-\twhile ((p = va_arg(args, const char *)))\n-\t\tstrvec_push(&hook.args, p);\n-\thook.env = env;\n-\thook.no_stdin = 1;\n-\thook.stdout_to_stderr = 1;\n-\thook.trace2_hook_name = name;\n-\n-\treturn run_command(&hook);\n-}\n-\n-int run_hook_le(const char *const *env, const char *name, ...)\n-{\n-\tva_list args;\n-\tint ret;\n-\n-\tva_start(args, name);\n-\tret = run_hook_ve(env, name, args);\n-\tva_end(args);\n-\n-\treturn ret;\n-}\n-\n struct io_pump {\n \t/* initialized by caller */\n \tint fd;\ndiff --git a/run-command.h b/run-command.h\nindex f06b39c50ee..6d6c50fe0a9 100644\n--- a/run-command.h\n+++ b/run-command.h\n@@ -224,22 +224,6 @@ int finish_command_in_signal(struct child_process *);\n */\n int run_command(struct child_process *);\n \n-/**\n- * Run a hook.\n- * The first argument is a pathname to an index file, or NULL\n- * if the hook uses the default index file or no index is needed.\n- * The second argument is the name of the hook.\n- * The further arguments correspond to the hook arguments.\n- * The last argument has to be NULL to terminate the arguments list.\n- * If the hook does not exist or is not executable, the return\n- * value will be zero.\n- * If it is executable, the hook will be executed and the exit\n- * status of the hook is returned.\n- * On execution, .stdout_to_stderr and .no_stdin will be set.\n- */\n-LAST_ARG_MUST_BE_NULL\n-int run_hook_le(const char *const *env, const char *name, ...);\n-\n /*\n * Trigger an auto-gc\n */\n-- \n2.33.0.1567.g7b23ce7ed9e\n\n"},{"id":"438549","messageId":"patch-08.13-1d30e2dbbe0-20211012T131934Z-avarab@gmail.com","threadId":"56692","inReplyTo":"cover-00.13-00000000000-20211012T131934Z-avarab@gmail.com","subject":"[PATCH 08/13] send-email: use 'git hook run' for 'sendemail-validate'","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2021-10-12T13:30:33Z","receivedAt":"2021-10-12T13:33:12Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"From: Emily Shaffer \n\nChange the \"sendmail-validate\" hook to be run via the \"git hook run\"\nwrapper instead of via a direct invocation.\n\nThis is the smallest possibly change to get \"send-email\" using \"git\nhook run\". We still check the hook itself with \"-x\", and set a\n\"GIT_DIR\" variable, both of which are asserted by our tests. We'll\nneed to get rid of this special behavior if we start running N hooks,\nbut for now let's be as close to bug-for-bug compatible as possible.\n\nSigned-off-by: Emily Shaffer \nSigned-off-by: Ævar Arnfjörð Bjarmason \n---\n git-send-email.perl | 22 ++++++++++++++--------\n t/t9001-send-email.sh | 4 ++--\n 2 files changed, 16 insertions(+), 10 deletions(-)\n\ndiff --git a/git-send-email.perl b/git-send-email.perl\nindex 5262d88ee32..4c20c0bbb79 100755\n--- a/git-send-email.perl\n+++ b/git-send-email.perl\n@@ -195,13 +195,13 @@ sub format_2822_time {\n my $editor;\n \n sub system_or_msg {\n-\tmy ($args, $msg) = @_;\n+\tmy ($args, $msg, $cmd_name) = @_;\n \tsystem(@$args);\n \tmy $signalled = $? & 127;\n \tmy $exit_code = $? >> 8;\n \treturn unless $signalled or $exit_code;\n \n-\tmy @sprintf_args = ($args->[0], $exit_code);\n+\tmy @sprintf_args = ($cmd_name ? $cmd_name : $args->[0], $exit_code);\n \tif (defined $msg) {\n \t\t# Quiet the 'redundant' warning category, except we\n \t\t# need to support down to Perl 5.8, so we can't do a\n@@ -2039,10 +2039,10 @@ sub validate_patch {\n \tmy ($fn, $xfer_encoding) = @_;\n \n \tif ($repo) {\n+\t\tmy $hook_name = 'sendemail-validate';\n \t\tmy $hooks_path = $repo->command_oneline('rev-parse', '--git-path', 'hooks');\n \t\trequire File::Spec;\n-\t\tmy $validate_hook = File::Spec->catfile($hooks_path,\n-\t\t\t\t\t 'sendemail-validate');\n+\t\tmy $validate_hook = File::Spec->catfile($hooks_path, $hook_name);\n \t\tmy $hook_error;\n \t\tif (-x $validate_hook) {\n \t\t\trequire Cwd;\n@@ -2052,13 +2052,19 @@ sub validate_patch {\n \t\t\tchdir($repo->wc_path() or $repo->repo_path())\n \t\t\t\tor die(\"chdir: $!\");\n \t\t\tlocal $ENV{\"GIT_DIR\"} = $repo->repo_path();\n-\t\t\t$hook_error = system_or_msg([$validate_hook, $target]);\n+\t\t\tmy @cmd = (\"git\", \"hook\", \"run\", \"--ignore-missing\",\n+\t\t\t\t $hook_name, \"--\");\n+\t\t\tmy @cmd_msg = (@cmd, \"\");\n+\t\t\tmy @cmd_run = (@cmd, $target);\n+\t\t\t$hook_error = system_or_msg(\\@cmd_run, undef, \"@cmd_msg\");\n \t\t\tchdir($cwd_save) or die(\"chdir: $!\");\n \t\t}\n \t\tif ($hook_error) {\n-\t\t\tdie sprintf(__(\"fatal: %s: rejected by sendemail-validate hook\\n\" .\n-\t\t\t\t \"%s\\n\" .\n-\t\t\t\t \"warning: no patches were sent\\n\"), $fn, $hook_error);\n+\t\t\t$hook_error = sprintf(__(\"fatal: %s: rejected by %s hook\\n\" .\n+\t\t\t\t\t\t $hook_error . \"\\n\" .\n+\t\t\t\t\t\t \"warning: no patches were sent\\n\"),\n+\t\t\t\t\t $fn, $hook_name);\n+\t\t\tdie $hook_error;\n \t\t}\n \t}\n \ndiff --git a/t/t9001-send-email.sh b/t/t9001-send-email.sh\nindex aa0c20499ba..84d0f40d76a 100755\n--- a/t/t9001-send-email.sh\n+++ b/t/t9001-send-email.sh\n@@ -539,7 +539,7 @@ test_expect_success $PREREQ \"--validate respects relative core.hooksPath path\" '\n \ttest_path_is_file my-hooks.ran &&\n \tcat >expect <<-EOF &&\n \tfatal: longline.patch: rejected by sendemail-validate hook\n-\tfatal: command '\"'\"'my-hooks/sendemail-validate'\"'\"' died with exit code 1\n+\tfatal: command '\"'\"'git hook run --ignore-missing sendemail-validate -- '\"'\"' died with exit code 1\n \twarning: no patches were sent\n \tEOF\n \ttest_cmp expect actual\n@@ -558,7 +558,7 @@ test_expect_success $PREREQ \"--validate respects absolute core.hooksPath path\" '\n \ttest_path_is_file my-hooks.ran &&\n \tcat >expect <<-EOF &&\n \tfatal: longline.patch: rejected by sendemail-validate hook\n-\tfatal: command '\"'\"'$hooks_path/sendemail-validate'\"'\"' died with exit code 1\n+\tfatal: command '\"'\"'git hook run --ignore-missing sendemail-validate -- '\"'\"' died with exit code 1\n \twarning: no patches were sent\n \tEOF\n \ttest_cmp expect actual\n-- \n2.33.0.1567.g7b23ce7ed9e\n\n"},{"id":"438574","messageId":"d5e2ac19-30ad-ccbd-c90c-41ba6b597cc0@web.de","threadId":"56692","inReplyTo":"patch-01.13-a39c0748d3f-20211012T131934Z-avarab@gmail.com","subject":"Re: [PATCH 01/13] hook: add 'run' subcommand","fromName":"René Scharfe","fromEmail":"l.s.r@web.de","sentAt":"2021-10-12T17:35:44Z","receivedAt":"2021-10-12T17:35:58Z","isPatch":true,"sender":{"key":"l.s.r@web.de","avatar":"https://avatars.githubusercontent.com/u/26122331?v=4"},"body":"Am 12.10.21 um 15:30 schrieb Ævar Arnfjörð Bjarmason:\n> From: Emily Shaffer \n>\n> In order to enable hooks to be run as an external process, by a\n> standalone Git command, or by tools which wrap Git, provide an external\n> means to run all configured hook commands for a given hook event.\n>\n> Most of our hooks require more complex functionality than this, but\n> let's start with the bare minimum required to support our simplest\n> hooks.\n>\n> In terms of implementation the usage_with_options() and \"goto usage\"\n> pattern here mirrors that of\n> builtin/{commit-graph,multi-pack-index}.c.\n>\n> Signed-off-by: Emily Shaffer \n> Signed-off-by: Ævar Arnfjörð Bjarmason \n> ---\n> .gitignore | 1 +\n> Documentation/git-hook.txt | 38 +++++++++++\n> Documentation/githooks.txt | 4 ++\n> Makefile | 1 +\n> builtin.h | 1 +\n> builtin/hook.c | 90 ++++++++++++++++++++++++++\n> command-list.txt | 1 +\n> git.c | 1 +\n> hook.c | 101 +++++++++++++++++++++++++++++\n> hook.h | 39 +++++++++++\n> t/t1800-hook.sh | 129 +++++++++++++++++++++++++++++++++++++\n> 11 files changed, 406 insertions(+)\n> create mode 100644 Documentation/git-hook.txt\n> create mode 100644 builtin/hook.c\n> create mode 100755 t/t1800-hook.sh\n>\n> diff --git a/.gitignore b/.gitignore\n> index 6be9de41ae8..66189ca3cdc 100644\n> --- a/.gitignore\n> +++ b/.gitignore\n> @@ -77,6 +77,7 @@\n> /git-grep\n> /git-hash-object\n> /git-help\n> +/git-hook\n> /git-http-backend\n> /git-http-fetch\n> /git-http-push\n> diff --git a/Documentation/git-hook.txt b/Documentation/git-hook.txt\n> new file mode 100644\n> index 00000000000..660d6a992a0\n> --- /dev/null\n> +++ b/Documentation/git-hook.txt\n> @@ -0,0 +1,38 @@\n> +git-hook(1)\n> +===========\n> +\n> +NAME\n> +----\n> +git-hook - run git hooks\n> +\n> +SYNOPSIS\n> +--------\n> +[verse]\n> +'git hook' run [-- ]\n> +\n> +DESCRIPTION\n> +-----------\n> +\n> +This command is an interface to git hooks (see linkgit:githooks[5]).\n> +Currently it only provides a convenience wrapper for running hooks for\n> +use by git itself. In the future it might gain other functionality.\n\nAs a man page reader I'm only interested in the present features of the\ncommand up here. Breaking changes could be mentioned in a HISTORY\nsection, and missing critical features in a BUGS section at the bottom.\n\nI assume the last sentence applies to almost all programs, and could be\nsafely removed.\n\n> +\n> +SUBCOMMANDS\n> +-----------\n> +\n> +run::\n> +\tRun the `` hook. See linkgit:githooks[5] for\n> +\tthe hook names we support.\n> ++\n> +Any positional arguments to the hook should be passed after an\n> +optional `--` (or `--end-of-options`, see linkgit:gitcli[7]). The\n\nDash dash (or EoO) is optional.\n\n> +arguments (if any) differ by hook name, see linkgit:githooks[5] for\n> +what those are.\n> +\n> +SEE ALSO\n> +--------\n> +linkgit:githooks[5]\n> +\n> +GIT\n> +---\n> +Part of the linkgit:git[1] suite\n> diff --git a/Documentation/githooks.txt b/Documentation/githooks.txt\n> index b51959ff941..a16e62bc8c8 100644\n> --- a/Documentation/githooks.txt\n> +++ b/Documentation/githooks.txt\n> @@ -698,6 +698,10 @@ and \"0\" meaning they were not.\n> Only one parameter should be set to \"1\" when the hook runs. The hook\n> running passing \"1\", \"1\" should not be possible.\n>\n> +SEE ALSO\n> +--------\n> +linkgit:git-hook[1]\n> +\n> GIT\n> ---\n> Part of the linkgit:git[1] suite\n> diff --git a/Makefile b/Makefile\n> index 877492386ee..12b481fdabe 100644\n> --- a/Makefile\n> +++ b/Makefile\n> @@ -1108,6 +1108,7 @@ BUILTIN_OBJS += builtin/get-tar-commit-id.o\n> BUILTIN_OBJS += builtin/grep.o\n> BUILTIN_OBJS += builtin/hash-object.o\n> BUILTIN_OBJS += builtin/help.o\n> +BUILTIN_OBJS += builtin/hook.o\n> BUILTIN_OBJS += builtin/index-pack.o\n> BUILTIN_OBJS += builtin/init-db.o\n> BUILTIN_OBJS += builtin/interpret-trailers.o\n> diff --git a/builtin.h b/builtin.h\n> index 8a58743ed63..83379f3832c 100644\n> --- a/builtin.h\n> +++ b/builtin.h\n> @@ -164,6 +164,7 @@ int cmd_get_tar_commit_id(int argc, const char **argv, const char *prefix);\n> int cmd_grep(int argc, const char **argv, const char *prefix);\n> int cmd_hash_object(int argc, const char **argv, const char *prefix);\n> int cmd_help(int argc, const char **argv, const char *prefix);\n> +int cmd_hook(int argc, const char **argv, const char *prefix);\n> int cmd_index_pack(int argc, const char **argv, const char *prefix);\n> int cmd_init_db(int argc, const char **argv, const char *prefix);\n> int cmd_interpret_trailers(int argc, const char **argv, const char *prefix);\n> diff --git a/builtin/hook.c b/builtin/hook.c\n> new file mode 100644\n> index 00000000000..012a2973b38\n> --- /dev/null\n> +++ b/builtin/hook.c\n> @@ -0,0 +1,90 @@\n> +#include \"cache.h\"\n> +#include \"builtin.h\"\n> +#include \"config.h\"\n> +#include \"hook.h\"\n> +#include \"parse-options.h\"\n> +#include \"strbuf.h\"\n> +#include \"strvec.h\"\n> +\n> +#define BUILTIN_HOOK_RUN_USAGE \\\n> +\tN_(\"git hook run [-- ]\")\n> +\n> +static const char * const builtin_hook_usage[] = {\n> +\tBUILTIN_HOOK_RUN_USAGE,\n> +\tNULL\n> +};\n> +\n> +static const char * const builtin_hook_run_usage[] = {\n> +\tBUILTIN_HOOK_RUN_USAGE,\n> +\tNULL\n> +};\n> +\n> +static int run(int argc, const char **argv, const char *prefix)\n> +{\n> +\tint i;\n> +\tstruct run_hooks_opt opt = RUN_HOOKS_OPT_INIT;\n> +\tconst char *hook_name;\n> +\tconst char *hook_path;\n> +\tstruct option run_options[] = {\n> +\t\tOPT_END(),\n> +\t};\n> +\tint ret;\n> +\n> +\targc = parse_options(argc, argv, prefix, run_options,\n> +\t\t\t builtin_hook_run_usage,\n> +\t\t\t PARSE_OPT_KEEP_DASHDASH);\n> +\n> +\tif (!argc)\n> +\t\tgoto usage;\n> +\n> +\t/*\n> +\t * Having a -- for \"run\" when providing is\n> +\t * mandatory.\n> +\t */\n> +\tif (argc > 1 && strcmp(argv[1], \"--\") &&\n> +\t strcmp(argv[1], \"--end-of-options\"))\n> +\t\tgoto usage;\nDash dash (or EoO) is mandatory unless parse_options() left no\narguments, contrary to what the documentation says above. Update\nthe doc above?\n\n> +\n> +\t/* Add our arguments, start after -- */\n> +\tfor (i = 2 ; i < argc; i++)\n> +\t\tstrvec_push(&opt.args, argv[i]);\n> +\n> +\t/* Need to take into account core.hooksPath */\n> +\tgit_config(git_default_config, NULL);\n> +\n> +\t/*\n> +\t * We are not using a plain run_hooks() because we'd like to\n> +\t * detect missing hooks. Let's find it ourselves and call\n> +\t * run_hooks() instead.\n\nSo run_hooks(), which is introduced later in this patch, can find a\nhook as well, but would do so silently?\n\n> +\t */\n> +\thook_name = argv[0];\n> +\thook_path = find_hook(hook_name);\n\nThis is the only find_hook() call introduced by this patch.\nDoes run_hooks() really posses an unused hook-finding capability?\n\n> +\tif (!hook_path) {\n> +\t\terror(\"cannot find a hook named %s\", hook_name);\n> +\t\treturn 1;\n> +\t}\n> +\n> +\tret = run_hooks(hook_name, hook_path, &opt);\n> +\trun_hooks_opt_clear(&opt);\n> +\treturn ret;\n> +usage:\n> +\tusage_with_options(builtin_hook_run_usage, run_options);\n> +}\n> +\n> +int cmd_hook(int argc, const char **argv, const char *prefix)\n> +{\n> +\tstruct option builtin_hook_options[] = {\n> +\t\tOPT_END(),\n> +\t};\n> +\n> +\targc = parse_options(argc, argv, NULL, builtin_hook_options,\n> +\t\t\t builtin_hook_usage, PARSE_OPT_STOP_AT_NON_OPTION);\n> +\tif (!argc)\n> +\t\tgoto usage;\n> +\n> +\tif (!strcmp(argv[0], \"run\"))\n> +\t\treturn run(argc, argv, prefix);\n> +\n> +usage:\n> +\tusage_with_options(builtin_hook_usage, builtin_hook_options);\n> +}\n> diff --git a/command-list.txt b/command-list.txt\n> index a289f09ed6f..9ccd8e5aebe 100644\n> --- a/command-list.txt\n> +++ b/command-list.txt\n> @@ -103,6 +103,7 @@ git-grep mainporcelain info\n> git-gui mainporcelain\n> git-hash-object plumbingmanipulators\n> git-help ancillaryinterrogators complete\n> +git-hook mainporcelain\n> git-http-backend synchingrepositories\n> git-http-fetch synchelpers\n> git-http-push synchelpers\n> diff --git a/git.c b/git.c\n> index 60c2784be45..e5891e02021 100644\n> --- a/git.c\n> +++ b/git.c\n> @@ -538,6 +538,7 @@ static struct cmd_struct commands[] = {\n> \t{ \"grep\", cmd_grep, RUN_SETUP_GENTLY },\n> \t{ \"hash-object\", cmd_hash_object },\n> \t{ \"help\", cmd_help },\n> +\t{ \"hook\", cmd_hook, RUN_SETUP },\n> \t{ \"index-pack\", cmd_index_pack, RUN_SETUP_GENTLY | NO_PARSEOPT },\n> \t{ \"init\", cmd_init_db },\n> \t{ \"init-db\", cmd_init_db },\n> diff --git a/hook.c b/hook.c\n> index 55e1145a4b7..240270db64e 100644\n> --- a/hook.c\n> +++ b/hook.c\n> @@ -1,6 +1,7 @@\n> #include \"cache.h\"\n> #include \"hook.h\"\n> #include \"run-command.h\"\n> +#include \"config.h\"\n>\n> const char *find_hook(const char *name)\n> {\n> @@ -40,3 +41,103 @@ int hook_exists(const char *name)\n> {\n> \treturn !!find_hook(name);\n> }\n> +\n> +void run_hooks_opt_clear(struct run_hooks_opt *o)\n> +{\n> +\tstrvec_clear(&o->env);\n> +\tstrvec_clear(&o->args);\n> +}\n> +\n> +static int pick_next_hook(struct child_process *cp,\n> +\t\t\t struct strbuf *out,\n> +\t\t\t void *pp_cb,\n> +\t\t\t void **pp_task_cb)\n> +{\n> +\tstruct hook_cb_data *hook_cb = pp_cb;\n> +\tstruct hook *run_me = hook_cb->run_me;\n> +\n> +\tif (!run_me)\n> +\t\treturn 0;\n> +\n> +\tcp->no_stdin = 1;\n> +\tcp->env = hook_cb->options->env.v;\n> +\tcp->stdout_to_stderr = 1;\n> +\tcp->trace2_hook_name = hook_cb->hook_name;\n> +\n> +\t/* add command */\n> +\tstrvec_push(&cp->args, run_me->hook_path);\n> +\n> +\t/*\n> +\t * add passed-in argv, without expanding - let the user get back\n> +\t * exactly what they put in\n\nWhat kind of expanding is it doing without? I was expecting the\narguments to passed on verbatim before reading this comment, so it\nleaves me wondering what I'm missing.\n\n> +\t */\n> +\tstrvec_pushv(&cp->args, hook_cb->options->args.v);\n> +\n> +\t/* Provide context for errors if necessary */\n> +\t*pp_task_cb = run_me;\n> +\n> +\t/*\n> +\t * This pick_next_hook() will be called again, we're only\n> +\t * running one hook, so indicate that no more work will be\n> +\t * done.\n> +\t */\n> +\thook_cb->run_me = NULL;\n\nA single hook is picked.\n\n> +\n> +\treturn 1;\n> +}\n> +\n> +static int notify_start_failure(struct strbuf *out,\n> +\t\t\t\tvoid *pp_cb,\n> +\t\t\t\tvoid *pp_task_cp)\n> +{\n> +\tstruct hook_cb_data *hook_cb = pp_cb;\n> +\tstruct hook *attempted = pp_task_cp;\n> +\n> +\thook_cb->rc |= 1;\n> +\n> +\tstrbuf_addf(out, _(\"Couldn't start hook '%s'\\n\"),\n> +\t\t attempted->hook_path);\n> +\n> +\treturn 1;\n> +}\n> +\n> +static int notify_hook_finished(int result,\n> +\t\t\t\tstruct strbuf *out,\n> +\t\t\t\tvoid *pp_cb,\n> +\t\t\t\tvoid *pp_task_cb)\n> +{\n> +\tstruct hook_cb_data *hook_cb = pp_cb;\n> +\n> +\thook_cb->rc |= result;\n> +\n> +\treturn 0;\n> +}\n> +\n> +int run_hooks(const char *hook_name, const char *hook_path,\n> +\t struct run_hooks_opt *options)\n> +{\n> +\tstruct hook my_hook = {\n> +\t\t.hook_path = hook_path,\n> +\t};\n> +\tstruct hook_cb_data cb_data = {\n> +\t\t.rc = 0,\n> +\t\t.hook_name = hook_name,\n> +\t\t.options = options,\n> +\t};\n> +\tint jobs = 1;\n> +\n> +\tif (!options)\n> +\t\tBUG(\"a struct run_hooks_opt must be provided to run_hooks\");\n> +\n> +\tcb_data.run_me = &my_hook;\n> +\n> +\trun_processes_parallel_tr2(jobs,\n> +\t\t\t\t pick_next_hook,\n> +\t\t\t\t notify_start_failure,\n> +\t\t\t\t notify_hook_finished,\n> +\t\t\t\t &cb_data,\n> +\t\t\t\t \"hook\",\n> +\t\t\t\t hook_name);\n\nA single process (jobs == 1) is used to run a single hook. Why use\nrun_processes_parallel_tr2() for that instead of run_command()? The\nlatter would require a lot less code to achieve the same outcome, no?\n\n> +\n> +\treturn cb_data.rc;\n> +}\n> diff --git a/hook.h b/hook.h\n> index 6aa36fc7ff9..111c5cf9296 100644\n> --- a/hook.h\n> +++ b/hook.h\n> @@ -1,5 +1,33 @@\n> #ifndef HOOK_H\n> #define HOOK_H\n> +#include \"strvec.h\"\n> +\n> +struct hook {\n> +\t/* The path to the hook */\n> +\tconst char *hook_path;\n> +};\n\nNone of the patches in this series extend this structure. Why not\nuse a bare string directly?\n\n> +\n> +struct run_hooks_opt\n> +{\n> +\t/* Environment vars to be set for each hook */\n> +\tstruct strvec env;\n> +\n> +\t/* Args to be passed to each hook */\n> +\tstruct strvec args;\n> +};\n> +\n> +#define RUN_HOOKS_OPT_INIT { \\\n> +\t.env = STRVEC_INIT, \\\n> +\t.args = STRVEC_INIT, \\\n> +}\n> +\n> +struct hook_cb_data {\n> +\t/* rc reflects the cumulative failure state */\n> +\tint rc;\n> +\tconst char *hook_name;\n> +\tstruct hook *run_me;\n> +\tstruct run_hooks_opt *options;\n> +};\n>\n> /*\n> * Returns the path to the hook file, or NULL if the hook is missing\n> @@ -13,4 +41,15 @@ const char *find_hook(const char *name);\n> */\n> int hook_exists(const char *hookname);\n>\n> +/**\n> + * Clear data from an initialized \"struct run_hooks-opt\".\n ^\ny/-/_/\n\n> + */\n> +void run_hooks_opt_clear(struct run_hooks_opt *o);\n> +\n> +/**\n> + * Takes an already resolved hook found via find_hook() and runs\n> + * it. Does not call run_hooks_opt_clear() for you.\n> + */\n> +int run_hooks(const char *hookname, const char *hook_path,\n> +\t struct run_hooks_opt *options);\n\nIf it runs one hook, why is it called run_hooks(), plural?\n\n> #endif\n> diff --git a/t/t1800-hook.sh b/t/t1800-hook.sh\n> new file mode 100755\n> index 00000000000..3aea1b105f0\n> --- /dev/null\n> +++ b/t/t1800-hook.sh\n> @@ -0,0 +1,129 @@\n> +#!/bin/sh\n> +\n> +test_description='git-hook command'\n> +\n> +TEST_PASSES_SANITIZE_LEAK=true\n> +. ./test-lib.sh\n> +\n> +test_expect_success 'git hook usage' '\n> +\ttest_expect_code 129 git hook &&\n> +\ttest_expect_code 129 git hook run &&\n> +\ttest_expect_code 129 git hook run -h &&\n> +\ttest_expect_code 129 git hook run --unknown 2>err &&\n> +\tgrep \"unknown option\" err\n> +'\n> +\n> +test_expect_success 'git hook run: nonexistent hook' '\n> +\tcat >stderr.expect <<-\\EOF &&\n> +\terror: cannot find a hook named test-hook\n> +\tEOF\n> +\ttest_expect_code 1 git hook run test-hook 2>stderr.actual &&\n> +\ttest_cmp stderr.expect stderr.actual\n> +'\n> +\n> +test_expect_success 'git hook run: basic' '\n> +\twrite_script .git/hooks/test-hook <<-EOF &&\n> +\techo Test hook\n> +\tEOF\n> +\n> +\tcat >expect <<-\\EOF &&\n> +\tTest hook\n> +\tEOF\n> +\tgit hook run test-hook 2>actual &&\n> +\ttest_cmp expect actual\n> +'\n> +\n> +test_expect_success 'git hook run: stdout and stderr both write to our stderr' '\n> +\twrite_script .git/hooks/test-hook <<-EOF &&\n> +\techo >&1 Will end up on stderr\n> +\techo >&2 Will end up on stderr\n> +\tEOF\n> +\n> +\tcat >stderr.expect <<-\\EOF &&\n> +\tWill end up on stderr\n> +\tWill end up on stderr\n> +\tEOF\n> +\tgit hook run test-hook >stdout.actual 2>stderr.actual &&\n> +\ttest_cmp stderr.expect stderr.actual &&\n> +\ttest_must_be_empty stdout.actual\n> +'\n> +\n> +test_expect_success 'git hook run: exit codes are passed along' '\n> +\twrite_script .git/hooks/test-hook <<-EOF &&\n> +\texit 1\n> +\tEOF\n> +\n> +\ttest_expect_code 1 git hook run test-hook &&\n> +\n> +\twrite_script .git/hooks/test-hook <<-EOF &&\n> +\texit 2\n> +\tEOF\n> +\n> +\ttest_expect_code 2 git hook run test-hook &&\n> +\n> +\twrite_script .git/hooks/test-hook <<-EOF &&\n> +\texit 128\n> +\tEOF\n> +\n> +\ttest_expect_code 128 git hook run test-hook &&\n> +\n> +\twrite_script .git/hooks/test-hook <<-EOF &&\n> +\texit 129\n> +\tEOF\n> +\n> +\ttest_expect_code 129 git hook run test-hook\n> +'\n> +\n> +test_expect_success 'git hook run arg u ments without -- is not allowed' '\n> +\ttest_expect_code 129 git hook run test-hook arg u ments\n> +'\n> +\n> +test_expect_success 'git hook run -- pass arguments' '\n> +\twrite_script .git/hooks/test-hook <<-\\EOF &&\n> +\techo $1\n> +\techo $2\n> +\tEOF\n> +\n> +\tcat >expect <<-EOF &&\n> +\targ\n> +\tu ments\n> +\tEOF\n> +\n> +\tgit hook run test-hook -- arg \"u ments\" 2>actual &&\n> +\ttest_cmp expect actual\n> +'\n> +\n> +test_expect_success 'git hook run -- out-of-repo runs excluded' '\n> +\twrite_script .git/hooks/test-hook <<-EOF &&\n> +\techo Test hook\n> +\tEOF\n> +\n> +\tnongit test_must_fail git hook run test-hook\n> +'\n> +\n> +test_expect_success 'git -c core.hooksPath= hook run' '\n> +\tmkdir my-hooks &&\n> +\twrite_script my-hooks/test-hook <<-\\EOF &&\n> +\techo Hook ran $1 >>actual\n> +\tEOF\n> +\n> +\tcat >expect <<-\\EOF &&\n> +\tTest hook\n> +\tHook ran one\n> +\tHook ran two\n> +\tHook ran three\n> +\tHook ran four\n> +\tEOF\n> +\n> +\t# Test various ways of specifying the path. See also\n> +\t# t1350-config-hooks-path.sh\n> +\t>actual &&\n> +\tgit hook run test-hook -- ignored 2>>actual &&\n> +\tgit -c core.hooksPath=my-hooks hook run test-hook -- one 2>>actual &&\n> +\tgit -c core.hooksPath=my-hooks/ hook run test-hook -- two 2>>actual &&\n> +\tgit -c core.hooksPath=\"$PWD/my-hooks\" hook run test-hook -- three 2>>actual &&\n> +\tgit -c core.hooksPath=\"$PWD/my-hooks/\" hook run test-hook -- four 2>>actual &&\n> +\ttest_cmp expect actual\n> +'\n> +\n> +test_done\n>\n"},{"id":"438624","messageId":"32bd578c-0369-8efe-8a92-d9ec248c0e34@gmail.com","threadId":"56692","inReplyTo":"patch-01.13-a39c0748d3f-20211012T131934Z-avarab@gmail.com","subject":"Re: [PATCH 01/13] hook: add 'run' subcommand","fromName":"Bagas Sanjaya","fromEmail":"bagasdotme@gmail.com","sentAt":"2021-10-13T00:52:39Z","receivedAt":"2021-10-13T00:52:47Z","isPatch":true,"sender":{"key":"bagasdotme@gmail.com","avatar":"https://avatars.githubusercontent.com/u/40219486?v=4"},"body":"On 12/10/21 20.30, Ævar Arnfjörð Bjarmason wrote:\n> +run::\n> +\tRun the `` hook. See linkgit:githooks[5] for\n> +\tthe hook names we support.\n> ++\n\nDrop the first person, so it become `list of supported hooks`.\n\n> +Any positional arguments to the hook should be passed after an\n> +optional `--` (or `--end-of-options`, see linkgit:gitcli[7]). The\n> +arguments (if any) differ by hook name, see linkgit:githooks[5] for\n> +what those are.\n> +\n\ns/what those are/supported list of them/ (dunno?)\n\n-- \nAn old man doll... just what I always wanted! - Clara\n"},{"id":"438647","messageId":"87k0ihhya2.fsf@evledraar.gmail.com","threadId":"56692","inReplyTo":"d5e2ac19-30ad-ccbd-c90c-41ba6b597cc0@web.de","subject":"Re: [PATCH 01/13] hook: add 'run' subcommand","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2021-10-13T12:58:48Z","receivedAt":"2021-10-13T13:03:54Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"\nOn Tue, Oct 12 2021, René Scharfe wrote:\n\n> Am 12.10.21 um 15:30 schrieb Ævar Arnfjörð Bjarmason:\n>> From: Emily Shaffer \n>>\n>> In order to enable hooks to be run as an external process, by a\n>> standalone Git command, or by tools which wrap Git, provide an external\n>> means to run all configured hook commands for a given hook event.\n>>\n>> Most of our hooks require more complex functionality than this, but\n>> let's start with the bare minimum required to support our simplest\n>> hooks.\n>>\n>> In terms of implementation the usage_with_options() and \"goto usage\"\n>> pattern here mirrors that of\n>> builtin/{commit-graph,multi-pack-index}.c.\n>>\n>> Signed-off-by: Emily Shaffer \n>> Signed-off-by: Ævar Arnfjörð Bjarmason \n>> ---\n>> .gitignore | 1 +\n>> Documentation/git-hook.txt | 38 +++++++++++\n>> Documentation/githooks.txt | 4 ++\n>> Makefile | 1 +\n>> builtin.h | 1 +\n>> builtin/hook.c | 90 ++++++++++++++++++++++++++\n>> command-list.txt | 1 +\n>> git.c | 1 +\n>> hook.c | 101 +++++++++++++++++++++++++++++\n>> hook.h | 39 +++++++++++\n>> t/t1800-hook.sh | 129 +++++++++++++++++++++++++++++++++++++\n>> 11 files changed, 406 insertions(+)\n>> create mode 100644 Documentation/git-hook.txt\n>> create mode 100644 builtin/hook.c\n>> create mode 100755 t/t1800-hook.sh\n>>\n>> diff --git a/.gitignore b/.gitignore\n>> index 6be9de41ae8..66189ca3cdc 100644\n>> --- a/.gitignore\n>> +++ b/.gitignore\n>> @@ -77,6 +77,7 @@\n>> /git-grep\n>> /git-hash-object\n>> /git-help\n>> +/git-hook\n>> /git-http-backend\n>> /git-http-fetch\n>> /git-http-push\n>> diff --git a/Documentation/git-hook.txt b/Documentation/git-hook.txt\n>> new file mode 100644\n>> index 00000000000..660d6a992a0\n>> --- /dev/null\n>> +++ b/Documentation/git-hook.txt\n>> @@ -0,0 +1,38 @@\n>> +git-hook(1)\n>> +===========\n>> +\n>> +NAME\n>> +----\n>> +git-hook - run git hooks\n>> +\n>> +SYNOPSIS\n>> +--------\n>> +[verse]\n>> +'git hook' run [-- ]\n>> +\n>> +DESCRIPTION\n>> +-----------\n>> +\n>> +This command is an interface to git hooks (see linkgit:githooks[5]).\n>> +Currently it only provides a convenience wrapper for running hooks for\n>> +use by git itself. In the future it might gain other functionality.\n>\n> As a man page reader I'm only interested in the present features of the\n> command up here. Breaking changes could be mentioned in a HISTORY\n> section, and missing critical features in a BUGS section at the bottom.\n>\n> I assume the last sentence applies to almost all programs, and could be\n> safely removed.\n\nWilldo.\n\n>> +\n>> +SUBCOMMANDS\n>> +-----------\n>> +\n>> +run::\n>> +\tRun the `` hook. See linkgit:githooks[5] for\n>> +\tthe hook names we support.\n>> ++\n>> +Any positional arguments to the hook should be passed after an\n>> +optional `--` (or `--end-of-options`, see linkgit:gitcli[7]). The\n>\n> Dash dash (or EoO) is optional.\n\nThis just needs to be rephrased, the -- is mandatory, i.e. it's either:\n\n git hook run [args] hook-name\n\nOr:\n\n git hook run [args] hook-name -- [hook-args]\n\nBut not:\n\n git hook run [args] hook-name [hook-args]\n\n>> +arguments (if any) differ by hook name, see linkgit:githooks[5] for\n>> +what those are.\n>> +\n>> +SEE ALSO\n>> +--------\n>> +linkgit:githooks[5]\n>> +\n>> +GIT\n>> +---\n>> +Part of the linkgit:git[1] suite\n>> diff --git a/Documentation/githooks.txt b/Documentation/githooks.txt\n>> index b51959ff941..a16e62bc8c8 100644\n>> --- a/Documentation/githooks.txt\n>> +++ b/Documentation/githooks.txt\n>> @@ -698,6 +698,10 @@ and \"0\" meaning they were not.\n>> Only one parameter should be set to \"1\" when the hook runs. The hook\n>> running passing \"1\", \"1\" should not be possible.\n>>\n>> +SEE ALSO\n>> +--------\n>> +linkgit:git-hook[1]\n>> +\n>> GIT\n>> ---\n>> Part of the linkgit:git[1] suite\n>> diff --git a/Makefile b/Makefile\n>> index 877492386ee..12b481fdabe 100644\n>> --- a/Makefile\n>> +++ b/Makefile\n>> @@ -1108,6 +1108,7 @@ BUILTIN_OBJS += builtin/get-tar-commit-id.o\n>> BUILTIN_OBJS += builtin/grep.o\n>> BUILTIN_OBJS += builtin/hash-object.o\n>> BUILTIN_OBJS += builtin/help.o\n>> +BUILTIN_OBJS += builtin/hook.o\n>> BUILTIN_OBJS += builtin/index-pack.o\n>> BUILTIN_OBJS += builtin/init-db.o\n>> BUILTIN_OBJS += builtin/interpret-trailers.o\n>> diff --git a/builtin.h b/builtin.h\n>> index 8a58743ed63..83379f3832c 100644\n>> --- a/builtin.h\n>> +++ b/builtin.h\n>> @@ -164,6 +164,7 @@ int cmd_get_tar_commit_id(int argc, const char **argv, const char *prefix);\n>> int cmd_grep(int argc, const char **argv, const char *prefix);\n>> int cmd_hash_object(int argc, const char **argv, const char *prefix);\n>> int cmd_help(int argc, const char **argv, const char *prefix);\n>> +int cmd_hook(int argc, const char **argv, const char *prefix);\n>> int cmd_index_pack(int argc, const char **argv, const char *prefix);\n>> int cmd_init_db(int argc, const char **argv, const char *prefix);\n>> int cmd_interpret_trailers(int argc, const char **argv, const char *prefix);\n>> diff --git a/builtin/hook.c b/builtin/hook.c\n>> new file mode 100644\n>> index 00000000000..012a2973b38\n>> --- /dev/null\n>> +++ b/builtin/hook.c\n>> @@ -0,0 +1,90 @@\n>> +#include \"cache.h\"\n>> +#include \"builtin.h\"\n>> +#include \"config.h\"\n>> +#include \"hook.h\"\n>> +#include \"parse-options.h\"\n>> +#include \"strbuf.h\"\n>> +#include \"strvec.h\"\n>> +\n>> +#define BUILTIN_HOOK_RUN_USAGE \\\n>> +\tN_(\"git hook run [-- ]\")\n>> +\n>> +static const char * const builtin_hook_usage[] = {\n>> +\tBUILTIN_HOOK_RUN_USAGE,\n>> +\tNULL\n>> +};\n>> +\n>> +static const char * const builtin_hook_run_usage[] = {\n>> +\tBUILTIN_HOOK_RUN_USAGE,\n>> +\tNULL\n>> +};\n>> +\n>> +static int run(int argc, const char **argv, const char *prefix)\n>> +{\n>> +\tint i;\n>> +\tstruct run_hooks_opt opt = RUN_HOOKS_OPT_INIT;\n>> +\tconst char *hook_name;\n>> +\tconst char *hook_path;\n>> +\tstruct option run_options[] = {\n>> +\t\tOPT_END(),\n>> +\t};\n>> +\tint ret;\n>> +\n>> +\targc = parse_options(argc, argv, prefix, run_options,\n>> +\t\t\t builtin_hook_run_usage,\n>> +\t\t\t PARSE_OPT_KEEP_DASHDASH);\n>> +\n>> +\tif (!argc)\n>> +\t\tgoto usage;\n>> +\n>> +\t/*\n>> +\t * Having a -- for \"run\" when providing is\n>> +\t * mandatory.\n>> +\t */\n>> +\tif (argc > 1 && strcmp(argv[1], \"--\") &&\n>> +\t strcmp(argv[1], \"--end-of-options\"))\n>> +\t\tgoto usage;\n> Dash dash (or EoO) is mandatory unless parse_options() left no\n> arguments, contrary to what the documentation says above. Update\n> the doc above?\n\n*nod*\n\n>> +\n>> +\t/* Add our arguments, start after -- */\n>> +\tfor (i = 2 ; i < argc; i++)\n>> +\t\tstrvec_push(&opt.args, argv[i]);\n>> +\n>> +\t/* Need to take into account core.hooksPath */\n>> +\tgit_config(git_default_config, NULL);\n>> +\n>> +\t/*\n>> +\t * We are not using a plain run_hooks() because we'd like to\n>> +\t * detect missing hooks. Let's find it ourselves and call\n>> +\t * run_hooks() instead.\n>\n> So run_hooks(), which is introduced later in this patch, can find a\n> hook as well, but would do so silently?\n\nWill clarify, I think Emily and I went back and forth on this comment a\nbit, and at some point I removed it entirely I think...\n\n>> +\t */\n>> +\thook_name = argv[0];\n>> +\thook_path = find_hook(hook_name);\n>\n> This is the only find_hook() call introduced by this patch.\n> Does run_hooks() really posses an unused hook-finding capability?\n\nWill address...\n\n>> +\tif (!hook_path) {\n>> +\t\terror(\"cannot find a hook named %s\", hook_name);\n>> +\t\treturn 1;\n>> +\t}\n>> +\n>> +\tret = run_hooks(hook_name, hook_path, &opt);\n>> +\trun_hooks_opt_clear(&opt);\n>> +\treturn ret;\n>> +usage:\n>> +\tusage_with_options(builtin_hook_run_usage, run_options);\n>> +}\n>> +\n>> +int cmd_hook(int argc, const char **argv, const char *prefix)\n>> +{\n>> +\tstruct option builtin_hook_options[] = {\n>> +\t\tOPT_END(),\n>> +\t};\n>> +\n>> +\targc = parse_options(argc, argv, NULL, builtin_hook_options,\n>> +\t\t\t builtin_hook_usage, PARSE_OPT_STOP_AT_NON_OPTION);\n>> +\tif (!argc)\n>> +\t\tgoto usage;\n>> +\n>> +\tif (!strcmp(argv[0], \"run\"))\n>> +\t\treturn run(argc, argv, prefix);\n>> +\n>> +usage:\n>> +\tusage_with_options(builtin_hook_usage, builtin_hook_options);\n>> +}\n>> diff --git a/command-list.txt b/command-list.txt\n>> index a289f09ed6f..9ccd8e5aebe 100644\n>> --- a/command-list.txt\n>> +++ b/command-list.txt\n>> @@ -103,6 +103,7 @@ git-grep mainporcelain info\n>> git-gui mainporcelain\n>> git-hash-object plumbingmanipulators\n>> git-help ancillaryinterrogators complete\n>> +git-hook mainporcelain\n>> git-http-backend synchingrepositories\n>> git-http-fetch synchelpers\n>> git-http-push synchelpers\n>> diff --git a/git.c b/git.c\n>> index 60c2784be45..e5891e02021 100644\n>> --- a/git.c\n>> +++ b/git.c\n>> @@ -538,6 +538,7 @@ static struct cmd_struct commands[] = {\n>> \t{ \"grep\", cmd_grep, RUN_SETUP_GENTLY },\n>> \t{ \"hash-object\", cmd_hash_object },\n>> \t{ \"help\", cmd_help },\n>> +\t{ \"hook\", cmd_hook, RUN_SETUP },\n>> \t{ \"index-pack\", cmd_index_pack, RUN_SETUP_GENTLY | NO_PARSEOPT },\n>> \t{ \"init\", cmd_init_db },\n>> \t{ \"init-db\", cmd_init_db },\n>> diff --git a/hook.c b/hook.c\n>> index 55e1145a4b7..240270db64e 100644\n>> --- a/hook.c\n>> +++ b/hook.c\n>> @@ -1,6 +1,7 @@\n>> #include \"cache.h\"\n>> #include \"hook.h\"\n>> #include \"run-command.h\"\n>> +#include \"config.h\"\n>>\n>> const char *find_hook(const char *name)\n>> {\n>> @@ -40,3 +41,103 @@ int hook_exists(const char *name)\n>> {\n>> \treturn !!find_hook(name);\n>> }\n>> +\n>> +void run_hooks_opt_clear(struct run_hooks_opt *o)\n>> +{\n>> +\tstrvec_clear(&o->env);\n>> +\tstrvec_clear(&o->args);\n>> +}\n>> +\n>> +static int pick_next_hook(struct child_process *cp,\n>> +\t\t\t struct strbuf *out,\n>> +\t\t\t void *pp_cb,\n>> +\t\t\t void **pp_task_cb)\n>> +{\n>> +\tstruct hook_cb_data *hook_cb = pp_cb;\n>> +\tstruct hook *run_me = hook_cb->run_me;\n>> +\n>> +\tif (!run_me)\n>> +\t\treturn 0;\n>> +\n>> +\tcp->no_stdin = 1;\n>> +\tcp->env = hook_cb->options->env.v;\n>> +\tcp->stdout_to_stderr = 1;\n>> +\tcp->trace2_hook_name = hook_cb->hook_name;\n>> +\n>> +\t/* add command */\n>> +\tstrvec_push(&cp->args, run_me->hook_path);\n>> +\n>> +\t/*\n>> +\t * add passed-in argv, without expanding - let the user get back\n>> +\t * exactly what they put in\n>\n> What kind of expanding is it doing without? I was expecting the\n> arguments to passed on verbatim before reading this comment, so it\n> leaves me wondering what I'm missing.\n\nI think that's one of Emily's comments, will find out...\n\n>> +\t */\n>> +\tstrvec_pushv(&cp->args, hook_cb->options->args.v);\n>> +\n>> +\t/* Provide context for errors if necessary */\n>> +\t*pp_task_cb = run_me;\n>> +\n>> +\t/*\n>> +\t * This pick_next_hook() will be called again, we're only\n>> +\t * running one hook, so indicate that no more work will be\n>> +\t * done.\n>> +\t */\n>> +\thook_cb->run_me = NULL;\n>\n> A single hook is picked.\n\nRight...\n\n>> +\n>> +\treturn 1;\n>> +}\n>> +\n>> +static int notify_start_failure(struct strbuf *out,\n>> +\t\t\t\tvoid *pp_cb,\n>> +\t\t\t\tvoid *pp_task_cp)\n>> +{\n>> +\tstruct hook_cb_data *hook_cb = pp_cb;\n>> +\tstruct hook *attempted = pp_task_cp;\n>> +\n>> +\thook_cb->rc |= 1;\n>> +\n>> +\tstrbuf_addf(out, _(\"Couldn't start hook '%s'\\n\"),\n>> +\t\t attempted->hook_path);\n>> +\n>> +\treturn 1;\n>> +}\n>> +\n>> +static int notify_hook_finished(int result,\n>> +\t\t\t\tstruct strbuf *out,\n>> +\t\t\t\tvoid *pp_cb,\n>> +\t\t\t\tvoid *pp_task_cb)\n>> +{\n>> +\tstruct hook_cb_data *hook_cb = pp_cb;\n>> +\n>> +\thook_cb->rc |= result;\n>> +\n>> +\treturn 0;\n>> +}\n>> +\n>> +int run_hooks(const char *hook_name, const char *hook_path,\n>> +\t struct run_hooks_opt *options)\n>> +{\n>> +\tstruct hook my_hook = {\n>> +\t\t.hook_path = hook_path,\n>> +\t};\n>> +\tstruct hook_cb_data cb_data = {\n>> +\t\t.rc = 0,\n>> +\t\t.hook_name = hook_name,\n>> +\t\t.options = options,\n>> +\t};\n>> +\tint jobs = 1;\n>> +\n>> +\tif (!options)\n>> +\t\tBUG(\"a struct run_hooks_opt must be provided to run_hooks\");\n>> +\n>> +\tcb_data.run_me = &my_hook;\n>> +\n>> +\trun_processes_parallel_tr2(jobs,\n>> +\t\t\t\t pick_next_hook,\n>> +\t\t\t\t notify_start_failure,\n>> +\t\t\t\t notify_hook_finished,\n>> +\t\t\t\t &cb_data,\n>> +\t\t\t\t \"hook\",\n>> +\t\t\t\t hook_name);\n>\n> A single process (jobs == 1) is used to run a single hook. Why use\n> run_processes_parallel_tr2() for that instead of run_command()? The\n> latter would require a lot less code to achieve the same outcome, no?\n\n...also for this...\n\n>> +\n>> +\treturn cb_data.rc;\n>> +}\n>> diff --git a/hook.h b/hook.h\n>> index 6aa36fc7ff9..111c5cf9296 100644\n>> --- a/hook.h\n>> +++ b/hook.h\n>> @@ -1,5 +1,33 @@\n>> #ifndef HOOK_H\n>> #define HOOK_H\n>> +#include \"strvec.h\"\n>> +\n>> +struct hook {\n>> +\t/* The path to the hook */\n>> +\tconst char *hook_path;\n>> +};\n>\n> None of the patches in this series extend this structure. Why not\n> use a bare string directly?\n\n...the reason for all of this is that it's a multi-part series where\nwe're eventually headed towards multi-hook parallel support.\n\nAn earlier version of this started with run_command() et al, then later\nmigrated to the parallel API, since the parallel API can do a single run\nwith a jobs=1 I found that easier than the curn of switching back and\nforth.\n\nDitto some other scaffolding like this one-member struct, which will be\nexpanded later on.\n\nWill address this by clarifying the plans in commit messages.\n\n>> +\n>> +struct run_hooks_opt\n>> +{\n>> +\t/* Environment vars to be set for each hook */\n>> +\tstruct strvec env;\n>> +\n>> +\t/* Args to be passed to each hook */\n>> +\tstruct strvec args;\n>> +};\n>> +\n>> +#define RUN_HOOKS_OPT_INIT { \\\n>> +\t.env = STRVEC_INIT, \\\n>> +\t.args = STRVEC_INIT, \\\n>> +}\n>> +\n>> +struct hook_cb_data {\n>> +\t/* rc reflects the cumulative failure state */\n>> +\tint rc;\n>> +\tconst char *hook_name;\n>> +\tstruct hook *run_me;\n>> +\tstruct run_hooks_opt *options;\n>> +};\n>>\n>> /*\n>> * Returns the path to the hook file, or NULL if the hook is missing\n>> @@ -13,4 +41,15 @@ const char *find_hook(const char *name);\n>> */\n>> int hook_exists(const char *hookname);\n>>\n>> +/**\n>> + * Clear data from an initialized \"struct run_hooks-opt\".\n> ^\n> y/-/_/\n\n*nod*\n\n>> + */\n>> +void run_hooks_opt_clear(struct run_hooks_opt *o);\n>> +\n>> +/**\n>> + * Takes an already resolved hook found via find_hook() and runs\n>> + * it. Does not call run_hooks_opt_clear() for you.\n>> + */\n>> +int run_hooks(const char *hookname, const char *hook_path,\n>> +\t struct run_hooks_opt *options);\n>\n> If it runs one hook, why is it called run_hooks(), plural?\n\nDitto above, will clarify, eventually it will run N, and then having the\nchurn of changing all callers/re-indenting argument lists...\n\n\nThanks a lot for your time & the review. Will wait on a re-roll for more\ncomments...\n"},{"id":"438648","messageId":"87fst5hy9f.fsf@evledraar.gmail.com","threadId":"56692","inReplyTo":"32bd578c-0369-8efe-8a92-d9ec248c0e34@gmail.com","subject":"Re: [PATCH 01/13] hook: add 'run' subcommand","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2021-10-13T13:04:08Z","receivedAt":"2021-10-13T13:04:19Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"\nOn Wed, Oct 13 2021, Bagas Sanjaya wrote:\n\n> On 12/10/21 20.30, Ævar Arnfjörð Bjarmason wrote:\n>> +run::\n>> +\tRun the `` hook. See linkgit:githooks[5] for\n>> +\tthe hook names we support.\n>> ++\n>\n> Drop the first person, so it become `list of supported hooks`.\n>\n>> +Any positional arguments to the hook should be passed after an\n>> +optional `--` (or `--end-of-options`, see linkgit:gitcli[7]). The\n>> +arguments (if any) differ by hook name, see linkgit:githooks[5] for\n>> +what those are.\n>> +\n>\n> s/what those are/supported list of them/ (dunno?)\n\nThanks, will address both.\n"}]}