Re: [PATCH 5/8] rebase: preserve interactive todo file on checkout failure
- From
- Phillip Wood <phillip.wood123@gmail.com>
- Date
- Mar 23, 2023, 19:31 UTC
- Message-ID
- <47558c14-ba2c-18ec-0532-b21fdfd223f8@dunelm.org.uk>
- In-Reply-To
- <20230323162235.995574-6-oswald.buddenhagen@gmx.de>
Hi Oswald
On 23/03/2023 16:22, Oswald Buddenhagen wrote:
> Creating a suitable todo file is a potentially labor-intensive process, > so be less cavalier about discarding it when something goes wrong (e.g., > the user messed with the repo while editing the todo).
I was thinking about this problem the other day in the context of rescheduling commands when they cannot be executed because they would overwrite an untracked file. My thought was that we should prepend a "reset" command to the todo list so that the checkout happened when the user continued the rebase. How does this patch ensure the checkout happens when the user continues the rebase?
Best Wishes
Phillip
Show 67 quoted lines
> Signed-off-by: Oswald Buddenhagen <oswald.buddenhagen@gmx.de>
> ---
> builtin/rebase.c | 1 +
> sequencer.c | 4 ++++
> sequencer.h | 1 +
> t/t3404-rebase-interactive.sh | 3 ++-
> 4 files changed, 8 insertions(+), 1 deletion(-)
>
> diff --git a/builtin/rebase.c b/builtin/rebase.c
> index a309addd50..728c869db4 100644
> --- a/builtin/rebase.c
> +++ b/builtin/rebase.c
> @@ -153,6 +153,7 @@ static struct replay_opts get_replay_opts(const struct rebase_options *opts)
> replay.keep_redundant_commits = (opts->empty == EMPTY_KEEP);
> replay.quiet = !(opts->flags & REBASE_NO_QUIET);
> replay.verbose = opts->flags & REBASE_VERBOSE;
> + replay.precious_todo = opts->flags & REBASE_INTERACTIVE_EXPLICIT;
> replay.reschedule_failed_exec = opts->reschedule_failed_exec;
> replay.committer_date_is_author_date =
> opts->committer_date_is_author_date;
> diff --git a/sequencer.c b/sequencer.c
> index b1c29c8802..f8a7f4e721 100644
> --- a/sequencer.c
> +++ b/sequencer.c
> @@ -4570,6 +4570,10 @@ static int checkout_onto(struct repository *r, struct replay_opts *opts,
> .default_reflog_action = sequencer_reflog_action(opts)
> };
> if (reset_head(r, &ropts)) {
> + // Editing the todo may have been costly; don't just discard it.
> + if (opts->precious_todo)
> + exit(1); // Error was already printed
> +
> apply_autostash(rebase_path_autostash());
> sequencer_remove_state(opts);
> return error(_("could not detach HEAD"));
> diff --git a/sequencer.h b/sequencer.h
> index 1a3e616af2..a1b8ca6eb1 100644
> --- a/sequencer.h
> +++ b/sequencer.h
> @@ -47,6 +47,7 @@ struct replay_opts {
> int keep_redundant_commits;
> int verbose;
> int quiet;
> + int precious_todo;
> int reschedule_failed_exec;
> int committer_date_is_author_date;
> int ignore_date;
> diff --git a/t/t3404-rebase-interactive.sh b/t/t3404-rebase-interactive.sh
> index ff0afad63e..c625aad10a 100755
> --- a/t/t3404-rebase-interactive.sh
> +++ b/t/t3404-rebase-interactive.sh
> @@ -288,13 +288,14 @@ test_expect_success 'abort' '
> '
>
> test_expect_success 'abort with error when new base cannot be checked out' '
> + test_when_finished "git rebase --abort ||:" &&
> git rm --cached file1 &&
> git commit -m "remove file in base" &&
> test_must_fail git rebase -i primary > output 2>&1 &&
> test_i18ngrep "The following untracked working tree files would be overwritten by checkout:" \
> output &&
> test_i18ngrep "file1" output &&
> - test_path_is_missing .git/rebase-merge &&
> + test_path_is_dir .git/rebase-merge &&
> rm file1 &&
> git reset --hard HEAD^
> '