Merge branch 'sb/sequencer-abort-safety'

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

diff --git a/builtin/am.c b/builtin/am.c
index bb5da422fc1ae06a8e5ad3735cd0cda69ff5abc1..31fb60578f6caacfb376fe84938dda9432dcfc5a 100644 (file)
--- a/builtin/am.c
+++ b/builtin/am.c
@@ -2124,7 +2124,7 @@ static int safe_to_abort(const struct am_state *state)
 
        if (read_state_file(&sb, state, "abort-safety", 1) > 0) {
                if (get_oid_hex(sb.buf, &abort_safety))
-                       die(_("could not parse %s"), am_path(state, "abort_safety"));
+                       die(_("could not parse %s"), am_path(state, "abort-safety"));
        } else
                oidclr(&abort_safety);
 
@@ -2134,7 +2134,7 @@ static int safe_to_abort(const struct am_state *state)
        if (!oidcmp(&head, &abort_safety))
                return 1;
 
-       error(_("You seem to have moved HEAD since the last 'am' failure.\n"
+       warning(_("You seem to have moved HEAD since the last 'am' failure.\n"
                "Not rewinding to ORIG_HEAD"));
 
        return 0;

diff --git a/sequencer.c b/sequencer.c
index 28061dcb78b5db5711f77da1026c0cb81acee27b..9adb7bbf1d4815d9e4f67b66e90c9ec327b2fb72 100644 (file)
--- a/sequencer.c
+++ b/sequencer.c
@@ -28,6 +28,7 @@ GIT_PATH_FUNC(git_path_seq_dir, "sequencer")
 static GIT_PATH_FUNC(git_path_todo_file, "sequencer/todo")
 static GIT_PATH_FUNC(git_path_opts_file, "sequencer/opts")
 static GIT_PATH_FUNC(git_path_head_file, "sequencer/head")
+static GIT_PATH_FUNC(git_path_abort_safety_file, "sequencer/abort-safety")
 
 /*
  * A script to set the GIT_AUTHOR_NAME, GIT_AUTHOR_EMAIL, and
@@ -263,6 +264,20 @@ static int error_dirty_index(struct replay_opts *opts)
        return -1;
 }
 
+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 (!get_oid("HEAD", &head))
+               write_file(git_path_abort_safety_file(), "%s", oid_to_hex(&head));
+       else
+               write_file(git_path_abort_safety_file(), "%s", "");
+}
+
 static int fast_forward_to(const unsigned char *to, const unsigned char *from,
                        int unborn, struct replay_opts *opts)
 {
@@ -292,6 +307,7 @@ static int fast_forward_to(const unsigned char *to, const unsigned char *from,
        strbuf_release(&sb);
        strbuf_release(&err);
        ref_transaction_free(transaction);
+       update_abort_safety_file();
        return 0;
 }
 
@@ -766,6 +782,7 @@ static int do_pick_commit(enum todo_command command, struct commit *commit,
 
 leave:
        free_message(commit, &msg);
+       update_abort_safety_file();
 
        return res;
 }
@@ -1085,9 +1102,34 @@ static int save_head(const char *head)
        return 0;
 }
 
+static int rollback_is_safe(void)
+{
+       struct strbuf sb = STRBUF_INIT;
+       struct object_id expected_head, actual_head;
+
+       if (strbuf_read_file(&sb, git_path_abort_safety_file(), 0) >= 0) {
+               strbuf_trim(&sb);
+               if (get_oid_hex(sb.buf, &expected_head)) {
+                       strbuf_release(&sb);
+                       die(_("could not parse %s"), git_path_abort_safety_file());
+               }
+               strbuf_release(&sb);
+       }
+       else if (errno == ENOENT)
+               oidclr(&expected_head);
+       else
+               die_errno(_("could not read '%s'"), git_path_abort_safety_file());
+
+       if (get_oid("HEAD", &actual_head))
+               oidclr(&actual_head);
+
+       return !oidcmp(&actual_head, &expected_head);
+}
+
 static int reset_for_rollback(const unsigned char *sha1)
 {
        const char *argv[4];    /* reset --merge <arg> + NULL */
+
        argv[0] = "reset";
        argv[1] = "--merge";
        argv[2] = sha1_to_hex(sha1);
@@ -1142,6 +1184,12 @@ int sequencer_rollback(struct replay_opts *opts)
                error(_("cannot abort from a branch yet to be born"));
                goto fail;
        }
+
+       if (!rollback_is_safe()) {
+               /* Do not error, just do not rollback */
+               warning(_("You seem to have moved HEAD. "
+                         "Not rewinding, check your HEAD!"));
+       } else
        if (reset_for_rollback(sha1))
                goto fail;
        strbuf_release(&buf);
@@ -1346,6 +1394,7 @@ int sequencer_pick_revisions(struct replay_opts *opts)
                return -1;
        if (save_opts(opts))
                return -1;
+       update_abort_safety_file();
        res = pick_commits(&todo_list, opts);
        todo_list_release(&todo_list);
        return res;

diff --git a/t/t3510-cherry-pick-sequence.sh b/t/t3510-cherry-pick-sequence.sh
index 7b7a89dbd5ce578e0a722345a00f247e383689ef..372307c21b983437b870ac65ff4f91d2c36fab30 100755 (executable)
--- a/t/t3510-cherry-pick-sequence.sh
+++ b/t/t3510-cherry-pick-sequence.sh
@@ -147,6 +147,16 @@ test_expect_success '--abort to cancel single cherry-pick' '
        git diff-index --exit-code HEAD
 '
 
+test_expect_success '--abort does not unsafely change HEAD' '
+       pristine_detach initial &&
+       test_must_fail git cherry-pick picked anotherpick &&
+       git reset --hard base &&
+       test_must_fail git cherry-pick picked anotherpick &&
+       git cherry-pick --abort 2>actual &&
+       test_i18ngrep "You seem to have moved HEAD" actual &&
+       test_cmp_rev base HEAD
+'
+
 test_expect_success 'cherry-pick --abort to cancel multiple revert' '
        pristine_detach anotherpick &&
        test_expect_code 1 git revert base..picked &&