Merge branch 'pw/rebase-i-keep-reword-after-conflict'
authorJunio C Hamano <gitster@pobox.com>
Wed, 18 Jul 2018 19:20:29 +0000 (12:20 -0700)
committerJunio C Hamano <gitster@pobox.com>
Wed, 18 Jul 2018 19:20:29 +0000 (12:20 -0700)
Bugfix for "rebase -i" corner case regression.

* pw/rebase-i-keep-reword-after-conflict:
sequencer: do not squash 'reword' commits when we hit conflicts

1  2 
sequencer.c
diff --combined sequencer.c
index 0a291c91fe8937428c063db5e22afc93ea7691b6,7bf2b62727dcff2629347d944a2843fa6e778077..39363a950f841afff28032e619a1c10e294bd857
@@@ -2,7 -2,6 +2,7 @@@
  #include "config.h"
  #include "lockfile.h"
  #include "dir.h"
 +#include "object-store.h"
  #include "object.h"
  #include "commit.h"
  #include "sequencer.h"
@@@ -28,7 -27,6 +28,7 @@@
  #include "worktree.h"
  #include "oidmap.h"
  #include "oidset.h"
 +#include "commit-slab.h"
  #include "alias.h"
  
  #define GIT_REFLOG_ACTION "GIT_REFLOG_ACTION"
@@@ -177,7 -175,6 +177,7 @@@ static int git_sequencer_config(const c
                        warning(_("invalid commit message cleanup mode '%s'"),
                                  s);
  
 +              free((char *)s);
                return status;
        }
  
@@@ -358,7 -355,7 +358,7 @@@ static void print_advice(int show_hint
                 * (typically rebase --interactive) wants to take care
                 * of the commit itself so remove CHERRY_PICK_HEAD
                 */
 -              unlink(git_path_cherry_pick_head());
 +              unlink(git_path_cherry_pick_head(the_repository));
                return;
        }
  
@@@ -1325,8 -1322,8 +1325,8 @@@ static int do_commit(const char *msg_fi
                                    &oid);
                strbuf_release(&sb);
                if (!res) {
 -                      unlink(git_path_cherry_pick_head());
 -                      unlink(git_path_merge_msg());
 +                      unlink(git_path_cherry_pick_head(the_repository));
 +                      unlink(git_path_merge_msg(the_repository));
                        if (!is_rebase_i(opts))
                                print_commit_summary(NULL, &oid,
                                                SUMMARY_SHOW_AUTHOR_DATE);
@@@ -1614,7 -1611,7 +1614,7 @@@ static int do_pick_commit(enum todo_com
                struct replay_opts *opts, int final_fixup)
  {
        unsigned int flags = opts->edit ? EDIT_MSG : 0;
 -      const char *msg_file = opts->edit ? NULL : git_path_merge_msg();
 +      const char *msg_file = opts->edit ? NULL : git_path_merge_msg(the_repository);
        struct object_id head;
        struct commit *base, *next, *parent;
        const char *base_label, *next_label;
                        flags |= CLEANUP_MSG;
                        msg_file = rebase_path_fixup_msg();
                } else {
 -                      const char *dest = git_path_squash_msg();
 +                      const char *dest = git_path_squash_msg(the_repository);
                        unlink(dest);
                        if (copy_file(dest, rebase_path_squash_msg(), 0666))
                                return error(_("could not rename '%s' to '%s'"),
                                             rebase_path_squash_msg(), dest);
 -                      unlink(git_path_merge_msg());
 +                      unlink(git_path_merge_msg(the_repository));
                        msg_file = dest;
                        flags |= EDIT_MSG;
                }
                res = do_recursive_merge(base, next, base_label, next_label,
                                         &head, &msgbuf, opts);
                if (res < 0)
 -                      return res;
 +                      goto leave;
 +
                res |= write_message(msgbuf.buf, msgbuf.len,
 -                                   git_path_merge_msg(), 0);
 +                                   git_path_merge_msg(the_repository), 0);
        } else {
                struct commit_list *common = NULL;
                struct commit_list *remotes = NULL;
  
                res = write_message(msgbuf.buf, msgbuf.len,
 -                                  git_path_merge_msg(), 0);
 +                                  git_path_merge_msg(the_repository), 0);
  
                commit_list_insert(base, &common);
                commit_list_insert(next, &remotes);
