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

From
Emily Shaffer <hidden>
Date
2021-07-27 00:57:39
Reply to
Jonathan Tan, 2021-07-20
On Tue, Jul 20, 2021 at 02:48:09PM -0700, Jonathan Tan wrote:
quoted
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.
quoted
...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.
quoted
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).
I think this is a pretty important point. To me, the ideal flow looks
like this:

 - I clone some repo, planning to just use the source code. I ignore the
   hook prompt.
 - I notice some bug which is within my power to fix. I have forgotten
   about the hook prompt, because I was having so much fun using the
   source code in the repo.
 - I 'git commit' - and 'git commit' says, "Did you know this repo
   suggests installing a commit-msg hook? You can install it by running
   'git hook install pre-commit' and run it by running 'git commit
   --amend --no-edit'. You can audit the commit-msg hook by running 'git
   hook magic-audit-command-name-tbd'. You can hide this advice <typical
   advice-hiding advice here>."

That way I don't add privilege (tell my computer it's OK to execute code
I didn't look at) until the very possible moment. This workflow also
captures changing intentions - I did not clone the code intending to
contribute back, but at the moment my intentions changed, I was nudged
to answer differently to a question I was asked with different earlier
intentions. That use case isn't easy to capture with a MOTD, unless you
run one on push, at which point it may be too late (e.g. if while fixing
I also accidentally uploaded my oauth password, and now it'll live
forever on GitHub in infamy).

MOTD approach also makes it hard to *update* hooks when the maintainer
so recommends - would be nice to have something baked in to notice when
there are new changes to the hooks, so we hopefully don't have
developers running hook implementations correlating to the date they
most recently cloned the project.

 - Emily
Thread (25 messages) 25 messages, 7 authors, last reply 2021-07-27
Keyboard shortcuts
hback out one level
jnext message in thread / next series-diff position
kprevious message in thread / previous series-diff position
ldrill in
Escclose help / fold thread tree
?toggle this help