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

[Tim Ooms]

Hi Thomas

Thanks for looking into this.

On 20/10/2020 22:12, Thomas De Schampheleire wrote:
> 
> El mar., 20 oct. 2020 a las 21:12, Tim Ooms (<tim.ooms at aeronomie.be 
> <mailto:tim.ooms at aeronomie.be>>) escribió:
> 
>     The goal of the patch is to not break the non-kallithea hook scripts
>     that may be present when doing a rescan and reinstall (after an update
>     for example) and at the same time supporting the execution of
>     third-party hook scripts next to the kallithea ones.
> 
>     When using the force parameter, instead of overwriting the
>     non-kallithea
>     hook script, it renames the script by adding
>     .<date>.<kallithea-version>
>     to it (can off course be something else).
> 
>     The new version of the kallithea hook script will execute all
>     scripts in
>     the hook-directory matching <hook-name>.*.* (should just match previous
>     pattern). This way, it executes the previously installed hook script
>     and
>     possible other hook scripts the admin added matching the pattern.
> 
> 
> Thanks for the background.
> In general, we encourage contributors to write clear commit messages 
> with all necessary info to understand the what/why/how of changes. So 
> info like the above would be great to have in such commit message.

I thought it too big for a commit message. Will do in the future.

> For these changes, I think they can be seen as two separate things and 
> should therefore better be two separate patches:
> - support running additional git hooks in addition to the kallithea one
> - rename existing git hooks rather than overwriting them, when 'force' 
> is given.

In fact, it can be 3 separate patches:
1) support running additional git hooks
2) rename existing hooks
3) keep default given permissions of created hook and add any possible 
necessary permission bits

> I'm not sure if the second part is really necessary: it should normally 
> only occur once, the code adds quite some complexity for such mundane 
> task of renaming a file. Perhaps it could be enough to have clear enough 
> documentation or messages?
> Let's see what Mads Kiilerich thinks.

For now, it occurs every update of Kallithea or its venv's python. But 
indeed, if additional hooks are supported, it should occur only once. In 
my opinion, if you do support additional hooks, you get a safe overwrite 
by moving for free.
If you take out the other commits, log messages and comments, the only 
stuff that remains for the renaming is:
if other_hook:
     moved_hook_file = name based on date and version
     os.rename
So I don't think that's much complexity?

> For the first part, I can see that it can be useful.
> See also the comments on following proposed change, for a recent 
> discussion regarding the git hooks and how it is implemented internally:
> https://kallithea-scm.org/repos/kallithea-incoming/changeset/c63dd3aeacdbbd77349bb2b9100dee5ba0c3fda6

What I thought of while creating the patch is that I would create a 
single template for pre & post-receive to lower the code duplication.
When using a wrapper in venv/bin will the correct python be executed 
automatically? Otherwise this (still) has to be changed (too). This can 
simply be done while "copying", no?

-- 
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/20201021/1f8c0564/attachment.vcf>