On Mon, Apr 15, 2024 at 08:41:25AM +0200, Patrick Steinhardt wrote: > In 0fea6b73f1 (Merge branch 'tb/multi-pack-verbatim-reuse', 2024-01-12) > we have introduced multi-pack verbatim reuse of objects. This series has > introduced a new BTMP chunk, which encodes information about bitmapped > objects in the multi-pack index. Starting with dab60934e3 (pack-bitmap: > pass `bitmapped_pack` struct to pack-reuse functions, 2023-12-14) we use > this information to figure out objects which we can reuse from each of > the packfiles. > > One thing that we glossed over though is backwards compatibility with > repositories that do not yet have BTMP chunks in their multi-pack index. > In that case, `nth_bitmapped_pack()` would return an error, which causes > us to emit a warning followed by another error message. These warnings > are visible to users that fetch from a repository: > > ``` > $ git fetch > ... > remote: error: MIDX does not contain the BTMP chunk > remote: warning: unable to load pack: 'pack-f6bb7bd71d345ea9fe604b60cab9ba9ece54ffbe.idx', disabling pack-reuse > remote: Enumerating objects: 40, done. > remote: Counting objects: 100% (40/40), done. > remote: Compressing objects: 100% (39/39), done. > remote: Total 40 (delta 5), reused 0 (delta 0), pack-reused 0 (from 0) > ... > ``` > > While the fetch succeeds the user is left wondering what they did wrong. > Furthermore, as visible both from the warning and from the reuse stats, > pack-reuse is completely disabled in such repositories. > > What is quite interesting is that this issue can even be triggered in > case `pack.allowPackReuse=single` is set, which is the default value. > One could have expected that in this case we fall back to the old logic, > which is to use the preferred packfile without consulting BTMP chunks at > all. But either we fail with the above error in case they are missing, > or we use the first pack in the multi-pack-index. The former case > disables pack-reuse altogether, whereas the latter case may result in > reusing objects from a suboptimal packfile. > > Fix this issue by partially reverting the logic back to what we had > before this patch series landed. Namely, in the case where we have no > BTMP chunks or when `pack.allowPackReuse=single` are set, we use the > preferred pack instead of consulting the BTMP chunks. > > Helped-by: Taylor Blau <me@xxxxxxxxxxxx> > Signed-off-by: Patrick Steinhardt <ps@xxxxxx> Junio, it would be great if we could still land this fix in Git v2.45 given that it is addressing a regression in Git v2.44. This of course assumes that the current version of this patch looks good to Taylor. Patrick
Attachment:
signature.asc
Description: PGP signature
