From: Patrick Steinhardt <ps@pks.im>
To: Junio C Hamano <gitster@pobox.com>
Cc: git@vger.kernel.org
Subject: Re: [PATCH v2 0/7] reflog: introduce subcommand to list reflogs
Date: Wed, 21 Feb 2024 12:48:01 +0100 [thread overview]
Message-ID: <ZdXi8agx5oxKfwrD@tanuki> (raw)
In-Reply-To: <xmqq34tnrqxv.fsf@gitster.g>
[-- Attachment #1: Type: text/plain, Size: 1840 bytes --]
On Tue, Feb 20, 2024 at 09:22:36AM -0800, Junio C Hamano wrote:
> Patrick Steinhardt <ps@pks.im> writes:
[snip]
> > 3: e4e4fac05c ! 3: 32b24a3d4b refs/files: sort reflogs returned by the reflog iterator
> > @@ refs/files-backend.c: static struct ref_iterator *reflog_iterator_begin(struct r
> > iter->dir_iterator = diter;
> > iter->ref_store = ref_store;
> > strbuf_release(&sb);
> > +@@ refs/files-backend.c: 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);
> > + }
>
> This hunk is new. Is there a downside to force merged iterators to
> always be sorted? The ones that are combined are all sorted so it
> is natural to force sorting like this code does? It might deserve
> explaining, and would certainly help future readers who runs "blame"
> on this code to figure out what made us think always sorting is a
> good direction forward.
Not really -- it merely gets passed down to the base ref iterator to
indicate that the entries are returned in lexicographic order. But I've
been jumping the gun here: the `reflog_iterator_select()` function does
not ensure lexicographic ordering between the two merged iterators right
now. I was assuming so because I implemented it in the reftable backend
like that. Should've double checked.
It's an easy fix though, which I'll add as another patch on top. Thanks
for making me think twice.
Patrick
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
next prev parent reply other threads:[~2024-02-21 11:48 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 [this message]
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 ` [PATCH v3 4/8] refs/files: sort merged worktree and common reflogs Patrick Steinhardt
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=ZdXi8agx5oxKfwrD@tanuki \
--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).