Skip to content

Commit fa24bbe

Browse files
gitstergitster
committed
Merge branch 'js/rebase-i-with-colliding-hash' into maint
"git rebase -i" identifies existing commits in its todo file with their abbreviated object name, which could become ambigous as it goes to create new commits, and has a mechanism to avoid ambiguity in the main part of its execution. A few other cases however were not covered by the protection against ambiguity, which has been corrected. * js/rebase-i-with-colliding-hash: rebase -i: also avoid SHA-1 collisions with missingCommitsCheck rebase -i: re-fix short SHA-1 collision parse_insn_line(): improve error message when parsing failed
2 parents a7a2e12 + 2602762 commit fa24bbe

File tree

3 files changed

+34
-9
lines changed

3 files changed

+34
-9
lines changed

‎rebase-interactive.c‎

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -104,9 +104,11 @@ int edit_todo_list(struct repository *r, struct todo_list *todo_list,
104104
-1, flags | TODO_LIST_SHORTEN_IDS | TODO_LIST_APPEND_TODO_HELP))
105105
return error_errno(_("could not write '%s'"), todo_file);
106106

107-
if (initial && copy_file(rebase_path_todo_backup(), todo_file, 0666))
108-
return error(_("could not copy '%s' to '%s'."), todo_file,
109-
rebase_path_todo_backup());
107+
if (initial &&
108+
todo_list_write_to_file(r, todo_list, rebase_path_todo_backup(),
109+
shortrevisions, shortonto, -1,
110+
(flags | TODO_LIST_APPEND_TODO_HELP) & ~TODO_LIST_SHORTEN_IDS) < 0)
111+
return error(_("could not write '%s'."), rebase_path_todo_backup());
110112

111113
if (launch_sequence_editor(todo_file, &new_todo->buf, NULL))
112114
return -2;

‎sequencer.c‎

Lines changed: 14 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -2118,18 +2118,19 @@ static int parse_insn_line(struct repository *r, struct todo_item *item,
21182118
saved = *end_of_object_name;
21192119
*end_of_object_name = '\0';
21202120
status = get_oid(bol, &commit_oid);
2121+
if (status < 0)
2122+
error(_("could not parse '%s'"), bol); /* return later */
21212123
*end_of_object_name = saved;
21222124

21232125
bol = end_of_object_name + strspn(end_of_object_name, " \t");
21242126
item->arg_offset = bol - buf;
21252127
item->arg_len = (int)(eol - bol);
21262128

21272129
if (status < 0)
2128-
return error(_("could not parse '%.*s'"),
2129-
(int)(end_of_object_name - bol), bol);
2130+
return status;
21302131

21312132
item->commit = lookup_commit_reference(r, &commit_oid);
2132-
return !item->commit;
2133+
return item->commit ? 0 : -1;
21332134
}
21342135

21352136
int sequencer_get_last_command(struct repository *r, enum replay_action *action)
@@ -5075,7 +5076,7 @@ int complete_action(struct repository *r, struct replay_opts *opts, unsigned fla
50755076
{
50765077
const char *shortonto, *todo_file = rebase_path_todo();
50775078
struct todo_list new_todo = TODO_LIST_INIT;
5078-
struct strbuf *buf = &todo_list->buf;
5079+
struct strbuf *buf = &todo_list->buf, buf2 = STRBUF_INIT;
50795080
struct object_id oid = onto->object.oid;
50805081
int res;
50815082

@@ -5127,6 +5128,15 @@ int complete_action(struct repository *r, struct replay_opts *opts, unsigned fla
51275128
return -1;
51285129
}
51295130

5131+
/* Expand the commit IDs */
5132+
todo_list_to_strbuf(r, &new_todo, &buf2, -1, 0);
5133+
strbuf_swap(&new_todo.buf, &buf2);
5134+
strbuf_release(&buf2);
5135+
new_todo.total_nr -= new_todo.nr;
5136+
if (todo_list_parse_insn_buffer(r, new_todo.buf.buf, &new_todo) < 0)
5137+
BUG("invalid todo list after expanding IDs:\n%s",
5138+
new_todo.buf.buf);
5139+
51305140
if (opts->allow_ff && skip_unnecessary_picks(r, &new_todo, &oid)) {
51315141
todo_list_release(&new_todo);
51325142
return error(_("could not skip unnecessary pick commands"));

‎t/t3404-rebase-interactive.sh‎

Lines changed: 15 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1264,13 +1264,26 @@ test_expect_success SHA1 'short SHA-1 setup' '
12641264
test_expect_success SHA1 'short SHA-1 collide' '
12651265
test_when_finished "reset_rebase && git checkout master" &&
12661266
git checkout collide &&
1267+
colliding_sha1=6bcda37 &&
1268+
test $colliding_sha1 = "$(git rev-parse HEAD | cut -c 1-7)" &&
12671269
(
12681270
unset test_tick &&
12691271
test_tick &&
12701272
set_fake_editor &&
12711273
FAKE_COMMIT_MESSAGE="collide2 ac4f2ee" \
1272-
FAKE_LINES="reword 1 2" git rebase -i HEAD~2
1273-
)
1274+
FAKE_LINES="reword 1 break 2" git rebase -i HEAD~2 &&
1275+
test $colliding_sha1 = "$(git rev-parse HEAD | cut -c 1-7)" &&
1276+
grep "^pick $colliding_sha1 " \
1277+
.git/rebase-merge/git-rebase-todo.tmp &&
1278+
grep "^pick [0-9a-f]\{40\}" \
1279+
.git/rebase-merge/git-rebase-todo &&
1280+
grep "^pick [0-9a-f]\{40\}" \
1281+
.git/rebase-merge/git-rebase-todo.backup &&
1282+
git rebase --continue
1283+
) &&
1284+
collide2="$(git rev-parse HEAD~1 | cut -c 1-4)" &&
1285+
collide3="$(git rev-parse collide3 | cut -c 1-4)" &&
1286+
test "$collide2" = "$collide3"
12741287
'
12751288

12761289
test_expect_success 'respect core.abbrev' '

0 commit comments

Comments
 (0)