sequencer: make sequencer abort safer
In contrast to "git am --abort", a sequencer abort did not check whether the current HEAD is the one that is expected. This can lead to loss of work (when not spotted and resolved using reflog before the garbage collector chimes in). This behavior is now changed by mimicking "git am --abort". The abortion is done but HEAD is not changed when the current HEAD is not the expected HEAD. A new file "sequencer/abort-safety" is added to save the expected HEAD. The new behavior is only active when --abort is invoked on multiple picks. The problem does not occur for the single-pick case because it is handled differently. Signed-off-by: Stephan Beyer <s-beyer@gmx.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Stephan Beyer committed
Dec 7, 2016 at 22:51 UTC
1e41229d962b43208e6bf79e729b400c31697cc9
2 files changed
+50
-1
sequencer.c
+49
@@ -27,6 +27,7 @@ GIT_PATH_FUNC(git_path_seq_dir, "sequencer")
27
static GIT_PATH_FUNC(git_path_todo_file, "sequencer/todo")
28
static GIT_PATH_FUNC(git_path_opts_file, "sequencer/opts")
29
static GIT_PATH_FUNC(git_path_head_file, "sequencer/head")
30
+static GIT_PATH_FUNC(git_path_abort_safety_file, "sequencer/abort-safety")
31
32
/*
33
* 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)
311
return -1;
312
}
313
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
+
328
static int fast_forward_to(const unsigned char *to, const unsigned char *from,
329
int unborn, struct replay_opts *opts)
330
{
@@ -339,6 +354,7 @@ static int fast_forward_to(const unsigned char *to, const unsigned char *from,
354
strbuf_release(&sb);
355
strbuf_release(&err);
356
ref_transaction_free(transaction);
357
+ update_abort_safety_file();
358
return 0;
359
}
360
@@ -813,6 +829,7 @@ static int do_pick_commit(enum todo_command command, struct commit *commit,
829
830
leave:
831
free_message(commit, &msg);
832
+ update_abort_safety_file();
833
834
return res;
835
}
@@ -1132,9 +1149,34 @@ static int save_head(const char *head)
1149
return 0;
1150
}
1151
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
+
1176
static int reset_for_rollback(const unsigned char *sha1)
1177
{
1178
const char *argv[4]; /* reset --merge <arg> + NULL */
1179
+
1180
argv[0] = "reset";
1181
argv[1] = "--merge";
1182
argv[2] = sha1_to_hex(sha1);
@@ -1189,6 +1231,12 @@ int sequencer_rollback(struct replay_opts *opts)
1231
error(_("cannot abort from a branch yet to be born"));
1232
goto fail;
1233
}
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
1240
if (reset_for_rollback(sha1))
1241
goto fail;
1242
strbuf_release(&buf);
@@ -1393,6 +1441,7 @@ int sequencer_pick_revisions(struct replay_opts *opts)
1441
return -1;
1442
if (save_opts(opts))
1443
return -1;
1444
+ update_abort_safety_file();
1445
res = pick_commits(&todo_list, opts);
1446
todo_list_release(&todo_list);
1447
return res;
t/t3510-cherry-pick-sequence.sh
+1
-1
@@ -147,7 +147,7 @@ test_expect_success '--abort to cancel single cherry-pick' '
147
git diff-index --exit-code HEAD
148
'
149
150
-test_expect_failure '--abort does not unsafely change HEAD' '
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 &&