From: Junio C Hamano Date: Wed, 21 Dec 2016 22:55:01 +0000 (-0800) Subject: Merge branch 'sb/sequencer-abort-safety' X-Git-Tag: v2.12.0-rc0~116 X-Git-Url: https://git.lorimer.id.au/gitweb.git/diff_plain/4fcc0911989493e6e818dd933133cb18a32131fc?ds=inline;hp=-c 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 --- 4fcc0911989493e6e818dd933133cb18a32131fc diff --combined builtin/am.c index bb5da422fc,826f18ba12..31fb60578f --- a/builtin/am.c +++ b/builtin/am.c @@@ -1119,7 -1119,7 +1119,7 @@@ static void refresh_and_write_cache(voi { struct lock_file *lock_file = xcalloc(1, sizeof(struct lock_file)); - hold_locked_index(lock_file, 1); + hold_locked_index(lock_file, LOCK_DIE_ON_ERROR); refresh_cache(REFRESH_QUIET); if (write_locked_index(&the_index, lock_file, COMMIT_LOCK)) die(_("unable to write index file")); @@@ -1976,7 -1976,7 +1976,7 @@@ static int fast_forward_to(struct tree return -1; lock_file = xcalloc(1, sizeof(struct lock_file)); - hold_locked_index(lock_file, 1); + hold_locked_index(lock_file, LOCK_DIE_ON_ERROR); refresh_cache(REFRESH_QUIET); @@@ -2016,7 -2016,7 +2016,7 @@@ static int merge_tree(struct tree *tree return -1; lock_file = xcalloc(1, sizeof(struct lock_file)); - hold_locked_index(lock_file, 1); + hold_locked_index(lock_file, LOCK_DIE_ON_ERROR); memset(&opts, 0, sizeof(opts)); opts.head_idx = 1; @@@ -2124,7 -2124,7 +2124,7 @@@ static int safe_to_abort(const struct a 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 +2134,7 @@@ 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 --combined sequencer.c index 28061dcb78,0b78f3149f..9adb7bbf1d --- a/sequencer.c +++ b/sequencer.c @@@ -16,7 -16,6 +16,7 @@@ #include "refs.h" #include "argv-array.h" #include "quote.h" +#include "trailer.h" #define GIT_REFLOG_ACTION "GIT_REFLOG_ACTION" @@@ -28,6 -27,7 +28,7 @@@ GIT_PATH_FUNC(git_path_seq_dir, "sequen 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 @@@ -57,6 -57,30 +58,6 @@@ static const char *get_todo_path(const return git_path_todo_file(); } -static int is_rfc2822_line(const char *buf, int len) -{ - int i; - - for (i = 0; i < len; i++) { - int ch = buf[i]; - if (ch == ':') - return 1; - if (!isalnum(ch) && ch != '-') - break; - } - - return 0; -} - -static int is_cherry_picked_from_line(const char *buf, int len) -{ - /* - * We only care that it looks roughly like (cherry picked from ...) - */ - return len > strlen(cherry_picked_prefix) + 1 && - starts_with(buf, cherry_picked_prefix) && buf[len - 1] == ')'; -} - /* * Returns 0 for non-conforming footer * Returns 1 for conforming footer @@@ -66,25 -90,49 +67,25 @@@ static int has_conforming_footer(struct strbuf *sb, struct strbuf *sob, int ignore_footer) { - char prev; - int i, k; - int len = sb->len - ignore_footer; - const char *buf = sb->buf; - int found_sob = 0; - - /* footer must end with newline */ - if (!len || buf[len - 1] != '\n') - return 0; + struct trailer_info info; + int i; + int found_sob = 0, found_sob_last = 0; - prev = '\0'; - for (i = len - 1; i > 0; i--) { - char ch = buf[i]; - if (prev == '\n' && ch == '\n') /* paragraph break */ - break; - prev = ch; - } + trailer_info_get(&info, sb->buf); - /* require at least one blank line */ - if (prev != '\n' || buf[i] != '\n') + if (info.trailer_start == info.trailer_end) return 0; - /* advance to start of last paragraph */ - while (i < len - 1 && buf[i] == '\n') - i++; - - for (; i < len; i = k) { - int found_rfc2822; - - for (k = i; k < len && buf[k] != '\n'; k++) - ; /* do nothing */ - k++; + for (i = 0; i < info.trailer_nr; i++) + if (sob && !strncmp(info.trailers[i], sob->buf, sob->len)) { + found_sob = 1; + if (i == info.trailer_nr - 1) + found_sob_last = 1; + } - found_rfc2822 = is_rfc2822_line(buf + i, k - i - 1); - if (found_rfc2822 && sob && - !strncmp(buf + i, sob->buf, sob->len)) - found_sob = k; + trailer_info_release(&info); - if (!(found_rfc2822 || - is_cherry_picked_from_line(buf + i, k - i - 1))) - return 0; - } - if (found_sob == i) + if (found_sob_last) return 3; if (found_sob) return 2; @@@ -263,6 -311,20 +264,20 @@@ static int error_dirty_index(struct rep 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 -354,7 +307,7 @@@ strbuf_release(&sb); strbuf_release(&err); ref_transaction_free(transaction); + update_abort_safety_file(); return 0; } @@@ -323,7 -386,7 +339,7 @@@ static int do_recursive_merge(struct co char **xopt; static struct lock_file index_lock; - hold_locked_index(&index_lock, 1); + hold_locked_index(&index_lock, LOCK_DIE_ON_ERROR); read_cache(); @@@ -766,6 -829,7 +782,7 @@@ static int do_pick_commit(enum todo_com leave: free_message(commit, &msg); + update_abort_safety_file(); return res; } @@@ -1085,9 -1149,34 +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 + NULL */ + argv[0] = "reset"; argv[1] = "--merge"; argv[2] = sha1_to_hex(sha1); @@@ -1142,6 -1231,12 +1184,12 @@@ int sequencer_rollback(struct replay_op 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 -1441,7 +1394,7 @@@ int sequencer_pick_revisions(struct rep 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;