rebase: --fork-point regression fix

"git rebase --fork-point master" used to work OK, as it internally
called "git merge-base --fork-point" that knew how to handle short
refname and dwim it to the full refname before calling the
underlying get_fork_point() function.

This is no longer true after the command was rewritten in C, as its
internall call made directly to get_fork_point() does not dwim a
short ref.

Move the "dwim the refname argument to the full refname" logic that
is used in "git merge-base" to the underlying get_fork_point()
function, so that the other caller of the function in the
implementation of "git rebase" behaves the same way to fix this
regression.

Signed-off-by: Alex Torok <alext9@gmail.com>
[jc: revamped the fix and used Alex's tests]
Signed-off-by: Junio C Hamano <gitster@pobox.com>

diff --git a/builtin/merge-base.c b/builtin/merge-base.c
index e3f8da13b69b8dbe6898bf71ee2b5df206179bd1..6719ac198dc2092ae49829d5946ca00664e34b0e 100644 (file)
--- a/builtin/merge-base.c
+++ b/builtin/merge-base.c
@@ -114,26 +114,16 @@ static int handle_is_ancestor(int argc, const char **argv)
 static int handle_fork_point(int argc, const char **argv)
 {
        struct object_id oid;
-       char *refname;
        struct commit *derived, *fork_point;
        const char *commitname;
 
-       switch (dwim_ref(argv[0], strlen(argv[0]), &oid, &refname)) {
-       case 0:
-               die("No such ref: '%s'", argv[0]);
-       case 1:
-               break; /* good */
-       default:
-               die("Ambiguous refname: '%s'", argv[0]);
-       }
-
        commitname = (argc == 2) ? argv[1] : "HEAD";
        if (get_oid(commitname, &oid))
                die("Not a valid object name: '%s'", commitname);
 
        derived = lookup_commit_reference(the_repository, &oid);
 
-       fork_point = get_fork_point(refname, derived);
+       fork_point = get_fork_point(argv[0], derived);
 
        if (!fork_point)
                return 1;

diff --git a/commit.c b/commit.c
index 40890ae7ce8a48a2da06bcde6fc5a9db77260707..016f14fe95f237c84f10c0ae2e4463da1b4fba12 100644 (file)
--- a/commit.c
+++ b/commit.c
@@ -903,12 +903,22 @@ struct commit *get_fork_point(const char *refname, struct commit *commit)
        struct commit_list *bases;
        int i;
        struct commit *ret = NULL;
+       char *full_refname;
+
+       switch (dwim_ref(refname, strlen(refname), &oid, &full_refname)) {
+       case 0:
+               die("No such ref: '%s'", refname);
+       case 1:
+               break; /* good */
+       default:
+               die("Ambiguous refname: '%s'", refname);
+       }
 
        memset(&revs, 0, sizeof(revs));
        revs.initial = 1;
-       for_each_reflog_ent(refname, collect_one_reflog_ent, &revs);
+       for_each_reflog_ent(full_refname, collect_one_reflog_ent, &revs);
 
-       if (!revs.nr && !get_oid(refname, &oid))
+       if (!revs.nr)
                add_one_commit(&oid, &revs);
 
        for (i = 0; i < revs.nr; i++)
@@ -934,6 +944,7 @@ struct commit *get_fork_point(const char *refname, struct commit *commit)
 
 cleanup_return:
        free_commit_list(bases);
+       free(full_refname);
        return ret;
 }
 

diff --git a/t/t3431-rebase-fork-point.sh b/t/t3431-rebase-fork-point.sh
index 78851b9a2ae84d6b73b3cffc0026082abce322e1..172562789e474042922db296c0fce842f2d25437 100755 (executable)
--- a/t/t3431-rebase-fork-point.sh
+++ b/t/t3431-rebase-fork-point.sh
@@ -47,11 +47,31 @@ test_rebase 'G F B A' --keep-base
 test_rebase 'G F C E D B A' --no-fork-point
 test_rebase 'G F C D B A' --no-fork-point --onto D
 test_rebase 'G F C B A' --no-fork-point --keep-base
+
 test_rebase 'G F E D B A' --fork-point refs/heads/master
+test_rebase 'G F E D B A' --fork-point master
+
 test_rebase 'G F D B A' --fork-point --onto D refs/heads/master
+test_rebase 'G F D B A' --fork-point --onto D master
+
 test_rebase 'G F B A' --fork-point --keep-base refs/heads/master
+test_rebase 'G F B A' --fork-point --keep-base master
+
 test_rebase 'G F C E D B A' refs/heads/master
+test_rebase 'G F C E D B A' master
+
 test_rebase 'G F C D B A' --onto D refs/heads/master
+test_rebase 'G F C D B A' --onto D master
+
 test_rebase 'G F C B A' --keep-base refs/heads/master
+test_rebase 'G F C B A' --keep-base master
+
+test_expect_success 'git rebase --fork-point with ambigous refname' '
+       git checkout master &&
+       git checkout -b one &&
+       git checkout side &&
+       git tag one &&
+       test_must_fail git rebase --fork-point --onto D one
+'
 
 test_done