mirrors/git
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

Merge branch 'pw/rebase-i-after-failure'

Browse Source 
Various fixes to the behaviour of "rebase -i" when the command got
interrupted by conflicting changes.

* pw/rebase-i-after-failure:
  rebase -i: fix adding failed command to the todo list
  rebase --continue: refuse to commit after failed command
  rebase: fix rewritten list for failed pick
  sequencer: factor out part of pick_commits()
  sequencer: use rebase_path_message()
  rebase -i: remove patch file after conflict resolution
  rebase -i: move unlink() calls
This commit is contained in:
Junio C Hamano
2023-09-14 11:17:00 -07:00
parent f73604fabf 203573b024
commit b995e78147
5 changed files with 227 additions and 102 deletions
Download Patch File Download Diff File Expand all files Collapse all files

184
sequencer.c
View File

@ -147,6 +147,11 @@ static GIT_PATH_FUNC(rebase_path_amend, "rebase-merge/amend")
* the commit object name of the corresponding patch. * the commit object name of the corresponding patch.
*/ */
static GIT_PATH_FUNC(rebase_path_stopped_sha, "rebase-merge/stopped-sha") static GIT_PATH_FUNC(rebase_path_stopped_sha, "rebase-merge/stopped-sha")
/*
* When we stop for the user to resolve conflicts this file contains
* the patch of the commit that is being picked.
*/
static GIT_PATH_FUNC(rebase_path_patch, "rebase-merge/patch")
/* /*
* For the post-rewrite hook, we make a list of rewritten commits and * For the post-rewrite hook, we make a list of rewritten commits and
* their new sha1s. The rewritten-pending list keeps the sha1s of * their new sha1s. The rewritten-pending list keeps the sha1s of
@ -3412,7 +3417,8 @@ give_advice:
return -1; return -1;
} }
static int save_todo(struct todo_list *todo_list, struct replay_opts *opts) static int save_todo(struct todo_list *todo_list, struct replay_opts *opts,
int reschedule)
{ {
struct lock_file todo_lock = LOCK_INIT; struct lock_file todo_lock = LOCK_INIT;
const char *todo_path = get_todo_path(opts); const char *todo_path = get_todo_path(opts);
@ -3422,7 +3428,7 @@ static int save_todo(struct todo_list *todo_list, struct replay_opts *opts)
* rebase -i writes "git-rebase-todo" without the currently executing * rebase -i writes "git-rebase-todo" without the currently executing
* command, appending it to "done" instead. * command, appending it to "done" instead.
*/ */
if (is_rebase_i(opts)) if (is_rebase_i(opts) && !reschedule)
next++; next++;
fd = hold_lock_file_for_update(&todo_lock, todo_path, 0); fd = hold_lock_file_for_update(&todo_lock, todo_path, 0);
@ -3435,7 +3441,7 @@ static int save_todo(struct todo_list *todo_list, struct replay_opts *opts)
if (commit_lock_file(&todo_lock) < 0) if (commit_lock_file(&todo_lock) < 0)
return error(_("failed to finalize '%s'"), todo_path); return error(_("failed to finalize '%s'"), todo_path);
if (is_rebase_i(opts) && next > 0) { if (is_rebase_i(opts) && !reschedule && next > 0) {
const char *done = rebase_path_done(); const char *done = rebase_path_done();
int fd = open(done, O_CREAT | O_WRONLY | O_APPEND, 0666); int fd = open(done, O_CREAT | O_WRONLY | O_APPEND, 0666);
int ret = 0; int ret = 0;
@ -3516,18 +3522,19 @@ static int make_patch(struct repository *r,
struct commit *commit, struct commit *commit,
struct replay_opts *opts) struct replay_opts *opts)
{ {
struct strbuf buf = STRBUF_INIT;
struct rev_info log_tree_opt; struct rev_info log_tree_opt;
const char *subject; const char *subject;
char hex[GIT_MAX_HEXSZ + 1]; char hex[GIT_MAX_HEXSZ + 1];
int res = 0; int res = 0;
if (!is_rebase_i(opts))
BUG("make_patch should only be called when rebasing");
oid_to_hex_r(hex, &commit->object.oid); oid_to_hex_r(hex, &commit->object.oid);
if (write_message(hex, strlen(hex), rebase_path_stopped_sha(), 1) < 0) if (write_message(hex, strlen(hex), rebase_path_stopped_sha(), 1) < 0)
return -1; return -1;
res |= write_rebase_head(&commit->object.oid); res |= write_rebase_head(&commit->object.oid);
strbuf_addf(&buf, "%s/patch", get_dir(opts));
memset(&log_tree_opt, 0, sizeof(log_tree_opt)); memset(&log_tree_opt, 0, sizeof(log_tree_opt));
repo_init_revisions(r, &log_tree_opt, NULL); repo_init_revisions(r, &log_tree_opt, NULL);
log_tree_opt.abbrev = 0; log_tree_opt.abbrev = 0;
@ -3535,28 +3542,26 @@ static int make_patch(struct repository *r,
log_tree_opt.diffopt.output_format = DIFF_FORMAT_PATCH; log_tree_opt.diffopt.output_format = DIFF_FORMAT_PATCH;
log_tree_opt.disable_stdin = 1; log_tree_opt.disable_stdin = 1;
log_tree_opt.no_commit_id = 1; log_tree_opt.no_commit_id = 1;
log_tree_opt.diffopt.file = fopen(buf.buf, "w"); log_tree_opt.diffopt.file = fopen(rebase_path_patch(), "w");
log_tree_opt.diffopt.use_color = GIT_COLOR_NEVER; log_tree_opt.diffopt.use_color = GIT_COLOR_NEVER;
if (!log_tree_opt.diffopt.file) if (!log_tree_opt.diffopt.file)
res |= error_errno(_("could not open '%s'"), buf.buf); res |= error_errno(_("could not open '%s'"),
rebase_path_patch());
else { else {
res |= log_tree_commit(&log_tree_opt, commit); res |= log_tree_commit(&log_tree_opt, commit);
fclose(log_tree_opt.diffopt.file); fclose(log_tree_opt.diffopt.file);
} }
strbuf_reset(&buf);
strbuf_addf(&buf, "%s/message", get_dir(opts)); if (!file_exists(rebase_path_message())) {
if (!file_exists(buf.buf)) {
const char *encoding = get_commit_output_encoding(); const char *encoding = get_commit_output_encoding();
const char *commit_buffer = repo_logmsg_reencode(r, const char *commit_buffer = repo_logmsg_reencode(r,
commit, NULL, commit, NULL,
encoding); encoding);
find_commit_subject(commit_buffer, &subject); find_commit_subject(commit_buffer, &subject);
res |= write_message(subject, strlen(subject), buf.buf, 1); res |= write_message(subject, strlen(subject), rebase_path_message(), 1);
repo_unuse_commit_buffer(r, commit, repo_unuse_commit_buffer(r, commit,
commit_buffer); commit_buffer);
} }
strbuf_release(&buf);
release_revisions(&log_tree_opt); release_revisions(&log_tree_opt);
return res; return res;
@ -4174,6 +4179,7 @@ static int do_merge(struct repository *r,
if (ret < 0) { if (ret < 0) {
error(_("could not even attempt to merge '%.*s'"), error(_("could not even attempt to merge '%.*s'"),
merge_arg_len, arg); merge_arg_len, arg);
unlink(git_path_merge_msg(r));
goto leave_merge; goto leave_merge;
} }
/* /*
@ -4661,75 +4667,23 @@ N_("Could not execute the todo command\n"
" git rebase --edit-todo\n" " git rebase --edit-todo\n"
" git rebase --continue\n"); " git rebase --continue\n");
static int pick_commits(struct repository *r, static int pick_one_commit(struct repository *r,
struct todo_list *todo_list, struct todo_list *todo_list,
struct replay_opts *opts) struct replay_opts *opts,
int *check_todo, int* reschedule)
{ {
int res = 0, reschedule = 0; int res;
opts->reflog_message = sequencer_reflog_action(opts);
if (opts->allow_ff)
assert(!(opts->signoff || opts->no_commit ||
opts->record_origin || should_edit(opts) ||
opts->committer_date_is_author_date ||
opts->ignore_date));
if (read_and_refresh_cache(r, opts))
return -1;
while (todo_list->current < todo_list->nr) {
struct todo_item *item = todo_list->items + todo_list->current; struct todo_item *item = todo_list->items + todo_list->current;
const char *arg = todo_item_get_arg(todo_list, item); const char *arg = todo_item_get_arg(todo_list, item);
int check_todo = 0;
if (save_todo(todo_list, opts))
return -1;
if (is_rebase_i(opts)) {
if (item->command != TODO_COMMENT) {
FILE *f = fopen(rebase_path_msgnum(), "w");
todo_list->done_nr++;
if (f) {
fprintf(f, "%d\n", todo_list->done_nr);
fclose(f);
}
if (!opts->quiet)
fprintf(stderr, _("Rebasing (%d/%d)%s"),
todo_list->done_nr,
todo_list->total_nr,
opts->verbose ? "\n" : "\r");
}
unlink(rebase_path_message());
unlink(rebase_path_author_script());
unlink(rebase_path_stopped_sha());
unlink(rebase_path_amend());
unlink(git_path_merge_head(r));
unlink(git_path_auto_merge(r));
delete_ref(NULL, "REBASE_HEAD", NULL, REF_NO_DEREF);
if (item->command == TODO_BREAK) {
if (!opts->verbose)
term_clear_line();
return stopped_at_head(r);
}
}
if (item->command <= TODO_SQUASH) {
if (is_rebase_i(opts)) if (is_rebase_i(opts))
opts->reflog_message = reflog_message(opts, opts->reflog_message = reflog_message(
command_to_string(item->command), NULL); opts, command_to_string(item->command), NULL);
res = do_pick_commit(r, item, opts, res = do_pick_commit(r, item, opts, is_final_fixup(todo_list),
is_final_fixup(todo_list), check_todo);
&check_todo);
if (is_rebase_i(opts) && res < 0) { if (is_rebase_i(opts) && res < 0) {
/* Reschedule */ /* Reschedule */
advise(_(rescheduled_advice), *reschedule = 1;
get_item_line_length(todo_list,
todo_list->current),
get_item_line(todo_list,
todo_list->current));
todo_list->current--;
if (save_todo(todo_list, opts))
return -1; return -1;
} }
if (item->command == TODO_EDIT) { if (item->command == TODO_EDIT) {
@ -4737,10 +4691,8 @@ static int pick_commits(struct repository *r,
if (!res) { if (!res) {
if (!opts->verbose) if (!opts->verbose)
term_clear_line(); term_clear_line();
fprintf(stderr, fprintf(stderr, _("Stopped at %s... %.*s\n"),
_("Stopped at %s... %.*s\n"), short_commit_name(r, commit), item->arg_len, arg);
short_commit_name(r, commit),
item->arg_len, arg);
} }
return error_with_patch(r, commit, return error_with_patch(r, commit,
arg, item->arg_len, opts, res, !res); arg, item->arg_len, opts, res, !res);
@ -4774,6 +4726,68 @@ static int pick_commits(struct repository *r,
arg, item->arg_len, opts, arg, item->arg_len, opts,
res, to_amend); res, to_amend);
} }
return res;
}
static int pick_commits(struct repository *r,
struct todo_list *todo_list,
struct replay_opts *opts)
{
int res = 0, reschedule = 0;
opts->reflog_message = sequencer_reflog_action(opts);
if (opts->allow_ff)
assert(!(opts->signoff || opts->no_commit ||
opts->record_origin || should_edit(opts) ||
opts->committer_date_is_author_date ||
opts->ignore_date));
if (read_and_refresh_cache(r, opts))
return -1;
unlink(rebase_path_message());
unlink(rebase_path_stopped_sha());
unlink(rebase_path_amend());
unlink(rebase_path_patch());
while (todo_list->current < todo_list->nr) {
struct todo_item *item = todo_list->items + todo_list->current;
const char *arg = todo_item_get_arg(todo_list, item);
int check_todo = 0;
if (save_todo(todo_list, opts, reschedule))
return -1;
if (is_rebase_i(opts)) {
if (item->command != TODO_COMMENT) {
FILE *f = fopen(rebase_path_msgnum(), "w");
todo_list->done_nr++;
if (f) {
fprintf(f, "%d\n", todo_list->done_nr);
fclose(f);
}
if (!opts->quiet)
fprintf(stderr, _("Rebasing (%d/%d)%s"),
todo_list->done_nr,
todo_list->total_nr,
opts->verbose ? "\n" : "\r");
}
unlink(rebase_path_author_script());
unlink(git_path_merge_head(r));
unlink(git_path_auto_merge(r));
delete_ref(NULL, "REBASE_HEAD", NULL, REF_NO_DEREF);
if (item->command == TODO_BREAK) {
if (!opts->verbose)
term_clear_line();
return stopped_at_head(r);
}
}
if (item->command <= TODO_SQUASH) {
res = pick_one_commit(r, todo_list, opts, &check_todo,
&reschedule);
if (!res && item->command == TODO_EDIT)
return 0;
} else if (item->command == TODO_EXEC) { } else if (item->command == TODO_EXEC) {
char *end_of_arg = (char *)(arg + item->arg_len); char *end_of_arg = (char *)(arg + item->arg_len);
int saved = *end_of_arg; int saved = *end_of_arg;
@ -4821,22 +4835,19 @@ static int pick_commits(struct repository *r,
get_item_line_length(todo_list, get_item_line_length(todo_list,
todo_list->current), todo_list->current),
get_item_line(todo_list, todo_list->current)); get_item_line(todo_list, todo_list->current));
todo_list->current--; if (save_todo(todo_list, opts, reschedule))
if (save_todo(todo_list, opts))
return -1; return -1;
if (item->commit) if (item->commit)
return error_with_patch(r, write_rebase_head(&item->commit->object.oid);
item->commit,
arg, item->arg_len,
opts, res, 0);
} else if (is_rebase_i(opts) && check_todo && !res && } else if (is_rebase_i(opts) && check_todo && !res &&
reread_todo_if_changed(r, todo_list, opts)) { reread_todo_if_changed(r, todo_list, opts)) {
return -1; return -1;
} }
todo_list->current++;
if (res) if (res)
return res; return res;
todo_list->current++;
} }
if (is_rebase_i(opts)) { if (is_rebase_i(opts)) {
@ -4989,6 +5000,11 @@ static int commit_staged_changes(struct repository *r,
is_clean = !has_uncommitted_changes(r, 0); is_clean = !has_uncommitted_changes(r, 0);
if (!is_clean && !file_exists(rebase_path_message())) {
const char *gpg_opt = gpg_sign_opt_quoted(opts);
return error(_(staged_changes_advice), gpg_opt, gpg_opt);
}
if (file_exists(rebase_path_amend())) { if (file_exists(rebase_path_amend())) {
struct strbuf rev = STRBUF_INIT; struct strbuf rev = STRBUF_INIT;
struct object_id head, to_amend; struct object_id head, to_amend;

51
t/t3404-rebase-interactive.sh
View File

@ -604,7 +604,8 @@ test_expect_success 'clean error after failed "exec"' '
echo "edited again" > file7 && echo "edited again" > file7 &&
git add file7 && git add file7 &&
test_must_fail git rebase --continue 2>error && test_must_fail git rebase --continue 2>error &&
test_i18ngrep "you have staged changes in your working tree" error test_i18ngrep "you have staged changes in your working tree" error &&
test_i18ngrep ! "could not open.*for reading" error
' '
test_expect_success 'rebase a detached HEAD' ' test_expect_success 'rebase a detached HEAD' '
@ -1276,20 +1277,34 @@ test_expect_success 'todo count' '
' '
test_expect_success 'rebase -i commits that overwrite untracked files (pick)' ' test_expect_success 'rebase -i commits that overwrite untracked files (pick)' '
git checkout --force branch2 && git checkout --force A &&
git clean -f && git clean -f &&
cat >todo <<-EOF &&
exec >file2
pick $(git rev-parse B) B
pick $(git rev-parse C) C
pick $(git rev-parse D) D
exec cat .git/rebase-merge/done >actual
EOF
( (
set_fake_editor && set_replace_editor todo &&
FAKE_LINES="edit 1 2" git rebase -i A test_must_fail git rebase -i A
) && ) &&
test_cmp_rev HEAD F && test_cmp_rev HEAD B &&
test_path_is_missing file6 && test_cmp_rev REBASE_HEAD C &&
>file6 && head -n3 todo >expect &&
test_must_fail git rebase --continue && test_cmp expect .git/rebase-merge/done &&
test_cmp_rev HEAD F && rm file2 &&
rm file6 && test_path_is_missing .git/rebase-merge/patch &&
echo changed >file1 &&
git add file1 &&
test_must_fail git rebase --continue 2>err &&
grep "error: you have staged changes in your working tree" err &&
git reset --hard HEAD &&
git rebase --continue && git rebase --continue &&
test_cmp_rev HEAD I test_cmp_rev HEAD D &&
tail -n3 todo >>expect &&
test_cmp expect actual
' '
test_expect_success 'rebase -i commits that overwrite untracked files (squash)' ' test_expect_success 'rebase -i commits that overwrite untracked files (squash)' '
@ -1305,7 +1320,14 @@ test_expect_success 'rebase -i commits that overwrite untracked files (squash)'
>file6 && >file6 &&
test_must_fail git rebase --continue && test_must_fail git rebase --continue &&
test_cmp_rev HEAD F && test_cmp_rev HEAD F &&
test_cmp_rev REBASE_HEAD I &&
rm file6 && rm file6 &&
test_path_is_missing .git/rebase-merge/patch &&
echo changed >file1 &&
git add file1 &&
test_must_fail git rebase --continue 2>err &&
grep "error: you have staged changes in your working tree" err &&
git reset --hard HEAD &&
git rebase --continue && git rebase --continue &&
test $(git cat-file commit HEAD | sed -ne \$p) = I && test $(git cat-file commit HEAD | sed -ne \$p) = I &&
git reset --hard original-branch2 git reset --hard original-branch2
@ -1323,7 +1345,14 @@ test_expect_success 'rebase -i commits that overwrite untracked files (no ff)' '
>file6 && >file6 &&
test_must_fail git rebase --continue && test_must_fail git rebase --continue &&
test $(git cat-file commit HEAD | sed -ne \$p) = F && test $(git cat-file commit HEAD | sed -ne \$p) = F &&
test_cmp_rev REBASE_HEAD I &&
rm file6 && rm file6 &&
test_path_is_missing .git/rebase-merge/patch &&
echo changed >file1 &&
git add file1 &&
test_must_fail git rebase --continue 2>err &&
grep "error: you have staged changes in your working tree" err &&
git reset --hard HEAD &&
git rebase --continue && git rebase --continue &&
test $(git cat-file commit HEAD | sed -ne \$p) = I test $(git cat-file commit HEAD | sed -ne \$p) = I
' '

18
t/t3418-rebase-continue.sh
View File

@ -268,6 +268,24 @@ test_expect_success 'the todo command "break" works' '
test_path_is_file execed test_path_is_file execed
' '
test_expect_success 'patch file is removed before break command' '
test_when_finished "git rebase --abort" &&
cat >todo <<-\EOF &&
pick commit-new-file-F2-on-topic-branch
break
EOF
(
set_replace_editor todo &&
test_must_fail git rebase -i --onto commit-new-file-F2 HEAD
) &&
test_path_is_file .git/rebase-merge/patch &&
echo 22>F2 &&
git add F2 &&
git rebase --continue &&
test_path_is_missing .git/rebase-merge/patch
'
test_expect_success '--reschedule-failed-exec' ' test_expect_success '--reschedule-failed-exec' '
test_when_finished "git rebase --abort" && test_when_finished "git rebase --abort" &&
test_must_fail git rebase -x false --reschedule-failed-exec HEAD^ && test_must_fail git rebase -x false --reschedule-failed-exec HEAD^ &&

30
t/t3430-rebase-merges.sh
View File

@ -128,14 +128,24 @@ test_expect_success 'generate correct todo list' '
' '
test_expect_success '`reset` refuses to overwrite untracked files' ' test_expect_success '`reset` refuses to overwrite untracked files' '
git checkout -b refuse-to-reset && git checkout B &&
test_commit dont-overwrite-untracked && test_commit dont-overwrite-untracked &&
git checkout @{-1} && cat >script-from-scratch <<-EOF &&
: >dont-overwrite-untracked.t && exec >dont-overwrite-untracked.t
echo "reset refs/tags/dont-overwrite-untracked" >script-from-scratch && pick $(git rev-parse B) B
reset refs/tags/dont-overwrite-untracked
pick $(git rev-parse C) C
exec cat .git/rebase-merge/done >actual
EOF
test_config sequence.editor \""$PWD"/replace-editor.sh\" && test_config sequence.editor \""$PWD"/replace-editor.sh\" &&
test_must_fail git rebase -ir HEAD && test_must_fail git rebase -ir A &&
git rebase --abort test_cmp_rev HEAD B &&
head -n3 script-from-scratch >expect &&
test_cmp expect .git/rebase-merge/done &&
rm dont-overwrite-untracked.t &&
git rebase --continue &&
tail -n3 script-from-scratch >>expect &&
test_cmp expect actual
' '
test_expect_success '`reset` rejects trees' ' test_expect_success '`reset` rejects trees' '
@ -165,12 +175,16 @@ test_expect_success 'failed `merge -C` writes patch (may be rescheduled, too)' '
test_config sequence.editor \""$PWD"/replace-editor.sh\" && test_config sequence.editor \""$PWD"/replace-editor.sh\" &&
test_tick && test_tick &&
test_must_fail git rebase -ir HEAD && test_must_fail git rebase -ir HEAD &&
test_cmp_rev REBASE_HEAD H^0 &&
grep "^merge -C .* G$" .git/rebase-merge/done && grep "^merge -C .* G$" .git/rebase-merge/done &&
grep "^merge -C .* G$" .git/rebase-merge/git-rebase-todo && grep "^merge -C .* G$" .git/rebase-merge/git-rebase-todo &&
test_path_is_file .git/rebase-merge/patch && test_path_is_missing .git/rebase-merge/patch &&
echo changed >file1 &&
git add file1 &&
test_must_fail git rebase --continue 2>err &&
grep "error: you have staged changes in your working tree" err &&
: fail because of merge conflict && : fail because of merge conflict &&
rm G.t .git/rebase-merge/patch &&
git reset --hard conflicting-G && git reset --hard conflicting-G &&
test_must_fail git rebase --continue && test_must_fail git rebase --continue &&
! grep "^merge -C .* G$" .git/rebase-merge/git-rebase-todo && ! grep "^merge -C .* G$" .git/rebase-merge/git-rebase-todo &&

48
t/t5407-post-rewrite-hook.sh
View File

@ -17,6 +17,12 @@ test_expect_success 'setup' '
git checkout A^0 && git checkout A^0 &&
test_commit E bar E && test_commit E bar E &&
test_commit F foo F && test_commit F foo F &&
git checkout B &&
git merge E &&
git tag merge-E &&
test_commit G G &&
test_commit H H &&
test_commit I I &&
git checkout main && git checkout main &&
test_hook --setup post-rewrite <<-EOF test_hook --setup post-rewrite <<-EOF
@ -173,6 +179,48 @@ test_fail_interactive_rebase () {
) )
} }
test_expect_success 'git rebase with failed pick' '
clear_hook_input &&
cat >todo <<-\EOF &&
exec >bar
merge -C merge-E E
exec >G
pick G
exec >H 2>I
pick H
fixup I
EOF
(
set_replace_editor todo &&
test_must_fail git rebase -i D D 2>err
) &&
grep "would be overwritten" err &&
rm bar &&
test_must_fail git rebase --continue 2>err &&
grep "would be overwritten" err &&
rm G &&
test_must_fail git rebase --continue 2>err &&
grep "would be overwritten" err &&
rm H &&
test_must_fail git rebase --continue 2>err &&
grep "would be overwritten" err &&
rm I &&
git rebase --continue &&
echo rebase >expected.args &&
cat >expected.data <<-EOF &&
$(git rev-parse merge-E) $(git rev-parse HEAD~2)
$(git rev-parse G) $(git rev-parse HEAD~1)
$(git rev-parse H) $(git rev-parse HEAD)
$(git rev-parse I) $(git rev-parse HEAD)
EOF
verify_hook_input
'
test_expect_success 'git rebase -i (unchanged)' ' test_expect_success 'git rebase -i (unchanged)' '
git reset --hard D && git reset --hard D &&
clear_hook_input && clear_hook_input &&