Re: [RFC] Hook management via 'git hooks' command

["brian m. carlson" <sandals@crustytoothpaste.net>]

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

On 2019-11-26 at 00:56:14, Emily Shaffer wrote:
> Hopefully I am not beating a dead horse here but I really want to
> understand. Let me take another guess with examples at what you mean;
> please correct me!
> 
> For our purposes, let's assume:
> 
> .git/hooks/
>   update
>   update.d/
>     update-1
>     update-2
> 
> update:
>   #!/bin/bash
> 
>   ./git/hooks/update.d/update-1 &&
>   ./git/hooks/update.d/update-2
> 
> The goal is to make sure update-1 and update-2 are run when other update
> hooks happen.



> With my proposal as is, I see two options:
> 
> 1)
> .git/config:
>   hook.runHookDir = true
> 
> This will run whatever else is in hook.update, and then it will run
> .git/hooks/update. This is not ideal because hookDir could change, and
> then the content of update will be incorrect:
> 
>   git config --add core.hookdir=~/hooks/
>   mv .git/hooks/update* ~/hooks/
>   # call something which invokes update hooks
>   # ~/hooks/update fails, .git/hooks/update-1 is gone :(
> 

This is actually fine.  We assume the user knows where they want to
store their hooks.  If they change that, then that's intentional.

> .git/config:
>   hook.update = 001:/project/path/.git/hooks/update.d/update-1
>   hook.update = 002:/project/path/.git/hooks/update.d/update-2
> 
> This will run each update hook individually and have no notion of
> whether they're in a "hook dir" or not. It sees a path, it hands it to
> 'sh' to run, it checks the return code.

Correct.  This is the logical porting of the above shell script to the
config syntax if you use an absolute path.

> Now I run 'mv .git/hooks/update* ~/hooks/'. Next time I invoke something
> which would run the 'update' hook, it fails, because the path in the
> config isn't pointing to anything. But this way is unrelated to hookdir.

Yes, that's correct.

It sounds like you're thinking of the config approach as completely
orthogonal to the hook directory.  However, I want to have multiple
per-repository hooks that do not live within the repository but move
with it.  The logical place to store those hooks is in the hook
directory, even if it's not being invoked by Git explicitly.  To use
that with the config approach so I can have multiple hooks in a useful
way that's compatible with other tools, I need some way to refer to
whatever the hook directory is at a given point in time.

> It sounds like you might be asking for something like:
> 
> .git/config:
>   hook.update = 001:__HOOKDIR__/update.d/update-1
> 
> I'm not sure that I like the idea of this. My own dream is to eliminate
> the need for a hookdir entirely, so it's logically easy for my Gerrit
> hook to live in ~/gerrit-tools/ and my linter to live in ~/clang-tidy/
> and so on.

Using the config syntax eliminates per-repository storage for hooks.  I,
personally, want to store multiple one-off hooks in my hooks directory.
I want to use tools that install one-off hooks into my repository
without needing to depend on the location of those tools in the future.
I don't want those hooks to live elsewhere on my file system, since that
makes my repository no longer self contained.

I want to store those hooks in the hook directory, wherever I've
configured that, and whatever that happens to be at this point in time.

I may additionally have tools that live in other locations as well and
may use other syntaxes to invoke them.  For example, I may install a
hook that's provided by a Debian package and refer to it using a bare
program name.

If your goal is to eliminate the hook directory entirely, then I can't
say that I'm in support of that.  A design which does that won't meet my
needs, and it likely won't meet the needs of other users.

The benefit of your proposed config syntax for me is that it provides a
standard way to configure multiple hooks.  I still want many of those
hooks to live in the hook directory, although others may live elsewhere.

> I could see option 1 being alleviated by writing it as something like:
> 
> update:
>   $GIT_HOOKDIR/update.d/update-1 &&
>   $GIT_HOOKDIR/update.d/update-2
> 
> or
> update:
>   $(git config core.hookdir)/update.d/update-1 &&
>   $(git config core.hookdir)/update.d/update-2

This is similar to what I want, and why I want to pass it to the shell.
I can write the following, and everything just works:

.git/config:
  hook.update = 001:$(git config core.hookdir || echo .git/hooks)/update.d/update-1
  hook.update = 002:$(git config core.hookdir || echo .git/hooks)/update.d/update-2

Wherever I have placed my hooks for this repository, I can refer to
them with a shell script.  I can also add the following line as well:

.git/config:
  hook.update = debian-package-hook update

…and everything just works.

> But, I think once you "buy in" to the idea of providing a full path to
> the final target - not to a trampoline script - in your config, you
> should forget about the idea of "core.hookdir" having anything to do
> with it.

I have no intention of providing a full path to any hook, since that's
quite brittle.  There are very few paths on my system which can be
guaranteed not to change, and all of them are things like /bin/sh or
/usr/bin/env.  If my hooks are in the hook directory (or even in the
working tree) with a full path and I move that repository, they become
broken.  If they're shipped by Debian and Debian decides to move the
command, they're broken.

I'm very interested in learning more about why you seem to want to
specify full paths.  It seems very at odds with the way the rest of Git
works.  If the goal is to support other syntaxes in the future, then
let's use a prefix character (e.g. !) to denote commands vs.
non-commands or something like that.  If the goal is security, then I'd
like to hear more about the security model you're trying to achieve with
this design.

> To quote you out-of-order now:
> 
> > If we want to allow people to have multiple hooks under something like
> > .git/hooks/pre-push.d, then we need to have a way to wire them up in the
> > configuration using the correct location of the hook directory instead
> > of using a helper script like the one I linked above.
> 
> I think I may spot the misunderstanding. It sounds like you hope someone
> can provide "hook.update=001:~/update.d/" and have all the contents of
> update.d executed. I'll be clear and say that I didn't have the
> intention to support that at all; instead I was hoping to support a case
> like 2. above. Recursing through directories like that sounds scary to
> order.

I don't need a particular way to do that, since I can do it already, but
I do need a way to wire up multiple hooks that are per-repository and
move with the repository but aren't in the repository.  In other words,
I need a functional replacement for that approach if we're not going to
use that approach itself.

Hopefully I've done a better job at explaining myself here.
-- 
brian m. carlson: Houston, Texas, US
OpenPGP: https://keybase.io/bk2204

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