existing git hooks get destroed on install & update

[wommel]

Q	A
Version	0.12.0
Bug?	yes
New feature?	no
Question?	no
Documentation?	no
Related tickets

My configuration

# you may change this settings by copy this file to "grumphp.yml" #
###################################################################
parameters:
    bin_dir: "./vendor/bin"
    git_dir: "."
    hooks_dir: ~
    hooks_preset: local
    stop_on_failure: false
    ignore_unstaged_changes: false
    process_async_limit: 10
    process_async_wait: 1000
    process_timeout: 60
    ascii:
        failed: config-dev/dopeman.txt
        succeeded: grumphp-happy.txt
    tasks:
        git_blacklist: ~
        git_commit_message:
            matchers:
                - /((MVP|FIN|VER)-([0-9]+))|#/
            case_insensitive: true
            multiline: true
            additional_modifiers: ''
        git_conflict: ~
        phpcsfixer2:
            allow_risky: false
            cache_file: ~
            config: ".php_cs.dist"
            using_cache: false
            path_mode: ~
            verbose: true
            triggered_by: ["php", "php5"]
        phpmd:
            exclude: []
            ruleset: ["config-dev/phpmd.xml.dist"]
            triggered_by: ["php", "php5"]
        phpversion:
            project: "5.6"
    testsuites: []
    extensions: []

Steps to reproduce:

• have a git repo with outdated or without grumphp installed
• modify/create the pre-commit hook
• update/install grumphp

Result:
the existing pre-commit hook gets bluntly replaced with the grumphp one.

i would expect grumphp to tell me about this conflict. and also not to replace the existing hook but to put the new one to $HOOKS_DIR/pre-commit.grumphp (or at least backup the old one to HOOKS_DIR/pre-commit.old).

same problem exists for other hooks grumphp uses. (i don't know if it is config dependent which hooks get set but with my config it is commit-msg as well)

[veewee]

Hi @wommel,

That is indeed the way it is currently implemented. Feel free to provide a backup mechanism.

[SerkanYildiz]

Hi, what kind of solution do we want here, running multiple git hooks or back up the old one, and insert grumphp git hooks? I am currently working on this, the solution I have now is:

1. Save the grumphp hooks for ex: $HOOKNAME.d/$HOOKNAME.grumphp.sh
2. append to .git/hooks/$HOOKNAME : sh $HOOKNAME.d/$HOOKNAME.grumphp.sh

Which one do you prefer @veewee
/ping @Landerstraeten

[veewee]

I think its a bit overkill to keep backups since the hooks are replaced every time grumphp updates.
Maybe its best to check if the hook file exists.

• If it exists and if it contains the keyword "grumphp": no backup is needed
• If it exists and does not contain the keyword "grumphp": Create a backup of the git hook

It would be very cool if the existing hook could be automatically added to the grumphp shell tasks. (We probably only want to add pre-commit hooks)

[sanmai]

Best way as I see it would be:

• User-installed hooks must be kept installed side by side with grum's. It should never delete anything it does not own. Not only because that's the rule, but also because Git hooks are usually not under version control, which means if they're gone, they're gone. Okay, there are backups, but that's a major hassle all the same. Not everyone does them.

  Imagine someone doing deploys from CI in some peculiar fashion. Deploys from CI are usually done from a hook, and if GrumPHP removes that hook, it can be a major disaster.

• Hooks installed by GrumPHP must be the very bare-bones. And they must stay same as long as possible. This way GrumPHP will be able always tell if that's its own hook, or that's a user-installed hook. Right now GrumPHP's hooks are fairly elaborate.

One way to accomplish this is to introduce a hook-manager concept.

#!/bin/bash

# Runs all executable hookname-* hooks and exits after,
# if any of them was not successful.
#
# Based on
# http://osdir.com/ml/git/2009-01/msg00308.html

data=$(cat)
exitcodes=()
hookname=$(basename $0)

# Run each hook, passing through STDIN and storing the exit code.
# We don't want to bail at the first failure, as the user might
# then bypass the hooks without knowing about additional issues.

for hook in $GIT_DIR/hooks/$hookname-*; do
	echo "$hook"
  test -x "$hook" || continue
  echo "$data" | "$hook"
  exitcodes+=($?)
done

# If any exit code isn't 0, bail.

for i in "${exitcodes[@]}"; do
  [ "$i" == 0 ] || exit $i
done

This file is fairly long, but it isn't going to change between versions. That, unless Git changes a lot in a new version, but that's going to break all hooks all the same, and not like that's going to happen.

With this "githook manager" in place, GrumPHP's will rename original hooks into *-stock, and install own as *-grumphp. Everybody's happy!

[veewee]

@sanmai : did you know that grumphp has a built-in shell task which does exactly what you've pasted here? You only need to configure which shell scripts to run.

The way I see it, there is no need for such feature since you allready have a lot of options to choose from:

• Don't want grumphp to overwrite your hook? Fine: run composer install with --no-plugins or --no-scripts
• Want grumphp to install git hook? Fine: run composer install
• Want to keep your existing hook AND grumphp: Fine: add your existing git hook to git and configure the Shell task to run that existing hook as well.
• Dont want to share your existing hook AND use grumphp: Fine: keep the shell file inside your .git/hooks directory and use the grumphp.yml.dist file and a local grumphp.yml file instead. In your local file you can configure the shell task to run the existing hook.

I think the backup logic is a bit of overkill which just causes maintenance overhead.

[sanmai]

Yeah, right. This is irresponsible and and reckless. I do make backups, but what if someone does not? And you just want to ruin all their hooks with one composer install? Do you think that's cool, that they're be a lot happier from this? Please reconsider. At very least grumphp should ask about existing hooks, and keep backups.

This discussion just caused me to

chmod a-w .git/hooks/*

And I'm looking if I can block my composer from ever installing grumphp anywhere.

[veewee]

Since git hooks arent stored in git, you should always have a backup somewhere. If you don't, you will be screwed at some point. I am open for discussion on the topic, but lets keep it strictly objective.

[sanmai]

The problem is that GrumPHP is supposed to be used by all folks who can't be "following the best practices you've determined as a team" from the start. If they can't code right guess if the do backups. No, they don't.

[veewee]

The idea of this package is to create one easy to configure git hook for everybody contributing to the project. From that point of view it doesn't make sense to have a custom git hook locally. Can you explain to me why you still want to use your own git hook? What is in there that can't be added to grumphp instead?

[sanmai]

I see no problem in using shell task or what GrumPHP can use to run a script.

The problem is that GrumPHP won't make backups of user-installed hooks.

Imagine someone who didn't know this package existed, and they had own hooks to do this and that. That, for example, which won't let one to commit on the master. Or that doing some project-specific testing. Or doing post-CI deploys. Hooks can have many uses.

And then you do composer require --dev phpro/grumphp and, boom, all your precious hooks are gone. And then you push your project to a CI server, and, boom, hooks are gone there too. And then you sit scratching your head because your project does not deploy. Because you can't do CI without developer dependencies. Or it wasn't you who did composer require --dev phpro/grumphp but your careless colleague. See what am I talking about?