builtin/gc.c: ignore cruft packs with `--keep-largest-pack`

When cruft packs were implemented, we never adjusted the code for `git
gc`'s `--keep-largest-pack` and `gc.bigPackThreshold` to ignore cruft
packs. This option and configuration option share a common
implementation, but including cruft packs is wrong in both cases:

  - Running `git gc --keep-largest-pack` in a repository where the
    largest pack is the cruft pack itself will make it impossible for
    `git gc` to prune objects, since the cruft pack itself is kept.

  - The same is true for `gc.bigPackThreshold`, if the size of the cruft
    pack exceeds the limit set by the caller.

In the future, it is possible that `gc.bigPackThreshold` could be used
to write a separate cruft pack containing any new unreachable objects
that entered the repository since the last time a cruft pack was
written.

There are some complexities to doing so, mainly around handling
pruning objects that are in an existing cruft pack that is above the
threshold (which would either need to be rewritten, or else delay
pruning). Rewriting a substantially similar cruft pack isn't ideal, but
it is significantly better than the status-quo.

If users have large cruft packs that they don't want to rewrite, they
can mark them as `*.keep` packs. But in general, if a repository has a
cruft pack that is so large it is slowing down GC's, it should probably
be pruned anyway.

In the meantime, ignore cruft packs in the common implementation for
both of these options, and add a pair of tests to prevent any future
regressions here.

Signed-off-by: Taylor Blau <me@ttaylorr.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

diff --git a/Documentation/config/gc.txt b/Documentation/config/gc.txt
index 38fea076a26d0f1f7bf137723baba945675901fc..8d5353e9e00eeaf21499b8bce70039f0ffdc3d9f 100644 (file)
--- a/Documentation/config/gc.txt
+++ b/Documentation/config/gc.txt
@@ -43,11 +43,11 @@ gc.autoDetach::
        if the system supports it. Default is true.
 
 gc.bigPackThreshold::
-       If non-zero, all packs larger than this limit are kept when
-       `git gc` is run. This is very similar to `--keep-largest-pack`
-       except that all packs that meet the threshold are kept, not
-       just the largest pack. Defaults to zero. Common unit suffixes of
-       'k', 'm', or 'g' are supported.
+       If non-zero, all non-cruft packs larger than this limit are kept
+       when `git gc` is run. This is very similar to
+       `--keep-largest-pack` except that all non-cruft packs that meet
+       the threshold are kept, not just the largest pack. Defaults to
+       zero. Common unit suffixes of 'k', 'm', or 'g' are supported.
 +
 Note that if the number of kept packs is more than gc.autoPackLimit,
 this configuration variable is ignored, all packs except the base pack

diff --git a/Documentation/git-gc.txt b/Documentation/git-gc.txt
index a65c9aa62d64112dfd03335af82a680e54d00c58..fef382a70fa693e2cd524ba397f912276e8c4c1f 100644 (file)
--- a/Documentation/git-gc.txt
+++ b/Documentation/git-gc.txt
@@ -77,9 +77,10 @@ be performed as well.
        instance running on this repository.
 
 --keep-largest-pack::
-       All packs except the largest pack and those marked with a
-       `.keep` files are consolidated into a single pack. When this
-       option is used, `gc.bigPackThreshold` is ignored.
+       All packs except the largest non-cruft pack, any packs marked
+       with a `.keep` file, and any cruft pack(s) are consolidated into
+       a single pack. When this option is used, `gc.bigPackThreshold`
+       is ignored.
 
 AGGRESSIVE
 ----------

diff --git a/builtin/gc.c b/builtin/gc.c
index edd98d35a5a460a06bd812e455be201a0309d334..53ef137e1d3e33d0ea5fbcc06472699b9894816a 100644 (file)
--- a/builtin/gc.c
+++ b/builtin/gc.c
@@ -219,7 +219,7 @@ static struct packed_git *find_base_packs(struct string_list *packs,
        struct packed_git *p, *base = NULL;
 
        for (p = get_all_packs(the_repository); p; p = p->next) {
-               if (!p->pack_local)
+               if (!p->pack_local || p->is_cruft)
                        continue;
                if (limit) {
                        if (p->pack_size >= limit)

diff --git a/t/t6500-gc.sh b/t/t6500-gc.sh
index d9acb639512ae6bdf30c8bdd4a0b4d498541dd84..df6f2e6e52c0ee34fbb8244f1263cdb66a9e3010 100755 (executable)
--- a/t/t6500-gc.sh
+++ b/t/t6500-gc.sh
@@ -298,6 +298,49 @@ test_expect_success 'feature.experimental=false avoids cruft packs by default' '
        )
 '
 
+test_expect_success '--keep-largest-pack ignores cruft packs' '
+       test_when_finished "rm -fr repo" &&
+       git init repo &&
+       (
+               cd repo &&
+
+               # Generate a pack for reachable objects (of which there
+               # are 3), and one for unreachable objects (of which
+               # there are 6).
+               prepare_cruft_history &&
+               git gc --cruft &&
+
+               mtimes="$(find .git/objects/pack -type f -name "pack-*.mtimes")" &&
+               sz="$(test_file_size "${mtimes%.mtimes}.pack")" &&
+
+               # Ensure that the cruft pack gets removed (due to
+               # `--prune=now`) despite it being the largest pack.
+               git -c gc.bigPackThreshold=$sz gc --cruft --prune=now &&
+
+               assert_no_cruft_packs
+       )
+'
+
+test_expect_success 'gc.bigPackThreshold ignores cruft packs' '
+       test_when_finished "rm -fr repo" &&
+       git init repo &&
+       (
+               cd repo &&
+
+               # Generate a pack for reachable objects (of which there
+               # are 3), and one for unreachable objects (of which
+               # there are 6).
+               prepare_cruft_history &&
+               git gc --cruft &&
+
+               # Ensure that the cruft pack gets removed (due to
+               # `--prune=now`) despite it being the largest pack.
+               git gc --cruft --prune=now --keep-largest-pack &&
+
+               assert_no_cruft_packs
+       )
+'
+
 run_and_wait_for_auto_gc () {
        # We read stdout from gc for the side effect of waiting until the
        # background gc process exits, closing its fd 9.  Furthermore, the