git@vger.kernel.org list mirror (unofficial, one of many)
 help / color / Atom feed
From: "brian m. carlson" <sandals@crustytoothpaste.net>
To: Emily Shaffer <emilyshaffer@google.com>
Cc: git@vger.kernel.org, "Jonathan Nieder" <jrnieder@gmail.com>,
	"Ævar Arnfjörð Bjarmason" <avarab@gmail.com>
Subject: Re: [RFC] Hook management via 'git hooks' command
Date: Sat, 16 Nov 2019 05:45:01 +0000
Message-ID: <20191116054501.GA6538@camp.crustytoothpaste.net> (raw)
In-Reply-To: <20191116011125.GG22855@google.com>

[-- Attachment #1: Type: text/plain, Size: 4230 bytes --]

On 2019-11-16 at 01:11:25, Emily Shaffer wrote:
> Here's my suggestion.
> 
>  - Let configs handle the hook list and ordering, via 'hook.{hookname}' which
>    can be specified more than once.
>    - global config: hook.update = /path/to/firsthook
>      user config: hook.update = /path/to/secondhook
>      worktree config: hook.update = -/path/to/firsthook #eliminate the firsthook
>       call
>    - global config: hook.update = /path/to/firsthook
>      repo config: hook.update = /path/to/secondhook
>      repo config: hook.update = ^/path/to/firsthook #move firsthook execution
>        after secondhook for this project

I'd like to hear more about how we handle multiple hooks that are
repo-specific and don't live in the PATH.  This is a common situation
for existing tools that handle multiple hooks, and it's one I think we
should support.

Perhaps we add a "git hook execute" subcommand that executes scripts
from the hook directory.

>  - Let users decide whether they want to check core.hookDir in addition to their
>    config, instead of their config, or not at all, via a config
>    'hook.runHookDir' "hookdir-first", "hookdir-only", "config-only", etc.
>  - Let users ask to be notified if running a hook from .git/hooks via a config
>    'hook.warnHookDir'. (Mentioning .git/hooks rather than core.hookDir is
>    intentional here.) Alternatively, ask for 'hook.warn' with values "hookdir",
>    "all" depending on how trusting the user feels.
>    - If we want to phase out .git/hooks, we can make this notification opt-out
>      instead of opt-in, someday.
>    - between runHookDir and warnHookDir, users are able to phase out
>      .git/hooks scripts at their own pace.
>  - Abstract most of this via a 'git hooks' command.
>    - 'git hooks add <hookname> [--<scope>] <path>' to append another hook;
>      let the scope setting match 'git config'.
>    - 'git hooks show [<hookname>]' to indicate which hooks will be run, either
>      on a specific event or for all events, and in which order, as well as each
>      hook's config origin
>    - 'git hooks edit <hookname>' to modify the hook order, or choose not to run
>      a hook locally
>    - By managing it through a command, we can reorder the contents of config
>      files if it makes sense to do so.
>  - Add a hook library.
>    - Optionally specify all hook events via enum, so we aren't string-typing
>      calls to find_hook() anymore.
>    - Resolve configs into a list of hooks to run by concatenating them together
>      in config precedence order.
>      - Also allow configs formatted like "-<path>" to remove an
>        earlier-configured invocation of that hook, or "^<path>" to move the
>        earlier-configured invocation to this point in the execution order
>    - Move the find_hook() implementation to here, to account for the multihook
>      ordering

I think this addresses most of the concerns that I had about ordering.
It is still a little suboptimal that we're relying on the ordering of
the config file, since it makes things equivalent to numbered files in
.d directories hard.

Possibly as an alternative to the ^ syntax, we could make the hook value
be of the form "key program", where key is a sort key (e.g., a number)
and program is the program to run.  We pick normal config file ordering
if the keys are identical.  Then if the system config wants to have a
hook that always runs at the end, it can do so easily.

In addition, we should be sure that attempting to remove a hook which
doesn't exist isn't an error, since a user might want to set their
~/.gitconfig file to always unset a global hook that may or may not
exist.

We also need to address exit codes with multiple hooks and whether we
fail fast or not.  My series may provide some valuable options here, or
we may choose to go with a single, simpler approach.  Whatever we do, we
should document the behavior clearly.

Overall, though, I think this approach has a lot of potential and I feel
positive about it.  Thanks for bringing this up again in a productive
and helpful way.
-- 
brian m. carlson: Houston, Texas, US
OpenPGP: https://keybase.io/bk2204

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 868 bytes --]

  reply index

Thread overview: 12+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-11-16  1:11 Emily Shaffer
2019-11-16  5:45 ` brian m. carlson [this message]
2019-11-18 22:38   ` Emily Shaffer
2019-11-19  0:51     ` brian m. carlson
2019-11-23  1:19       ` Emily Shaffer
2019-11-25  3:04         ` brian m. carlson
2019-11-25 22:21           ` Emily Shaffer
2019-11-25 22:45             ` Emily Shaffer
2019-11-26  0:28             ` brian m. carlson
2019-11-26  0:56               ` Emily Shaffer
2019-11-26  2:41                 ` brian m. carlson
2019-12-02 23:46                   ` Emily Shaffer

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

  List information: http://vger.kernel.org/majordomo-info.html

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20191116054501.GA6538@camp.crustytoothpaste.net \
    --to=sandals@crustytoothpaste.net \
    --cc=avarab@gmail.com \
    --cc=emilyshaffer@google.com \
    --cc=git@vger.kernel.org \
    --cc=jrnieder@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

git@vger.kernel.org list mirror (unofficial, one of many)

Archives are clonable:
	git clone --mirror https://public-inbox.org/git
	git clone --mirror http://ou63pmih66umazou.onion/git
	git clone --mirror http://czquwvybam4bgbro.onion/git
	git clone --mirror http://hjrcffqmbrq6wope.onion/git

Example config snippet for mirrors

Newsgroups are available over NNTP:
	nntp://news.public-inbox.org/inbox.comp.version-control.git
	nntp://ou63pmih66umazou.onion/inbox.comp.version-control.git
	nntp://czquwvybam4bgbro.onion/inbox.comp.version-control.git
	nntp://hjrcffqmbrq6wope.onion/inbox.comp.version-control.git
	nntp://news.gmane.io/gmane.comp.version-control.git

 note: .onion URLs require Tor: https://www.torproject.org/

AGPL code for this site: git clone https://public-inbox.org/public-inbox.git