[6/6] hook: teach --porcelain mode
diff mbox series

Message ID 20191210023335.49987-7-emilyshaffer@google.com
State New
Headers show
Series
  • configuration-based hook management
Related show

Commit Message

Emily Shaffer Dec. 10, 2019, 2:33 a.m. UTC
It might be desirable - for a user script, or a scripted Git command -
to run the appropriate set of hooks from outside of the compiled Git
binary. So, teach --porcelain in a way that enables the following:

  git hook --list --porcelain pre-commit | xargs -I% sh "%"

Signed-off-by: Emily Shaffer <emilyshaffer@google.com>
---
 Documentation/git-hook.txt    |  5 ++++-
 builtin/hook.c                | 19 +++++++++++++------
 t/t1360-config-based-hooks.sh | 12 ++++++++++++
 3 files changed, 29 insertions(+), 7 deletions(-)

Comments

Junio C Hamano Dec. 11, 2019, 7:33 p.m. UTC | #1
Emily Shaffer <emilyshaffer@google.com> writes:

> It might be desirable - for a user script, or a scripted Git command -
> to run the appropriate set of hooks from outside of the compiled Git
> binary. So, teach --porcelain in a way that enables the following:
>
>   git hook --list --porcelain pre-commit | xargs -I% sh "%"
>
> Signed-off-by: Emily Shaffer <emilyshaffer@google.com>
> ---

