Support installation through git clone?

[matthijskooijman]

I'm looking at using githooks (not using it yet) and reviewing the install script, but it looks like the installation is a bit complicated, more than I would think is needed. I suspect that most of this might be so that install.sh becomes a self-contained file that can be run through a sh $(curl ...) command? However, I don't quite like that way of installing and updating stuff, I'd rather just clone this repository and update things by pulling. Is this, or can this be, a supported usecase?

I originally wanted to remark that the contents of base-template.sh are duplicated inside install.sh. Before I realized install.sh should be self-contained, it seemed sane to just let install.sh read base-template.sh at runtime, but that won't work here, I guess. I do see that there is a pre-commit hook that actually updates install.sh with the content from some other files.

Anyway, ignoring the self-contained scenario for a short while, here's how my ideal flow would look like:

• Clone this repo somewhere in my homedir, let's say ~/githooks
• Configure git (manually is fine) to point the hookdir (i.e. core.hooksPath) towards this checkout (probably a subdir, e.g. core.hooksPath = ~/githooks/hooks). Alternatively, you can point the hook template config value to this directory.
• Inside this subdir, there would be the base-template.sh file (maybe with better name) to do the actual work, and have small files for each supported hook, that do nothing more than calling base-template.sh (taking care not to change $0, or otherwise passing some variables like HOOK_NAME explicitly). This could be done with symlinks, but that is not portable to Windows I think.
• Then, symlink cli.sh into my $PATH so I can call that.

Putting it like this, this might mean this does not actually need the install script at all, so this approach could likely co-exist nicely with the existing auto-generated self-contained install script maybe?

[matthijskooijman]

I just tried this and it actually seems to work quite well so far. I:

• Created a git clone in ~/.local/etc/
• Created a ~/.githook/hooks subdirectory
• Inside that subdir, I ran:

  HOOKS="applypatch-msg commit-msg post-applypatch post-checkout post-commit post-merge post-receive post-rewrite post-update pre-applypatch pre-auto-gc pre-commit prepare-commit-msg pre-push pre-rebase pre-receive push-to-checkout sendemail-validate update"
  for i in $HOOKS; do ln -s ../base-template.sh $i;done

  This creates the hooks to point git at, which all just run the base-template through a symlink.

• Then: chmod a+x base-template.sh to make that executable (I think git needs that?).
• Then: git config --global core.hooksPath ~/.githooks/hooks/ to point git at the hooks.

With that, things seem to work fine. I suspect that there might be some details incorrect (e.g. $INSTALL_DIR is empty, so custom tools won't work), but overall this might be nice to officially support?

[gabyx]

I was also thinking about having the whole githooks project in the way as brew works would be nice. We just clone this repo (so install.sh would only basically clone this repo from a branch releases or so). So updating only means -> git fetch -> check if we can merge -> ask the user if we want to do so -> git merge -> done.

So far the install.sh is self-contained.

• base_template.sh gets copied for each hook into the git repo or it does not if we have set a core.hooksPath and we installed to use the core.hooksPath mode. Here might be room for improvement, by not copying the whole file, but this is another story...
• We bookkeep a "cat ~/.githooks/autoupdate/registered" list with all repos (which are not single installs) which will be updated.

The install.sh (which is used always, for update or for first time install) is more sophisticated as it will replace existing hooks in place, and this is a crucial point. Repositories can have different already existing hooks in .git/hooks (or ./hooksfor bare repos) (LFS hooks are a special example, they will be removed and considered internally), which will be taken care of by the install script.

Its also to mention that, we can install a custom download app dispatch git hooks tools help with witch we can override the download of the install.sh (for example when we want to get it from another fork (as it is my use-case)).

[gabyx]

I am just trying this out, ;-). See #86 This opt-in feature should to 95% be the version we should head towards, having a release checkout in the install folder... Note its now the default and only way in the PR, because it works :-)

[rycus86]

Thanks for raising this issue, and huge thanks @gabyx for the PR! It's quite big, so will have to find some time somewhere to review it properly, but sounds good as an option.

Before we do that though, I want to clarify what is the actual problem we're trying to solve? You can already clone this repo and run ./install.sh - what's the benefit of only allowing that? Alternative, what's wrong with the self contained install script?

