mbox series

[v3,0/6] propose config-based hooks

Message ID 20200728222455.3023400-1-emilyshaffer@google.com (mailing list archive)
Headers show
Series propose config-based hooks | expand

Message

Emily Shaffer July 28, 2020, 10:24 p.m. UTC
Hi all,

After taking a few weeks to work on other items, I've got another update
to the config-based hook series. Patches 5 and 6 are RFC - a sketch of
how the hook library could run the appropriate set of hooks. There's
more work to do, which I'll outline later in the cover letter.

Since last time, I took into account review comments, including Dscho's
fixups to make the tests work in Windows. It seems those tests are
passing now, according to the GH Actions run:
https://github.com/nasamuffin/git/actions/runs/186242637

One thing I didn't decide on was the benefit of moving the hookcmd
resolution outside of the hook config pass; that code is unchanged. I
still haven't decided quite which approach I like better, but it's still
on my mind.

In the 'run_hook()' implementation I flipped the 'use_shell' bit, which
by my understanding only uses a shell if it can't find the command in
PATH; this seems like a reasonable approach especially because the code
is so brief, but I'm interested in hearing why I'm wrong or it won't
work well :)

There is still some work I've got locally which isn't quite ready:
 - support for hook.runHookDir. This is turning into a yak shave about
   who decides where and when to display or run the hookdir hook. I
   think I've got it mostly figured out and there's a patch locally, but
   it's not polished.
 - Drafts for 'git hook add' and 'git hook edit'. These features are
   probably the most complicated part of the series, but it's possible
   to use config-based hooks without them. In the interest of getting
   something out for people to try on their own, I'll probably leave
   these for later.
 - Support for stdin redirection to hooks. Since this means we want to
   point the same stdin to multiple processes, I'm thinking it will be
   slightly complicated. Maybe someone has a hint for me? :) Without
   having looked at what's available or not yet, I'm planning to do this
   by reading the whole stdin to memory and then streaming it to each
   process in turn, as I can't seek back to the beginning of the stream
   when I start each new process.
 - Conversion of codebase to use the hook library instead. Partly, this
   is gated on the previous point - there are plenty of callers who,
   instead of using run-command's run_hook_*(), just use find_hook() and
   roll their own struct child_process so they can use stdin/stdout. I
   do plan to consider the hook lib's run_hooks() implementation as
   non-final until I start this process - I'm expecting to learn more
   about what I do and don't have to support when I do this.

Thanks, all. Hopefully I can do better than a 2-month wait for the
series after this one... although I imagine I cursed myself by saying
that. :)

 - Emily


Emily Shaffer (6):
  doc: propose hooks managed by the config
  hook: scaffolding for git-hook subcommand
  hook: add list command
  hook: add --porcelain to list command
  parse-options: parse into argv_array
  hook: add 'run' subcommand

 .gitignore                                    |   1 +
 Documentation/Makefile                        |   1 +
 Documentation/git-hook.txt                    |  63 ++++
 Documentation/technical/api-parse-options.txt |   5 +
 .../technical/config-based-hooks.txt          | 354 ++++++++++++++++++
 Makefile                                      |   2 +
 builtin.h                                     |   1 +
 builtin/hook.c                                | 107 ++++++
 git.c                                         |   1 +
 hook.c                                        | 132 +++++++
 hook.h                                        |  18 +
 parse-options-cb.c                            |  16 +
 parse-options.h                               |   4 +
 t/t1360-config-based-hooks.sh                 | 115 ++++++
 14 files changed, 820 insertions(+)
 create mode 100644 Documentation/git-hook.txt
 create mode 100644 Documentation/technical/config-based-hooks.txt
 create mode 100644 builtin/hook.c
 create mode 100644 hook.c
 create mode 100644 hook.h
 create mode 100755 t/t1360-config-based-hooks.sh