Merge branch 'js/rebase-orig-head-fix'
authorJunio C Hamano <gitster@pobox.com>
Wed, 20 Mar 2019 06:16:05 +0000 (15:16 +0900)
committerJunio C Hamano <gitster@pobox.com>
Wed, 20 Mar 2019 06:16:05 +0000 (15:16 +0900)
"git rebase" that was reimplemented in C did not set ORIG_HEAD
correctly, which has been corrected.

* js/rebase-orig-head-fix:
built-in rebase: set ORIG_HEAD just once, before the rebase
built-in rebase: demonstrate that ORIG_HEAD is not set correctly
built-in rebase: use the correct reflog when switching branches
built-in rebase: no need to check out `onto` twice

1  2 
builtin/rebase.c
diff --combined builtin/rebase.c
index 52114cbf0d9f98d52a5b48d8806019d14b1c7029,0f4e1ead4902c38ab534904e3e09428182528109..77deebc65c6bd7fbe6261b91cfb78c7ec3f85c2d
@@@ -4,7 -4,6 +4,7 @@@
   * Copyright (c) 2018 Pratik Karki
   */
  
 +#define USE_THE_INDEX_COMPATIBILITY_MACROS
  #include "builtin.h"
  #include "run-command.h"
  #include "exec-cmd.h"
@@@ -105,7 -104,6 +105,7 @@@ struct rebase_options 
        int rebase_merges, rebase_cousins;
        char *strategy, *strategy_opts;
        struct strbuf git_format_patch_opt;
 +      int reschedule_failed_exec;
  };
  
  static int is_interactive(struct rebase_options *opts)
