Teach 'git merge' and 'git pull' the option --ff-only
PATCH 1/1 Teach 'git merge' and 'git pull' the option --ff-only
For convenience in scripts and aliases, add the option --ff-only to only allow fast-forwards.
Acknowledgements: I did look at Yuval Kogman's earlier patch (107768 in gmane), mainly as shortcut to find my way in the code, but I did not copy anything directly.
---
When I started to use git earlier this year, I was suprised that there was a --no-ff option but no --ff-only option.
I saw in the mailing list archive at gmane that there has been two previous attempts to implement --ff-only. The first patch was made to the git-merge.sh script (before builtin-merge.c was created). As far as I understand it, the author of the patch said he would send some corrections of the patch, but he never did, and nothing more happened.
So here is my patch, the third attempt.
Documentation/merge-options.txt | 4 ++++ builtin-merge.c | 16 ++++++++++++++-- git-pull.sh | 7 +++++-- t/t7600-merge.sh | 29 +++++++++++++++++++++++++++++ 4 files changed, 52 insertions(+), 4 deletions(-)
Documentation/merge-options.txt +4 -0
@@ -60,6 +60,10 @@ a fast-forward, only update the branch pointer. This is the default behavior of git-merge. +--ff-only:: + Refuse to merge unless the merge can be resolved as a + fast-forward. + -s <strategy>:: --strategy=<strategy>:: Use the given merge strategy; can be supplied more than
builtin-merge.c +14 -2
@@ -43,6 +43,7 @@ static const char * const builtin_merge_usage[] = {static int show_diffstat =1 , option_log, squash;static int option_commit =1 , allow_fast_forward =1 ; +static int fast_forward_only;static int allow_trivial =1 , have_message;static struct strbuf merge_msg;static struct commit_list *remoteheads;
@@ -167,6 +168,8 @@ static struct option builtin_merge_options[] = { "perform a commit if the merge succeeds (default)"),OPT_BOOLEAN (0 ,"ff" , &allow_fast_forward,"allow fast forward (default)" ), +OPT_BOOLEAN (0 ,"ff-only" , &fast_forward_only, +"abort if fast forward is not possible" ),OPT_CALLBACK ('s' ,"strategy" , &use_strategies,"strategy" ,"merge strategy to use" , option_parse_strategy), OPT_CALLBACK('m', "message", &merge_msg, "message",
@@ -874,6 +877,9 @@ int cmd_merge(int argc, const char **argv, const char *prefix) option_commit =0 ; } +if (!allow_fast_forward && fast_forward_only) +die ("You cannot combine --no-ff with --ff-only." ); +if (!argc)usage_with_options (builtin_merge_usage, builtin_merge_options);
@@ -969,8 +975,11 @@ int cmd_merge(int argc, const char **argv, const char *prefix) }for (i =0 ; i < use_strategies_nr; i++) { -if (use_strategies[i]->attr &NO_FAST_FORWARD ) +if (use_strategies[i]->attr &NO_FAST_FORWARD ) { allow_fast_forward =0 ; +if (fast_forward_only) +die ("You cannot combine --ff-only with the merge strategy '%s'." , use_strategies[i]->name); + }if (use_strategies[i]->attr &NO_TRIVIAL ) allow_trivial =0 ; }
@@ -1040,7 +1049,7 @@ int cmd_merge(int argc, const char **argv, const char *prefix)* only one common. */ refresh_cache (REFRESH_QUIET ); -if (allow_trivial) { +if (allow_trivial && !fast_forward_only) {/* See if it is really trivial. */ git_committer_info (IDENT_ERROR_ON_NO_NAME );printf ("Trying really trivial in-index merge...\n" );
@@ -1079,6 +1088,9 @@ int cmd_merge(int argc, const char **argv, const char *prefix) } } +if (fast_forward_only) +die ("Not possible to fast forward, aborting." ); +/* We are going to make a new commit. */ git_committer_info (IDENT_ERROR_ON_NO_NAME );
git-pull.sh +5 -2
@@ -16,7 +16,8 @@ cd_to_toplevel test -z "$(git ls-files -u)" || die "You are in the middle of a conflicted merge." -strategy_args= diffstat= no_commit= squash= no_ff= log_arg= verbosity= +strategy_args= diffstat= no_commit= squash= no_ff= ff_only= +log_arg= verbosity= curr_branch=$(git symbolic-ref -q HEAD) curr_branch_short=$(echo "$curr_branch" | sed "s|refs/heads/||") rebase=$(git config --bool branch.$curr_branch_short.rebase)
@@ -45,6 +46,8 @@ do no_ff=--ff ;; --no-ff) no_ff=--no-ff ;; + --ff-only) + ff_only=--ff-only ;; -s=*|--s=*|--st=*|--str=*|--stra=*|--strat=*|--strate=*|\ --strateg=*|--strategy=*|\ -s|--s|--st|--str|--stra|--strat|--strate|--strateg|--strategy)
@@ -215,5 +218,5 @@ merge_name=$(git fmt-merge-msg $log_arg <"$GIT_DIR/FETCH_HEAD") || exit test true = "$rebase" && exec git-rebase $diffstat $strategy_args --onto $merge_head \ ${oldremoteref:-$merge_head} -exec git-merge $diffstat $no_commit $squash $no_ff $log_arg $strategy_args \ +exec git-merge $diffstat $no_commit $squash $no_ff $ff_only $log_arg $strategy_args \ "$merge_name" HEAD $merge_head $verbosity
t/t7600-merge.sh +29 -0
@@ -243,6 +243,16 @@ test_expect_success 'merge c0 with c1' ' test_debug 'gitk --all' +test_expect_success 'merge c0 with c1 with --ff-only' ' + git reset --hard c0 && + git merge --ff-only c1 && + git merge --ff-only HEAD c0 c1 && + verify_merge file result.1 && + verify_head "$c1" +' + +test_debug 'gitk --all' + test_expect_success 'merge c1 with c2' ' git reset --hard c1 && test_tick &&
@@ -263,6 +273,14 @@ test_expect_success 'merge c1 with c2 and c3' ' test_debug 'gitk --all' +test_expect_success 'failing merges with --ff-only' ' + git reset --hard c1 && + test_tick && + test_must_fail git merge --ff-only c2 && + test_must_fail git merge --ff-only c3 && + test_must_fail git merge --ff-only c2 c3 +' + test_expect_success 'merge c0 with c1 (no-commit)' ' git reset --hard c0 && git merge --no-commit c1 &&
@@ -432,6 +450,17 @@ test_expect_success 'combining --squash and --no-ff is refused' ' test_must_fail git merge --no-ff --squash c1 ' +test_expect_success 'combining --ff-only and --no-ff is refused' ' + test_must_fail git merge --ff-only --no-ff c1 && + test_must_fail git merge --no-ff --ff-only c1 +' + +test_expect_success 'combining --ff-only with certain merge strategies is refused' ' + git reset --hard c0 && + test_must_fail git merge --ff-only --strategy=ours c1 && + test_must_fail git merge --ff-only --strategy=subtree c1 +' + test_expect_success 'merge c0 with c1 (ff overrides no-ff)' ' git reset --hard c0 && git config branch.master.mergeoptions "--no-ff" &&
Signature · 1 line
1.6.5.1.69.g36942
Quoted 8 lines · Björn Gustavsson
For convenience in scripts and aliases, add the option --ff-only to only allow fast-forwards.
Acknowledgements: I did look at Yuval Kogman's earlier patch (107768 in gmane), mainly as shortcut to find my way in the code, but I did not copy anything directly.
Signed-off-by Björn Gustavsson <[email protected]>
Thanks. I think you covered the points in the old discussion thread.
Documentation/merge-options.txt line 65
Quoted 11 lines
Documentation/merge-options.txt +3
-0@@ -60,6 +60,10 @@ a fast-forward, only update the branch pointer. This is the default behavior of git-merge. +--ff-only:: + Refuse to merge unless the merge can be resolved as a + fast-forward.
Do you or do you not allow "already up to date"? I think it makes sense to allow it, but it is unclear from these two lines.
Documentation/merge-options.txt int cmd_merge(int argc, const char **argv, const char *prefix) · line 881
Quoted 6 lines
Documentation/merge-options.txt +2
-0@@ -874,6 +877,9 @@ int cmd_merge(int argc, const char **argv, const char *prefix) option_commit = 0; } + if (!allow_fast_forward && fast_forward_only) + die("You cannot combine --no-ff with --ff-only.");
Are these the only nonsensical combinations? How should this interact with other options, e.g. --squash or --message?
Documentation/merge-options.txt int cmd_merge(int argc, const char **argv, const char *prefix) · line 982
Quoted 10 lines
Documentation/merge-options.txt +4
-1@@ -969,8 +975,11 @@ int cmd_merge(int argc, const char **argv, const char *prefix) } for (i = 0; i < use_strategies_nr; i++) { - if (use_strategies[i]->attr & NO_FAST_FORWARD) + if (use_strategies[i]->attr & NO_FAST_FORWARD) { allow_fast_forward = 0; + if (fast_forward_only) + die("You cannot combine --ff-only with the merge strategy '%s'.", use_strategies[i]->name); + }
I am not convinced this tests the right condition nor it is placed at the right place in the codepath---even if a specified strategy happens to allow fast-forward, wouldn't it be nonsense to say
$ git merge --ff-only -s resolve that-one
in the first place? Note that I am not saying "I am convinced this is wrong."
Documentation/merge-options.txt int cmd_merge(int argc, const char **argv, const char *prefix) · line 1052
Quoted 6 lines
Documentation/merge-options.txt +1
-1@@ -1040,7 +1049,7 @@ int cmd_merge(int argc, const char **argv, const char *prefix) * only one common. */ refresh_cache(REFRESH_QUIET); - if (allow_trivial) { + if (allow_trivial && !fast_forward_only) {
Good.
Documentation/merge-options.txt int cmd_merge(int argc, const char **argv, const char *prefix) · line 1092
Quoted 6 lines
Documentation/merge-options.txt +2
-0@@ -1079,6 +1088,9 @@ int cmd_merge(int argc, const char **argv, const char *prefix) } } + if (fast_forward_only) + die("Not possible to fast forward, aborting.");
Good.
Quoted 8 lines · Junio C Hamano
Documentation/merge-options.txt line 65
Quoted 4 lines · Björn Gustavsson
+--ff-only::
Documentation/merge-options.txt +2
-0+ Refuse to merge unless the merge can be resolved as a + fast-forward.Do you or do you not allow "already up to date"? I think it makes sense to allow it, but it is unclear from these two lines.
I do allow it. I will change the description to the following in the re-roll:
--ff-only::
Refuse to merge and exit with a non-zero status unless the current HEAD is already up-to-date or the merge can be resolved as a fast-forward.
Quoted 10 lines
builtin-merge.c int cmd_merge(int argc, const char **argv, const char *prefix) · line 881
Quoted 7 lines
builtin-merge.c +2
-0@@ -874,6 +877,9 @@ int cmd_merge(int argc, const char **argv, const char *prefix) option_commit =0 ; } +if (!allow_fast_forward && fast_forward_only) +die ("You cannot combine --no-ff with --ff-only." );Are these the only nonsensical combinations? How should this interact with other options, e.g. --squash or --message?
They are the only options I can think of that flatly contradict each other.
Combining --squash and --ff-only will succeed if the current HEAD can be fast-forwarded and will abort otherwise. I don't know how useful that would be in practice, but I see no strong reason to forbid it.
The -m option will always be ignored, of course, and there will be the usual warning if fast-forward is possible:
Fast forward (no commit created; -m option ignored)
I don't think there is any need to explicitly forbid the combination of -m and --ff-only.
I should probably update the commit message in the re-roll to include the information in the previous paragraphs.
Quoted 15 lines
builtin-merge.c int cmd_merge(int argc, const char **argv, const char *prefix) · line 982
Quoted 10 lines
builtin-merge.c +4
-1@@ -969,8 +975,11 @@ int cmd_merge(int argc, const char **argv, const char *prefix) }for (i =0 ; i < use_strategies_nr; i++) { -if (use_strategies[i]->attr &NO_FAST_FORWARD ) +if (use_strategies[i]->attr &NO_FAST_FORWARD ) { allow_fast_forward =0 ; +if (fast_forward_only) +die ("You cannot combine --ff-only with the merge strategy '%s'." , use_strategies[i]->name); + }I am not convinced this tests the right condition nor it is placed at the right place in the codepath---even if a specified strategy happens to allow fast-forward, wouldn't it be nonsense to say
Quoted 4 lines
$ git merge --ff-only -s resolve that-onein the first place? Note that I am not saying "I am convinced this is wrong."
Re-thinking it, I think that the test should be removed. It seemed like a good idea at the time to point out which strategy that prevented the fast-forward, but if there is a list of merge strategies, the test will prevent --ff-only to succeed if *any* of merge strategies cannot fast-forward. (Also, but I am not sure about this, a merge strategy that does not allow fast-forward might allow up-to-date.)
Therefore, I will remove the test in the re-roll.
Thanks for the comments!
/Björn