Skip to content

Commit e336bb9

Browse files
venkatr21gitster
authored andcommitted
refs: fix loose-ref seeking across directory boundaries
When seeking through loose refs, cache_ref_iterator_seek() compares each directory component without its trailing slash. This can confuse a directory with a similarly named ref that sorts before it. For example, with these refs: refs/tags/releases-z refs/tags/releases/v1 refs/tags/releases/v2 Starting after refs/tags/releases/v1 can position the iterator at releases-z instead of descending into releases/. Iteration then returns v1 again, even though it should return only v2. Include the trailing slash when comparing directory components so that the seek follows the correct directory. Also handle two related positioning errors. If every entry in a directory precedes the cursor, stop searching that level rather than descending into its last entry. If a leaf sorts after the cursor, leave the iterator positioned to return it, even when the cursor has remaining path components. Add regression tests for shared-prefix names, missing nested markers, and markers beyond the end of the ref listing, using loose, packed, and mixed refs. Signed-off-by: Venkat Ragavan S <mail2venkat21@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
1 parent e9019fc commit e336bb9

2 files changed

Lines changed: 59 additions & 2 deletions

File tree

‎refs/ref-cache.c‎

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -506,7 +506,7 @@ static int cache_ref_iterator_seek(struct ref_iterator *ref_iterator,
506506
sort_ref_dir(dir);
507507

508508
slash = strchr(slash, '/');
509-
len = slash ? (size_t)(slash - refname) : strlen(refname);
509+
len = slash ? (size_t)(slash - refname + 1) : strlen(refname);
510510

511511
for (idx = 0; idx < dir->nr; idx++) {
512512
cmp = strncmp(refname, dir->entries[idx]->name, len);
@@ -520,6 +520,8 @@ static int cache_ref_iterator_seek(struct ref_iterator *ref_iterator,
520520
slash = slash + 1;
521521

522522
level->index = idx;
523+
if (cmp > 0)
524+
break;
523525
if (dir->entries[idx]->flag & REF_DIR) {
524526
/* push down a level */
525527
dir = get_ref_dir(dir->entries[idx]);
@@ -532,7 +534,7 @@ static int cache_ref_iterator_seek(struct ref_iterator *ref_iterator,
532534
level->prefix_state = PREFIX_CONTAINS_DIR;
533535
} else {
534536
/* reduce the index so the leaf node is iterated over */
535-
if (cmp <= 0 && !slash)
537+
if (cmp < 0 || (cmp == 0 && !slash))
536538
level->index = idx - 1;
537539
/*
538540
* while the seek path may not be exhausted, our

‎t/t6302-for-each-ref-filter.sh‎

Lines changed: 55 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -746,6 +746,61 @@ test_expect_success 'start after used with a pattern' '
746746
test_cmp expect actual
747747
'
748748

749+
for storage in loose packed mixed
750+
do
751+
test_expect_success "start after respects directory boundaries ($storage)" '
752+
test_when_finished "rm -rf repo" &&
753+
git init repo &&
754+
(
755+
cd repo &&
756+
test_commit --no-tag base &&
757+
git update-ref refs/tags/packed HEAD &&
758+
if test "$storage" = mixed
759+
then
760+
git pack-refs --all || return 1
761+
fi &&
762+
git update-ref --stdin <<-\EOF &&
763+
create refs/tags/a-leaf HEAD
764+
create refs/tags/a/nested HEAD
765+
create refs/tags/a0 HEAD
766+
create refs/tags/b HEAD
767+
create refs/tags/releases-z HEAD
768+
create refs/tags/releases/v1 HEAD
769+
create refs/tags/releases/v2 HEAD
770+
EOF
771+
if test "$storage" = packed
772+
then
773+
git pack-refs --all || return 1
774+
fi &&
775+
echo refs/tags/releases/v2 >expect &&
776+
git for-each-ref --no-sort --format="%(refname)" \
777+
--start-after=refs/tags/releases/v1 >actual &&
778+
test_cmp expect actual &&
779+
cat >expect <<-\EOF &&
780+
refs/tags/a0
781+
refs/tags/b
782+
refs/tags/packed
783+
refs/tags/releases-z
784+
refs/tags/releases/v1
785+
refs/tags/releases/v2
786+
EOF
787+
git for-each-ref --no-sort --format="%(refname)" \
788+
--start-after=refs/tags/a/next >actual &&
789+
test_cmp expect actual &&
790+
tail -n 5 expect >expect-later &&
791+
git for-each-ref --no-sort --format="%(refname)" \
792+
--start-after=refs/tags/aa/missing >actual &&
793+
test_cmp expect-later actual &&
794+
for marker in refs/tags/zzzz refs/zzzz zzzz
795+
do
796+
git for-each-ref --no-sort --format="%(refname)" \
797+
--start-after="$marker" >actual &&
798+
test_must_be_empty actual || return 1
799+
done
800+
)
801+
'
802+
done
803+
749804
test_expect_success 'start after used with custom sort order' '
750805
cat >expect <<-\EOF &&
751806
fatal: cannot use --start-after with custom sort options

0 commit comments

Comments
 (0)