[matthijskooijman]

Before we do that though, I want to clarify what is the actual problem we're trying to solve? You can already clone this repo and run ./install.sh - what's the benefit of only allowing that? Alternative, what's wrong with the self contained install script?

I guess it is that:

• I do not like big, somewhat opaque install scripts. Who knows where they will put things on my system, I'd rather be in control. This is not so much a matter of distrust or security - I'm going to run arbitrary code anyway, but more a matter of knowing what happens.
• I do not like working form untracked files. That is, the install script installs files into untracked directories. If I need to make a change to them, there is no quick "git diff" that tracks this change. There's no quick "git log" to see what version I'm using exactly, etc.

Both of these seem easily solved by actually using hooks directly from the git repo checkout, instead of using the install script to copy them.

I'll respond to @gabyx's comment in #86, which is probably more appropriate there.

[rycus86]

Thanks for clarifying things @matthijskooijman !

Who knows where they will put things on my system, I'd rather be in control

Yeah, it's definitely a trade-off between control and convenience. This install script is big, so it does what most users would expect, and you don't have to do anything manually except answering some prompts.

I do not like working form untracked files. That is, the install script installs files into untracked directories. If I need to make a change to them, there is no quick "git diff" that tracks this change.

The idea is that you never need to change these Git hooks, because the actual hook logic you need to change is inside your tracked repository.

There's no quick "git log" to see what version I'm using exactly, etc.

If you really do care about what githooks version of the base hook template you're running on, then have the version number at the start of the file.

Does this help at all?

[matthijskooijman]

The idea is that you never need to change these Git hooks, because the actual hook logic you need to change is inside your tracked repository.

This is true, but in practice I might make local changes anyway (such as an extra local dir for #82 or a fix for #84).

If you really do care about what githooks version of the base hook template you're running on, then have the version number at the start of the file.

That would only work if I adjust the version number or add a comment there whenever I make a local change, and still does not allow easily doing "git diff" to see what changed exactly.

I hate to sound stubborn, but the advantages of running code out of a git repo directly, rather than running from copies generated by a script, are real and significant to me. There are of course alternatives and I can get by without this, but given that it would be relatively easy to support this (alongside the current approach), I greatly prefer to do it like that. Having said that, I already did this somewhat manually and it works for me now, so I'm happy already. I hope you agree with my assessment and also think it is worth the effort to support this, but if you don't that's also fine (provided you don't change anything to break this :-p).

Also note that I said "alongside the current approach", which seems rather easy. Using it instead of the current approach (e.g. what #86 does, I guess) would also seem good to me, but is more work. I think it would allow significantly simplifying things, though.

[rycus86]

Thanks a lot for your feedback @matthijskooijman , really appreciate it!

Since you're making your own local changes anyway, perhaps it would make sense to have your own setup too? 🙂 What you describe is basically something like:

$ git clone git://...githooks.git /local/dir
$ mkdir -p /local/hooks && ln -s /local/dir/base-template.sh /local/hooks/pre-commit
$ git config --global core.hooksPath /local/dir

I have not tested any of the above, but you see what I mean hopefully. Then you could make your local changes in that local checkout in /local/dir and have it applied across all your local repos.

Also note that I said "alongside the current approach", which seems rather easy

Adding a new way of doing things is usually easy, supporting and maintaining many different variations is not so much. 🙂

Does this help you at all?

[matthijskooijman]

Since you're making your own local changes anyway, perhaps it would make sense to have your own setup too?

Yeah, what you describe is pretty much what I did already, see: https://github.com/rycus86/githooks/issues/83#issuecomment-614800948

That works, so like I said: I'm already happy here, I just think more people could be happy with this if it's officially supported :-)

[gabyx]

This will be somehow similarly supported with #86:

You install with curl .../install.sh --release-clone-url "<your-local-fork-or-whatever>" --release-clone-branch "my-super-duper-setup" which will make a clone in $INSTALL_DIR/release. If you make changes in there, it want be visible already. You make it in the cloned <your-local-fork-or-whatever> path and do a git hooks update which will propagate the changes... Its not so straight forward.

