rebase: store orig_head as a commit

Using a struct commit rather than a struct oid to hold orig_head means
that we error out straight away if the branch being rebased does not
point to a commit. It also simplifies the code that handles finding
the merge base and fork point as it no longer has to convert from an
oid to a commit.

To avoid changing the behavior of "git rebase <upstream> <branch>" we
keep the existing call to read_ref() and use lookup_commit_object()
on the oid returned by that rather than calling
lookup_commit_reference_by_name() which applies the ref dwim rules to
its argument.

Helped-by: Junio C Hamano <gitster@pobox.com>
Signed-off-by: Phillip Wood <phillip.wood@dunelm.org.uk>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

diff --git a/builtin/rebase.c b/builtin/rebase.c
index 4139562cdb38a491eb8136c122bd08acb418961e..1df5e2f2eb7a440d5c2386f95ba760d8145269d1 100644 (file)
--- a/builtin/rebase.c
+++ b/builtin/rebase.c
@@ -68,7 +68,7 @@ struct rebase_options {
        const char *upstream_name;
        const char *upstream_arg;
        char *head_name;
-       struct object_id orig_head;
+       struct commit *orig_head;
        struct commit *onto;
        const char *onto_name;
        const char *revisions;
@@ -260,13 +260,13 @@ static int do_interactive_rebase(struct rebase_options *opts, unsigned flags)
        struct replay_opts replay = get_replay_opts(opts);
        struct string_list commands = STRING_LIST_INIT_DUP;
 
-       if (get_revision_ranges(opts->upstream, opts->onto, &opts->orig_head,
+       if (get_revision_ranges(opts->upstream, opts->onto, &opts->orig_head->object.oid,
                                &revisions, &shortrevisions))
                return -1;
 
        if (init_basic_state(&replay,
                             opts->head_name ? opts->head_name : "detached HEAD",
-                            opts->onto, &opts->orig_head)) {
+                            opts->onto, &opts->orig_head->object.oid)) {
                free(revisions);
                free(shortrevisions);
 
@@ -297,8 +297,8 @@ static int do_interactive_rebase(struct rebase_options *opts, unsigned flags)
                split_exec_commands(opts->cmd, &commands);
                ret = complete_action(the_repository, &replay, flags,
                        shortrevisions, opts->onto_name, opts->onto,
-                       &opts->orig_head, &commands, opts->autosquash,
-                       &todo_list);
+                       &opts->orig_head->object.oid, &commands,
+                       opts->autosquash, &todo_list);
        }
 
        string_list_clear(&commands, 0);
@@ -446,7 +446,8 @@ static int read_basic_state(struct rebase_options *opts)
        } else if (!read_oneliner(&buf, state_dir_path("head", opts),
                                  READ_ONELINER_WARN_MISSING))
                return -1;
-       if (get_oid_hex(buf.buf, &opts->orig_head))
+       if (get_oid_hex(buf.buf, &oid) ||
+           !(opts->orig_head = lookup_commit_object(the_repository, &oid)))
                return error(_("invalid orig-head: '%s'"), buf.buf);
 
        if (file_exists(state_dir_path("quiet", opts)))
@@ -515,7 +516,7 @@ static int rebase_write_basic_state(struct rebase_options *opts)
        write_file(state_dir_path("onto", opts), "%s",
                   opts->onto ? oid_to_hex(&opts->onto->object.oid) : "");
        write_file(state_dir_path("orig-head", opts), "%s",
-                  oid_to_hex(&opts->orig_head));
+                  oid_to_hex(&opts->orig_head->object.oid));
        if (!(opts->flags & REBASE_NO_QUIET))
                write_file(state_dir_path("quiet", opts), "%s", "");
        if (opts->flags & REBASE_VERBOSE)
@@ -644,7 +645,7 @@ static int run_am(struct rebase_options *opts)
                               /* this is now equivalent to !opts->upstream */
                               &opts->onto->object.oid :
                               &opts->upstream->object.oid),
