Skip to content

Commit 5305474

Browse files
vdyegitster
authored andcommitted
ref-cache.c: fix prefix matching in ref iteration
Update 'cache_ref_iterator_advance' to skip over refs that are not matched by the given prefix. Currently, a ref entry is considered "matched" if the entry name is fully contained within the prefix: * prefix: "refs/heads/v1" * entry: "refs/heads/v1.0" OR if the prefix is fully contained in the entry name: * prefix: "refs/heads/v1.0" * entry: "refs/heads/v1" The first case is always correct, but the second is only correct if the ref cache entry is a directory, for example: * prefix: "refs/heads/example" * entry: "refs/heads/" Modify the logic in 'cache_ref_iterator_advance' to reflect these expectations: 1. If 'overlaps_prefix' returns 'PREFIX_EXCLUDES_DIR', then the prefix and ref cache entry do not overlap at all. Skip this entry. 2. If 'overlaps_prefix' returns 'PREFIX_WITHIN_DIR', then the prefix matches inside this entry if it is a directory. Skip if the entry is not a directory, otherwise iterate over it. 3. Otherwise, 'overlaps_prefix' returned 'PREFIX_CONTAINS_DIR', indicating that the cache entry (directory or not) is fully contained by or equal to the prefix. Iterate over this entry. Note that condition 2 relies on the names of directory entries having the appropriate trailing slash. The existing function documentation of 'create_dir_entry' explicitly calls out the trailing slash requirement, so this is a safe assumption to make. This bug generally doesn't have any user-facing impact, since it requires: 1. using a non-empty prefix without a trailing slash in an iteration like 'for_each_fullref_in', 2. the callback to said iteration not reapplying the original filter (as for-each-ref does) to ensure unmatched refs are skipped, and 3. the repository having one or more refs that match part of, but not all of, the prefix. However, there are some niche scenarios that meet those criteria (specifically, 'rev-parse --bisect' and '(log|show|shortlog) --bisect'). Add tests covering those cases to demonstrate the fix in this patch. Signed-off-by: Victoria Dye <[email protected]> Signed-off-by: Junio C Hamano <[email protected]>
1 parent 43c8a30 commit 5305474

File tree

3 files changed

+55
-1
lines changed

3 files changed

+55
-1
lines changed

refs/ref-cache.c

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -412,7 +412,8 @@ static int cache_ref_iterator_advance(struct ref_iterator *ref_iterator)
412412

413413
if (level->prefix_state == PREFIX_WITHIN_DIR) {
414414
entry_prefix_state = overlaps_prefix(entry->name, iter->prefix);
415-
if (entry_prefix_state == PREFIX_EXCLUDES_DIR)
415+
if (entry_prefix_state == PREFIX_EXCLUDES_DIR ||
416+
(entry_prefix_state == PREFIX_WITHIN_DIR && !(entry->flag & REF_DIR)))
416417
continue;
417418
} else {
418419
entry_prefix_state = level->prefix_state;

t/t1500-rev-parse.sh

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -264,4 +264,27 @@ test_expect_success 'rev-parse --since= unsqueezed ordering' '
264264
test_cmp expect actual
265265
'
266266

267+
test_expect_success 'rev-parse --bisect includes bad, excludes good' '
268+
test_commit_bulk 6 &&
269+
270+
git update-ref refs/bisect/bad-1 HEAD~1 &&
271+
git update-ref refs/bisect/b HEAD~2 &&
272+
git update-ref refs/bisect/bad-3 HEAD~3 &&
273+
git update-ref refs/bisect/good-3 HEAD~3 &&
274+
git update-ref refs/bisect/bad-4 HEAD~4 &&
275+
git update-ref refs/bisect/go HEAD~4 &&
276+
277+
# Note: refs/bisect/b and refs/bisect/go should be ignored because they
278+
# do not match the refs/bisect/bad or refs/bisect/good prefixes.
279+
cat >expect <<-EOF &&
280+
refs/bisect/bad-1
281+
refs/bisect/bad-3
282+
refs/bisect/bad-4
283+
^refs/bisect/good-3
284+
EOF
285+
286+
git rev-parse --symbolic-full-name --bisect >actual &&
287+
test_cmp expect actual
288+
'
289+
267290
test_done

t/t4205-log-pretty-formats.sh

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -924,6 +924,36 @@ test_expect_success '%S in git log --format works with other placeholders (part
924924
test_cmp expect actual
925925
'
926926

927+
test_expect_success 'setup more commits for %S with --bisect' '
928+
test_commit four &&
929+
test_commit five &&
930+
931+
head1=$(git rev-parse --verify HEAD~0) &&
932+
head2=$(git rev-parse --verify HEAD~1) &&
933+
head3=$(git rev-parse --verify HEAD~2) &&
934+
head4=$(git rev-parse --verify HEAD~3)
935+
'
936+
937+
test_expect_success '%S with --bisect labels commits with refs/bisect/bad ref' '
938+
git update-ref refs/bisect/bad-$head1 $head1 &&
939+
git update-ref refs/bisect/go $head1 &&
940+
git update-ref refs/bisect/bad-$head2 $head2 &&
941+
git update-ref refs/bisect/b $head3 &&
942+
git update-ref refs/bisect/bad-$head4 $head4 &&
943+
git update-ref refs/bisect/good-$head4 $head4 &&
944+
945+
# We expect to see the range of commits betwee refs/bisect/good-$head4
946+
# and refs/bisect/bad-$head1. The "source" ref is the nearest bisect ref
947+
# from which the commit is reachable.
948+
cat >expect <<-EOF &&
949+
$head1 refs/bisect/bad-$head1
950+
$head2 refs/bisect/bad-$head2
951+
$head3 refs/bisect/bad-$head2
952+
EOF
953+
git log --bisect --format="%H %S" >actual &&
954+
test_cmp expect actual
955+
'
956+
927957
test_expect_success 'log --pretty=reference' '
928958
git log --pretty="tformat:%h (%s, %as)" >expect &&
929959
git log --pretty=reference >actual &&

0 commit comments

Comments
 (0)