On Tue, May 14, 2019 at 7:24 AM brian m. carlson <sandals@xxxxxxxxxxxxxxxxxxxx> wrote: > -int run_hook_ve(const char *const *env, const char *name, va_list args) > +int find_hooks(const char *name, struct string_list *list) > { > - struct child_process hook = CHILD_PROCESS_INIT; > - const char *p; > + struct strbuf path = STRBUF_INIT; > + DIR *d; > + struct dirent *de; > > - p = find_hook(name); > - if (!p) > + /* > + * We look for a single hook. If present, return it, and skip the > + * individual directories. > + */ > + strbuf_git_path(&path, "hooks/%s", name); > + if (has_hook(&path, 1, X_OK)) { > + if (list) > + string_list_append(list, path.buf); > + return 1; > + } > + > + if (has_hook(&path, 1, F_OK)) > return 0; > > - argv_array_push(&hook.args, p); > - while ((p = va_arg(args, const char *))) > - argv_array_push(&hook.args, p); > - hook.env = env; > + strbuf_reset(&path); > + strbuf_git_path(&path, "hooks/%s.d", name); > + d = opendir(path.buf); > + if (!d) { > + if (list) > + string_list_clear(list, 0); > + return 0; > + } > + while ((de = readdir(d))) { > + if (!strcmp(de->d_name, ".") || !strcmp(de->d_name, "..")) > + continue; > + strbuf_reset(&path); > + strbuf_git_path(&path, "hooks/%s.d/%s", name, de->d_name); > + if (has_hook(&path, 0, X_OK)) { Do we want to support hooks in subdirectories as well (if so, using dir-iterator.h might be more appropriate) If not, what happens when "path" is a directory. X_OK could be set (and often are) on them too. > + if (list) > + string_list_append(list, path.buf); > + else > + return 1; > + } > + } > + closedir(d); > + strbuf_reset(&path); > + if (!list->nr) { > + return 0; > + } > + > + string_list_sort(list); This is going to be interesting on case-insensitive filesystems because we do strcmp by default, not the friendlier fspathcmp. And the ".exe" suffix might affect sort order too. But I suppose we just need to be clear here (in documentation). They can always prefix with a number to keep hook files in expected order. > + return 1; > +} > + > diff --git a/run-command.h b/run-command.h > index a6950691c0..1b3677fcac 100644 > --- a/run-command.h > +++ b/run-command.h > @@ -4,6 +4,7 @@ > #include "thread-utils.h" > > #include "argv-array.h" > +#include "string-list.h" 'struct string_list;' should be enough (and a bit lighter) although I don't suppose it really matters. > > struct child_process { > const char **argv; -- Duy