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.