Git Mailing List Archive mirror
 help / color / mirror / Atom feed
From: Patrick Steinhardt <ps@pks.im>
To: git@vger.kernel.org
Cc: Junio C Hamano <gitster@pobox.com>
Subject: [PATCH v3 4/8] refs/files: sort merged worktree and common reflogs
Date: Wed, 21 Feb 2024 13:37:31 +0100	[thread overview]
Message-ID: <0b52f6c4afea258133b4553e8c40fffe2143e6c6.1708518982.git.ps@pks.im> (raw)
In-Reply-To: <cover.1708518982.git.ps@pks.im>

[-- Attachment #1: Type: text/plain, Size: 8134 bytes --]

When iterating through reflogs in a worktree we create a merged iterator
that merges reflogs from both refdbs. The resulting refs are ordered so
that instead we first return all worktree reflogs before we return all
common refs.

This is the only remaining case where a ref iterator returns entries in
a non-lexicographic order. The result would look something like the
following (listed with a command we introduce in a subsequent commit):

```
$ git reflog list
HEAD
refs/worktree/per-worktree
refs/heads/main
refs/heads/wt
```

So we first print the per-worktree reflogs in lexicographic order, then
the common reflogs in lexicographic order. This is confusing and not
consistent with how we print per-worktree refs, which are exclusively
sorted lexicographically.

Sort reflogs lexicographically in the same way as we sort normal refs.
As this is already implemented properly by the "reftable" backend via a
separate selection function, we simply pull out that logic and reuse it
for the "files" backend. As logs are properly sorted now, mark the
merged reflog iterator as sorted.

Tests will be added in a subsequent commit.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
 refs/files-backend.c    | 30 ++------------------------
 refs/iterator.c         | 43 +++++++++++++++++++++++++++++++++++++
 refs/refs-internal.h    |  9 ++++++++
 refs/reftable-backend.c | 47 ++---------------------------------------
 4 files changed, 56 insertions(+), 73 deletions(-)

diff --git a/refs/files-backend.c b/refs/files-backend.c
index 2ffc63185f..551cafdf76 100644
--- a/refs/files-backend.c
+++ b/refs/files-backend.c
@@ -2210,32 +2210,6 @@ static struct ref_iterator *reflog_iterator_begin(struct ref_store *ref_store,
 	return ref_iterator;
 }
 
-static enum iterator_selection reflog_iterator_select(
-	struct ref_iterator *iter_worktree,
-	struct ref_iterator *iter_common,
-	void *cb_data UNUSED)
-{
-	if (iter_worktree) {
-		/*
-		 * We're a bit loose here. We probably should ignore
-		 * common refs if they are accidentally added as
-		 * per-worktree refs.
-		 */
-		return ITER_SELECT_0;
-	} else if (iter_common) {
-		if (parse_worktree_ref(iter_common->refname, NULL, NULL,
-				       NULL) == REF_WORKTREE_SHARED)
-			return ITER_SELECT_1;
-
-		/*
-		 * The main ref store may contain main worktree's
-		 * per-worktree refs, which should be ignored
-		 */
-		return ITER_SKIP_1;
-	} else
-		return ITER_DONE;
-}
-
 static struct ref_iterator *files_reflog_iterator_begin(struct ref_store *ref_store)
 {
 	struct files_ref_store *refs =
@@ -2246,9 +2220,9 @@ 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(
-			0, reflog_iterator_begin(ref_store, refs->base.gitdir),
+			1, reflog_iterator_begin(ref_store, refs->base.gitdir),
 			reflog_iterator_begin(ref_store, refs->gitcommondir),
-			reflog_iterator_select, refs);
+			ref_iterator_select, refs);
 	}
 }
 
diff --git a/refs/iterator.c b/refs/iterator.c
index 6b680f610e..b7ab5dc92f 100644
--- a/refs/iterator.c
+++ b/refs/iterator.c
@@ -98,6 +98,49 @@ struct merge_ref_iterator {
 	struct ref_iterator **current;
 };
 
