Re: [PATCH v5 3/6] pack-bitmap-write: learn pack.writeBitmapLookupTable and add tests

From: Abhradeep Chakraborty <chakrabortyabhradeep79@gmail.com>
Date: Aug 9, 2022, 12:03 UTC
Message-ID: <CAPOJW5yNQvO3quG91jjC9pT-+NNhJta+H_E2R9-1wUzR+rPXnw@mail.gmail.com>
In-Reply-To: <s714sq49-o13q-5417-0o21-6397s3646q9o@tzk.qr>

On Tue, Aug 9, 2022 at 2:33 PM Johannes Schindelin <Johannes.Schindelin@gmx.de> wrote:

Show 33 quoted lines

>
> Hi Abhradeep,
>
> On Mon, 8 Aug 2022, Abhradeep Chakraborty wrote:
>
> > On Mon, Aug 8, 2022 at 6:36 PM Johannes Schindelin
> > <Johannes.Schindelin@gmx.de> wrote:
> > >
> > > On Tue, 2 Aug 2022, Abhradeep Chakraborty wrote:
> > >
> > > > On Tue, Aug 2, 2022 at 9:05 PM Johannes Schindelin
> > > > <Johannes.Schindelin@gmx.de> wrote:
> > > >
> > > > > Since you are very familiar with the details of bitmaps now, I would
> > > > > like to encourage you to work on some kind of validator/inspector,
> > > > > e.g. something along the lines of a `test-tool midx-bitmap dump` (and
> > > > > later `... verify`) that would help future you (and future me)
> > > > > investigate similar breakages. Ideally, that tool will not only parse
> > > > > the `.bitmap` file but immediately print out everything in a
> > > > > human-readable form.
> > >
> > > Have you made progress on this? I am interested mostly because I am trying
> > > very hard to maintain passing CI runs of Git for Windows' `shears/seen`
> > > branch (which essentially tries to rebase all of Git for Windows' patches
> > > on top of `seen`), and this failure is consistently causing said CI runs
> > > to fail for a while already.
> >
> > Hey Dscho, I am trying hard to solve the issue but unfortunately I
> > haven't found the key yet.
>
> The tool I proposed could potentially help, in particular with
> distributing the burden of the investigation on more shoulders than just
> yours.

Yeah, it should. I thought that I would do that after fixing the bug. Now I think I was wrong.

Show 7 quoted lines

> > I investigated the bitmap code-base and used debug lines but didn't
> > find a way to fix it.
>
> Have you investigated whether the `.bitmap` file was produced for the
> latest set of pack files? It should be relatively quick to investigate
> that, and if it turns out not to be the case, the fix should be quick,
> too.

Frankly speaking, I doubt that the generated multi-pack-index file is faulty. The first reason is the `.bitmap` filename. As you said before (and as I noticed here), `.bitmap` filenames in failing case and in passing case are different. As far as I know the hash value in the filename depends on the content of its respective midx file. So, if the midx contents were the same in both cases, `.bitmap` filename should not differ.

I compared both the multi-pack-index files (i.e. passing case and failing case) using `cmp ./trash\ directory.t5326-multi-pack-bitmaps/.git/objects/pack/multi-pack-index ../tmp/trash\ directory.t5326-multi-pack-bitmaps/.git/objects/pack/multi-pack-index` and found that these both defers -

    differ: char 3124, line 10

I also checked whether the `packing_data->in_pack_by_idx` contained all the packs. For this I wrote a debug error message in `prepare_in_pack_by_idx()[1]` function and found that `packing_data` is using the latest packs.

 I noticed in the 'setup partial bitmaps' test case that if we comment
out the line `git repack &&` , it runs successfully.

    test_expect_success 'setup partial bitmaps' '
        test_commit packed &&
        # git repack &&
        test_commit loose &&
        git multi-pack-index write --bitmap 2>err &&
        ...
    '