> +--porcelain::
> +	Print in a machine-readable format suitable for scripting.
> +
> ...
> +static int print_hook_list(const struct strbuf *hookname, int porcelain)
>  {
>  	struct list_head *head, *pos;
>  	struct hook *item;
> @@ -25,10 +25,14 @@ static int print_hook_list(const struct strbuf *hookname)
>  
>  	list_for_each(pos, head) {
>  		item = list_entry(pos, struct hook, list);
> +		if (item) {
> +			if (porcelain)
> +				printf("%s\n", item->command.buf);
> +			else
> +				printf("%.3d\t%s\t%s\n", item->order,
> +				       config_scope_to_string(item->origin),
> +				       item->command.buf);
> +		}

So, a Porcelain script cannot learn where the hook command comes
from, or what the precedence order of each line of the output is?
Emily Shaffer Dec. 11, 2019, 10 p.m. UTC | #2
On Wed, Dec 11, 2019 at 11:33:38AM -0800, Junio C Hamano wrote:
> Emily Shaffer <emilyshaffer@google.com> writes:
> 
> > It might be desirable - for a user script, or a scripted Git command -
> > to run the appropriate set of hooks from outside of the compiled Git
> > binary. So, teach --porcelain in a way that enables the following:
> >
> >   git hook --list --porcelain pre-commit | xargs -I% sh "%"
> >
> > Signed-off-by: Emily Shaffer <emilyshaffer@google.com>
> > ---
> 
> > +--porcelain::
> > +	Print in a machine-readable format suitable for scripting.
> > +
> > ...
> > +static int print_hook_list(const struct strbuf *hookname, int porcelain)
> >  {
> >  	struct list_head *head, *pos;
> >  	struct hook *item;
> > @@ -25,10 +25,14 @@ static int print_hook_list(const struct strbuf *hookname)
> >  
> >  	list_for_each(pos, head) {
> >  		item = list_entry(pos, struct hook, list);
> > +		if (item) {
> > +			if (porcelain)
> > +				printf("%s\n", item->command.buf);
> > +			else
> > +				printf("%.3d\t%s\t%s\n", item->order,
> > +				       config_scope_to_string(item->origin),
> > +				       item->command.buf);
> > +		}
> 
> So, a Porcelain script cannot learn where the hook command comes
> from,

Not as I had envisioned.

> or what the precedence order of each line of the output is?
> 

They're printed in the order they should be executed; the explicit order
isn't provided.


I suppose I had considered really just the one use case listed in the
commit message, especially since other inquiry into the hooks to be run
can be done against the config files themselves. But - I'm of course
open to use cases. What did you have in mind?

Maybe this can be solved better with a --pretty=format type of argument.
Junio C Hamano Dec. 11, 2019, 10:07 p.m. UTC | #3
Emily Shaffer <emilyshaffer@google.com> writes:

>> So, a Porcelain script cannot learn where the hook command comes
>> from,
>
> Not as I had envisioned.
>
>> or what the precedence order of each line of the output is?
>> 
>
> They're printed in the order they should be executed; the explicit order
> isn't provided.
>
>
> I suppose I had considered really just the one use case listed in the
> commit message, especially since other inquiry into the hooks to be run
> can be done against the config files themselves. But - I'm of course
> open to use cases. What did you have in mind?

A tool to diagnose why the hooks are not firing in the order the
user intended them to, for example?

Or a tool to help editing the list of hooks.
Emily Shaffer Dec. 11, 2019, 11:15 p.m. UTC | #4
On Wed, Dec 11, 2019 at 02:07:45PM -0800, Junio C Hamano wrote:
> Emily Shaffer <emilyshaffer@google.com> writes:
> 
> >> So, a Porcelain script cannot learn where the hook command comes
> >> from,
> >
> > Not as I had envisioned.
> >
> >> or what the precedence order of each line of the output is?
> >> 
> >
> > They're printed in the order they should be executed; the explicit order
> > isn't provided.
> >
> >
> > I suppose I had considered really just the one use case listed in the
> > commit message, especially since other inquiry into the hooks to be run
> > can be done against the config files themselves. But - I'm of course
> > open to use cases. What did you have in mind?
> 
> A tool to diagnose why the hooks are not firing in the order the
> user intended them to, for example?
> 
> Or a tool to help editing the list of hooks.
FWIW, the next step for this 'git hook' tool is just such a mode,
although I certainly won't argue with anybody who wants to interact with
them somewhat differently.

Does allowing a format string solve this, then? Maybe it's less
Git-idiomatic, but it seems to me to be a very explicit format contract
that the scripter can write, and probably more useful than guessing what
info one might want when scripting. It also doesn't paint us into a
corner if we add other interesting info later.

Unless you have a complaint about it, I'll try to add that kind of
argument instead of --porcelain for this command.

 - Emily

Patch
diff mbox series

diff --git a/Documentation/git-hook.txt b/Documentation/git-hook.txt
index b4a992d43f..34276f5bce 100644
--- a/Documentation/git-hook.txt
+++ b/Documentation/git-hook.txt
@@ -8,7 +8,7 @@  git-hook - Manage configured hooks
 SYNOPSIS
 --------
 [verse]
-'git hook' -l | --list <hook-name>
+'git hook' -l | --list [--porcelain] <hook-name>
 
 DESCRIPTION
 -----------
@@ -45,6 +45,9 @@  OPTIONS
 	in the order they should be run. Output of this command follows the
 	format '<order number> <origin config> <hook command>'.
 
+--porcelain::
+	Print in a machine-readable format suitable for scripting.
+
 GIT
 ---
 Part of the linkgit:git[1] suite
diff --git a/builtin/hook.c b/builtin/hook.c
index 8261302b27..b76dd3ad8f 100644
--- a/builtin/hook.c
+++ b/builtin/hook.c
@@ -16,7 +16,7 @@  enum hook_command {
 	HOOK_LIST,
 };
 
-static int print_hook_list(const struct strbuf *hookname)
+static int print_hook_list(const struct strbuf *hookname, int porcelain)
 {
 	struct list_head *head, *pos;
 	struct hook *item;
@@ -25,10 +25,14 @@  static int print_hook_list(const struct strbuf *hookname)
 
 	list_for_each(pos, head) {
 		item = list_entry(pos, struct hook, list);
-		if (item)
-			printf("%.3d\t%s\t%s\n", item->order,
-			       config_scope_to_string(item->origin),
-			       item->command.buf);
+		if (item) {
+			if (porcelain)
+				printf("%s\n", item->command.buf);
+			else
+				printf("%.3d\t%s\t%s\n", item->order,
+				       config_scope_to_string(item->origin),
+				       item->command.buf);
+		}
 	}
 
 	return 0;
@@ -38,11 +42,14 @@  int cmd_hook(int argc, const char **argv, const char *prefix)
 {
 	enum hook_command command = 0;
 	struct strbuf hookname = STRBUF_INIT;
+	int porcelain = 0;
 
 	struct option builtin_hook_options[] = {
 		OPT_CMDMODE('l', "list", &command,
 			    N_("list scripts which will be run for <hookname>"),
 			    HOOK_LIST),
+		OPT_BOOL(0, "porcelain", &porcelain,
+			 N_("display in machine parseable format")),
 		OPT_END(),
 	};
 
@@ -59,7 +66,7 @@  int cmd_hook(int argc, const char **argv, const char *prefix)
 
 	switch(command) {
 		case HOOK_LIST:
-			return print_hook_list(&hookname);
+			return print_hook_list(&hookname, porcelain);
 			break;
 		default:
 			usage_msg_opt("no command given.", builtin_hook_usage,
diff --git a/t/t1360-config-based-hooks.sh b/t/t1360-config-based-hooks.sh
index 66e70ae222..6f16ea1dd8 100755
--- a/t/t1360-config-based-hooks.sh
+++ b/t/t1360-config-based-hooks.sh
@@ -33,6 +33,18 @@  test_expect_success 'git hook --list orders by order number' '
 	test_cmp expected actual
 '
 
+test_expect_success 'git hook --list --porcelain' '
+	cat >expected <<-\EOF &&
+	/path/def
+	/path/ghi
+	/path/rst
+	/path/uvw
+	EOF
+
+	git hook --list --porcelain pre-commit >actual &&
+	test_cmp expected actual
+'
+
 test_expect_success 'order number collisions resolved in config order' '
 	cat >expected <<-\EOF &&
 	010	global	/path/def