git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Oswald Buddenhagen <oswald.buddenhagen@gmx.de>
To: git@vger.kernel.org
Cc: Stephan Beyer <s-beyer@gmx.net>,
	Johannes Schindelin <johannes.schindelin@gmx.de>,
	Phillip Wood <phillip.wood123@gmail.com>
Subject: [PATCH] sequencer: update abort safety file more sparingly
Date: Sun,  3 Sep 2023 17:11:32 +0200	[thread overview]
Message-ID: <20230903151132.739166-1-oswald.buddenhagen@gmx.de> (raw)

The only situation where the file's content matters is --continue'ing
(after a multi-cherry-pick merge conflict). This means that it is
sufficient to write it in a single place, when we are prematurely
exiting the main workhorse. This is much easier to reason about than the
three dispersed calls originally introduced in 1e41229d ("sequencer:
make sequencer abort safer", 2016-12-07). We now can also remove the
inefficient file-based check whether the file needs writing, which
wasn't even reliable: a single pick executed during an interrupted
sequence would bypass the safety.

Signed-off-by: Oswald Buddenhagen <oswald.buddenhagen@gmx.de>

---
Cc: Stephan Beyer <s-beyer@gmx.net>
Cc: Johannes Schindelin <johannes.schindelin@gmx.de>
Cc: Phillip Wood <phillip.wood123@gmail.com>
---
 sequencer.c                     | 9 ++-------
 t/t3510-cherry-pick-sequence.sh | 9 +++++++++
 2 files changed, 11 insertions(+), 7 deletions(-)

diff --git a/sequencer.c b/sequencer.c
index a66dcf8ab2..716384cc7b 100644
--- a/sequencer.c
+++ b/sequencer.c
@@ -575,10 +575,6 @@ static void update_abort_safety_file(void)
 {
 	struct object_id head;
 
-	/* Do nothing on a single-pick */
-	if (!file_exists(git_path_seq_dir()))
-		return;
-
 	if (!repo_get_oid(the_repository, "HEAD", &head))
 		write_file(git_path_abort_safety_file(), "%s", oid_to_hex(&head));
 	else
@@ -618,7 +614,6 @@ static int fast_forward_to(struct repository *r,
 	strbuf_release(&sb);
 	strbuf_release(&err);
 	ref_transaction_free(transaction);
-	update_abort_safety_file();
 	return 0;
 }
 
@@ -2435,7 +2430,6 @@ static int do_pick_commit(struct repository *r,
 	free_message(commit, &msg);
 	free(author);
 	strbuf_release(&msgbuf);
-	update_abort_safety_file();
 
 	return res;
 }
@@ -5269,8 +5263,9 @@ int sequencer_pick_revisions(struct repository *r,
 		return -1;
 	if (save_opts(opts))
 		return -1;
-	update_abort_safety_file();
 	res = pick_commits(r, &todo_list, opts);
+	if (todo_list.current < todo_list.nr)
+		update_abort_safety_file();
 	todo_list_release(&todo_list);
 	return res;
 }
diff --git a/t/t3510-cherry-pick-sequence.sh b/t/t3510-cherry-pick-sequence.sh
index 3b0fa66c33..170b664c33 100755
--- a/t/t3510-cherry-pick-sequence.sh
+++ b/t/t3510-cherry-pick-sequence.sh
@@ -318,6 +318,15 @@ test_expect_success '--abort does not unsafely change HEAD' '
 	test_cmp_rev base HEAD
 '
 
+test_expect_success '--abort after single pick does not unsafely change HEAD' '
+	pristine_detach initial &&
+	test_must_fail git cherry-pick picked anotherpick &&
+	git reset --hard &&
+	git cherry-pick unrelatedpick &&
+	git cherry-pick --abort 2>actual &&
+	test_i18ngrep "You seem to have moved HEAD" actual
+'
+
 test_expect_success 'cherry-pick --abort to cancel multiple revert' '
 	pristine_detach anotherpick &&
 	test_expect_code 1 git revert base..picked &&
-- 
2.40.0.152.g15d061e6df


             reply	other threads:[~2023-09-03 15:11 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-09-03 15:11 Oswald Buddenhagen [this message]
2023-09-03 18:40 ` [PATCH] sequencer: update abort safety file more sparingly Phillip Wood
2023-09-03 19:25   ` Oswald Buddenhagen
2023-09-03 19:48     ` Phillip Wood
2023-09-03 20:18       ` Oswald Buddenhagen
2023-09-04 10:05         ` Phillip Wood
2023-09-04 12:48           ` Oswald Buddenhagen

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=20230903151132.739166-1-oswald.buddenhagen@gmx.de \
    --to=oswald.buddenhagen@gmx.de \
    --cc=git@vger.kernel.org \
    --cc=johannes.schindelin@gmx.de \
    --cc=phillip.wood123@gmail.com \
    --cc=s-beyer@gmx.net \
    /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 NNTP newsgroup(s).