Merge branch 'js/rebase-r-and-merge-head'
authorJunio C Hamano <gitster@pobox.com>
Sun, 18 Nov 2018 09:23:56 +0000 (18:23 +0900)
committerJunio C Hamano <gitster@pobox.com>
Sun, 18 Nov 2018 09:23:56 +0000 (18:23 +0900)
Bugfix for the recently graduated "git rebase --rebase-merges".

* js/rebase-r-and-merge-head:
status: rebase and merge can be in progress at the same time
built-in rebase --skip/--abort: clean up stale .git/<name> files
rebase -i: include MERGE_HEAD into files to clean up
rebase -r: do not write MERGE_HEAD unless needed
rebase -r: demonstrate bug with conflicting merges

1  2 
builtin/rebase.c
sequencer.c
diff --combined builtin/rebase.c
index 337c5c7f99f748b802ed2d77bb8ed48672174fe9,017dce1b503c3b6c6858b421d42541a88daed88a..59e6f7852f5cd2ca77e0aef721f2875edd7a6794
@@@ -23,6 -23,7 +23,7 @@@
  #include "revision.h"
  #include "commit-reach.h"
  #include "rerere.h"
+ #include "branch.h"
  
  static char const * const builtin_rebase_usage[] = {
        N_("git rebase [-i] [options] [--exec <cmd>] [--onto <newbase>] "
@@@ -522,17 -523,12 +523,17 @@@ finished_rebase
  
  #define GIT_REFLOG_ACTION_ENVIRONMENT "GIT_REFLOG_ACTION"
  
 +#define RESET_HEAD_DETACH (1<<0)
 +#define RESET_HEAD_HARD (1<<1)
 +
  static int reset_head(struct object_id *oid, const char *action,
 -                    const char *switch_to_branch, int detach_head,
 +                    const char *switch_to_branch, unsigned flags,
                      const char *reflog_orig_head, const char *reflog_head)
  {
 +      unsigned detach_head = flags & RESET_HEAD_DETACH;
 +      unsigned reset_hard = flags & RESET_HEAD_HARD;
        struct object_id head_oid;
 -      struct tree_desc desc;
 +      struct tree_desc desc[2] = { { NULL }, { NULL } };
        struct lock_file lock = LOCK_INIT;
        struct unpack_trees_options unpack_tree_opts;
        struct tree *tree;
        size_t prefix_len;
        struct object_id *orig = NULL, oid_orig,
                *old_orig = NULL, oid_old_orig;
 -      int ret = 0;
 +      int ret = 0, nr = 0;
  
        if (switch_to_branch && !starts_with(switch_to_branch, "refs/"))
                BUG("Not a fully qualified branch: '%s'", switch_to_branch);
  
 -      if (hold_locked_index(&lock, LOCK_REPORT_ON_ERROR) < 0)
 -              return -1;
 +      if (hold_locked_index(&lock, LOCK_REPORT_ON_ERROR) < 0) {
 +              ret = -1;
 +              goto leave_reset_head;
 +      }
  
 -      if (!oid) {
 -              if (get_oid("HEAD", &head_oid)) {
 -                      rollback_lock_file(&lock);
 -                      return error(_("could not determine HEAD revision"));
 -              }
 -              oid = &head_oid;
 +      if ((!oid || !reset_hard) && get_oid("HEAD", &head_oid)) {
 +              ret = error(_("could not determine HEAD revision"));
 +              goto leave_reset_head;
        }
  
 +      if (!oid)
 +              oid = &head_oid;
 +
        memset(&unpack_tree_opts, 0, sizeof(unpack_tree_opts));
        setup_unpack_trees_porcelain(&unpack_tree_opts, action);
        unpack_tree_opts.head_idx = 1;
        unpack_tree_opts.src_index = the_repository->index;
        unpack_tree_opts.dst_index = the_repository->index;
 -      unpack_tree_opts.fn = oneway_merge;
 +      unpack_tree_opts.fn = reset_hard ? oneway_merge : twoway_merge;
        unpack_tree_opts.update = 1;
        unpack_tree_opts.merge = 1;
        if (!detach_head)
                unpack_tree_opts.reset = 1;
  
        if (read_index_unmerged(the_repository->index) < 0) {
 -              rollback_lock_file(&lock);
 -              return error(_("could not read index"));
 +              ret = error(_("could not read index"));
 +              goto leave_reset_head;
        }
  
 -      if (!fill_tree_descriptor(&desc, oid)) {
 -              error(_("failed to find tree of %s"), oid_to_hex(oid));
 -              rollback_lock_file(&lock);
 -              free((void *)desc.buffer);
 -              return -1;
 +      if (!reset_hard && !fill_tree_descriptor(&desc[nr++], &head_oid)) {
 +              ret = error(_("failed to find tree of %s"), oid_to_hex(oid));
 +              goto leave_reset_head;
        }
  
 -      if (unpack_trees(1, &desc, &unpack_tree_opts)) {
 -              rollback_lock_file(&lock);
 -              free((void *)desc.buffer);
 -              return -1;
 +      if (!fill_tree_descriptor(&desc[nr++], oid)) {
 +              ret = error(_("failed to find tree of %s"), oid_to_hex(oid));
 +              goto leave_reset_head;
 +      }
 +
 +      if (unpack_trees(nr, desc, &unpack_tree_opts)) {
 +              ret = -1;
 +              goto leave_reset_head;
        }
  
        tree = parse_tree_indirect(oid);
        prime_cache_tree(the_repository->index, tree);
  
 -      if (write_locked_index(the_repository->index, &lock, COMMIT_LOCK) < 0)
 +      if (write_locked_index(the_repository->index, &lock, COMMIT_LOCK) < 0) {
                ret = error(_("could not write index"));
 -      free((void *)desc.buffer);
 -
 -      if (ret)
 -              return ret;
 +              goto leave_reset_head;
 +      }
  
        reflog_action = getenv(GIT_REFLOG_ACTION_ENVIRONMENT);
        strbuf_addf(&msg, "%s: ", reflog_action ? reflog_action : "rebase");
                reflog_head = msg.buf;
        }
        if (!switch_to_branch)
 -              ret = update_ref(reflog_head, "HEAD", oid, orig, REF_NO_DEREF,
 +              ret = update_ref(reflog_head, "HEAD", oid, orig,
 +                               detach_head ? REF_NO_DEREF : 0,
                                 UPDATE_REFS_MSG_ON_ERR);
        else {
                ret = create_symref("HEAD", switch_to_branch, msg.buf);
                                         UPDATE_REFS_MSG_ON_ERR);
        }
  
 +leave_reset_head:
        strbuf_release(&msg);
 +      rollback_lock_file(&lock);
 +      while (nr)
 +              free((void *)desc[--nr].buffer);
        return ret;
  }
  
@@@ -715,9 -704,6 +716,9 @@@ static int parse_opt_merge(const struc
  {
        struct rebase_options *opts = opt->value;
  
 +      BUG_ON_OPT_NEG(unset);
 +      BUG_ON_OPT_ARG(arg);
 +
        if (!is_interactive(opts))
                opts->type = REBASE_MERGE;
  
@@@ -730,9 -716,6 +731,9 @@@ static int parse_opt_interactive(const 
  {
        struct rebase_options *opts = opt->value;
  
 +      BUG_ON_OPT_NEG(unset);
 +      BUG_ON_OPT_ARG(arg);
 +
        opts->type = REBASE_INTERACTIVE;
        opts->flags |= REBASE_INTERACTIVE_EXPLICIT;
  
@@@ -1018,9 -1001,9 +1019,10 @@@ int cmd_rebase(int argc, const char **a
                rerere_clear(&merge_rr);
                string_list_clear(&merge_rr, 1);
  
 -              if (reset_head(NULL, "reset", NULL, 0, NULL, NULL) < 0)
 +              if (reset_head(NULL, "reset", NULL, RESET_HEAD_HARD,
 +                             NULL, NULL) < 0)
                        die(_("could not discard worktree changes"));
+               remove_branch_state();
                if (read_basic_state(&options))
                        exit(1);
                goto run_rebase;
                if (read_basic_state(&options))
                        exit(1);
                if (reset_head(&options.orig_head, "reset",
 -                             options.head_name, 0, NULL, NULL) < 0)
 +                             options.head_name, RESET_HEAD_HARD,
 +                             NULL, NULL) < 0)
                        die(_("could not move back to %s"),
                            oid_to_hex(&options.orig_head));
+               remove_branch_state();
                ret = finish_rebase(&options);
                goto cleanup;
        }
                        write_file(autostash, "%s", oid_to_hex(&oid));
                        printf(_("Created autostash: %s\n"), buf.buf);
                        if (reset_head(&head->object.oid, "reset --hard",
 -                                     NULL, 0, NULL, NULL) < 0)
 +                                     NULL, RESET_HEAD_HARD, NULL, NULL) < 0)
                                die(_("could not reset --hard"));
                        printf(_("HEAD is now at %s"),
                               find_unique_abbrev(&head->object.oid,
                         "it...\n"));
  
        strbuf_addf(&msg, "rebase: checkout %s", options.onto_name);
 -      if (reset_head(&options.onto->object.oid, "checkout", NULL, 1,
 -          NULL, msg.buf))
 +      if (reset_head(&options.onto->object.oid, "checkout", NULL,
 +                     RESET_HEAD_DETACH, NULL, msg.buf))
                die(_("Could not detach HEAD"));
        strbuf_release(&msg);
  
diff --combined sequencer.c
index 0d87b0739be5c5fdaf686063729aca02559638b8,2f526390acb5036d7f66c3522df8133d743778c7..e1a4dd15f1a826c7bd1bf4780c8f85c21117c43b
@@@ -669,131 -669,55 +669,131 @@@ missing_author
        return res;
  }
  
 +/**
 + * Take a series of KEY='VALUE' lines where VALUE part is
 + * sq-quoted, and append <KEY, VALUE> at the end of the string list
 + */
 +static int parse_key_value_squoted(char *buf, struct string_list *list)
 +{
 +      while (*buf) {
 +              struct string_list_item *item;
 +              char *np;
 +              char *cp = strchr(buf, '=');
 +              if (!cp) {
 +                      np = strchrnul(buf, '\n');
 +                      return error(_("no key present in '%.*s'"),
 +                                   (int) (np - buf), buf);
 +              }
 +              np = strchrnul(cp, '\n');
 +              *cp++ = '\0';
 +              item = string_list_append(list, buf);
 +
 +              buf = np + (*np == '\n');
 +              *np = '\0';
 +              cp = sq_dequote(cp);
 +              if (!cp)
 +                      return error(_("unable to dequote value of '%s'"),
 +                                   item->string);
 +              item->util = xstrdup(cp);
 +      }
 +      return 0;
 +}
  
 -/*
 - * write_author_script() used to fail to terminate the last line with a "'" and
 - * also escaped "'" incorrectly as "'\\\\''" rather than "'\\''". We check for
 - * the terminating "'" on the last line to see how "'" has been escaped in case
 - * git was upgraded while rebase was stopped.
 +/**
 + * Reads and parses the state directory's "author-script" file, and sets name,
 + * email and date accordingly.
 + * Returns 0 on success, -1 if the file could not be parsed.
 + *
 + * The author script is of the format:
 + *
 + *    GIT_AUTHOR_NAME='$author_name'
 + *    GIT_AUTHOR_EMAIL='$author_email'
 + *    GIT_AUTHOR_DATE='$author_date'
 + *
 + * where $author_name, $author_email and $author_date are quoted. We are strict
 + * with our parsing, as the file was meant to be eval'd in the old
 + * git-am.sh/git-rebase--interactive.sh scripts, and thus if the file differs
 + * from what this function expects, it is better to bail out than to do
 + * something that the user does not expect.
   */
 -static int quoting_is_broken(const char *s, size_t n)
 +int read_author_script(const char *path, char **name, char **email, char **date,
 +                     int allow_missing)
  {
 -      /* Skip any empty lines in case the file was hand edited */
 -      while (n > 0 && s[--n] == '\n')
 -              ; /* empty */
 -      if (n > 0 && s[n] != '\'')
 -              return 1;
 +      struct strbuf buf = STRBUF_INIT;
 +      struct string_list kv = STRING_LIST_INIT_DUP;
 +      int retval = -1; /* assume failure */
 +      int i, name_i = -2, email_i = -2, date_i = -2, err = 0;
  
 -      return 0;
 +      if (strbuf_read_file(&buf, path, 256) <= 0) {
 +              strbuf_release(&buf);
 +              if (errno == ENOENT && allow_missing)
 +                      return 0;
 +              else
 +                      return error_errno(_("could not open '%s' for reading"),
 +                                         path);
 +      }
 +
 +      if (parse_key_value_squoted(buf.buf, &kv))
 +              goto finish;
 +
 +      for (i = 0; i < kv.nr; i++) {
 +              if (!strcmp(kv.items[i].string, "GIT_AUTHOR_NAME")) {
 +                      if (name_i != -2)
 +                              name_i = error(_("'GIT_AUTHOR_NAME' already given"));
 +                      else
 +                              name_i = i;
 +              } else if (!strcmp(kv.items[i].string, "GIT_AUTHOR_EMAIL")) {
 +                      if (email_i != -2)
 +                              email_i = error(_("'GIT_AUTHOR_EMAIL' already given"));
 +                      else
 +                              email_i = i;
 +              } else if (!strcmp(kv.items[i].string, "GIT_AUTHOR_DATE")) {
 +                      if (date_i != -2)
 +                              date_i = error(_("'GIT_AUTHOR_DATE' already given"));
 +                      else
 +                              date_i = i;
 +              } else {
 +                      err = error(_("unknown variable '%s'"),
 +                                  kv.items[i].string);
 +              }
 +      }
 +      if (name_i == -2)
 +              error(_("missing 'GIT_AUTHOR_NAME'"));
 +      if (email_i == -2)
 +              error(_("missing 'GIT_AUTHOR_EMAIL'"));
 +      if (date_i == -2)
 +              error(_("missing 'GIT_AUTHOR_DATE'"));
 +      if (date_i < 0 || email_i < 0 || date_i < 0 || err)
 +              goto finish;
 +      *name = kv.items[name_i].util;
 +      *email = kv.items[email_i].util;
 +      *date = kv.items[date_i].util;
 +      retval = 0;
 +finish:
 +      string_list_clear(&kv, !!retval);
 +      strbuf_release(&buf);
 +      return retval;
  }
  
  /*
 - * Read a list of environment variable assignments (such as the author-script
 - * file) into an environment block. Returns -1 on error, 0 otherwise.
 + * Read a GIT_AUTHOR_NAME, GIT_AUTHOR_EMAIL AND GIT_AUTHOR_DATE from a
 + * file with shell quoting into struct argv_array. Returns -1 on
 + * error, 0 otherwise.
   */
  static int read_env_script(struct argv_array *env)
  {
 -      struct strbuf script = STRBUF_INIT;
 -      int i, count = 0, sq_bug;
 -      const char *p2;
 -      char *p;
 +      char *name, *email, *date;
  
 -      if (strbuf_read_file(&script, rebase_path_author_script(), 256) <= 0)
 +      if (read_author_script(rebase_path_author_script(),
 +                             &name, &email, &date, 0))
                return -1;
 -      /* write_author_script() used to quote incorrectly */
 -      sq_bug = quoting_is_broken(script.buf, script.len);
 -      for (p = script.buf; *p; p++)
 -              if (sq_bug && skip_prefix(p, "'\\\\''", &p2))
 -                      strbuf_splice(&script, p - script.buf, p2 - p, "'", 1);
 -              else if (skip_prefix(p, "'\\''", &p2))
 -                      strbuf_splice(&script, p - script.buf, p2 - p, "'", 1);
 -              else if (*p == '\'')
 -                      strbuf_splice(&script, p-- - script.buf, 1, "", 0);
 -              else if (*p == '\n') {
 -                      *p = '\0';
 -                      count++;
 -              }
  
 -      for (i = 0, p = script.buf; i < count; i++) {
 -              argv_array_push(env, p);
 -              p += strlen(p) + 1;
 -      }
 +      argv_array_pushf(env, "GIT_AUTHOR_NAME=%s", name);
 +      argv_array_pushf(env, "GIT_AUTHOR_EMAIL=%s", email);
 +      argv_array_pushf(env, "GIT_AUTHOR_DATE=%s", date);
 +      free(name);
 +      free(email);
 +      free(date);
  
        return 0;
  }
@@@ -813,28 -737,54 +813,28 @@@ static char *get_author(const char *mes
  /* Read author-script and return an ident line (author <email> timestamp) */
  static const char *read_author_ident(struct strbuf *buf)
  {
 -      const char *keys[] = {
 -              "GIT_AUTHOR_NAME=", "GIT_AUTHOR_EMAIL=", "GIT_AUTHOR_DATE="
 -      };
        struct strbuf out = STRBUF_INIT;
 -      char *in, *eol;
 -      const char *val[3];
 -      int i = 0;
 -
 -      if (strbuf_read_file(buf, rebase_path_author_script(), 256) <= 0)
 -              return NULL;
 -
 -      /* dequote values and construct ident line in-place */
 -      for (in = buf->buf; i < 3 && in - buf->buf < buf->len; i++) {
 -              if (!skip_prefix(in, keys[i], (const char **)&in)) {
 -                      warning(_("could not parse '%s' (looking for '%s')"),
 -                              rebase_path_author_script(), keys[i]);
 -                      return NULL;
 -              }
 -
 -              eol = strchrnul(in, '\n');
 -              *eol = '\0';
 -              if (!sq_dequote(in)) {
 -                      warning(_("bad quoting on %s value in '%s'"),
 -                              keys[i], rebase_path_author_script());
 -                      return NULL;
 -              }
 -              val[i] = in;
 -              in = eol + 1;
 -      }
 +      char *name, *email, *date;
  
 -      if (i < 3) {
 -              warning(_("could not parse '%s' (looking for '%s')"),
 -                      rebase_path_author_script(), keys[i]);
 +      if (read_author_script(rebase_path_author_script(),
 +                             &name, &email, &date, 0))
                return NULL;
 -      }
  
        /* validate date since fmt_ident() will die() on bad value */
 -      if (parse_date(val[2], &out)){
 +      if (parse_date(date, &out)){
                warning(_("invalid date format '%s' in '%s'"),
 -                      val[2], rebase_path_author_script());
 +                      date, rebase_path_author_script());
                strbuf_release(&out);
                return NULL;
        }
  
        strbuf_reset(&out);
 -      strbuf_addstr(&out, fmt_ident(val[0], val[1], val[2], 0));
 +      strbuf_addstr(&out, fmt_ident(name, email, date, 0));
        strbuf_swap(buf, &out);
        strbuf_release(&out);
 +      free(name);
 +      free(email);
 +      free(date);
        return buf->buf;
  }
  
@@@ -1969,7 -1919,7 +1969,7 @@@ static int read_and_refresh_cache(struc
  {
        struct lock_file index_lock = LOCK_INIT;
        int index_fd = hold_locked_index(&index_lock, 0);
 -      if (read_index_preload(&the_index, NULL, 0) < 0) {
 +      if (read_index(&the_index) < 0) {
                rollback_lock_file(&index_lock);
                return error(_("git %s: failed to read the index"),
                        _(action_name(opts)));
@@@ -2940,7 -2890,7 +2940,7 @@@ static int do_reset(const char *name, i
        struct tree_desc desc;
        struct tree *tree;
        struct unpack_trees_options unpack_tree_opts;
 -      int ret = 0, i;
 +      int ret = 0;
  
        if (hold_locked_index(&lock, LOCK_REPORT_ON_ERROR) < 0)
                return -1;
                }
                oidcpy(&oid, &opts->squash_onto);
        } else {
 +              int i;
 +
                /* Determine the length of the label */
                for (i = 0; i < len; i++)
                        if (isspace(name[i]))
 -                              len = i;
 +                              break;
 +              len = i;
  
                strbuf_addf(&ref_name, "refs/rewritten/%.*s", len, name);
                if (get_oid(ref_name.buf, &oid) &&
@@@ -3244,10 -3191,6 +3244,6 @@@ static int do_merge(struct commit *comm
        }
  
        merge_commit = to_merge->item;
-       write_message(oid_to_hex(&merge_commit->object.oid), GIT_SHA1_HEXSZ,
-                     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 && oideq(&merge_commit->object.oid,
                           &bases->item->object.oid)) {
                goto leave_merge;
        }
  
+       write_message(oid_to_hex(&merge_commit->object.oid), GIT_SHA1_HEXSZ,
+                     git_path_merge_head(the_repository), 0);
+       write_message("no-ff", 5, git_path_merge_mode(the_repository), 0);
        for (j = bases; j; j = j->next)
                commit_list_insert(j->item, &reversed);
        free_commit_list(bases);
@@@ -3512,6 -3459,7 +3512,7 @@@ static int pick_commits(struct todo_lis
                        unlink(rebase_path_author_script());
                        unlink(rebase_path_stopped_sha());
                        unlink(rebase_path_amend());
+                       unlink(git_path_merge_head(the_repository));
                        delete_ref(NULL, "REBASE_HEAD", NULL, REF_NO_DEREF);
  
                        if (item->command == TODO_BREAK)
@@@ -3882,6 -3830,7 +3883,7 @@@ static int commit_staged_changes(struc
                           opts, flags))
                return error(_("could not commit staged changes."));
        unlink(rebase_path_amend());
+       unlink(git_path_merge_head(the_repository));
        if (final_fixup) {
                unlink(rebase_path_fixup_msg());
                unlink(rebase_path_squash_msg());
@@@ -4899,7 -4848,7 +4901,7 @@@ int complete_action(struct replay_opts 
  
        if (checkout_onto(opts, onto_name, oid_to_hex(&oid), orig_head))
                return -1;
 -;
 +
        if (require_clean_work_tree("rebase", "", 1, 1))
                return -1;