@matheusbsbs / matheus-bagliano / commits / d559f502c5

rebase --abort/--quit: cleanup refs/rewritten

When `rebase -r` finishes it removes any refs under refs/rewritten that it has created. However if the user aborts or quits the rebase refs are not removed. This can cause problems for future rebases. For example I recently wanted to merge a updated version of a topic branch into an integration branch so ran `rebase -ir` and removed the picks and label for the topic branch from the todo list so that merge -C <old-merge> topic would pick up the new version of topic. Unfortunately refs/rewritten/topic already existed from a previous rebase that had been aborted so the rebase just used the old topic, not the new one. The logic for the non-interactive quit case is changed to ensure `buf` is always freed. Signed-off-by: Phillip Wood <phillip.wood@dunelm.org.uk> Signed-off-by: Junio C Hamano <gitster@pobox.com>

Phillip Wood committed May 14, 2019 at 19:03 UTC d559f502c59658ea1f360fd28d8909894e40a4fe
2 files changed +42 -10
builtin/rebase.c
+25 -9
@@ -770,10 +770,18 @@ static int finish_rebase(struct rebase_options *opts)
770 * user should see them.
771 */
772 run_command_v_opt(argv_gc_auto, RUN_GIT_CMD);
773 - strbuf_addstr(&dir, opts->state_dir);
774 - if (remove_dir_recursively(&dir, 0))
775 - ret = error(_("could not remove '%s'"), opts->state_dir);
776 - strbuf_release(&dir);
773 + if (opts->type == REBASE_INTERACTIVE) {
774 + struct replay_opts replay = REPLAY_OPTS_INIT;
775 +
776 + replay.action = REPLAY_INTERACTIVE_REBASE;
777 + ret = sequencer_remove_state(&replay);
778 + } else {
779 + strbuf_addstr(&dir, opts->state_dir);
780 + if (remove_dir_recursively(&dir, 0))
781 + ret = error(_("could not remove '%s'"),
782 + opts->state_dir);
783 + strbuf_release(&dir);
784 + }
785
786 return ret;
787 }
@@ -1651,11 +1659,19 @@ int cmd_rebase(int argc, const char **argv, const char *prefix)
1659 goto cleanup;
1660 }
1661 case ACTION_QUIT: {
1654 - strbuf_reset(&buf);
1655 - strbuf_addstr(&buf, options.state_dir);
1656 - ret = !!remove_dir_recursively(&buf, 0);
1657 - if (ret)
1658 - die(_("could not remove '%s'"), options.state_dir);
1662 + if (options.type == REBASE_INTERACTIVE) {
1663 + struct replay_opts replay = REPLAY_OPTS_INIT;
1664 +
1665 + replay.action = REPLAY_INTERACTIVE_REBASE;
1666 + ret = !!sequencer_remove_state(&replay);
1667 + } else {
1668 + strbuf_reset(&buf);
1669 + strbuf_addstr(&buf, options.state_dir);
1670 + ret = !!remove_dir_recursively(&buf, 0);
1671 + if (ret)
1672 + error(_("could not remove '%s'"),
1673 + options.state_dir);
1674 + }
1675 goto cleanup;
1676 }
1677 case ACTION_EDIT_TODO:
t/t3430-rebase-merges.sh
+17 -1
@@ -224,8 +224,24 @@ test_expect_success 'refs/rewritten/* is worktree-local' '
224 test_cmp_rev HEAD "$(cat wt/b)"
225 '
226
227 +test_expect_success '--abort cleans up refs/rewritten' '
228 + git checkout -b abort-cleans-refs-rewritten H &&
229 + GIT_SEQUENCE_EDITOR="echo break >>" git rebase -ir @^ &&
230 + git rev-parse --verify refs/rewritten/onto &&
231 + git rebase --abort &&
232 + test_must_fail git rev-parse --verify refs/rewritten/onto
233 +'
234 +
235 +test_expect_success '--quit cleans up refs/rewritten' '
236 + git checkout -b quit-cleans-refs-rewritten H &&
237 + GIT_SEQUENCE_EDITOR="echo break >>" git rebase -ir @^ &&
238 + git rev-parse --verify refs/rewritten/onto &&
239 + git rebase --quit &&
240 + test_must_fail git rev-parse --verify refs/rewritten/onto
241 +'
242 +
243 test_expect_success 'post-rewrite hook and fixups work for merges' '
228 - git checkout -b post-rewrite &&
244 + git checkout -b post-rewrite H &&
245 test_commit same1 &&
246 git reset --hard HEAD^ &&
247 test_commit same2 &&