Merge branch 'js/rebase-i-root-fix'
authorJunio C Hamano <gitster@pobox.com>
Mon, 18 Jun 2018 18:23:22 +0000 (11:23 -0700)
committerJunio C Hamano <gitster@pobox.com>
Mon, 18 Jun 2018 18:23:22 +0000 (11:23 -0700)
A regression to "rebase -i --root" introduced during this cycle has
been fixed.

* js/rebase-i-root-fix:
rebase --root: fix amending root commit messages
rebase --root: demonstrate a bug while amending root commit messages

1  2 
sequencer.c
diff --combined sequencer.c
index cca968043ea86c4515e6af1b4fe9251a0140c0a3,0f017c058c0e42b810c25850fb836a3ee848f891..4034c0461b5022dad01b25d824cdc4f47ee09d13
@@@ -27,7 -27,6 +27,7 @@@
  #include "worktree.h"
  #include "oidmap.h"
  #include "oidset.h"
 +#include "alias.h"
  
  #define GIT_REFLOG_ACTION "GIT_REFLOG_ACTION"
  
@@@ -79,6 -78,13 +79,6 @@@ static GIT_PATH_FUNC(rebase_path_messag
   * previous commit and from the first squash/fixup commit are written
   * to it. The commit message for each subsequent squash/fixup commit
   * is appended to the file as it is processed.
 - *
 - * The first line of the file is of the form
 - *     # This is a combination of $count commits.
 - * where $count is the number of commits whose messages have been
 - * written to the file so far (including the initial "pick" commit).
 - * Each time that a commit message is processed, this line is read and
 - * updated. It is deleted just before the combined commit is made.
   */
  static GIT_PATH_FUNC(rebase_path_squash_msg, "rebase-merge/message-squash")
  /*
   * commit without opening the editor.)
   */
  static GIT_PATH_FUNC(rebase_path_fixup_msg, "rebase-merge/message-fixup")
 +/*
 + * This file contains the list fixup/squash commands that have been
 + * accumulated into message-fixup or message-squash so far.
 + */
 +static GIT_PATH_FUNC(rebase_path_current_fixups, "rebase-merge/current-fixups")
  /*
   * A script to set the GIT_AUTHOR_NAME, GIT_AUTHOR_EMAIL, and
   * GIT_AUTHOR_DATE that will be used for the commit that is currently
@@@ -284,7 -285,6 +284,7 @@@ int sequencer_remove_state(struct repla
        for (i = 0; i < opts->xopts_nr; i++)
                free(opts->xopts[i]);
        free(opts->xopts);
 +      strbuf_release(&opts->current_fixups);
  
        strbuf_reset(&buf);
        strbuf_addstr(&buf, get_dir(opts));
@@@ -536,8 -536,8 +536,8 @@@ static int do_recursive_merge(struct co
        o.show_rename_progress = 1;
  
        head_tree = parse_tree_indirect(head);
 -      next_tree = next ? next->tree : empty_tree();
 -      base_tree = base ? base->tree : empty_tree();
 +      next_tree = next ? get_commit_tree(next) : empty_tree();
 +      base_tree = base ? get_commit_tree(base) : empty_tree();
  
        for (xopt = opts->xopts; xopt != opts->xopts + opts->xopts_nr; xopt++)
                parse_merge_opt(&o, *xopt);
@@@ -607,7 -607,7 +607,7 @@@ static int is_index_unchanged(void
        if (!(cache_tree_oid = get_cache_tree_oid()))
                return -1;
  
 -      return !oidcmp(cache_tree_oid, &head_commit->tree->object.oid);
 +      return !oidcmp(cache_tree_oid, get_commit_tree_oid(head_commit));
  }
  
  static int write_author_script(const char *message)
@@@ -784,7 -784,7 +784,7 @@@ static int run_git_commit(const char *d
        struct child_process cmd = CHILD_PROCESS_INIT;
        const char *value;
  
-       if (flags & CREATE_ROOT_COMMIT) {
+       if ((flags & CREATE_ROOT_COMMIT) && !(flags & AMEND_MSG)) {
                struct strbuf msg = STRBUF_INIT, script = STRBUF_INIT;
                const char *author = is_rebase_i(opts) ?
                        read_author_ident(&script) : NULL;
                argv_array_pushf(&cmd.args, "-S%s", opts->gpg_sign);
        if (defmsg)
                argv_array_pushl(&cmd.args, "-F", defmsg, NULL);
 +      else if (!(flags & EDIT_MSG))
 +              argv_array_pushl(&cmd.args, "-C", "HEAD", NULL);
        if ((flags & CLEANUP_MSG))
                argv_array_push(&cmd.args, "--cleanup=strip");
        if ((flags & EDIT_MSG))
@@@ -1247,8 -1245,8 +1247,8 @@@ static int try_to_commit(struct strbuf 
        }
  
        if (!(flags & ALLOW_EMPTY) && !oidcmp(current_head ?
 -                                            &current_head->tree->object.oid :
 -                                            &empty_tree_oid, &tree)) {
 +                                            get_commit_tree_oid(current_head) :
 +                                            the_hash_algo->empty_tree, &tree)) {
                res = 1; /* run 'git commit' to display error message */
                goto out;
        }
                goto out;
        }
  
 +      reset_ident_date();
 +
        if (commit_tree_extended(msg->buf, msg->len, &tree, parents,
                                 oid, author, opts->gpg_sign, extra)) {
                res = error(_("failed to write commit object"));
@@@ -1348,12 -1344,12 +1348,12 @@@ static int is_original_commit_empty(str
                if (parse_commit(parent))
                        return error(_("could not parse parent commit %s"),
                                oid_to_hex(&parent->object.oid));
 -              ptree_oid = &parent->tree->object.oid;
 +              ptree_oid = get_commit_tree_oid(parent);
        } else {
                ptree_oid = the_hash_algo->empty_tree; /* commit is root */
        }
  
 -      return !oidcmp(ptree_oid, &commit->tree->object.oid);
 +      return !oidcmp(ptree_oid, get_commit_tree_oid(commit));
  }
  
  /*
@@@ -1482,23 -1478,34 +1482,23 @@@ static int update_squash_messages(enum 
                struct commit *commit, struct replay_opts *opts)
  {
        struct strbuf buf = STRBUF_INIT;
 -      int count, res;
 +      int res;
        const char *message, *body;
  
 -      if (file_exists(rebase_path_squash_msg())) {
 +      if (opts->current_fixup_count > 0) {
                struct strbuf header = STRBUF_INIT;
 -              char *eol, *p;
 +              char *eol;
  
 -              if (strbuf_read_file(&buf, rebase_path_squash_msg(), 2048) <= 0)
 +              if (strbuf_read_file(&buf, rebase_path_squash_msg(), 9) <= 0)
                        return error(_("could not read '%s'"),
                                rebase_path_squash_msg());
  
 -              p = buf.buf + 1;
 -              eol = strchrnul(buf.buf, '\n');
 -              if (buf.buf[0] != comment_line_char ||
 -                  (p += strcspn(p, "0123456789\n")) == eol)
 -                      return error(_("unexpected 1st line of squash message:"
 -                                     "\n\n\t%.*s"),
 -                                   (int)(eol - buf.buf), buf.buf);
 -              count = strtol(p, NULL, 10);
 -
 -              if (count < 1)
 -                      return error(_("invalid 1st line of squash message:\n"
 -                                     "\n\t%.*s"),
 -                                   (int)(eol - buf.buf), buf.buf);
 +              eol = buf.buf[0] != comment_line_char ?
 +                      buf.buf : strchrnul(buf.buf, '\n');
  
                strbuf_addf(&header, "%c ", comment_line_char);
 -              strbuf_addf(&header,
 -                          _("This is a combination of %d commits."), ++count);
 +              strbuf_addf(&header, _("This is a combination of %d commits."),
 +                          opts->current_fixup_count + 2);
                strbuf_splice(&buf, 0, eol - buf.buf, header.buf, header.len);
                strbuf_release(&header);
        } else {
                                     rebase_path_fixup_msg());
                }
  
 -              count = 2;
                strbuf_addf(&buf, "%c ", comment_line_char);
 -              strbuf_addf(&buf, _("This is a combination of %d commits."),
 -                          count);
 +              strbuf_addf(&buf, _("This is a combination of %d commits."), 2);
                strbuf_addf(&buf, "\n%c ", comment_line_char);
                strbuf_addstr(&buf, _("This is the 1st commit message:"));
                strbuf_addstr(&buf, "\n\n");
        if (command == TODO_SQUASH) {
                unlink(rebase_path_fixup_msg());
                strbuf_addf(&buf, "\n%c ", comment_line_char);
 -              strbuf_addf(&buf, _("This is the commit message #%d:"), count);
 +              strbuf_addf(&buf, _("This is the commit message #%d:"),
 +                          ++opts->current_fixup_count);
                strbuf_addstr(&buf, "\n\n");
                strbuf_addstr(&buf, body);
        } else if (command == TODO_FIXUP) {
                strbuf_addf(&buf, "\n%c ", comment_line_char);
                strbuf_addf(&buf, _("The commit message #%d will be skipped:"),
 -                          count);
 +                          ++opts->current_fixup_count);
                strbuf_addstr(&buf, "\n\n");
                strbuf_add_commented_lines(&buf, body, strlen(body));
        } else
  
        res = write_message(buf.buf, buf.len, rebase_path_squash_msg(), 0);
        strbuf_release(&buf);
 +
 +      if (!res) {
 +              strbuf_addf(&opts->current_fixups, "%s%s %s",
 +                          opts->current_fixups.len ? "\n" : "",
 +                          command_to_string(command),
 +                          oid_to_hex(&commit->object.oid));
 +              res = write_message(opts->current_fixups.buf,
 +                                  opts->current_fixups.len,
 +                                  rebase_path_current_fixups(), 0);
 +      }
 +
        return res;
  }
  
@@@ -1640,7 -1637,7 +1640,7 @@@ static int do_pick_commit(enum todo_com
                        unborn = 1;
                } else if (unborn)
                        oidcpy(&head, the_hash_algo->empty_tree);
 -              if (index_differs_from(unborn ? EMPTY_TREE_SHA1_HEX : "HEAD",
 +              if (index_differs_from(unborn ? empty_tree_oid_hex() : "HEAD",
                                       NULL, 0))
                        return error_dirty_index(opts);
        }
@@@ -1835,9 -1832,6 +1835,9 @@@ fast_forward_edit
        if (!res && final_fixup) {
                unlink(rebase_path_fixup_msg());
                unlink(rebase_path_squash_msg());
 +              unlink(rebase_path_current_fixups());
 +              strbuf_reset(&opts->current_fixups);
 +              opts->current_fixup_count = 0;
        }
  
  leave:
@@@ -2254,16 -2248,6 +2254,16 @@@ static int read_populate_opts(struct re
                read_strategy_opts(opts, &buf);
                strbuf_release(&buf);
  
 +              if (read_oneliner(&opts->current_fixups,
 +                                rebase_path_current_fixups(), 1)) {
 +                      const char *p = opts->current_fixups.buf;
 +                      opts->current_fixup_count = 1;
 +                      while ((p = strchr(p, '\n'))) {
 +                              opts->current_fixup_count++;
 +                              p++;
 +                      }
 +              }
 +
                if (read_oneliner(&buf, rebase_path_squash_onto(), 0)) {
                        if (get_oid_hex(buf.buf, &opts->squash_onto) < 0)
                                return error(_("unusable squash-onto"));
@@@ -2614,9 -2598,10 +2614,9 @@@ static int error_with_patch(struct comm
  static int error_failed_squash(struct commit *commit,
        struct replay_opts *opts, int subject_len, const char *subject)
  {
 -      if (rename(rebase_path_squash_msg(), rebase_path_message()))
 -              return error(_("could not rename '%s' to '%s'"),
 +      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(rebase_path_fixup_msg());
        unlink(git_path_merge_msg());
        if (copy_file(git_path_merge_msg(), rebase_path_message(), 0666))
                return error(_("could not copy '%s' to '%s'"),
@@@ -2708,7 -2693,7 +2708,7 @@@ static int safe_append(const char *file
  
  static int do_label(const char *name, int len)
  {
 -      struct ref_store *refs = get_main_ref_store();
 +      struct ref_store *refs = get_main_ref_store(the_repository);
        struct ref_transaction *transaction;
        struct strbuf ref_name = STRBUF_INIT, err = STRBUF_INIT;
        struct strbuf msg = STRBUF_INIT;
@@@ -2762,34 -2747,18 +2762,34 @@@ static int do_reset(const char *name, i
        if (hold_locked_index(&lock, LOCK_REPORT_ON_ERROR) < 0)
                return -1;
  
 -      /* Determine the length of the label */
 -      for (i = 0; i < len; i++)
 -              if (isspace(name[i]))
 -                      len = i;
 -
 -      strbuf_addf(&ref_name, "refs/rewritten/%.*s", len, name);
 -      if (get_oid(ref_name.buf, &oid) &&
 -          get_oid(ref_name.buf + strlen("refs/rewritten/"), &oid)) {
 -              error(_("could not read '%s'"), ref_name.buf);
 -              rollback_lock_file(&lock);
 -              strbuf_release(&ref_name);
 -              return -1;
 +      if (len == 10 && !strncmp("[new root]", name, len)) {
 +              if (!opts->have_squash_onto) {
 +                      const char *hex;
 +                      if (commit_tree("", 0, the_hash_algo->empty_tree,
 +                                      NULL, &opts->squash_onto,
 +                                      NULL, NULL))
 +                              return error(_("writing fake root commit"));
 +                      opts->have_squash_onto = 1;
 +                      hex = oid_to_hex(&opts->squash_onto);
 +                      if (write_message(hex, strlen(hex),
 +                                        rebase_path_squash_onto(), 0))
 +                              return error(_("writing squash-onto"));
 +              }
 +              oidcpy(&oid, &opts->squash_onto);
 +      } else {
 +              /* Determine the length of the label */
 +              for (i = 0; i < len; i++)
 +                      if (isspace(name[i]))
 +                              len = i;
 +
 +              strbuf_addf(&ref_name, "refs/rewritten/%.*s", len, name);
 +              if (get_oid(ref_name.buf, &oid) &&
 +                  get_oid(ref_name.buf + strlen("refs/rewritten/"), &oid)) {
 +                      error(_("could not read '%s'"), ref_name.buf);
 +                      rollback_lock_file(&lock);
 +                      strbuf_release(&ref_name);
 +                      return -1;
 +              }
        }
  
        memset(&unpack_tree_opts, 0, sizeof(unpack_tree_opts));
@@@ -2885,18 -2854,6 +2885,18 @@@ static int do_merge(struct commit *comm
                goto leave_merge;
        }
  
 +      if (opts->have_squash_onto &&
 +          !oidcmp(&head_commit->object.oid, &opts->squash_onto)) {
 +              /*
 +               * When the user tells us to "merge" something into a
 +               * "[new root]", let's simply fast-forward to the merge head.
 +               */
 +              rollback_lock_file(&lock);
 +              ret = fast_forward_to(&merge_commit->object.oid,
 +                                     &head_commit->object.oid, 0, opts);
 +              goto leave_merge;
 +      }
 +
        if (commit) {
                const char *message = get_commit_buffer(commit, NULL);
                const char *body;
@@@ -3404,16 -3361,19 +3404,16 @@@ static int continue_single_pick(void
        return run_command_v_opt(argv, RUN_GIT_CMD);
  }
  
 -static int commit_staged_changes(struct replay_opts *opts)
 +static int commit_staged_changes(struct replay_opts *opts,
 +                               struct todo_list *todo_list)
  {
        unsigned int flags = ALLOW_EMPTY | EDIT_MSG;
 +      unsigned int final_fixup = 0, is_clean;
  
        if (has_unstaged_changes(1))
                return error(_("cannot rebase: You have unstaged changes."));
 -      if (!has_uncommitted_changes(0)) {
 -              const char *cherry_pick_head = git_path_cherry_pick_head();
  
 -              if (file_exists(cherry_pick_head) && unlink(cherry_pick_head))
 -                      return error(_("could not remove CHERRY_PICK_HEAD"));
 -              return 0;
 -      }
 +      is_clean = !has_uncommitted_changes(0);
  
        if (file_exists(rebase_path_amend())) {
                struct strbuf rev = STRBUF_INIT;
                if (get_oid_hex(rev.buf, &to_amend))
                        return error(_("invalid contents: '%s'"),
                                rebase_path_amend());
 -              if (oidcmp(&head, &to_amend))
 +              if (!is_clean && oidcmp(&head, &to_amend))
                        return error(_("\nYou have uncommitted changes in your "
                                       "working tree. Please, commit them\n"
                                       "first and then run 'git rebase "
                                       "--continue' again."));
 +              /*
 +               * When skipping a failed fixup/squash, we need to edit the
 +               * commit message, the current fixup list and count, and if it
 +               * was the last fixup/squash in the chain, we need to clean up
 +               * the commit message and if there was a squash, let the user
 +               * edit it.
 +               */
 +              if (is_clean && !oidcmp(&head, &to_amend) &&
 +                  opts->current_fixup_count > 0 &&
 +                  file_exists(rebase_path_stopped_sha())) {
 +                      const char *p = opts->current_fixups.buf;
 +                      int len = opts->current_fixups.len;
 +
 +                      opts->current_fixup_count--;
 +                      if (!len)
 +                              BUG("Incorrect current_fixups:\n%s", p);
 +                      while (len && p[len - 1] != '\n')
 +                              len--;
 +                      strbuf_setlen(&opts->current_fixups, len);
 +                      if (write_message(p, len, rebase_path_current_fixups(),
 +                                        0) < 0)
 +                              return error(_("could not write file: '%s'"),
 +                                           rebase_path_current_fixups());
 +
 +                      /*
 +                       * If a fixup/squash in a fixup/squash chain failed, the
 +                       * commit message is already correct, no need to commit
 +                       * it again.
 +                       *
 +                       * Only if it is the final command in the fixup/squash
 +                       * chain, and only if the chain is longer than a single
 +                       * fixup/squash command (which was just skipped), do we
 +                       * actually need to re-commit with a cleaned up commit
 +                       * message.
 +                       */
 +                      if (opts->current_fixup_count > 0 &&
 +                          !is_fixup(peek_command(todo_list, 0))) {
 +                              final_fixup = 1;
 +                              /*
 +                               * If there was not a single "squash" in the
 +                               * chain, we only need to clean up the commit
 +                               * message, no need to bother the user with
 +                               * opening the commit message in the editor.
 +                               */
 +                              if (!starts_with(p, "squash ") &&
 +                                  !strstr(p, "\nsquash "))
 +                                      flags = (flags & ~EDIT_MSG) | CLEANUP_MSG;
 +                      } else if (is_fixup(peek_command(todo_list, 0))) {
 +                              /*
 +                               * We need to update the squash message to skip
 +                               * the latest commit message.
 +                               */
 +                              struct commit *commit;
 +                              const char *path = rebase_path_squash_msg();
 +
 +                              if (parse_head(&commit) ||
 +                                  !(p = get_commit_buffer(commit, NULL)) ||
 +                                  write_message(p, strlen(p), path, 0)) {
 +                                      unuse_commit_buffer(commit, p);
 +                                      return error(_("could not write file: "
 +                                                     "'%s'"), path);
 +                              }
 +                              unuse_commit_buffer(commit, p);
 +                      }
 +              }
  
                strbuf_release(&rev);
                flags |= AMEND_MSG;
        }
  
 -      if (run_git_commit(rebase_path_message(), opts, flags))
 +      if (is_clean) {
 +              const char *cherry_pick_head = git_path_cherry_pick_head();
 +
 +              if (file_exists(cherry_pick_head) && unlink(cherry_pick_head))
 +                      return error(_("could not remove CHERRY_PICK_HEAD"));
 +              if (!final_fixup)
 +                      return 0;
 +      }
 +
 +      if (run_git_commit(final_fixup ? NULL : rebase_path_message(),
 +                         opts, flags))
                return error(_("could not commit staged changes."));
        unlink(rebase_path_amend());
 +      if (final_fixup) {
 +              unlink(rebase_path_fixup_msg());
 +              unlink(rebase_path_squash_msg());
 +      }
 +      if (opts->current_fixup_count > 0) {
 +              /*
 +               * Whether final fixup or not, we just cleaned up the commit
 +               * message...
 +               */
 +              unlink(rebase_path_current_fixups());
 +              strbuf_reset(&opts->current_fixups);
 +              opts->current_fixup_count = 0;
 +      }
        return 0;
  }
  
@@@ -3538,16 -3410,14 +3538,16 @@@ int sequencer_continue(struct replay_op
        if (read_and_refresh_cache(opts))
                return -1;
  
 +      if (read_populate_opts(opts))
 +              return -1;
        if (is_rebase_i(opts)) {
 -              if (commit_staged_changes(opts))
 +              if ((res = read_populate_todo(&todo_list, opts)))
 +                      goto release_todo_list;
 +              if (commit_staged_changes(opts, &todo_list))
                        return -1;
        } else if (!file_exists(get_todo_path(opts)))
                return continue_single_pick();
 -      if (read_populate_opts(opts))
 -              return -1;
 -      if ((res = read_populate_todo(&todo_list, opts)))
 +      else if ((res = read_populate_todo(&todo_list, opts)))
                goto release_todo_list;
  
        if (!is_rebase_i(opts)) {
@@@ -3606,8 -3476,7 +3606,8 @@@ int sequencer_pick_revisions(struct rep
  
                if (!get_oid(name, &oid)) {
                        if (!lookup_commit_reference_gently(&oid, 1)) {
 -                              enum object_type type = oid_object_info(&oid,
 +                              enum object_type type = oid_object_info(the_repository,
 +                                                                      &oid,
                                                                        NULL);
                                return error(_("%s: can't cherry-pick a %s"),
                                        name, type_name(type));
@@@ -3792,7 -3661,7 +3792,7 @@@ static const char *label_oid(struct obj
                                p[i] = save;
                        }
                }
 -      } else if (((len = strlen(label)) == GIT_SHA1_RAWSZ &&
 +      } else if (((len = strlen(label)) == the_hash_algo->hexsz &&
                    !get_oid_hex(label, &dummy)) ||
                   (len == 1 && *label == '#') ||
                   hashmap_get_from_hash(&state->labels,
@@@ -4006,8 -3875,7 +4006,8 @@@ static int make_script_with_merges(stru
                }
  
                if (!commit)
 -                      fprintf(out, "%s onto\n", cmd_reset);
 +                      fprintf(out, "%s %s\n", cmd_reset,
 +                              rebase_cousins ? "onto" : "[new root]");
                else {
                        const char *to = NULL;