diff mbox series

[v4,2/9] hook: scaffolding for git-hook subcommand

Message ID 20200909004939.1942347-3-emilyshaffer@google.com (mailing list archive)
State New, archived
Headers show
Series propose config-based hooks | expand

Commit Message

Emily Shaffer Sept. 9, 2020, 12:49 a.m. UTC
Introduce infrastructure for a new subcommand, git-hook, which will be
used to ease config-based hook management. This command will handle
parsing configs to compose a list of hooks to run for a given event, as
well as adding or modifying hook configs in an interactive fashion.

Signed-off-by: Emily Shaffer <emilyshaffer@google.com>
---
 .gitignore                    |  1 +
 Documentation/git-hook.txt    | 19 +++++++++++++++++++
 Makefile                      |  1 +
 builtin.h                     |  1 +
 builtin/hook.c                | 21 +++++++++++++++++++++
 git.c                         |  1 +
 t/t1360-config-based-hooks.sh | 11 +++++++++++
 7 files changed, 55 insertions(+)
 create mode 100644 Documentation/git-hook.txt
 create mode 100644 builtin/hook.c
 create mode 100755 t/t1360-config-based-hooks.sh

Comments

Jonathan Nieder Oct. 5, 2020, 11:24 p.m. UTC | #1
Hi,

Emily Shaffer wrote:

> Introduce infrastructure for a new subcommand, git-hook, which will be
> used to ease config-based hook management. This command will handle
> parsing configs to compose a list of hooks to run for a given event, as
> well as adding or modifying hook configs in an interactive fashion.
>
> Signed-off-by: Emily Shaffer <emilyshaffer@google.com>
> ---
>  .gitignore                    |  1 +
>  Documentation/git-hook.txt    | 19 +++++++++++++++++++
>  Makefile                      |  1 +
>  builtin.h                     |  1 +
>  builtin/hook.c                | 21 +++++++++++++++++++++
>  git.c                         |  1 +
>  t/t1360-config-based-hooks.sh | 11 +++++++++++
>  7 files changed, 55 insertions(+)
>  create mode 100644 Documentation/git-hook.txt
>  create mode 100644 builtin/hook.c
>  create mode 100755 t/t1360-config-based-hooks.sh

optional: I could imagine this being squashed into patch 3 --- that way,
the command has functionality as soon as it exists.  Alternatively:

[...]
> --- /dev/null
> +++ b/Documentation/git-hook.txt
> @@ -0,0 +1,19 @@
> +git-hook(1)
> +===========
> +
> +NAME
> +----
> +git-hook - Manage configured hooks
> +
> +SYNOPSIS
> +--------
> +[verse]
> +'git hook'
> +
> +DESCRIPTION
> +-----------
> +You can list, add, and modify hooks with this command.

This could say something like "This is a placeholder command that will
gain functionality in subsequent patches" to make the current state
clear.

[...]
> --- a/git.c
> +++ b/git.c
> @@ -519,6 +519,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 },

This makes the command require that it run within a git repository,
but I can imagine wanting to list hooks outside of any.  Should it use
RUN_SETUP_GENTLY instead?

Thanks,
Jonathan
Emily Shaffer Oct. 6, 2020, 7:06 p.m. UTC | #2
On Mon, Oct 05, 2020 at 04:24:18PM -0700, Jonathan Nieder wrote:
> 
> Hi,
> 
> Emily Shaffer wrote:
> 
> > Introduce infrastructure for a new subcommand, git-hook, which will be
> > used to ease config-based hook management. This command will handle
> > parsing configs to compose a list of hooks to run for a given event, as
> > well as adding or modifying hook configs in an interactive fashion.
> >
> > Signed-off-by: Emily Shaffer <emilyshaffer@google.com>
> > ---
> >  .gitignore                    |  1 +
> >  Documentation/git-hook.txt    | 19 +++++++++++++++++++
> >  Makefile                      |  1 +
> >  builtin.h                     |  1 +
> >  builtin/hook.c                | 21 +++++++++++++++++++++
> >  git.c                         |  1 +
> >  t/t1360-config-based-hooks.sh | 11 +++++++++++
> >  7 files changed, 55 insertions(+)
> >  create mode 100644 Documentation/git-hook.txt
> >  create mode 100644 builtin/hook.c
> >  create mode 100755 t/t1360-config-based-hooks.sh
> 
> optional: I could imagine this being squashed into patch 3 --- that way,
> the command has functionality as soon as it exists.  Alternatively:

