| Message ID | e4e4fac05c7f4bcac8ef96bdebb8a68eef40ead4.1708353264.git.ps@pks.im (mailing list archive) |
|---|---|
| State | Superseded |
| Commit | fb9d5598f08be9b0480eeeabda9a5a71d5a898c5 |
| Headers | show |
| Series | reflog: introduce subcommand to list reflogs | expand |
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?
On Mon, Feb 19, 2024 at 04:04:16PM -0800, Junio C Hamano wrote: > 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
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); iter->dir_iterator = diter; iter->ref_store = ref_store; strbuf_release(&sb); diff --git a/t/t0600-reffiles-backend.sh b/t/t0600-reffiles-backend.sh index e6a5f1868f..4f860285cc 100755 --- a/t/t0600-reffiles-backend.sh +++ b/t/t0600-reffiles-backend.sh @@ -287,7 +287,7 @@ test_expect_success 'for_each_reflog()' ' mkdir -p .git/worktrees/wt/logs/refs/bisect && echo $ZERO_OID > .git/worktrees/wt/logs/refs/bisect/wt-random && - $RWT for-each-reflog | cut -d" " -f 2- | sort >actual && + $RWT for-each-reflog | cut -d" " -f 2- >actual && cat >expected <<-\EOF && HEAD 0x1 PSEUDO-WT 0x0 @@ -297,7 +297,7 @@ test_expect_success 'for_each_reflog()' ' EOF test_cmp expected actual && - $RMAIN for-each-reflog | cut -d" " -f 2- | sort >actual && + $RMAIN for-each-reflog | cut -d" " -f 2- >actual && cat >expected <<-\EOF && HEAD 0x1 PSEUDO-MAIN 0x0 diff --git a/t/t1405-main-ref-store.sh b/t/t1405-main-ref-store.sh index 976bd71efb..cfb583f544 100755 --- a/t/t1405-main-ref-store.sh +++ b/t/t1405-main-ref-store.sh @@ -74,7 +74,7 @@ test_expect_success 'verify_ref(new-main)' ' ' test_expect_success 'for_each_reflog()' ' - $RUN for-each-reflog | sort -k2 | cut -d" " -f 2- >actual && + $RUN for-each-reflog | cut -d" " -f 2- >actual && cat >expected <<-\EOF && HEAD 0x1 refs/heads/main 0x0 diff --git a/t/t1406-submodule-ref-store.sh b/t/t1406-submodule-ref-store.sh index e6a7f7334b..40332e23cc 100755 --- a/t/t1406-submodule-ref-store.sh +++ b/t/t1406-submodule-ref-store.sh @@ -63,7 +63,7 @@ test_expect_success 'verify_ref(new-main)' ' ' test_expect_success 'for_each_reflog()' ' - $RUN for-each-reflog | sort | cut -d" " -f 2- >actual && + $RUN for-each-reflog | cut -d" " -f 2- >actual && cat >expected <<-\EOF && HEAD 0x1 refs/heads/main 0x0
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(-)