| Message ID | 20191210023335.49987-4-emilyshaffer@google.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | configuration-based hook management | expand |
On Tue, Dec 10, 2019 at 3:34 AM Emily Shaffer <emilyshaffer@google.com> wrote: > > Teach 'git hook --list <hookname>', which checks the known configs in > order to create an ordered list of hooks to run on a given hook event. > > The hook config format is "hook.<hookname> = <order>:<path-to-hook>". > This paves the way for multiple hook support; hooks should be run in the > order specified by the user in the config, and in the case of an order > number collision, configuration order should be used (e.g. global hook > 004 will run before repo hook 004). > > For example: > > $ grep -A2 "\[hook\]" ~/.gitconfig > [hook] > pre-commit = 001:~/test.sh > pre-commit = 999:~/baz.sh > > $ grep -A1 "\[hook\]" ~/git/.git/config > [hook] > pre-commit = 900:~/bar.sh > > $ ./bin-wrappers/git hook --list pre-commit > 001 global ~/test.sh > 900 repo ~/bar.sh > 999 global ~/baz.sh > > Signed-off-by: Emily Shaffer <emilyshaffer@google.com> > --- > Documentation/git-hook.txt | 17 +++++++- > Makefile | 1 + > builtin/hook.c | 54 ++++++++++++++++++++++- > hook.c | 81 +++++++++++++++++++++++++++++++++++ > hook.h | 14 ++++++ > t/t1360-config-based-hooks.sh | 43 ++++++++++++++++++- > 6 files changed, 206 insertions(+), 4 deletions(-) > create mode 100644 hook.c > create mode 100644 hook.h > > diff --git a/Documentation/git-hook.txt b/Documentation/git-hook.txt > index 2d50c414cc..a141884239 100644 > --- a/Documentation/git-hook.txt > +++ b/Documentation/git-hook.txt > @@ -8,12 +8,27 @@ git-hook - Manage configured hooks > SYNOPSIS > -------- > [verse] > -'git hook' > +'git hook' -l | --list <hook-name> > > DESCRIPTION > ----------- > You can list, add, and modify hooks with this command. > > +This command parses the default configuration files for lines which look like > +"hook.<hook-name> = <order number>:<hook command>", e.g. "hook.pre-commit = > +010:/path/to/script.sh". In this way, multiple scripts can be run during a > +single hook. Hooks are sorted in ascending order by order number; in the event > +of an order number conflict, they are sorted in configuration order. > + > +OPTIONS > +------- > + > +-l:: > +--list:: > + List the hooks which have been configured for <hook-name>. Hooks appear > + in the order they should be run. Output of this command follows the > + format '<order number> <origin config> <hook command>'. > + > GIT > --- > Part of the linkgit:git[1] suite > diff --git a/Makefile b/Makefile > index 83263505c0..21b3a82208 100644 > --- a/Makefile > +++ b/Makefile > @@ -892,6 +892,7 @@ LIB_OBJS += hashmap.o > LIB_OBJS += linear-assignment.o > LIB_OBJS += help.o > LIB_OBJS += hex.o > +LIB_OBJS += hook.o > LIB_OBJS += ident.o > LIB_OBJS += interdiff.o > LIB_OBJS += json-writer.o > diff --git a/builtin/hook.c b/builtin/hook.c > index b2bbc84d4d..8261302b27 100644 > --- a/builtin/hook.c > +++ b/builtin/hook.c > @@ -1,21 +1,73 @@ > #include "cache.h" > > #include "builtin.h" > +#include "config.h" > +#include "hook.h" > #include "parse-options.h" > +#include "strbuf.h" > > static const char * const builtin_hook_usage[] = { > - N_("git hook"), > + N_("git hook --list <hookname>"), Its "<hook-name>" in Documentation/git-hook.txt > NULL > }; > > +enum hook_command { > + HOOK_NO_COMMAND = 0, > + HOOK_LIST, > +}; > + > +static int print_hook_list(const struct strbuf *hookname) > +{ > + struct list_head *head, *pos; > + struct hook *item; > + > + head = hook_list(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); > + } > + > + return 0; > +} > + > int cmd_hook(int argc, const char **argv, const char *prefix) > { > + enum hook_command command = 0; > + struct strbuf hookname = STRBUF_INIT; > + > struct option builtin_hook_options[] = { > + OPT_CMDMODE('l', "list", &command, > + N_("list scripts which will be run for <hookname>"), Its "<hook-name>" in Documentation/git-hook.txt > + HOOK_LIST), > OPT_END(), > }; > > argc = parse_options(argc, argv, prefix, builtin_hook_options, > builtin_hook_usage, 0); > > + if (argc < 1) { > + usage_msg_opt("a hookname must be provided to operate on.", > + builtin_hook_usage, builtin_hook_options); > + } > + > + strbuf_addstr(&hookname, "hook."); > + strbuf_addstr(&hookname, argv[0]); The arg is never checked, if this is a valid/known hook. Bert > + > + switch(command) { > + case HOOK_LIST: > + return print_hook_list(&hookname); > + break; > + default: > + usage_msg_opt("no command given.", builtin_hook_usage, > + builtin_hook_options); > + } > + > + clear_hook_list(); > + strbuf_release(&hookname); > + > return 0; > } > diff --git a/hook.c b/hook.c > new file mode 100644 > index 0000000000..f8d1109084 > --- /dev/null > +++ b/hook.c > @@ -0,0 +1,81 @@ > +#include "cache.h" > + > +#include "hook.h" > +#include "config.h" > + > +static LIST_HEAD(hook_head); > + > +void free_hook(struct hook *ptr) > +{ > + if (ptr) { > + strbuf_release(&ptr->command); > + free(ptr); > + } > +} > + > +static void emplace_hook(struct list_head *pos, int order, const char *command) > +{ > + struct hook *to_add = malloc(sizeof(struct hook)); > + to_add->order = order; > + to_add->origin = current_config_scope(); > + strbuf_init(&to_add->command, 0); > + strbuf_addstr(&to_add->command, command); > + > + list_add_tail(&to_add->list, pos); > +} > + > +static void remove_hook(struct list_head *to_remove) > +{ > + struct hook *hook_to_remove = list_entry(to_remove, struct hook, list); > + list_del(to_remove); > + free_hook(hook_to_remove); > +} > + > +void clear_hook_list() > +{ > + struct list_head *pos, *tmp; > + list_for_each_safe(pos, tmp, &hook_head) > + remove_hook(pos); > +} > + > +static int check_config_for_hooks(const char *var, const char *value, void *hookname) > +{ > + struct list_head *pos, *p; > + struct hook *item; > + const struct strbuf *hookname_strbuf = hookname; > + > + if (!strcmp(var, hookname_strbuf->buf)) { > + int order = 0; > + // TODO this is bad - open to overflows > + char command[256]; > + int added = 0; > + if (!sscanf(value, "%d:%s", &order, command)) > + die(_("hook config '%s' doesn't match expected format"), > + value); > + > + list_for_each_safe(pos, p, &hook_head) { > + item = list_entry(pos, struct hook, list); > + > + /* > + * the new entry should go just before the first entry > + * which has a higher order number than it. > + */ > + if (item->order > order && !added) { > + emplace_hook(pos, order, command); > + added = 1; > + } > + } > + > + if (!added) > + emplace_hook(pos, order, command); > + } > + > + return 0; > +} > + > +struct list_head* hook_list(const struct strbuf* hookname) > +{ > + git_config(check_config_for_hooks, (void*)hookname); > + > + return &hook_head; > +} > diff --git a/hook.h b/hook.h > new file mode 100644 > index 0000000000..104df4c088 > --- /dev/null > +++ b/hook.h > @@ -0,0 +1,14 @@ > +#include "config.h" > + > +struct hook > +{ > + struct list_head list; > + int order; > + enum config_scope origin; > + struct strbuf command; > +}; > + > +struct list_head* hook_list(const struct strbuf *hookname); > + > +void free_hook(struct hook *ptr); > +void clear_hook_list(); > diff --git a/t/t1360-config-based-hooks.sh b/t/t1360-config-based-hooks.sh > index 34b0df5216..1434051db3 100755 > --- a/t/t1360-config-based-hooks.sh > +++ b/t/t1360-config-based-hooks.sh > @@ -4,8 +4,47 @@ test_description='config-managed multihooks, including git-hook command' > > . ./test-lib.sh > > -test_expect_success 'git hook command does not crash' ' > - git hook > +test_expect_success 'git hook rejects commands without a mode' ' > + test_must_fail git hook pre-commit > +' > + > + > +test_expect_success 'git hook rejects commands without a hookname' ' > + test_must_fail git hook --list > +' > + > +test_expect_success 'setup hooks in system, global, and local' ' > + git config --add --global hook.pre-commit "010:/path/def" && > + git config --add --global hook.pre-commit "999:/path/uvw" && > + > + git config --add --local hook.pre-commit "100:/path/ghi" && > + git config --add --local hook.pre-commit "990:/path/rst" > +' > + > +test_expect_success 'git hook --list orders by order number' ' > + cat >expected <<-\EOF && > + 010 global /path/def > + 100 repo /path/ghi > + 990 repo /path/rst > + 999 global /path/uvw > + EOF > + > + git hook --list pre-commit >actual && > + test_cmp expected actual > +' > + > +test_expect_success 'order number collisions resolved in config order' ' > + cat >expected <<-\EOF && > + 010 global /path/def > + 010 repo /path/abc > + 100 repo /path/ghi > + 990 repo /path/rst > + 999 global /path/uvw > + EOF > + > + git config --add --local hook.pre-commit "010:/path/abc" && > + git hook --list pre-commit >actual && > + test_cmp expected actual > ' > > test_done > -- > 2.24.0.393.g34dc348eaf-goog >
On Mon, Dec 09, 2019 at 06:33:32PM -0800, Emily Shaffer wrote: > Teach 'git hook --list <hookname>', which checks the known configs in > order to create an ordered list of hooks to run on a given hook event. > > The hook config format is "hook.<hookname> = <order>:<path-to-hook>". > This paves the way for multiple hook support; hooks should be run in the > order specified by the user in the config, and in the case of an order > number collision, configuration order should be used (e.g. global hook > 004 will run before repo hook 004). > > For example: > > $ grep -A2 "\[hook\]" ~/.gitconfig > [hook] > pre-commit = 001:~/test.sh > pre-commit = 999:~/baz.sh > > $ grep -A1 "\[hook\]" ~/git/.git/config > [hook] > pre-commit = 900:~/bar.sh > > $ ./bin-wrappers/git hook --list pre-commit > 001 global ~/test.sh > 900 repo ~/bar.sh > 999 global ~/baz.sh > > Signed-off-by: Emily Shaffer <emilyshaffer@google.com> > --- > Documentation/git-hook.txt | 17 +++++++- > Makefile | 1 + > builtin/hook.c | 54 ++++++++++++++++++++++- > hook.c | 81 +++++++++++++++++++++++++++++++++++ > hook.h | 14 ++++++ > t/t1360-config-based-hooks.sh | 43 ++++++++++++++++++- > 6 files changed, 206 insertions(+), 4 deletions(-) > create mode 100644 hook.c > create mode 100644 hook.h > > diff --git a/Documentation/git-hook.txt b/Documentation/git-hook.txt > index 2d50c414cc..a141884239 100644 > --- a/Documentation/git-hook.txt > +++ b/Documentation/git-hook.txt > @@ -8,12 +8,27 @@ git-hook - Manage configured hooks > SYNOPSIS > -------- > [verse] > -'git hook' > +'git hook' -l | --list <hook-name> > > DESCRIPTION > ----------- > You can list, add, and modify hooks with this command. > > +This command parses the default configuration files for lines which look like > +"hook.<hook-name> = <order number>:<hook command>", e.g. "hook.pre-commit = > +010:/path/to/script.sh". In this way, multiple scripts can be run during a > +single hook. Hooks are sorted in ascending order by order number; in the event > +of an order number conflict, they are sorted in configuration order. > + > +OPTIONS > +------- > + > +-l:: > +--list:: > + List the hooks which have been configured for <hook-name>. Hooks appear > + in the order they should be run. Output of this command follows the > + format '<order number> <origin config> <hook command>'. > + > GIT > --- > Part of the linkgit:git[1] suite > diff --git a/builtin/hook.c b/builtin/hook.c > index b2bbc84d4d..8261302b27 100644 > --- a/builtin/hook.c > +++ b/builtin/hook.c > @@ -1,21 +1,73 @@ > #include "cache.h" > > #include "builtin.h" > +#include "config.h" > +#include "hook.h" > #include "parse-options.h" > +#include "strbuf.h" > > static const char * const builtin_hook_usage[] = { > - N_("git hook"), > + N_("git hook --list <hookname>"), > NULL > }; > > +enum hook_command { > + HOOK_NO_COMMAND = 0, > + HOOK_LIST, > +}; > + > +static int print_hook_list(const struct strbuf *hookname) > +{ > + struct list_head *head, *pos; > + struct hook *item; > + > + head = hook_list(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); > + } > + > + return 0; > +} > + > int cmd_hook(int argc, const char **argv, const char *prefix) > { > + enum hook_command command = 0; > + struct strbuf hookname = STRBUF_INIT; > + > struct option builtin_hook_options[] = { > + OPT_CMDMODE('l', "list", &command, > + N_("list scripts which will be run for <hookname>"), > + HOOK_LIST), I'm not sure about '--list' being an option. I don't know what other operations you have in mind for this 'git hook' command, but I suppose that besides listing configured hooks it will be able to at least add, remove, and reorder them as well. These seem to be better implemented as subcommands, along the lines of e.g. how notes and remotes can be added, removed, etc. > OPT_END(), > };
diff --git a/Documentation/git-hook.txt b/Documentation/git-hook.txt index 2d50c414cc..a141884239 100644 --- a/Documentation/git-hook.txt +++ b/Documentation/git-hook.txt @@ -8,12 +8,27 @@ git-hook - Manage configured hooks SYNOPSIS -------- [verse] -'git hook' +'git hook' -l | --list <hook-name> DESCRIPTION ----------- You can list, add, and modify hooks with this command. +This command parses the default configuration files for lines which look like +"hook.<hook-name> = <order number>:<hook command>", e.g. "hook.pre-commit = +010:/path/to/script.sh". In this way, multiple scripts can be run during a +single hook. Hooks are sorted in ascending order by order number; in the event +of an order number conflict, they are sorted in configuration order. + +OPTIONS +------- + +-l:: +--list:: + List the hooks which have been configured for <hook-name>. Hooks appear + in the order they should be run. Output of this command follows the + format '<order number> <origin config> <hook command>'. + GIT --- Part of the linkgit:git[1] suite diff --git a/Makefile b/Makefile index 83263505c0..21b3a82208 100644 --- a/Makefile +++ b/Makefile @@ -892,6 +892,7 @@ LIB_OBJS += hashmap.o LIB_OBJS += linear-assignment.o LIB_OBJS += help.o LIB_OBJS += hex.o +LIB_OBJS += hook.o LIB_OBJS += ident.o LIB_OBJS += interdiff.o LIB_OBJS += json-writer.o diff --git a/builtin/hook.c b/builtin/hook.c index b2bbc84d4d..8261302b27 100644 --- a/builtin/hook.c +++ b/builtin/hook.c @@ -1,21 +1,73 @@ #include "cache.h" #include "builtin.h" +#include "config.h" +#include "hook.h" #include "parse-options.h" +#include "strbuf.h" static const char * const builtin_hook_usage[] = { - N_("git hook"), + N_("git hook --list <hookname>"), NULL }; +enum hook_command { + HOOK_NO_COMMAND = 0, + HOOK_LIST, +}; + +static int print_hook_list(const struct strbuf *hookname) +{ + struct list_head *head, *pos; + struct hook *item; + + head = hook_list(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); + } + + return 0; +} + int cmd_hook(int argc, const char **argv, const char *prefix) { + enum hook_command command = 0; + struct strbuf hookname = STRBUF_INIT; + struct option builtin_hook_options[] = { + OPT_CMDMODE('l', "list", &command, + N_("list scripts which will be run for <hookname>"), + HOOK_LIST), OPT_END(), }; argc = parse_options(argc, argv, prefix, builtin_hook_options, builtin_hook_usage, 0); + if (argc < 1) { + usage_msg_opt("a hookname must be provided to operate on.", + builtin_hook_usage, builtin_hook_options); + } + + strbuf_addstr(&hookname, "hook."); + strbuf_addstr(&hookname, argv[0]); + + switch(command) { + case HOOK_LIST: + return print_hook_list(&hookname); + break; + default: + usage_msg_opt("no command given.", builtin_hook_usage, + builtin_hook_options); + } + + clear_hook_list(); + strbuf_release(&hookname); + return 0; } diff --git a/hook.c b/hook.c new file mode 100644 index 0000000000..f8d1109084 --- /dev/null +++ b/hook.c @@ -0,0 +1,81 @@ +#include "cache.h" + +#include "hook.h" +#include "config.h" + +static LIST_HEAD(hook_head); + +void free_hook(struct hook *ptr) +{ + if (ptr) { + strbuf_release(&ptr->command); + free(ptr); + } +} + +static void emplace_hook(struct list_head *pos, int order, const char *command) +{ + struct hook *to_add = malloc(sizeof(struct hook)); + to_add->order = order; + to_add->origin = current_config_scope(); + strbuf_init(&to_add->command, 0); + strbuf_addstr(&to_add->command, command); + + list_add_tail(&to_add->list, pos); +} + +static void remove_hook(struct list_head *to_remove) +{ + struct hook *hook_to_remove = list_entry(to_remove, struct hook, list); + list_del(to_remove); + free_hook(hook_to_remove); +} + +void clear_hook_list() +{ + struct list_head *pos, *tmp; + list_for_each_safe(pos, tmp, &hook_head) + remove_hook(pos); +} + +static int check_config_for_hooks(const char *var, const char *value, void *hookname) +{ + struct list_head *pos, *p; + struct hook *item; + const struct strbuf *hookname_strbuf = hookname; + + if (!strcmp(var, hookname_strbuf->buf)) { + int order = 0; + // TODO this is bad - open to overflows + char command[256]; + int added = 0; + if (!sscanf(value, "%d:%s", &order, command)) + die(_("hook config '%s' doesn't match expected format"), + value); + + list_for_each_safe(pos, p, &hook_head) { + item = list_entry(pos, struct hook, list); + + /* + * the new entry should go just before the first entry + * which has a higher order number than it. + */ + if (item->order > order && !added) { + emplace_hook(pos, order, command); + added = 1; + } + } + + if (!added) + emplace_hook(pos, order, command); + } + + return 0; +} + +struct list_head* hook_list(const struct strbuf* hookname) +{ + git_config(check_config_for_hooks, (void*)hookname); + + return &hook_head; +} diff --git a/hook.h b/hook.h new file mode 100644 index 0000000000..104df4c088 --- /dev/null +++ b/hook.h @@ -0,0 +1,14 @@ +#include "config.h" + +struct hook +{ + struct list_head list; + int order; + enum config_scope origin; + struct strbuf command; +}; + +struct list_head* hook_list(const struct strbuf *hookname); + +void free_hook(struct hook *ptr); +void clear_hook_list(); diff --git a/t/t1360-config-based-hooks.sh b/t/t1360-config-based-hooks.sh index 34b0df5216..1434051db3 100755 --- a/t/t1360-config-based-hooks.sh +++ b/t/t1360-config-based-hooks.sh @@ -4,8 +4,47 @@ test_description='config-managed multihooks, including git-hook command' . ./test-lib.sh -test_expect_success 'git hook command does not crash' ' - git hook +test_expect_success 'git hook rejects commands without a mode' ' + test_must_fail git hook pre-commit +' + + +test_expect_success 'git hook rejects commands without a hookname' ' + test_must_fail git hook --list +' + +test_expect_success 'setup hooks in system, global, and local' ' + git config --add --global hook.pre-commit "010:/path/def" && + git config --add --global hook.pre-commit "999:/path/uvw" && + + git config --add --local hook.pre-commit "100:/path/ghi" && + git config --add --local hook.pre-commit "990:/path/rst" +' + +test_expect_success 'git hook --list orders by order number' ' + cat >expected <<-\EOF && + 010 global /path/def + 100 repo /path/ghi + 990 repo /path/rst + 999 global /path/uvw + EOF + + git hook --list pre-commit >actual && + test_cmp expected actual +' + +test_expect_success 'order number collisions resolved in config order' ' + cat >expected <<-\EOF && + 010 global /path/def + 010 repo /path/abc + 100 repo /path/ghi + 990 repo /path/rst + 999 global /path/uvw + EOF + + git config --add --local hook.pre-commit "010:/path/abc" && + git hook --list pre-commit >actual && + test_cmp expected actual ' test_done
Teach 'git hook --list <hookname>', which checks the known configs in order to create an ordered list of hooks to run on a given hook event. The hook config format is "hook.<hookname> = <order>:<path-to-hook>". This paves the way for multiple hook support; hooks should be run in the order specified by the user in the config, and in the case of an order number collision, configuration order should be used (e.g. global hook 004 will run before repo hook 004). For example: $ grep -A2 "\[hook\]" ~/.gitconfig [hook] pre-commit = 001:~/test.sh pre-commit = 999:~/baz.sh $ grep -A1 "\[hook\]" ~/git/.git/config [hook] pre-commit = 900:~/bar.sh $ ./bin-wrappers/git hook --list pre-commit 001 global ~/test.sh 900 repo ~/bar.sh 999 global ~/baz.sh Signed-off-by: Emily Shaffer <emilyshaffer@google.com> --- Documentation/git-hook.txt | 17 +++++++- Makefile | 1 + builtin/hook.c | 54 ++++++++++++++++++++++- hook.c | 81 +++++++++++++++++++++++++++++++++++ hook.h | 14 ++++++ t/t1360-config-based-hooks.sh | 43 ++++++++++++++++++- 6 files changed, 206 insertions(+), 4 deletions(-) create mode 100644 hook.c create mode 100644 hook.h