Merge branch 'jk/ref-filter-flags-cleanup'
"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
diff --git a/ref-filter.c b/ref-filter.c
index 89798dc..9c82b5b 100644
--- a/ref-filter.c
+++ b/ref-filter.c
@@ -15,6 +15,7 @@
#include "version.h"
#include "trailer.h"
#include "wt-status.h"
+#include "commit-slab.h"
static struct ref_msg {
const char *gone;
@@ -1470,10 +1471,22 @@
*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 @@
* 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)
@@ -1532,10 +1545,11 @@
}
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;
@@ -1547,16 +1561,16 @@
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:
@@ -1568,13 +1582,14 @@
}
}
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 @@
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 @@
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");
@@ -1893,6 +1910,7 @@
head_ref(ref_filter_handler, &ref_cbdata);
}
+ clear_contains_cache(&ref_cbdata.contains_cache);
/* Filters that need revision walking */
if (filter->merge_commit)
diff --git a/ref-filter.h b/ref-filter.h
index 154e24c..e738c5d 100644
--- a/ref-filter.h
+++ b/ref-filter.h
@@ -71,11 +71,6 @@
verbose;
};
-struct ref_filter_cbdata {
- struct ref_array *array;
- struct ref_filter *filter;
-};
-
/* Macros for checking --merged and --no-merged options */
#define _OPT_MERGED_NO_MERGED(option, filter, h) \
{ OPTION_CALLBACK, 0, option, (filter), N_("commit"), (h), \