And just a awesome thought: With my comment about <protocol>:// in #82 it would be super awsome to also treat <your-local-fork-or-whatever> the exact same way. If it contains <protocol>:// we clone it in the install folder (we are under control) otherwise we use it directly and we never do modifications to this repository in any circumstance (no update of githooks). Meaning the user has to manually update

cd  <your-local-fork-or-whatever> && git pull 
git hooks update # to trigger the install ...

That feature could be implemented on top of #86 after a review and merge. This way your scenario is supported and even better we have a handling of urls which is consistent for every git repo we clone/pull/update... (githooks itself ;-) + global/local shared hooks)

[gabyx]

@rycus86 Please see my last comment in #86

[rycus86]

@rycus86 Please see my last comment in #86

Will do, but only later today.

[matthijskooijman]

I just left a review on #92. While reviewing, I noticed some other things, which probably related more to #86, or are otherwise possible improvements of the current code, not strictly related to #92, so I thought to post them here.

I noticed that cli.sh is installed by copying it into githooks/bin and then pointing a git alias at it. Why not just point the git alias at the original release/cli.sh instead? That saves code duplication, and ensures only running git tracked code?

I also wondered a bit about the global hook dir approach used now. It seems that the hook dir is treated similarly to a template dir (though I couldn't quickly see whether it actually installs all of the template stuff in that case, or just the hooks). However, it seems that this can maybe also add (or overwrite) hooks to an existing global hooks directory, which does not seem so useful? For templates, this makes sense, since there can be other things than hooks in there, so it makes sense to merge the hooks with the existing template stuff.

However, for global hooks, I would suggest to create the hooks directory in a fixed location, and pointing the git config variable at that location, completely ignoring (and not touching) any previously set location. In addition, if this fixed location would be a directory inside the repository, which is prefilled with wrapper scripts (as I keep nagging about), then global hooks installation is really simple and can skip all the (IMHO complex) template stuff. Maybe the template stuff can then become simpler as well when it no longer has to account for global hooks?

[rycus86]

I also wondered a bit about the global hook dir approach used now. It seems that the hook dir is treated similarly to a template dir (though I couldn't quickly see whether it actually installs all of the template stuff in that case, or just the hooks).

What do you mean by global hooks here? Do you mean global shared hook repos? Or the git core.hooksPath mode? Or something else?

[matthijskooijman]

Or the git core.hooksPath mode?

That one.

[rycus86]

That was a feature request a while back. In that mode, git will use the hooks from a folder this config points to, regardless of template directories and whatnot. We should be able to just point it to the pregenerated wrappers' folder eventually and that should work.

[matthijskooijman]

Yeah, exactly. But the current code does not seem to work like that yet, it still uses the template directory code to generate a core.hooksPath directory AFAICS. This is likely just a matter of "not gotten around to improving it yet", though, so we can just leave this for later I think.

[rycus86]

Hm, I can't remember the exact details now, but I thought it either uses the existing core.hooksPath directory or creates one if it does not exist, copies the hook files into it (basically copies of the base-template) and points the config at it. Not sure what else is there to improve on this?

The template dir install mode finds or creates a template directory, and copies the hook files (again copies of base-template) into its hooks subfolder IIRC, then points the init.templateDir at it. Git will then copy all the files from here into .git of new clones or git init repos.

[matthijskooijman]

Hm, I can't remember the exact details now, but I thought it either uses the existing core.hooksPath directory or creates one if it does not exist, copies the hook files into it (basically copies of the base-template) and points the config at it.

Well, using an existing directory and copying files into it seems potentially harmful: If that existing directory is not created by githooks, it could overwrite files. If you let git manage the hook directory (with pregenerated wrappers) and just point git at it, you'll not risk overwriting anything.

As for the current implementation, it seems that hooksPath mode is interwoven with template mode, e.g. https://github.com/rycus86/githooks/blob/8f26882b8636789ba9be547b54ff7de861deb347/install.sh#L342-L355

That looks for a template directory (this returns hooksPath if already set, but that is not the case on the initial install, then potentially returns a path in /usr/ if that turns out to be writable, or creating one in $INSTALL_DIR as a last resort), and then points hooksPath at it: https://github.com/rycus86/githooks/blob/8f26882b8636789ba9be547b54ff7de861deb347/install.sh#L1159-L1163

This seems terribly complicated for the hooksPath, approach, which really only ever needs to support a path inside the $INSTALL_DIR (and, if pregenerated, only inside the git checkout without even generating anything).

Thinking about this, I even wonder why this /usr searching happens at all? Previously, I thought this was to look for the default template directory so the files in there can be copied into a new custom templates directory, but it seems this actually installs into /usr, which is not really recommended (that's package manager territority, and prone to be overwritten on git updates).

[rycus86]

Well, using an existing directory and copying files into it seems potentially harmful: If that existing directory is not created by githooks, it could overwrite files. If you let git manage the hook directory (with pregenerated wrappers) and just point git at it, you'll not risk overwriting anything.

As for the current implementation, it seems that hooksPath mode is interwoven with template mode

I'm happy to review PRs when you find what doesn't work properly and needs correction. :)

[gabyx]

I just left a review on #92. While reviewing, I noticed some other things, which probably related more to #86, or are otherwise possible improvements of the current code, not strictly related to #92, so I thought to post them here.

I noticed that cli.sh is installed by copying it into githooks/bin and then pointing a git alias at it. Why not just point the git alias at the original release/cli.sh instead? That saves code duplication, and ensures only running git tracked code? I intend to doo that, once we get onwith deprecating single instal...

I also wondered a bit about the global hook dir approach used now. It seems that the hook dir is treated similarly to a template dir (though I couldn't quickly see whether it actually installs all of the template stuff in that case, or just the hooks). However, it seems that this can maybe also add (or overwrite) hooks to an existing global hooks directory, which does not seem so useful? For templates, this makes sense, since there can be other things than hooks in there, so it makes sense to merge the hooks with the existing template stuff. Its good to have the same behavior of placing hooks for the template dir as well as the hooks folders:

The thing is you dont know what users have done to their git repo and I would hardly adjust this behavior: So the status quo is not so bad I think: Renaming already existing stuff and then placing our symlink wrapper there...

However, for global hooks, I would suggest to create the hooks directory in a fixed location, and pointing the git config variable at that location, completely ignoring (and not touching) any previously set location. In addition, if this fixed location would be a directory inside the repository, which is prefilled with wrapper scripts (as I keep nagging about), then global hooks installation is really simple and can skip all the (IMHO complex) template stuff. Maybe the template stuff can then become simpler as well when it no longer has to account for global hooks?

I do not quite agree here, the user can already have set core.templateDir ahead of any githooks installation. Also the template stuff is ok IMO, we will only do wrappers in the future (symlink not possible because of platform), also pointing a --local git config core.hooksPath directly towards a our release folder $INSTALL_DIR/release is applicable in a simple setup, (but what would it be if its not git, git isn't simple lol), but I think is not the right choice: Better be not invasive and try to just place our template (a wrapper in the future) inside the hooks folder. Thats what we do so far.

I agree that some steps might be a little bit an overshoot and I am trying towards making githooks better, simpler, but without devestating to many features, "simple but not to simple" From a code and understanding perspective I have several concerns:

• dont repeat your self -> lots of copy past code
• wrong logical behavior with the update mechanism so far... (will get better), install.sh ist solely repsonsible for the update, not the template...
• etc...
• another thing is, if the community and the author also have little time to review these PRs (sometimes it needs more indepth time) I am getting more and more frustrated and progress slowes down significantly. The project in all has a pretty good setup. the testing works and is good (of course its bash, it sucks, but thats another problem). So thanks for your effort and work....

[matthijskooijman]

I intend to doo that, once we get onwith deprecating single instal...

Ok!

I do not quite agree here, the user can already have set core.templateDir ahead of any githooks installation.

Yes, so the templateDir handling would not change much. It necessarily needs to be more complex than the core.hooksPath handling. However, I'm suggesting that by splitting these two, the core.templateDir handling can become slightly simpler (since the code now has some special handling related to hooksPath, e.g. here and here), and maybe there are some other things that could be simplified). Then the core.hooksPath can become really simple: Just point core.hooksPath to a path within the $INSTALL_DIR (possibly pre-generated), without having to do anything at all with existing hooks or an existing hooksPath directory.

but I think is not the right choice: Better be not invasive and try to just place our template (a wrapper in the future) inside the hooks folder. Thats what we do so far.

I do not see what we would gain from this, other than complexity. I think you would never leave any existing hooks in place, at best you would rename them and then replace them with the githooks versions. Then if you do not use anything from the existing hooksPath, why not leave it untouched and redirect core.hooksPath instead? If the user does need some additional global hooks, they can easily just let githooks call them (especially if #82 is also implemented). Also, I would think that putting files into an existing folder actually is invasive, more than just pointing core.hooksDir at githooks.

Maybe some of my reasoning is influence by my perception that the core.templateDir code is quite complex and I do not fully understand it, though.

another thing is, if the community and the author also have little time to review these PRs (sometimes it needs more indepth time) I am getting more and more frustrated and progress slowes down significantly.

I can understand that. I would really love to also put in some actual work (and with all the suggestions and criticisms, I feel a little obliged to do so), but with all the projects and work I already have, I really can't commit to more than advice and limited review... Hence I'm all the more grateful of the great work you've been putting in so far!

And even though we seem to disagreeing on certain things, I'm convinced we're both just looking for the best/simplest/easiest/nicest/cleanest/feasible solutions. In that sense I rather enjoy disagreeing with people, since that usually leads to better understanding and better solutions :-)

[gabyx]

One thing about hooks leaving in place an such is also the thing about the git LFS hooks, they get installed by git directly. These are 100% crucial. If you delete them (most novice git users do not know about hooks and LFS clearly) you forget to push LFS objects on the the LFS server... I will think about your suggestion again, to little time now. Maybe I understand then better what yu mean.

[matthijskooijman]

One thing about hooks leaving in place an such is also the thing about the git LFS hooks, they get installed by git directly.

Ok, good example. But how does that work right now? Aren't these existing hooks also replaced by the githooks base-template.sh? Git can only run a single hook of each type, right (which is why this project exists in the first place)?

But thinking about this more, I can imagine an approach where you repoint core.hooksDir as I suggested, but then put the old core.hooksDir in githooks.shared to make sure the old hooks are still being ran (this probably needs #82 and maybe also change the code to allow a single hook file per type, rather than a directory named after the hook type with multiple hooks for that type, if this is not already supported).

[gabyx]

Hmm, right now I am thinking if we dont have the single install stuff anymore: why not only use core.hooksPath for all cases. ** no templates. Wouldnt that be possible? For already placed hooks inside .git/hooks we can directly execute them too if we implement such a thing (configurable per repo, ), also no lfd stuff is needed since by default we execute repo hooks... simple and covers almost anything. what did i forget??

@rycus86 Think about it. :)

Von meinem iPhone gesendet

Am 24.05.2020 um 10:06 schrieb Matthijs Kooijman notifications@github.com:

One thing about hooks leaving in place an such is also the thing about the git LFS hooks, they get installed by git directly.

Ok, good example. But how does that work right now? Aren't these existing hooks also replaced by the githooks base-template.sh? Git can only run a single hook of each type, right (which is why this project exists in the first place)?

But thinking about this more, I can imagine an approach where you repoint core.hooksDir as I suggested, but then put the old core.hooksDir in githooks.shared to make sure the old hooks are still being ran (this probably needs #82 and maybe also change the code to allow a single hook file per type, rather than a directory named after the hook type with multiple hooks for that type, if this is not already supported).

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub, or unsubscribe.

[rycus86]

I'm quite happy with Git's init.templateDir approach, you'll get the templates in new repos as long as you wish, but it's easy to opt out. It also means people will find the thing that is going to be executed in the .git/hooks directory, which is what a quick web search would most likely suggest, so it would be the least surprising to me.

Dealing with these through core.hooksPath globally takes away something for me on configurabilty/opt-in I feel, and setting it locally per repo is pretty much the same. So while I don't think I can name anything wrong with core.hooksPath, I prefer the template dir approach. Once the hooks that get copies are simply wrappers to point back to the central checkout, the only issue I have with them goes away, which is potential staleness.

[matthijskooijman]

I personally would not want to use the templateDir approach (I like the centrally managed hooksPath better), but I can see the value of it, so I would also think keeping both makes sense.

[gabyx]

Ok, thanks for the summary and reasoning @rycus86. IMO: both are valuable.

@matthijskooijman : LFS hooks are completey supported by githooks it self. They get move and disabled if detected in the .git/hooks folder. disable_lfs_hook_if_detected()

Your suggestion about core.hooksPath and putting the old into the global shared directory results in seperate code path were as the current logic is exactly the same for templateDir and hooksPath, rename all hooks -> done. Therefore I like this more. It covers both cased and you have the same behavior/code I value personally more than having an optimized version as you suggest, because that would only account for the core.hooksPath if I understood correctly.

[matthijskooijman]

@matthijskooijman : LFS hooks are completey supported by githooks it self. They get move and disabled if detected in the .git/hooks folder. disable_lfs_hook_if_detected()

Ah, I see that now. It seems that any existing LFS-hooks get replaced by githooks, and then githooks just runs lfs unconditionally for all git repos that use githooks (provided git-lfs is available). So that means that my earlier suggestion about adding an existing core.hooksPath as a global/shared hooks dir entirely might actually be counter-productive: If the lfs hooks are installed into that existing directory, they would then run twice (but I guess the existing lfs-hook-detection could be used to skip those hooks instead, maybe...).

I guess this is currently done somewhat like this, by deleting lfs hooks or renaming them to .disabled.githooks, unlike all other hooks which are renamed to .replaced.githooks and still being ran if found.

Your suggestion about core.hooksPath and putting the old into the global shared directory results in seperate code path were as the current logic is exactly the same for templateDir and hooksPath, rename all hooks -> done. Therefore I like this more. It covers both cased and you have the same behavior/code I value personally more than having an optimized version as you suggest, because that would only account for the core.hooksPath if I understood correctly.

Yeah, it seems that things are a bit more involved than I thought. With things like LFS being treated specially, my "simple hooksPath" approach would become a bit less simple, and start to duplicate some parts of the template code again, maybe. Also, I had not realize that there is already code to run .replaced.githooks hooks, so there wouldn't be any gain there either.

I still think that there is merit in a no-install scenario, where hooks are pre-generated and githooks can be installed by cloning and setting core.hooksPath without needing the install.sh script at all (i.e. only run git-tracked githooks code without needing any copies at all), but I can see might lead to a bit more complexities than I originally thought, unfortunately.

[gabyx]

@matthijskooijman : Awesome: Actually with #111 , I have now added a folder hooks to the repo with all hooks (modified base-template-symlink.sh) git supports. In this way a simple install by cloning and setting core.hooksPath becomes reality. Could you please check the last commit

[matthijskooijman]

With #111 merged, it seems the original request of this issue is solved. To install githooks without any install script, you can now:

I just tried this and it actually seems to work quite well so far. I:

• Create a git clone of this repositorty, say as ~/.githooks by running git clone git@github.com:rycus86/githooks.git ~/.githooks
• Then point git at it using: git config --global core.hooksPath ~/.githooks/hooks/.

Done, githooks is now installed and runs for all git repos on your machine.

I only did some very casual testing (pushed a ref, it ran my hooks as expected), though.

Before closing this issue, I guess:

• Some tests need to be added
• This approach should be documented

If this is indeed an approach that you want to officially support, I can maybe give a go at documenting it?

[gabyx]

Jeah sounds great. If you are willingly, why not also add some tests. Maybe look at the newer ones above > 100. Hm... :> We loose the ability for the cli.sh tool. Maybe we can use the one in the repo?, I think there might be some problems regarding the clone&update procedure which will result in a clone suddenly located at ~/.githooks/release...?

Maybe we can add another variable git config --global githooks.releaseCloneDir which by default points to $(git config --global githooks.installDir)/release if not set. So one can override this variable after cloneing, to be able to use the cli.sh appropriately hm....

[gabyx]

@matthijskooijman Do you give it a go at documenting this feature? :-) Woule be great