* [PATCH 00/13] hook.[ch]: new library to run hooks + simple hook conversion
@ 2021-10-12 13:30 Ævar Arnfjörð Bjarmason
2021-10-12 13:30 ` [PATCH 01/13] hook: add 'run' subcommand Ævar Arnfjörð Bjarmason
` (12 more replies)
0 siblings, 13 replies; 18+ messages in thread
From: Ævar Arnfjörð Bjarmason @ 2021-10-12 13:30 UTC (permalink / raw)
To: git
Cc: Junio C Hamano, Phillip Wood, René Scharfe, Emily Shaffer,
Joachim Kuebart, Daniel Levin, Luke Diamand,
Ævar Arnfjörð Bjarmason
This series is part 2 of an incremental restart of the greater
"config-based hooks" topic by Emily Shaffer that I've been helping
with. See [1] for part 1, and [2] (search for "mark2") for a summary
of parts to come.
This goes on top of the "ab/config-based-hooks-1" topic that's already
in "next" and marked for graduation to "master".
In this topic we build upon the skeleton hook.[ch] library and build
infrastructure added in part 1 and add a hook running library and new
"git hook" command.
The new "git hook" command is (for now) only for the benefit of
in-tree non-C programs that need to run hooks, and this series
converts git-send-email and git-p4 to use it.
At the end of the series we remove
"run_hook_{le,ve}()" from run-command.c, as we've migrated all its
callers.
What we don't do is convert any of the more complex in-tree hooks that
require input on stdin such as "pre-receive" and the like, that's in
later parts once this lands.
This series is approximately patch 6-20/36 of the previous 36 patch
ab/config-based-hooks-base topic. A range-diff to that v5[3] is
included below.
The changes since that v5 are rather trivial, they are:
* Formatting changes to reduce the diff to parts that come after this
(which Emily & I were juggling at the time), and re-flowing some
overly long lines.
* The new test is now marked for SANITIZE=leak testing, with the new
test facility I've added recently (and which just landed in
"master"). They all pass, the new API doesn't leak.
* I rewrote some of the git-send-email.perl code to avoid
de-duplication and hardcoding (just using intermediate variables).
Things to focus on reviewing:
* This should all be pretty solid and well tested, but the git-p4.py
part in particular I've never tested for real (not having access to
p4), and think Emily hasn't either[4].
The relevant patch looks trivially correct to me[5], and I've tried it
out in the Python REPL. But if any of the CC'd people active in
git-p4.py development could give it some end-to-end testing that
would be much appreciated.
* Both Python and Perl code now calls the in-between "git hook run"
command rather than calling hooks directly. Will this behave
differently due to any special behavior running via a git built-in
adds?
I vaguely recall a third-party "git-foo" program breaking in the
past when invoked as "git foo" but not "git-foo", due to git
squatting on SIGINT, but none of that should be relevant here
(we're not starting a pager etc.).
1. https://lore.kernel.org/git/cover-v2-0.8-00000000000-20210926T185800Z-avarab@gmail.com/
2. https://lore.kernel.org/git/875yut8nns.fsf@evledraar.gmail.com/
3. https://lore.kernel.org/git/cover-v5-00.36-00000000000-20210902T125110Z-avarab@gmail.com/
4. https://lore.kernel.org/git/20210311021037.3001235-26-emilyshaffer@google.com/
5. https://lore.kernel.org/git/patch-09.13-69cc447a1e1-20211012T131934Z-avarab@gmail.com
Emily Shaffer (12):
hook: add 'run' subcommand
gc: use hook library for pre-auto-gc hook
rebase: convert pre-rebase to use hook.h
am: convert applypatch to use hook.h
hooks: convert 'post-checkout' hook to hook library
merge: convert post-merge to use hook.h
send-email: use 'git hook run' for 'sendemail-validate'
git-p4: use 'git hook' to run hooks
commit: convert {pre-commit,prepare-commit-msg} hook to hook.h
read-cache: convert post-index-change to use hook.h
receive-pack: convert push-to-checkout hook to hook.h
run-command: remove old run_hook_{le,ve}() hook API
Ævar Arnfjörð Bjarmason (1):
git hook run: add an --ignore-missing flag
.gitignore | 1 +
Documentation/git-hook.txt | 46 +++++++++++++
Documentation/githooks.txt | 4 ++
Makefile | 1 +
builtin.h | 1 +
builtin/am.c | 8 ++-
builtin/checkout.c | 14 ++--
builtin/clone.c | 7 +-
builtin/gc.c | 3 +-
builtin/hook.c | 96 ++++++++++++++++++++++++++
builtin/merge.c | 4 +-
builtin/rebase.c | 8 ++-
builtin/receive-pack.c | 7 +-
builtin/worktree.c | 28 ++++----
command-list.txt | 1 +
commit.c | 15 +++--
git-p4.py | 72 ++------------------
git-send-email.perl | 22 +++---
git.c | 1 +
hook.c | 130 +++++++++++++++++++++++++++++++++++
hook.h | 61 +++++++++++++++++
read-cache.c | 11 ++-
reset.c | 14 ++--
run-command.c | 32 ---------
run-command.h | 17 -----
t/t1800-hook.sh | 134 +++++++++++++++++++++++++++++++++++++
t/t9001-send-email.sh | 4 +-
27 files changed, 572 insertions(+), 170 deletions(-)
create mode 100644 Documentation/git-hook.txt
create mode 100644 builtin/hook.c
create mode 100755 t/t1800-hook.sh
Range-diff:
1: 72dd1010f5b ! 1: a39c0748d3f hook: add 'run' subcommand
@@ hook.h
+ struct run_hooks_opt *options;
+};
- /**
+ /*
* Returns the path to the hook file, or NULL if the hook is missing
@@ hook.h: const char *find_hook(const char *name);
*/
@@ t/t1800-hook.sh (new)
+
+test_description='git-hook command'
+
++TEST_PASSES_SANITIZE_LEAK=true
+. ./test-lib.sh
+
+test_expect_success 'git hook usage' '
2: 821cc9bf11e ! 2: dbac4204f7b gc: use hook library for pre-auto-gc hook
@@ hook.c: int run_hooks(const char *hook_name, const char *hook_path,
+int run_hooks_oneshot(const char *hook_name, struct run_hooks_opt *options)
+{
+ const char *hook_path;
-+ int ret;
+ struct run_hooks_opt hook_opt_scratch = RUN_HOOKS_OPT_INIT;
++ int ret = 0;
+
+ if (!options)
+ options = &hook_opt_scratch;
+
+ hook_path = find_hook(hook_name);
-+ if (!hook_path) {
-+ ret = 0;
++ if (!hook_path)
+ goto cleanup;
-+ }
+
+ ret = run_hooks(hook_name, hook_path, options);
-+
+cleanup:
+ run_hooks_opt_clear(options);
+
3: d71c90254ea ! 3: ff306debcb8 rebase: convert pre-rebase to use hook.h
@@ builtin/rebase.c: int cmd_rebase(int argc, const char **argv, const char *prefix
}
/* If a hook exists, give it a chance to interrupt*/
-+ strvec_pushl(&hook_opt.args, options.upstream_arg, argc ? argv[0] : NULL, NULL);
++ strvec_push(&hook_opt.args, options.upstream_arg);
++ if (argc)
++ strvec_push(&hook_opt.args, argv[0]);
if (!ok_to_skip_pre_rebase &&
- run_hook_le(NULL, "pre-rebase", options.upstream_arg,
- argc ? argv[0] : NULL, NULL))
4: ea3af2ccc4d = 4: b1d529ca485 am: convert applypatch to use hook.h
5: fed0b52f88f ! 5: 15d71fc210b hooks: convert 'post-checkout' hook to hook library
@@ builtin/clone.c: static int checkout(int submodule_progress)
- err |= run_hook_le(NULL, "post-checkout", oid_to_hex(null_oid()),
- oid_to_hex(&oid), "1", NULL);
-+ strvec_pushl(&hook_opt.args, oid_to_hex(null_oid()), oid_to_hex(&oid), "1", NULL);
++ strvec_pushl(&hook_opt.args, oid_to_hex(null_oid()), oid_to_hex(&oid),
++ "1", NULL);
+ err |= run_hooks_oneshot("post-checkout", &hook_opt);
if (!err && (option_recurse_submodules.nr > 0)) {
6: 53d8721a0e3 = 6: 08f27f0d6be merge: convert post-merge to use hook.h
7: d60827a2856 ! 7: 107c14d740f git hook run: add an --ignore-missing flag
@@ builtin/hook.c: static int run(int argc, const char **argv, const char *prefix)
const char *hook_path;
struct option run_options[] = {
+ OPT_BOOL(0, "ignore-missing", &ignore_missing,
-+ N_("exit quietly with a zero exit code if the requested hook cannot be found")),
++ N_("silently ignore missing requested <hook-name>")),
OPT_END(),
};
int ret;
8: d4976a0821f ! 8: 1d30e2dbbe0 send-email: use 'git hook run' for 'sendemail-validate'
@@ git-send-email.perl: sub validate_patch {
or die("chdir: $!");
local $ENV{"GIT_DIR"} = $repo->repo_path();
- $hook_error = system_or_msg([$validate_hook, $target]);
-+ my @validate_hook = ("git", "hook", "run", "--ignore-missing", $hook_name, "--", $target);
-+ $hook_error = system_or_msg(\@validate_hook, undef,
-+ "git hook run $hook_name -- <patch>");
++ my @cmd = ("git", "hook", "run", "--ignore-missing",
++ $hook_name, "--");
++ my @cmd_msg = (@cmd, "<patch>");
++ my @cmd_run = (@cmd, $target);
++ $hook_error = system_or_msg(\@cmd_run, undef, "@cmd_msg");
chdir($cwd_save) or die("chdir: $!");
}
if ($hook_error) {
@@ t/t9001-send-email.sh: test_expect_success $PREREQ "--validate respects relative
cat >expect <<-EOF &&
fatal: longline.patch: rejected by sendemail-validate hook
- fatal: command '"'"'my-hooks/sendemail-validate'"'"' died with exit code 1
-+ fatal: command '"'"'git hook run sendemail-validate -- <patch>'"'"' died with exit code 1
++ fatal: command '"'"'git hook run --ignore-missing sendemail-validate -- <patch>'"'"' died with exit code 1
warning: no patches were sent
EOF
test_cmp expect actual
@@ t/t9001-send-email.sh: test_expect_success $PREREQ "--validate respects absolute
cat >expect <<-EOF &&
fatal: longline.patch: rejected by sendemail-validate hook
- fatal: command '"'"'$hooks_path/sendemail-validate'"'"' died with exit code 1
-+ fatal: command '"'"'git hook run sendemail-validate -- <patch>'"'"' died with exit code 1
++ fatal: command '"'"'git hook run --ignore-missing sendemail-validate -- <patch>'"'"' died with exit code 1
warning: no patches were sent
EOF
test_cmp expect actual
9: 99f3dcd1945 = 9: 69cc447a1e1 git-p4: use 'git hook' to run hooks
10: 509761454e6 = 10: 1c22b2992cf commit: convert {pre-commit,prepare-commit-msg} hook to hook.h
11: e2c94d95427 ! 11: e762fce32af read-cache: convert post-index-change to use hook.h
@@ Commit message
Move the post-index-change hook away from run-command.h to and over to
the new hook.h library.
- This removes the last direct user of run_hook_ve(), so we can make the
- function static now. It'll be removed entirely soon.
+ This removes the last direct user of "run_hook_ve()" outside of
+ run-command.c ("run_hook_le()" still uses it). So we can make the
+ function static now. A subsequent commit will remove this code
+ entirely when "run_hook_le()" itself goes away.
Signed-off-by: Emily Shaffer <emilyshaffer@google.com>
Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
@@ read-cache.c: static int do_write_locked_index(struct index_state *istate, struc
int was_full = !istate->sparse_index;
+ struct run_hooks_opt hook_opt = RUN_HOOKS_OPT_INIT;
- ret = convert_to_sparse(istate);
+ ret = convert_to_sparse(istate, 0);
@@ read-cache.c: static int do_write_locked_index(struct index_state *istate, struct lock_file *l
else
12: fa7d0d24ea2 = 12: d63b91196ae receive-pack: convert push-to-checkout hook to hook.h
13: 428bb5a6792 = 13: fe8996dda3e run-command: remove old run_hook_{le,ve}() hook API
--
2.33.0.1567.g7b23ce7ed9e
^ permalink raw reply [flat|nested] 18+ messages in thread
* [PATCH 01/13] hook: add 'run' subcommand
2021-10-12 13:30 [PATCH 00/13] hook.[ch]: new library to run hooks + simple hook conversion Ævar Arnfjörð Bjarmason
@ 2021-10-12 13:30 ` Ævar Arnfjörð Bjarmason
2021-10-12 17:35 ` René Scharfe
2021-10-13 0:52 ` Bagas Sanjaya
2021-10-12 13:30 ` [PATCH 02/13] gc: use hook library for pre-auto-gc hook Ævar Arnfjörð Bjarmason
` (11 subsequent siblings)
12 siblings, 2 replies; 18+ messages in thread
From: Ævar Arnfjörð Bjarmason @ 2021-10-12 13:30 UTC (permalink / raw)
To: git
Cc: Junio C Hamano, Phillip Wood, René Scharfe, Emily Shaffer,
Joachim Kuebart, Daniel Levin, Luke Diamand,
Ævar Arnfjörð Bjarmason
From: Emily Shaffer <emilyshaffer@google.com>
In order to enable hooks to be run as an external process, by a
standalone Git command, or by tools which wrap Git, provide an external
means to run all configured hook commands for a given hook event.
Most of our hooks require more complex functionality than this, but
let's start with the bare minimum required to support our simplest
hooks.
In terms of implementation the usage_with_options() and "goto usage"
pattern here mirrors that of
builtin/{commit-graph,multi-pack-index}.c.
Signed-off-by: Emily Shaffer <emilyshaffer@google.com>
Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
.gitignore | 1 +
Documentation/git-hook.txt | 38 +++++++++++
Documentation/githooks.txt | 4 ++
Makefile | 1 +
builtin.h | 1 +
builtin/hook.c | 90 ++++++++++++++++++++++++++
command-list.txt | 1 +
git.c | 1 +
hook.c | 101 +++++++++++++++++++++++++++++
hook.h | 39 +++++++++++
t/t1800-hook.sh | 129 +++++++++++++++++++++++++++++++++++++
11 files changed, 406 insertions(+)
create mode 100644 Documentation/git-hook.txt
create mode 100644 builtin/hook.c
create mode 100755 t/t1800-hook.sh
diff --git a/.gitignore b/.gitignore
index 6be9de41ae8..66189ca3cdc 100644
--- a/.gitignore
+++ b/.gitignore
@@ -77,6 +77,7 @@
/git-grep
/git-hash-object
/git-help
+/git-hook
/git-http-backend
/git-http-fetch
/git-http-push
diff --git a/Documentation/git-hook.txt b/Documentation/git-hook.txt
new file mode 100644
index 00000000000..660d6a992a0
--- /dev/null
+++ b/Documentation/git-hook.txt
@@ -0,0 +1,38 @@
+git-hook(1)
+===========
+
+NAME
+----
+git-hook - run git hooks
+
+SYNOPSIS
+--------
+[verse]
+'git hook' run <hook-name> [-- <hook-args>]
+
+DESCRIPTION
+-----------
+
+This command is an interface to git hooks (see linkgit:githooks[5]).
+Currently it only provides a convenience wrapper for running hooks for
+use by git itself. In the future it might gain other functionality.
+
+SUBCOMMANDS
+-----------
+
+run::
+ Run the `<hook-name>` hook. See linkgit:githooks[5] for
+ the hook names we support.
++
+Any positional arguments to the hook should be passed after an
+optional `--` (or `--end-of-options`, see linkgit:gitcli[7]). The
+arguments (if any) differ by hook name, see linkgit:githooks[5] for
+what those are.
+
+SEE ALSO
+--------
+linkgit:githooks[5]
+
+GIT
+---
+Part of the linkgit:git[1] suite
diff --git a/Documentation/githooks.txt b/Documentation/githooks.txt
index b51959ff941..a16e62bc8c8 100644
--- a/Documentation/githooks.txt
+++ b/Documentation/githooks.txt
@@ -698,6 +698,10 @@ and "0" meaning they were not.
Only one parameter should be set to "1" when the hook runs. The hook
running passing "1", "1" should not be possible.
+SEE ALSO
+--------
+linkgit:git-hook[1]
+
GIT
---
Part of the linkgit:git[1] suite
diff --git a/Makefile b/Makefile
index 877492386ee..12b481fdabe 100644
--- a/Makefile
+++ b/Makefile
@@ -1108,6 +1108,7 @@ BUILTIN_OBJS += builtin/get-tar-commit-id.o
BUILTIN_OBJS += builtin/grep.o
BUILTIN_OBJS += builtin/hash-object.o
BUILTIN_OBJS += builtin/help.o
+BUILTIN_OBJS += builtin/hook.o
BUILTIN_OBJS += builtin/index-pack.o
BUILTIN_OBJS += builtin/init-db.o
BUILTIN_OBJS += builtin/interpret-trailers.o
diff --git a/builtin.h b/builtin.h
index 8a58743ed63..83379f3832c 100644
--- a/builtin.h
+++ b/builtin.h
@@ -164,6 +164,7 @@ int cmd_get_tar_commit_id(int argc, const char **argv, const char *prefix);
int cmd_grep(int argc, const char **argv, const char *prefix);
int cmd_hash_object(int argc, const char **argv, const char *prefix);
int cmd_help(int argc, const char **argv, const char *prefix);
+int cmd_hook(int argc, const char **argv, const char *prefix);
int cmd_index_pack(int argc, const char **argv, const char *prefix);
int cmd_init_db(int argc, const char **argv, const char *prefix);
int cmd_interpret_trailers(int argc, const char **argv, const char *prefix);
diff --git a/builtin/hook.c b/builtin/hook.c
new file mode 100644
index 00000000000..012a2973b38
--- /dev/null
+++ b/builtin/hook.c
@@ -0,0 +1,90 @@
+#include "cache.h"
+#include "builtin.h"
+#include "config.h"
+#include "hook.h"
+#include "parse-options.h"
+#include "strbuf.h"
+#include "strvec.h"
+
+#define BUILTIN_HOOK_RUN_USAGE \
+ N_("git hook run <hook-name> [-- <hook-args>]")
+
+static const char * const builtin_hook_usage[] = {
+ BUILTIN_HOOK_RUN_USAGE,
+ NULL
+};
+
+static const char * const builtin_hook_run_usage[] = {
+ BUILTIN_HOOK_RUN_USAGE,
+ NULL
+};
+
+static int run(int argc, const char **argv, const char *prefix)
+{
+ int i;
+ struct run_hooks_opt opt = RUN_HOOKS_OPT_INIT;
+ const char *hook_name;
+ const char *hook_path;
+ struct option run_options[] = {
+ OPT_END(),
+ };
+ int ret;
+
+ argc = parse_options(argc, argv, prefix, run_options,
+ builtin_hook_run_usage,
+ PARSE_OPT_KEEP_DASHDASH);
+
+ if (!argc)
+ goto usage;
+
+ /*
+ * Having a -- for "run" when providing <hook-args> is
+ * mandatory.
+ */
+ if (argc > 1 && strcmp(argv[1], "--") &&
+ strcmp(argv[1], "--end-of-options"))
+ goto usage;
+
+ /* Add our arguments, start after -- */
+ for (i = 2 ; i < argc; i++)
+ strvec_push(&opt.args, argv[i]);
+
+ /* Need to take into account core.hooksPath */
+ git_config(git_default_config, NULL);
+
+ /*
+ * We are not using a plain run_hooks() because we'd like to
+ * detect missing hooks. Let's find it ourselves and call
+ * run_hooks() instead.
+ */
+ hook_name = argv[0];
+ hook_path = find_hook(hook_name);
+ if (!hook_path) {
+ error("cannot find a hook named %s", hook_name);
+ return 1;
+ }
+
+ ret = run_hooks(hook_name, hook_path, &opt);
+ run_hooks_opt_clear(&opt);
+ return ret;
+usage:
+ usage_with_options(builtin_hook_run_usage, run_options);
+}
+
+int cmd_hook(int argc, const char **argv, const char *prefix)
+{
+ struct option builtin_hook_options[] = {
+ OPT_END(),
+ };
+
+ argc = parse_options(argc, argv, NULL, builtin_hook_options,
+ builtin_hook_usage, PARSE_OPT_STOP_AT_NON_OPTION);
+ if (!argc)
+ goto usage;
+
+ if (!strcmp(argv[0], "run"))
+ return run(argc, argv, prefix);
+
+usage:
+ usage_with_options(builtin_hook_usage, builtin_hook_options);
+}
diff --git a/command-list.txt b/command-list.txt
index a289f09ed6f..9ccd8e5aebe 100644
--- a/command-list.txt
+++ b/command-list.txt
@@ -103,6 +103,7 @@ git-grep mainporcelain info
git-gui mainporcelain
git-hash-object plumbingmanipulators
git-help ancillaryinterrogators complete
+git-hook mainporcelain
git-http-backend synchingrepositories
git-http-fetch synchelpers
git-http-push synchelpers
diff --git a/git.c b/git.c
index 60c2784be45..e5891e02021 100644
--- a/git.c
+++ b/git.c
@@ -538,6 +538,7 @@ static struct cmd_struct commands[] = {
{ "grep", cmd_grep, RUN_SETUP_GENTLY },
{ "hash-object", cmd_hash_object },
{ "help", cmd_help },
+ { "hook", cmd_hook, RUN_SETUP },
{ "index-pack", cmd_index_pack, RUN_SETUP_GENTLY | NO_PARSEOPT },
{ "init", cmd_init_db },
{ "init-db", cmd_init_db },
diff --git a/hook.c b/hook.c
index 55e1145a4b7..240270db64e 100644
--- a/hook.c
+++ b/hook.c
@@ -1,6 +1,7 @@
#include "cache.h"
#include "hook.h"
#include "run-command.h"
+#include "config.h"
const char *find_hook(const char *name)
{
@@ -40,3 +41,103 @@ int hook_exists(const char *name)
{
return !!find_hook(name);
}
+
+void run_hooks_opt_clear(struct run_hooks_opt *o)
+{
+ strvec_clear(&o->env);
+ strvec_clear(&o->args);
+}
+
+static int pick_next_hook(struct child_process *cp,
+ struct strbuf *out,
+ void *pp_cb,
+ void **pp_task_cb)
+{
+ struct hook_cb_data *hook_cb = pp_cb;
+ struct hook *run_me = hook_cb->run_me;
+
+ if (!run_me)
+ return 0;
+
+ cp->no_stdin = 1;
+ cp->env = hook_cb->options->env.v;
+ cp->stdout_to_stderr = 1;
+ cp->trace2_hook_name = hook_cb->hook_name;
+
+ /* add command */
+ strvec_push(&cp->args, run_me->hook_path);
+
+ /*
+ * add passed-in argv, without expanding - let the user get back
+ * exactly what they put in
+ */
+ strvec_pushv(&cp->args, hook_cb->options->args.v);
+
+ /* Provide context for errors if necessary */
+ *pp_task_cb = run_me;
+
+ /*
+ * This pick_next_hook() will be called again, we're only
+ * running one hook, so indicate that no more work will be
+ * done.
+ */
+ hook_cb->run_me = NULL;
+
+ return 1;
+}
+
+static int notify_start_failure(struct strbuf *out,
+ void *pp_cb,
+ void *pp_task_cp)
+{
+ struct hook_cb_data *hook_cb = pp_cb;
+ struct hook *attempted = pp_task_cp;
+
+ hook_cb->rc |= 1;
+
+ strbuf_addf(out, _("Couldn't start hook '%s'\n"),
+ attempted->hook_path);
+
+ return 1;
+}
+
+static int notify_hook_finished(int result,
+ struct strbuf *out,
+ void *pp_cb,
+ void *pp_task_cb)
+{
+ struct hook_cb_data *hook_cb = pp_cb;
+
+ hook_cb->rc |= result;
+
+ return 0;
+}
+
+int run_hooks(const char *hook_name, const char *hook_path,
+ struct run_hooks_opt *options)
+{
+ struct hook my_hook = {
+ .hook_path = hook_path,
+ };
+ struct hook_cb_data cb_data = {
+ .rc = 0,
+ .hook_name = hook_name,
+ .options = options,
+ };
+ int jobs = 1;
+
+ if (!options)
+ BUG("a struct run_hooks_opt must be provided to run_hooks");
+
+ cb_data.run_me = &my_hook;
+
+ run_processes_parallel_tr2(jobs,
+ pick_next_hook,
+ notify_start_failure,
+ notify_hook_finished,
+ &cb_data,
+ "hook",
+ hook_name);
+
+ return cb_data.rc;
+}
diff --git a/hook.h b/hook.h
index 6aa36fc7ff9..111c5cf9296 100644
--- a/hook.h
+++ b/hook.h
@@ -1,5 +1,33 @@
#ifndef HOOK_H
#define HOOK_H
+#include "strvec.h"
+
+struct hook {
+ /* The path to the hook */
+ const char *hook_path;
+};
+
+struct run_hooks_opt
+{
+ /* Environment vars to be set for each hook */
+ struct strvec env;
+
+ /* Args to be passed to each hook */
+ struct strvec args;
+};
+
+#define RUN_HOOKS_OPT_INIT { \
+ .env = STRVEC_INIT, \
+ .args = STRVEC_INIT, \
+}
+
+struct hook_cb_data {
+ /* rc reflects the cumulative failure state */
+ int rc;
+ const char *hook_name;
+ struct hook *run_me;
+ struct run_hooks_opt *options;
+};
/*
* Returns the path to the hook file, or NULL if the hook is missing
@@ -13,4 +41,15 @@ const char *find_hook(const char *name);
*/
int hook_exists(const char *hookname);
+/**
+ * Clear data from an initialized "struct run_hooks-opt".
+ */
+void run_hooks_opt_clear(struct run_hooks_opt *o);
+
+/**
+ * Takes an already resolved hook found via find_hook() and runs
+ * it. Does not call run_hooks_opt_clear() for you.
+ */
+int run_hooks(const char *hookname, const char *hook_path,
+ struct run_hooks_opt *options);
#endif
diff --git a/t/t1800-hook.sh b/t/t1800-hook.sh
new file mode 100755
index 00000000000..3aea1b105f0
--- /dev/null
+++ b/t/t1800-hook.sh
@@ -0,0 +1,129 @@
+#!/bin/sh
+
+test_description='git-hook command'
+
+TEST_PASSES_SANITIZE_LEAK=true
+. ./test-lib.sh
+
+test_expect_success 'git hook usage' '
+ test_expect_code 129 git hook &&
+ test_expect_code 129 git hook run &&
+ test_expect_code 129 git hook run -h &&
+ test_expect_code 129 git hook run --unknown 2>err &&
+ grep "unknown option" err
+'
+
+test_expect_success 'git hook run: nonexistent hook' '
+ cat >stderr.expect <<-\EOF &&
+ error: cannot find a hook named test-hook
+ EOF
+ test_expect_code 1 git hook run test-hook 2>stderr.actual &&
+ test_cmp stderr.expect stderr.actual
+'
+
+test_expect_success 'git hook run: basic' '
+ write_script .git/hooks/test-hook <<-EOF &&
+ echo Test hook
+ EOF
+
+ cat >expect <<-\EOF &&
+ Test hook
+ EOF
+ git hook run test-hook 2>actual &&
+ test_cmp expect actual
+'
+
+test_expect_success 'git hook run: stdout and stderr both write to our stderr' '
+ write_script .git/hooks/test-hook <<-EOF &&
+ echo >&1 Will end up on stderr
+ echo >&2 Will end up on stderr
+ EOF
+
+ cat >stderr.expect <<-\EOF &&
+ Will end up on stderr
+ Will end up on stderr
+ EOF
+ git hook run test-hook >stdout.actual 2>stderr.actual &&
+ test_cmp stderr.expect stderr.actual &&
+ test_must_be_empty stdout.actual
+'
+
+test_expect_success 'git hook run: exit codes are passed along' '
+ write_script .git/hooks/test-hook <<-EOF &&
+ exit 1
+ EOF
+
+ test_expect_code 1 git hook run test-hook &&
+
+ write_script .git/hooks/test-hook <<-EOF &&
+ exit 2
+ EOF
+
+ test_expect_code 2 git hook run test-hook &&
+
+ write_script .git/hooks/test-hook <<-EOF &&
+ exit 128
+ EOF
+
+ test_expect_code 128 git hook run test-hook &&
+
+ write_script .git/hooks/test-hook <<-EOF &&
+ exit 129
+ EOF
+
+ test_expect_code 129 git hook run test-hook
+'
+
+test_expect_success 'git hook run arg u ments without -- is not allowed' '
+ test_expect_code 129 git hook run test-hook arg u ments
+'
+
+test_expect_success 'git hook run -- pass arguments' '
+ write_script .git/hooks/test-hook <<-\EOF &&
+ echo $1
+ echo $2
+ EOF
+
+ cat >expect <<-EOF &&
+ arg
+ u ments
+ EOF
+
+ git hook run test-hook -- arg "u ments" 2>actual &&
+ test_cmp expect actual
+'
+
+test_expect_success 'git hook run -- out-of-repo runs excluded' '
+ write_script .git/hooks/test-hook <<-EOF &&
+ echo Test hook
+ EOF
+
+ nongit test_must_fail git hook run test-hook
+'
+
+test_expect_success 'git -c core.hooksPath=<PATH> hook run' '
+ mkdir my-hooks &&
+ write_script my-hooks/test-hook <<-\EOF &&
+ echo Hook ran $1 >>actual
+ EOF
+
+ cat >expect <<-\EOF &&
+ Test hook
+ Hook ran one
+ Hook ran two
+ Hook ran three
+ Hook ran four
+ EOF
+
+ # Test various ways of specifying the path. See also
+ # t1350-config-hooks-path.sh
+ >actual &&
+ git hook run test-hook -- ignored 2>>actual &&
+ git -c core.hooksPath=my-hooks hook run test-hook -- one 2>>actual &&
+ git -c core.hooksPath=my-hooks/ hook run test-hook -- two 2>>actual &&
+ git -c core.hooksPath="$PWD/my-hooks" hook run test-hook -- three 2>>actual &&
+ git -c core.hooksPath="$PWD/my-hooks/" hook run test-hook -- four 2>>actual &&
+ test_cmp expect actual
+'
+
+test_done
--
2.33.0.1567.g7b23ce7ed9e
^ permalink raw reply related [flat|nested] 18+ messages in thread
* [PATCH 02/13] gc: use hook library for pre-auto-gc hook
2021-10-12 13:30 [PATCH 00/13] hook.[ch]: new library to run hooks + simple hook conversion Ævar Arnfjörð Bjarmason
2021-10-12 13:30 ` [PATCH 01/13] hook: add 'run' subcommand Ævar Arnfjörð Bjarmason
@ 2021-10-12 13:30 ` Ævar Arnfjörð Bjarmason
2021-10-12 13:30 ` [PATCH 03/13] rebase: convert pre-rebase to use hook.h Ævar Arnfjörð Bjarmason
` (10 subsequent siblings)
12 siblings, 0 replies; 18+ messages in thread
From: Ævar Arnfjörð Bjarmason @ 2021-10-12 13:30 UTC (permalink / raw)
To: git
Cc: Junio C Hamano, Phillip Wood, René Scharfe, Emily Shaffer,
Joachim Kuebart, Daniel Levin, Luke Diamand,
Ævar Arnfjörð Bjarmason
From: Emily Shaffer <emilyshaffer@google.com>
Move the pre-auto-gc hook away from run-command.h to and over to the
new hook.h library.
To do this introduce a simple run_hooks_oneshot() wrapper, we'll be
using it extensively for these simple cases of wanting to run a single
hook under a given name, and having it free the memory we allocate for
us.
Signed-off-by: Emily Shaffer <emilyshaffer@google.com>
Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
builtin/gc.c | 3 ++-
hook.c | 20 ++++++++++++++++++++
hook.h | 13 +++++++++++++
3 files changed, 35 insertions(+), 1 deletion(-)
diff --git a/builtin/gc.c b/builtin/gc.c
index 6b3de3dd514..95939e77f53 100644
--- a/builtin/gc.c
+++ b/builtin/gc.c
@@ -32,6 +32,7 @@
#include "remote.h"
#include "object-store.h"
#include "exec-cmd.h"
+#include "hook.h"
#define FAILED_RUN "failed to run %s"
@@ -394,7 +395,7 @@ static int need_to_gc(void)
else
return 0;
- if (run_hook_le(NULL, "pre-auto-gc", NULL))
+ if (run_hooks_oneshot("pre-auto-gc", NULL))
return 0;
return 1;
}
diff --git a/hook.c b/hook.c
index 240270db64e..bfdc79e2f1a 100644
--- a/hook.c
+++ b/hook.c
@@ -141,3 +141,23 @@ int run_hooks(const char *hook_name, const char *hook_path,
return cb_data.rc;
}
+
+int run_hooks_oneshot(const char *hook_name, struct run_hooks_opt *options)
+{
+ const char *hook_path;
+ struct run_hooks_opt hook_opt_scratch = RUN_HOOKS_OPT_INIT;
+ int ret = 0;
+
+ if (!options)
+ options = &hook_opt_scratch;
+
+ hook_path = find_hook(hook_name);
+ if (!hook_path)
+ goto cleanup;
+
+ ret = run_hooks(hook_name, hook_path, options);
+cleanup:
+ run_hooks_opt_clear(options);
+
+ return ret;
+}
diff --git a/hook.h b/hook.h
index 111c5cf9296..a2b8d4fc6bd 100644
--- a/hook.h
+++ b/hook.h
@@ -49,7 +49,20 @@ void run_hooks_opt_clear(struct run_hooks_opt *o);
/**
* Takes an already resolved hook found via find_hook() and runs
* it. Does not call run_hooks_opt_clear() for you.
+ *
+ * See run_hooks_oneshot() for the simpler one-shot API.
*/
int run_hooks(const char *hookname, const char *hook_path,
struct run_hooks_opt *options);
+
+/**
+ * Calls find_hook() on your "hook_name" and runs the hooks (if any)
+ * with run_hooks().
+ *
+ * If "options" is provided calls run_hooks_opt_clear() on it for
+ * you. If "options" is NULL the default options from
+ * RUN_HOOKS_OPT_INIT will be used.
+ */
+int run_hooks_oneshot(const char *hook_name, struct run_hooks_opt *options);
+
#endif
--
2.33.0.1567.g7b23ce7ed9e
^ permalink raw reply related [flat|nested] 18+ messages in thread
* [PATCH 03/13] rebase: convert pre-rebase to use hook.h
2021-10-12 13:30 [PATCH 00/13] hook.[ch]: new library to run hooks + simple hook conversion Ævar Arnfjörð Bjarmason
2021-10-12 13:30 ` [PATCH 01/13] hook: add 'run' subcommand Ævar Arnfjörð Bjarmason
2021-10-12 13:30 ` [PATCH 02/13] gc: use hook library for pre-auto-gc hook Ævar Arnfjörð Bjarmason
@ 2021-10-12 13:30 ` Ævar Arnfjörð Bjarmason
2021-10-12 13:30 ` [PATCH 04/13] am: convert applypatch " Ævar Arnfjörð Bjarmason
` (9 subsequent siblings)
12 siblings, 0 replies; 18+ messages in thread
From: Ævar Arnfjörð Bjarmason @ 2021-10-12 13:30 UTC (permalink / raw)
To: git
Cc: Junio C Hamano, Phillip Wood, René Scharfe, Emily Shaffer,
Joachim Kuebart, Daniel Levin, Luke Diamand,
Ævar Arnfjörð Bjarmason
From: Emily Shaffer <emilyshaffer@google.com>
Move the pre-rebase hook away from run-command.h to and over to the
new hook.h library.
Signed-off-by: Emily Shaffer <emilyshaffer@google.com>
Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
builtin/rebase.c | 8 ++++++--
1 file changed, 6 insertions(+), 2 deletions(-)
diff --git a/builtin/rebase.c b/builtin/rebase.c
index 8c6393f6d78..9a44939d662 100644
--- a/builtin/rebase.c
+++ b/builtin/rebase.c
@@ -28,6 +28,7 @@
#include "sequencer.h"
#include "rebase-interactive.h"
#include "reset.h"
+#include "hook.h"
#define DEFAULT_REFLOG_ACTION "rebase"
@@ -1305,6 +1306,7 @@ int cmd_rebase(int argc, const char **argv, const char *prefix)
char *squash_onto_name = NULL;
int reschedule_failed_exec = -1;
int allow_preemptive_ff = 1;
+ struct run_hooks_opt hook_opt = RUN_HOOKS_OPT_INIT;
struct option builtin_rebase_options[] = {
OPT_STRING(0, "onto", &options.onto_name,
N_("revision"),
@@ -2021,9 +2023,11 @@ int cmd_rebase(int argc, const char **argv, const char *prefix)
}
/* If a hook exists, give it a chance to interrupt*/
+ strvec_push(&hook_opt.args, options.upstream_arg);
+ if (argc)
+ strvec_push(&hook_opt.args, argv[0]);
if (!ok_to_skip_pre_rebase &&
- run_hook_le(NULL, "pre-rebase", options.upstream_arg,
- argc ? argv[0] : NULL, NULL))
+ run_hooks_oneshot("pre-rebase", &hook_opt))
die(_("The pre-rebase hook refused to rebase."));
if (options.flags & REBASE_DIFFSTAT) {
--
2.33.0.1567.g7b23ce7ed9e
^ permalink raw reply related [flat|nested] 18+ messages in thread
* [PATCH 04/13] am: convert applypatch to use hook.h
2021-10-12 13:30 [PATCH 00/13] hook.[ch]: new library to run hooks + simple hook conversion Ævar Arnfjörð Bjarmason
` (2 preceding siblings ...)
2021-10-12 13:30 ` [PATCH 03/13] rebase: convert pre-rebase to use hook.h Ævar Arnfjörð Bjarmason
@ 2021-10-12 13:30 ` Ævar Arnfjörð Bjarmason
2021-10-12 13:30 ` [PATCH 05/13] hooks: convert 'post-checkout' hook to hook library Ævar Arnfjörð Bjarmason
` (8 subsequent siblings)
12 siblings, 0 replies; 18+ messages in thread
From: Ævar Arnfjörð Bjarmason @ 2021-10-12 13:30 UTC (permalink / raw)
To: git
Cc: Junio C Hamano, Phillip Wood, René Scharfe, Emily Shaffer,
Joachim Kuebart, Daniel Levin, Luke Diamand,
Ævar Arnfjörð Bjarmason
From: Emily Shaffer <emilyshaffer@google.com>
Teach pre-applypatch, post-applypatch, and applypatch-msg to use the
hook.h library instead of the run-command.h library.
Signed-off-by: Emily Shaffer <emilyshaffer@google.com>
Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
builtin/am.c | 8 +++++---
1 file changed, 5 insertions(+), 3 deletions(-)
diff --git a/builtin/am.c b/builtin/am.c
index 3527945a467..2cfe6451b6f 100644
--- a/builtin/am.c
+++ b/builtin/am.c
@@ -446,9 +446,11 @@ static void am_destroy(const struct am_state *state)
static int run_applypatch_msg_hook(struct am_state *state)
{
int ret;
+ struct run_hooks_opt opt = RUN_HOOKS_OPT_INIT;
assert(state->msg);
- ret = run_hook_le(NULL, "applypatch-msg", am_path(state, "final-commit"), NULL);
+ strvec_push(&opt.args, am_path(state, "final-commit"));
+ ret = run_hooks_oneshot("applypatch-msg", &opt);
if (!ret) {
FREE_AND_NULL(state->msg);
@@ -1609,7 +1611,7 @@ static void do_commit(const struct am_state *state)
const char *reflog_msg, *author, *committer = NULL;
struct strbuf sb = STRBUF_INIT;
- if (run_hook_le(NULL, "pre-applypatch", NULL))
+ if (run_hooks_oneshot("pre-applypatch", NULL))
exit(1);
if (write_cache_as_tree(&tree, 0, NULL))
@@ -1661,7 +1663,7 @@ static void do_commit(const struct am_state *state)
fclose(fp);
}
- run_hook_le(NULL, "post-applypatch", NULL);
+ run_hooks_oneshot("post-applypatch", NULL);
strbuf_release(&sb);
}
--
2.33.0.1567.g7b23ce7ed9e
^ permalink raw reply related [flat|nested] 18+ messages in thread
* [PATCH 05/13] hooks: convert 'post-checkout' hook to hook library
2021-10-12 13:30 [PATCH 00/13] hook.[ch]: new library to run hooks + simple hook conversion Ævar Arnfjörð Bjarmason
` (3 preceding siblings ...)
2021-10-12 13:30 ` [PATCH 04/13] am: convert applypatch " Ævar Arnfjörð Bjarmason
@ 2021-10-12 13:30 ` Ævar Arnfjörð Bjarmason
2021-10-12 13:30 ` [PATCH 06/13] merge: convert post-merge to use hook.h Ævar Arnfjörð Bjarmason
` (7 subsequent siblings)
12 siblings, 0 replies; 18+ messages in thread
From: Ævar Arnfjörð Bjarmason @ 2021-10-12 13:30 UTC (permalink / raw)
To: git
Cc: Junio C Hamano, Phillip Wood, René Scharfe, Emily Shaffer,
Joachim Kuebart, Daniel Levin, Luke Diamand,
Ævar Arnfjörð Bjarmason
From: Emily Shaffer <emilyshaffer@google.com>
Move the running of the 'post-checkout' hook away from run-command.h
to the new hook.h library. For "worktree" this requires a change to it
to run the hooks from a given directory.
We could strictly speaking skip the "absolute_path" flag and just
check if "dir" is specified, but let's split them up for clarity, as
well as for any future user who'd like to set "dir" but not implicitly
change the argument to an absolute path.
Signed-off-by: Emily Shaffer <emilyshaffer@google.com>
Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
builtin/checkout.c | 14 +++++++++-----
builtin/clone.c | 7 +++++--
builtin/worktree.c | 28 ++++++++++++----------------
hook.c | 9 +++++++++
hook.h | 9 +++++++++
read-cache.c | 1 +
reset.c | 14 ++++++++++----
7 files changed, 55 insertions(+), 27 deletions(-)
diff --git a/builtin/checkout.c b/builtin/checkout.c
index 8c69dcdf72a..555119cc322 100644
--- a/builtin/checkout.c
+++ b/builtin/checkout.c
@@ -9,6 +9,7 @@
#include "config.h"
#include "diff.h"
#include "dir.h"
+#include "hook.h"
#include "ll-merge.h"
#include "lockfile.h"
#include "merge-recursive.h"
@@ -106,13 +107,16 @@ struct branch_info {
static int post_checkout_hook(struct commit *old_commit, struct commit *new_commit,
int changed)
{
- return run_hook_le(NULL, "post-checkout",
- oid_to_hex(old_commit ? &old_commit->object.oid : null_oid()),
- oid_to_hex(new_commit ? &new_commit->object.oid : null_oid()),
- changed ? "1" : "0", NULL);
+ struct run_hooks_opt opt = RUN_HOOKS_OPT_INIT;
+
/* "new_commit" can be NULL when checking out from the index before
a commit exists. */
-
+ strvec_pushl(&opt.args,
+ oid_to_hex(old_commit ? &old_commit->object.oid : null_oid()),
+ oid_to_hex(new_commit ? &new_commit->object.oid : null_oid()),
+ changed ? "1" : "0",
+ NULL);
+ return run_hooks_oneshot("post-checkout", &opt);
}
static int update_some(const struct object_id *oid, struct strbuf *base,
diff --git a/builtin/clone.c b/builtin/clone.c
index c9d4ca26640..4b356770739 100644
--- a/builtin/clone.c
+++ b/builtin/clone.c
@@ -32,6 +32,7 @@
#include "connected.h"
#include "packfile.h"
#include "list-objects-filter-options.h"
+#include "hook.h"
/*
* Overall FIXMEs:
@@ -659,6 +660,7 @@ static int checkout(int submodule_progress)
struct tree *tree;
struct tree_desc t;
int err = 0;
+ struct run_hooks_opt hook_opt = RUN_HOOKS_OPT_INIT;
if (option_no_checkout)
return 0;
@@ -704,8 +706,9 @@ static int checkout(int submodule_progress)
if (write_locked_index(&the_index, &lock_file, COMMIT_LOCK))
die(_("unable to write new index file"));
- err |= run_hook_le(NULL, "post-checkout", oid_to_hex(null_oid()),
- oid_to_hex(&oid), "1", NULL);
+ strvec_pushl(&hook_opt.args, oid_to_hex(null_oid()), oid_to_hex(&oid),
+ "1", NULL);
+ err |= run_hooks_oneshot("post-checkout", &hook_opt);
if (!err && (option_recurse_submodules.nr > 0)) {
struct strvec args = STRVEC_INIT;
diff --git a/builtin/worktree.c b/builtin/worktree.c
index d22ece93e1a..330867c19bf 100644
--- a/builtin/worktree.c
+++ b/builtin/worktree.c
@@ -382,22 +382,18 @@ static int add_worktree(const char *path, const char *refname,
* is_junk is cleared, but do return appropriate code when hook fails.
*/
if (!ret && opts->checkout) {
- const char *hook = find_hook("post-checkout");
- if (hook) {
- const char *env[] = { "GIT_DIR", "GIT_WORK_TREE", NULL };
- cp.git_cmd = 0;
- cp.no_stdin = 1;
- cp.stdout_to_stderr = 1;
- cp.dir = path;
- cp.env = env;
- cp.argv = NULL;
- cp.trace2_hook_name = "post-checkout";
- strvec_pushl(&cp.args, absolute_path(hook),
- oid_to_hex(null_oid()),
- oid_to_hex(&commit->object.oid),
- "1", NULL);
- ret = run_command(&cp);
- }
+ struct run_hooks_opt opt = RUN_HOOKS_OPT_INIT;
+
+ strvec_pushl(&opt.env, "GIT_DIR", "GIT_WORK_TREE", NULL);
+ strvec_pushl(&opt.args,
+ oid_to_hex(null_oid()),
+ oid_to_hex(&commit->object.oid),
+ "1",
+ NULL);
+ opt.dir = path;
+ opt.absolute_path = 1;
+
+ ret = run_hooks_oneshot("post-checkout", &opt);
}
strvec_clear(&child_env);
diff --git a/hook.c b/hook.c
index bfdc79e2f1a..186c249bbec 100644
--- a/hook.c
+++ b/hook.c
@@ -63,6 +63,7 @@ static int pick_next_hook(struct child_process *cp,
cp->env = hook_cb->options->env.v;
cp->stdout_to_stderr = 1;
cp->trace2_hook_name = hook_cb->hook_name;
+ cp->dir = hook_cb->options->dir;
/* add command */
strvec_push(&cp->args, run_me->hook_path);
@@ -116,6 +117,7 @@ static int notify_hook_finished(int result,
int run_hooks(const char *hook_name, const char *hook_path,
struct run_hooks_opt *options)
{
+ struct strbuf abs_path = STRBUF_INIT;
struct hook my_hook = {
.hook_path = hook_path,
};
@@ -129,6 +131,10 @@ int run_hooks(const char *hook_name, const char *hook_path,
if (!options)
BUG("a struct run_hooks_opt must be provided to run_hooks");
+ if (options->absolute_path) {
+ strbuf_add_absolute_path(&abs_path, hook_path);
+ my_hook.hook_path = abs_path.buf;
+ }
cb_data.run_me = &my_hook;
run_processes_parallel_tr2(jobs,
@@ -139,6 +145,9 @@ int run_hooks(const char *hook_name, const char *hook_path,
"hook",
hook_name);
+ if (options->absolute_path)
+ strbuf_release(&abs_path);
+
return cb_data.rc;
}
diff --git a/hook.h b/hook.h
index a2b8d4fc6bd..7a858a6beed 100644
--- a/hook.h
+++ b/hook.h
@@ -14,6 +14,15 @@ struct run_hooks_opt
/* Args to be passed to each hook */
struct strvec args;
+
+ /*
+ * Resolve and run the "absolute_path(hook)" instead of
+ * "hook". Used for "git worktree" hooks
+ */
+ int absolute_path;
+
+ /* Path to initial working directory for subprocess */
+ const char *dir;
};
#define RUN_HOOKS_OPT_INIT { \
diff --git a/read-cache.c b/read-cache.c
index a78b88a41bf..9773118d078 100644
--- a/read-cache.c
+++ b/read-cache.c
@@ -28,6 +28,7 @@
#include "sparse-index.h"
#include "csum-file.h"
#include "promisor-remote.h"
+#include "hook.h"
/* Mask for the name length in ce_flags in the on-disk index */
diff --git a/reset.c b/reset.c
index 79310ae071b..1237ced8a58 100644
--- a/reset.c
+++ b/reset.c
@@ -7,6 +7,7 @@
#include "tree-walk.h"
#include "tree.h"
#include "unpack-trees.h"
+#include "hook.h"
int reset_head(struct repository *r, struct object_id *oid, const char *action,
const char *switch_to_branch, unsigned flags,
@@ -125,10 +126,15 @@ int reset_head(struct repository *r, struct object_id *oid, const char *action,
ret = create_symref("HEAD", switch_to_branch,
reflog_head);
}
- if (run_hook)
- run_hook_le(NULL, "post-checkout",
- oid_to_hex(orig ? orig : null_oid()),
- oid_to_hex(oid), "1", NULL);
+ if (run_hook) {
+ struct run_hooks_opt opt = RUN_HOOKS_OPT_INIT;
+ strvec_pushl(&opt.args,
+ oid_to_hex(orig ? orig : null_oid()),
+ oid_to_hex(oid),
+ "1",
+ NULL);
+ run_hooks_oneshot("post-checkout", &opt);
+ }
leave_reset_head:
strbuf_release(&msg);
--
2.33.0.1567.g7b23ce7ed9e
^ permalink raw reply related [flat|nested] 18+ messages in thread
* [PATCH 06/13] merge: convert post-merge to use hook.h
2021-10-12 13:30 [PATCH 00/13] hook.[ch]: new library to run hooks + simple hook conversion Ævar Arnfjörð Bjarmason
` (4 preceding siblings ...)
2021-10-12 13:30 ` [PATCH 05/13] hooks: convert 'post-checkout' hook to hook library Ævar Arnfjörð Bjarmason
@ 2021-10-12 13:30 ` Ævar Arnfjörð Bjarmason
2021-10-12 13:30 ` [PATCH 07/13] git hook run: add an --ignore-missing flag Ævar Arnfjörð Bjarmason
` (6 subsequent siblings)
12 siblings, 0 replies; 18+ messages in thread
From: Ævar Arnfjörð Bjarmason @ 2021-10-12 13:30 UTC (permalink / raw)
To: git
Cc: Junio C Hamano, Phillip Wood, René Scharfe, Emily Shaffer,
Joachim Kuebart, Daniel Levin, Luke Diamand,
Ævar Arnfjörð Bjarmason
From: Emily Shaffer <emilyshaffer@google.com>
Teach post-merge to use the hook.h library instead of the
run-command.h library to run hooks.
Signed-off-by: Emily Shaffer <emilyshaffer@google.com>
Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
builtin/merge.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/builtin/merge.c b/builtin/merge.c
index 956b6259f21..167d13bd737 100644
--- a/builtin/merge.c
+++ b/builtin/merge.c
@@ -448,6 +448,7 @@ static void finish(struct commit *head_commit,
const struct object_id *new_head, const char *msg)
{
struct strbuf reflog_message = STRBUF_INIT;
+ struct run_hooks_opt opt = RUN_HOOKS_OPT_INIT;
const struct object_id *head = &head_commit->object.oid;
if (!msg)
@@ -488,7 +489,8 @@ static void finish(struct commit *head_commit,
}
/* Run a post-merge hook */
- run_hook_le(NULL, "post-merge", squash ? "1" : "0", NULL);
+ strvec_push(&opt.args, squash ? "1" : "0");
+ run_hooks_oneshot("post-merge", &opt);
apply_autostash(git_path_merge_autostash(the_repository));
strbuf_release(&reflog_message);
--
2.33.0.1567.g7b23ce7ed9e
^ permalink raw reply related [flat|nested] 18+ messages in thread
* [PATCH 07/13] git hook run: add an --ignore-missing flag
2021-10-12 13:30 [PATCH 00/13] hook.[ch]: new library to run hooks + simple hook conversion Ævar Arnfjörð Bjarmason
` (5 preceding siblings ...)
2021-10-12 13:30 ` [PATCH 06/13] merge: convert post-merge to use hook.h Ævar Arnfjörð Bjarmason
@ 2021-10-12 13:30 ` Ævar Arnfjörð Bjarmason
2021-10-12 13:30 ` [PATCH 08/13] send-email: use 'git hook run' for 'sendemail-validate' Ævar Arnfjörð Bjarmason
` (5 subsequent siblings)
12 siblings, 0 replies; 18+ messages in thread
From: Ævar Arnfjörð Bjarmason @ 2021-10-12 13:30 UTC (permalink / raw)
To: git
Cc: Junio C Hamano, Phillip Wood, René Scharfe, Emily Shaffer,
Joachim Kuebart, Daniel Levin, Luke Diamand,
Ævar Arnfjörð Bjarmason
For certain one-shot hooks we'd like to optimistically run them, and
not complain if they don't exist. This will be used by send-email in a
subsequent commit.
Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
Documentation/git-hook.txt | 10 +++++++++-
builtin/hook.c | 10 ++++++++--
t/t1800-hook.sh | 5 +++++
3 files changed, 22 insertions(+), 3 deletions(-)
diff --git a/Documentation/git-hook.txt b/Documentation/git-hook.txt
index 660d6a992a0..097fb9de63b 100644
--- a/Documentation/git-hook.txt
+++ b/Documentation/git-hook.txt
@@ -8,7 +8,7 @@ git-hook - run git hooks
SYNOPSIS
--------
[verse]
-'git hook' run <hook-name> [-- <hook-args>]
+'git hook' run [--ignore-missing] <hook-name> [-- <hook-args>]
DESCRIPTION
-----------
@@ -29,6 +29,14 @@ optional `--` (or `--end-of-options`, see linkgit:gitcli[7]). The
arguments (if any) differ by hook name, see linkgit:githooks[5] for
what those are.
+OPTIONS
+-------
+
+--ignore-missing::
+ Ignore any missing hook by quietly returning zero. Used for
+ tools that want to do a blind one-shot run of a hook that may
+ or may not be present.
+
SEE ALSO
--------
linkgit:githooks[5]
diff --git a/builtin/hook.c b/builtin/hook.c
index 012a2973b38..e6b01a7f5c6 100644
--- a/builtin/hook.c
+++ b/builtin/hook.c
@@ -7,7 +7,7 @@
#include "strvec.h"
#define BUILTIN_HOOK_RUN_USAGE \
- N_("git hook run <hook-name> [-- <hook-args>]")
+ N_("git hook run [--ignore-missing] <hook-name> [-- <hook-args>]")
static const char * const builtin_hook_usage[] = {
BUILTIN_HOOK_RUN_USAGE,
@@ -23,9 +23,12 @@ static int run(int argc, const char **argv, const char *prefix)
{
int i;
struct run_hooks_opt opt = RUN_HOOKS_OPT_INIT;
+ int ignore_missing = 0;
const char *hook_name;
const char *hook_path;
struct option run_options[] = {
+ OPT_BOOL(0, "ignore-missing", &ignore_missing,
+ N_("silently ignore missing requested <hook-name>")),
OPT_END(),
};
int ret;
@@ -55,9 +58,12 @@ static int run(int argc, const char **argv, const char *prefix)
/*
* We are not using a plain run_hooks() because we'd like to
* detect missing hooks. Let's find it ourselves and call
- * run_hooks() instead.
+ * run_hooks() instead...
*/
hook_name = argv[0];
+ if (ignore_missing)
+ /* ... act like a plain run_hooks() under --ignore-missing */
+ return run_hooks_oneshot(hook_name, &opt);
hook_path = find_hook(hook_name);
if (!hook_path) {
error("cannot find a hook named %s", hook_name);
diff --git a/t/t1800-hook.sh b/t/t1800-hook.sh
index 3aea1b105f0..29718aa9913 100755
--- a/t/t1800-hook.sh
+++ b/t/t1800-hook.sh
@@ -21,6 +21,11 @@ test_expect_success 'git hook run: nonexistent hook' '
test_cmp stderr.expect stderr.actual
'
+test_expect_success 'git hook run: nonexistent hook with --ignore-missing' '
+ git hook run --ignore-missing does-not-exist 2>stderr.actual &&
+ test_must_be_empty stderr.actual
+'
+
test_expect_success 'git hook run: basic' '
write_script .git/hooks/test-hook <<-EOF &&
echo Test hook
--
2.33.0.1567.g7b23ce7ed9e
^ permalink raw reply related [flat|nested] 18+ messages in thread
* [PATCH 08/13] send-email: use 'git hook run' for 'sendemail-validate'
2021-10-12 13:30 [PATCH 00/13] hook.[ch]: new library to run hooks + simple hook conversion Ævar Arnfjörð Bjarmason
` (6 preceding siblings ...)
2021-10-12 13:30 ` [PATCH 07/13] git hook run: add an --ignore-missing flag Ævar Arnfjörð Bjarmason
@ 2021-10-12 13:30 ` Ævar Arnfjörð Bjarmason
2021-10-12 13:30 ` [PATCH 09/13] git-p4: use 'git hook' to run hooks Ævar Arnfjörð Bjarmason
` (4 subsequent siblings)
12 siblings, 0 replies; 18+ messages in thread
From: Ævar Arnfjörð Bjarmason @ 2021-10-12 13:30 UTC (permalink / raw)
To: git
Cc: Junio C Hamano, Phillip Wood, René Scharfe, Emily Shaffer,
Joachim Kuebart, Daniel Levin, Luke Diamand,
Ævar Arnfjörð Bjarmason
From: Emily Shaffer <emilyshaffer@google.com>
Change the "sendmail-validate" hook to be run via the "git hook run"
wrapper instead of via a direct invocation.
This is the smallest possibly change to get "send-email" using "git
hook run". We still check the hook itself with "-x", and set a
"GIT_DIR" variable, both of which are asserted by our tests. We'll
need to get rid of this special behavior if we start running N hooks,
but for now let's be as close to bug-for-bug compatible as possible.
Signed-off-by: Emily Shaffer <emilyshaffer@google.com>
Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
git-send-email.perl | 22 ++++++++++++++--------
t/t9001-send-email.sh | 4 ++--
2 files changed, 16 insertions(+), 10 deletions(-)
diff --git a/git-send-email.perl b/git-send-email.perl
index 5262d88ee32..4c20c0bbb79 100755
--- a/git-send-email.perl
+++ b/git-send-email.perl
@@ -195,13 +195,13 @@ sub format_2822_time {
my $editor;
sub system_or_msg {
- my ($args, $msg) = @_;
+ my ($args, $msg, $cmd_name) = @_;
system(@$args);
my $signalled = $? & 127;
my $exit_code = $? >> 8;
return unless $signalled or $exit_code;
- my @sprintf_args = ($args->[0], $exit_code);
+ my @sprintf_args = ($cmd_name ? $cmd_name : $args->[0], $exit_code);
if (defined $msg) {
# Quiet the 'redundant' warning category, except we
# need to support down to Perl 5.8, so we can't do a
@@ -2039,10 +2039,10 @@ sub validate_patch {
my ($fn, $xfer_encoding) = @_;
if ($repo) {
+ my $hook_name = 'sendemail-validate';
my $hooks_path = $repo->command_oneline('rev-parse', '--git-path', 'hooks');
require File::Spec;
- my $validate_hook = File::Spec->catfile($hooks_path,
- 'sendemail-validate');
+ my $validate_hook = File::Spec->catfile($hooks_path, $hook_name);
my $hook_error;
if (-x $validate_hook) {
require Cwd;
@@ -2052,13 +2052,19 @@ sub validate_patch {
chdir($repo->wc_path() or $repo->repo_path())
or die("chdir: $!");
local $ENV{"GIT_DIR"} = $repo->repo_path();
- $hook_error = system_or_msg([$validate_hook, $target]);
+ my @cmd = ("git", "hook", "run", "--ignore-missing",
+ $hook_name, "--");
+ my @cmd_msg = (@cmd, "<patch>");
+ my @cmd_run = (@cmd, $target);
+ $hook_error = system_or_msg(\@cmd_run, undef, "@cmd_msg");
chdir($cwd_save) or die("chdir: $!");
}
if ($hook_error) {
- die sprintf(__("fatal: %s: rejected by sendemail-validate hook\n" .
- "%s\n" .
- "warning: no patches were sent\n"), $fn, $hook_error);
+ $hook_error = sprintf(__("fatal: %s: rejected by %s hook\n" .
+ $hook_error . "\n" .
+ "warning: no patches were sent\n"),
+ $fn, $hook_name);
+ die $hook_error;
}
}
diff --git a/t/t9001-send-email.sh b/t/t9001-send-email.sh
index aa0c20499ba..84d0f40d76a 100755
--- a/t/t9001-send-email.sh
+++ b/t/t9001-send-email.sh
@@ -539,7 +539,7 @@ test_expect_success $PREREQ "--validate respects relative core.hooksPath path" '
test_path_is_file my-hooks.ran &&
cat >expect <<-EOF &&
fatal: longline.patch: rejected by sendemail-validate hook
- fatal: command '"'"'my-hooks/sendemail-validate'"'"' died with exit code 1
+ fatal: command '"'"'git hook run --ignore-missing sendemail-validate -- <patch>'"'"' died with exit code 1
warning: no patches were sent
EOF
test_cmp expect actual
@@ -558,7 +558,7 @@ test_expect_success $PREREQ "--validate respects absolute core.hooksPath path" '
test_path_is_file my-hooks.ran &&
cat >expect <<-EOF &&
fatal: longline.patch: rejected by sendemail-validate hook
- fatal: command '"'"'$hooks_path/sendemail-validate'"'"' died with exit code 1
+ fatal: command '"'"'git hook run --ignore-missing sendemail-validate -- <patch>'"'"' died with exit code 1
warning: no patches were sent
EOF
test_cmp expect actual
--
2.33.0.1567.g7b23ce7ed9e
^ permalink raw reply related [flat|nested] 18+ messages in thread
* [PATCH 09/13] git-p4: use 'git hook' to run hooks
2021-10-12 13:30 [PATCH 00/13] hook.[ch]: new library to run hooks + simple hook conversion Ævar Arnfjörð Bjarmason
` (7 preceding siblings ...)
2021-10-12 13:30 ` [PATCH 08/13] send-email: use 'git hook run' for 'sendemail-validate' Ævar Arnfjörð Bjarmason
@ 2021-10-12 13:30 ` Ævar Arnfjörð Bjarmason
2021-10-12 13:30 ` [PATCH 10/13] commit: convert {pre-commit,prepare-commit-msg} hook to hook.h Ævar Arnfjörð Bjarmason
` (3 subsequent siblings)
12 siblings, 0 replies; 18+ messages in thread
From: Ævar Arnfjörð Bjarmason @ 2021-10-12 13:30 UTC (permalink / raw)
To: git
Cc: Junio C Hamano, Phillip Wood, René Scharfe, Emily Shaffer,
Joachim Kuebart, Daniel Levin, Luke Diamand,
Ævar Arnfjörð Bjarmason
From: Emily Shaffer <emilyshaffer@google.com>
Instead of duplicating the behavior of run-command.h:run_hook_le() in
Python, we can directly call 'git hook run'. We emulate the existence
check with the --ignore-missing flag.
As this is the last hook execution in git.git to not go through "git
hook run" or the hook.[ch] library we can now be absolutely sure that
our assertion in hook.c that only hooks known by the generated (from
githooks(5)) hook-list.h are permitted.
Signed-off-by: Emily Shaffer <emilyshaffer@google.com>
Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
git-p4.py | 72 ++++++-------------------------------------------------
1 file changed, 7 insertions(+), 65 deletions(-)
diff --git a/git-p4.py b/git-p4.py
index 2b4500226aa..1f24cbf0bca 100755
--- a/git-p4.py
+++ b/git-p4.py
@@ -207,71 +207,13 @@ def decode_path(path):
return path
def run_git_hook(cmd, param=[]):
- """Execute a hook if the hook exists."""
- if verbose:
- sys.stderr.write("Looking for hook: %s\n" % cmd)
- sys.stderr.flush()
-
- hooks_path = gitConfig("core.hooksPath")
- if len(hooks_path) <= 0:
- hooks_path = os.path.join(os.environ["GIT_DIR"], "hooks")
-
- if not isinstance(param, list):
- param=[param]
-
- # resolve hook file name, OS depdenent
- hook_file = os.path.join(hooks_path, cmd)
- if platform.system() == 'Windows':
- if not os.path.isfile(hook_file):
- # look for the file with an extension
- files = glob.glob(hook_file + ".*")
- if not files:
- return True
- files.sort()
- hook_file = files.pop()
- while hook_file.upper().endswith(".SAMPLE"):
- # The file is a sample hook. We don't want it
- if len(files) > 0:
- hook_file = files.pop()
- else:
- return True
-
- if not os.path.isfile(hook_file) or not os.access(hook_file, os.X_OK):
- return True
-
- return run_hook_command(hook_file, param) == 0
-
-def run_hook_command(cmd, param):
- """Executes a git hook command
- cmd = the command line file to be executed. This can be
- a file that is run by OS association.
-
- param = a list of parameters to pass to the cmd command
-
- On windows, the extension is checked to see if it should
- be run with the Git for Windows Bash shell. If there
- is no file extension, the file is deemed a bash shell
- and will be handed off to sh.exe. Otherwise, Windows
- will be called with the shell to handle the file assocation.
-
- For non Windows operating systems, the file is called
- as an executable.
- """
- cli = [cmd] + param
- use_shell = False
- if platform.system() == 'Windows':
- (root,ext) = os.path.splitext(cmd)
- if ext == "":
- exe_path = os.environ.get("EXEPATH")
- if exe_path is None:
- exe_path = ""
- else:
- exe_path = os.path.join(exe_path, "bin")
- cli = [os.path.join(exe_path, "SH.EXE")] + cli
- else:
- use_shell = True
- return subprocess.call(cli, shell=use_shell)
-
+ """args are specified with -a <arg> -a <arg> -a <arg>"""
+ args = ['git', 'hook', 'run', '--ignore-missing', cmd]
+ if param:
+ args.append("--")
+ for p in param:
+ args.append(p)
+ return subprocess.call(args) == 0
def write_pipe(c, stdin):
if verbose:
--
2.33.0.1567.g7b23ce7ed9e
^ permalink raw reply related [flat|nested] 18+ messages in thread
* [PATCH 10/13] commit: convert {pre-commit,prepare-commit-msg} hook to hook.h
2021-10-12 13:30 [PATCH 00/13] hook.[ch]: new library to run hooks + simple hook conversion Ævar Arnfjörð Bjarmason
` (8 preceding siblings ...)
2021-10-12 13:30 ` [PATCH 09/13] git-p4: use 'git hook' to run hooks Ævar Arnfjörð Bjarmason
@ 2021-10-12 13:30 ` Ævar Arnfjörð Bjarmason
2021-10-12 13:30 ` [PATCH 11/13] read-cache: convert post-index-change to use hook.h Ævar Arnfjörð Bjarmason
` (2 subsequent siblings)
12 siblings, 0 replies; 18+ messages in thread
From: Ævar Arnfjörð Bjarmason @ 2021-10-12 13:30 UTC (permalink / raw)
To: git
Cc: Junio C Hamano, Phillip Wood, René Scharfe, Emily Shaffer,
Joachim Kuebart, Daniel Levin, Luke Diamand,
Ævar Arnfjörð Bjarmason
From: Emily Shaffer <emilyshaffer@google.com>
Move these hooks hook away from run-command.h to and over to the new
hook.h library.
Signed-off-by: Emily Shaffer <emilyshaffer@google.com>
Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
commit.c | 15 ++++++++-------
1 file changed, 8 insertions(+), 7 deletions(-)
diff --git a/commit.c b/commit.c
index 551de4903c1..4e7cbd7585d 100644
--- a/commit.c
+++ b/commit.c
@@ -21,6 +21,7 @@
#include "commit-reach.h"
#include "run-command.h"
#include "shallow.h"
+#include "hook.h"
static struct commit_extra_header *read_commit_extra_header_lines(const char *buf, size_t len, const char **);
@@ -1700,22 +1701,22 @@ size_t ignore_non_trailer(const char *buf, size_t len)
int run_commit_hook(int editor_is_used, const char *index_file,
const char *name, ...)
{
- struct strvec hook_env = STRVEC_INIT;
+ struct run_hooks_opt opt = RUN_HOOKS_OPT_INIT;
va_list args;
- int ret;
+ const char *arg;
- strvec_pushf(&hook_env, "GIT_INDEX_FILE=%s", index_file);
+ strvec_pushf(&opt.env, "GIT_INDEX_FILE=%s", index_file);
/*
* Let the hook know that no editor will be launched.
*/
if (!editor_is_used)
- strvec_push(&hook_env, "GIT_EDITOR=:");
+ strvec_push(&opt.env, "GIT_EDITOR=:");
va_start(args, name);
- ret = run_hook_ve(hook_env.v, name, args);
+ while ((arg = va_arg(args, const char *)))
+ strvec_push(&opt.args, arg);
va_end(args);
- strvec_clear(&hook_env);
- return ret;
+ return run_hooks_oneshot(name, &opt);
}
--
2.33.0.1567.g7b23ce7ed9e
^ permalink raw reply related [flat|nested] 18+ messages in thread
* [PATCH 11/13] read-cache: convert post-index-change to use hook.h
2021-10-12 13:30 [PATCH 00/13] hook.[ch]: new library to run hooks + simple hook conversion Ævar Arnfjörð Bjarmason
` (9 preceding siblings ...)
2021-10-12 13:30 ` [PATCH 10/13] commit: convert {pre-commit,prepare-commit-msg} hook to hook.h Ævar Arnfjörð Bjarmason
@ 2021-10-12 13:30 ` Ævar Arnfjörð Bjarmason
2021-10-12 13:30 ` [PATCH 12/13] receive-pack: convert push-to-checkout hook to hook.h Ævar Arnfjörð Bjarmason
2021-10-12 13:30 ` [PATCH 13/13] run-command: remove old run_hook_{le,ve}() hook API Ævar Arnfjörð Bjarmason
12 siblings, 0 replies; 18+ messages in thread
From: Ævar Arnfjörð Bjarmason @ 2021-10-12 13:30 UTC (permalink / raw)
To: git
Cc: Junio C Hamano, Phillip Wood, René Scharfe, Emily Shaffer,
Joachim Kuebart, Daniel Levin, Luke Diamand,
Ævar Arnfjörð Bjarmason
From: Emily Shaffer <emilyshaffer@google.com>
Move the post-index-change hook away from run-command.h to and over to
the new hook.h library.
This removes the last direct user of "run_hook_ve()" outside of
run-command.c ("run_hook_le()" still uses it). So we can make the
function static now. A subsequent commit will remove this code
entirely when "run_hook_le()" itself goes away.
Signed-off-by: Emily Shaffer <emilyshaffer@google.com>
Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
read-cache.c | 10 +++++++---
run-command.c | 2 +-
run-command.h | 1 -
3 files changed, 8 insertions(+), 5 deletions(-)
diff --git a/read-cache.c b/read-cache.c
index 9773118d078..84cc3d88196 100644
--- a/read-cache.c
+++ b/read-cache.c
@@ -3087,6 +3087,7 @@ static int do_write_locked_index(struct index_state *istate, struct lock_file *l
{
int ret;
int was_full = !istate->sparse_index;
+ struct run_hooks_opt hook_opt = RUN_HOOKS_OPT_INIT;
ret = convert_to_sparse(istate, 0);
@@ -3115,9 +3116,12 @@ static int do_write_locked_index(struct index_state *istate, struct lock_file *l
else
ret = close_lock_file_gently(lock);
- run_hook_le(NULL, "post-index-change",
- istate->updated_workdir ? "1" : "0",
- istate->updated_skipworktree ? "1" : "0", NULL);
+ strvec_pushl(&hook_opt.args,
+ istate->updated_workdir ? "1" : "0",
+ istate->updated_skipworktree ? "1" : "0",
+ NULL);
+ run_hooks_oneshot("post-index-change", &hook_opt);
+
istate->updated_workdir = 0;
istate->updated_skipworktree = 0;
diff --git a/run-command.c b/run-command.c
index e4862b8f46d..7a05aa73e3d 100644
--- a/run-command.c
+++ b/run-command.c
@@ -1323,7 +1323,7 @@ int async_with_fork(void)
#endif
}
-int run_hook_ve(const char *const *env, const char *name, va_list args)
+static int run_hook_ve(const char *const *env, const char *name, va_list args)
{
struct child_process hook = CHILD_PROCESS_INIT;
const char *p;
diff --git a/run-command.h b/run-command.h
index 5e544acf4bc..f06b39c50ee 100644
--- a/run-command.h
+++ b/run-command.h
@@ -239,7 +239,6 @@ int run_command(struct child_process *);
*/
LAST_ARG_MUST_BE_NULL
int run_hook_le(const char *const *env, const char *name, ...);
-int run_hook_ve(const char *const *env, const char *name, va_list args);
/*
* Trigger an auto-gc
--
2.33.0.1567.g7b23ce7ed9e
^ permalink raw reply related [flat|nested] 18+ messages in thread
* [PATCH 12/13] receive-pack: convert push-to-checkout hook to hook.h
2021-10-12 13:30 [PATCH 00/13] hook.[ch]: new library to run hooks + simple hook conversion Ævar Arnfjörð Bjarmason
` (10 preceding siblings ...)
2021-10-12 13:30 ` [PATCH 11/13] read-cache: convert post-index-change to use hook.h Ævar Arnfjörð Bjarmason
@ 2021-10-12 13:30 ` Ævar Arnfjörð Bjarmason
2021-10-12 13:30 ` [PATCH 13/13] run-command: remove old run_hook_{le,ve}() hook API Ævar Arnfjörð Bjarmason
12 siblings, 0 replies; 18+ messages in thread
From: Ævar Arnfjörð Bjarmason @ 2021-10-12 13:30 UTC (permalink / raw)
To: git
Cc: Junio C Hamano, Phillip Wood, René Scharfe, Emily Shaffer,
Joachim Kuebart, Daniel Levin, Luke Diamand,
Ævar Arnfjörð Bjarmason
From: Emily Shaffer <emilyshaffer@google.com>
Move the push-to-checkout hook away from run-command.h to and over to
the new hook.h library.
This removes the last direct user of run_hook_le(), so we could remove
that function now, but let's leave that to a follow-up cleanup commit.
Signed-off-by: Emily Shaffer <emilyshaffer@google.com>
Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
builtin/receive-pack.c | 7 +++++--
1 file changed, 5 insertions(+), 2 deletions(-)
diff --git a/builtin/receive-pack.c b/builtin/receive-pack.c
index 25cc0c907e1..22e7a431728 100644
--- a/builtin/receive-pack.c
+++ b/builtin/receive-pack.c
@@ -1435,9 +1435,12 @@ static const char *push_to_checkout(unsigned char *hash,
struct strvec *env,
const char *work_tree)
{
+ struct run_hooks_opt opt = RUN_HOOKS_OPT_INIT;
+
strvec_pushf(env, "GIT_WORK_TREE=%s", absolute_path(work_tree));
- if (run_hook_le(env->v, push_to_checkout_hook,
- hash_to_hex(hash), NULL))
+ strvec_pushv(&opt.env, env->v);
+ strvec_push(&opt.args, hash_to_hex(hash));
+ if (run_hooks_oneshot(push_to_checkout_hook, &opt))
return "push-to-checkout hook declined";
else
return NULL;
--
2.33.0.1567.g7b23ce7ed9e
^ permalink raw reply related [flat|nested] 18+ messages in thread
* [PATCH 13/13] run-command: remove old run_hook_{le,ve}() hook API
2021-10-12 13:30 [PATCH 00/13] hook.[ch]: new library to run hooks + simple hook conversion Ævar Arnfjörð Bjarmason
` (11 preceding siblings ...)
2021-10-12 13:30 ` [PATCH 12/13] receive-pack: convert push-to-checkout hook to hook.h Ævar Arnfjörð Bjarmason
@ 2021-10-12 13:30 ` Ævar Arnfjörð Bjarmason
12 siblings, 0 replies; 18+ messages in thread
From: Ævar Arnfjörð Bjarmason @ 2021-10-12 13:30 UTC (permalink / raw)
To: git
Cc: Junio C Hamano, Phillip Wood, René Scharfe, Emily Shaffer,
Joachim Kuebart, Daniel Levin, Luke Diamand,
Ævar Arnfjörð Bjarmason
From: Emily Shaffer <emilyshaffer@google.com>
The new hook.h library has replaced all run-command.h hook-related
functionality. So let's delete this dead code.
Signed-off-by: Emily Shaffer <emilyshaffer@google.com>
Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
run-command.c | 32 --------------------------------
run-command.h | 16 ----------------
2 files changed, 48 deletions(-)
diff --git a/run-command.c b/run-command.c
index 7a05aa73e3d..e39ce17d009 100644
--- a/run-command.c
+++ b/run-command.c
@@ -1323,38 +1323,6 @@ int async_with_fork(void)
#endif
}
-static int run_hook_ve(const char *const *env, const char *name, va_list args)
-{
- struct child_process hook = CHILD_PROCESS_INIT;
- const char *p;
-
- p = find_hook(name);
- if (!p)
- return 0;
-
- strvec_push(&hook.args, p);
- while ((p = va_arg(args, const char *)))
- strvec_push(&hook.args, p);
- hook.env = env;
- hook.no_stdin = 1;
- hook.stdout_to_stderr = 1;
- hook.trace2_hook_name = name;
-
- return run_command(&hook);
-}
-
-int run_hook_le(const char *const *env, const char *name, ...)
-{
- va_list args;
- int ret;
-
- va_start(args, name);
- ret = run_hook_ve(env, name, args);
- va_end(args);
-
- return ret;
-}
-
struct io_pump {
/* initialized by caller */
int fd;
diff --git a/run-command.h b/run-command.h
index f06b39c50ee..6d6c50fe0a9 100644
--- a/run-command.h
+++ b/run-command.h
@@ -224,22 +224,6 @@ int finish_command_in_signal(struct child_process *);
*/
int run_command(struct child_process *);
-/**
- * Run a hook.
- * The first argument is a pathname to an index file, or NULL
- * if the hook uses the default index file or no index is needed.
- * The second argument is the name of the hook.
- * The further arguments correspond to the hook arguments.
- * The last argument has to be NULL to terminate the arguments list.
- * If the hook does not exist or is not executable, the return
- * value will be zero.
- * If it is executable, the hook will be executed and the exit
- * status of the hook is returned.
- * On execution, .stdout_to_stderr and .no_stdin will be set.
- */
-LAST_ARG_MUST_BE_NULL
-int run_hook_le(const char *const *env, const char *name, ...);
-
/*
* Trigger an auto-gc
*/
--
2.33.0.1567.g7b23ce7ed9e
^ permalink raw reply related [flat|nested] 18+ messages in thread
* Re: [PATCH 01/13] hook: add 'run' subcommand
2021-10-12 13:30 ` [PATCH 01/13] hook: add 'run' subcommand Ævar Arnfjörð Bjarmason
@ 2021-10-12 17:35 ` René Scharfe
2021-10-13 12:58 ` Ævar Arnfjörð Bjarmason
2021-10-13 0:52 ` Bagas Sanjaya
1 sibling, 1 reply; 18+ messages in thread
From: René Scharfe @ 2021-10-12 17:35 UTC (permalink / raw)
To: Ævar Arnfjörð Bjarmason, git
Cc: Junio C Hamano, Phillip Wood, Emily Shaffer, Joachim Kuebart,
Daniel Levin, Luke Diamand
Am 12.10.21 um 15:30 schrieb Ævar Arnfjörð Bjarmason:
> From: Emily Shaffer <emilyshaffer@google.com>
>
> In order to enable hooks to be run as an external process, by a
> standalone Git command, or by tools which wrap Git, provide an external
> means to run all configured hook commands for a given hook event.
>
> Most of our hooks require more complex functionality than this, but
> let's start with the bare minimum required to support our simplest
> hooks.
>
> In terms of implementation the usage_with_options() and "goto usage"
> pattern here mirrors that of
> builtin/{commit-graph,multi-pack-index}.c.
>
> Signed-off-by: Emily Shaffer <emilyshaffer@google.com>
> Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
> ---
> .gitignore | 1 +
> Documentation/git-hook.txt | 38 +++++++++++
> Documentation/githooks.txt | 4 ++
> Makefile | 1 +
> builtin.h | 1 +
> builtin/hook.c | 90 ++++++++++++++++++++++++++
> command-list.txt | 1 +
> git.c | 1 +
> hook.c | 101 +++++++++++++++++++++++++++++
> hook.h | 39 +++++++++++
> t/t1800-hook.sh | 129 +++++++++++++++++++++++++++++++++++++
> 11 files changed, 406 insertions(+)
> create mode 100644 Documentation/git-hook.txt
> create mode 100644 builtin/hook.c
> create mode 100755 t/t1800-hook.sh
>
> diff --git a/.gitignore b/.gitignore
> index 6be9de41ae8..66189ca3cdc 100644
> --- a/.gitignore
> +++ b/.gitignore
> @@ -77,6 +77,7 @@
> /git-grep
> /git-hash-object
> /git-help
> +/git-hook
> /git-http-backend
> /git-http-fetch
> /git-http-push
> diff --git a/Documentation/git-hook.txt b/Documentation/git-hook.txt
> new file mode 100644
> index 00000000000..660d6a992a0
> --- /dev/null
> +++ b/Documentation/git-hook.txt
> @@ -0,0 +1,38 @@
> +git-hook(1)
> +===========
> +
> +NAME
> +----
> +git-hook - run git hooks
> +
> +SYNOPSIS
> +--------
> +[verse]
> +'git hook' run <hook-name> [-- <hook-args>]
> +
> +DESCRIPTION
> +-----------
> +
> +This command is an interface to git hooks (see linkgit:githooks[5]).
> +Currently it only provides a convenience wrapper for running hooks for
> +use by git itself. In the future it might gain other functionality.
As a man page reader I'm only interested in the present features of the
command up here. Breaking changes could be mentioned in a HISTORY
section, and missing critical features in a BUGS section at the bottom.
I assume the last sentence applies to almost all programs, and could be
safely removed.
> +
> +SUBCOMMANDS
> +-----------
> +
> +run::
> + Run the `<hook-name>` hook. See linkgit:githooks[5] for
> + the hook names we support.
> ++
> +Any positional arguments to the hook should be passed after an
> +optional `--` (or `--end-of-options`, see linkgit:gitcli[7]). The
Dash dash (or EoO) is optional.
> +arguments (if any) differ by hook name, see linkgit:githooks[5] for
> +what those are.
> +
> +SEE ALSO
> +--------
> +linkgit:githooks[5]
> +
> +GIT
> +---
> +Part of the linkgit:git[1] suite
> diff --git a/Documentation/githooks.txt b/Documentation/githooks.txt
> index b51959ff941..a16e62bc8c8 100644
> --- a/Documentation/githooks.txt
> +++ b/Documentation/githooks.txt
> @@ -698,6 +698,10 @@ and "0" meaning they were not.
> Only one parameter should be set to "1" when the hook runs. The hook
> running passing "1", "1" should not be possible.
>
> +SEE ALSO
> +--------
> +linkgit:git-hook[1]
> +
> GIT
> ---
> Part of the linkgit:git[1] suite
> diff --git a/Makefile b/Makefile
> index 877492386ee..12b481fdabe 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -1108,6 +1108,7 @@ BUILTIN_OBJS += builtin/get-tar-commit-id.o
> BUILTIN_OBJS += builtin/grep.o
> BUILTIN_OBJS += builtin/hash-object.o
> BUILTIN_OBJS += builtin/help.o
> +BUILTIN_OBJS += builtin/hook.o
> BUILTIN_OBJS += builtin/index-pack.o
> BUILTIN_OBJS += builtin/init-db.o
> BUILTIN_OBJS += builtin/interpret-trailers.o
> diff --git a/builtin.h b/builtin.h
> index 8a58743ed63..83379f3832c 100644
> --- a/builtin.h
> +++ b/builtin.h
> @@ -164,6 +164,7 @@ int cmd_get_tar_commit_id(int argc, const char **argv, const char *prefix);
> int cmd_grep(int argc, const char **argv, const char *prefix);
> int cmd_hash_object(int argc, const char **argv, const char *prefix);
> int cmd_help(int argc, const char **argv, const char *prefix);
> +int cmd_hook(int argc, const char **argv, const char *prefix);
> int cmd_index_pack(int argc, const char **argv, const char *prefix);
> int cmd_init_db(int argc, const char **argv, const char *prefix);
> int cmd_interpret_trailers(int argc, const char **argv, const char *prefix);
> diff --git a/builtin/hook.c b/builtin/hook.c
> new file mode 100644
> index 00000000000..012a2973b38
> --- /dev/null
> +++ b/builtin/hook.c
> @@ -0,0 +1,90 @@
> +#include "cache.h"
> +#include "builtin.h"
> +#include "config.h"
> +#include "hook.h"
> +#include "parse-options.h"
> +#include "strbuf.h"
> +#include "strvec.h"
> +
> +#define BUILTIN_HOOK_RUN_USAGE \
> + N_("git hook run <hook-name> [-- <hook-args>]")
> +
> +static const char * const builtin_hook_usage[] = {
> + BUILTIN_HOOK_RUN_USAGE,
> + NULL
> +};
> +
> +static const char * const builtin_hook_run_usage[] = {
> + BUILTIN_HOOK_RUN_USAGE,
> + NULL
> +};
> +
> +static int run(int argc, const char **argv, const char *prefix)
> +{
> + int i;
> + struct run_hooks_opt opt = RUN_HOOKS_OPT_INIT;
> + const char *hook_name;
> + const char *hook_path;
> + struct option run_options[] = {
> + OPT_END(),
> + };
> + int ret;
> +
> + argc = parse_options(argc, argv, prefix, run_options,
> + builtin_hook_run_usage,
> + PARSE_OPT_KEEP_DASHDASH);
> +
> + if (!argc)
> + goto usage;
> +
> + /*
> + * Having a -- for "run" when providing <hook-args> is
> + * mandatory.
> + */
> + if (argc > 1 && strcmp(argv[1], "--") &&
> + strcmp(argv[1], "--end-of-options"))
> + goto usage;
Dash dash (or EoO) is mandatory unless parse_options() left no
arguments, contrary to what the documentation says above. Update
the doc above?
> +
> + /* Add our arguments, start after -- */
> + for (i = 2 ; i < argc; i++)
> + strvec_push(&opt.args, argv[i]);
> +
> + /* Need to take into account core.hooksPath */
> + git_config(git_default_config, NULL);
> +
> + /*
> + * We are not using a plain run_hooks() because we'd like to
> + * detect missing hooks. Let's find it ourselves and call
> + * run_hooks() instead.
So run_hooks(), which is introduced later in this patch, can find a
hook as well, but would do so silently?
> + */
> + hook_name = argv[0];
> + hook_path = find_hook(hook_name);
This is the only find_hook() call introduced by this patch.
Does run_hooks() really posses an unused hook-finding capability?
> + if (!hook_path) {
> + error("cannot find a hook named %s", hook_name);
> + return 1;
> + }
> +
> + ret = run_hooks(hook_name, hook_path, &opt);
> + run_hooks_opt_clear(&opt);
> + return ret;
> +usage:
> + usage_with_options(builtin_hook_run_usage, run_options);
> +}
> +
> +int cmd_hook(int argc, const char **argv, const char *prefix)
> +{
> + struct option builtin_hook_options[] = {
> + OPT_END(),
> + };
> +
> + argc = parse_options(argc, argv, NULL, builtin_hook_options,
> + builtin_hook_usage, PARSE_OPT_STOP_AT_NON_OPTION);
> + if (!argc)
> + goto usage;
> +
> + if (!strcmp(argv[0], "run"))
> + return run(argc, argv, prefix);
> +
> +usage:
> + usage_with_options(builtin_hook_usage, builtin_hook_options);
> +}
> diff --git a/command-list.txt b/command-list.txt
> index a289f09ed6f..9ccd8e5aebe 100644
> --- a/command-list.txt
> +++ b/command-list.txt
> @@ -103,6 +103,7 @@ git-grep mainporcelain info
> git-gui mainporcelain
> git-hash-object plumbingmanipulators
> git-help ancillaryinterrogators complete
> +git-hook mainporcelain
> git-http-backend synchingrepositories
> git-http-fetch synchelpers
> git-http-push synchelpers
> diff --git a/git.c b/git.c
> index 60c2784be45..e5891e02021 100644
> --- a/git.c
> +++ b/git.c
> @@ -538,6 +538,7 @@ static struct cmd_struct commands[] = {
> { "grep", cmd_grep, RUN_SETUP_GENTLY },
> { "hash-object", cmd_hash_object },
> { "help", cmd_help },
> + { "hook", cmd_hook, RUN_SETUP },
> { "index-pack", cmd_index_pack, RUN_SETUP_GENTLY | NO_PARSEOPT },
> { "init", cmd_init_db },
> { "init-db", cmd_init_db },
> diff --git a/hook.c b/hook.c
> index 55e1145a4b7..240270db64e 100644
> --- a/hook.c
> +++ b/hook.c
> @@ -1,6 +1,7 @@
> #include "cache.h"
> #include "hook.h"
> #include "run-command.h"
> +#include "config.h"
>
> const char *find_hook(const char *name)
> {
> @@ -40,3 +41,103 @@ int hook_exists(const char *name)
> {
> return !!find_hook(name);
> }
> +
> +void run_hooks_opt_clear(struct run_hooks_opt *o)
> +{
> + strvec_clear(&o->env);
> + strvec_clear(&o->args);
> +}
> +
> +static int pick_next_hook(struct child_process *cp,
> + struct strbuf *out,
> + void *pp_cb,
> + void **pp_task_cb)
> +{
> + struct hook_cb_data *hook_cb = pp_cb;
> + struct hook *run_me = hook_cb->run_me;
> +
> + if (!run_me)
> + return 0;
> +
> + cp->no_stdin = 1;
> + cp->env = hook_cb->options->env.v;
> + cp->stdout_to_stderr = 1;
> + cp->trace2_hook_name = hook_cb->hook_name;
> +
> + /* add command */
> + strvec_push(&cp->args, run_me->hook_path);
> +
> + /*
> + * add passed-in argv, without expanding - let the user get back
> + * exactly what they put in
What kind of expanding is it doing without? I was expecting the
arguments to passed on verbatim before reading this comment, so it
leaves me wondering what I'm missing.
> + */
> + strvec_pushv(&cp->args, hook_cb->options->args.v);
> +
> + /* Provide context for errors if necessary */
> + *pp_task_cb = run_me;
> +
> + /*
> + * This pick_next_hook() will be called again, we're only
> + * running one hook, so indicate that no more work will be
> + * done.
> + */
> + hook_cb->run_me = NULL;
A single hook is picked.
> +
> + return 1;
> +}
> +
> +static int notify_start_failure(struct strbuf *out,
> + void *pp_cb,
> + void *pp_task_cp)
> +{
> + struct hook_cb_data *hook_cb = pp_cb;
> + struct hook *attempted = pp_task_cp;
> +
> + hook_cb->rc |= 1;
> +
> + strbuf_addf(out, _("Couldn't start hook '%s'\n"),
> + attempted->hook_path);
> +
> + return 1;
> +}
> +
> +static int notify_hook_finished(int result,
> + struct strbuf *out,
> + void *pp_cb,
> + void *pp_task_cb)
> +{
> + struct hook_cb_data *hook_cb = pp_cb;
> +
> + hook_cb->rc |= result;
> +
> + return 0;
> +}
> +
> +int run_hooks(const char *hook_name, const char *hook_path,
> + struct run_hooks_opt *options)
> +{
> + struct hook my_hook = {
> + .hook_path = hook_path,
> + };
> + struct hook_cb_data cb_data = {
> + .rc = 0,
> + .hook_name = hook_name,
> + .options = options,
> + };
> + int jobs = 1;
> +
> + if (!options)
> + BUG("a struct run_hooks_opt must be provided to run_hooks");
> +
> + cb_data.run_me = &my_hook;
> +
> + run_processes_parallel_tr2(jobs,
> + pick_next_hook,
> + notify_start_failure,
> + notify_hook_finished,
> + &cb_data,
> + "hook",
> + hook_name);
A single process (jobs == 1) is used to run a single hook. Why use
run_processes_parallel_tr2() for that instead of run_command()? The
latter would require a lot less code to achieve the same outcome, no?
> +
> + return cb_data.rc;
> +}
> diff --git a/hook.h b/hook.h
> index 6aa36fc7ff9..111c5cf9296 100644
> --- a/hook.h
> +++ b/hook.h
> @@ -1,5 +1,33 @@
> #ifndef HOOK_H
> #define HOOK_H
> +#include "strvec.h"
> +
> +struct hook {
> + /* The path to the hook */
> + const char *hook_path;
> +};
None of the patches in this series extend this structure. Why not
use a bare string directly?
> +
> +struct run_hooks_opt
> +{
> + /* Environment vars to be set for each hook */
> + struct strvec env;
> +
> + /* Args to be passed to each hook */
> + struct strvec args;
> +};
> +
> +#define RUN_HOOKS_OPT_INIT { \
> + .env = STRVEC_INIT, \
> + .args = STRVEC_INIT, \
> +}
> +
> +struct hook_cb_data {
> + /* rc reflects the cumulative failure state */
> + int rc;
> + const char *hook_name;
> + struct hook *run_me;
> + struct run_hooks_opt *options;
> +};
>
> /*
> * Returns the path to the hook file, or NULL if the hook is missing
> @@ -13,4 +41,15 @@ const char *find_hook(const char *name);
> */
> int hook_exists(const char *hookname);
>
> +/**
> + * Clear data from an initialized "struct run_hooks-opt".
^
y/-/_/
> + */
> +void run_hooks_opt_clear(struct run_hooks_opt *o);
> +
> +/**
> + * Takes an already resolved hook found via find_hook() and runs
> + * it. Does not call run_hooks_opt_clear() for you.
> + */
> +int run_hooks(const char *hookname, const char *hook_path,
> + struct run_hooks_opt *options);
If it runs one hook, why is it called run_hooks(), plural?
> #endif
> diff --git a/t/t1800-hook.sh b/t/t1800-hook.sh
> new file mode 100755
> index 00000000000..3aea1b105f0
> --- /dev/null
> +++ b/t/t1800-hook.sh
> @@ -0,0 +1,129 @@
> +#!/bin/sh
> +
> +test_description='git-hook command'
> +
> +TEST_PASSES_SANITIZE_LEAK=true
> +. ./test-lib.sh
> +
> +test_expect_success 'git hook usage' '
> + test_expect_code 129 git hook &&
> + test_expect_code 129 git hook run &&
> + test_expect_code 129 git hook run -h &&
> + test_expect_code 129 git hook run --unknown 2>err &&
> + grep "unknown option" err
> +'
> +
> +test_expect_success 'git hook run: nonexistent hook' '
> + cat >stderr.expect <<-\EOF &&
> + error: cannot find a hook named test-hook
> + EOF
> + test_expect_code 1 git hook run test-hook 2>stderr.actual &&
> + test_cmp stderr.expect stderr.actual
> +'
> +
> +test_expect_success 'git hook run: basic' '
> + write_script .git/hooks/test-hook <<-EOF &&
> + echo Test hook
> + EOF
> +
> + cat >expect <<-\EOF &&
> + Test hook
> + EOF
> + git hook run test-hook 2>actual &&
> + test_cmp expect actual
> +'
> +
> +test_expect_success 'git hook run: stdout and stderr both write to our stderr' '
> + write_script .git/hooks/test-hook <<-EOF &&
> + echo >&1 Will end up on stderr
> + echo >&2 Will end up on stderr
> + EOF
> +
> + cat >stderr.expect <<-\EOF &&
> + Will end up on stderr
> + Will end up on stderr
> + EOF
> + git hook run test-hook >stdout.actual 2>stderr.actual &&
> + test_cmp stderr.expect stderr.actual &&
> + test_must_be_empty stdout.actual
> +'
> +
> +test_expect_success 'git hook run: exit codes are passed along' '
> + write_script .git/hooks/test-hook <<-EOF &&
> + exit 1
> + EOF
> +
> + test_expect_code 1 git hook run test-hook &&
> +
> + write_script .git/hooks/test-hook <<-EOF &&
> + exit 2
> + EOF
> +
> + test_expect_code 2 git hook run test-hook &&
> +
> + write_script .git/hooks/test-hook <<-EOF &&
> + exit 128
> + EOF
> +
> + test_expect_code 128 git hook run test-hook &&
> +
> + write_script .git/hooks/test-hook <<-EOF &&
> + exit 129
> + EOF
> +
> + test_expect_code 129 git hook run test-hook
> +'
> +
> +test_expect_success 'git hook run arg u ments without -- is not allowed' '
> + test_expect_code 129 git hook run test-hook arg u ments
> +'
> +
> +test_expect_success 'git hook run -- pass arguments' '
> + write_script .git/hooks/test-hook <<-\EOF &&
> + echo $1
> + echo $2
> + EOF
> +
> + cat >expect <<-EOF &&
> + arg
> + u ments
> + EOF
> +
> + git hook run test-hook -- arg "u ments" 2>actual &&
> + test_cmp expect actual
> +'
> +
> +test_expect_success 'git hook run -- out-of-repo runs excluded' '
> + write_script .git/hooks/test-hook <<-EOF &&
> + echo Test hook
> + EOF
> +
> + nongit test_must_fail git hook run test-hook
> +'
> +
> +test_expect_success 'git -c core.hooksPath=<PATH> hook run' '
> + mkdir my-hooks &&
> + write_script my-hooks/test-hook <<-\EOF &&
> + echo Hook ran $1 >>actual
> + EOF
> +
> + cat >expect <<-\EOF &&
> + Test hook
> + Hook ran one
> + Hook ran two
> + Hook ran three
> + Hook ran four
> + EOF
> +
> + # Test various ways of specifying the path. See also
> + # t1350-config-hooks-path.sh
> + >actual &&
> + git hook run test-hook -- ignored 2>>actual &&
> + git -c core.hooksPath=my-hooks hook run test-hook -- one 2>>actual &&
> + git -c core.hooksPath=my-hooks/ hook run test-hook -- two 2>>actual &&
> + git -c core.hooksPath="$PWD/my-hooks" hook run test-hook -- three 2>>actual &&
> + git -c core.hooksPath="$PWD/my-hooks/" hook run test-hook -- four 2>>actual &&
> + test_cmp expect actual
> +'
> +
> +test_done
>
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 01/13] hook: add 'run' subcommand
2021-10-12 13:30 ` [PATCH 01/13] hook: add 'run' subcommand Ævar Arnfjörð Bjarmason
2021-10-12 17:35 ` René Scharfe
@ 2021-10-13 0:52 ` Bagas Sanjaya
2021-10-13 13:04 ` Ævar Arnfjörð Bjarmason
1 sibling, 1 reply; 18+ messages in thread
From: Bagas Sanjaya @ 2021-10-13 0:52 UTC (permalink / raw)
To: Ævar Arnfjörð Bjarmason, git
Cc: Junio C Hamano, Phillip Wood, René Scharfe, Emily Shaffer,
Joachim Kuebart, Daniel Levin, Luke Diamand
On 12/10/21 20.30, Ævar Arnfjörð Bjarmason wrote:
> +run::
> + Run the `<hook-name>` hook. See linkgit:githooks[5] for
> + the hook names we support.
> ++
Drop the first person, so it become `list of supported hooks`.
> +Any positional arguments to the hook should be passed after an
> +optional `--` (or `--end-of-options`, see linkgit:gitcli[7]). The
> +arguments (if any) differ by hook name, see linkgit:githooks[5] for
> +what those are.
> +
s/what those are/supported list of them/ (dunno?)
--
An old man doll... just what I always wanted! - Clara
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 01/13] hook: add 'run' subcommand
2021-10-12 17:35 ` René Scharfe
@ 2021-10-13 12:58 ` Ævar Arnfjörð Bjarmason
0 siblings, 0 replies; 18+ messages in thread
From: Ævar Arnfjörð Bjarmason @ 2021-10-13 12:58 UTC (permalink / raw)
To: René Scharfe
Cc: git, Junio C Hamano, Phillip Wood, Emily Shaffer, Joachim Kuebart,
Daniel Levin, Luke Diamand
On Tue, Oct 12 2021, René Scharfe wrote:
> Am 12.10.21 um 15:30 schrieb Ævar Arnfjörð Bjarmason:
>> From: Emily Shaffer <emilyshaffer@google.com>
>>
>> In order to enable hooks to be run as an external process, by a
>> standalone Git command, or by tools which wrap Git, provide an external
>> means to run all configured hook commands for a given hook event.
>>
>> Most of our hooks require more complex functionality than this, but
>> let's start with the bare minimum required to support our simplest
>> hooks.
>>
>> In terms of implementation the usage_with_options() and "goto usage"
>> pattern here mirrors that of
>> builtin/{commit-graph,multi-pack-index}.c.
>>
>> Signed-off-by: Emily Shaffer <emilyshaffer@google.com>
>> Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
>> ---
>> .gitignore | 1 +
>> Documentation/git-hook.txt | 38 +++++++++++
>> Documentation/githooks.txt | 4 ++
>> Makefile | 1 +
>> builtin.h | 1 +
>> builtin/hook.c | 90 ++++++++++++++++++++++++++
>> command-list.txt | 1 +
>> git.c | 1 +
>> hook.c | 101 +++++++++++++++++++++++++++++
>> hook.h | 39 +++++++++++
>> t/t1800-hook.sh | 129 +++++++++++++++++++++++++++++++++++++
>> 11 files changed, 406 insertions(+)
>> create mode 100644 Documentation/git-hook.txt
>> create mode 100644 builtin/hook.c
>> create mode 100755 t/t1800-hook.sh
>>
>> diff --git a/.gitignore b/.gitignore
>> index 6be9de41ae8..66189ca3cdc 100644
>> --- a/.gitignore
>> +++ b/.gitignore
>> @@ -77,6 +77,7 @@
>> /git-grep
>> /git-hash-object
>> /git-help
>> +/git-hook
>> /git-http-backend
>> /git-http-fetch
>> /git-http-push
>> diff --git a/Documentation/git-hook.txt b/Documentation/git-hook.txt
>> new file mode 100644
>> index 00000000000..660d6a992a0
>> --- /dev/null
>> +++ b/Documentation/git-hook.txt
>> @@ -0,0 +1,38 @@
>> +git-hook(1)
>> +===========
>> +
>> +NAME
>> +----
>> +git-hook - run git hooks
>> +
>> +SYNOPSIS
>> +--------
>> +[verse]
>> +'git hook' run <hook-name> [-- <hook-args>]
>> +
>> +DESCRIPTION
>> +-----------
>> +
>> +This command is an interface to git hooks (see linkgit:githooks[5]).
>> +Currently it only provides a convenience wrapper for running hooks for
>> +use by git itself. In the future it might gain other functionality.
>
> As a man page reader I'm only interested in the present features of the
> command up here. Breaking changes could be mentioned in a HISTORY
> section, and missing critical features in a BUGS section at the bottom.
>
> I assume the last sentence applies to almost all programs, and could be
> safely removed.
Willdo.
>> +
>> +SUBCOMMANDS
>> +-----------
>> +
>> +run::
>> + Run the `<hook-name>` hook. See linkgit:githooks[5] for
>> + the hook names we support.
>> ++
>> +Any positional arguments to the hook should be passed after an
>> +optional `--` (or `--end-of-options`, see linkgit:gitcli[7]). The
>
> Dash dash (or EoO) is optional.
This just needs to be rephrased, the -- is mandatory, i.e. it's either:
git hook run [args] hook-name
Or:
git hook run [args] hook-name -- [hook-args]
But not:
git hook run [args] hook-name [hook-args]
>> +arguments (if any) differ by hook name, see linkgit:githooks[5] for
>> +what those are.
>> +
>> +SEE ALSO
>> +--------
>> +linkgit:githooks[5]
>> +
>> +GIT
>> +---
>> +Part of the linkgit:git[1] suite
>> diff --git a/Documentation/githooks.txt b/Documentation/githooks.txt
>> index b51959ff941..a16e62bc8c8 100644
>> --- a/Documentation/githooks.txt
>> +++ b/Documentation/githooks.txt
>> @@ -698,6 +698,10 @@ and "0" meaning they were not.
>> Only one parameter should be set to "1" when the hook runs. The hook
>> running passing "1", "1" should not be possible.
>>
>> +SEE ALSO
>> +--------
>> +linkgit:git-hook[1]
>> +
>> GIT
>> ---
>> Part of the linkgit:git[1] suite
>> diff --git a/Makefile b/Makefile
>> index 877492386ee..12b481fdabe 100644
>> --- a/Makefile
>> +++ b/Makefile
>> @@ -1108,6 +1108,7 @@ BUILTIN_OBJS += builtin/get-tar-commit-id.o
>> BUILTIN_OBJS += builtin/grep.o
>> BUILTIN_OBJS += builtin/hash-object.o
>> BUILTIN_OBJS += builtin/help.o
>> +BUILTIN_OBJS += builtin/hook.o
>> BUILTIN_OBJS += builtin/index-pack.o
>> BUILTIN_OBJS += builtin/init-db.o
>> BUILTIN_OBJS += builtin/interpret-trailers.o
>> diff --git a/builtin.h b/builtin.h
>> index 8a58743ed63..83379f3832c 100644
>> --- a/builtin.h
>> +++ b/builtin.h
>> @@ -164,6 +164,7 @@ int cmd_get_tar_commit_id(int argc, const char **argv, const char *prefix);
>> int cmd_grep(int argc, const char **argv, const char *prefix);
>> int cmd_hash_object(int argc, const char **argv, const char *prefix);
>> int cmd_help(int argc, const char **argv, const char *prefix);
>> +int cmd_hook(int argc, const char **argv, const char *prefix);
>> int cmd_index_pack(int argc, const char **argv, const char *prefix);
>> int cmd_init_db(int argc, const char **argv, const char *prefix);
>> int cmd_interpret_trailers(int argc, const char **argv, const char *prefix);
>> diff --git a/builtin/hook.c b/builtin/hook.c
>> new file mode 100644
>> index 00000000000..012a2973b38
>> --- /dev/null
>> +++ b/builtin/hook.c
>> @@ -0,0 +1,90 @@
>> +#include "cache.h"
>> +#include "builtin.h"
>> +#include "config.h"
>> +#include "hook.h"
>> +#include "parse-options.h"
>> +#include "strbuf.h"
>> +#include "strvec.h"
>> +
>> +#define BUILTIN_HOOK_RUN_USAGE \
>> + N_("git hook run <hook-name> [-- <hook-args>]")
>> +
>> +static const char * const builtin_hook_usage[] = {
>> + BUILTIN_HOOK_RUN_USAGE,
>> + NULL
>> +};
>> +
>> +static const char * const builtin_hook_run_usage[] = {
>> + BUILTIN_HOOK_RUN_USAGE,
>> + NULL
>> +};
>> +
>> +static int run(int argc, const char **argv, const char *prefix)
>> +{
>> + int i;
>> + struct run_hooks_opt opt = RUN_HOOKS_OPT_INIT;
>> + const char *hook_name;
>> + const char *hook_path;
>> + struct option run_options[] = {
>> + OPT_END(),
>> + };
>> + int ret;
>> +
>> + argc = parse_options(argc, argv, prefix, run_options,
>> + builtin_hook_run_usage,
>> + PARSE_OPT_KEEP_DASHDASH);
>> +
>> + if (!argc)
>> + goto usage;
>> +
>> + /*
>> + * Having a -- for "run" when providing <hook-args> is
>> + * mandatory.
>> + */
>> + if (argc > 1 && strcmp(argv[1], "--") &&
>> + strcmp(argv[1], "--end-of-options"))
>> + goto usage;
> Dash dash (or EoO) is mandatory unless parse_options() left no
> arguments, contrary to what the documentation says above. Update
> the doc above?
*nod*
>> +
>> + /* Add our arguments, start after -- */
>> + for (i = 2 ; i < argc; i++)
>> + strvec_push(&opt.args, argv[i]);
>> +
>> + /* Need to take into account core.hooksPath */
>> + git_config(git_default_config, NULL);
>> +
>> + /*
>> + * We are not using a plain run_hooks() because we'd like to
>> + * detect missing hooks. Let's find it ourselves and call
>> + * run_hooks() instead.
>
> So run_hooks(), which is introduced later in this patch, can find a
> hook as well, but would do so silently?
Will clarify, I think Emily and I went back and forth on this comment a
bit, and at some point I removed it entirely I think...
>> + */
>> + hook_name = argv[0];
>> + hook_path = find_hook(hook_name);
>
> This is the only find_hook() call introduced by this patch.
> Does run_hooks() really posses an unused hook-finding capability?
Will address...
>> + if (!hook_path) {
>> + error("cannot find a hook named %s", hook_name);
>> + return 1;
>> + }
>> +
>> + ret = run_hooks(hook_name, hook_path, &opt);
>> + run_hooks_opt_clear(&opt);
>> + return ret;
>> +usage:
>> + usage_with_options(builtin_hook_run_usage, run_options);
>> +}
>> +
>> +int cmd_hook(int argc, const char **argv, const char *prefix)
>> +{
>> + struct option builtin_hook_options[] = {
>> + OPT_END(),
>> + };
>> +
>> + argc = parse_options(argc, argv, NULL, builtin_hook_options,
>> + builtin_hook_usage, PARSE_OPT_STOP_AT_NON_OPTION);
>> + if (!argc)
>> + goto usage;
>> +
>> + if (!strcmp(argv[0], "run"))
>> + return run(argc, argv, prefix);
>> +
>> +usage:
>> + usage_with_options(builtin_hook_usage, builtin_hook_options);
>> +}
>> diff --git a/command-list.txt b/command-list.txt
>> index a289f09ed6f..9ccd8e5aebe 100644
>> --- a/command-list.txt
>> +++ b/command-list.txt
>> @@ -103,6 +103,7 @@ git-grep mainporcelain info
>> git-gui mainporcelain
>> git-hash-object plumbingmanipulators
>> git-help ancillaryinterrogators complete
>> +git-hook mainporcelain
>> git-http-backend synchingrepositories
>> git-http-fetch synchelpers
>> git-http-push synchelpers
>> diff --git a/git.c b/git.c
>> index 60c2784be45..e5891e02021 100644
>> --- a/git.c
>> +++ b/git.c
>> @@ -538,6 +538,7 @@ static struct cmd_struct commands[] = {
>> { "grep", cmd_grep, RUN_SETUP_GENTLY },
>> { "hash-object", cmd_hash_object },
>> { "help", cmd_help },
>> + { "hook", cmd_hook, RUN_SETUP },
>> { "index-pack", cmd_index_pack, RUN_SETUP_GENTLY | NO_PARSEOPT },
>> { "init", cmd_init_db },
>> { "init-db", cmd_init_db },
>> diff --git a/hook.c b/hook.c
>> index 55e1145a4b7..240270db64e 100644
>> --- a/hook.c
>> +++ b/hook.c
>> @@ -1,6 +1,7 @@
>> #include "cache.h"
>> #include "hook.h"
>> #include "run-command.h"
>> +#include "config.h"
>>
>> const char *find_hook(const char *name)
>> {
>> @@ -40,3 +41,103 @@ int hook_exists(const char *name)
>> {
>> return !!find_hook(name);
>> }
>> +
>> +void run_hooks_opt_clear(struct run_hooks_opt *o)
>> +{
>> + strvec_clear(&o->env);
>> + strvec_clear(&o->args);
>> +}
>> +
>> +static int pick_next_hook(struct child_process *cp,
>> + struct strbuf *out,
>> + void *pp_cb,
>> + void **pp_task_cb)
>> +{
>> + struct hook_cb_data *hook_cb = pp_cb;
>> + struct hook *run_me = hook_cb->run_me;
>> +
>> + if (!run_me)
>> + return 0;
>> +
>> + cp->no_stdin = 1;
>> + cp->env = hook_cb->options->env.v;
>> + cp->stdout_to_stderr = 1;
>> + cp->trace2_hook_name = hook_cb->hook_name;
>> +
>> + /* add command */
>> + strvec_push(&cp->args, run_me->hook_path);
>> +
>> + /*
>> + * add passed-in argv, without expanding - let the user get back
>> + * exactly what they put in
>
> What kind of expanding is it doing without? I was expecting the
> arguments to passed on verbatim before reading this comment, so it
> leaves me wondering what I'm missing.
I think that's one of Emily's comments, will find out...
>> + */
>> + strvec_pushv(&cp->args, hook_cb->options->args.v);
>> +
>> + /* Provide context for errors if necessary */
>> + *pp_task_cb = run_me;
>> +
>> + /*
>> + * This pick_next_hook() will be called again, we're only
>> + * running one hook, so indicate that no more work will be
>> + * done.
>> + */
>> + hook_cb->run_me = NULL;
>
> A single hook is picked.
Right...
>> +
>> + return 1;
>> +}
>> +
>> +static int notify_start_failure(struct strbuf *out,
>> + void *pp_cb,
>> + void *pp_task_cp)
>> +{
>> + struct hook_cb_data *hook_cb = pp_cb;
>> + struct hook *attempted = pp_task_cp;
>> +
>> + hook_cb->rc |= 1;
>> +
>> + strbuf_addf(out, _("Couldn't start hook '%s'\n"),
>> + attempted->hook_path);
>> +
>> + return 1;
>> +}
>> +
>> +static int notify_hook_finished(int result,
>> + struct strbuf *out,
>> + void *pp_cb,
>> + void *pp_task_cb)
>> +{
>> + struct hook_cb_data *hook_cb = pp_cb;
>> +
>> + hook_cb->rc |= result;
>> +
>> + return 0;
>> +}
>> +
>> +int run_hooks(const char *hook_name, const char *hook_path,
>> + struct run_hooks_opt *options)
>> +{
>> + struct hook my_hook = {
>> + .hook_path = hook_path,
>> + };
>> + struct hook_cb_data cb_data = {
>> + .rc = 0,
>> + .hook_name = hook_name,
>> + .options = options,
>> + };
>> + int jobs = 1;
>> +
>> + if (!options)
>> + BUG("a struct run_hooks_opt must be provided to run_hooks");
>> +
>> + cb_data.run_me = &my_hook;
>> +
>> + run_processes_parallel_tr2(jobs,
>> + pick_next_hook,
>> + notify_start_failure,
>> + notify_hook_finished,
>> + &cb_data,
>> + "hook",
>> + hook_name);
>
> A single process (jobs == 1) is used to run a single hook. Why use
> run_processes_parallel_tr2() for that instead of run_command()? The
> latter would require a lot less code to achieve the same outcome, no?
...also for this...
>> +
>> + return cb_data.rc;
>> +}
>> diff --git a/hook.h b/hook.h
>> index 6aa36fc7ff9..111c5cf9296 100644
>> --- a/hook.h
>> +++ b/hook.h
>> @@ -1,5 +1,33 @@
>> #ifndef HOOK_H
>> #define HOOK_H
>> +#include "strvec.h"
>> +
>> +struct hook {
>> + /* The path to the hook */
>> + const char *hook_path;
>> +};
>
> None of the patches in this series extend this structure. Why not
> use a bare string directly?
...the reason for all of this is that it's a multi-part series where
we're eventually headed towards multi-hook parallel support.
An earlier version of this started with run_command() et al, then later
migrated to the parallel API, since the parallel API can do a single run
with a jobs=1 I found that easier than the curn of switching back and
forth.
Ditto some other scaffolding like this one-member struct, which will be
expanded later on.
Will address this by clarifying the plans in commit messages.
>> +
>> +struct run_hooks_opt
>> +{
>> + /* Environment vars to be set for each hook */
>> + struct strvec env;
>> +
>> + /* Args to be passed to each hook */
>> + struct strvec args;
>> +};
>> +
>> +#define RUN_HOOKS_OPT_INIT { \
>> + .env = STRVEC_INIT, \
>> + .args = STRVEC_INIT, \
>> +}
>> +
>> +struct hook_cb_data {
>> + /* rc reflects the cumulative failure state */
>> + int rc;
>> + const char *hook_name;
>> + struct hook *run_me;
>> + struct run_hooks_opt *options;
>> +};
>>
>> /*
>> * Returns the path to the hook file, or NULL if the hook is missing
>> @@ -13,4 +41,15 @@ const char *find_hook(const char *name);
>> */
>> int hook_exists(const char *hookname);
>>
>> +/**
>> + * Clear data from an initialized "struct run_hooks-opt".
> ^
> y/-/_/
*nod*
>> + */
>> +void run_hooks_opt_clear(struct run_hooks_opt *o);
>> +
>> +/**
>> + * Takes an already resolved hook found via find_hook() and runs
>> + * it. Does not call run_hooks_opt_clear() for you.
>> + */
>> +int run_hooks(const char *hookname, const char *hook_path,
>> + struct run_hooks_opt *options);
>
> If it runs one hook, why is it called run_hooks(), plural?
Ditto above, will clarify, eventually it will run N, and then having the
churn of changing all callers/re-indenting argument lists...
Thanks a lot for your time & the review. Will wait on a re-roll for more
comments...
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 01/13] hook: add 'run' subcommand
2021-10-13 0:52 ` Bagas Sanjaya
@ 2021-10-13 13:04 ` Ævar Arnfjörð Bjarmason
0 siblings, 0 replies; 18+ messages in thread
From: Ævar Arnfjörð Bjarmason @ 2021-10-13 13:04 UTC (permalink / raw)
To: Bagas Sanjaya
Cc: git, Junio C Hamano, Phillip Wood, René Scharfe,
Emily Shaffer, Joachim Kuebart, Daniel Levin, Luke Diamand
On Wed, Oct 13 2021, Bagas Sanjaya wrote:
> On 12/10/21 20.30, Ævar Arnfjörð Bjarmason wrote:
>> +run::
>> + Run the `<hook-name>` hook. See linkgit:githooks[5] for
>> + the hook names we support.
>> ++
>
> Drop the first person, so it become `list of supported hooks`.
>
>> +Any positional arguments to the hook should be passed after an
>> +optional `--` (or `--end-of-options`, see linkgit:gitcli[7]). The
>> +arguments (if any) differ by hook name, see linkgit:githooks[5] for
>> +what those are.
>> +
>
> s/what those are/supported list of them/ (dunno?)
Thanks, will address both.
^ permalink raw reply [flat|nested] 18+ messages in thread
end of thread, other threads:[~2021-10-13 13:04 UTC | newest]
Thread overview: 18+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-10-12 13:30 [PATCH 00/13] hook.[ch]: new library to run hooks + simple hook conversion Ævar Arnfjörð Bjarmason
2021-10-12 13:30 ` [PATCH 01/13] hook: add 'run' subcommand Ævar Arnfjörð Bjarmason
2021-10-12 17:35 ` René Scharfe
2021-10-13 12:58 ` Ævar Arnfjörð Bjarmason
2021-10-13 0:52 ` Bagas Sanjaya
2021-10-13 13:04 ` Ævar Arnfjörð Bjarmason
2021-10-12 13:30 ` [PATCH 02/13] gc: use hook library for pre-auto-gc hook Ævar Arnfjörð Bjarmason
2021-10-12 13:30 ` [PATCH 03/13] rebase: convert pre-rebase to use hook.h Ævar Arnfjörð Bjarmason
2021-10-12 13:30 ` [PATCH 04/13] am: convert applypatch " Ævar Arnfjörð Bjarmason
2021-10-12 13:30 ` [PATCH 05/13] hooks: convert 'post-checkout' hook to hook library Ævar Arnfjörð Bjarmason
2021-10-12 13:30 ` [PATCH 06/13] merge: convert post-merge to use hook.h Ævar Arnfjörð Bjarmason
2021-10-12 13:30 ` [PATCH 07/13] git hook run: add an --ignore-missing flag Ævar Arnfjörð Bjarmason
2021-10-12 13:30 ` [PATCH 08/13] send-email: use 'git hook run' for 'sendemail-validate' Ævar Arnfjörð Bjarmason
2021-10-12 13:30 ` [PATCH 09/13] git-p4: use 'git hook' to run hooks Ævar Arnfjörð Bjarmason
2021-10-12 13:30 ` [PATCH 10/13] commit: convert {pre-commit,prepare-commit-msg} hook to hook.h Ævar Arnfjörð Bjarmason
2021-10-12 13:30 ` [PATCH 11/13] read-cache: convert post-index-change to use hook.h Ævar Arnfjörð Bjarmason
2021-10-12 13:30 ` [PATCH 12/13] receive-pack: convert push-to-checkout hook to hook.h Ævar Arnfjörð Bjarmason
2021-10-12 13:30 ` [PATCH 13/13] run-command: remove old run_hook_{le,ve}() hook API Ævar Arnfjörð Bjarmason
Code repositories for project(s) associated with this public inbox
https://80x24.org/mirrors/git.git
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).