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>
Junio C Hamano committed
Dec 9, 2019 at 10:51 UTC
f08132f889c00a8108f61541e047649ad0e660e4
3 files changed
+34
-13
builtin/merge-base.c
+1
-11
@@ -114,26 +114,16 @@ static int handle_is_ancestor(int argc, const char **argv)
114
static int handle_fork_point(int argc, const char **argv)
115
{
116
struct object_id oid;
117
- char *refname;
117
struct commit *derived, *fork_point;
118
const char *commitname;
119
121
- switch (dwim_ref(argv[0], strlen(argv[0]), &oid, &refname)) {
122
- case 0:
123
- die("No such ref: '%s'", argv[0]);
124
- case 1:
125
- break; /* good */
126
- default:
127
- die("Ambiguous refname: '%s'", argv[0]);
128
- }
129
-
120
commitname = (argc == 2) ? argv[1] : "HEAD";
121
if (get_oid(commitname, &oid))
122
die("Not a valid object name: '%s'", commitname);
123
124
derived = lookup_commit_reference(the_repository, &oid);
125
136
- fork_point = get_fork_point(refname, derived);
126
+ fork_point = get_fork_point(argv[0], derived);
127
128
if (!fork_point)
129
return 1;
commit.c
+13
-2
@@ -903,12 +903,22 @@ struct commit *get_fork_point(const char *refname, struct commit *commit)
903
struct commit_list *bases;
904
int i;
905
struct commit *ret = NULL;
906
+ char *full_refname;
907
+
908
+ switch (dwim_ref(refname, strlen(refname), &oid, &full_refname)) {
909
+ case 0:
910
+ die("No such ref: '%s'", refname);
911
+ case 1:
912
+ break; /* good */
913
+ default:
914
+ die("Ambiguous refname: '%s'", refname);
915
+ }
916
917
memset(&revs, 0, sizeof(revs));
918
revs.initial = 1;
909
- for_each_reflog_ent(refname, collect_one_reflog_ent, &revs);
919
+ for_each_reflog_ent(full_refname, collect_one_reflog_ent, &revs);
920
911
- if (!revs.nr && !get_oid(refname, &oid))
921
+ if (!revs.nr)
922
add_one_commit(&oid, &revs);
923
924
for (i = 0; i < revs.nr; i++)
@@ -934,6 +944,7 @@ struct commit *get_fork_point(const char *refname, struct commit *commit)
944
945
cleanup_return:
946
free_commit_list(bases);
947
+ free(full_refname);
948
return ret;
949
}
950
t/t3431-rebase-fork-point.sh
+20
@@ -47,11 +47,31 @@ test_rebase 'G F B A' --keep-base
47
test_rebase 'G F C E D B A' --no-fork-point
48
test_rebase 'G F C D B A' --no-fork-point --onto D
49
test_rebase 'G F C B A' --no-fork-point --keep-base
50
+
51
test_rebase 'G F E D B A' --fork-point refs/heads/master
52
+test_rebase 'G F E D B A' --fork-point master
53
+
54
test_rebase 'G F D B A' --fork-point --onto D refs/heads/master
55
+test_rebase 'G F D B A' --fork-point --onto D master
56
+
57
test_rebase 'G F B A' --fork-point --keep-base refs/heads/master
58
+test_rebase 'G F B A' --fork-point --keep-base master
59
+
60
test_rebase 'G F C E D B A' refs/heads/master
61
+test_rebase 'G F C E D B A' master
62
+
63
test_rebase 'G F C D B A' --onto D refs/heads/master
64
+test_rebase 'G F C D B A' --onto D master
65
+
66
test_rebase 'G F C B A' --keep-base refs/heads/master
67
+test_rebase 'G F C B A' --keep-base master
68
+
69
+test_expect_success 'git rebase --fork-point with ambigous refname' '
70
+ git checkout master &&
71
+ git checkout -b one &&
72
+ git checkout side &&
73
+ git tag one &&
74
+ test_must_fail git rebase --fork-point --onto D one
75
+'
76
77
test_done