Re: [PATCH v6 14/16] refs/iterator: implement seeking for packed-ref iterators

From: Elijah Newren <newren@gmail.com>
Date: Apr 3, 2025, 19:56 UTC
Message-ID: <CABPp-BFBqC_t5QSexRQpYsqXBa11WK+OqGt167E=K=xod=buQw@mail.gmail.com>
In-Reply-To: <20250312-pks-update-ref-optimization-v6-14-f778e0414f55@pks.im>

On Wed, Mar 12, 2025 at 11:42 PM Patrick Steinhardt <ps@pks.im> wrote:

Show 7 quoted lines

> @@ -951,12 +954,41 @@ static int packed_ref_iterator_advance(struct ref_iterator *ref_iterator)
>                                             &iter->oid, iter->flags))
>                         continue;
>
> +               while (prefix && *prefix) {
> +                       if (*refname < *prefix)
> +                               BUG("packed-refs backend yielded reference preceding its prefix");

I just triggered this bug upon a "git pull" in an internal repository:

$ git pull
remote: Enumerating objects: 161255, done.
remote: Counting objects: 100% (55884/55884), done.
remote: Compressing objects: 100% (5518/5518), done.
remote: Total 161255 (delta 54253), reused 50509 (delta 50364),
pack-reused 105371 (from 4)
Receiving objects: 100% (161255/161255), 309.90 MiB | 16.87 MiB/s, done.
Resolving deltas: 100% (118048/118048), completed with 13416 local objects.
From github.com:github/github
   97ab7ae3f3745..8fb2f9fa180ed  master
                    -> origin/master
[...snip many screenfuls of updates to origin remotes...]
BUG: refs/packed-backend.c:984: packed-refs backend yielded reference
preceding its prefix
error: fetch died of signal 6

I made a backup of the repo with rsync.

It leaves around a lock file, which after I clear out and run `git fetch --update-head-ok` again, it'll hit the same error.

If I put a little debugging into the while loop:

        while (prefix && *prefix) {
            if (*refname < *prefix) {
                printf("Comparing %s to %s\n",
                       refname, prefix);
                BUG("packed-refs backend yielded reference preceding
its prefix");

then my fetch produces:

Comparing 🍧 to zuora-params-orders-sorbet/ (in case that doesn't come through well, that's a shaved-ice-emoji, according to a google search, which someone decided to use within one of their branch names)

In this case, the utf-8 characters will have individual bytes whose values are greater than 127, which for a signed character will be represented by a negative number. I tried tweaking the loop to:

        while (prefix && *prefix) {
            if ((unsigned char)*refname < (unsigned char)*prefix) {
                printf("Comparing %d to %d\n",
                       *refname, *prefix);
                printf("Comparing %s to %s\n",
                       refname, prefix);
                BUG("packed-refs backend yielded reference preceding
its prefix");

but that just lets it proceed through the loop four times before dying:

Comparing 0 to 97
Comparing  to a-params-orders-sorbet/
BUG: refs/packed-backend.c:988: packed-refs backend yielded reference
preceding its prefix
Aborted (core dumped)

meaning it eats up the four bytes from the shaved-ice-emoji and the first four bytes of prefix "zuor" and then hits the error, so I suspect it needs to handle comparisons a little more broadly.