I would prefer to leave it on its own. Managing changes like
builtin<->standalone or even the one you mentioned below about
RUN_SETUP_GENTLY is somewhat easier to manage when they aren't in the
same patch as the business logic, IMO.

> 
> [...]
> > --- /dev/null
> > +++ b/Documentation/git-hook.txt
> > @@ -0,0 +1,19 @@
> > +git-hook(1)
> > +===========
> > +
> > +NAME
> > +----
> > +git-hook - Manage configured hooks
> > +
> > +SYNOPSIS
> > +--------
> > +[verse]
> > +'git hook'
> > +
> > +DESCRIPTION
> > +-----------
> > +You can list, add, and modify hooks with this command.
> 
> This could say something like "This is a placeholder command that will
> gain functionality in subsequent patches" to make the current state
> clear.

Done.

> 
> [...]
> > --- a/git.c
> > +++ b/git.c
> > @@ -519,6 +519,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 },
> 
> This makes the command require that it run within a git repository,
> but I can imagine wanting to list hooks outside of any.  Should it use
> RUN_SETUP_GENTLY instead?

Nice catch. I'll add a test to the list patch to that effect also.
diff mbox series

Patch

diff --git a/.gitignore b/.gitignore
index ee509a2ad2..0694a34884 100644
--- a/.gitignore
+++ b/.gitignore
@@ -75,6 +75,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 0000000000..2d50c414cc
--- /dev/null
+++ b/Documentation/git-hook.txt
@@ -0,0 +1,19 @@ 
+git-hook(1)
+===========
+
+NAME
+----
+git-hook - Manage configured hooks
+
+SYNOPSIS
+--------
+[verse]
+'git hook'
+
+DESCRIPTION
+-----------
+You can list, add, and modify hooks with this command.
+
+GIT
+---
+Part of the linkgit:git[1] suite
diff --git a/Makefile b/Makefile
index 65f8cfb236..6eee75555e 100644
--- a/Makefile
+++ b/Makefile
@@ -1077,6 +1077,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 a5ae15bfe5..4e736499c0 100644
--- a/builtin.h
+++ b/builtin.h
@@ -157,6 +157,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 0000000000..b2bbc84d4d
--- /dev/null
+++ b/builtin/hook.c
@@ -0,0 +1,21 @@ 
+#include "cache.h"
+
+#include "builtin.h"
+#include "parse-options.h"
+
+static const char * const builtin_hook_usage[] = {
+	N_("git hook"),
+	NULL
+};
+
+int cmd_hook(int argc, const char **argv, const char *prefix)
+{
+	struct option builtin_hook_options[] = {
+		OPT_END(),
+	};
+
+	argc = parse_options(argc, argv, prefix, builtin_hook_options,
+			     builtin_hook_usage, 0);
+
+	return 0;
+}
diff --git a/git.c b/git.c
index 8bd1d7551d..1cdb3221a5 100644
--- a/git.c
+++ b/git.c
@@ -519,6 +519,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/t/t1360-config-based-hooks.sh b/t/t1360-config-based-hooks.sh
new file mode 100755
index 0000000000..34b0df5216
--- /dev/null
+++ b/t/t1360-config-based-hooks.sh
@@ -0,0 +1,11 @@ 
+#!/bin/bash
+
+test_description='config-managed multihooks, including git-hook command'
+
+. ./test-lib.sh
+
+test_expect_success 'git hook command does not crash' '
+	git hook
+'
+
+test_done