Merge branch 'js/deprecate-grafts'
authorJunio C Hamano <gitster@pobox.com>
Wed, 23 May 2018 05:38:17 +0000 (14:38 +0900)
committerJunio C Hamano <gitster@pobox.com>
Wed, 23 May 2018 05:38:17 +0000 (14:38 +0900)
The functionality of "$GIT_DIR/info/grafts" has been superseded by
the "refs/replace/" mechanism for some time now, but the internal
code had support for it in many places, which has been cleaned up
in order to drop support of the "grafts" mechanism.

* js/deprecate-grafts:
Remove obsolete script to convert grafts to replace refs
technical/shallow: describe why shallow cannot use replace refs
technical/shallow: stop referring to grafts
filter-branch: stop suggesting to use grafts
Deprecate support for .git/info/grafts
Add a test for `git replace --convert-graft-file`
replace: introduce --convert-graft-file
replace: prepare create_graft() for converting graft files wholesale
replace: "libify" create_graft() and callees
replace: avoid using die() to indicate a bug
commit: Let the callback of for_each_mergetag return on error
argv_array: offer to split a string by whitespace

1  2 
advice.c
builtin/replace.c
commit.c
commit.h
log-tree.c
diff --combined advice.c
index 89fda1de55bfc80ba884ab45dd7a31087dea66df,4411704fd45a020a60535917905c8e70dd3d791a..370a56d0546bb3fc7aa4a62203c600e9a8af2d7a
+++ b/advice.c
@@@ -1,6 -1,5 +1,6 @@@
  #include "cache.h"
  #include "config.h"
 +#include "color.h"
  
  int advice_push_update_rejected = 1;
  int advice_push_non_ff_current = 1;
@@@ -20,34 -19,8 +20,35 @@@ int advice_rm_hints = 1
  int advice_add_embedded_repo = 1;
  int advice_ignored_hook = 1;
  int advice_waiting_for_editor = 1;
