Michael Haggerty venit, vidit, dixit 05.09.2012 17:30: > On 09/05/2012 03:39 PM, Michael J Gruber wrote: >> git-merge does not honor the pre-commit hook when doing automatic merge >> commits, and for compatibility reasons this is going to stay. >> >> Introduce a pre-merge hook which is called for an automatic merge commit >> just like pre-commit is called for a non-automatic merge commit (or any >> other commit). > > What exactly is an "automatic merge commit"? Is it any merge that > doesn't have a conflict? A merge that doesn't invoke the editor? A > merge done as part of another operation (e.g., pull)? I don't see the > term mentioned in the git-merge or githooks man pages. > > I think it would be good if you would define this term in the > documentation files that your patch touched, and perhaps in the githooks > section about "pre-commit" as well. "git merge" can go three ways: F: fast forward: no commit is created, only a ref is changed A: automatic: true merge (non-ff) without conflicts (i.e. chosen strategy can perform the merge); a new commit is created C: merge with conflicts: no commit is created but the index is prepared (partially) for a merge commit In case F, no commit hook is run (talking only about pre-commit/pre-merge). In case A, no commit is run so far but my patch proposes pre-merge to be run. In case C, pre-commit (!) is run so far and after my patch. > Secondly, though it is impossible (for backwards compatibility reasons) > for the pre-commit hook to be invoked for automatic merges, no such > considerations prohibit the pre-merge commit from being invoked for > non-automatic merges. In other words, both hooks, pre-commit *and* > pre-merge, could be invoked for non-automatic merges. Would this be > preferable? > > It depends on what pre-merge scripts are likely to be used for. If they > will tend to be used for merge-specific actions, then it might be more > convenient for *all* merges to be vetted by them. On the other hand, if > they tend to do the same actions as pre-commit hooks, then having > non-automatic merge commits go through both hooks would tend to be more > annoying than helpful. Specifically, one of the scripts would probably > have to check whether the merge is a non-automatic merge, and if so do > nothing (i.e., letting the other script take care of it). This would > also require an easy way for a script to determine whether a commit is a > non-automatic merge commit. > > Have you considered this? Your second paragraph explains why I did it the way I did. One can easily have pre-merge call pre-commit, or have them be different. One can not easily have only pre-merge called for a non-automatic merge commit, but that is because of backward compatibility. The way *I* would like it is: - call pre-merge for any non-ff merge commit (automatic or not) - call pre-commit for any non-merge commit (#parents <=1) But that would break compatibility. So I hope my patch is the best approximation to the above which keeps compatibility and is simple to handle in most situations. Cheers Michael -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html