git/list[1] front-page[2] threads[3] people[4] search[5] about
 

[PATCH v2 4/7] refs: always treat iterators as ordered

From
Patrick Steinhardt <ps@pks.im>
Date
Feb 20, 2024, 09:06 UTC
Message-ID
<4254f23fd40076857ec093365a8adbb860803a72.1708418805.git.ps@pks.im>
In-Reply-To
<cover.1708418805.git.ps@pks.im>

In the preceding commit we have converted the reflog iterator of the "files" backend to be ordered, which was the only remaining ref iterator that wasn't ordered. Refactor the ref iterator infrastructure so that we always assume iterators to be ordered, thus simplifying the code.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
 refs.c                  |  4 ----
 refs/debug.c            |  3 +--
 refs/files-backend.c    |  7 +++----
 refs/iterator.c         | 26 ++++++++------------------
 refs/packed-backend.c   |  2 +-
 refs/ref-cache.c        |  2 +-
 refs/refs-internal.h    | 18 ++----------------
 refs/reftable-backend.c |  8 ++++----
 8 files changed, 20 insertions(+), 50 deletions(-)
diff --git a/refs.c b/refs.c
index fff343c256..dc25606a82 100644
--- a/refs.c
+++ b/refs.c
@@ -1594,10 +1594,6 @@ struct ref_iterator *refs_ref_iterator_begin(
 	if (trim)
 		iter = prefix_ref_iterator_begin(iter, "", trim);
 
-	/* Sanity check for subclasses: */
-	if (!iter->ordered)
-		BUG("reference iterator is not ordered");
-
 	return iter;
 }
 
diff --git a/refs/debug.c b/refs/debug.c
index 634681ca44..c7531b17f0 100644
--- a/refs/debug.c
+++ b/refs/debug.c
@@ -181,7 +181,6 @@ static int debug_ref_iterator_advance(struct ref_iterator *ref_iterator)
 		trace_printf_key(&trace_refs, "iterator_advance: %s (0)\n",
 			diter->iter->refname);
 
-	diter->base.ordered = diter->iter->ordered;
 	diter->base.refname = diter->iter->refname;
 	diter->base.oid = diter->iter->oid;
 	diter->base.flags = diter->iter->flags;
