Subject: [PATCH v3 0/7] Document/fix/warn about rebase incompatibilities and inconsistencies

From: Elijah Newren <[email-masked]>
Date: Thu, 21 Jun 2018 08:00:16 -0700

git-rebase has lots of options that are mutually incompatible.  Even among
aspects of its behavior that is common to all rebase types, it has a number
of inconsistencies.  This series tries to document, fix, and/or warn users
about many of these.

I have left patch 7 in RFC state.  We probably need more opinions
about it, and whether cherry-pick should also be changed to match.

Changes since v2 (full branch-diff below):
  - Slight changes to documentation wording
  - Add incompatibilitiy of --rebase-merges and --strategy-option
  - Add tests and code for incompatibilities involving either
    --preserve-merges or --rebase-merges
  - Slightly more detail in the commit message for making -m and -i behave
    the same with empty commit messages as am-based rebases.

Many thanks to Phillip for reviewing the last series, and Eric who pointed
out the wording improvements.

Elijah Newren (7):
  git-rebase.txt: document incompatible options
  git-rebase.sh: update help messages a bit
  t3422: new testcases for checking when incompatible options passed
  git-rebase: error out when incompatible options passed
  git-rebase.txt: document behavioral inconsistencies between modes
  git-rebase.txt: address confusion between --no-ff vs --force-rebase
  git-rebase: make --allow-empty-message the default

 Documentation/git-rebase.txt           | 155 ++++++++++++++++++++-----
 git-rebase.sh                          |  42 ++++++-
 t/t3404-rebase-interactive.sh          |   7 +-
 t/t3405-rebase-malformed.sh            |  11 +-
 t/t3422-rebase-incompatible-options.sh |  89 ++++++++++++++
 5 files changed, 262 insertions(+), 42 deletions(-)
 create mode 100755 t/t3422-rebase-incompatible-options.sh

 1: d184d5bd3f !  1: 4cdf9130cc git-rebase.txt: document incompatible options
    @@ -191,14 +191,15 @@
     + * --keep-empty
     + * --autosquash
     + * --edit-todo
    -+ * --root + --onto
    ++ * --root when used in combination with --onto
     +
     +Other incompatible flag pairs:
     +
    -+ * --preserve-merges && --interactive
    -+ * --preserve-merges && --signoff
    -+ * --preserve-merges && --rebase-merges
    -+ * --rebase-merges && --strategy
    ++ * --preserve-merges and --interactive
    ++ * --preserve-merges and --signoff
    ++ * --preserve-merges and --rebase-merges
    ++ * --rebase-merges and --strategy
    ++ * --rebase-merges and --strategy-option
     +
      include::merge-strategies.txt[]
      
 2: 788df9fa43 =  2: e336f76c5e git-rebase.sh: update help messages a bit
 3: d3d124795a !  3: 4ab38d8a5f t3422: new testcases for checking when incompatible options passed
    @@ -83,4 +83,24 @@
     +test_run_rebase --committer-date-is-author-date
     +test_run_rebase -C4
     +
    ++test_expect_success '--preserve-merges incompatible with --signoff' '
    ++	git checkout B^0 &&
    ++	test_must_fail git rebase --preserve-merges --signoff A
    ++'
    ++
    ++test_expect_failure '--preserve-merges incompatible with --rebase-merges' '
    ++	git checkout B^0 &&
    ++	test_must_fail git rebase --preserve-merges --rebase-merges A
    ++'
    ++
    ++test_expect_failure '--rebase-merges incompatible with --strategy' '
    ++	git checkout B^0 &&
    ++	test_must_fail git rebase --rebase-merges -s resolve A
    ++'
    ++
    ++test_expect_failure '--rebase-merges incompatible with --strategy-option' '
    ++	git checkout B^0 &&
    ++	test_must_fail git rebase --rebase-merges -Xignore-space-change A
    ++'
    ++
     +test_done
 4: 28d2dfd49a !  4: 5223954caf git-rebase: error out when incompatible options passed
    @@ -56,6 +56,30 @@
      if test -n "$signoff"
      then
      	test -n "$preserve_merges" &&
    +@@
    + 	force_rebase=t
    + fi
    + 
    ++if test -n "$preserve_merges"
    ++then
    ++	# Note: incompatibility with --signoff handled in signoff block above
    ++	# Note: incompatibility with --interactive is just a strong warning;
    ++	#       git-rebase.txt caveats with "unless you know what you are doing"
    ++	test -n "$rebase_merges" &&
    ++		die "$(gettext "error: cannot combine '--preserve_merges' with '--rebase-merges'")"
    ++fi
    ++
    ++if test -n "$rebase_merges"
    ++then
    ++	test -n "$strategy_opts" &&
    ++		die "$(gettext "error: cannot combine '--rebase_merges' with '--strategy-option'")"
    ++	test -n "$strategy" &&
    ++		die "$(gettext "error: cannot combine '--rebase_merges' with '--strategy'")"
    ++fi
    ++
    + if test -z "$rebase_root"
    + then
    + 	case "$#" in
     
     diff --git a/t/t3422-rebase-incompatible-options.sh b/t/t3422-rebase-incompatible-options.sh
     --- a/t/t3422-rebase-incompatible-options.sh
    @@ -93,3 +117,24 @@
      		git checkout B^0 &&
      		test_must_fail git rebase $opt --exec 'true' A
      	"
    +@@
    + 	test_must_fail git rebase --preserve-merges --signoff A
    + '
    + 
    +-test_expect_failure '--preserve-merges incompatible with --rebase-merges' '
    ++test_expect_success '--preserve-merges incompatible with --rebase-merges' '
    + 	git checkout B^0 &&
    + 	test_must_fail git rebase --preserve-merges --rebase-merges A
    + '
    + 
    +-test_expect_failure '--rebase-merges incompatible with --strategy' '
    ++test_expect_success '--rebase-merges incompatible with --strategy' '
    + 	git checkout B^0 &&
    + 	test_must_fail git rebase --rebase-merges -s resolve A
    + '
    + 
    +-test_expect_failure '--rebase-merges incompatible with --strategy-option' '
    ++test_expect_success '--rebase-merges incompatible with --strategy-option' '
    + 	git checkout B^0 &&
    + 	test_must_fail git rebase --rebase-merges -Xignore-space-change A
    + '
 5: b2eec2cc5a !  5: 96f7ba98bc git-rebase.txt: document behavioral inconsistencies between modes
    @@ -14,8 +14,8 @@
     --- a/Documentation/git-rebase.txt
     +++ b/Documentation/git-rebase.txt
     @@
    -  * --preserve-merges && --rebase-merges
    -  * --rebase-merges && --strategy
    +  * --rebase-merges and --strategy
    +  * --rebase-merges and --strategy-option
      
     +BEHAVIORAL INCONSISTENCIES
     +--------------------------
 6: cea18d7e60 =  6: 7bb7b380ac git-rebase.txt: address confusion between --no-ff vs --force-rebase
 7: ab8805c40a !  7: 3ed07548a6 git-rebase: make --allow-empty-message the default
    @@ -2,9 +2,13 @@
     
         git-rebase: make --allow-empty-message the default
     
    -    am-based rebases already apply commits with an empty commit message
    -    without requiring the user to specify an extra flag.  Make merge-based and
    -    interactive-based rebases behave the same.
    +    All rebase backends should behave the same with empty commit messages, but
    +    currently do not.  am-based rebases already apply commits with an empty
    +    commit message without stopping or requiring the user to specify an extra
    +    flag.  Since am-based rebases are the default rebase type, and since it
    +    appears no one has ever requested a --no-allow-empty-message flag to
    +    change this behavior, make --allow-empty-message the default so that
    +    merge-based and interactive-based rebases will behave the same.
     
         Signed-off-by: Elijah Newren <[email-masked]>
     
-- 
2.18.0.rc2.92.g133ed01dde