Skip to content

Commit 7cebe73

Browse files
gitstergitster
committed
Merge branch 'pw/rebase-of-a-tag-fix'
"git rebase <upstream> <tag>" failed when aborted in the middle, as it mistakenly tried to write the tag object instead of peeling it to HEAD. * pw/rebase-of-a-tag-fix: rebase: dereference tags rebase: use lookup_commit_reference_by_name() rebase: use our standard error return value t3407: rework rebase --quit tests t3407: strengthen rebase --abort tests t3407: use test_path_is_missing t3407: rename a variable t3407: use test_cmp_rev t3407: use test_commit t3407: run tests in $TEST_DIRECTORY
2 parents 921c795 + 7740ac6 commit 7cebe73

File tree

2 files changed

+67
-87
lines changed

2 files changed

+67
-87
lines changed

‎builtin/rebase.c‎

Lines changed: 21 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -765,17 +765,6 @@ static int finish_rebase(struct rebase_options *opts)
765765
return ret;
766766
}
767767

768-
static struct commit *peel_committish(const char *name)
769-
{
770-
struct object *obj;
771-
struct object_id oid;
772-
773-
if (get_oid(name, &oid))
774-
return NULL;
775-
obj = parse_object(the_repository, &oid);
776-
return (struct commit *)peel_to_type(name, 0, obj, OBJ_COMMIT);
777-
}
778-
779768
static void add_var(struct strbuf *buf, const char *name, const char *value)
780769
{
781770
if (!value)
@@ -1580,7 +1569,7 @@ int cmd_rebase(int argc, const char **argv, const char *prefix)
15801569
die(_("could not move back to %s"),
15811570
oid_to_hex(&options.orig_head));
15821571
remove_branch_state(the_repository, 0);
1583-
ret = !!finish_rebase(&options);
1572+
ret = finish_rebase(&options);
15841573
goto cleanup;
15851574
}
15861575
case ACTION_QUIT: {
@@ -1589,11 +1578,11 @@ int cmd_rebase(int argc, const char **argv, const char *prefix)
15891578
struct replay_opts replay = REPLAY_OPTS_INIT;
15901579

15911580
replay.action = REPLAY_INTERACTIVE_REBASE;
1592-
ret = !!sequencer_remove_state(&replay);
1581+
ret = sequencer_remove_state(&replay);
15931582
} else {
15941583
strbuf_reset(&buf);
15951584
strbuf_addstr(&buf, options.state_dir);
1596-
ret = !!remove_dir_recursively(&buf, 0);
1585+
ret = remove_dir_recursively(&buf, 0);
15971586
if (ret)
15981587
error(_("could not remove '%s'"),
15991588
options.state_dir);
@@ -1851,7 +1840,8 @@ int cmd_rebase(int argc, const char **argv, const char *prefix)
18511840
if (!strcmp(options.upstream_name, "-"))
18521841
options.upstream_name = "@{-1}";
18531842
}
1854-
options.upstream = peel_committish(options.upstream_name);
1843+
options.upstream =
1844+
lookup_commit_reference_by_name(options.upstream_name);
18551845
if (!options.upstream)
18561846
die(_("invalid upstream '%s'"), options.upstream_name);
18571847
options.upstream_arg = options.upstream_name;
@@ -1894,7 +1884,8 @@ int cmd_rebase(int argc, const char **argv, const char *prefix)
18941884
options.onto = lookup_commit_or_die(&merge_base,
18951885
options.onto_name);
18961886
} else {
1897-
options.onto = peel_committish(options.onto_name);
1887+
options.onto =
1888+
lookup_commit_reference_by_name(options.onto_name);
18981889
if (!options.onto)
18991890
die(_("Does not point to a valid commit '%s'"),
19001891
options.onto_name);
@@ -1919,13 +1910,15 @@ int cmd_rebase(int argc, const char **argv, const char *prefix)
19191910
die_if_checked_out(buf.buf, 1);
19201911
options.head_name = xstrdup(buf.buf);
19211912
/* If not is it a valid ref (branch or commit)? */
1922-
} else if (!get_oid(branch_name, &options.orig_head) &&
1923-
lookup_commit_reference(the_repository,
1924-
&options.orig_head))
1913+
} else {
1914+
struct commit *commit =
1915+
lookup_commit_reference_by_name(branch_name);
1916+
if (!commit)
1917+
die(_("no such branch/commit '%s'"),
1918+
branch_name);
1919+
oidcpy(&options.orig_head, &commit->object.oid);
19251920
options.head_name = NULL;
1926-
else
1927-
die(_("no such branch/commit '%s'"),
1928-
branch_name);
1921+
}
19291922
} else if (argc == 0) {
19301923
/* Do not need to switch branches, we are already on it. */
19311924
options.head_name =
@@ -1965,7 +1958,7 @@ int cmd_rebase(int argc, const char **argv, const char *prefix)
19651958

19661959
if (require_clean_work_tree(the_repository, "rebase",
19671960
_("Please commit or stash them."), 1, 1)) {
1968-
ret = 1;
1961+
ret = -1;
19691962
goto cleanup;
19701963
}
19711964

@@ -2000,7 +1993,7 @@ int cmd_rebase(int argc, const char **argv, const char *prefix)
20001993
RESET_HEAD_RUN_POST_CHECKOUT_HOOK,
20011994
NULL, buf.buf,
20021995
DEFAULT_REFLOG_ACTION) < 0) {
2003-
ret = !!error(_("could not switch to "
1996+
ret = error(_("could not switch to "
20041997
"%s"),
20051998
options.switch_to);
20061999
goto cleanup;
@@ -2015,7 +2008,7 @@ int cmd_rebase(int argc, const char **argv, const char *prefix)
20152008
else
20162009
printf(_("Current branch %s is up to date.\n"),
20172010
branch_name);
2018-
ret = !!finish_rebase(&options);
2011+
ret = finish_rebase(&options);
20192012
goto cleanup;
20202013
} else if (!(options.flags & REBASE_NO_QUIET))
20212014
; /* be quiet */
@@ -2093,7 +2086,7 @@ int cmd_rebase(int argc, const char **argv, const char *prefix)
20932086
RESET_HEAD_REFS_ONLY, "HEAD", msg.buf,
20942087
DEFAULT_REFLOG_ACTION);
20952088
strbuf_release(&msg);
2096-
ret = !!finish_rebase(&options);
2089+
ret = finish_rebase(&options);
20972090
goto cleanup;
20982091
}
20992092