@@ -222,7 +221,7 @@ debug_ref_iterator_begin(struct ref_store *ref_store, const char *prefix,
 		drefs->refs->be->iterator_begin(drefs->refs, prefix,
 						exclude_patterns, flags);
 	struct debug_ref_iterator *diter = xcalloc(1, sizeof(*diter));
-	base_ref_iterator_init(&diter->base, &debug_ref_iterator_vtable, 1);
+	base_ref_iterator_init(&diter->base, &debug_ref_iterator_vtable);
 	diter->iter = res;
 	trace_printf_key(&trace_refs, "ref_iterator_begin: \"%s\" (0x%x)\n",
 			 prefix, flags);
diff --git a/refs/files-backend.c b/refs/files-backend.c
index a7b7cdef36..51d57d98d2 100644
--- a/refs/files-backend.c
+++ b/refs/files-backend.c
@@ -879,8 +879,7 @@ static struct ref_iterator *files_ref_iterator_begin(
 
 	CALLOC_ARRAY(iter, 1);
 	ref_iterator = &iter->base;
-	base_ref_iterator_init(ref_iterator, &files_ref_iterator_vtable,
-			       overlay_iter->ordered);
+	base_ref_iterator_init(ref_iterator, &files_ref_iterator_vtable);
 	iter->iter0 = overlay_iter;
 	iter->repo = ref_store->repo;
 	iter->flags = flags;
@@ -2202,7 +2201,7 @@ static struct ref_iterator *reflog_iterator_begin(struct ref_store *ref_store,
 	CALLOC_ARRAY(iter, 1);
 	ref_iterator = &iter->base;
 
-	base_ref_iterator_init(ref_iterator, &files_reflog_iterator_vtable, 1);
+	base_ref_iterator_init(ref_iterator, &files_reflog_iterator_vtable);
 	iter->dir_iterator = diter;
 	iter->ref_store = ref_store;
 	strbuf_release(&sb);
@@ -2246,7 +2245,7 @@ static struct ref_iterator *files_reflog_iterator_begin(struct ref_store *ref_st
 		return reflog_iterator_begin(ref_store, refs->gitcommondir);
 	} else {
 		return merge_ref_iterator_begin(
-			1, reflog_iterator_begin(ref_store, refs->base.gitdir),
+			reflog_iterator_begin(ref_store, refs->base.gitdir),
 			reflog_iterator_begin(ref_store, refs->gitcommondir),
 			reflog_iterator_select, refs);
 	}
diff --git a/refs/iterator.c b/refs/iterator.c
index 6b680f610e..f9a9a808e0 100644
--- a/refs/iterator.c
+++ b/refs/iterator.c
@@ -25,11 +25,9 @@ int ref_iterator_abort(struct ref_iterator *ref_iterator)
 }
 
 void base_ref_iterator_init(struct ref_iterator *iter,
-			    struct ref_iterator_vtable *vtable,
-			    int ordered)
+			    struct ref_iterator_vtable *vtable)
 {
 	iter->vtable = vtable;
-	iter->ordered = !!ordered;
 	iter->refname = NULL;
 	iter->oid = NULL;
 	iter->flags = 0;
@@ -74,7 +72,7 @@ struct ref_iterator *empty_ref_iterator_begin(void)
 	struct empty_ref_iterator *iter = xcalloc(1, sizeof(*iter));
 	struct ref_iterator *ref_iterator = &iter->base;
 
-	base_ref_iterator_init(ref_iterator, &empty_ref_iterator_vtable, 1);
+	base_ref_iterator_init(ref_iterator, &empty_ref_iterator_vtable);
 	return ref_iterator;
 }
 
@@ -207,7 +205,6 @@ static struct ref_iterator_vtable merge_ref_iterator_vtable = {
 };
 
 struct ref_iterator *merge_ref_iterator_begin(
-		int ordered,
 		struct ref_iterator *iter0, struct ref_iterator *iter1,
 		ref_iterator_select_fn *select, void *cb_data)
 {
@@ -222,7 +219,7 @@ struct ref_iterator *merge_ref_iterator_begin(
 	 * references through only if they exist in both iterators.
 	 */
 
-	base_ref_iterator_init(ref_iterator, &merge_ref_iterator_vtable, ordered);
+	base_ref_iterator_init(ref_iterator, &merge_ref_iterator_vtable);
 	iter->iter0 = iter0;
 	iter->iter1 = iter1;
 	iter->select = select;
@@ -271,12 +268,9 @@ struct ref_iterator *overlay_ref_iterator_begin(
 	} else if (is_empty_ref_iterator(back)) {
 		ref_iterator_abort(back);
 		return front;
-	} else if (!front->ordered || !back->ordered) {
-		BUG("overlay_ref_iterator requires ordered inputs");
 	}
 
-	return merge_ref_iterator_begin(1, front, back,
-					overlay_iterator_select, NULL);
+	return merge_ref_iterator_begin(front, back, overlay_iterator_select, NULL);
 }
 
 struct prefix_ref_iterator {
@@ -315,16 +309,12 @@ static int prefix_ref_iterator_advance(struct ref_iterator *ref_iterator)
 
 		if (cmp > 0) {
 			/*
-			 * If the source iterator is ordered, then we
+			 * As the source iterator is ordered, we
 			 * can stop the iteration as soon as we see a
 			 * refname that comes after the prefix:
 			 */
-			if (iter->iter0->ordered) {
-				ok = ref_iterator_abort(iter->iter0);
-				break;
-			} else {
-				continue;
-			}
+			ok = ref_iterator_abort(iter->iter0);
+			break;
 		}
 
 		if (iter->trim) {
@@ -396,7 +386,7 @@ struct ref_iterator *prefix_ref_iterator_begin(struct ref_iterator *iter0,
 	CALLOC_ARRAY(iter, 1);
 	ref_iterator = &iter->base;
 
-	base_ref_iterator_init(ref_iterator, &prefix_ref_iterator_vtable, iter0->ordered);
+	base_ref_iterator_init(ref_iterator, &prefix_ref_iterator_vtable);
 
 	iter->iter0 = iter0;
 	iter->prefix = xstrdup(prefix);
diff --git a/refs/packed-backend.c b/refs/packed-backend.c
index a499a91c7e..4e826c05ff 100644
--- a/refs/packed-backend.c
+++ b/refs/packed-backend.c
@@ -1111,7 +1111,7 @@ static struct ref_iterator *packed_ref_iterator_begin(
 
 	CALLOC_ARRAY(iter, 1);
 	ref_iterator = &iter->base;
-	base_ref_iterator_init(ref_iterator, &packed_ref_iterator_vtable, 1);
+	base_ref_iterator_init(ref_iterator, &packed_ref_iterator_vtable);
 
 	if (exclude_patterns)
 		populate_excluded_jump_list(iter, snapshot, exclude_patterns);
diff --git a/refs/ref-cache.c b/refs/ref-cache.c
index a372a00941..9f9797209a 100644
--- a/refs/ref-cache.c
+++ b/refs/ref-cache.c
@@ -486,7 +486,7 @@ struct ref_iterator *cache_ref_iterator_begin(struct ref_cache *cache,
 
 	CALLOC_ARRAY(iter, 1);
 	ref_iterator = &iter->base;
-	base_ref_iterator_init(ref_iterator, &cache_ref_iterator_vtable, 1);
+	base_ref_iterator_init(ref_iterator, &cache_ref_iterator_vtable);
 	ALLOC_GROW(iter->levels, 10, iter->levels_alloc);
 
 	iter->levels_nr = 1;
diff --git a/refs/refs-internal.h b/refs/refs-internal.h
index 83e0f0bba3..1e8a9f9f13 100644
--- a/refs/refs-internal.h
+++ b/refs/refs-internal.h
@@ -312,13 +312,6 @@ enum do_for_each_ref_flags {
  */
 struct ref_iterator {
 	struct ref_iterator_vtable *vtable;
-
-	/*
-	 * Does this `ref_iterator` iterate over references in order
-	 * by refname?
-	 */
-	unsigned int ordered : 1;
-
 	const char *refname;
 	const struct object_id *oid;
 	unsigned int flags;
@@ -390,11 +383,9 @@ typedef enum iterator_selection ref_iterator_select_fn(
  * Iterate over the entries from iter0 and iter1, with the values
  * interleaved as directed by the select function. The iterator takes
  * ownership of iter0 and iter1 and frees them when the iteration is
- * over. A derived class should set `ordered` to 1 or 0 based on
- * whether it generates its output in order by reference name.
+ * over.
  */
 struct ref_iterator *merge_ref_iterator_begin(
-		int ordered,
 		struct ref_iterator *iter0, struct ref_iterator *iter1,
 		ref_iterator_select_fn *select, void *cb_data);
 
@@ -423,8 +414,6 @@ struct ref_iterator *overlay_ref_iterator_begin(
  * As an convenience to callers, if prefix is the empty string and
  * trim is zero, this function returns iter0 directly, without
  * wrapping it.
- *
- * The resulting ref_iterator is ordered if iter0 is.
  */
 struct ref_iterator *prefix_ref_iterator_begin(struct ref_iterator *iter0,
 					       const char *prefix,
@@ -435,14 +424,11 @@ struct ref_iterator *prefix_ref_iterator_begin(struct ref_iterator *iter0,
 /*
  * Base class constructor for ref_iterators. Initialize the
  * ref_iterator part of iter, setting its vtable pointer as specified.
- * `ordered` should be set to 1 if the iterator will iterate over
- * references in order by refname; otherwise it should be set to 0.
  * This is meant to be called only by the initializers of derived
  * classes.
  */
 void base_ref_iterator_init(struct ref_iterator *iter,
-			    struct ref_iterator_vtable *vtable,
-			    int ordered);
+			    struct ref_iterator_vtable *vtable);
 
 /*
  * Base class destructor for ref_iterators. Destroy the ref_iterator
diff --git a/refs/reftable-backend.c b/refs/reftable-backend.c
index a14f2ad7f4..70a16dfb9e 100644
--- a/refs/reftable-backend.c
+++ b/refs/reftable-backend.c
@@ -479,7 +479,7 @@ static struct reftable_ref_iterator *ref_iterator_for_stack(struct reftable_ref_
 	int ret;
 
 	iter = xcalloc(1, sizeof(*iter));
-	base_ref_iterator_init(&iter->base, &reftable_ref_iterator_vtable, 1);
+	base_ref_iterator_init(&iter->base, &reftable_ref_iterator_vtable);
 	iter->prefix = prefix;
 	iter->base.oid = &iter->oid;
 	iter->flags = flags;
@@ -575,7 +575,7 @@ static struct ref_iterator *reftable_be_iterator_begin(struct ref_store *ref_sto
 	 * single iterator.
 	 */
 	worktree_iter = ref_iterator_for_stack(refs, refs->worktree_stack, prefix, flags);
-	return merge_ref_iterator_begin(1, &worktree_iter->base, &main_iter->base,
+	return merge_ref_iterator_begin(&worktree_iter->base, &main_iter->base,
 					iterator_select, NULL);
 }
 
@@ -1723,7 +1723,7 @@ static struct reftable_reflog_iterator *reflog_iterator_for_stack(struct reftabl
 	int ret;
 
 	iter = xcalloc(1, sizeof(*iter));
-	base_ref_iterator_init(&iter->base, &reftable_reflog_iterator_vtable, 1);
+	base_ref_iterator_init(&iter->base, &reftable_reflog_iterator_vtable);
 	iter->refs = refs;
 	iter->base.oid = &iter->oid;
 
@@ -1758,7 +1758,7 @@ static struct ref_iterator *reftable_be_reflog_iterator_begin(struct ref_store *
 
 	worktree_iter = reflog_iterator_for_stack(refs, refs->worktree_stack);
 
-	return merge_ref_iterator_begin(1, &worktree_iter->base, &main_iter->base,
+	return merge_ref_iterator_begin(&worktree_iter->base, &main_iter->base,
 					iterator_select, NULL);
 }
 

-- 
2.44.0-rc1
Previous: Patrick SteinhardtNext: Patrick Steinhardt
Message 22 of 39 in “reflog: introduce subcommand to list reflogs”
  1. 0/6 reflog: introduce subcommand to list reflogsPatrick Steinhardt, Feb 19, 2024
  2. 1/6 dir-iterator: pass name to `prepare_next_entry_data()` directlyPatrick Steinhardt, Feb 19, 2024
  3. 2/6 dir-iterator: support iteration in sorted orderPatrick Steinhardt, Feb 19, 2024
  4. Junio C HamanoFeb 19, 2024
  5. Patrick SteinhardtFeb 20, 2024
  6. 3/6 refs/files: sort reflogs returned by the reflog iteratorPatrick Steinhardt, Feb 19, 2024
  7. Junio C HamanoFeb 20, 2024
  8. Patrick SteinhardtFeb 20, 2024
  9. 4/6 refs: drop unused params from the reflog iterator callbackPatrick Steinhardt, Feb 19, 2024
  10. Junio C HamanoFeb 20, 2024
  11. Patrick SteinhardtFeb 20, 2024
  12. 5/6 refs: stop resolving ref corresponding to reflogsPatrick Steinhardt, Feb 19, 2024
  13. Junio C HamanoFeb 20, 2024
  14. Patrick SteinhardtFeb 20, 2024
  15. 6/6 builtin/reflog: introduce subcommand to list reflogsPatrick Steinhardt, Feb 19, 2024
  16. Junio C HamanoFeb 20, 2024
  17. Patrick SteinhardtFeb 20, 2024
  18. 0/7 reflog: introduce subcommand to list reflogsPatrick Steinhardt, Feb 20, 2024
  19. 1/7 dir-iterator: pass name to `prepare_next_entry_data()` directlyPatrick Steinhardt, Feb 20, 2024
  20. 2/7 dir-iterator: support iteration in sorted orderPatrick Steinhardt, Feb 20, 2024
  21. 3/7 refs/files: sort reflogs returned by the reflog iteratorPatrick Steinhardt, Feb 20, 2024
  22. 4/7 refs: always treat iterators as orderedPatrick Steinhardt, Feb 20, 2024
  23. 5/7 refs: drop unused params from the reflog iterator callbackPatrick Steinhardt, Feb 20, 2024
  24. 6/7 refs: stop resolving ref corresponding to reflogsPatrick Steinhardt, Feb 20, 2024
  25. 7/7 builtin/reflog: introduce subcommand to list reflogsPatrick Steinhardt, Feb 20, 2024
  26. 7/7 builtin/reflog: introduce subcommand to list reflogsTeng Long, Apr 24, 2024
  27. Patrick SteinhardtApr 24, 2024
  28. Junio C HamanoApr 24, 2024
  29. Junio C HamanoFeb 20, 2024
  30. Patrick SteinhardtFeb 21, 2024
  31. 0/8 reflog: introduce subcommand to list reflogsPatrick Steinhardt, Feb 21, 2024
  32. 1/8 dir-iterator: pass name to `prepare_next_entry_data()` directlyPatrick Steinhardt, Feb 21, 2024
  33. 2/8 dir-iterator: support iteration in sorted orderPatrick Steinhardt, Feb 21, 2024
  34. 3/8 refs/files: sort reflogs returned by the reflog iteratorPatrick Steinhardt, Feb 21, 2024
  35. 4/8 refs/files: sort merged worktree and common reflogsPatrick Steinhardt, Feb 21, 2024
  36. 5/8 refs: always treat iterators as orderedPatrick Steinhardt, Feb 21, 2024
  37. 6/8 refs: drop unused params from the reflog iterator callbackPatrick Steinhardt, Feb 21, 2024
  38. 7/8 refs: stop resolving ref corresponding to reflogsPatrick Steinhardt, Feb 21, 2024
  39. 8/8 builtin/reflog: introduce subcommand to list reflogsPatrick Steinhardt, Feb 21, 2024

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.