+ int advice_graft_file_deprecated = 1;
  
 +static int advice_use_color = -1;
 +static char advice_colors[][COLOR_MAXLEN] = {
 +      GIT_COLOR_RESET,
 +      GIT_COLOR_YELLOW,       /* HINT */
 +};
 +
 +enum color_advice {
 +      ADVICE_COLOR_RESET = 0,
 +      ADVICE_COLOR_HINT = 1,
 +};
 +
 +static int parse_advise_color_slot(const char *slot)
 +{
 +      if (!strcasecmp(slot, "reset"))
 +              return ADVICE_COLOR_RESET;
 +      if (!strcasecmp(slot, "hint"))
 +              return ADVICE_COLOR_HINT;
 +      return -1;
 +}
 +
 +static const char *advise_get_color(enum color_advice ix)
 +{
 +      if (want_color_stderr(advice_use_color))
 +              return advice_colors[ix];
 +      return "";
 +}
 +
  static struct {
        const char *name;
        int *preference;
@@@ -70,6 -43,7 +71,7 @@@
        { "addembeddedrepo", &advice_add_embedded_repo },
        { "ignoredhook", &advice_ignored_hook },
        { "waitingforeditor", &advice_waiting_for_editor },
+       { "graftfiledeprecated", &advice_graft_file_deprecated },
  
        /* make this an alias for backward compatibility */
        { "pushnonfastforward", &advice_push_update_rejected }
@@@ -87,10 -61,7 +89,10 @@@ void advise(const char *advice, ...
  
        for (cp = buf.buf; *cp; cp = np) {
                np = strchrnul(cp, '\n');
 -              fprintf(stderr, _("hint: %.*s\n"), (int)(np - cp), cp);
 +              fprintf(stderr, _("%shint: %.*s%s\n"),
 +                      advise_get_color(ADVICE_COLOR_HINT),
 +                      (int)(np - cp), cp,
 +                      advise_get_color(ADVICE_COLOR_RESET));
                if (*np)
                        np++;
        }
  
  int git_default_advice_config(const char *var, const char *value)
  {
 -      const char *k;
 +      const char *k, *slot_name;
        int i;
  
 +      if (!strcmp(var, "color.advice")) {
 +              advice_use_color = git_config_colorbool(var, value);
 +              return 0;
 +      }
 +
 +      if (skip_prefix(var, "color.advice.", &slot_name)) {
 +              int slot = parse_advise_color_slot(slot_name);
 +              if (slot < 0)
 +                      return 0;
 +              if (!value)
 +                      return config_error_nonbool(var);
 +              return color_parse(value, advice_colors[slot]);
 +      }
 +
        if (!skip_prefix(var, "advice.", &k))
                return 0;
  
diff --combined builtin/replace.c
index 14e142d5a80044a6e9565db7fcb7b327387fd128,a87fca045be831c2e14b0c4977d48f205833bace..6da2411e14b9f94f309671e14124ef32d89ef528
  #include "refs.h"
  #include "parse-options.h"
  #include "run-command.h"
 +#include "object-store.h"
 +#include "repository.h"
  #include "tag.h"
  
  static const char * const git_replace_usage[] = {
        N_("git replace [-f] <object> <replacement>"),
        N_("git replace [-f] --edit <object>"),
        N_("git replace [-f] --graft <commit> [<parent>...]"),
+       N_("git replace [-f] --convert-graft-file"),
        N_("git replace -d <object>..."),
        N_("git replace [--format=<format>] [-l [<pattern>]]"),
        NULL
@@@ -55,9 -54,8 +56,9 @@@ static int show_reference(const char *r
                        if (get_oid(refname, &object))
                                return error("Failed to resolve '%s' as a valid ref.", refname);
  
 -                      obj_type = oid_object_info(&object, NULL);
 -                      repl_type = oid_object_info(oid, NULL);
 +                      obj_type = oid_object_info(the_repository, &object,
 +                                                 NULL);
 +                      repl_type = oid_object_info(the_repository, oid, NULL);
  
                        printf("%s (%s) -> %s (%s)\n", refname, type_name(obj_type),
                               oid_to_hex(oid), type_name(repl_type));
@@@ -82,11 -80,11 +83,11 @@@ static int list_replace_refs(const cha
        else if (!strcmp(format, "long"))
                data.format = REPLACE_FORMAT_LONG;
        else
-               die("invalid replace format '%s'\n"
-                   "valid formats are 'short', 'medium' and 'long'\n",
-                   format);
+               return error("invalid replace format '%s'\n"
+                            "valid formats are 'short', 'medium' and 'long'\n",
+                            format);
  
 -      for_each_replace_ref(show_reference, (void *)&data);
 +      for_each_replace_ref(the_repository, show_reference, (void *)&data);
  
        return 0;
  }
@@@ -137,7 -135,7 +138,7 @@@ static int delete_replace_ref(const cha
        return 0;
  }
  
- static void check_ref_valid(struct object_id *object,
+ static int check_ref_valid(struct object_id *object,
                            struct object_id *prev,
                            struct strbuf *ref,
                            int force)
        strbuf_reset(ref);
        strbuf_addf(ref, "%s%s", git_replace_ref_base, oid_to_hex(object));
        if (check_refname_format(ref->buf, 0))
-               die("'%s' is not a valid ref name.", ref->buf);
+               return error("'%s' is not a valid ref name.", ref->buf);
  
        if (read_ref(ref->buf, prev))
                oidclr(prev);
        else if (!force)
-               die("replace ref '%s' already exists", ref->buf);
+               return error("replace ref '%s' already exists", ref->buf);
+       return 0;
  }
  
  static int replace_object_oid(const char *object_ref,
        struct strbuf ref = STRBUF_INIT;
        struct ref_transaction *transaction;
        struct strbuf err = STRBUF_INIT;
+       int res = 0;
  
 -      obj_type = oid_object_info(object, NULL);
 -      repl_type = oid_object_info(repl, NULL);
 +      obj_type = oid_object_info(the_repository, object, NULL);
 +      repl_type = oid_object_info(the_repository, repl, NULL);
        if (!force && obj_type != repl_type)
-               die("Objects must be of the same type.\n"
-                   "'%s' points to a replaced object of type '%s'\n"
-                   "while '%s' points to a replacement object of type '%s'.",
-                   object_ref, type_name(obj_type),
-                   replace_ref, type_name(repl_type));
-       check_ref_valid(object, &prev, &ref, force);
+               return error("Objects must be of the same type.\n"
+                            "'%s' points to a replaced object of type '%s'\n"
+                            "while '%s' points to a replacement object of "
+                            "type '%s'.",
+                            object_ref, type_name(obj_type),
+                            replace_ref, type_name(repl_type));
+       if (check_ref_valid(object, &prev, &ref, force)) {
+               strbuf_release(&ref);
+               return -1;
+       }
  
        transaction = ref_transaction_begin(&err);
        if (!transaction ||
            ref_transaction_update(transaction, ref.buf, repl, &prev,
                                   0, NULL, &err) ||
            ref_transaction_commit(transaction, &err))
-               die("%s", err.buf);
+               res = error("%s", err.buf);
  
        ref_transaction_free(transaction);
        strbuf_release(&ref);
-       return 0;
+       return res;
  }
  
  static int replace_object(const char *object_ref, const char *replace_ref, int force)
        struct object_id object, repl;
  
        if (get_oid(object_ref, &object))
-               die("Failed to resolve '%s' as a valid ref.", object_ref);
+               return error("Failed to resolve '%s' as a valid ref.",
+                            object_ref);
        if (get_oid(replace_ref, &repl))
-               die("Failed to resolve '%s' as a valid ref.", replace_ref);
+               return error("Failed to resolve '%s' as a valid ref.",
+                            replace_ref);
  
        return replace_object_oid(object_ref, &object, replace_ref, &repl, force);
  }
   * If "raw" is true, then the object's raw contents are printed according to
   * "type". Otherwise, we pretty-print the contents for human editing.
   */
- static void export_object(const struct object_id *oid, enum object_type type,
+ static int export_object(const struct object_id *oid, enum object_type type,
                          int raw, const char *filename)
  {
        struct child_process cmd = CHILD_PROCESS_INIT;
  
        fd = open(filename, O_WRONLY | O_CREAT | O_TRUNC, 0666);
        if (fd < 0)
-               die_errno("unable to open %s for writing", filename);
+               return error_errno("unable to open %s for writing", filename);
  
        argv_array_push(&cmd.args, "--no-replace-objects");
        argv_array_push(&cmd.args, "cat-file");
        cmd.out = fd;
  
        if (run_command(&cmd))
-               die("cat-file reported failure");
+               return error("cat-file reported failure");
+       return 0;
  }
  
  /*
   * interpreting it as "type", and writing the result to the object database.
   * The sha1 of the written object is returned via sha1.
   */
- static void import_object(struct object_id *oid, enum object_type type,
+ static int import_object(struct object_id *oid, enum object_type type,
                          int raw, const char *filename)
  {
        int fd;
  
        fd = open(filename, O_RDONLY);
        if (fd < 0)
-               die_errno("unable to open %s for reading", filename);
+               return error_errno("unable to open %s for reading", filename);
  
        if (!raw && type == OBJ_TREE) {
                const char *argv[] = { "mktree", NULL };
                cmd.in = fd;
                cmd.out = -1;
  
-               if (start_command(&cmd))
-                       die("unable to spawn mktree");
+               if (start_command(&cmd)) {
+                       close(fd);
+                       return error("unable to spawn mktree");
+               }
  
-               if (strbuf_read(&result, cmd.out, 41) < 0)
-                       die_errno("unable to read from mktree");
+               if (strbuf_read(&result, cmd.out, 41) < 0) {
+                       error_errno("unable to read from mktree");
+                       close(fd);
+                       close(cmd.out);
+                       return -1;
+               }
                close(cmd.out);
  
-               if (finish_command(&cmd))
-                       die("mktree reported failure");
-               if (get_oid_hex(result.buf, oid) < 0)
-                       die("mktree did not return an object name");
+               if (finish_command(&cmd)) {
+                       strbuf_release(&result);
+                       return error("mktree reported failure");
+               }
+               if (get_oid_hex(result.buf, oid) < 0) {
+                       strbuf_release(&result);
+                       return error("mktree did not return an object name");
+               }
  
                strbuf_release(&result);
        } else {
                struct stat st;
                int flags = HASH_FORMAT_CHECK | HASH_WRITE_OBJECT;
  
-               if (fstat(fd, &st) < 0)
-                       die_errno("unable to fstat %s", filename);
+               if (fstat(fd, &st) < 0) {
+                       error_errno("unable to fstat %s", filename);
+                       close(fd);
+                       return -1;
+               }
                if (index_fd(oid, fd, &st, type, NULL, flags) < 0)
-                       die("unable to write object to database");
+                       return error("unable to write object to database");
                /* index_fd close()s fd for us */
        }
  
         * No need to close(fd) here; both run-command and index-fd
         * will have done it for us.
         */
+       return 0;
  }
  
  static int edit_and_replace(const char *object_ref, int force, int raw)
  {
-       char *tmpfile = git_pathdup("REPLACE_EDITOBJ");
+       char *tmpfile;
        enum object_type type;
        struct object_id old_oid, new_oid, prev;
        struct strbuf ref = STRBUF_INIT;
  
        if (get_oid(object_ref, &old_oid) < 0)
-               die("Not a valid object name: '%s'", object_ref);
+               return error("Not a valid object name: '%s'", object_ref);
  
 -      type = oid_object_info(&old_oid, NULL);
 +      type = oid_object_info(the_repository, &old_oid, NULL);
        if (type < 0)
-               die("unable to get object type for %s", oid_to_hex(&old_oid));
+               return error("unable to get object type for %s",
+                            oid_to_hex(&old_oid));
  
-       check_ref_valid(&old_oid, &prev, &ref, force);
+       if (check_ref_valid(&old_oid, &prev, &ref, force)) {
+               strbuf_release(&ref);
+               return -1;
+       }
        strbuf_release(&ref);
  
-       export_object(&old_oid, type, raw, tmpfile);
-       if (launch_editor(tmpfile, NULL, NULL) < 0)
-               die("editing object file failed");
-       import_object(&new_oid, type, raw, tmpfile);
+       tmpfile = git_pathdup("REPLACE_EDITOBJ");
+       if (export_object(&old_oid, type, raw, tmpfile)) {
+               free(tmpfile);
+               return -1;
+       }
+       if (launch_editor(tmpfile, NULL, NULL) < 0) {
+               free(tmpfile);
+               return error("editing object file failed");
+       }
+       if (import_object(&new_oid, type, raw, tmpfile)) {
+               free(tmpfile);
+               return -1;
+       }
        free(tmpfile);
  
        if (!oidcmp(&old_oid, &new_oid))
        return replace_object_oid(object_ref, &old_oid, "replacement", &new_oid, force);
  }
  
- static void replace_parents(struct strbuf *buf, int argc, const char **argv)
+ static int replace_parents(struct strbuf *buf, int argc, const char **argv)
  {
        struct strbuf new_parents = STRBUF_INIT;
        const char *parent_start, *parent_end;
        /* prepare new parents */
        for (i = 0; i < argc; i++) {
                struct object_id oid;
-               if (get_oid(argv[i], &oid) < 0)
-                       die(_("Not a valid object name: '%s'"), argv[i]);
-               lookup_commit_or_die(&oid, argv[i]);
+               if (get_oid(argv[i], &oid) < 0) {
+                       strbuf_release(&new_parents);
+                       return error(_("Not a valid object name: '%s'"),
+                                    argv[i]);
+               }
+               if (!lookup_commit_reference(&oid)) {
+                       strbuf_release(&new_parents);
+                       return error(_("could not parse %s"), argv[i]);
+               }
                strbuf_addf(&new_parents, "parent %s\n", oid_to_hex(&oid));
        }
  
                      new_parents.buf, new_parents.len);
  
        strbuf_release(&new_parents);
+       return 0;
  }
  
  struct check_mergetag_data {
        const char **argv;
  };
  
- static void check_one_mergetag(struct commit *commit,
+ static int check_one_mergetag(struct commit *commit,
                               struct commit_extra_header *extra,
                               void *data)
  {
        hash_object_file(extra->value, extra->len, type_name(OBJ_TAG), &tag_oid);
        tag = lookup_tag(&tag_oid);
        if (!tag)
-               die(_("bad mergetag in commit '%s'"), ref);
+               return error(_("bad mergetag in commit '%s'"), ref);
        if (parse_tag_buffer(tag, extra->value, extra->len))
-               die(_("malformed mergetag in commit '%s'"), ref);
+               return error(_("malformed mergetag in commit '%s'"), ref);
  
        /* iterate over new parents */
        for (i = 1; i < mergetag_data->argc; i++) {
                struct object_id oid;
                if (get_oid(mergetag_data->argv[i], &oid) < 0)
-                       die(_("Not a valid object name: '%s'"), mergetag_data->argv[i]);
+                       return error(_("Not a valid object name: '%s'"),
+                                    mergetag_data->argv[i]);
                if (!oidcmp(&tag->tagged->oid, &oid))
-                       return; /* found */
+                       return 0; /* found */
        }
  
-       die(_("original commit '%s' contains mergetag '%s' that is discarded; "
-             "use --edit instead of --graft"), ref, oid_to_hex(&tag_oid));
+       return error(_("original commit '%s' contains mergetag '%s' that is "
+                      "discarded; use --edit instead of --graft"), ref,
+                    oid_to_hex(&tag_oid));
  }
  
- static void check_mergetags(struct commit *commit, int argc, const char **argv)
+ static int check_mergetags(struct commit *commit, int argc, const char **argv)
  {
        struct check_mergetag_data mergetag_data;
  
        mergetag_data.argc = argc;
        mergetag_data.argv = argv;
-       for_each_mergetag(check_one_mergetag, commit, &mergetag_data);
+       return for_each_mergetag(check_one_mergetag, commit, &mergetag_data);
  }
  
- static int create_graft(int argc, const char **argv, int force)
+ static int create_graft(int argc, const char **argv, int force, int gentle)
  {
        struct object_id old_oid, new_oid;
        const char *old_ref = argv[0];
        unsigned long size;
  
        if (get_oid(old_ref, &old_oid) < 0)
-               die(_("Not a valid object name: '%s'"), old_ref);
-       commit = lookup_commit_or_die(&old_oid, old_ref);
+               return error(_("Not a valid object name: '%s'"), old_ref);
+       commit = lookup_commit_reference(&old_oid);
+       if (!commit)
+               return error(_("could not parse %s"), old_ref);
  
        buffer = get_commit_buffer(commit, &size);
        strbuf_add(&buf, buffer, size);
        unuse_commit_buffer(commit, buffer);
  
-       replace_parents(&buf, argc - 1, &argv[1]);
+       if (replace_parents(&buf, argc - 1, &argv[1]) < 0) {
+               strbuf_release(&buf);
+               return -1;
+       }
  
        if (remove_signature(&buf)) {
                warning(_("the original commit '%s' has a gpg signature."), old_ref);
                warning(_("the signature will be removed in the replacement commit!"));
        }
  
-       check_mergetags(commit, argc, argv);
+       if (check_mergetags(commit, argc, argv)) {
+               strbuf_release(&buf);
+               return -1;
+       }
  
-       if (write_object_file(buf.buf, buf.len, commit_type, &new_oid))
-               die(_("could not write replacement commit for: '%s'"), old_ref);
+       if (write_object_file(buf.buf, buf.len, commit_type, &new_oid)) {
+               strbuf_release(&buf);
+               return error(_("could not write replacement commit for: '%s'"),
+                            old_ref);
+       }
  
        strbuf_release(&buf);
  
-       if (!oidcmp(&old_oid, &new_oid))
+       if (!oidcmp(&old_oid, &new_oid)) {
+               if (gentle) {
+                       warning("graft for '%s' unnecessary", oid_to_hex(&old_oid));
+                       return 0;
+               }
                return error("new commit is the same as the old one: '%s'", oid_to_hex(&old_oid));
+       }
  
        return replace_object_oid(old_ref, &old_oid, "replacement", &new_oid, force);
  }
  
+ static int convert_graft_file(int force)
+ {
+       const char *graft_file = get_graft_file();
+       FILE *fp = fopen_or_warn(graft_file, "r");
+       struct strbuf buf = STRBUF_INIT, err = STRBUF_INIT;
+       struct argv_array args = ARGV_ARRAY_INIT;
+       if (!fp)
+               return -1;
+       while (strbuf_getline(&buf, fp) != EOF) {
+               if (*buf.buf == '#')
+                       continue;
+               argv_array_split(&args, buf.buf);
+               if (args.argc && create_graft(args.argc, args.argv, force, 1))
+                       strbuf_addf(&err, "\n\t%s", buf.buf);
+               argv_array_clear(&args);
+       }
+       fclose(fp);
+       strbuf_release(&buf);
+       if (!err.len)
+               return unlink_or_warn(graft_file);
+       warning(_("could not convert the following graft(s):\n%s"), err.buf);
+       strbuf_release(&err);
+       return -1;
+ }
  int cmd_replace(int argc, const char **argv, const char *prefix)
  {
        int force = 0;
                MODE_DELETE,
                MODE_EDIT,
                MODE_GRAFT,
+               MODE_CONVERT_GRAFT_FILE,
                MODE_REPLACE
        } cmdmode = MODE_UNSPECIFIED;
        struct option options[] = {
                OPT_CMDMODE('d', "delete", &cmdmode, N_("delete replace refs"), MODE_DELETE),
                OPT_CMDMODE('e', "edit", &cmdmode, N_("edit existing object"), MODE_EDIT),
                OPT_CMDMODE('g', "graft", &cmdmode, N_("change a commit's parents"), MODE_GRAFT),
+               OPT_CMDMODE(0, "convert-graft-file", &cmdmode, N_("convert existing graft file"), MODE_CONVERT_GRAFT_FILE),
                OPT_BOOL_F('f', "force", &force, N_("replace the ref if it exists"),
                           PARSE_OPT_NOCOMPLETE),
                OPT_BOOL(0, "raw", &raw, N_("do not pretty-print contents for --edit")),
        if (force &&
            cmdmode != MODE_REPLACE &&
            cmdmode != MODE_EDIT &&
-           cmdmode != MODE_GRAFT)
+           cmdmode != MODE_GRAFT &&
+           cmdmode != MODE_CONVERT_GRAFT_FILE)
                usage_msg_opt("-f only makes sense when writing a replacement",
                              git_replace_usage, options);
  
                if (argc < 1)
                        usage_msg_opt("-g needs at least one argument",
                                      git_replace_usage, options);
-               return create_graft(argc, argv, force);
+               return create_graft(argc, argv, force, 0);
+       case MODE_CONVERT_GRAFT_FILE:
+               if (argc != 0)
+                       usage_msg_opt("--convert-graft-file takes no argument",
+                                     git_replace_usage, options);
+               return !!convert_graft_file(force);
  
        case MODE_LIST:
                if (argc > 1)
                return list_replace_refs(argv[0], format);
  
        default:
-               die("BUG: invalid cmdmode %d", (int)cmdmode);
+               BUG("invalid cmdmode %d", (int)cmdmode);
        }
  }
diff --combined commit.c
index 1d7622b2ccaea0e30edada1f271f7ca3a694db2d,451d3ce8dfec5ca085c557beca4a0299e8e18606..f9714ed74ccdccba4d9b5f00bc8ac31cb022dd02
+++ b/commit.c
@@@ -1,7 -1,6 +1,7 @@@
  #include "cache.h"
  #include "tag.h"
  #include "commit.h"
 +#include "commit-graph.h"
  #include "pkt-line.h"
  #include "utf8.h"
  #include "diff.h"
@@@ -13,6 -12,7 +13,7 @@@
  #include "prio-queue.h"
  #include "sha1-lookup.h"
  #include "wt-status.h"
+ #include "advice.h"
  
  static struct commit_extra_header *read_commit_extra_header_lines(const char *buf, size_t len, const char **);
  
@@@ -177,6 -177,15 +178,15 @@@ static int read_graft_file(const char *
        struct strbuf buf = STRBUF_INIT;
        if (!fp)
                return -1;
+       if (advice_graft_file_deprecated)
+               advise(_("Support for <GIT_DIR>/info/grafts is deprecated\n"
+                        "and will be removed in a future Git version.\n"
+                        "\n"
+                        "Please use \"git replace --convert-graft-file\"\n"
+                        "to convert the grafts into replace refs.\n"
+                        "\n"
+                        "Turn this message off by running\n"
+                        "\"git config advice.graftFileDeprecated false\""));
        while (!strbuf_getwholeline(&buf, fp, '\n')) {
                /* The format is just "Commit Parent1 Parent2 ...\n" */
                struct commit_graft *graft = read_graft_line(&buf);
@@@ -296,22 -305,6 +306,22 @@@ void free_commit_buffer(struct commit *
        }
  }
  
 +struct tree *get_commit_tree(const struct commit *commit)
 +{
 +      if (commit->maybe_tree || !commit->object.parsed)
 +              return commit->maybe_tree;
 +
 +      if (commit->graph_pos == COMMIT_NOT_FROM_GRAPH)
 +              BUG("commit has NULL tree, but was not loaded from commit-graph");
 +
 +      return get_commit_tree_in_graph(commit);
 +}
 +
 +struct object_id *get_commit_tree_oid(const struct commit *commit)
 +{
 +      return &get_commit_tree(commit)->object.oid;
 +}
 +
  const void *detach_commit_buffer(struct commit *commit, unsigned long *sizep)
  {
        struct commit_buffer *v = buffer_slab_peek(&buffer_slab, commit);
@@@ -351,7 -344,7 +361,7 @@@ int parse_commit_buffer(struct commit *
        if (get_sha1_hex(bufptr + 5, parent.hash) < 0)
                return error("bad tree pointer in commit %s",
                             oid_to_hex(&item->object.oid));
 -      item->tree = lookup_tree(&parent);
 +      item->maybe_tree = lookup_tree(&parent);
        bufptr += tree_entry_len + 1; /* "tree " + "hex sha1" + "\n" */
        pptr = &item->parents;
  
@@@ -400,8 -393,6 +410,8 @@@ int parse_commit_gently(struct commit *
                return -1;
        if (item->object.parsed)
                return 0;
 +      if (parse_commit_in_graph(item))
 +              return 0;
        buffer = read_object_file(&item->object.oid, &type, &size);
        if (!buffer)
                return quiet_on_missing ? -1 :
@@@ -1307,17 -1298,19 +1317,19 @@@ struct commit_extra_header *read_commit
        return extra;
  }
  
void for_each_mergetag(each_mergetag_fn fn, struct commit *commit, void *data)
int for_each_mergetag(each_mergetag_fn fn, struct commit *commit, void *data)
  {
        struct commit_extra_header *extra, *to_free;
+       int res = 0;
  
        to_free = read_commit_extra_headers(commit, NULL);
-       for (extra = to_free; extra; extra = extra->next) {
+       for (extra = to_free; !res && extra; extra = extra->next) {
                if (strcmp(extra->key, "mergetag"))
                        continue; /* not a merge tag */
-               fn(commit, extra, data);
+               res = fn(commit, extra, data);
        }
        free_commit_extra_headers(to_free);
+       return res;
  }
  
  static inline int standard_header_field(const char *field, size_t len)
diff --combined commit.h
index 23a3f364edec6b609a39f15518cfda06af69e487,9000895ad912e4fac1aee792429c256f792edc8f..10e34e1a18b726b0168a9fe2e4937a23d6cefd11
+++ b/commit.h
@@@ -9,8 -9,6 +9,8 @@@
  #include "string-list.h"
  #include "pretty.h"
  
 +#define COMMIT_NOT_FROM_GRAPH 0xFFFFFFFF
 +
  struct commit_list {
        struct commit *item;
        struct commit_list *next;
@@@ -22,14 -20,7 +22,14 @@@ struct commit 
        unsigned int index;
        timestamp_t date;
        struct commit_list *parents;
 -      struct tree *tree;
 +
 +      /*
 +       * If the commit is loaded from the commit-graph file, then this
 +       * member may be NULL. Only access it through get_commit_tree()
 +       * or get_commit_tree_oid().
 +       */
 +      struct tree *maybe_tree;
 +      uint32_t graph_pos;
  };
  
  extern int save_commit_buffer;
@@@ -108,9 -99,6 +108,9 @@@ void unuse_commit_buffer(const struct c
   */
  void free_commit_buffer(struct commit *);
  
 +struct tree *get_commit_tree(const struct commit *);
 +struct object_id *get_commit_tree_oid(const struct commit *);
 +
  /*
   * Disassociate any cached object buffer from the commit, but do not free it.
   * The buffer (or NULL, if none) is returned.
@@@ -303,10 -291,10 +303,10 @@@ extern const char *find_commit_header(c
  /* Find the end of the log message, the right place for a new trailer. */
  extern int ignore_non_trailer(const char *buf, size_t len);
  
- typedef void (*each_mergetag_fn)(struct commit *commit, struct commit_extra_header *extra,
+ typedef int (*each_mergetag_fn)(struct commit *commit, struct commit_extra_header *extra,
                                 void *cb_data);
  
- extern void for_each_mergetag(each_mergetag_fn fn, struct commit *commit, void *data);
+ extern int for_each_mergetag(each_mergetag_fn fn, struct commit *commit, void *data);
  
  struct merge_remote_desc {
        struct object *obj; /* the named object, could be a tag */
diff --combined log-tree.c
index 2e6b486140f8e8591bd0c7564bcb4d4d507c330f,f3a51a6e72678add0816ef4604a7d70d7b62328e..66329d089d8521b8335cfc373f127b16c4f860ed
@@@ -488,9 -488,9 +488,9 @@@ static int is_common_merge(const struc
                && !commit->parents->next->next);
  }
  
- static void show_one_mergetag(struct commit *commit,
-                             struct commit_extra_header *extra,
-                             void *data)
+ static int show_one_mergetag(struct commit *commit,
+                            struct commit_extra_header *extra,
+                            void *data)
  {
        struct rev_info *opt = (struct rev_info *)data;
        struct object_id oid;
        hash_object_file(extra->value, extra->len, type_name(OBJ_TAG), &oid);
        tag = lookup_tag(&oid);
        if (!tag)
-               return; /* error message already given */
+               return -1; /* error message already given */
  
        strbuf_init(&verify_message, 256);
        if (parse_tag_buffer(tag, extra->value, extra->len))
  
        show_sig_lines(opt, status, verify_message.buf);
        strbuf_release(&verify_message);
+       return 0;
  }
  
- static void show_mergetag(struct rev_info *opt, struct commit *commit)
+ static int show_mergetag(struct rev_info *opt, struct commit *commit)
  {
-       for_each_mergetag(show_one_mergetag, commit, opt);
+       return for_each_mergetag(show_one_mergetag, commit, opt);
  }
  
  void show_log(struct rev_info *opt)
@@@ -806,7 -807,7 +807,7 @@@ static int log_tree_diff(struct rev_inf
                return 0;
  
        parse_commit_or_die(commit);
 -      oid = &commit->tree->object.oid;
 +      oid = get_commit_tree_oid(commit);
  
        /* Root commit? */
        parents = get_saved_parents(opt, commit);
                         * we merged _in_.
                         */
                        parse_commit_or_die(parents->item);
 -                      diff_tree_oid(&parents->item->tree->object.oid,
 +                      diff_tree_oid(get_commit_tree_oid(parents->item),
                                      oid, "", &opt->diffopt);
                        log_tree_diff_flush(opt);
                        return !opt->loginfo;
                struct commit *parent = parents->item;
  
                parse_commit_or_die(parent);
 -              diff_tree_oid(&parent->tree->object.oid,
 +              diff_tree_oid(get_commit_tree_oid(parent),
                              oid, "", &opt->diffopt);
                log_tree_diff_flush(opt);