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 <vdye@github.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>

Victoria Dye committed Oct 9, 2023 at 21:58 UTC 5305474ec4650755f2c0437c004cabfb1c60cf6a
3 files changed +55 -1
refs/ref-cache.c
+2 -1
@@ -412,7 +412,8 @@ static int cache_ref_iterator_advance(struct ref_iterator *ref_iterator)
412
413 if (level->prefix_state == PREFIX_WITHIN_DIR) {
414 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)))
417 continue;
418 } else {
419 entry_prefix_state = level->prefix_state;
t/t1500-rev-parse.sh
+23
@@ -264,4 +264,27 @@ test_expect_success 'rev-parse --since= unsqueezed ordering' '
264 test_cmp expect actual
265 '
266
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 +
290 test_done
t/t4205-log-pretty-formats.sh
+30
@@ -924,6 +924,36 @@ test_expect_success '%S in git log --format works with other placeholders (part
924 test_cmp expect actual
925 '
926
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 +
957 test_expect_success 'log --pretty=reference' '
958 git log --pretty="tformat:%h (%s, %as)" >expect &&
959 git log --pretty=reference >actual &&