@@ -2107,7 +2100,7 @@ int cmd_rebase(int argc, const char **argv, const char *prefix)
21072100
options.revisions = revisions.buf;
21082101

21092102
run_rebase:
2110-
ret = !!run_specific_rebase(&options, action);
2103+
ret = run_specific_rebase(&options, action);
21112104

21122105
cleanup:
21132106
strbuf_release(&buf);
@@ -2118,5 +2111,5 @@ int cmd_rebase(int argc, const char **argv, const char *prefix)
21182111
free(options.strategy);
21192112
strbuf_release(&options.git_format_patch_opt);
21202113
free(squash_onto_name);
2121-
return ret;
2114+
return !!ret;
21222115
}

‎t/t3407-rebase-abort.sh‎

Lines changed: 46 additions & 59 deletions
Original file line numberDiff line numberDiff line change
@@ -7,77 +7,77 @@ export GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME
77

88
. ./test-lib.sh
99

10-
### Test that we handle space characters properly
11-
work_dir="$(pwd)/test dir"
12-
1310
test_expect_success setup '
14-
mkdir -p "$work_dir" &&
15-
cd "$work_dir" &&
16-
git init &&
17-
echo a > a &&
18-
git add a &&
19-
git commit -m a &&
11+
test_commit a a a &&
2012
git branch to-rebase &&
2113
22-
echo b > a &&
23-
git commit -a -m b &&
24-
echo c > a &&
25-
git commit -a -m c &&
14+
test_commit --annotate b a b &&
15+
test_commit --annotate c a c &&
2616
2717
git checkout to-rebase &&
28-
echo d > a &&
29-
git commit -a -m "merge should fail on this" &&
30-
echo e > a &&
31-
git commit -a -m "merge should fail on this, too" &&
32-
git branch pre-rebase
18+
test_commit "merge should fail on this" a d d &&
19+
test_commit --annotate "merge should fail on this, too" a e pre-rebase
3320
'
3421

