[PATCH] git: move non-kallithea hooks and execute other hooks

[Tim Ooms]

Hi Mads,

Also thanks for having a look.

On 22/10/2020 19:31, Mads Kiilerich wrote:
> On 10/20/20 5:19 PM, Tim Ooms wrote:
>> # HG changeset patch
>> # User Tim Ooms <tatankat at users.noreply.github.com>
>> # Date 1603201903 -7200
>> #      Tue Oct 20 15:51:43 2020 +0200
>> # Node ID 672e57b165d0c1774b692b5706a174bf98f42e4c
>> # Parent  b9b53e25a08d3714c54d82641b419e6d01820e12
>> git: move non-kallithea hooks and execute other hooks

> It might have rough edges to bypass Kallithea when accessing 
> Kallithea-managed repos. It is perhaps better to go with the assumption 
> that Kallithea fully owns the repositories it manage.

I tried to not bypass Kallithea by letting it execute the other hooks. I 
know the mercurial hooks are more integrated, but as the git hooks don't 
work the same way I think this is the next best thing. Although there 
may be better ways, I'm not sure.

> Can you say more about your use case? What other hooks are installed? 
> What precautions do they use to avoid clobbering the Kallithea hooks?

Use cases I think of:
post:
* trigger CI build (like jenkins)
* notify bug tracker (like the bug genie)
pre:
* check for commit message
* verify syntax/codestyle
Other examples: 
https://scriptrunner.adaptavist.com/stash/3.0.14-beta1/docs/pre_receive_hooks/
These hooks are really independent and don't touch the repository. If 
they fail, it has no effect on the other hooks. A hook can mess up if it 
just hangs and does not return. But you will always have this if people 
are using custom hooks, even in mercurial.

> It must be a general challenge with Git to fit multiple hooks into the 
> single hook slot. Is there any prior art for solving / working around 
> that problem?

Yes, there is:
* solution using "pee": 
https://git.seveas.net/using-multiple-post-receive-hooks.html
* using bash built-ins: 
https://serverfault.com/questions/909153/how-to-add-multiple-post-receive-hooks-to-a-git-repository

> It would be nice if there was a "standard" dispatcher (perhaps a simple 
> shell script) that could be installed in The Hook Location - something 
> that picked up multiple hook files. Instead of having our hook call 
> others, it would perhaps be better to install a generic-ish hook and let 
> it dispatch to both our hook and the existing one.

I would indeed like a shell solution (I used the bash solution till now, 
but till now that causes problems when doing an update), but that ain't 
portable and I think Kallithea can run on Windows. It can have 
advantages to let Kallithea run the other hooks: they can be managed (in 
a later stage) by Kallithea, can be logged by Kallithea and admins can 
be notified if a hook fails to run.
Also, running directly the Kallithea hook, Kallithea -being the one 
managing the repository- is sure that its hook is correctly called and 
it's shebang is correctly set and executable.

This can also be done using something generic-ish (and I like the idea), 
but you add another layer that can fail and another file to keep up to 
date like the existing hook.

> The solution proposed here also touch upon lessons learned from general 
> /etc/xxx.rc/ configuration systems: People will often want to rename 
> configuration files to something like '.bu' or '.old' or '.disabled'. It 
> is thus a very good idea to only match on one file extension - and 
> perhaps also a prefix. The "real" hooks do not have any extension, but 
> the globbed ones could perhaps match 'pre-receive.*.hook'? Also, we 
> should check that they are executable for the current user before trying 
> to run them.

Nice catches! That would indeed improve the robustness. A try-catch-log 
would probably also be in place, so we always try all hooks.
For pre-receive hooks, we should also check the return code of all hooks 
and return non-zero if one of them does not return 0. For post-receive, 
only the Kallithea return code matters.

> For the chmod changes, I think we only should care about the hooks we 
> install. If the existing hook isn't executable, git will ignore it and 
> we should do the same - not try to clean things up we don't know about. 
> We just wrote our own hook file, so we know it isn't a symlink, and 0755 
> should be fine? Ideally, we should probably use umask and set X bit ... 
> but I doubt that ever will make any difference.

I only change the permissions of the Kallithea hook. The check for the 
symlink is because the Kallithea hook could have been changed by an 
admin to a symlink. Kallithea then logs an error about not being able to 
write the hook, while in fact the hook was updated, only the mode could 
not be changed.

The umask can make a difference if you let your web administrators not 
run under the web server's user, but they should be able to manage the 
hooks. That is why you would like to have g+w set and not be removed by 
the software.

-- 
kind regards,
Tim
-------------- next part --------------
A non-text attachment was scrubbed...
Name: tim_ooms.vcf
Type: text/x-vcard
Size: 4 bytes
Desc: not available
URL: <http://lists.sfconservancy.org/pipermail/kallithea-general/attachments/20201023/dfe36aac/attachment.vcf>