Merge branch 'jk/unavailable-can-be-missing'
authorJunio C Hamano <gitster@pobox.com>
Wed, 30 May 2018 05:04:07 +0000 (14:04 +0900)
committerJunio C Hamano <gitster@pobox.com>
Wed, 30 May 2018 05:04:08 +0000 (14:04 +0900)
Code clean-up to turn history traversal more robust in a
semi-corrupt repository.

* jk/unavailable-can-be-missing:
mark_parents_uninteresting(): avoid most allocation
mark_parents_uninteresting(): replace list with stack
mark_parents_uninteresting(): drop missing object check
mark_tree_contents_uninteresting(): drop missing object check

1  2 
revision.c
diff --combined revision.c
index dc756918e89ba5f6114f716881dbe46166e407bc,c21acb2cb8144cb0c4f642c3556c7d58f5901e62..5112c9af9b0784a812ba1c3312acfc254602335e
@@@ -6,7 -6,6 +6,7 @@@
  #include "diff.h"
  #include "refs.h"
  #include "revision.h"
 +#include "repository.h"
  #include "graph.h"
  #include "grep.h"
  #include "reflog-walk.h"
@@@ -52,12 -51,9 +52,9 @@@ static void mark_tree_contents_unintere
  {
        struct tree_desc desc;
        struct name_entry entry;
-       struct object *obj = &tree->object;
  
-       if (!has_object_file(&obj->oid))
+       if (parse_tree_gently(tree, 1) < 0)
                return;
-       if (parse_tree(tree) < 0)
-               die("bad tree %s", oid_to_hex(&obj->oid));
  
        init_tree_desc(&desc, tree->buffer, tree->size);
        while (tree_entry(&desc, &entry)) {
@@@ -95,50 -91,63 +92,63 @@@ void mark_tree_uninteresting(struct tre
        mark_tree_contents_uninteresting(tree);
  }
  
+ struct commit_stack {
+       struct commit **items;
+       size_t nr, alloc;
+ };
+ #define COMMIT_STACK_INIT { NULL, 0, 0 }
+ static void commit_stack_push(struct commit_stack *stack, struct commit *commit)
+ {
+       ALLOC_GROW(stack->items, stack->nr + 1, stack->alloc);
+       stack->items[stack->nr++] = commit;
+ }
+ static struct commit *commit_stack_pop(struct commit_stack *stack)
+ {
+       return stack->nr ? stack->items[--stack->nr] : NULL;
+ }
+ static void commit_stack_clear(struct commit_stack *stack)
+ {
+       FREE_AND_NULL(stack->items);
+       stack->nr = stack->alloc = 0;
+ }
+ static void mark_one_parent_uninteresting(struct commit *commit,
+                                         struct commit_stack *pending)
+ {
+       struct commit_list *l;
+       if (commit->object.flags & UNINTERESTING)
+               return;
+       commit->object.flags |= UNINTERESTING;
+       /*
+        * Normally we haven't parsed the parent
+        * yet, so we won't have a parent of a parent
+        * here. However, it may turn out that we've
+        * reached this commit some other way (where it
+        * wasn't uninteresting), in which case we need
+        * to mark its parents recursively too..
+        */
+       for (l = commit->parents; l; l = l->next)
+               commit_stack_push(pending, l->item);
+ }
  void mark_parents_uninteresting(struct commit *commit)
  {
-       struct commit_list *parents = NULL, *l;
+       struct commit_stack pending = COMMIT_STACK_INIT;
+       struct commit_list *l;
  
        for (l = commit->parents; l; l = l->next)
-               commit_list_insert(l->item, &parents);
-       while (parents) {
-               struct commit *commit = pop_commit(&parents);
-               while (commit) {
-                       /*
-                        * A missing commit is ok iff its parent is marked
-                        * uninteresting.
-                        *
-                        * We just mark such a thing parsed, so that when
-                        * it is popped next time around, we won't be trying
-                        * to parse it and get an error.
-                        */
-                       if (!commit->object.parsed &&
-                           !has_object_file(&commit->object.oid))
-                               commit->object.parsed = 1;
-                       if (commit->object.flags & UNINTERESTING)
-                               break;
-                       commit->object.flags |= UNINTERESTING;
-                       /*
-                        * Normally we haven't parsed the parent
-                        * yet, so we won't have a parent of a parent
-                        * here. However, it may turn out that we've
-                        * reached this commit some other way (where it
-                        * wasn't uninteresting), in which case we need
-                        * to mark its parents recursively too..
-                        */
-                       if (!commit->parents)
-                               break;
-                       for (l = commit->parents->next; l; l = l->next)
-                               commit_list_insert(l->item, &parents);
-                       commit = commit->parents->item;
-               }
-       }
+               mark_one_parent_uninteresting(l->item, &pending);
+       while (pending.nr > 0)
+               mark_one_parent_uninteresting(commit_stack_pop(&pending),
+                                             &pending);
+       commit_stack_clear(&pending);
  }
  
  static void add_pending_object_with_path(struct rev_info *revs,
@@@ -441,8 -450,8 +451,8 @@@ static void file_change(struct diff_opt
  static int rev_compare_tree(struct rev_info *revs,
                            struct commit *parent, struct commit *commit)
  {
 -      struct tree *t1 = parent->tree;
 -      struct tree *t2 = commit->tree;
 +      struct tree *t1 = get_commit_tree(parent);
 +      struct tree *t2 = get_commit_tree(commit);
  
        if (!t1)
                return REV_TREE_NEW;
  static int rev_same_tree_as_empty(struct rev_info *revs, struct commit *commit)
  {
        int retval;
 -      struct tree *t1 = commit->tree;
 +      struct tree *t1 = get_commit_tree(commit);
  
        if (!t1)
                return 0;
@@@ -616,7 -625,7 +626,7 @@@ static void try_to_simplify_commit(stru
        if (!revs->prune)
                return;
  
 -      if (!commit->tree)
 +      if (!get_commit_tree(commit))
                return;
  
        if (!commit->parents) {
@@@ -1286,7 -1295,7 +1296,7 @@@ void add_reflogs_to_pending(struct rev_
  
        cb.all_revs = revs;
        cb.all_flags = flags;
 -      cb.refs = get_main_ref_store();
 +      cb.refs = get_main_ref_store(the_repository);
        for_each_reflog(handle_one_reflog, &cb);
  
        if (!revs->single_worktree)
@@@ -2108,7 -2117,7 +2118,7 @@@ static int handle_revision_opt(struct r
                revs->ignore_missing = 1;
        } else if (!strcmp(arg, "--exclude-promisor-objects")) {
                if (fetch_if_missing)
 -                      die("BUG: exclude_promisor_objects can only be used when fetch_if_missing is 0");
 +                      BUG("exclude_promisor_objects can only be used when fetch_if_missing is 0");
                revs->exclude_promisor_objects = 1;
        } else {
                int opts = diff_opt_parse(&revs->diffopt, argv, argc, revs->prefix);
@@@ -2174,10 -2183,10 +2184,10 @@@ static int handle_revision_pseudo_opt(c
                 * supported right now, so stick to single worktree.
                 */
                if (!revs->single_worktree)
 -                      die("BUG: --single-worktree cannot be used together with submodule");
 +                      BUG("--single-worktree cannot be used together with submodule");
                refs = get_submodule_ref_store(submodule);
        } else
 -              refs = get_main_ref_store();
 +              refs = get_main_ref_store(the_repository);
  
        /*
         * NOTE!