@@@ -2395,8 -2391,8 +2395,8 @@@ static int rollback_single_pick(void
  {
        struct object_id head_oid;
  
 -      if (!file_exists(git_path_cherry_pick_head()) &&
 -          !file_exists(git_path_revert_head()))
 +      if (!file_exists(git_path_cherry_pick_head(the_repository)) &&
 +          !file_exists(git_path_revert_head(the_repository)))
                return error(_("no cherry-pick or revert in progress"));
        if (read_ref_full("HEAD", 0, &head_oid, NULL))
                return error(_("cannot resolve HEAD"));
@@@ -2621,11 -2617,10 +2621,11 @@@ static int error_failed_squash(struct c
        if (copy_file(rebase_path_message(), rebase_path_squash_msg(), 0666))
                return error(_("could not copy '%s' to '%s'"),
                        rebase_path_squash_msg(), rebase_path_message());
 -      unlink(git_path_merge_msg());
 -      if (copy_file(git_path_merge_msg(), rebase_path_message(), 0666))
 +      unlink(git_path_merge_msg(the_repository));
 +      if (copy_file(git_path_merge_msg(the_repository), rebase_path_message(), 0666))
                return error(_("could not copy '%s' to '%s'"),
 -                           rebase_path_message(), git_path_merge_msg());
 +                           rebase_path_message(),
 +                           git_path_merge_msg(the_repository));
        return error_with_patch(commit, subject, subject_len, opts, 1, 0);
  }
  
@@@ -2915,11 -2910,11 +2915,11 @@@ static int do_merge(struct commit *comm
                write_author_script(message);
                find_commit_subject(message, &body);
                len = strlen(body);
 -              ret = write_message(body, len, git_path_merge_msg(), 0);
 +              ret = write_message(body, len, git_path_merge_msg(the_repository), 0);
                unuse_commit_buffer(commit, message);
                if (ret) {
                        error_errno(_("could not write '%s'"),
 -                                  git_path_merge_msg());
 +                                  git_path_merge_msg(the_repository));
                        goto leave_merge;
                }
        } else {
                        len = buf.len;
                }
  
 -              ret = write_message(p, len, git_path_merge_msg(), 0);
 +              ret = write_message(p, len, git_path_merge_msg(the_repository), 0);
                strbuf_release(&buf);
                if (ret) {
                        error_errno(_("could not write '%s'"),
 -                                  git_path_merge_msg());
 +                                  git_path_merge_msg(the_repository));
                        goto leave_merge;
                }
        }
        }
  
        write_message(oid_to_hex(&merge_commit->object.oid), GIT_SHA1_HEXSZ,
 -                    git_path_merge_head(), 0);
 -      write_message("no-ff", 5, git_path_merge_mode(), 0);
 +                    git_path_merge_head(the_repository), 0);
 +      write_message("no-ff", 5, git_path_merge_mode(the_repository), 0);
  
        bases = get_merge_bases(head_commit, merge_commit);
        if (bases && !oidcmp(&merge_commit->object.oid,
                 * value (a negative one would indicate that the `merge`
                 * command needs to be rescheduled).
                 */
 -              ret = !!run_git_commit(git_path_merge_msg(), opts,
 +              ret = !!run_git_commit(git_path_merge_msg(the_repository), opts,
                                     run_commit_flags);
  
  leave_merge:
@@@ -3219,10 -3214,27 +3219,27 @@@ static int pick_commits(struct todo_lis
                                        intend_to_amend();
                                return error_failed_squash(item->commit, opts,
                                        item->arg_len, item->arg);
-                       } else if (res && is_rebase_i(opts) && item->commit)
+                       } else if (res && is_rebase_i(opts) && item->commit) {
+                               int to_amend = 0;
+                               struct object_id oid;
+                               /*
+                                * If we are rewording and have either
+                                * fast-forwarded already, or are about to
+                                * create a new root commit, we want to amend,
+                                * otherwise we do not.
+                                */
+                               if (item->command == TODO_REWORD &&
+                                   !get_oid("HEAD", &oid) &&
+                                   (!oidcmp(&item->commit->object.oid, &oid) ||
+                                    (opts->have_squash_onto &&
+                                     !oidcmp(&opts->squash_onto, &oid))))
+                                       to_amend = 1;
                                return res | error_with_patch(item->commit,
-                                       item->arg, item->arg_len, opts, res,
-                                       item->command == TODO_REWORD);
+                                               item->arg, item->arg_len, opts,
+                                               res, to_amend);
+                       }
                } else if (item->command == TODO_EXEC) {
                        char *end_of_arg = (char *)(item->arg + item->arg_len);
                        int saved = *end_of_arg;
@@@ -3403,8 -3415,8 +3420,8 @@@ static int continue_single_pick(void
  {
        const char *argv[] = { "commit", NULL };
  
 -      if (!file_exists(git_path_cherry_pick_head()) &&
 -          !file_exists(git_path_revert_head()))
 +      if (!file_exists(git_path_cherry_pick_head(the_repository)) &&
 +          !file_exists(git_path_revert_head(the_repository)))
                return error(_("no cherry-pick or revert in progress"));
        return run_command_v_opt(argv, RUN_GIT_CMD);
  }
@@@ -3507,7 -3519,7 +3524,7 @@@ static int commit_staged_changes(struc
        }
  
        if (is_clean) {
 -              const char *cherry_pick_head = git_path_cherry_pick_head();
 +              const char *cherry_pick_head = git_path_cherry_pick_head(the_repository);
  
                if (file_exists(cherry_pick_head) && unlink(cherry_pick_head))
                        return error(_("could not remove CHERRY_PICK_HEAD"));
@@@ -3557,8 -3569,8 +3574,8 @@@ int sequencer_continue(struct replay_op
  
        if (!is_rebase_i(opts)) {
                /* Verify that the conflict has been resolved */
 -              if (file_exists(git_path_cherry_pick_head()) ||
 -                  file_exists(git_path_revert_head())) {
 +              if (file_exists(git_path_cherry_pick_head(the_repository)) ||
 +                  file_exists(git_path_revert_head(the_repository))) {
                        res = continue_single_pick();
                        if (res)
                                goto release_todo_list;
@@@ -4243,7 -4255,6 +4260,7 @@@ static enum check_level get_missing_com
        return CHECK_IGNORE;
  }
  
 +define_commit_slab(commit_seen, unsigned char);
  /*
   * Check if the user dropped some commits by mistake
   * Behaviour determined by rebase.missingCommitsCheck.
@@@ -4257,9 -4268,6 +4274,9 @@@ int check_todo_list(void
        struct todo_list todo_list = TODO_LIST_INIT;
        struct strbuf missing = STRBUF_INIT;
        int advise_to_edit_todo = 0, res = 0, i;
 +      struct commit_seen commit_seen;
 +
 +      init_commit_seen(&commit_seen);
  
        strbuf_addstr(&todo_file, rebase_path_todo());
        if (strbuf_read_file_or_whine(&todo_list.buf, todo_file.buf) < 0) {
        for (i = 0; i < todo_list.nr; i++) {
                struct commit *commit = todo_list.items[i].commit;
                if (commit)
 -                      commit->util = (void *)1;
 +                      *commit_seen_at(&commit_seen, commit) = 1;
        }
  
        todo_list_release(&todo_list);
        for (i = todo_list.nr - 1; i >= 0; i--) {
                struct todo_item *item = todo_list.items + i;
                struct commit *commit = item->commit;
 -              if (commit && !commit->util) {
 +              if (commit && !*commit_seen_at(&commit_seen, commit)) {
                        strbuf_addf(&missing, " - %s %.*s\n",
                                    short_commit_name(commit),
                                    item->arg_len, item->arg);
 -                      commit->util = (void *)1;
 +                      *commit_seen_at(&commit_seen, commit) = 1;
                }
        }
  
                "The possible behaviours are: ignore, warn, error.\n\n"));
  
  leave_check:
 +      clear_commit_seen(&commit_seen);
        strbuf_release(&todo_file);
        todo_list_release(&todo_list);
  
@@@ -4443,8 -4450,6 +4460,8 @@@ static int subject2item_cmp(const void 
        return key ? strcmp(a->subject, key) : strcmp(a->subject, b->subject);
  }
  
 +define_commit_slab(commit_todo_item, struct todo_item *);
 +
  /*
   * Rearrange the todo list that has both "pick commit-id msg" and "pick
   * commit-id fixup!/squash! msg" in it so that the latter is put immediately
@@@ -4461,7 -4466,6 +4478,7 @@@ int rearrange_squash(void
        struct hashmap subject2item;
        int res = 0, rearranged = 0, *next, *tail, i;
        char **subjects;
 +      struct commit_todo_item commit_todo;
  
        if (strbuf_read_file_or_whine(&todo_list.buf, todo_file) < 0)
                return -1;
                return -1;
        }
  
 +      init_commit_todo_item(&commit_todo);
        /*
         * The hashmap maps onelines to the respective todo list index.
         *
  
                if (is_fixup(item->command)) {
                        todo_list_release(&todo_list);
 +                      clear_commit_todo_item(&commit_todo);
                        return error(_("the script was already rearranged."));
                }
  
 -              item->commit->util = item;
 +              *commit_todo_item_at(&commit_todo, item->commit) = item;
  
                parse_commit(item->commit);
                commit_buffer = get_commit_buffer(item->commit, NULL);
                        else if (!strchr(p, ' ') &&
                                 (commit2 =
                                  lookup_commit_reference_by_name(p)) &&
 -                               commit2->util)
 +                               *commit_todo_item_at(&commit_todo, commit2))
                                /* found by commit name */
 -                              i2 = (struct todo_item *)commit2->util
 +                              i2 = *commit_todo_item_at(&commit_todo, commit2)
                                        - todo_list.items;
                        else {
                                /* copy can be a prefix of the commit subject */
        hashmap_free(&subject2item, 1);
        todo_list_release(&todo_list);
  
 +      clear_commit_todo_item(&commit_todo);
        return res;
  }