ref_iterator: keep track of whether the iterator output is ordered

References are iterated over in order by refname, but reflogs are not. Some consumers of reference iteration care about the difference. Teach each `ref_iterator` to keep track of whether its output is ordered. `overlay_ref_iterator` is one of the picky consumers. Add a sanity check in `overlay_ref_iterator_begin()` to verify that its inputs are ordered. Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu> Signed-off-by: Junio C Hamano <gitster@pobox.com>

Michael Haggerty committed Sep 13, 2017 at 19:15 UTC 8738a8a4df9ee50112b5f5a757c58988166974d3
7 files changed +46 -19
refs.c
+4
@@ -1309,6 +1309,10 @@ struct ref_iterator *refs_ref_iterator_begin(
1309 if (trim)
1310 iter = prefix_ref_iterator_begin(iter, "", trim);
1311
1312 + /* Sanity check for subclasses: */
1313 + if (!iter->ordered)
1314 + BUG("reference iterator is not ordered");
1315 +
1316 return iter;
1317 }
1318
refs/files-backend.c
+9 -7
@@ -762,7 +762,7 @@ static struct ref_iterator *files_ref_iterator_begin(
762 const char *prefix, unsigned int flags)
763 {
764 struct files_ref_store *refs;
765 - struct ref_iterator *loose_iter, *packed_iter;
765 + struct ref_iterator *loose_iter, *packed_iter, *overlay_iter;
766 struct files_ref_iterator *iter;
767 struct ref_iterator *ref_iterator;
768 unsigned int required_flags = REF_STORE_READ;
@@ -772,10 +772,6 @@ static struct ref_iterator *files_ref_iterator_begin(
772
773 refs = files_downcast(ref_store, required_flags, "ref_iterator_begin");
774
775 - iter = xcalloc(1, sizeof(*iter));
776 - ref_iterator = &iter->base;
777 - base_ref_iterator_init(ref_iterator, &files_ref_iterator_vtable);
778 -
775 /*
776 * We must make sure that all loose refs are read before
777 * accessing the packed-refs file; this avoids a race
@@ -811,7 +807,13 @@ static struct ref_iterator *files_ref_iterator_begin(
807 refs->packed_ref_store, prefix, 0,
808 DO_FOR_EACH_INCLUDE_BROKEN);
809
814 - iter->iter0 = overlay_ref_iterator_begin(loose_iter, packed_iter);
810 + overlay_iter = overlay_ref_iterator_begin(loose_iter, packed_iter);
811 +
812 + iter = xcalloc(1, sizeof(*iter));
813 + ref_iterator = &iter->base;
814 + base_ref_iterator_init(ref_iterator, &files_ref_iterator_vtable,
815 + overlay_iter->ordered);
816 + iter->iter0 = overlay_iter;
817 iter->flags = flags;
818
819 return ref_iterator;
@@ -2084,7 +2086,7 @@ static struct ref_iterator *files_reflog_iterator_begin(struct ref_store *ref_st
2086 struct ref_iterator *ref_iterator = &iter->base;
2087 struct strbuf sb = STRBUF_INIT;
2088
2087 - base_ref_iterator_init(ref_iterator, &files_reflog_iterator_vtable);
2089 + base_ref_iterator_init(ref_iterator, &files_reflog_iterator_vtable, 0);
2090 files_reflog_path(refs, &sb, NULL);
2091 iter->dir_iterator = dir_iterator_begin(sb.buf);
2092 iter->ref_store = ref_store;
refs/iterator.c
+10 -5
@@ -25,9 +25,11 @@ int ref_iterator_abort(struct ref_iterator *ref_iterator)
25 }
26
27 void base_ref_iterator_init(struct ref_iterator *iter,
28 - struct ref_iterator_vtable *vtable)
28 + struct ref_iterator_vtable *vtable,
29 + int ordered)
30 {
31 iter->vtable = vtable;
32 + iter->ordered = !!ordered;
33 iter->refname = NULL;
34 iter->oid = NULL;
35 iter->flags = 0;
@@ -72,7 +74,7 @@ struct ref_iterator *empty_ref_iterator_begin(void)
74 struct empty_ref_iterator *iter = xcalloc(1, sizeof(*iter));
75 struct ref_iterator *ref_iterator = &iter->base;
76
75 - base_ref_iterator_init(ref_iterator, &empty_ref_iterator_vtable);
77 + base_ref_iterator_init(ref_iterator, &empty_ref_iterator_vtable, 1);
78 return ref_iterator;
79 }
80
@@ -205,6 +207,7 @@ static struct ref_iterator_vtable merge_ref_iterator_vtable = {
207 };
208
209 struct ref_iterator *merge_ref_iterator_begin(
210 + int ordered,
211 struct ref_iterator *iter0, struct ref_iterator *iter1,
212 ref_iterator_select_fn *select, void *cb_data)
213 {
@@ -219,7 +222,7 @@ struct ref_iterator *merge_ref_iterator_begin(
222 * references through only if they exist in both iterators.
223 */
224
222 - base_ref_iterator_init(ref_iterator, &merge_ref_iterator_vtable);
225 + base_ref_iterator_init(ref_iterator, &merge_ref_iterator_vtable, ordered);
226 iter->iter0 = iter0;
227 iter->iter1 = iter1;
228 iter->select = select;
@@ -268,9 +271,11 @@ struct ref_iterator *overlay_ref_iterator_begin(
271 } else if (is_empty_ref_iterator(back)) {
272 ref_iterator_abort(back);
273 return front;
274 + } else if (!front->ordered || !back->ordered) {
275 + BUG("overlay_ref_iterator requires ordered inputs");
276 }
277
273 - return merge_ref_iterator_begin(front, back,
278 + return merge_ref_iterator_begin(1, front, back,
279 overlay_iterator_select, NULL);
280 }
281
@@ -361,7 +366,7 @@ struct ref_iterator *prefix_ref_iterator_begin(struct ref_iterator *iter0,
366 iter = xcalloc(1, sizeof(*iter));
367 ref_iterator = &iter->base;
368
364 - base_ref_iterator_init(ref_iterator, &prefix_ref_iterator_vtable);
369 + base_ref_iterator_init(ref_iterator, &prefix_ref_iterator_vtable, iter0->ordered);
370
371 iter->iter0 = iter0;
372 iter->prefix = xstrdup(prefix);
refs/packed-backend.c
+1 -1
@@ -437,7 +437,7 @@ static struct ref_iterator *packed_ref_iterator_begin(
437
438 iter = xcalloc(1, sizeof(*iter));
439 ref_iterator = &iter->base;
440 - base_ref_iterator_init(ref_iterator, &packed_ref_iterator_vtable);
440 + base_ref_iterator_init(ref_iterator, &packed_ref_iterator_vtable, 1);
441
442 /*
443 * Note that get_packed_ref_cache() internally checks whether
refs/ref-cache.c
+1 -1
@@ -574,7 +574,7 @@ struct ref_iterator *cache_ref_iterator_begin(struct ref_cache *cache,
574
575 iter = xcalloc(1, sizeof(*iter));
576 ref_iterator = &iter->base;
577 - base_ref_iterator_init(ref_iterator, &cache_ref_iterator_vtable);
577 + base_ref_iterator_init(ref_iterator, &cache_ref_iterator_vtable, 1);
578 ALLOC_GROW(iter->levels, 10, iter->levels_alloc);
579
580 iter->levels_nr = 1;
refs/ref-cache.h
+2 -1
@@ -245,7 +245,8 @@ struct ref_entry *find_ref_entry(struct ref_dir *dir, const char *refname);
245 * Start iterating over references in `cache`. If `prefix` is
246 * specified, only include references whose names start with that
247 * prefix. If `prime_dir` is true, then fill any incomplete
248 - * directories before beginning the iteration.
248 + * directories before beginning the iteration. The output is ordered
249 + * by refname.
250 */
251 struct ref_iterator *cache_ref_iterator_begin(struct ref_cache *cache,
252 const char *prefix,
refs/refs-internal.h
+19 -4
@@ -329,6 +329,13 @@ int refs_rename_ref_available(struct ref_store *refs,
329 */
330 struct ref_iterator {
331 struct ref_iterator_vtable *vtable;
332 +
333 + /*
334 + * Does this `ref_iterator` iterate over references in order
335 + * by refname?
336 + */
337 + unsigned int ordered : 1;
338 +
339 const char *refname;
340 const struct object_id *oid;
341 unsigned int flags;
@@ -374,7 +381,7 @@ int is_empty_ref_iterator(struct ref_iterator *ref_iterator);
381 * which the refname begins with prefix. If trim is non-zero, then
382 * trim that many characters off the beginning of each refname. flags
383 * can be DO_FOR_EACH_INCLUDE_BROKEN to include broken references in
377 - * the iteration.
384 + * the iteration. The output is ordered by refname.
385 */
386 struct ref_iterator *refs_ref_iterator_begin(
387 struct ref_store *refs,
@@ -400,9 +407,11 @@ typedef enum iterator_selection ref_iterator_select_fn(
407 * Iterate over the entries from iter0 and iter1, with the values
408 * interleaved as directed by the select function. The iterator takes
409 * ownership of iter0 and iter1 and frees them when the iteration is
403 - * over.
410 + * over. A derived class should set `ordered` to 1 or 0 based on
411 + * whether it generates its output in order by reference name.
412 */
413 struct ref_iterator *merge_ref_iterator_begin(
414 + int ordered,
415 struct ref_iterator *iter0, struct ref_iterator *iter1,
416 ref_iterator_select_fn *select, void *cb_data);
417
@@ -431,6 +440,8 @@ struct ref_iterator *overlay_ref_iterator_begin(
440 * As an convenience to callers, if prefix is the empty string and
441 * trim is zero, this function returns iter0 directly, without
442 * wrapping it.
443 + *
444 + * The resulting ref_iterator is ordered if iter0 is.
445 */
446 struct ref_iterator *prefix_ref_iterator_begin(struct ref_iterator *iter0,
447 const char *prefix,
@@ -441,11 +452,14 @@ struct ref_iterator *prefix_ref_iterator_begin(struct ref_iterator *iter0,
452 /*
453 * Base class constructor for ref_iterators. Initialize the
454 * ref_iterator part of iter, setting its vtable pointer as specified.
455 + * `ordered` should be set to 1 if the iterator will iterate over
456 + * references in order by refname; otherwise it should be set to 0.
457 * This is meant to be called only by the initializers of derived
458 * classes.
459 */
460 void base_ref_iterator_init(struct ref_iterator *iter,
448 - struct ref_iterator_vtable *vtable);
461 + struct ref_iterator_vtable *vtable,
462 + int ordered);
463
464 /*
465 * Base class destructor for ref_iterators. Destroy the ref_iterator
@@ -564,7 +578,8 @@ typedef int rename_ref_fn(struct ref_store *ref_store,
578 * Iterate over the references in `ref_store` whose names start with
579 * `prefix`. `prefix` is matched as a literal string, without regard
580 * for path separators. If prefix is NULL or the empty string, iterate
567 - * over all references in `ref_store`.
581 + * over all references in `ref_store`. The output is ordered by
582 + * refname.
583 */
584 typedef struct ref_iterator *ref_iterator_begin_fn(
585 struct ref_store *ref_store,