Git Mailing List Archive mirror
 help / color / mirror / Atom feed
From: Phillip Wood <phillip.wood123@gmail.com>
To: Oswald Buddenhagen <oswald.buddenhagen@gmx.de>, git@vger.kernel.org
Subject: Re: [PATCH 5/8] rebase: preserve interactive todo file on checkout failure
Date: Thu, 23 Mar 2023 19:31:04 +0000	[thread overview]
Message-ID: <47558c14-ba2c-18ec-0532-b21fdfd223f8@dunelm.org.uk> (raw)
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

> 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^
>   '

  reply	other threads:[~2023-03-23 19:31 UTC|newest]

Thread overview: 49+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-03-23 16:22 [PATCH 0/8] sequencer refactoring Oswald Buddenhagen
2023-03-23 16:22 ` [PATCH 1/8] rebase: simplify code related to imply_merge() Oswald Buddenhagen
2023-03-23 19:40   ` Phillip Wood
2023-03-23 20:00     ` Junio C Hamano
2023-03-23 21:08       ` Felipe Contreras
2023-08-09 17:15       ` [PATCH v2 0/3] rebase refactoring Oswald Buddenhagen
2023-08-09 17:15         ` [PATCH v2 1/3] rebase: simplify code related to imply_merge() Oswald Buddenhagen
2023-08-09 17:15         ` [PATCH v2 2/3] rebase: handle --strategy via imply_merge() as well Oswald Buddenhagen
2023-08-09 17:15         ` [PATCH v2 3/3] rebase: move parse_opt_keep_empty() down Oswald Buddenhagen
2023-08-15 14:01           ` Phillip Wood
2023-10-20  9:36         ` [PATCH v3 0/3] rebase refactoring Oswald Buddenhagen
2023-10-20  9:36           ` [PATCH v3 1/3] rebase: simplify code related to imply_merge() Oswald Buddenhagen
2023-10-20  9:36           ` [PATCH v3 2/3] rebase: handle --strategy via imply_merge() as well Oswald Buddenhagen
2023-10-20 21:51             ` Junio C Hamano
2023-10-20  9:36           ` [PATCH v3 3/3] rebase: move parse_opt_keep_empty() down Oswald Buddenhagen
2023-10-20 22:07           ` [PATCH v3 0/3] rebase refactoring Junio C Hamano
2023-10-23 15:43             ` Phillip Wood
2023-10-23 19:02               ` Junio C Hamano
2023-03-23 16:22 ` [PATCH 2/8] rebase: move parse_opt_keep_empty() down Oswald Buddenhagen
2023-03-23 19:39   ` Phillip Wood
2023-03-23 16:22 ` [PATCH 3/8] sequencer: pass around rebase action explicitly Oswald Buddenhagen
2023-03-23 19:27   ` Phillip Wood
2023-03-23 21:27     ` Oswald Buddenhagen
2023-03-23 16:22 ` [PATCH 4/8] sequencer: create enum for edit_todo_list() return value Oswald Buddenhagen
2023-03-23 19:27   ` Phillip Wood
2023-03-23 16:22 ` [PATCH 5/8] rebase: preserve interactive todo file on checkout failure Oswald Buddenhagen
2023-03-23 19:31   ` Phillip Wood [this message]
2023-03-23 22:38     ` Oswald Buddenhagen
2023-03-24 14:15       ` Phillip Wood
2023-03-24 14:42         ` Oswald Buddenhagen
2023-03-23 20:16   ` Junio C Hamano
2023-03-23 23:23     ` Oswald Buddenhagen
2023-03-24  4:31       ` Junio C Hamano
2023-03-23 16:22 ` [PATCH 6/8] sequencer: simplify allocation of result array in todo_list_rearrange_squash() Oswald Buddenhagen
2023-03-23 19:46   ` Phillip Wood
2023-03-23 22:13     ` Oswald Buddenhagen
2023-03-23 16:22 ` [PATCH 7/8] sequencer: pass `onto` to complete_action() as object-id Oswald Buddenhagen
2023-03-23 19:34   ` Phillip Wood
2023-03-23 21:36     ` Oswald Buddenhagen
2023-03-24 14:18       ` Phillip Wood
2023-03-23 16:22 ` [PATCH 8/8] rebase: improve resumption from incorrect initial todo list Oswald Buddenhagen
2023-03-26 14:28   ` Phillip Wood
2023-04-26 15:34     ` Oswald Buddenhagen
2023-05-17 12:13       ` Phillip Wood
2023-08-24 16:46         ` Oswald Buddenhagen
2023-03-23 19:38 ` [PATCH 0/8] sequencer refactoring Phillip Wood
2023-03-25 11:08 ` Phillip Wood
2023-04-06 12:09   ` Phillip Wood
2023-05-17 13:10 ` Phillip Wood

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=47558c14-ba2c-18ec-0532-b21fdfd223f8@dunelm.org.uk \
    --to=phillip.wood123@gmail.com \
    --cc=git@vger.kernel.org \
    --cc=oswald.buddenhagen@gmx.de \
    --cc=phillip.wood@dunelm.org.uk \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).