Re: [External] Re: [RFC PATCH resend] builtin/repack: Honor --keep-pack and .keep when repacking promisor objects

From: Han Young <hanyang.tony@bytedance.com>
Date: Feb 19, 2025, 12:55 UTC
Message-ID: <CAG1j3zG-FcGZe-64dmZJOAitjKsnbB5KUUkmQt6edyhM7z_NTw@mail.gmail.com>
In-Reply-To: <2289498.vFx2qVVIhK@electra>

On Mon, Feb 10, 2025 at 8:53 PM Tomáš Trnka <trnka@scm.com> wrote:

> With my patch, gc did not touch these two big packs but still collected all
> the small ones into one new pack as expected. Everything else also seems to
> work fine.

Sorry for the long wait. I have tested the patch against a repo with only promisor packs. There are one big promisor pack and many small ones. This patch do work as expected, I'll take back the earlier

"... does not prevent the keep promisor packs from being repacked."

Show 6 quoted lines

> According to the man page for git-pack-objects, it seems to me that this is
> how it's meant to work, because the description for --keep-pack says "This
> flag causes an object already in the given pack to be ignored, even if it
> would have otherwise been packed." (and something similar for --honor-pack-
> keep). To my untrained eyes, it looks like that's also how
> want_found_object()/add_object_entry() in pack-objects.c handle it.

This is also true. However, the for_each_packed_object macro in repack.c does not ignore the keep packs. repack still iterating objects in keep packs and sending them to pack-objects. pack-objects will then exclude these objects. To avoid doing unnecessary work, objects in keep packs should not be send over to pack-objects. Checking if the object should be ignore takes some time, after all.

As for the test, t0410-partial-clone.sh is a better place imo. For the test is for the partial-clone repos.

Thanks.