mbox series

[RFC,v2,0/2] MVP implementation of remote-suggested hooks

Message ID cover.1626453569.git.jonathantanmy@google.com (mailing list archive)
Headers show
Series MVP implementation of remote-suggested hooks | expand

Message

Jonathan Tan July 16, 2021, 5:57 p.m. UTC
Here's an updated version of the remote-suggested hooks RFC, hopefully
addressing some of the concerns people have brought up (e.g.
auto-updating without letting the user verify, or prompting about hooks
by default). It consists of two main parts:

 - Non-interactive prompts during certain Git commands about the
   existence of hooks. These prompts are turned *off* by default.

 - New "git hook" subcommands that can install these hooks (so that the
   aforementioned Git prompts or out-of-band installation instructions
   can tell users to install these hooks in a platform-independent way).
   These subcommands work whether or not prompts are enabled.

You can see how they work in patch 2's t1361.

In doing this, I have tried to scope down the overall effort as much as
possible (to avoid security risks as much as possible, among other
things), concentrating on situations that server-side hooks cannot
handle. I think the main thing is that server-side hooks can check and
prompt upon push, but they cannot catch problems that the client could
have noticed upon commit; if the end user pushes a commit chain and only
notices problems then, the user would have to redo the commits (possibly
through an interactive rebase on the spot). Catching problems upon
commit would prevent this problem, only necessitating amending the
commit.

In the Gerrit use case, Gerrit requires a specific "Change-Id" trailer
in the commit message, but I can foresee the same issue in projects that
require their commit messages to fit a certain template or projects that
want lints to be run but their builds, for some reason, don't run
arbitrary code (or they have no builds at all).

To that end, I have added a prompt for the "commit-msg" hook if the
remote has suggested one, but there are none currently installed.
(Similar prompts could be added for other commit-related hooks.) The
other prompts are upon clone and upon fetch (if the hooks have been
updated).

Jonathan Tan (2):
  hook: move list of hooks
  hook: remote-suggested hooks

 builtin/bugreport.c               |  38 +-----
 builtin/clone.c                   |  12 ++
 builtin/fetch.c                   |  17 +++
 builtin/hook.c                    |  17 ++-
 hook.c                            | 185 +++++++++++++++++++++++++++++-
 hook.h                            |   6 +
 t/t1361-remote-suggested-hooks.sh | 105 +++++++++++++++++
 7 files changed, 340 insertions(+), 40 deletions(-)
 create mode 100755 t/t1361-remote-suggested-hooks.sh

Comments

Junio C Hamano July 19, 2021, 9:06 p.m. UTC | #1
Jonathan Tan <jonathantanmy@google.com> writes:

> Here's an updated version of the remote-suggested hooks RFC, hopefully
> addressing some of the concerns people have brought up (e.g.
> auto-updating without letting the user verify, or prompting about hooks
> by default). It consists of two main parts:
>
>  - Non-interactive prompts during certain Git commands about the
>    existence of hooks. These prompts are turned *off* by default.
>
>  - New "git hook" subcommands that can install these hooks (so that the
>    aforementioned Git prompts or out-of-band installation instructions
>    can tell users to install these hooks in a platform-independent way).
>    These subcommands work whether or not prompts are enabled.
>
> You can see how they work in patch 2's t1361.

I really wanted to try this out, but which commit was this based on?

> In doing this, I have tried to scope down the overall effort as much as
> possible (to avoid security risks as much as possible, among other
> things), concentrating on situations that server-side hooks cannot
> handle. I think the main thing is that server-side hooks can check and
> prompt upon push, but they cannot catch problems that the client could
> have noticed upon commit; if the end user pushes a commit chain and only
> notices problems then, the user would have to redo the commits (possibly
> through an interactive rebase on the spot). Catching problems upon
> commit would prevent this problem, only necessitating amending the
> commit.
>
> In the Gerrit use case, Gerrit requires a specific "Change-Id" trailer
> in the commit message, but I can foresee the same issue in projects that
> require their commit messages to fit a certain template or projects that
> want lints to be run but their builds, for some reason, don't run
> arbitrary code (or they have no builds at all).
>
> To that end, I have added a prompt for the "commit-msg" hook if the
> remote has suggested one, but there are none currently installed.
> (Similar prompts could be added for other commit-related hooks.) The
> other prompts are upon clone and upon fetch (if the hooks have been
> updated).
>
> Jonathan Tan (2):
>   hook: move list of hooks
>   hook: remote-suggested hooks
>
>  builtin/bugreport.c               |  38 +-----
>  builtin/clone.c                   |  12 ++
>  builtin/fetch.c                   |  17 +++
>  builtin/hook.c                    |  17 ++-
>  hook.c                            | 185 +++++++++++++++++++++++++++++-
>  hook.h                            |   6 +
>  t/t1361-remote-suggested-hooks.sh | 105 +++++++++++++++++
>  7 files changed, 340 insertions(+), 40 deletions(-)
>  create mode 100755 t/t1361-remote-suggested-hooks.sh
Jonathan Tan July 20, 2021, 8:49 p.m. UTC | #2
> Jonathan Tan <jonathantanmy@google.com> writes:
> 
> > Here's an updated version of the remote-suggested hooks RFC, hopefully
> > addressing some of the concerns people have brought up (e.g.
> > auto-updating without letting the user verify, or prompting about hooks
> > by default). It consists of two main parts:
> >
> >  - Non-interactive prompts during certain Git commands about the
> >    existence of hooks. These prompts are turned *off* by default.
> >
> >  - New "git hook" subcommands that can install these hooks (so that the
> >    aforementioned Git prompts or out-of-band installation instructions
> >    can tell users to install these hooks in a platform-independent way).
> >    These subcommands work whether or not prompts are enabled.
> >
> > You can see how they work in patch 2's t1361.
> 
> I really wanted to try this out, but which commit was this based on?

Ah, good question. Like v1, it's based on the old es/config-based-hooks
(f2e1003b62 ("docs: link githooks and git-hook manpages", 2021-05-27)).
In v3, I'll rebase this on ab/config-based-hooks-base instead,
especially since that one has a programmatically generated list of hooks
(instead of a hardcoded one).