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

Re: [PATCH 3/6] refs/files: sort reflogs returned by the reflog iterator

From
Patrick Steinhardt <ps@pks.im>
Date
Feb 20, 2024, 08:34 UTC
Message-ID
<ZdRkLylHKj44tstQ@tanuki>
In-Reply-To
<xmqq34to0znj.fsf@gitster.g>
On Mon, Feb 19, 2024 at 04:04:16PM -0800, Junio C Hamano wrote:
Show 47 quoted lines
> Patrick Steinhardt <ps@pks.im> writes:
> 
> > We use a directory iterator to return reflogs via the reflog iterator.
> > This iterator returns entries in the same order as readdir(3P) would and
> > will thus yield reflogs with no discernible order.
> >
> > Set the new `DIR_ITERATOR_SORTED` flag that was introduced in the
> > preceding commit so that the order is deterministic. While the effect of
> > this can only been observed in a test tool, a subsequent commit will
> > start to expose this functionality to users via a new `git reflog list`
> > subcommand.
> >
> > Signed-off-by: Patrick Steinhardt <ps@pks.im>
> > ---
> >  refs/files-backend.c           | 4 ++--
> >  t/t0600-reffiles-backend.sh    | 4 ++--
> >  t/t1405-main-ref-store.sh      | 2 +-
> >  t/t1406-submodule-ref-store.sh | 2 +-
> >  4 files changed, 6 insertions(+), 6 deletions(-)
> >
> > diff --git a/refs/files-backend.c b/refs/files-backend.c
> > index 75dcc21ecb..2ffc63185f 100644
> > --- a/refs/files-backend.c
> > +++ b/refs/files-backend.c
> > @@ -2193,7 +2193,7 @@ static struct ref_iterator *reflog_iterator_begin(struct ref_store *ref_store,
> >  
> >  	strbuf_addf(&sb, "%s/logs", gitdir);
> >  
> > -	diter = dir_iterator_begin(sb.buf, 0);
> > +	diter = dir_iterator_begin(sb.buf, DIR_ITERATOR_SORTED);
> >  	if (!diter) {
> >  		strbuf_release(&sb);
> >  		return empty_ref_iterator_begin();
> > @@ -2202,7 +2202,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, 0);
> > +	base_ref_iterator_init(ref_iterator, &files_reflog_iterator_vtable, 1);
> 
> This caught my attention.  Once we apply this patch, the only way
> base_ref_iterator_init() can receive 0 for its last parameter
> (i.e. 'ordered') is via the merge_ref_iterator_begin() call in
> files_reflog_iterator_begin() that passes 0 as 'ordered'.  If we
> force files_reflog_iterator_begin() to ask for an ordered
> merge_ref_iterator, then we will have no unordered ref iterators.
> Am I reading the code right?

Ah, true indeed. The "files" reflog iterator was the only remaining iterator that wasn't ordered. I'll include an additional patch on top that drops the `ordered` bit altogether.

Patrick
Previous: Junio C HamanoNext: Patrick Steinhardt
Message 8 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.