Re: RFC: Supporting .git/hooks/$NAME.d/* && /etc/git/hooks/$NAME.d/*

From: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Date: Apr 26, 2016, 10:58 UTC
Message-ID: <CACBZZX6AYBYeb5S4nEBhYbx1r=icJ81JGYBx5=H4wacPhHjFbQ@mail.gmail.com>
In-Reply-To: <xmqq4mapmvjq.fsf@gitster.mtv.corp.google.com>

On Mon, Apr 25, 2016 at 7:45 PM, Junio C Hamano <gitster@pobox.com> wrote:

Show 10 quoted lines

> Ævar Arnfjörð Bjarmason <avarab@gmail.com> writes:
>
>> The reason for supporting the *.d directories was that I spotted a lot
>> of hooks people had hacked up at work using the pee(1) command[1] to
>> run sequences of other unrelated hook commands.
>
> IIRC, we wanted to do this several years ago but after discussion
> decided that we didn't want to have this in the core, because we
> didn't want to hardcode the policy on interaction among multiple
> hooks.

Ah, would be interesting to see that discussion if someone knows enough to dig it up, didn't find it with some brief searching.

> You can easily resolve the ordering of hooks--just declare that they
> are executed sequentially in strcmp() order of filenames and users
> will know to prefix them with fixed-number-of-digits to force their
> desired ordering without complaining.

In principle you're describing glob() order here: http://pubs.opengroup.org/onlinepubs/7908799/xsh/glob.html

We don't set LC_COLLATE in git so this'll be C order, which I think will just fall back to strcmp().

If it doesn't and there's a functional difference, I'm not sure, it's probably less confusing to use glob() order, since that's what you'll get with shell expansion.

I.e. it would be confusing if you expand the hooks in the shell, and git executes them in a slightly different order.

Show 9 quoted lines

> What is harder and the core part cannot unilaterally dictate is what
> should happen after seeing a failure/rejection from a hook.  Some
> hooks among the remainder would not want to be even called.  Some
> others do want to be called but want to learn that the previous
> hooks already have decided to fail/reject the operation.  There may
> even be some others that cannot be moved to earlier part of the hook
> chain for other external constraints (e.g. side effect of some
> previous hook is part of its input), but would want to override the
> previous decision to reject and let the operation pass.

I think it's fair enough to say that if we had this facility this would be good enough:

 * Your hooks are executed in glob() order, local .git first, then /etc/git/...

 * If it's a hook like pre-commit that can reject something the first
hook to fail short-circuits. I.e. none of the rest get executed.

 * If it's not a hook like that e.g. post-commit all of the hooks will
get executed.

 * If you need anything more complex you can just wrap your hooks in
your own shellscript.

I.e. it takes care of the common case where:

 * You just want to execute N hooks and don't want to write a wrapper.

 * For pre-* hooks the common case is it doesn't matter /what/
rejected things, just that it gets rejected, e.g. for pre-receive.
Also if you care about performance you can order them in
cheapest-first order.

Show 10 quoted lines

> I am happy to see that the idea brought back alive again, but I
> think we prefer this start its life clearly marked as "highly
> experimental and subject to change", then invite interested and
> brave users who tolerate backward incompatible changes to
> experiment, in order to allow us to gauge what the right semantics
> and flexibility the users would want.  One way to do so may be an
> opt-in configuration variable e.g. "experimental.multiHooks";
> another may be to implement the logic as a pair of scripts (one for
> the command line argument variant, the other for stdin variant) and
> ship them in contrib/.

Makes sense to have an experimental.* config tree for git for stuff like this.

> The latter approach (i.e. scripting) might be easier for people to
> experiment and tweak, and in the olden days that would certainly be
> the approach would would have taken, but I am not too afraid of
> appearing uninviting to casual scripters anymore these days, so...

Yeah, actually one thing I didn't think of is that the core.hooksPath patch I submitted makes this rather trivial to implement as a collection of scripts...