Merge branch 'jk/ref-filter-flags-cleanup'
authorJunio C Hamano <gitster@pobox.com>
Fri, 17 Mar 2017 20:50:25 +0000 (13:50 -0700)
committerJunio C Hamano <gitster@pobox.com>
Fri, 17 Mar 2017 20:50:25 +0000 (13:50 -0700)
"git tag --contains" used to (ab)use the object bits to keep track
of the state of object reachability without clearing them after
use; this has been cleaned up and made to use the newer commit-slab
facility.

* jk/ref-filter-flags-cleanup:
ref-filter: use separate cache for contains_tag_algo
ref-filter: die on parse_commit errors
ref-filter: use contains_result enum consistently
ref-filter: move ref_cbdata definition into ref-filter.c

1  2 
ref-filter.c
diff --combined ref-filter.c
index 89798dc59ba75ca3bfc7b7207b41834ace22677b,7eeecc608f58776d436fa0b6a85a91b5ac9d9386..9c82b5b9d632bbbe66332b6c54badb5bc88d28d8
@@@ -15,6 -15,7 +15,7 @@@
  #include "version.h"
  #include "trailer.h"
  #include "wt-status.h"
+ #include "commit-slab.h"
  
  static struct ref_msg {
        const char *gone;
@@@ -1297,9 -1298,9 +1298,9 @@@ static void populate_value(struct ref_a
        ref->value = xcalloc(used_atom_cnt, sizeof(struct atom_value));
  
        if (need_symref && (ref->flag & REF_ISSYMREF) && !ref->symref) {
 -              unsigned char unused1[20];
 +              struct object_id unused1;
                ref->symref = resolve_refdup(ref->refname, RESOLVE_REF_READING,
 -                                           unused1, NULL);
 +                                           unused1.hash, NULL);
                if (!ref->symref)
                        ref->symref = "";
        }
@@@ -1470,10 -1471,22 +1471,22 @@@ static void get_ref_atom_value(struct r
        *v = &ref->value[atom];
  }
  
+ /*
+  * Unknown has to be "0" here, because that's the default value for
+  * contains_cache slab entries that have not yet been assigned.
+  */
  enum contains_result {
-       CONTAINS_UNKNOWN = -1,
-       CONTAINS_NO = 0,
-       CONTAINS_YES = 1
+       CONTAINS_UNKNOWN = 0,
+       CONTAINS_NO,
+       CONTAINS_YES
+ };
+ define_commit_slab(contains_cache, enum contains_result);
+ struct ref_filter_cbdata {
+       struct ref_array *array;
+       struct ref_filter *filter;
+       struct contains_cache contains_cache;
  };
  
  /*
@@@ -1504,24 -1517,24 +1517,24 @@@ static int in_commit_list(const struct 
   * Do not recurse to find out, though, but return -1 if inconclusive.
   */
  static enum contains_result contains_test(struct commit *candidate,
-                           const struct commit_list *want)
+                                         const struct commit_list *want,
+                                         struct contains_cache *cache)
  {
-       /* was it previously marked as containing a want commit? */
-       if (candidate->object.flags & TMP_MARK)
-               return 1;
-       /* or marked as not possibly containing a want commit? */
-       if (candidate->object.flags & UNINTERESTING)
-               return 0;
+       enum contains_result *cached = contains_cache_at(cache, candidate);
+       /* If we already have the answer cached, return that. */
+       if (*cached)
+               return *cached;
        /* or are we it? */
        if (in_commit_list(want, candidate)) {
-               candidate->object.flags |= TMP_MARK;
-               return 1;
+               *cached = CONTAINS_YES;
+               return CONTAINS_YES;
        }
  
-       if (parse_commit(candidate) < 0)
-               return 0;
-       return -1;
+       /* Otherwise, we don't know; prepare to recurse */
+       parse_commit_or_die(candidate);
+       return CONTAINS_UNKNOWN;
  }
  
  static void push_to_contains_stack(struct commit *candidate, struct contains_stack *contains_stack)
  }
  
  static enum contains_result contains_tag_algo(struct commit *candidate,
-               const struct commit_list *want)
+                                             const struct commit_list *want,
+                                             struct contains_cache *cache)
  {
        struct contains_stack contains_stack = { 0, 0, NULL };
-       int result = contains_test(candidate, want);
+       enum contains_result result = contains_test(candidate, want, cache);
  
        if (result != CONTAINS_UNKNOWN)
                return result;
                struct commit_list *parents = entry->parents;
  
                if (!parents) {
-                       commit->object.flags |= UNINTERESTING;
+                       *contains_cache_at(cache, commit) = CONTAINS_NO;
                        contains_stack.nr--;
                }
                /*
                 * If we just popped the stack, parents->item has been marked,
-                * therefore contains_test will return a meaningful 0 or 1.
+                * therefore contains_test will return a meaningful yes/no.
                 */
-               else switch (contains_test(parents->item, want)) {
+               else switch (contains_test(parents->item, want, cache)) {
                case CONTAINS_YES:
-                       commit->object.flags |= TMP_MARK;
+                       *contains_cache_at(cache, commit) = CONTAINS_YES;
                        contains_stack.nr--;
                        break;
                case CONTAINS_NO:
                }
        }
        free(contains_stack.contains_stack);
-       return contains_test(candidate, want);
+       return contains_test(candidate, want, cache);
  }
  
- static int commit_contains(struct ref_filter *filter, struct commit *commit)
+ static int commit_contains(struct ref_filter *filter, struct commit *commit,
+                          struct contains_cache *cache)
  {
        if (filter->with_commit_tag_algo)
-               return contains_tag_algo(commit, filter->with_commit);
+               return contains_tag_algo(commit, filter->with_commit, cache) == CONTAINS_YES;
        return is_descendant_of(commit, filter->with_commit);
  }
  
@@@ -1771,7 -1786,7 +1786,7 @@@ static int ref_filter_handler(const cha
                        return 0;
                /* We perform the filtering for the '--contains' option */
                if (filter->with_commit &&
-                   !commit_contains(filter, commit))
+                   !commit_contains(filter, commit, &ref_cbdata->contains_cache))
                        return 0;
        }
  
@@@ -1871,6 -1886,8 +1886,8 @@@ int filter_refs(struct ref_array *array
                broken = 1;
        filter->kind = type & FILTER_REFS_KIND_MASK;
  
+       init_contains_cache(&ref_cbdata.contains_cache);
        /*  Simple per-ref filtering */
        if (!filter->kind)
                die("filter_refs: invalid type");
                        head_ref(ref_filter_handler, &ref_cbdata);
        }
  
+       clear_contains_cache(&ref_cbdata.contains_cache);
  
        /*  Filters that need revision walking */
        if (filter->merge_commit)