| Message ID | 20191210023335.49987-7-emilyshaffer@google.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | configuration-based hook management | expand |
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?
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.
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.
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
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
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(-)