Patrick Steinhardt <ps@xxxxxx> writes: > The "--keep-unreachable" flag is supposed to append any unreachable > objects to the newly written pack. This flag is explicitly documented as > appending both packed and loose unreachable objects to the new packfile. > And while this works alright when repacking with preexisting packfiles, > it stops working when the repository does not have any packfiles at all. Chuckle. That's a cute corner case the developers never considered, it seems ;-). > The root cause are the conditions used to decide whether or not we want > to append "--pack-loose-unreachable" to git-pack-objects(1). There are > a couple of conditions here: > > - `has_existing_non_kept_packs()` checks whether there are existing > packfiles. This condition makes sense to guard "--keep-pack=", > "--unpack-unreachable" and "--keep-unreachable", because all of > these flags only make sense in combination with existing packfiles. > But it does not make sense to disable `--pack-loose-unreachable` > when there aren't any preexisting packfiles, as loose objects can be > packed into the new packfile regardless of that. > > - `delete_redundant` checks whether we want to delete any objects or > packs that are about to become redundant. The documentation of > `--keep-unreachable` explicitly says that `git repack -ad` needs to > be executed for the flag to have an effect. > > It is not immediately obvious why such redundant objects need to be > deleted in order for "--pack-unreachable-objects" to be effective. > But as things are working as documented this is nothing we'll change > for now. > > - `pack_everything & PACK_CRUFT` checks that we're not creating a > cruft pack. This condition makes sense in the context of > "--pack-loose-unreachable", as unreachable objects would end up in > the cruft pack anyway. > > So while the second and third condition are sensible, it does not make > any sense to condition `--pack-loose-unreachable` on the existence of > packfiles. > > Fix the bug by splitting out the "--pack-loose-unreachable" and only > making it depend on the second and third condition. Like this, loose > unreachable objects will be packed regardless of any preexisting > packfiles. > > Signed-off-by: Patrick Steinhardt <ps@xxxxxx> > --- > builtin/repack.c | 5 ++++- > t/t7700-repack.sh | 2 +- > 2 files changed, 5 insertions(+), 2 deletions(-) Nicely analized and described. Thanks.
