Skip to content

Commit 7902b72

Browse files
committed
Merge branch 'sb/sequencer-abort-safety' into maint
Unlike "git am --abort", "git cherry-pick --abort" moved HEAD back to where cherry-pick started while picking multiple changes, when the cherry-pick stopped to ask for help from the user, and the user did "git reset --hard" to a different commit in order to re-attempt the operation. * sb/sequencer-abort-safety: Revert "sequencer: remove useless get_dir() function" sequencer: remove useless get_dir() function sequencer: make sequencer abort safer t3510: test that cherry-pick --abort does not unsafely change HEAD am: change safe_to_abort()'s not rewinding error into a warning am: fix filename in safe_to_abort() error message
2 parents 6d1f93a + ce73bb2 commit 7902b72

3 files changed

Lines changed: 61 additions & 2 deletions

File tree

builtin/am.c

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2124,7 +2124,7 @@ static int safe_to_abort(const struct am_state *state)
21242124

21252125
if (read_state_file(&sb, state, "abort-safety", 1) > 0) {
21262126
if (get_oid_hex(sb.buf, &abort_safety))
2127-
die(_("could not parse %s"), am_path(state, "abort_safety"));
2127+
die(_("could not parse %s"), am_path(state, "abort-safety"));
21282128
} else
21292129
oidclr(&abort_safety);
21302130

@@ -2134,7 +2134,7 @@ static int safe_to_abort(const struct am_state *state)
21342134
if (!oidcmp(&head, &abort_safety))
21352135
return 1;
21362136

2137-
error(_("You seem to have moved HEAD since the last 'am' failure.\n"
2137+
warning(_("You seem to have moved HEAD since the last 'am' failure.\n"
21382138
"Not rewinding to ORIG_HEAD"));
21392139

21402140
return 0;

sequencer.c

Lines changed: 49 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,7 @@ GIT_PATH_FUNC(git_path_seq_dir, "sequencer")
2727
static GIT_PATH_FUNC(git_path_todo_file, "sequencer/todo")
2828
static GIT_PATH_FUNC(git_path_opts_file, "sequencer/opts")
2929
static GIT_PATH_FUNC(git_path_head_file, "sequencer/head")
30+
static GIT_PATH_FUNC(git_path_abort_safety_file, "sequencer/abort-safety")
3031

3132
/*
3233
* A script to set the GIT_AUTHOR_NAME, GIT_AUTHOR_EMAIL, and
@@ -310,6 +311,20 @@ static int error_dirty_index(struct replay_opts *opts)
310311
return -1;
311312
}
312313

314+
static void update_abort_safety_file(void)
315+
{
316+
struct object_id head;
317+
318+
/* Do nothing on a single-pick */
319+
if (!file_exists(git_path_seq_dir()))
320+
return;
321+
322+
if (!get_oid("HEAD", &head))
323+
write_file(git_path_abort_safety_file(), "%s", oid_to_hex(&head));
324+
else
325+
write_file(git_path_abort_safety_file(), "%s", "");
326+
}
327+
313328
static int fast_forward_to(const unsigned char *to, const unsigned char *from,
314329
int unborn, struct replay_opts *opts)
315330
{
@@ -339,6 +354,7 @@ static int fast_forward_to(const unsigned char *to, const unsigned char *from,
339354
strbuf_release(&sb);
340355
strbuf_release(&err);
341356
ref_transaction_free(transaction);
357+
update_abort_safety_file();
342358
return 0;
343359
}
344360

@@ -813,6 +829,7 @@ static int do_pick_commit(enum todo_command command, struct commit *commit,
813829

814830
leave:
815831
free_message(commit, &msg);
832+
update_abort_safety_file();
816833

817834
return res;
818835
}
@@ -1132,9 +1149,34 @@ static int save_head(const char *head)
11321149
return 0;
11331150
}
11341151

1152+
static int rollback_is_safe(void)
1153+
{
1154+
struct strbuf sb = STRBUF_INIT;
1155+
struct object_id expected_head, actual_head;
1156+
1157+
if (strbuf_read_file(&sb, git_path_abort_safety_file(), 0) >= 0) {
1158+
strbuf_trim(&sb);
1159+
if (get_oid_hex(sb.buf, &expected_head)) {
1160+
strbuf_release(&sb);
1161+
die(_("could not parse %s"), git_path_abort_safety_file());
1162+
}
1163+
strbuf_release(&sb);
1164+
}
1165+
else if (errno == ENOENT)
1166+
oidclr(&expected_head);
1167+
else
1168+
die_errno(_("could not read '%s'"), git_path_abort_safety_file());
1169+
1170+
if (get_oid("HEAD", &actual_head))
1171+
oidclr(&actual_head);
1172+
1173+
return !oidcmp(&actual_head, &expected_head);
1174+
}
1175+
11351176
static int reset_for_rollback(const unsigned char *sha1)
11361177
{
11371178
const char *argv[4]; /* reset --merge <arg> + NULL */
1179+
11381180
argv[0] = "reset";
11391181
argv[1] = "--merge";
11401182
argv[2] = sha1_to_hex(sha1);
@@ -1189,6 +1231,12 @@ int sequencer_rollback(struct replay_opts *opts)
11891231
error(_("cannot abort from a branch yet to be born"));
11901232
goto fail;
11911233
}
1234+
1235+
if (!rollback_is_safe()) {
1236+
/* Do not error, just do not rollback */
1237+
warning(_("You seem to have moved HEAD. "
1238+
"Not rewinding, check your HEAD!"));
1239+
} else
11921240
if (reset_for_rollback(sha1))
11931241
goto fail;
11941242
strbuf_release(&buf);
@@ -1393,6 +1441,7 @@ int sequencer_pick_revisions(struct replay_opts *opts)
13931441
return -1;
13941442
if (save_opts(opts))
13951443
return -1;
1444+
update_abort_safety_file();
13961445
res = pick_commits(&todo_list, opts);
13971446
todo_list_release(&todo_list);
13981447
return res;

t/t3510-cherry-pick-sequence.sh

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -147,6 +147,16 @@ test_expect_success '--abort to cancel single cherry-pick' '
147147
git diff-index --exit-code HEAD
148148
'
149149

150+
test_expect_success '--abort does not unsafely change HEAD' '
151+
pristine_detach initial &&
152+
test_must_fail git cherry-pick picked anotherpick &&
153+
git reset --hard base &&
154+
test_must_fail git cherry-pick picked anotherpick &&
155+
git cherry-pick --abort 2>actual &&
156+
test_i18ngrep "You seem to have moved HEAD" actual &&
157+
test_cmp_rev base HEAD
158+
'
159+
150160
test_expect_success 'cherry-pick --abort to cancel multiple revert' '
151161
pristine_detach anotherpick &&
152162
test_expect_code 1 git revert base..picked &&

0 commit comments

Comments
 (0)