refs.c: remove empty '--exclude' patterns

In 59c35fac54 (refs/packed-backend.c: implement jump lists to avoid
excluded pattern(s), 2023-07-10), the packed-refs backend learned how to
construct "jump lists" to avoid enumerating sections of the packed-refs
file that we know the caller is going to throw out anyway.

This process works by finding the start- and end-points (that is, where
in the packed-refs file corresponds to the range we're going to ignore)
for each exclude pattern, then constructing a jump list based on that.
At enumeration time we'll consult the jump list to skip past everything
in the range(s) found in the previous step, saving time when excluding a
large portion of references.

But when there is a --exclude pattern which is just the empty string,
the behavior is a little funky. When we try and exclude the empty
string, the matched range covers the entire packed-refs file, meaning
that we won't output any packed references. But the empty pattern
doesn't actually match any references to begin with! For example, on my
copy of git.git I can do:

    $ git for-each-ref '' | wc -l
    0

So "git for-each-ref --exclude=''" shouldn't actually remove anything
from the output, and ought to be equivalent to "git for-each-ref". But
it's not, and in fact:

    $ git for-each-ref | wc -l
    2229
    $ git for-each-ref --exclude='' | wc -l
    480

But why does the '--exclude' version output only some of the references
in the repository? Here's a hint:

    $ find .git/refs -type f | wc -l
    480

Indeed, because the files backend doesn't implement[^1] the same jump
list concept as the packed backend we get the correct result for the
loose references, but none of the packed references.

Since the empty string exclude pattern doesn't match anything, we can
discard them before the packed-refs backend has a chance to even see it
(and likewise for reftable, which also implements a similar concept
since 1869525066 (refs/reftable: wire up support for exclude patterns,
2024-09-16)).

This approach (copying only some of the patterns into a strvec at the
refs.c layer) may seem heavy-handed, but it's setting us up to fix
another bug in the following commit where the fix will involve modifying
the incoming patterns.

[^1]: As noted in 59c35fac54. We technically could avoid opening and
  enumerating the contents of, for e.g., "$GIT_DIR/refs/heads/foo/" if
  we knew that we were excluding anything under the 'refs/heads/foo'
  hierarchy. But the --exclude stuff is all best-effort anyway, since
  the caller is expected to cull out any results that they don't want.

Noticed-by: Jeff King <peff@peff.net>
Signed-off-by: Taylor Blau <me@ttaylorr.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

diff --git a/refs.c b/refs.c
index 915aeb4d1dbb62ebc113188e83076b908f3b2c35..fa943d7d64b398ecbe57c4322a06b2e74332fc36 100644 (file)
--- a/refs.c
+++ b/refs.c
@@ -1560,6 +1560,20 @@ struct ref_iterator *refs_ref_iterator_begin(
                enum do_for_each_ref_flags flags)
 {
        struct ref_iterator *iter;
+       struct strvec normalized_exclude_patterns = STRVEC_INIT;
+
+       if (exclude_patterns) {
+               for (size_t i = 0; exclude_patterns[i]; i++) {
+                       const char *pattern = exclude_patterns[i];
+                       size_t len = strlen(pattern);
+                       if (!len)
+                               continue;
+
+                       strvec_push(&normalized_exclude_patterns, pattern);
+               }
+
+               exclude_patterns = normalized_exclude_patterns.v;
+       }
 
        if (!(flags & DO_FOR_EACH_INCLUDE_BROKEN)) {
                static int ref_paranoia = -1;
@@ -1580,6 +1594,8 @@ struct ref_iterator *refs_ref_iterator_begin(
        if (trim)
                iter = prefix_ref_iterator_begin(iter, "", trim);
 
+       strvec_clear(&normalized_exclude_patterns);
+
        return iter;
 }
 

diff --git a/t/t1419-exclude-refs.sh b/t/t1419-exclude-refs.sh
index 13595744190b54e539a581f2409ab3be5e3932d3..b5e01e9f45b7546e8261e803bce9059ee426b6f3 100755 (executable)
--- a/t/t1419-exclude-refs.sh
+++ b/t/t1419-exclude-refs.sh
@@ -125,4 +125,14 @@ test_expect_success 'meta-characters are discarded' '
        assert_no_jumps perf
 '
 
+test_expect_success 'empty string exclude pattern is ignored' '
+       git update-ref refs/heads/loose $(git rev-parse refs/heads/foo/1) &&
+
+       for_each_ref__exclude refs/heads "" >actual 2>perf &&
+       for_each_ref >expect &&
+
+       test_cmp expect actual &&
+       assert_no_jumps perf
+'
+
 test_done