git-merge.sh: better handling of combined --squash,--no-ff,--no-commit options

git-merge used to use either the --squash,--no-squash, --no-ff,--ff,
--no-commit,--commit option, whichever came last in the command line.
This lead to some un-intuitive behavior, having

 git merge --no-commit --no-ff <branch>

actually commit the merge.  Now git-merge respects --no-commit together
with --no-ff, as well as other combinations of the options.  However,
this broke a selftest in t/t7600-merge.sh which expected to have --no-ff
completely override the --squash option, so that

 git merge --squash --no-ff <branch>

fast-forwards, and makes a merge commit; combining --squash with --no-ff
doesn't really make sense though, and is now refused by git-merge.  The
test is adapted to test --no-ff without the preceding --squash, and
another test is added to make sure the --squash --no-ff combination is
refused.

The unexpected behavior was reported by John Goerzen through
 http://bing.sdebian.org/468568

Signed-off-by: Gerrit Pape <pape@smarden.org>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

diff --git a/git-merge.sh b/git-merge.sh
index 1c123a3..03cd398 100755 (executable)
--- a/git-merge.sh
+++ b/git-merge.sh
@@ -37,6 +37,7 @@ use_strategies=
 
 allow_fast_forward=t
 allow_trivial_merge=t
+squash= no_commit=
 
 dropsave() {
        rm -f -- "$GIT_DIR/MERGE_HEAD" "$GIT_DIR/MERGE_MSG" \
@@ -152,17 +153,21 @@ parse_config () {
                --summary)
                        show_diffstat=t ;;
                --squash)
-                       allow_fast_forward=t squash=t no_commit=t ;;
+                       test "$allow_fast_forward" = t ||
+                               die "You cannot combine --squash with --no-ff."
+                       squash=t no_commit=t ;;
                --no-squash)
-                       allow_fast_forward=t squash= no_commit= ;;
+                       squash= no_commit= ;;
                --commit)
-                       allow_fast_forward=t squash= no_commit= ;;
+                       no_commit= ;;
                --no-commit)
-                       allow_fast_forward=t squash= no_commit=t ;;
+                       no_commit=t ;;
                --ff)
-                       allow_fast_forward=t squash= no_commit= ;;
+                       allow_fast_forward=t ;;
                --no-ff)
-                       allow_fast_forward=false squash= no_commit= ;;
+                       test "$squash" != t ||
+                               die "You cannot combine --squash with --no-ff."
+                       allow_fast_forward=f ;;
                -s|--strategy)
                        shift
                        case " $all_strategies " in

diff --git a/t/t7600-merge.sh b/t/t7600-merge.sh
index 50c51c8..5d16628 100755 (executable)
--- a/t/t7600-merge.sh
+++ b/t/t7600-merge.sh
@@ -419,6 +419,7 @@ test_debug 'gitk --all'
 
 test_expect_success 'merge c0 with c1 (no-ff)' '
        git reset --hard c0 &&
+       git config branch.master.mergeoptions "" &&
        test_tick &&
        git merge --no-ff c1 &&
        verify_merge file result.1 &&
@@ -427,6 +428,11 @@ test_expect_success 'merge c0 with c1 (no-ff)' '
 
 test_debug 'gitk --all'
 
+test_expect_success 'combining --squash and --no-ff is refused' '
+       test_must_fail git merge --squash --no-ff c1 &&
+       test_must_fail git merge --no-ff --squash c1
+'
+
 test_expect_success 'merge c0 with c1 (ff overrides no-ff)' '
        git reset --hard c0 &&
        git config branch.master.mergeoptions "--no-ff" &&