+enum iterator_selection ref_iterator_select(struct ref_iterator *iter_worktree,
+					    struct ref_iterator *iter_common,
+					    void *cb_data UNUSED)
+{
+	if (iter_worktree && !iter_common) {
+		/*
+		 * Return the worktree ref if there are no more common refs.
+		 */
+		return ITER_SELECT_0;
+	} else if (iter_common) {
+		/*
+		 * In case we have pending worktree and common refs we need to
+		 * yield them based on their lexicographical order. Worktree
+		 * refs that have the same name as common refs shadow the
+		 * latter.
+		 */
+		if (iter_worktree) {
+			int cmp = strcmp(iter_worktree->refname,
+					 iter_common->refname);
+			if (cmp < 0)
+				return ITER_SELECT_0;
+			else if (!cmp)
+				return ITER_SELECT_0_SKIP_1;
+		}
+
+		 /*
+		  * We now know that the lexicographically-next ref is a common
+		  * ref. When the common ref is a shared one we return it.
+		  */
+		if (parse_worktree_ref(iter_common->refname, NULL, NULL,
+				       NULL) == REF_WORKTREE_SHARED)
+			return ITER_SELECT_1;
+
+		/*
+		 * Otherwise, if the common ref is a per-worktree ref we skip
+		 * it because it would belong to the main worktree, not ours.
+		 */
+		return ITER_SKIP_1;
+	} else {
+		return ITER_DONE;
+	}
+}
+
 static int merge_ref_iterator_advance(struct ref_iterator *ref_iterator)
 {
 	struct merge_ref_iterator *iter =
diff --git a/refs/refs-internal.h b/refs/refs-internal.h
index 83e0f0bba3..51f612e122 100644
--- a/refs/refs-internal.h
+++ b/refs/refs-internal.h
@@ -386,6 +386,15 @@ typedef enum iterator_selection ref_iterator_select_fn(
 		struct ref_iterator *iter0, struct ref_iterator *iter1,
 		void *cb_data);
 
+/*
+ * An implementation of ref_iterator_select_fn that merges worktree and common
+ * refs. Per-worktree refs from the common iterator are ignored, worktree refs
+ * override common refs. Refs are selected lexicographically.
+ */
+enum iterator_selection ref_iterator_select(struct ref_iterator *iter_worktree,
+					    struct ref_iterator *iter_common,
+					    void *cb_data);
+
 /*
  * Iterate over the entries from iter0 and iter1, with the values
  * interleaved as directed by the select function. The iterator takes
diff --git a/refs/reftable-backend.c b/refs/reftable-backend.c
index a14f2ad7f4..68d32a9101 100644
--- a/refs/reftable-backend.c
+++ b/refs/reftable-backend.c
@@ -504,49 +504,6 @@ static struct reftable_ref_iterator *ref_iterator_for_stack(struct reftable_ref_
 	return iter;
 }
 
-static enum iterator_selection iterator_select(struct ref_iterator *iter_worktree,
-					       struct ref_iterator *iter_common,
-					       void *cb_data UNUSED)
-{
-	if (iter_worktree && !iter_common) {
-		/*
-		 * Return the worktree ref if there are no more common refs.
-		 */
-		return ITER_SELECT_0;
-	} else if (iter_common) {
-		/*
-		 * In case we have pending worktree and common refs we need to
-		 * yield them based on their lexicographical order. Worktree
-		 * refs that have the same name as common refs shadow the
-		 * latter.
-		 */
-		if (iter_worktree) {
-			int cmp = strcmp(iter_worktree->refname,
-					 iter_common->refname);
-			if (cmp < 0)
-				return ITER_SELECT_0;
-			else if (!cmp)
-				return ITER_SELECT_0_SKIP_1;
-		}
-
-		 /*
-		  * We now know that the lexicographically-next ref is a common
-		  * ref. When the common ref is a shared one we return it.
-		  */
-		if (parse_worktree_ref(iter_common->refname, NULL, NULL,
-				       NULL) == REF_WORKTREE_SHARED)
-			return ITER_SELECT_1;
-
-		/*
-		 * Otherwise, if the common ref is a per-worktree ref we skip
-		 * it because it would belong to the main worktree, not ours.
-		 */
-		return ITER_SKIP_1;
-	} else {
-		return ITER_DONE;
-	}
-}
-
 static struct ref_iterator *reftable_be_iterator_begin(struct ref_store *ref_store,
 						       const char *prefix,
 						       const char **exclude_patterns,
@@ -576,7 +533,7 @@ static struct ref_iterator *reftable_be_iterator_begin(struct ref_store *ref_sto
 	 */
 	worktree_iter = ref_iterator_for_stack(refs, refs->worktree_stack, prefix, flags);
 	return merge_ref_iterator_begin(1, &worktree_iter->base, &main_iter->base,
-					iterator_select, NULL);
+					ref_iterator_select, NULL);
 }
 
 static int reftable_be_read_raw_ref(struct ref_store *ref_store,
@@ -1759,7 +1716,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,
-					iterator_select, NULL);
+					ref_iterator_select, NULL);
 }
 
 static int yield_log_record(struct reftable_log_record *log,
-- 
2.44.0-rc1


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

  parent reply	other threads:[~2024-02-21 12:37 UTC|newest]

Thread overview: 39+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-02-19 14:35 [PATCH 0/6] reflog: introduce subcommand to list reflogs Patrick Steinhardt
2024-02-19 14:35 ` [PATCH 1/6] dir-iterator: pass name to `prepare_next_entry_data()` directly Patrick Steinhardt
2024-02-19 14:35 ` [PATCH 2/6] dir-iterator: support iteration in sorted order Patrick Steinhardt
2024-02-19 23:39   ` Junio C Hamano
2024-02-20  8:34     ` Patrick Steinhardt
2024-02-19 14:35 ` [PATCH 3/6] refs/files: sort reflogs returned by the reflog iterator Patrick Steinhardt
2024-02-20  0:04   ` Junio C Hamano
2024-02-20  8:34     ` Patrick Steinhardt
2024-02-19 14:35 ` [PATCH 4/6] refs: drop unused params from the reflog iterator callback Patrick Steinhardt
2024-02-20  0:14   ` Junio C Hamano
2024-02-20  8:34     ` Patrick Steinhardt
2024-02-19 14:35 ` [PATCH 5/6] refs: stop resolving ref corresponding to reflogs Patrick Steinhardt
2024-02-20  0:14   ` Junio C Hamano
2024-02-20  8:34     ` Patrick Steinhardt
2024-02-19 14:35 ` [PATCH 6/6] builtin/reflog: introduce subcommand to list reflogs Patrick Steinhardt
2024-02-20  0:32   ` Junio C Hamano
2024-02-20  8:34     ` Patrick Steinhardt
2024-02-20  9:06 ` [PATCH v2 0/7] reflog: " Patrick Steinhardt
2024-02-20  9:06   ` [PATCH v2 1/7] dir-iterator: pass name to `prepare_next_entry_data()` directly Patrick Steinhardt
2024-02-20  9:06   ` [PATCH v2 2/7] dir-iterator: support iteration in sorted order Patrick Steinhardt
2024-02-20  9:06   ` [PATCH v2 3/7] refs/files: sort reflogs returned by the reflog iterator Patrick Steinhardt
2024-02-20  9:06   ` [PATCH v2 4/7] refs: always treat iterators as ordered Patrick Steinhardt
2024-02-20  9:06   ` [PATCH v2 5/7] refs: drop unused params from the reflog iterator callback Patrick Steinhardt
2024-02-20  9:06   ` [PATCH v2 6/7] refs: stop resolving ref corresponding to reflogs Patrick Steinhardt
2024-02-20  9:06   ` [PATCH v2 7/7] builtin/reflog: introduce subcommand to list reflogs Patrick Steinhardt
2024-04-24  7:30     ` Teng Long
2024-04-24  8:01       ` Patrick Steinhardt
2024-04-24 14:53         ` Junio C Hamano
2024-02-20 17:22   ` [PATCH v2 0/7] reflog: " Junio C Hamano
2024-02-21 11:48     ` Patrick Steinhardt
2024-02-21 12:37 ` [PATCH v3 0/8] " Patrick Steinhardt
2024-02-21 12:37   ` [PATCH v3 1/8] dir-iterator: pass name to `prepare_next_entry_data()` directly Patrick Steinhardt
2024-02-21 12:37   ` [PATCH v3 2/8] dir-iterator: support iteration in sorted order Patrick Steinhardt
2024-02-21 12:37   ` [PATCH v3 3/8] refs/files: sort reflogs returned by the reflog iterator Patrick Steinhardt
2024-02-21 12:37   ` Patrick Steinhardt [this message]
2024-02-21 12:37   ` [PATCH v3 5/8] refs: always treat iterators as ordered Patrick Steinhardt
2024-02-21 12:37   ` [PATCH v3 6/8] refs: drop unused params from the reflog iterator callback Patrick Steinhardt
2024-02-21 12:37   ` [PATCH v3 7/8] refs: stop resolving ref corresponding to reflogs Patrick Steinhardt
2024-02-21 12:37   ` [PATCH v3 8/8] builtin/reflog: introduce subcommand to list reflogs Patrick Steinhardt

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=0b52f6c4afea258133b4553e8c40fffe2143e6c6.1708518982.git.ps@pks.im \
    --to=ps@pks.im \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).