rebase -i: remove patch file after conflict resolution

When a rebase stops for the user to resolve conflicts it writes a patch for the conflicting commit to .git/rebase-merge/patch. This file has been written since the introduction of "git-rebase-interactive.sh" in 1b1dce4bae7 (Teach rebase an interactive mode, 2007-06-25). I assume the idea was to enable the user inspect the conflicting commit in the same way as they could for the patch based rebase. This file should be deleted when the rebase continues as if the rebase stops for a failed "exec" command or a "break" command it is confusing to the user if there is a stale patch lying around from an unrelated command. As the path is now used in two different places rebase_path_patch() is added and used to obtain the path for the patch. To construct the path write_patch() previously used get_dir() which returns different paths depending on whether we're rebasing or cherry-picking/reverting. As this function is only called when rebasing it is safe to use a hard coded string for the directory instead. An assertion is added to make sure we don't starting calling this function when cherry-picking in the future. Signed-off-by: Phillip Wood <phillip.wood@dunelm.org.uk> Signed-off-by: Junio C Hamano <gitster@pobox.com>

Phillip Wood committed Sep 6, 2023 at 15:22 UTC 206a78d710853eaed97ab50ba336789e6b3499c7
2 files changed +30 -4
sequencer.c
+12 -4
@@ -132,6 +132,11 @@ static GIT_PATH_FUNC(rebase_path_amend, "rebase-merge/amend")
132 * the commit object name of the corresponding patch.
133 */
134 static GIT_PATH_FUNC(rebase_path_stopped_sha, "rebase-merge/stopped-sha")
135 +/*
136 + * When we stop for the user to resolve conflicts this file contains
137 + * the patch of the commit that is being picked.
138 + */
139 +static GIT_PATH_FUNC(rebase_path_patch, "rebase-merge/patch")
140 /*
141 * For the post-rewrite hook, we make a list of rewritten commits and
142 * their new sha1s. The rewritten-pending list keeps the sha1s of
@@ -3485,12 +3490,14 @@ static int make_patch(struct repository *r,
3490 char hex[GIT_MAX_HEXSZ + 1];
3491 int res = 0;
3492
3493 + if (!is_rebase_i(opts))
3494 + BUG("make_patch should only be called when rebasing");
3495 +
3496 oid_to_hex_r(hex, &commit->object.oid);
3497 if (write_message(hex, strlen(hex), rebase_path_stopped_sha(), 1) < 0)
3498 return -1;
3499 res |= write_rebase_head(&commit->object.oid);
3500
3493 - strbuf_addf(&buf, "%s/patch", get_dir(opts));
3501 memset(&log_tree_opt, 0, sizeof(log_tree_opt));
3502 repo_init_revisions(r, &log_tree_opt, NULL);
3503 log_tree_opt.abbrev = 0;
@@ -3498,15 +3505,15 @@ static int make_patch(struct repository *r,
3505 log_tree_opt.diffopt.output_format = DIFF_FORMAT_PATCH;
3506 log_tree_opt.disable_stdin = 1;
3507 log_tree_opt.no_commit_id = 1;
3501 - log_tree_opt.diffopt.file = fopen(buf.buf, "w");
3508 + log_tree_opt.diffopt.file = fopen(rebase_path_patch(), "w");
3509 log_tree_opt.diffopt.use_color = GIT_COLOR_NEVER;
3510 if (!log_tree_opt.diffopt.file)
3504 - res |= error_errno(_("could not open '%s'"), buf.buf);
3511 + res |= error_errno(_("could not open '%s'"),
3512 + rebase_path_patch());
3513 else {
3514 res |= log_tree_commit(&log_tree_opt, commit);
3515 fclose(log_tree_opt.diffopt.file);
3516 }
3509 - strbuf_reset(&buf);
3517
3518 strbuf_addf(&buf, "%s/message", get_dir(opts));
3519 if (!file_exists(buf.buf)) {
@@ -4642,6 +4649,7 @@ static int pick_commits(struct repository *r,
4649 unlink(rebase_path_message());
4650 unlink(rebase_path_stopped_sha());
4651 unlink(rebase_path_amend());
4652 + unlink(rebase_path_patch());
4653
4654 while (todo_list->current < todo_list->nr) {
4655 struct todo_item *item = todo_list->items + todo_list->current;
t/t3418-rebase-continue.sh
+18
@@ -244,6 +244,24 @@ test_expect_success 'the todo command "break" works' '
244 test_path_is_file execed
245 '
246
247 +test_expect_success 'patch file is removed before break command' '
248 + test_when_finished "git rebase --abort" &&
249 + cat >todo <<-\EOF &&
250 + pick commit-new-file-F2-on-topic-branch
251 + break
252 + EOF
253 +
254 + (
255 + set_replace_editor todo &&
256 + test_must_fail git rebase -i --onto commit-new-file-F2 HEAD
257 + ) &&
258 + test_path_is_file .git/rebase-merge/patch &&
259 + echo 22>F2 &&
260 + git add F2 &&
261 + git rebase --continue &&
262 + test_path_is_missing .git/rebase-merge/patch
263 +'
264 +
265 test_expect_success '--reschedule-failed-exec' '
266 test_when_finished "git rebase --abort" &&
267 test_must_fail git rebase -x false --reschedule-failed-exec HEAD^ &&