Template Dir Discussion and Improvements

[gabyx]

I looked again over the tests 113/114. I really don't understand what is tested in 113 especially (114 is strange too) ?

# run the install, and select installing hooks into existing repos
echo 'n
y
/tmp/test113
' | sh /var/lib/githooks/install.sh --template-dir ~/.githooks/templates || exit 3

# check if hooks are inside the template folder.
if ! sh /var/lib/githooks/cli.sh list | grep test-hook; then
    echo "! Hooks were not installed successfully"
    exit 4
fi

First, I dont understand why git hooks list would be a sufficient test if the hooks are placed in ~/.githooks/templates. I think its not...

Second, when you add

ls -al ~/.githooks/templates
exit 1

you see that all hooks are directly installed into ~/.githooks/templates. and no subfolder hooks is created. There is some missconception that --template-dir is directly the path where the hooks are installed, that should not be, since init.templateDir is also not in this manner. the --template-dir path should be the same as init.templateDir meaning its the template directory as git describes. Any other redefinition for --template-dir does not make sense and is only hard to understand for the user.

There is a little mess with the install folder, when to create/when not, and when to add hooks and when not. All that I tried to address in #71 , where I cleaned the control paths such that it avoids bugs and unforeseen behavior. Can you please have a look at it again in detail.

[rycus86]

First I couldn't remember either, but then I worked it out by following the existing logic and adding tests. The --template-dir is a directory for Githooks to write its own template files to, it is NOT necessarily the same as git config --global init.templateDir .

If you don't pass this in, Githooks might use init.templateDir if it's set and has a hooks directory. (see https://github.com/rycus86/githooks/blob/master/install.sh#L3724)

If you do pass it in, the installer will use that directory to copy the templates to (and from) but it won't mess with any of your existing Git config (see https://github.com/rycus86/githooks/blob/master/install.sh#L3692), e.g. with the next install you'll have to specify --template-dir again or let Githooks create a new template directory (which will be set to init.templateDir) - see https://github.com/rycus86/githooks/blob/master/install.sh#L3753 or https://github.com/rycus86/githooks/blob/master/install.sh#L3897

So in principle:

• if you pass in a template dir, we'll use that for reading stuff
• if you don't and you want it (you said yes interactively), then we'll use that as consent to create folders and change global Git config

Does this make any more sense this way?

[gabyx]

Well, a little bit :-).

If you do pass it in, the installer will use that directory to copy the templates to (and from) but it won't mess with any of your existing Git config

• I dont understand, the fact that here you dont have the init.templateDir set, How should that work when you do git init, the hooks are not installed. Isn't that strange? Basically where is the copy from statement yiou mentioned in the code. git hooks install uses install.sh --single, as well as triggered updates use install.sh which does not use the folder where githooks installed its templates..., they will be install/overwritten again into a "found" folder ($TARGET_TEMPLATE_DIR).
• If we pass --template-dir, for what use is it then to place them in a different folder which is (in the current implementation) forgotten and not used again, because init.templateDir is not touched.

I think we should not try to change git config values, that for sure. But wouldnt it be better to really support the following:

The find mechanism is basically ok, but:

• If we might use init.templateDir (meaning it exists) -> we really force create (mkdir) the hooks directory, because thats where the stuff should be.
• --template-dir is just a folder where the hook templates get installed, has nothing to do with init.tempalteDir. This is confusing, can we make this simpler? Whats the use case of using this option, but not setting init.tempalteDir nor core.hooksPath?

So in principle:

• if you pass in a template dir, we'll use that for reading stuff -> Dont get that point?
• if you don't and you want it (you said yes interactively), then we'll use that as consent to create folders and change global Git config 👍 Agree!

Uninstall:: Suggestion: Call remove_existing_hook_templates over all possible template dirs not just one which is found. Its cheap and safer.

Note: https://github.com/rycus86/githooks/blob/master/install.sh#L3753
the if does not make sense, as the function does not return a clear return value... (?)

[rycus86]

Ah yes, excellent points! I think we should start by converting these points into failing tests, then it should be easier to fix them.

[gabyx]

I would suggest the following changes:

You can also run the installation in non-interactive mode with the command below. This will determine an appropriate template directory (detect and use the existing one, or use the one passed by --template-dir, or use a default one), install the hooks automatically into this directory, and enable periodic update checks.

adding somthing like:

If you use --template-dir <path>, you either

• have init.templateDir unset (in which case it will be set to <path>) or pointing to the same dir <path>. Then, the path <path> will be created.

or

• for the case you use --use-core-hookspath: have core.hooksPath unset (in which case it will be set to <path>/hooks) or pointing to <path>/hooks. Then, the path <path>/hooks will be created.

In any other case the install with --template-dir will fail, since we do not alter already set different init.templateDir nor core.hooksPath variables.