rebase: use correct base for --keep-base when a branch is given

--keep-base rebases onto the merge base of the given upstream and the
current HEAD regardless of whether a branch is given. This is contrary
to the documentation and to the option's intended purpose. Instead,
rebase onto the merge base of the given upstream and the given branch.

Signed-off-by: Alex Henrie <alexhenrie24@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

diff --git a/Documentation/git-rebase.txt b/Documentation/git-rebase.txt
index 9da4647061cdc7e85df685169c81f2f08b9d6707..262fb01aec004c880e16d2630494a194f720a713 100644 (file)
--- a/Documentation/git-rebase.txt
+++ b/Documentation/git-rebase.txt
@@ -215,9 +215,10 @@ leave out at most one of A and B, in which case it defaults to HEAD.
 
 --keep-base::
        Set the starting point at which to create the new commits to the
-       merge base of <upstream> <branch>. Running
+       merge base of <upstream> and <branch>. Running
        'git rebase --keep-base <upstream> <branch>' is equivalent to
-       running 'git rebase --onto <upstream>... <upstream>'.
+       running
+       'git rebase --onto <upstream>...<branch> <upstream> <branch>'.
 +
 This option is useful in the case where one is developing a feature on
 top of an upstream branch. While the feature is being worked on, the

diff --git a/builtin/rebase.c b/builtin/rebase.c
index 27fde7bf2815d3f7af22f87d14c098edce5c552d..7f3bffc0a234ad54310258f47d185e907f8ac585 100644 (file)
--- a/builtin/rebase.c
+++ b/builtin/rebase.c
@@ -1583,33 +1583,6 @@ int cmd_rebase(int argc, const char **argv, const char *prefix)
                options.upstream_arg = "--root";
        }
 
-       /* Make sure the branch to rebase onto is valid. */
-       if (keep_base) {
-               strbuf_reset(&buf);
-               strbuf_addstr(&buf, options.upstream_name);
-               strbuf_addstr(&buf, "...");
-               options.onto_name = xstrdup(buf.buf);
-       } else if (!options.onto_name)
-               options.onto_name = options.upstream_name;
-       if (strstr(options.onto_name, "...")) {
-               if (get_oid_mb(options.onto_name, &merge_base) < 0) {
-                       if (keep_base)
-                               die(_("'%s': need exactly one merge base with branch"),
-                                   options.upstream_name);
-                       else
-                               die(_("'%s': need exactly one merge base"),
-                                   options.onto_name);
-               }
-               options.onto = lookup_commit_or_die(&merge_base,
-                                                   options.onto_name);
-       } else {
-               options.onto =
-                       lookup_commit_reference_by_name(options.onto_name);
-               if (!options.onto)
-                       die(_("Does not point to a valid commit '%s'"),
-                               options.onto_name);
-       }
-
        /*
         * If the branch to rebase is given, that is the branch we will rebase
         * branch_name -- branch/commit being rebased, or
@@ -1659,6 +1632,34 @@ int cmd_rebase(int argc, const char **argv, const char *prefix)
        } else
                BUG("unexpected number of arguments left to parse");
 
+       /* Make sure the branch to rebase onto is valid. */
+       if (keep_base) {
+               strbuf_reset(&buf);
+               strbuf_addstr(&buf, options.upstream_name);
+               strbuf_addstr(&buf, "...");
+               strbuf_addstr(&buf, branch_name);
+               options.onto_name = xstrdup(buf.buf);
+       } else if (!options.onto_name)
+               options.onto_name = options.upstream_name;
+       if (strstr(options.onto_name, "...")) {
+               if (get_oid_mb(options.onto_name, &merge_base) < 0) {
+                       if (keep_base)
+                               die(_("'%s': need exactly one merge base with branch"),
+                                   options.upstream_name);
+                       else
+                               die(_("'%s': need exactly one merge base"),
+                                   options.onto_name);
+               }
+               options.onto = lookup_commit_or_die(&merge_base,
+                                                   options.onto_name);
+       } else {
+               options.onto =
+                       lookup_commit_reference_by_name(options.onto_name);
+               if (!options.onto)
+                       die(_("Does not point to a valid commit '%s'"),
+                               options.onto_name);
+       }
+
        if (options.fork_point > 0) {
                struct commit *head =
                        lookup_commit_reference(the_repository,

diff --git a/t/t3416-rebase-onto-threedots.sh b/t/t3416-rebase-onto-threedots.sh
index 3716a42e81284f562cef9ba951edbbd1ff6701d6..3e04802cb003b027c0d70319465af4363877de06 100755 (executable)
--- a/t/t3416-rebase-onto-threedots.sh
+++ b/t/t3416-rebase-onto-threedots.sh
@@ -129,6 +129,20 @@ test_expect_success 'rebase --keep-base main from topic' '
        test_cmp expect actual
 '
 
+test_expect_success 'rebase --keep-base main topic from main' '
+       git checkout main &&
+       git branch -f topic G &&
+
+       git rebase --keep-base main topic &&
+       git rev-parse C >base.expect &&
+       git merge-base main HEAD >base.actual &&
+       test_cmp base.expect base.actual &&
+
+       git rev-parse HEAD~2 >actual &&
+       git rev-parse C^0 >expect &&
+       test_cmp expect actual
+'
+
 test_expect_success 'rebase --keep-base main from side' '
        git reset --hard &&
        git checkout side &&
@@ -153,6 +167,21 @@ test_expect_success 'rebase -i --keep-base main from topic' '
        test_cmp expect actual
 '
 
+test_expect_success 'rebase -i --keep-base main topic from main' '
+       git checkout main &&
+       git branch -f topic G &&
+
+       set_fake_editor &&
+       EXPECT_COUNT=2 git rebase -i --keep-base main topic &&
+       git rev-parse C >base.expect &&
+       git merge-base main HEAD >base.actual &&
+       test_cmp base.expect base.actual &&
+
+       git rev-parse HEAD~2 >actual &&
+       git rev-parse C^0 >expect &&
+       test_cmp expect actual
+'
+
 test_expect_success 'rebase -i --keep-base main from side' '
        git reset --hard &&
        git checkout side &&