builtin/repack: fix `--keep-unreachable` when there are no packs

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.

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@pks.im>
Acked-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

diff --git a/builtin/repack.c b/builtin/repack.c
index 0c6dad7df47a1665026a348921c33b2067b59976..ccbf0a5241aeca1b5f8fe35ab2d8a6959ef277eb 100644 (file)
--- a/builtin/repack.c
+++ b/builtin/repack.c
@@ -1370,9 +1370,12 @@ int cmd_repack(int argc,
                                            "--unpack-unreachable");
                        } else if (keep_unreachable) {
                                strvec_push(&cmd.args, "--keep-unreachable");
-                               strvec_push(&cmd.args, "--pack-loose-unreachable");
                        }
                }
+
+               if (keep_unreachable && delete_redundant &&
+                   !(pack_everything & PACK_CRUFT))
+                       strvec_push(&cmd.args, "--pack-loose-unreachable");
        } else if (geometry.split_factor) {
                strvec_push(&cmd.args, "--stdin-packs");
                strvec_push(&cmd.args, "--unpacked");

diff --git a/t/t7701-repack-unpack-unreachable.sh b/t/t7701-repack-unpack-unreachable.sh
index 5715f4d69a47632c76e6b31c0719b24264c636e0..5559d4ccb429e0ba9353a5acb7413c5f5f922112 100755 (executable)
--- a/t/t7701-repack-unpack-unreachable.sh
+++ b/t/t7701-repack-unpack-unreachable.sh
@@ -195,4 +195,20 @@ test_expect_success 'repack -k packs unreachable loose objects' '
        git cat-file -p $sha1
 '
 
+test_expect_success 'repack -k packs unreachable loose objects without existing packfiles' '
+       test_when_finished "rm -rf repo" &&
+       git init repo &&
+       (
+               cd repo &&
+
+               oid=$(echo would-be-deleted-loose | git hash-object -w --stdin) &&
+               objpath=.git/objects/$(echo $sha1 | sed "s,..,&/,") &&
+               test_path_is_file $objpath &&
+
+               git repack -ad --keep-unreachable &&
+               test_path_is_missing $objpath &&
+               git cat-file -p $oid
+       )
+'
+
 test_done