* [PATCH 0/3] pre-merge-hook @ 2012-09-05 13:39 Michael J Gruber 2012-09-05 13:39 ` [PATCH 1/3] git-merge: Honor pre-merge hook Michael J Gruber ` (3 more replies) 0 siblings, 4 replies; 16+ messages in thread From: Michael J Gruber @ 2012-09-05 13:39 UTC (permalink / raw) To: git The pre-commit hook is often used to ensure certain properties of each comitted tree like formatting or coding standards, validity (lint/make) or code quality (make test). But merges introduce new commits unless they are fast forwards, and therefore they can break these properties because the pre-commit hook is not run by "git merge". Introduce a pre-merge hook which works for (non ff, automatic) merges like pre-commit does for commits. Typically this will just call the pre-commit hook (like in the sample hook), but it does not need to. Michael J Gruber (3): git-merge: Honor pre-merge hook merge: --no-verify to bypass pre-merge hook t7503: add tests for pre-merge-hook Documentation/git-merge.txt | 2 +- Documentation/githooks.txt | 7 +++++ Documentation/merge-options.txt | 4 +++ builtin/merge.c | 15 ++++++++- t/t7503-pre-commit-hook.sh | 66 ++++++++++++++++++++++++++++++++++++++- templates/hooks--pre-merge.sample | 13 ++++++++ 6 files changed, 104 insertions(+), 3 deletions(-) create mode 100755 templates/hooks--pre-merge.sample -- 1.7.12.406.gafd3f81 ^ permalink raw reply [flat|nested] 16+ messages in thread
* [PATCH 1/3] git-merge: Honor pre-merge hook 2012-09-05 13:39 [PATCH 0/3] pre-merge-hook Michael J Gruber @ 2012-09-05 13:39 ` Michael J Gruber 2012-09-05 15:30 ` Michael Haggerty 2012-09-05 13:39 ` [PATCH 2/3] merge: --no-verify to bypass pre-merge hook Michael J Gruber ` (2 subsequent siblings) 3 siblings, 1 reply; 16+ messages in thread From: Michael J Gruber @ 2012-09-05 13:39 UTC (permalink / raw) To: git 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). Signed-off-by: Michael J Gruber <git@drmicha.warpmail.net> --- Documentation/githooks.txt | 7 +++++++ builtin/merge.c | 13 ++++++++++++- templates/hooks--pre-merge.sample | 13 +++++++++++++ 3 files changed, 32 insertions(+), 1 deletion(-) create mode 100755 templates/hooks--pre-merge.sample diff --git a/Documentation/githooks.txt b/Documentation/githooks.txt index b9003fe..3fae643 100644 --- a/Documentation/githooks.txt +++ b/Documentation/githooks.txt @@ -86,6 +86,13 @@ All the 'git commit' hooks are invoked with the environment variable `GIT_EDITOR=:` if the command will not bring up an editor to modify the commit message. +pre-merge +~~~~~~~~~ + +This hook is invoked by 'git merge' when doing an automatic merge +commit; it is equivalent to 'pre-commit' for a non-automatic commit +for a merge. + prepare-commit-msg ~~~~~~~~~~~~~~~~~~ diff --git a/builtin/merge.c b/builtin/merge.c index 0ec8f0d..7c09e0b 100644 --- a/builtin/merge.c +++ b/builtin/merge.c @@ -898,12 +898,23 @@ static void prepare_to_commit(struct commit_list *remoteheads) { struct strbuf msg = STRBUF_INIT; const char *comment = _(merge_editor_comment); + const char *index_file = get_index_file(); + + if (run_hook(index_file, "pre-merge", NULL)) + abort_commit(remoteheads, NULL); + /* + * Re-read the index as pre-merge hook could have updated it, + * and write it out as a tree. We must do this before we invoke + * the editor and after we invoke run_status above. + */ + discard_cache(); + read_cache_from(index_file); strbuf_addbuf(&msg, &merge_msg); strbuf_addch(&msg, '\n'); if (0 < option_edit) strbuf_add_lines(&msg, "# ", comment, strlen(comment)); write_merge_msg(&msg); - run_hook(get_index_file(), "prepare-commit-msg", + run_hook(index_file, "prepare-commit-msg", git_path("MERGE_MSG"), "merge", NULL, NULL); if (0 < option_edit) { if (launch_editor(git_path("MERGE_MSG"), NULL, NULL)) diff --git a/templates/hooks--pre-merge.sample b/templates/hooks--pre-merge.sample new file mode 100755 index 0000000..a6313e6 --- /dev/null +++ b/templates/hooks--pre-merge.sample @@ -0,0 +1,13 @@ +#!/bin/sh +# +# An example hook script to verify what is about to be committed. +# Called by "git merge" with no arguments. The hook should +# exit with non-zero status after issuing an appropriate message if +# it wants to stop the commit. +# +# To enable this hook, rename this file to "pre-merge". + +. git-sh-setup +test -x "$GIT_DIR/hooks/pre-commit" && + exec "$GIT_DIR/hooks/pre-commit" +: -- 1.7.12.406.gafd3f81 ^ permalink raw reply related [flat|nested] 16+ messages in thread
* Re: [PATCH 1/3] git-merge: Honor pre-merge hook 2012-09-05 13:39 ` [PATCH 1/3] git-merge: Honor pre-merge hook Michael J Gruber @ 2012-09-05 15:30 ` Michael Haggerty 2012-09-06 8:16 ` Michael J Gruber 0 siblings, 1 reply; 16+ messages in thread From: Michael Haggerty @ 2012-09-05 15:30 UTC (permalink / raw) To: Michael J Gruber; +Cc: git 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. 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? Michael -- Michael Haggerty mhagger@alum.mit.edu http://softwareswirl.blogspot.com/ ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH 1/3] git-merge: Honor pre-merge hook 2012-09-05 15:30 ` Michael Haggerty @ 2012-09-06 8:16 ` Michael J Gruber 2012-09-06 10:48 ` Michael Haggerty 0 siblings, 1 reply; 16+ messages in thread From: Michael J Gruber @ 2012-09-06 8:16 UTC (permalink / raw) To: Michael Haggerty; +Cc: git, Junio C Hamano 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 ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH 1/3] git-merge: Honor pre-merge hook 2012-09-06 8:16 ` Michael J Gruber @ 2012-09-06 10:48 ` Michael Haggerty 2012-09-06 14:25 ` [PATCHv2 0/4] pre-commit hook for merges Michael J Gruber 0 siblings, 1 reply; 16+ messages in thread From: Michael Haggerty @ 2012-09-06 10:48 UTC (permalink / raw) To: Michael J Gruber; +Cc: git, Junio C Hamano On 09/06/2012 10:16 AM, Michael J Gruber wrote: > 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. Thanks for the explanation. I hope you will explain this briefly in the patch to the docs. >> 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. I can understand your reasoning and won't object. But before I shut up, I will point out a third alternative that is arguably closer to your "ideal": - For non-merge commits, call pre-commit - For automatic merge commits, call pre-merge - For non-automatic merge commits: if pre-merge exists, call pre-merge (only) else if pre-commit exists, call pre-commit (for backwards comptibility) Michael -- Michael Haggerty mhagger@alum.mit.edu http://softwareswirl.blogspot.com/ ^ permalink raw reply [flat|nested] 16+ messages in thread
* [PATCHv2 0/4] pre-commit hook for merges 2012-09-06 10:48 ` Michael Haggerty @ 2012-09-06 14:25 ` Michael J Gruber 2012-09-06 14:25 ` [PATCHv2 1/4] merge: document prepare-commit-msg hook usage Michael J Gruber ` (3 more replies) 0 siblings, 4 replies; 16+ messages in thread From: Michael J Gruber @ 2012-09-06 14:25 UTC (permalink / raw) To: git; +Cc: Junio C Hamano, Michael Haggerty In this second iteration I implement Junio's suggestion: 'git merge' invokes the pre-commit hook when merge.usePreCommitHook is set to true. 1/4 documents that 'git merge' invokes the prepare-commit-msg hook unconditionally already. 2-4 are v2 of the previous 1-3. This leaves aside the issue of commit-msg and post-commit hooks. If we can live with a bit of incompatibility I would rename the config to merge.useCommitHooks and call all of them based on that. This would change the way prepare-commit-msg is treated now. We could also introduce 4 config variables, 3 defaulting to false, 1 to true, of course... [I had messed up my alias file when adding mhagger, and it seems that tripped up vger; resending, sorry.] Michael J Gruber (4): merge: document prepare-commit-msg hook usage git-merge: Honor pre-commit hook based on config merge: --no-verify to bypass pre-commit hook t7503: add tests for pre-commit hook (merge) Documentation/git-merge.txt | 7 ++++- Documentation/githooks.txt | 6 ++++ Documentation/merge-config.txt | 5 ++++ Documentation/merge-options.txt | 4 +++ builtin/merge.c | 19 ++++++++++++- t/t7503-pre-commit-hook.sh | 62 +++++++++++++++++++++++++++++++++++++++++ 6 files changed, 101 insertions(+), 2 deletions(-) -- 1.7.12.406.gafd3f81 ^ permalink raw reply [flat|nested] 16+ messages in thread
* [PATCHv2 1/4] merge: document prepare-commit-msg hook usage 2012-09-06 14:25 ` [PATCHv2 0/4] pre-commit hook for merges Michael J Gruber @ 2012-09-06 14:25 ` Michael J Gruber 2012-09-06 14:25 ` [PATCHv2 2/4] git-merge: Honor pre-commit hook based on config Michael J Gruber ` (2 subsequent siblings) 3 siblings, 0 replies; 16+ messages in thread From: Michael J Gruber @ 2012-09-06 14:25 UTC (permalink / raw) To: git; +Cc: Junio C Hamano, Michael Haggerty Signed-off-by: Michael J Gruber <git@drmicha.warpmail.net> --- Documentation/git-merge.txt | 5 +++++ Documentation/githooks.txt | 2 ++ 2 files changed, 7 insertions(+) diff --git a/Documentation/git-merge.txt b/Documentation/git-merge.txt index 20f9228..b3ba8a8 100644 --- a/Documentation/git-merge.txt +++ b/Documentation/git-merge.txt @@ -308,6 +308,11 @@ branch.<name>.mergeoptions:: supported options are the same as those of 'git merge', but option values containing whitespace characters are currently not supported. +HOOKS +----- +This command can run the `prepare-commit-msg` hook. +See linkgit:githooks[5] for more information. + SEE ALSO -------- linkgit:git-fmt-merge-msg[1], linkgit:git-pull[1], diff --git a/Documentation/githooks.txt b/Documentation/githooks.txt index b9003fe..b32134c 100644 --- a/Documentation/githooks.txt +++ b/Documentation/githooks.txt @@ -108,6 +108,8 @@ it is not suppressed by the `--no-verify` option. A non-zero exit means a failure of the hook and aborts the commit. It should not be used as replacement for pre-commit hook. +This hook is also called by 'git merge' when it creates a new commit. + The sample `prepare-commit-msg` hook that comes with git comments out the `Conflicts:` part of a merge's commit message. -- 1.7.12.406.gafd3f81 ^ permalink raw reply related [flat|nested] 16+ messages in thread
* [PATCHv2 2/4] git-merge: Honor pre-commit hook based on config 2012-09-06 14:25 ` [PATCHv2 0/4] pre-commit hook for merges Michael J Gruber 2012-09-06 14:25 ` [PATCHv2 1/4] merge: document prepare-commit-msg hook usage Michael J Gruber @ 2012-09-06 14:25 ` Michael J Gruber 2012-09-06 14:25 ` [PATCHv2 3/4] merge: --no-verify to bypass pre-commit hook Michael J Gruber 2012-09-06 14:25 ` [PATCHv2 4/4] t7503: add tests for pre-commit hook (merge) Michael J Gruber 3 siblings, 0 replies; 16+ messages in thread From: Michael J Gruber @ 2012-09-06 14:25 UTC (permalink / raw) To: git; +Cc: Junio C Hamano, Michael Haggerty 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 merge.usePreCommitHook which controls whether an automatic merge commit invokes pre-commit. Signed-off-by: Michael J Gruber <git@drmicha.warpmail.net> --- Documentation/git-merge.txt | 2 +- Documentation/githooks.txt | 3 +++ Documentation/merge-config.txt | 5 +++++ builtin/merge.c | 17 ++++++++++++++++- 4 files changed, 25 insertions(+), 2 deletions(-) diff --git a/Documentation/git-merge.txt b/Documentation/git-merge.txt index b3ba8a8..3501ae2 100644 --- a/Documentation/git-merge.txt +++ b/Documentation/git-merge.txt @@ -310,7 +310,7 @@ branch.<name>.mergeoptions:: HOOKS ----- -This command can run the `prepare-commit-msg` hook. +This command can run the `prepare-commit-msg` and `pre-commit` hooks. See linkgit:githooks[5] for more information. SEE ALSO diff --git a/Documentation/githooks.txt b/Documentation/githooks.txt index b32134c..d62e02d 100644 --- a/Documentation/githooks.txt +++ b/Documentation/githooks.txt @@ -86,6 +86,9 @@ All the 'git commit' hooks are invoked with the environment variable `GIT_EDITOR=:` if the command will not bring up an editor to modify the commit message. +If the configuration option `merge.usePreCommitHook` is set to `true` +then 'git merge' invokes this hook whenever it creates a new commit. + prepare-commit-msg ~~~~~~~~~~~~~~~~~~ diff --git a/Documentation/merge-config.txt b/Documentation/merge-config.txt index 861bd6f..727caad 100644 --- a/Documentation/merge-config.txt +++ b/Documentation/merge-config.txt @@ -59,6 +59,11 @@ merge.tool:: and "xxdiff". Any other value is treated is custom merge tool and there must be a corresponding mergetool.<tool>.cmd option. +merge.usePreCommitHook:: + Controls whether the pre-commit hook is invoked when 'git merge' + creates a new commit (true merge without conflicts). False by + default. + merge.verbosity:: Controls the amount of output shown by the recursive merge strategy. Level 0 outputs nothing except a final error diff --git a/builtin/merge.c b/builtin/merge.c index 0ec8f0d..a2590a9 100644 --- a/builtin/merge.c +++ b/builtin/merge.c @@ -64,6 +64,7 @@ static int allow_rerere_auto; static int abort_current_merge; static int show_progress = -1; static int default_to_upstream; +static int use_pre_commit_hook; static const char *sign_commit; static struct strategy all_strategy[] = { @@ -575,6 +576,9 @@ static int git_merge_config(const char *k, const char *v, void *cb) } else if (!strcmp(k, "merge.defaulttoupstream")) { default_to_upstream = git_config_bool(k, v); return 0; + } else if (!strcmp(k, "merge.useprecommithook")) { + use_pre_commit_hook = git_config_bool(k, v); + return 0; } status = fmt_merge_msg_config(k, v, cb); @@ -898,12 +902,23 @@ static void prepare_to_commit(struct commit_list *remoteheads) { struct strbuf msg = STRBUF_INIT; const char *comment = _(merge_editor_comment); + const char *index_file = get_index_file(); + + if (use_pre_commit_hook && run_hook(index_file, "pre-commit", NULL)) + abort_commit(remoteheads, NULL); + /* + * Re-read the index as pre-commit hook could have updated it, + * and write it out as a tree. We must do this before we invoke + * the editor and after we invoke run_status above. + */ + discard_cache(); + read_cache_from(index_file); strbuf_addbuf(&msg, &merge_msg); strbuf_addch(&msg, '\n'); if (0 < option_edit) strbuf_add_lines(&msg, "# ", comment, strlen(comment)); write_merge_msg(&msg); - run_hook(get_index_file(), "prepare-commit-msg", + run_hook(index_file, "prepare-commit-msg", git_path("MERGE_MSG"), "merge", NULL, NULL); if (0 < option_edit) { if (launch_editor(git_path("MERGE_MSG"), NULL, NULL)) -- 1.7.12.406.gafd3f81 ^ permalink raw reply related [flat|nested] 16+ messages in thread
* [PATCHv2 3/4] merge: --no-verify to bypass pre-commit hook 2012-09-06 14:25 ` [PATCHv2 0/4] pre-commit hook for merges Michael J Gruber 2012-09-06 14:25 ` [PATCHv2 1/4] merge: document prepare-commit-msg hook usage Michael J Gruber 2012-09-06 14:25 ` [PATCHv2 2/4] git-merge: Honor pre-commit hook based on config Michael J Gruber @ 2012-09-06 14:25 ` Michael J Gruber 2012-09-06 14:25 ` [PATCHv2 4/4] t7503: add tests for pre-commit hook (merge) Michael J Gruber 3 siblings, 0 replies; 16+ messages in thread From: Michael J Gruber @ 2012-09-06 14:25 UTC (permalink / raw) To: git; +Cc: Junio C Hamano, Michael Haggerty Analogous to commit, introduce a '--no-verify' option which bypasses the pre-commit hook. The shorthand '-n' is taken by the (non-existing) '--no-stat' already. Signed-off-by: Michael J Gruber <git@drmicha.warpmail.net> --- Documentation/git-merge.txt | 2 +- Documentation/githooks.txt | 1 + Documentation/merge-options.txt | 4 ++++ builtin/merge.c | 4 +++- 4 files changed, 9 insertions(+), 2 deletions(-) diff --git a/Documentation/git-merge.txt b/Documentation/git-merge.txt index 3501ae2..363fbea 100644 --- a/Documentation/git-merge.txt +++ b/Documentation/git-merge.txt @@ -10,7 +10,7 @@ SYNOPSIS -------- [verse] 'git merge' [-n] [--stat] [--no-commit] [--squash] [--[no-]edit] - [-s <strategy>] [-X <strategy-option>] + [--no-verify] [-s <strategy>] [-X <strategy-option>] [--[no-]rerere-autoupdate] [-m <msg>] [<commit>...] 'git merge' <msg> HEAD <commit>... 'git merge' --abort diff --git a/Documentation/githooks.txt b/Documentation/githooks.txt index d62e02d..c734e2c 100644 --- a/Documentation/githooks.txt +++ b/Documentation/githooks.txt @@ -88,6 +88,7 @@ to modify the commit message. If the configuration option `merge.usePreCommitHook` is set to `true` then 'git merge' invokes this hook whenever it creates a new commit. +It can be bypassed with the `\--no-verify` option. prepare-commit-msg ~~~~~~~~~~~~~~~~~~ diff --git a/Documentation/merge-options.txt b/Documentation/merge-options.txt index 0bcbe0a..5695fc6 100644 --- a/Documentation/merge-options.txt +++ b/Documentation/merge-options.txt @@ -70,6 +70,10 @@ merge. With --no-squash perform the merge and commit the result. This option can be used to override --squash. +--no-verify:: + This option bypasses the pre-commit hook. + See also linkgit:githooks[5]. + -s <strategy>:: --strategy=<strategy>:: Use the given merge strategy; can be supplied more than diff --git a/builtin/merge.c b/builtin/merge.c index a2590a9..58a848f 100644 --- a/builtin/merge.c +++ b/builtin/merge.c @@ -60,6 +60,7 @@ static const char *branch; static char *branch_mergeoptions; static int option_renormalize; static int verbosity; +static int no_verify; static int allow_rerere_auto; static int abort_current_merge; static int show_progress = -1; @@ -199,6 +200,7 @@ static struct option builtin_merge_options[] = { N_("allow fast-forward (default)")), OPT_BOOLEAN(0, "ff-only", &fast_forward_only, N_("abort if fast-forward is not possible")), + OPT_BOOLEAN(0, "no-verify", &no_verify, "bypass pre-merge hook"), OPT_RERERE_AUTOUPDATE(&allow_rerere_auto), OPT_CALLBACK('s', "strategy", &use_strategies, N_("strategy"), N_("merge strategy to use"), option_parse_strategy), @@ -904,7 +906,7 @@ static void prepare_to_commit(struct commit_list *remoteheads) const char *comment = _(merge_editor_comment); const char *index_file = get_index_file(); - if (use_pre_commit_hook && run_hook(index_file, "pre-commit", NULL)) + if (use_pre_commit_hook && !no_verify && run_hook(index_file, "pre-commit", NULL)) abort_commit(remoteheads, NULL); /* * Re-read the index as pre-commit hook could have updated it, -- 1.7.12.406.gafd3f81 ^ permalink raw reply related [flat|nested] 16+ messages in thread
* [PATCHv2 4/4] t7503: add tests for pre-commit hook (merge) 2012-09-06 14:25 ` [PATCHv2 0/4] pre-commit hook for merges Michael J Gruber ` (2 preceding siblings ...) 2012-09-06 14:25 ` [PATCHv2 3/4] merge: --no-verify to bypass pre-commit hook Michael J Gruber @ 2012-09-06 14:25 ` Michael J Gruber 3 siblings, 0 replies; 16+ messages in thread From: Michael J Gruber @ 2012-09-06 14:25 UTC (permalink / raw) To: git; +Cc: Junio C Hamano, Michael Haggerty Add tests which make sure that the pre-commit hook is called by 'git merge' when merge.usePreCommitHook is set, allows/disallows merge commits depending on its return value and is suppressed by "--no-verify". Signed-off-by: Michael J Gruber <git@drmicha.warpmail.net> --- t/t7503-pre-commit-hook.sh | 62 ++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 62 insertions(+) diff --git a/t/t7503-pre-commit-hook.sh b/t/t7503-pre-commit-hook.sh index 984889b..e4d324d 100755 --- a/t/t7503-pre-commit-hook.sh +++ b/t/t7503-pre-commit-hook.sh @@ -4,6 +4,19 @@ test_description='pre-commit hook' . ./test-lib.sh +test_expect_success 'root commit' ' + + echo "root" > file && + git add file && + git commit -m "zeroth" && + git checkout -b side && + echo "foo" > foo && + git add foo && + git commit -m "make it non-ff" && + git checkout master + +' + test_expect_success 'with no hook' ' echo "foo" > file && @@ -12,6 +25,14 @@ test_expect_success 'with no hook' ' ' +test_expect_success 'with no hook (merge)' ' + + git checkout side && + git merge -m "merge master" master && + git checkout master + +' + test_expect_success '--no-verify with no hook' ' echo "bar" > file && @@ -20,6 +41,14 @@ test_expect_success '--no-verify with no hook' ' ' +test_expect_success '--no-verify with no hook (merge)' ' + + git checkout side && + git merge --no-verify -m "merge master" master && + git checkout master + +' + # now install hook that always succeeds HOOKDIR="$(git rev-parse --git-dir)/hooks" HOOK="$HOOKDIR/pre-commit" @@ -29,6 +58,7 @@ cat > "$HOOK" <<EOF exit 0 EOF chmod +x "$HOOK" +git config merge.usePreCommitHook true test_expect_success 'with succeeding hook' ' @@ -38,6 +68,14 @@ test_expect_success 'with succeeding hook' ' ' +test_expect_success 'with succeeding hook (merge)' ' + + git checkout side && + git merge -m "merge master" master && + git checkout master + +' + test_expect_success '--no-verify with succeeding hook' ' echo "even more" >> file && @@ -46,6 +84,14 @@ test_expect_success '--no-verify with succeeding hook' ' ' +test_expect_success '--no-verify with succeeding hook (merge)' ' + + git checkout side && + git merge --no-verify -m "merge master" master && + git checkout master + +' + # now a hook that fails cat > "$HOOK" <<EOF #!/bin/sh @@ -68,6 +114,22 @@ test_expect_success '--no-verify with failing hook' ' ' +test_expect_success 'with failing hook (merge)' ' + + git checkout side && + test_must_fail git merge -m "merge master" master && + git checkout master + +' + +test_expect_success '--no-verify with failing hook (merge)' ' + + git checkout side && + git merge --no-verify -m "merge master" master && + git checkout master + +' + chmod -x "$HOOK" test_expect_success POSIXPERM 'with non-executable hook' ' -- 1.7.12.406.gafd3f81 ^ permalink raw reply related [flat|nested] 16+ messages in thread
* [PATCH 2/3] merge: --no-verify to bypass pre-merge hook 2012-09-05 13:39 [PATCH 0/3] pre-merge-hook Michael J Gruber 2012-09-05 13:39 ` [PATCH 1/3] git-merge: Honor pre-merge hook Michael J Gruber @ 2012-09-05 13:39 ` Michael J Gruber 2012-09-05 13:39 ` [PATCH 3/3] t7503: add tests for pre-merge-hook Michael J Gruber 2012-09-06 5:07 ` [PATCH 0/3] pre-merge-hook Junio C Hamano 3 siblings, 0 replies; 16+ messages in thread From: Michael J Gruber @ 2012-09-05 13:39 UTC (permalink / raw) To: git Analogous to commit, introduce a '--no-verify' option which bypasses the pre-merge hook. The shorthand '-n' is taken by the (non-existing) '--no-stat' already. Signed-off-by: Michael J Gruber <git@drmicha.warpmail.net> --- Documentation/git-merge.txt | 2 +- Documentation/githooks.txt | 2 +- Documentation/merge-options.txt | 4 ++++ builtin/merge.c | 4 +++- 4 files changed, 9 insertions(+), 3 deletions(-) diff --git a/Documentation/git-merge.txt b/Documentation/git-merge.txt index 20f9228..8816865 100644 --- a/Documentation/git-merge.txt +++ b/Documentation/git-merge.txt @@ -10,7 +10,7 @@ SYNOPSIS -------- [verse] 'git merge' [-n] [--stat] [--no-commit] [--squash] [--[no-]edit] - [-s <strategy>] [-X <strategy-option>] + [--no-verify] [-s <strategy>] [-X <strategy-option>] [--[no-]rerere-autoupdate] [-m <msg>] [<commit>...] 'git merge' <msg> HEAD <commit>... 'git merge' --abort diff --git a/Documentation/githooks.txt b/Documentation/githooks.txt index 3fae643..e20bfe0 100644 --- a/Documentation/githooks.txt +++ b/Documentation/githooks.txt @@ -91,7 +91,7 @@ pre-merge This hook is invoked by 'git merge' when doing an automatic merge commit; it is equivalent to 'pre-commit' for a non-automatic commit -for a merge. +for a merge, and can be bypassed with the `\--no-verify` option. prepare-commit-msg ~~~~~~~~~~~~~~~~~~ diff --git a/Documentation/merge-options.txt b/Documentation/merge-options.txt index 0bcbe0a..93e2e61 100644 --- a/Documentation/merge-options.txt +++ b/Documentation/merge-options.txt @@ -70,6 +70,10 @@ merge. With --no-squash perform the merge and commit the result. This option can be used to override --squash. +--no-verify:: + This option bypasses the pre-merge and commit-msg hooks. + See also linkgit:githooks[5]. + -s <strategy>:: --strategy=<strategy>:: Use the given merge strategy; can be supplied more than diff --git a/builtin/merge.c b/builtin/merge.c index 7c09e0b..4d36a00 100644 --- a/builtin/merge.c +++ b/builtin/merge.c @@ -60,6 +60,7 @@ static const char *branch; static char *branch_mergeoptions; static int option_renormalize; static int verbosity; +static int no_verify; static int allow_rerere_auto; static int abort_current_merge; static int show_progress = -1; @@ -198,6 +199,7 @@ static struct option builtin_merge_options[] = { N_("allow fast-forward (default)")), OPT_BOOLEAN(0, "ff-only", &fast_forward_only, N_("abort if fast-forward is not possible")), + OPT_BOOLEAN(0, "no-verify", &no_verify, "bypass pre-merge hook"), OPT_RERERE_AUTOUPDATE(&allow_rerere_auto), OPT_CALLBACK('s', "strategy", &use_strategies, N_("strategy"), N_("merge strategy to use"), option_parse_strategy), @@ -900,7 +902,7 @@ static void prepare_to_commit(struct commit_list *remoteheads) const char *comment = _(merge_editor_comment); const char *index_file = get_index_file(); - if (run_hook(index_file, "pre-merge", NULL)) + if (!no_verify && run_hook(index_file, "pre-merge", NULL)) abort_commit(remoteheads, NULL); /* * Re-read the index as pre-merge hook could have updated it, -- 1.7.12.406.gafd3f81 ^ permalink raw reply related [flat|nested] 16+ messages in thread
* [PATCH 3/3] t7503: add tests for pre-merge-hook 2012-09-05 13:39 [PATCH 0/3] pre-merge-hook Michael J Gruber 2012-09-05 13:39 ` [PATCH 1/3] git-merge: Honor pre-merge hook Michael J Gruber 2012-09-05 13:39 ` [PATCH 2/3] merge: --no-verify to bypass pre-merge hook Michael J Gruber @ 2012-09-05 13:39 ` Michael J Gruber 2012-09-06 5:07 ` [PATCH 0/3] pre-merge-hook Junio C Hamano 3 siblings, 0 replies; 16+ messages in thread From: Michael J Gruber @ 2012-09-05 13:39 UTC (permalink / raw) To: git Add tests which make sure that the pre-merge-hook is called when present, allows/disallows merge commits depending on its return value and is suppressed by "--no-verify". Signed-off-by: Michael J Gruber <git@drmicha.warpmail.net> --- t/t7503-pre-commit-hook.sh | 66 +++++++++++++++++++++++++++++++++++++++++++++- 1 file changed, 65 insertions(+), 1 deletion(-) diff --git a/t/t7503-pre-commit-hook.sh b/t/t7503-pre-commit-hook.sh index 984889b..36ae87f 100755 --- a/t/t7503-pre-commit-hook.sh +++ b/t/t7503-pre-commit-hook.sh @@ -1,9 +1,22 @@ #!/bin/sh -test_description='pre-commit hook' +test_description='pre-commit and pre-merge hooks' . ./test-lib.sh +test_expect_success 'root commit' ' + + echo "root" > file && + git add file && + git commit -m "zeroth" && + git checkout -b side && + echo "foo" > foo && + git add foo && + git commit -m "make it non-ff" && + git checkout master + +' + test_expect_success 'with no hook' ' echo "foo" > file && @@ -12,6 +25,14 @@ test_expect_success 'with no hook' ' ' +test_expect_success 'with no hook (merge)' ' + + git checkout side && + git merge -m "merge master" master && + git checkout master + +' + test_expect_success '--no-verify with no hook' ' echo "bar" > file && @@ -20,15 +41,25 @@ test_expect_success '--no-verify with no hook' ' ' +test_expect_success '--no-verify with no hook (merge)' ' + + git checkout side && + git merge --no-verify -m "merge master" master && + git checkout master + +' + # now install hook that always succeeds HOOKDIR="$(git rev-parse --git-dir)/hooks" HOOK="$HOOKDIR/pre-commit" +MERGEHOOK="$HOOKDIR/pre-merge" mkdir -p "$HOOKDIR" cat > "$HOOK" <<EOF #!/bin/sh exit 0 EOF chmod +x "$HOOK" +cp -p "$HOOK" "$MERGEHOOK" test_expect_success 'with succeeding hook' ' @@ -38,6 +69,14 @@ test_expect_success 'with succeeding hook' ' ' +test_expect_success 'with succeeding hook (merge)' ' + + git checkout side && + git merge -m "merge master" master && + git checkout master + +' + test_expect_success '--no-verify with succeeding hook' ' echo "even more" >> file && @@ -46,11 +85,20 @@ test_expect_success '--no-verify with succeeding hook' ' ' +test_expect_success '--no-verify with succeeding hook (merge)' ' + + git checkout side && + git merge --no-verify -m "merge master" master && + git checkout master + +' + # now a hook that fails cat > "$HOOK" <<EOF #!/bin/sh exit 1 EOF +cp -p "$HOOK" "$MERGEHOOK" test_expect_success 'with failing hook' ' @@ -68,6 +116,22 @@ test_expect_success '--no-verify with failing hook' ' ' +test_expect_success 'with failing hook (merge)' ' + + git checkout side && + test_must_fail git merge -m "merge master" master && + git checkout master + +' + +test_expect_success '--no-verify with failing hook (merge)' ' + + git checkout side && + git merge --no-verify -m "merge master" master && + git checkout master + +' + chmod -x "$HOOK" test_expect_success POSIXPERM 'with non-executable hook' ' -- 1.7.12.406.gafd3f81 ^ permalink raw reply related [flat|nested] 16+ messages in thread
* Re: [PATCH 0/3] pre-merge-hook 2012-09-05 13:39 [PATCH 0/3] pre-merge-hook Michael J Gruber ` (2 preceding siblings ...) 2012-09-05 13:39 ` [PATCH 3/3] t7503: add tests for pre-merge-hook Michael J Gruber @ 2012-09-06 5:07 ` Junio C Hamano 2012-09-06 8:16 ` Michael J Gruber 3 siblings, 1 reply; 16+ messages in thread From: Junio C Hamano @ 2012-09-06 5:07 UTC (permalink / raw) To: Michael J Gruber; +Cc: git Michael J Gruber <git@drmicha.warpmail.net> writes: > The pre-commit hook is often used to ensure certain properties of each > comitted tree like formatting or coding standards, validity (lint/make) > or code quality (make test). But merges introduce new commits unless > they are fast forwards, and therefore they can break these properties > because the pre-commit hook is not run by "git merge". > > Introduce a pre-merge hook which works for (non ff, automatic) merges > like pre-commit does for commits. Typically this will just call the > pre-commit hook (like in the sample hook), but it does not need to. When your merge asks for a help from you to resolve conflict, you conclude with "git commit", and at that point, pre-commit hook will have a chance to reject it, presumably. That means for any project that wants to audit a merge via hook, their pre-commit hook MUST be prepared to look at and judge a merge. Given that, is a separate hook that "can just call the pre-commit but does not need to" really needed and useful? I admit that I haven't thought things through, but adding a boolean "merge.usePreCommitHook" smells like a more appropriate approach to me. I dunno. ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH 0/3] pre-merge-hook 2012-09-06 5:07 ` [PATCH 0/3] pre-merge-hook Junio C Hamano @ 2012-09-06 8:16 ` Michael J Gruber 2012-09-06 18:34 ` Junio C Hamano 0 siblings, 1 reply; 16+ messages in thread From: Michael J Gruber @ 2012-09-06 8:16 UTC (permalink / raw) To: Junio C Hamano; +Cc: git Junio C Hamano venit, vidit, dixit 06.09.2012 07:07: > Michael J Gruber <git@drmicha.warpmail.net> writes: > >> The pre-commit hook is often used to ensure certain properties of each >> comitted tree like formatting or coding standards, validity (lint/make) >> or code quality (make test). But merges introduce new commits unless >> they are fast forwards, and therefore they can break these properties >> because the pre-commit hook is not run by "git merge". >> >> Introduce a pre-merge hook which works for (non ff, automatic) merges >> like pre-commit does for commits. Typically this will just call the >> pre-commit hook (like in the sample hook), but it does not need to. > > When your merge asks for a help from you to resolve conflict, you > conclude with "git commit", and at that point, pre-commit hook will > have a chance to reject it, presumably. That means for any project > that wants to audit a merge via hook, their pre-commit hook MUST be > prepared to look at and judge a merge. Given that, is a separate > hook that "can just call the pre-commit but does not need to" really > needed and useful? > > I admit that I haven't thought things through, but adding a boolean > "merge.usePreCommitHook" smells like a more appropriate approach to > me. > > I dunno. That would be an option ;) Seriously, that would make the case where both hooks are the same even simpler, obviously. On the other hand it would make the other case more difficult. But, really, since non-automatic merges call pre-commit now, and that is probably to stay, catching those with a different (part of the pre-commit) hook will stay difficult anyways. So the question is really more whether we prefer to bloat the config space or the name space by this. Either works for me, and if we don't change the current behaviour (pre-commit-hook resp. no hook for non-automatic merges resp. automatic merges) the config option is probably less confusing. Michael ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH 0/3] pre-merge-hook 2012-09-06 8:16 ` Michael J Gruber @ 2012-09-06 18:34 ` Junio C Hamano 2012-09-07 10:00 ` Michael J Gruber 0 siblings, 1 reply; 16+ messages in thread From: Junio C Hamano @ 2012-09-06 18:34 UTC (permalink / raw) To: Michael J Gruber; +Cc: git Michael J Gruber <git@drmicha.warpmail.net> writes: > Junio C Hamano venit, vidit, dixit 06.09.2012 07:07: >> Michael J Gruber <git@drmicha.warpmail.net> writes: >> >>> The pre-commit hook is often used to ensure certain properties of each >>> comitted tree like formatting or coding standards, validity (lint/make) >>> or code quality (make test). But merges introduce new commits unless >>> they are fast forwards, and therefore they can break these properties >>> because the pre-commit hook is not run by "git merge". >>> >>> Introduce a pre-merge hook which works for (non ff, automatic) merges >>> like pre-commit does for commits. Typically this will just call the >>> pre-commit hook (like in the sample hook), but it does not need to. >> >> When your merge asks for a help from you to resolve conflict, you >> conclude with "git commit", and at that point, pre-commit hook will >> have a chance to reject it, presumably. That means for any project >> that wants to audit a merge via hook, their pre-commit hook MUST be >> prepared to look at and judge a merge. Given that, is a separate >> hook that "can just call the pre-commit but does not need to" really >> needed and useful? >> >> I admit that I haven't thought things through, but adding a boolean >> "merge.usePreCommitHook" smells like a more appropriate approach to >> me. >> >> I dunno. > > That would be an option ;) I said "I dunno" as others may have opinions on the best direction to go. > Either works for me, and if we don't change the current behaviour > (pre-commit-hook resp. no hook for non-automatic merges resp. automatic > merges) the config option is probably less confusing. If we were to go in the "pre-commit has to be prepared to vet a merge already, so let it handle the automerge case" direction, I have another suggestion. Because you need to support "merge --no-verify" to override the hook anyway, I think it makes sense to introduce "merge --verify" to force it trigger the hook (and it needs to percolate up to "pull"). People who want it always on may want a boolean merge.verify, but that should come in a separate step. The patch that implements it must make sure all our internal uses of "merge" is updated to pass "--no-verify", unless there is a very good reason. Another direction to go would be to deprecate the "commit is the way to conclude a merge that stopped in the middle due to a conflict asking for your help" and introduce "merge --continue" [*1*]. That would open a way to a separate "pre/post-merge" hook much cleanly. At that point it would be clear that "pre-commit" won't be involved in "merge" (i.e. the user never will use "git commit" when merging). I am fairly negative on the current mess imposed on "git commit" that has to know who called it and behave differently (look for "whence" in builtin/commit.c), and would rather not to see it made worse by piling "call pre-merge if exists and in a merge, or call pre-commit" kind of hack on top of it. [Footnote] *1* This has been brought up a few times during discussion on sequencer and mergy operations, and I think it makes sense in the longer term. "am" and "rebase" were first to use "--continue", instead of having the user to use "commit" to conclude, and later "cherry-pick" and "revert" have been updated to follow suit, so "merge" may be the only oddball remaining now. ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH 0/3] pre-merge-hook 2012-09-06 18:34 ` Junio C Hamano @ 2012-09-07 10:00 ` Michael J Gruber 0 siblings, 0 replies; 16+ messages in thread From: Michael J Gruber @ 2012-09-07 10:00 UTC (permalink / raw) To: Junio C Hamano; +Cc: git Junio C Hamano venit, vidit, dixit 06.09.2012 20:34: > Michael J Gruber <git@drmicha.warpmail.net> writes: > >> Junio C Hamano venit, vidit, dixit 06.09.2012 07:07: >>> Michael J Gruber <git@drmicha.warpmail.net> writes: >>> >>>> The pre-commit hook is often used to ensure certain properties >>>> of each comitted tree like formatting or coding standards, >>>> validity (lint/make) or code quality (make test). But merges >>>> introduce new commits unless they are fast forwards, and >>>> therefore they can break these properties because the >>>> pre-commit hook is not run by "git merge". >>>> >>>> Introduce a pre-merge hook which works for (non ff, automatic) >>>> merges like pre-commit does for commits. Typically this will >>>> just call the pre-commit hook (like in the sample hook), but it >>>> does not need to. >>> >>> When your merge asks for a help from you to resolve conflict, >>> you conclude with "git commit", and at that point, pre-commit >>> hook will have a chance to reject it, presumably. That means for >>> any project that wants to audit a merge via hook, their >>> pre-commit hook MUST be prepared to look at and judge a merge. >>> Given that, is a separate hook that "can just call the pre-commit >>> but does not need to" really needed and useful? >>> >>> I admit that I haven't thought things through, but adding a >>> boolean "merge.usePreCommitHook" smells like a more appropriate >>> approach to me. >>> >>> I dunno. >> >> That would be an option ;) > > I said "I dunno" as others may have opinions on the best direction to > go. Sure. I meant to make a pun the config option approach being an option. >> Either works for me, and if we don't change the current behaviour >> (pre-commit-hook resp. no hook for non-automatic merges resp. >> automatic merges) the config option is probably less confusing. > > If we were to go in the "pre-commit has to be prepared to vet a merge > already, so let it handle the automerge case" direction, I have > another suggestion. > > Because you need to support "merge --no-verify" to override the hook > anyway, I think it makes sense to introduce "merge --verify" to force > it trigger the hook (and it needs to percolate up to "pull"). > > People who want it always on may want a boolean merge.verify, but > that should come in a separate step. The patch that implements it > must make sure all our internal uses of "merge" is updated to pass > "--no-verify", unless there is a very good reason. Hmm. Nothing calls cmd_merge() nor the relevant parts. "git pull" calls "git merge" and should probably obey that option. All others (am etc.) call git-merge-${strategy} directly and are not affected by the option. Am I overlooking something? > Another direction to go would be to deprecate the "commit is the way > to conclude a merge that stopped in the middle due to a conflict > asking for your help" and introduce "merge --continue" [*1*]. That > would open a way to a separate "pre/post-merge" hook much cleanly. At > that point it would be clear that "pre-commit" won't be involved in > "merge" (i.e. the user never will use "git commit" when merging). > > I am fairly negative on the current mess imposed on "git commit" that > has to know who called it and behave differently (look for "whence" > in builtin/commit.c), and would rather not to see it made worse by > piling "call pre-merge if exists and in a merge, or call pre-commit" > kind of hack on top of it. That is a mess, yes. Part of it is due to the fact that both builtin/{commit,merge}.c do a lot of "commit stuff" in parallel, often with copy & pasted code, and "commit" needs to be aware of other states (in-am, in-rebase) also. My feeling is that builtin/merge.c should rather get rid of the stuff that parallels things that builtin/commit.c does, so that all is in one place. (I think that redundancy is to blame for the inconsistent hook behaviour for merges even within builtin/merge.c). > [Footnote] > > *1* This has been brought up a few times during discussion on > sequencer and mergy operations, and I think it makes sense in the > longer term. "am" and "rebase" were first to use "--continue", > instead of having the user to use "commit" to conclude, and later > "cherry-pick" and "revert" have been updated to follow suit, so > "merge" may be the only oddball remaining now. "git merge --continue" to commit a resolved merge, but code-wise builtin/commit.c taking over because it's just one more case? I afraid that restructuring is beyond my available Git capacity and my self-imposed constraint to avoid anything with backwards compatibility or migration plan issues. (I took this up because I thought it's within, to share something I've been using for a while.) I fully agree that a big-picture-solution is preferable. Michael ^ permalink raw reply [flat|nested] 16+ messages in thread
end of thread, other threads:[~2012-09-07 10:00 UTC | newest] Thread overview: 16+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2012-09-05 13:39 [PATCH 0/3] pre-merge-hook Michael J Gruber 2012-09-05 13:39 ` [PATCH 1/3] git-merge: Honor pre-merge hook Michael J Gruber 2012-09-05 15:30 ` Michael Haggerty 2012-09-06 8:16 ` Michael J Gruber 2012-09-06 10:48 ` Michael Haggerty 2012-09-06 14:25 ` [PATCHv2 0/4] pre-commit hook for merges Michael J Gruber 2012-09-06 14:25 ` [PATCHv2 1/4] merge: document prepare-commit-msg hook usage Michael J Gruber 2012-09-06 14:25 ` [PATCHv2 2/4] git-merge: Honor pre-commit hook based on config Michael J Gruber 2012-09-06 14:25 ` [PATCHv2 3/4] merge: --no-verify to bypass pre-commit hook Michael J Gruber 2012-09-06 14:25 ` [PATCHv2 4/4] t7503: add tests for pre-commit hook (merge) Michael J Gruber 2012-09-05 13:39 ` [PATCH 2/3] merge: --no-verify to bypass pre-merge hook Michael J Gruber 2012-09-05 13:39 ` [PATCH 3/3] t7503: add tests for pre-merge-hook Michael J Gruber 2012-09-06 5:07 ` [PATCH 0/3] pre-merge-hook Junio C Hamano 2012-09-06 8:16 ` Michael J Gruber 2012-09-06 18:34 ` Junio C Hamano 2012-09-07 10:00 ` Michael J Gruber
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.