[PATCH v2 00/13] hook.[ch]: new library to run hooks + simple hook conversion

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Part 2 of the greater configurable hook saga, starting by converting
some existing simple hooks to the new hook.[ch] library and "git hook
run" utility.

See the v1 [1] CL for more context. I've dropped the git-p4 folks from
the CC list, your feedback on git-p4 would still be very appreciated,
but I don't think I need to re-spam you for every re-roll of this.

This v2:

 * Should address all outstanding comments on v1, which were mainly of
   the form that some boilerplate here didn't reflect the actual state
   of the code.

   I've updated the docs, code etc. to 1=1 map to the existing
   behavior. I said I'd punt on e.g. the one-member "struct hook" in
   reply to René Scharfe, but when I tried it and rebased the rest on
   it it actually made the progression more understandable, so I've
   done that here.

   I didn't convert some things, which are noted in the updated commit
   message on v1, e.g. we still use the parallel run-command.c API
   with jobs=1, and have a function called run_hooks() that only ever
   runs one hook.

1. https://lore.kernel.org/git/cover-00.13-00000000000-20211012T131934Z-avarab@xxxxxxxxx/

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 |  45 +++++++++++++
 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             |  90 +++++++++++++++++++++++++
 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                     | 121 +++++++++++++++++++++++++++++++++
 hook.h                     |  56 ++++++++++++++++
 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, 551 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 against v1:
 1:  a39c0748d3f !  1:  ba64faf0580 hook: add 'run' subcommand
    @@ Commit message
         pattern here mirrors that of
         builtin/{commit-graph,multi-pack-index}.c.
     
    +    Some of the implementation here, such as a function being named
    +    run_hooks() when it's tasked with running one hook, to using the
    +    run_processes_parallel_tr2() API to run with jobs=1 is somewhere
    +    between a bit odd and and an overkill for the current features of this
    +    "hook run" command and the hook.[ch] API.
    +
    +    This code will eventually be able to run multiple hooks declared in
    +    config in parallel, by starting out with these names and APIs we
    +    reduce the later churn of renaming functions, switching from the
    +    run_command() to run_processes_parallel_tr2() API etc.
    +
         Signed-off-by: Emily Shaffer <emilyshaffer@xxxxxxxxxx>
         Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@xxxxxxxxx>
     
    @@ Documentation/git-hook.txt (new)
     +
     +NAME
     +----
    -+git-hook - run git hooks
    ++git-hook - Run git hooks
     +
     +SYNOPSIS
     +--------
    @@ Documentation/git-hook.txt (new)
     +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.
    ++A command interface to running git hooks (see linkgit:githooks[5]),
    ++for use by other scripted git commands.
     +
     +SUBCOMMANDS
     +-----------
     +
     +run::
     +	Run the `<hook-name>` hook. See linkgit:githooks[5] for
    -+	the hook names we support.
    ++	supported hook names.
     ++
    -+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.
    ++
    ++Any positional arguments to the hook should be passed after a
    ++mandatory `--` (or `--end-of-options`, see linkgit:gitcli[7]). See
    ++linkgit:githooks[5] for arguments hooks might expect (if any).
     +
     +SEE ALSO
     +--------
    @@ builtin/hook.c (new)
     +	/* 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) {
    @@ command-list.txt: git-grep                                mainporcelain
      git-gui                                 mainporcelain
      git-hash-object                         plumbingmanipulators
      git-help                                ancillaryinterrogators          complete
    -+git-hook                                mainporcelain
    ++git-hook                                purehelpers
      git-http-backend                        synchingrepositories
      git-http-fetch                          synchelpers
      git-http-push                           synchelpers
    @@ hook.c: int hook_exists(const char *name)
     +			  void **pp_task_cb)
     +{
     +	struct hook_cb_data *hook_cb = pp_cb;
    -+	struct hook *run_me = hook_cb->run_me;
    ++	const char *hook_path = hook_cb->hook_path;
     +
    -+	if (!run_me)
    ++	if (!hook_path)
     +		return 0;
     +
     +	cp->no_stdin = 1;
    @@ hook.c: int hook_exists(const char *name)
     +	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_push(&cp->args, hook_path);
     +	strvec_pushv(&cp->args, hook_cb->options->args.v);
     +
     +	/* Provide context for errors if necessary */
    -+	*pp_task_cb = run_me;
    ++	*pp_task_cb = (char *)hook_path;
     +
     +	/*
     +	 * 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;
    ++	hook_cb->hook_path = NULL;
     +
     +	return 1;
     +}
    @@ hook.c: int hook_exists(const char *name)
     +				void *pp_task_cp)
     +{
     +	struct hook_cb_data *hook_cb = pp_cb;
    -+	struct hook *attempted = pp_task_cp;
    ++	const char *hook_path = pp_task_cp;
     +
     +	hook_cb->rc |= 1;
     +
     +	strbuf_addf(out, _("Couldn't start hook '%s'\n"),
    -+		    attempted->hook_path);
    ++		    hook_path);
     +
     +	return 1;
     +}
    @@ hook.c: int hook_exists(const char *name)
     +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,
    ++		.hook_path = hook_path,
     +		.options = options,
     +	};
     +	int jobs = 1;
    @@ hook.c: int hook_exists(const char *name)
     +	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,
    @@ 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 */
    @@ hook.h
     +	/* rc reflects the cumulative failure state */
     +	int rc;
     +	const char *hook_name;
    -+	struct hook *run_me;
    ++	const char *hook_path;
     +	struct run_hooks_opt *options;
     +};
      
    @@ hook.h: const char *find_hook(const char *name);
      int hook_exists(const char *hookname);
      
     +/**
    -+ * Clear data from an initialized "struct run_hooks-opt".
    ++ * Clear data from an initialized "struct run_hooks_opt".
     + */
     +void run_hooks_opt_clear(struct run_hooks_opt *o);
     +
 2:  dbac4204f7b =  2:  e3dc0aed81b gc: use hook library for pre-auto-gc hook
 3:  ff306debcb8 =  3:  6227a1e644d rebase: convert pre-rebase to use hook.h
 4:  b1d529ca485 =  4:  0e34eb54054 am: convert applypatch to use hook.h
 5:  15d71fc210b !  5:  a4df96c1719 hooks: convert 'post-checkout' hook to hook library
    @@ hook.c: static int pick_next_hook(struct child_process *cp,
      	cp->trace2_hook_name = hook_cb->hook_name;
     +	cp->dir = hook_cb->options->dir;
      
    - 	/* add command */
    - 	strvec_push(&cp->args, run_me->hook_path);
    + 	strvec_push(&cp->args, hook_path);
    + 	strvec_pushv(&cp->args, hook_cb->options->args.v);
     @@ hook.c: 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,
    - 	};
    + 	struct hook_cb_data cb_data = {
    + 		.rc = 0,
    + 		.hook_name = hook_name,
     @@ hook.c: 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;
    ++		hook_path = abs_path.buf;
     +	}
    - 	cb_data.run_me = &my_hook;
    - 
    ++
      	run_processes_parallel_tr2(jobs,
    + 				   pick_next_hook,
    + 				   notify_start_failure,
     @@ hook.c: int run_hooks(const char *hook_name, const char *hook_path,
      				   "hook",
      				   hook_name);
 6:  08f27f0d6be =  6:  327f916f8c3 merge: convert post-merge to use hook.h
 7:  107c14d740f !  7:  328767015b1 git hook run: add an --ignore-missing flag
    @@ Commit message
         Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@xxxxxxxxx>
     
      ## Documentation/git-hook.txt ##
    -@@ Documentation/git-hook.txt: git-hook - run git hooks
    +@@ Documentation/git-hook.txt: git-hook - Run git hooks
      SYNOPSIS
      --------
      [verse]
    @@ Documentation/git-hook.txt: git-hook - run git hooks
      
      DESCRIPTION
      -----------
    -@@ Documentation/git-hook.txt: 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.
    +@@ Documentation/git-hook.txt: Any positional arguments to the hook should be passed after a
    + mandatory `--` (or `--end-of-options`, see linkgit:gitcli[7]). See
    + linkgit:githooks[5] for arguments hooks might expect (if any).
      
     +OPTIONS
     +-------
    @@ builtin/hook.c: static int run(int argc, const char **argv, const char *prefix)
      	};
      	int ret;
     @@ builtin/hook.c: 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...
    - 	 */
    + 	git_config(git_default_config, NULL);
    + 
      	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) {
 8:  1d30e2dbbe0 =  8:  6c4ebd68d56 send-email: use 'git hook run' for 'sendemail-validate'
 9:  69cc447a1e1 =  9:  b1f52733e3c git-p4: use 'git hook' to run hooks
10:  1c22b2992cf = 10:  dc31d98acdf commit: convert {pre-commit,prepare-commit-msg} hook to hook.h
11:  e762fce32af = 11:  58b7689e4af read-cache: convert post-index-change to use hook.h
12:  d63b91196ae = 12:  ae1e2a82147 receive-pack: convert push-to-checkout hook to hook.h
13:  fe8996dda3e = 13:  289d5a2d849 run-command: remove old run_hook_{le,ve}() hook API
-- 
2.33.1.1338.g20da966911a




[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]

  Powered by Linux