Re: [RFC PATCH v2 2/2] hook: remote-suggested hooks

["Ævar Arnfjörð Bjarmason" <avarab@gmail.com>]

On Fri, Jul 16 2021, Jonathan Tan wrote:

> Teach the "git hook install all|<hook-name>" command, that can install
> one or all remote-suggested hooks.
>
> If a configuration option hook.promptRemoteSuggested is set, inform the
> user of the aforementioned command:
>
>  - when cloning, and refs/remotes/origin/suggested-hooks is present in
>    the newly cloned repo
>  - when fetching, and refs/remotes/origin/suggested-hooks is updated
>  - when committing, there is a remote-suggested commit-msg hook, and
>    there is currently no commit-msg hook configured
>
> NEEDSWORK: Write a more detailed commit message once the design is
> finalized.

This is a bit orthagonal to what you're going for I guess, so sorry in
advance about the "but what about" bikeshedding you must be getting
tired of by now...

> @@ -1393,6 +1393,18 @@ int cmd_clone(int argc, const char **argv, const char *prefix)
>  			   branch_top.buf, reflog_msg.buf, transport,
>  			   !is_local);
>  
> +	if (hook_should_prompt_suggestions()) {
> +		for (ref = mapped_refs; ref; ref = ref->next) {
> +			if (ref->peer_ref &&
> +			    !strcmp(ref->peer_ref->name,
> +				    "refs/remotes/origin/suggested-hooks")) {
> +				fprintf(stderr, _("The remote has suggested hooks in refs/remotes/origin/suggested-hooks.\n"
> +						  "Run `git hook install all` to install them.\n"));
> +				break;
> +			}
> +		}
> +	}
> +
>  	update_head(our_head_points_at, remote_head, reflog_msg.buf);
>  
>  	/*
> diff --git a/builtin/fetch.c b/builtin/fetch.c
> index 769af53ca4..e86c312473 100644
> --- a/builtin/fetch.c
> +++ b/builtin/fetch.c
> @@ -28,6 +28,7 @@
>  #include "promisor-remote.h"
>  #include "commit-graph.h"
>  #include "shallow.h"
> +#include "hook.h"
>  
>  #define FORCED_UPDATES_DELAY_WARNING_IN_MS (10 * 1000)
>  
> @@ -1313,6 +1314,22 @@ static int consume_refs(struct transport *transport, struct ref *ref_map)
>  				 ref_map);
>  	transport_unlock_pack(transport);
>  	trace2_region_leave("fetch", "consume_refs", the_repository);
> +
> +	if (hook_should_prompt_suggestions()) {
> +		struct ref *ref;
> +
> +		for (ref = ref_map; ref; ref = ref->next) {
> +			if (ref->peer_ref &&
> +			    !strcmp(ref->peer_ref->name,
> +				    "refs/remotes/origin/suggested-hooks") &&
> +			    oidcmp(&ref->old_oid, &ref->peer_ref->old_oid)) {
> +				fprintf(stderr, _("The remote has updated its suggested hooks.\n"));
> +				fprintf(stderr, _("Run 'git hook install all' to update.\n"));
> +				break;
> +			}
> +		}
> +	}
> +
>  	return ret;
>  }

...but this part makes me think that if this is all we're aiming for as
far as server-client interaction is concerned we'd be much better off
with some general "server message-of-the-day" feature. I.e. server says
while advertising:

    version 2
    agent=...
    # does protocol v2 have a nicer way to encode this in the capabilities? I think not...
    motd=tellmeaboutref:suggested-hooks;master

Client does, while handshake() etc.:

    # other stuff
    command=ls-refs
    ....
    0000
    # Get motd from server
    command=motd
    0001
    refat suggested-hooks $SUGGESTED_HOOKS_AT_OID
    refat master $MASTER_AT_OID
    0000

And server says, after just invoking a "motd" hook or whatever, which
would be passed the git version, the state of any refs we asked politely
about and the client was willing to tell us about etc.

    Hi, we've got suggested hooks in this repository, it seems:
    if $agent > $min_git_version
        you have a supported git version, great....
    else
        <sadtrombone> you might want to upgrade your git to....
    fi

We could even carry this specific message in git.git, but under the hood
it would be the equivalent of a default 'motd' hook you could enable.

Maybe where you're going with this precludes such a MOTD approach.

FWIW I think there's lots of use-cases for it, and this specific hook
case is just one, so if we could make it slightly more general & just
make this a special-case of a generally useful facility.

Even for your use-case it would be useful, e.g. the whole discussion
we've been having about should the hooks by in a magic ref or your
current branch or not.

With a motd hook it doesn't matter, you just make "git hook install"
support installing hooks from whatever rev/tree, and a combination of
the "tellmeaboutref" and that feature means you can pick one or the
other, or tell users they need to install <some custom dependency> first
or whatever.