22+
# Check that HEAD is equal to "pre-rebase" and the current branch is
23+
# "to-rebase"
24+
check_head() {
25+
test_cmp_rev HEAD pre-rebase^{commit} &&
26+
test "$(git symbolic-ref HEAD)" = refs/heads/to-rebase
27+
}
28+
3529
testrebase() {
3630
type=$1
37-
dotest=$2
31+
state_dir=$2
3832

3933
test_expect_success "rebase$type --abort" '
40-
cd "$work_dir" &&
4134
# Clean up the state from the previous one
4235
git reset --hard pre-rebase &&
4336
test_must_fail git rebase$type main &&
44-
test_path_is_dir "$dotest" &&
37+
test_path_is_dir "$state_dir" &&
4538
git rebase --abort &&
46-
test $(git rev-parse to-rebase) = $(git rev-parse pre-rebase) &&
47-
test ! -d "$dotest"
39+
check_head &&
40+
test_path_is_missing "$state_dir"
4841
'
4942

5043
test_expect_success "rebase$type --abort after --skip" '
51-
cd "$work_dir" &&
5244
# Clean up the state from the previous one
5345
git reset --hard pre-rebase &&
5446
test_must_fail git rebase$type main &&
55-
test_path_is_dir "$dotest" &&
47+
test_path_is_dir "$state_dir" &&
5648
test_must_fail git rebase --skip &&
57-
test $(git rev-parse HEAD) = $(git rev-parse main) &&
49+
test_cmp_rev HEAD main &&
5850
git rebase --abort &&
59-
test $(git rev-parse to-rebase) = $(git rev-parse pre-rebase) &&
60-
test ! -d "$dotest"
51+
check_head &&
52+
test_path_is_missing "$state_dir"
6153
'
6254

6355
test_expect_success "rebase$type --abort after --continue" '
64-
cd "$work_dir" &&
6556
# Clean up the state from the previous one
6657
git reset --hard pre-rebase &&
6758
test_must_fail git rebase$type main &&
68-
test_path_is_dir "$dotest" &&
59+
test_path_is_dir "$state_dir" &&
6960
echo c > a &&
7061
echo d >> a &&
7162
git add a &&
7263
test_must_fail git rebase --continue &&
73-
test $(git rev-parse HEAD) != $(git rev-parse main) &&
64+
test_cmp_rev ! HEAD main &&
65+
git rebase --abort &&
66+
check_head &&
67+
test_path_is_missing "$state_dir"
68+
'
69+
70+
test_expect_success "rebase$type --abort when checking out a tag" '
71+
test_when_finished "git symbolic-ref HEAD refs/heads/to-rebase" &&
72+
git reset --hard a -- &&
73+
test_must_fail git rebase$type --onto b c pre-rebase &&
74+
test_cmp_rev HEAD b^{commit} &&
7475
git rebase --abort &&
75-
test $(git rev-parse to-rebase) = $(git rev-parse pre-rebase) &&
76-
test ! -d "$dotest"
76+
test_cmp_rev HEAD pre-rebase^{commit} &&
77+
! git symbolic-ref HEAD
7778
'
7879

7980
test_expect_success "rebase$type --abort does not update reflog" '
80-
cd "$work_dir" &&
8181
# Clean up the state from the previous one
8282
git reset --hard pre-rebase &&
8383
git reflog show to-rebase > reflog_before &&
@@ -89,41 +89,28 @@ testrebase() {
8989
'
9090

9191
test_expect_success 'rebase --abort can not be used with other options' '
92-
cd "$work_dir" &&
9392
# Clean up the state from the previous one
9493
git reset --hard pre-rebase &&
9594
test_must_fail git rebase$type main &&
9695
test_must_fail git rebase -v --abort &&
9796
test_must_fail git rebase --abort -v &&
9897
git rebase --abort
9998
'
99+
100+
test_expect_success "rebase$type --quit" '
101+
test_when_finished "git symbolic-ref HEAD refs/heads/to-rebase" &&
102+
# Clean up the state from the previous one
103+
git reset --hard pre-rebase &&
104+
test_must_fail git rebase$type main &&
105+
test_path_is_dir $state_dir &&
106+
head_before=$(git rev-parse HEAD) &&
107+
git rebase --quit &&
108+
test_cmp_rev HEAD $head_before &&
109+
test_path_is_missing .git/rebase-apply
110+
'
100111
}
101112

102113
testrebase " --apply" .git/rebase-apply
103114
testrebase " --merge" .git/rebase-merge
104115

105-
test_expect_success 'rebase --apply --quit' '
106-
cd "$work_dir" &&
107-
# Clean up the state from the previous one
108-
git reset --hard pre-rebase &&
109-
test_must_fail git rebase --apply main &&
110-
test_path_is_dir .git/rebase-apply &&
111-
head_before=$(git rev-parse HEAD) &&
112-
git rebase --quit &&
113-
test $(git rev-parse HEAD) = $head_before &&
114-
test ! -d .git/rebase-apply
115-
'
116-
117-
test_expect_success 'rebase --merge --quit' '
118-
cd "$work_dir" &&
119-
# Clean up the state from the previous one
120-
git reset --hard pre-rebase &&
121-
test_must_fail git rebase --merge main &&
122-
test_path_is_dir .git/rebase-merge &&
123-
head_before=$(git rev-parse HEAD) &&
124-
git rebase --quit &&
125-
test $(git rev-parse HEAD) = $head_before &&
126-
test ! -d .git/rebase-merge
127-
'
128-
129116
test_done

0 commit comments

Comments
 (0)