-                   oid_to_hex(&opts->orig_head));
+                   oid_to_hex(&opts->orig_head->object.oid));
 
        rebased_patches = xstrdup(git_path("rebased-patches"));
        format_patch.out = open(rebased_patches,
@@ -678,7 +679,7 @@ static int run_am(struct rebase_options *opts)
                free(rebased_patches);
                strvec_clear(&am.args);
 
-               ropts.oid = &opts->orig_head;
+               ropts.oid = &opts->orig_head->object.oid;
                ropts.branch = opts->head_name;
                ropts.default_reflog_action = DEFAULT_REFLOG_ACTION;
                reset_head(the_repository, &ropts);
@@ -826,7 +827,7 @@ static int checkout_up_to_date(struct rebase_options *options)
        strbuf_addf(&buf, "%s: checkout %s",
                    getenv(GIT_REFLOG_ACTION_ENVIRONMENT),
                    options->switch_to);
-       ropts.oid = &options->orig_head;
+       ropts.oid = &options->orig_head->object.oid;
        ropts.branch = options->head_name;
        ropts.flags = RESET_HEAD_RUN_POST_CHECKOUT_HOOK;
        if (!ropts.branch)
@@ -859,15 +860,11 @@ static int is_linear_history(struct commit *from, struct commit *to)
 
 static int can_fast_forward(struct commit *onto, struct commit *upstream,
                            struct commit *restrict_revision,
-                           struct object_id *head_oid, struct object_id *merge_base)
+                           struct commit *head, struct object_id *merge_base)
 {
-       struct commit *head = lookup_commit(the_repository, head_oid);
        struct commit_list *merge_bases = NULL;
        int res = 0;
 
-       if (!head)
-               goto done;
-
        merge_bases = get_merge_bases(onto, head);
        if (!merge_bases || merge_bases->next) {
                oidcpy(merge_base, null_oid());
@@ -1302,13 +1299,13 @@ int cmd_rebase(int argc, const char **argv, const char *prefix)
 
                if (read_basic_state(&options))
                        exit(1);
-               ropts.oid = &options.orig_head;
+               ropts.oid = &options.orig_head->object.oid;
                ropts.branch = options.head_name;
                ropts.flags = RESET_HEAD_HARD;
                ropts.default_reflog_action = DEFAULT_REFLOG_ACTION;
                if (reset_head(the_repository, &ropts) < 0)
                        die(_("could not move back to %s"),
-                           oid_to_hex(&options.orig_head));
+                           oid_to_hex(&options.orig_head->object.oid));
                remove_branch_state(the_repository, 0);
                ret = finish_rebase(&options);
                goto cleanup;
@@ -1594,25 +1591,27 @@ int cmd_rebase(int argc, const char **argv, const char *prefix)
         */
        if (argc == 1) {
                /* Is it "rebase other branchname" or "rebase other commit"? */
+               struct object_id branch_oid;
                branch_name = argv[0];
                options.switch_to = argv[0];
 
                /* Is it a local branch? */
                strbuf_reset(&buf);
                strbuf_addf(&buf, "refs/heads/%s", branch_name);
-               if (!read_ref(buf.buf, &options.orig_head)) {
+               if (!read_ref(buf.buf, &branch_oid)) {
                        die_if_checked_out(buf.buf, 1);
                        options.head_name = xstrdup(buf.buf);
+                       options.orig_head =
+                               lookup_commit_object(the_repository,
+                                                    &branch_oid);
                /* If not is it a valid ref (branch or commit)? */
                } else {
-                       struct commit *commit =
+                       options.orig_head =
                                lookup_commit_reference_by_name(branch_name);
-                       if (!commit)
-                               die(_("no such branch/commit '%s'"),
-                                   branch_name);
-                       oidcpy(&options.orig_head, &commit->object.oid);
                        options.head_name = NULL;
                }
+               if (!options.orig_head)
+                       die(_("no such branch/commit '%s'"), branch_name);
        } else if (argc == 0) {
                /* Do not need to switch branches, we are already on it. */
                options.head_name =
@@ -1629,8 +1628,9 @@ int cmd_rebase(int argc, const char **argv, const char *prefix)
                        FREE_AND_NULL(options.head_name);
                        branch_name = "HEAD";
                }
-               if (get_oid("HEAD", &options.orig_head))
-                       die(_("Could not resolve HEAD to a revision"));
+               options.orig_head = lookup_commit_reference_by_name("HEAD");
+               if (!options.orig_head)
+                       die(_("Could not resolve HEAD to a commit"));
        } else
                BUG("unexpected number of arguments left to parse");
 
@@ -1662,13 +1662,9 @@ int cmd_rebase(int argc, const char **argv, const char *prefix)
                                options.onto_name);
        }
 
-       if (options.fork_point > 0) {
-               struct commit *head =
-                       lookup_commit_reference(the_repository,
-                                               &options.orig_head);
+       if (options.fork_point > 0)
                options.restrict_revision =
-                       get_fork_point(options.upstream_name, head);
-       }
+                       get_fork_point(options.upstream_name, options.orig_head);
 
        if (repo_read_index(the_repository) < 0)
                die(_("could not read index"));
@@ -1698,7 +1694,7 @@ int cmd_rebase(int argc, const char **argv, const char *prefix)
         * call it before checking allow_preemptive_ff.
         */
        if (can_fast_forward(options.onto, options.upstream, options.restrict_revision,
-                   &options.orig_head, &merge_base) &&
+                   options.orig_head, &merge_base) &&
            allow_preemptive_ff) {
                int flag;
 
@@ -1775,7 +1771,7 @@ int cmd_rebase(int argc, const char **argv, const char *prefix)
        strbuf_addf(&msg, "%s: checkout %s",
                    getenv(GIT_REFLOG_ACTION_ENVIRONMENT), options.onto_name);
        ropts.oid = &options.onto->object.oid;
-       ropts.orig_head = &options.orig_head,
+       ropts.orig_head = &options.orig_head->object.oid,
        ropts.flags = RESET_HEAD_DETACH | RESET_ORIG_HEAD |
                        RESET_HEAD_RUN_POST_CHECKOUT_HOOK;
        ropts.head_msg = msg.buf;
@@ -1789,7 +1785,7 @@ int cmd_rebase(int argc, const char **argv, const char *prefix)
         * we just fast-forwarded.
         */
        strbuf_reset(&msg);
-       if (oideq(&merge_base, &options.orig_head)) {
+       if (oideq(&merge_base, &options.orig_head->object.oid)) {
                printf(_("Fast-forwarded %s to %s.\n"),
                        branch_name, options.onto_name);
                strbuf_addf(&msg, "rebase finished: %s onto %s",
@@ -1810,7 +1806,7 @@ int cmd_rebase(int argc, const char **argv, const char *prefix)
                    (options.restrict_revision ?
                     oid_to_hex(&options.restrict_revision->object.oid) :
                     oid_to_hex(&options.upstream->object.oid)),
-                   oid_to_hex(&options.orig_head));
+                   oid_to_hex(&options.orig_head->object.oid));
 
        options.revisions = revisions.buf;