Re: [BUG] `git gc` or `git pack-refs` wipes all notes for `git notes` command

From: Jeff King <peff@peff.net>
Date: Jan 6, 2023, 08:57 UTC
Message-ID: <Y7fikyZV1ky2modr@coredump.intra.peff.net>
In-Reply-To: <CAAYtLE+PWK_v0cc8uqaiKnTHKghrkxuCCgfWyo9bhD23+vxK1g@mail.gmail.com>

On Thu, Jan 05, 2023 at 06:59:36PM +0200, Andrew Hlynskyi wrote:

Show 7 quoted lines

> > Can you share the .git directory of a repository that exhibits this
> > behavior? It's possible there's a bug or something in the packed-refs
> > code, though I find it pretty unlikely, as it's fairly well exercised in
> > normal use.
> 
> The above excerpts completely describe the issue and there is no more
> special in the repo.

Thanks for digging. Your explanation makes sense.

Show 6 quoted lines

> I was surprised that such an issue can remain in the
> `.git/packed-refs` for more than a year.
> I thought that commands like `git fsck` would report such unordering
> problems and `git gc` or `git pack-refs`
> make ordering checks and make full resorting of the `.git/packed-refs`
> file in case of issues with ordering.

I don't think we have any fsck checks that the packed-refs file is in sorted order. It might be reasonable to have them. Likewise, when pack-refs rewrites the file, it should be able to cheaply double-check that the input is sorted by comparing each entry against its previous.

I'm not _too_ surprised that the repo could persist for a while with an out-of-order packed-refs file, especially if you are not doing writes that would trigger a gc. Lookups are done by binary-search (and thus require sorting), but many operations will just iterate over the whole list and don't care. For an infrequently used ref, you might get unlucky and fall on it during a binary search, but otherwise it wouldn't affect most results.

> I understand that the `.git/packed-refs` file is for machines and not
> for humans but sometimes
> it's the fastest way to make several simple corrections in it manually.

Yes, I have certainly done this myself. There are is a header line at the top of the file, though, which tells what is guaranteed:

  $ head -1 .git/packed-refs
  # pack-refs with: peeled fully-peeled sorted

if you are going to muck with the file, deleting that line should be sufficient; the reading code will fall back to the older routines which don't assume it's sorted.

-Peff