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 <newren@xxxxxxxxx>
--
2.18.0.rc2.92.g133ed01dde
