On Fri, Feb 07, 2020 at 07:42:52AM +0100, Christian Couder wrote: > On Thu, Feb 6, 2020 at 12:36 AM Junio C Hamano <gitster@xxxxxxxxx> wrote: > > > * jk/packfile-reuse-cleanup (2020-01-23) 12 commits > > - pack-bitmap: don't rely on bitmap_git->reuse_objects > > - pack-objects: add checks for duplicate objects > > - pack-objects: improve partial packfile reuse > > - builtin/pack-objects: introduce obj_is_packed() > > - pack-objects: introduce pack.allowPackReuse > > - csum-file: introduce hashfile_total() > > - pack-bitmap: simplify bitmap_has_oid_in_uninteresting() > > - pack-bitmap: uninteresting oid can be outside bitmapped packfile > > - pack-bitmap: introduce bitmap_walk_contains() > > - ewah/bitmap: introduce bitmap_word_alloc() > > - packfile: expose get_delta_base() > > - builtin/pack-objects: report reused packfile objects > > > > The way "git pack-objects" reuses objects stored in existing pack > > to generate its result has been improved. > > > > Will merge to 'next'? > > I think that it would be indeed ok to merge to next if no one comments > further following the v4 [1] and my last reply to Peff [2] > > [1] https://lore.kernel.org/git/20191218112547.4974-1-chriscool@xxxxxxxxxxxxx/ > [2] https://lore.kernel.org/git/CAP8UFD0h1TFVNqH7g823psaQzmEmzoz200CkZuDOV8GqNV7mrQ@xxxxxxxxxxxxxx/ > > According to Peff the code in this patch series has been running by > GitHub for years. This patch series is also now used in a custom Git > that has been running on gitlab.com for a few days. This custom Git is > planned to be part of the GitLab 12.8 release later this month. Yeah, I think the only lingering issue is that one I pointed out where the big "initial chunk" reuse doesn't carefully check for any REF_DELTA objects whose bases we're not including. But that exists in the current scheme, and in what we've been running at GitHub for a long time. And is probably OK by the reasoning I gave in the thread: these packs must have bitmaps, and therefore we wrote them as non-thin, and therefore they would have only OFS deltas. I think it would also be OK even if you tried repacking without --delta-base-offset, because we always put bases before their deltas (aside from --fix-thin, maybe?) So I'd be OK to proceed with the series as-is, but it does give me a slight worry. Both that this assumption could somehow be violated, but also that the other parts of the code trying to handle REF_DELTAs are probably not getting exercised at all. It would be kind of interesting to cover this case with a test, but I'm not even sure there's a way that you could generate such a pack with `pack-objects` because of the bases-before-deltas ordering constraint. -Peff
