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

From: Jonathan Tan <jonathantanmy@google.com>
Date: Jul 20, 2021, 21:48 UTC
Message-ID: <20210720214809.3596513-1-jonathantanmy@google.com>
In-Reply-To: <87o8awvglr.fsf@evledraar.gmail.com>

> 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...

No - thanks for taking a look. More ideas are always welcome.

Show 9 quoted lines

> ...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

Right now we don't have a way in capabilities to include arbitrary strings, although we can extend it if needed.

Show 16 quoted lines

> 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.

Ah...so the main difference is that it is the server that computes whether a message is shown, based on information provided by the client (different from my patches wherein the client computes whether a message is shown).

I'm not sure how this is better, though. We don't need to build another mechanism to print server messages (since it can already do so - the same way it sends progress messages), but then we lose things like translatability, and we have to build another endpoint for the server ("command=motd").

Also, one thing to think about is that we want to be able to prompt users when they run hook-using commands (e.g. "commit"). With my patches, the necessary information is stored in a ref but with your idea, we need to figure out where to store it (and I think that it is not straightforward - I'd rather not use config or extra files in the .git directory to store remote state, although if the Git project is OK with doing this, we could do that).

Show 13 quoted lines

> 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.

True - we avoid the discussion by having essentially a new namespace of name-to-OID mappings. I still think it's simpler to use refs of some sort for this, though. (Also, even if we use a new sort of name-to-OID mapping, we need to make a ref for this OID so that it will be fetched, so we might as well use a ref for this.)