@@@ -124,7 -122,7 +124,7 @@@ static void imply_interactive(struct re
        case REBASE_PRESERVE_MERGES:
                break;
        case REBASE_MERGE:
 -              /* we silently *upgrade* --merge to --interactive if needed */
 +              /* we now implement --merge via --interactive */
        default:
                opts->type = REBASE_INTERACTIVE; /* implied */
                break;
@@@ -187,7 -185,10 +187,7 @@@ static int read_basic_state(struct reba
        if (get_oid(buf.buf, &opts->orig_head))
                return error(_("invalid orig-head: '%s'"), buf.buf);
  
 -      strbuf_reset(&buf);
 -      if (read_one(state_dir_path("quiet", opts), &buf))
 -              return -1;
 -      if (buf.len)
 +      if (file_exists(state_dir_path("quiet", opts)))
                opts->flags &= ~REBASE_NO_QUIET;
        else
                opts->flags |= REBASE_NO_QUIET;
@@@ -367,8 -368,8 +367,9 @@@ static void add_var(struct strbuf *buf
  
  #define RESET_HEAD_DETACH (1<<0)
  #define RESET_HEAD_HARD (1<<1)
 -#define RESET_HEAD_REFS_ONLY (1<<2)
 -#define RESET_ORIG_HEAD (1<<3)
 +#define RESET_HEAD_RUN_POST_CHECKOUT_HOOK (1<<2)
 +#define RESET_HEAD_REFS_ONLY (1<<3)
++#define RESET_ORIG_HEAD (1<<4)
  
  static int reset_head(struct object_id *oid, const char *action,
                      const char *switch_to_branch, unsigned flags,
  {
        unsigned detach_head = flags & RESET_HEAD_DETACH;
        unsigned reset_hard = flags & RESET_HEAD_HARD;
 +      unsigned run_hook = flags & RESET_HEAD_RUN_POST_CHECKOUT_HOOK;
        unsigned refs_only = flags & RESET_HEAD_REFS_ONLY;
+       unsigned update_orig_head = flags & RESET_ORIG_HEAD;
        struct object_id head_oid;
        struct tree_desc desc[2] = { { NULL }, { NULL } };
        struct lock_file lock = LOCK_INIT;
        if (!detach_head)
                unpack_tree_opts.reset = 1;
  
 -      if (read_index_unmerged(the_repository->index) < 0) {
 +      if (repo_read_index_unmerged(the_repository) < 0) {
                ret = error(_("could not read index"));
                goto leave_reset_head;
        }
        }
  
        tree = parse_tree_indirect(oid);
 -      prime_cache_tree(the_repository->index, tree);
 +      prime_cache_tree(the_repository, the_repository->index, tree);
  
        if (write_locked_index(the_repository->index, &lock, COMMIT_LOCK) < 0) {
                ret = error(_("could not write index"));
@@@ -454,18 -455,21 +456,21 @@@ reset_head_refs
        strbuf_addf(&msg, "%s: ", reflog_action ? reflog_action : "rebase");
        prefix_len = msg.len;
  
-       if (!get_oid("ORIG_HEAD", &oid_old_orig))
-               old_orig = &oid_old_orig;
-       if (!get_oid("HEAD", &oid_orig)) {
-               orig = &oid_orig;
-               if (!reflog_orig_head) {
-                       strbuf_addstr(&msg, "updating ORIG_HEAD");
-                       reflog_orig_head = msg.buf;
-               }
-               update_ref(reflog_orig_head, "ORIG_HEAD", orig, old_orig, 0,
-                          UPDATE_REFS_MSG_ON_ERR);
-       } else if (old_orig)
-               delete_ref(NULL, "ORIG_HEAD", old_orig, 0);
+       if (update_orig_head) {
+               if (!get_oid("ORIG_HEAD", &oid_old_orig))
+                       old_orig = &oid_old_orig;
+               if (!get_oid("HEAD", &oid_orig)) {
+                       orig = &oid_orig;
+                       if (!reflog_orig_head) {
+                               strbuf_addstr(&msg, "updating ORIG_HEAD");
+                               reflog_orig_head = msg.buf;
+                       }
+                       update_ref(reflog_orig_head, "ORIG_HEAD", orig,
+                                  old_orig, 0, UPDATE_REFS_MSG_ON_ERR);
+               } else if (old_orig)
+                       delete_ref(NULL, "ORIG_HEAD", old_orig, 0);
+       }
        if (!reflog_head) {
                strbuf_setlen(&msg, prefix_len);
                strbuf_addstr(&msg, "updating HEAD");
                                 detach_head ? REF_NO_DEREF : 0,
                                 UPDATE_REFS_MSG_ON_ERR);
        else {
-               ret = update_ref(reflog_orig_head, switch_to_branch, oid,
+               ret = update_ref(reflog_head, switch_to_branch, oid,
                                 NULL, 0, UPDATE_REFS_MSG_ON_ERR);
                if (!ret)
                        ret = create_symref("HEAD", switch_to_branch,
                                            reflog_head);
        }
 +      if (run_hook)
 +              run_hook_le(NULL, "post-checkout",
 +                          oid_to_hex(orig ? orig : &null_oid),
 +                          oid_to_hex(oid), "1", NULL);
  
  leave_reset_head:
        strbuf_release(&msg);
@@@ -583,7 -583,7 +588,7 @@@ static int run_am(struct rebase_option
        argv_array_pushl(&format_patch.args, "format-patch", "-k", "--stdout",
                         "--full-index", "--cherry-pick", "--right-only",
                         "--src-prefix=a/", "--dst-prefix=b/", "--no-renames",
 -                       "--no-cover-letter", "--pretty=mboxrd", NULL);
 +                       "--no-cover-letter", "--pretty=mboxrd", "--topo-order", NULL);
        if (opts->git_format_patch_opt.len)
                argv_array_split(&format_patch.args,
                                 opts->git_format_patch_opt.buf);
@@@ -659,8 -659,7 +664,8 @@@ static int run_specific_rebase(struct r
                argv_array_pushf(&child.env_array, "GIT_CHERRY_PICK_HELP=%s",
                                 resolvemsg);
                if (!(opts->flags & REBASE_INTERACTIVE_EXPLICIT)) {
 -                      argv_array_push(&child.env_array, "GIT_EDITOR=:");
 +                      argv_array_push(&child.env_array,
 +                                      "GIT_SEQUENCE_EDITOR=:");
                        opts->autosquash = 0;
                }
  
                        argv_array_push(&child.args, opts->gpg_sign_opt);
                if (opts->signoff)
                        argv_array_push(&child.args, "--signoff");
 +              if (opts->reschedule_failed_exec)
 +                      argv_array_push(&child.args, "--reschedule-failed-exec");
  
                status = run_command(&child);
                goto finished_rebase;
        if (is_interactive(opts) &&
            !(opts->flags & REBASE_INTERACTIVE_EXPLICIT)) {
                strbuf_addstr(&script_snippet,
 -                            "GIT_EDITOR=:; export GIT_EDITOR; ");
 +                            "GIT_SEQUENCE_EDITOR=:; export GIT_SEQUENCE_EDITOR; ");
                opts->autosquash = 0;
        }
  
                backend = "git-rebase--am";
                backend_func = "git_rebase__am";
                break;
 -      case REBASE_MERGE:
 -              backend = "git-rebase--merge";
 -              backend_func = "git_rebase__merge";
 -              break;
        case REBASE_PRESERVE_MERGES:
                backend = "git-rebase--preserve-merges";
                backend_func = "git_rebase__preserve_merges";
@@@ -864,11 -865,6 +869,11 @@@ static int rebase_config(const char *va
                return 0;
        }
  
 +      if (!strcmp(var, "rebase.reschedulefailedexec")) {
 +              opts->reschedule_failed_exec = git_config_bool(var, value);
 +              return 0;
 +      }
 +
        return git_default_config(var, value, data);
  }
  
@@@ -988,19 -984,6 +993,19 @@@ static void set_reflog_action(struct re
        strbuf_release(&buf);
  }
  
 +static int check_exec_cmd(const char *cmd)
 +{
 +      if (strchr(cmd, '\n'))
 +              return error(_("exec commands cannot contain newlines"));
 +
 +      /* Does the command consist purely of whitespace? */
 +      if (!cmd[strspn(cmd, " \t\r\f\v")])
 +              return error(_("empty exec command"));
 +
 +      return 0;
 +}
 +
 +
  int cmd_rebase(int argc, const char **argv, const char *prefix)
  {
        struct rebase_options options = {
                ACTION_EDIT_TODO,
                ACTION_SHOW_CURRENT_PATCH,
        } action = NO_ACTION;
 +      static const char *action_names[] = { N_("undefined"),
 +                                            N_("continue"),
 +                                            N_("skip"),
 +                                            N_("abort"),
 +                                            N_("quit"),
 +                                            N_("edit_todo"),
 +                                            N_("show_current_patch"),
 +                                            NULL };
        const char *gpg_sign = NULL;
        struct string_list exec = STRING_LIST_INIT_NODUP;
        const char *rebase_merges = NULL;
                                   "strategy")),
                OPT_BOOL(0, "root", &options.root,
                         N_("rebase all reachable commits up to the root(s)")),
 +              OPT_BOOL(0, "reschedule-failed-exec",
 +                       &options.reschedule_failed_exec,
 +                       N_("automatically re-schedule any `exec` that fails")),
                OPT_END(),
        };
        int i;
                die(_("The --edit-todo action can only be used during "
                      "interactive rebase."));
  
 +      if (trace2_is_enabled()) {
 +              if (is_interactive(&options))
 +                      trace2_cmd_mode("interactive");
 +              else if (exec.nr)
 +                      trace2_cmd_mode("interactive-exec");
 +              else
 +                      trace2_cmd_mode(action_names[action]);
 +      }
 +
        switch (action) {
        case ACTION_CONTINUE: {
                struct object_id head;
                        die(_("Cannot read HEAD"));
  
                fd = hold_locked_index(&lock_file, 0);
 -              if (read_index(the_repository->index) < 0)
 +              if (repo_read_index(the_repository) < 0)
                        die(_("could not read index"));
                refresh_index(the_repository->index, REFRESH_QUIET, NULL, NULL,
                              NULL);
                if (0 <= fd)
 -                      update_index_if_able(the_repository->index,
 -                                           &lock_file);
 +                      repo_update_index_if_able(the_repository, &lock_file);
                rollback_lock_file(&lock_file);
  
 -              if (has_unstaged_changes(1)) {
 +              if (has_unstaged_changes(the_repository, 1)) {
                        puts(_("You must edit all merge conflicts and then\n"
                               "mark them as resolved using git add"));
                        exit(1);
                options.action = "skip";
                set_reflog_action(&options);
  
 -              rerere_clear(&merge_rr);
 +              rerere_clear(the_repository, &merge_rr);
                string_list_clear(&merge_rr, 1);
  
                if (reset_head(NULL, "reset", NULL, RESET_HEAD_HARD,
                               NULL, NULL) < 0)
                        die(_("could not discard worktree changes"));
 -              remove_branch_state();
 +              remove_branch_state(the_repository);
                if (read_basic_state(&options))
                        exit(1);
                goto run_rebase;
                options.action = "abort";
                set_reflog_action(&options);
  
 -              rerere_clear(&merge_rr);
 +              rerere_clear(the_repository, &merge_rr);
                string_list_clear(&merge_rr, 1);
  
                if (read_basic_state(&options))
                               NULL, NULL) < 0)
                        die(_("could not move back to %s"),
                            oid_to_hex(&options.orig_head));
 -              remove_branch_state();
 +              remove_branch_state(the_repository);
                ret = finish_rebase(&options);
                goto cleanup;
        }
                }
        }
  
 +      for (i = 0; i < exec.nr; i++)
 +              if (check_exec_cmd(exec.items[i].string))
 +                      exit(1);
 +
        if (!(options.flags & REBASE_NO_QUIET))
                argv_array_push(&options.git_am_opts, "-q");
  
                }
        }
  
 +      if (options.type == REBASE_MERGE)
 +              imply_interactive(&options, "--merge");
 +
        if (options.root && !options.onto_name)
                imply_interactive(&options, "--root without --onto");
  
                break;
        }
  
 +      if (options.reschedule_failed_exec && !is_interactive(&options))
 +              die(_("%s requires an interactive rebase"), "--reschedule-failed-exec");
 +
        if (options.git_am_opts.argc) {
                /* all am options except -q are compatible only with --am */
                for (i = options.git_am_opts.argc - 1; i >= 0; i--)
                                break;
  
                if (is_interactive(&options) && i >= 0)
 -                      die(_("error: cannot combine interactive options "
 -                            "(--interactive, --exec, --rebase-merges, "
 -                            "--preserve-merges, --keep-empty, --root + "
 -                            "--onto) with am options (%s)"), buf.buf);
 -              if (options.type == REBASE_MERGE && i >= 0)
 -                      die(_("error: cannot combine merge options (--merge, "
 -                            "--strategy, --strategy-option) with am options "
 -                            "(%s)"), buf.buf);
 +                      die(_("cannot combine am options with either "
 +                            "interactive or merge options"));
        }
  
        if (options.signoff) {
                options.flags |= REBASE_FORCE;
        }
  
 -      if (options.type == REBASE_PRESERVE_MERGES)
 +      if (options.type == REBASE_PRESERVE_MERGES) {
                /*
                 * Note: incompatibility with --signoff handled in signoff block above
                 * Note: incompatibility with --interactive is just a strong warning;
                 *       git-rebase.txt caveats with "unless you know what you are doing"
                 */
                if (options.rebase_merges)
 -                      die(_("error: cannot combine '--preserve-merges' with "
 +                      die(_("cannot combine '--preserve-merges' with "
                              "'--rebase-merges'"));
  
 +              if (options.reschedule_failed_exec)
 +                      die(_("error: cannot combine '--preserve-merges' with "
 +                            "'--reschedule-failed-exec'"));
 +      }
 +
        if (options.rebase_merges) {
                if (strategy_options.nr)
 -                      die(_("error: cannot combine '--rebase-merges' with "
 +                      die(_("cannot combine '--rebase-merges' with "
                              "'--strategy-option'"));
                if (options.strategy)
 -                      die(_("error: cannot combine '--rebase-merges' with "
 +                      die(_("cannot combine '--rebase-merges' with "
                              "'--strategy'"));
        }
  
                        get_fork_point(options.upstream_name, head);
        }
  
 -      if (read_index(the_repository->index) < 0)
 +      if (repo_read_index(the_repository) < 0)
                die(_("could not read index"));
  
        if (options.autostash) {
                fd = hold_locked_index(&lock_file, 0);
                refresh_cache(REFRESH_QUIET);
                if (0 <= fd)
 -                      update_index_if_able(&the_index, &lock_file);
 +                      repo_update_index_if_able(the_repository, &lock_file);
                rollback_lock_file(&lock_file);
  
 -              if (has_unstaged_changes(1) || has_uncommitted_changes(1)) {
 +              if (has_unstaged_changes(the_repository, 1) ||
 +                  has_uncommitted_changes(the_repository, 1)) {
                        const char *autostash =
                                state_dir_path("autostash", &options);
                        struct child_process stash = CHILD_PROCESS_INIT;
                        putchar('\n');
  
                        if (discard_index(the_repository->index) < 0 ||
 -                              read_index(the_repository->index) < 0)
 +                              repo_read_index(the_repository) < 0)
                                die(_("could not read index"));
                }
        }
  
 -      if (require_clean_work_tree("rebase",
 +      if (require_clean_work_tree(the_repository, "rebase",
                                    _("Please commit or stash them."), 1, 1)) {
                ret = 1;
                goto cleanup;
                                            getenv(GIT_REFLOG_ACTION_ENVIRONMENT),
                                            options.switch_to);
                                if (reset_head(&oid, "checkout",
 -                                             options.head_name, 0,
 +                                             options.head_name,
 +                                             RESET_HEAD_RUN_POST_CHECKOUT_HOOK,
                                               NULL, buf.buf) < 0) {
                                        ret = !!error(_("could not switch to "
                                                        "%s"),
        strbuf_addf(&msg, "%s: checkout %s",
                    getenv(GIT_REFLOG_ACTION_ENVIRONMENT), options.onto_name);
        if (reset_head(&options.onto->object.oid, "checkout", NULL,
-                      RESET_HEAD_DETACH | RESET_HEAD_RUN_POST_CHECKOUT_HOOK,
 -                     RESET_HEAD_DETACH | RESET_ORIG_HEAD, NULL, msg.buf))
++                     RESET_HEAD_DETACH | RESET_ORIG_HEAD | 
++                     RESET_HEAD_RUN_POST_CHECKOUT_HOOK,
 +                     NULL, msg.buf))
                die(_("Could not detach HEAD"));
        strbuf_release(&msg);
  
                strbuf_addf(&msg, "rebase finished: %s onto %s",
                        options.head_name ? options.head_name : "detached HEAD",
                        oid_to_hex(&options.onto->object.oid));
-               reset_head(NULL, "Fast-forwarded", options.head_name, 0,
-                          "HEAD", msg.buf);
+               reset_head(NULL, "Fast-forwarded", options.head_name,
+                          RESET_HEAD_REFS_ONLY, "HEAD", msg.buf);
                strbuf_release(&msg);
                ret = !!finish_rebase(&options);
                goto cleanup;