rycus86 / githooks

Githooks: per-repo and global Git hooks with version control
MIT License
382 stars 20 forks source link

Automatically move and rename non-githooks hooks #155

Open mjk-gh opened 3 years ago

mjk-gh commented 3 years ago

I am just at the level of exploring githooks without having it used fully yet, so maybe this is a bit early to raise an issue, but here it goes anyway. :)

I noticed after installation of githooks, that -- as documented -- the githooks hooks are installed from the freshly generated git template directory after I do a "git init". If I now change any of the installed hooks in my local repository with something else and do a "git hooks update/install", githooks will recognize that one of its hooks have been overwritten and renames it to something that won't be executed.

Why not do the following instead:

Move the non-githooks-hook to

$GIT_DIR/.githooks/<original-name-of-hook>/<NEW_NAME>

where NEW_NAME can be either generated to not collide with the other hooks installed in that githooks subdirectory and/or get entered by the user:

Enter new name for the pre-commit hook: 04-pre-commit

-- with 04-pre-commit having been generated and pre-filled at the libreadline-supported prompt. And if you're really nice, you'll let the prompt have a (at least temporary) history, so that when some hook (like LFS (which is treated specially, I know)) gets installed for 4 different events, the user doesn't have to type "super-duper-hook" 4 times. Maybe you can check for existence of "rlwrap".

It would also be nice if githooks would somehow detect overwritten githooks hooks as early as possible (probably only possible by making "git" a wrapper script).

Hopefully I haven't overlooked that all of this is long possible -- already spent an hour on reading the docs, but githooks is quite a complex thing, despite it purportedly being "a simple script". :)

P.S.: Thanks for all the work you already put into this project! At least from the description, it seems to be the best of all projects dealing with these kinds of git hooks problems. And if you plan on rewriting this thing Go, then I'll just install the last bash version and never update. :-P (Reason: Go is not installed by default by many systems, also: one of the "competing" git hooks projects required certain versions of "Go", which strongly reminded me of "Best viewed with Internet Explorer v1.2.3", which doesn't sound like a desirable approach).

rycus86 commented 3 years ago

Hello @mjk-gh

Thanks for the suggestion and the detailed description!

githooks will recognize that one of its hooks have been overwritten and renames it to something that won't be executed.

I just had a look because I didn't remember and apparently the old hook will still be executed, see https://github.com/rycus86/githooks/blob/master/base-template.sh#L191-L196

I kind of like the idea of moving it to a more visible place instead of being hidden there, but it would also mean putting something into the repository that the user will have to decide if they want to check it in or not. Not saying that's bad, but a choice to make there I guess.

I think it might make sense for something like this to exist, but I'm not super sure about the UX of it all, maybe something like git hooks install --migrate-hooks or something then we'd do this copying business, not super keen on asking too many questions there though, so I'm interested in ideas if you have some.

if you plan on rewriting this thing Go

There's actually a port that @gabyx is working on and is available for testing in https://github.com/gabyx/githooks - and I don't think it needs any special dependencies to be installed locally. Have a look if you feel like it, I'm going to keep this original version around as well, might just not have as much time for new features as @gabyx would have. :)

gabyx commented 3 years ago

@mjk-gh Thanks for the input!

Guess what, I already thought about your proposed feature, but the points which @rycus86 already pointed out, have arised in my thoughts as well.

A command git hooks install --migrate-hooks would be feasible I think, and the best option. The most crucial point is, that the user somehow decided to overwrite the hook inside .git/hooks and possibly doesnt want it to be "checked in". It will however still be executed which is crucial. I think if the user uses Githooks one should never really deal with the .git/hooks folder anyway (like overwritting), and your troubles which you experienced, will not arrive, if they arrive, IMO the user is either deeply understanding all Git hooks issues/caveats/technics or the user doesnt know what he/she is doing or she/he got lost in the documentation.

I have to think again about the above command, if it is sensful to support this. It seems so.

@mjk-gh : The GO port does not need anything except a running OS which can execute the built executable. Its compiled! Go is not a script language and we dont use go run or something like that.