Skip to content

Commit 05b9013

Browse files
ttaylorrgitster
authored andcommitted
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 <[email protected]> Signed-off-by: Junio C Hamano <[email protected]>
1 parent c512f31 commit 05b9013

File tree

4 files changed

+53
-9
lines changed

4 files changed

+53
-9
lines changed

Documentation/config/gc.txt

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -43,11 +43,11 @@ gc.autoDetach::
4343
if the system supports it. Default is true.
4444

4545
gc.bigPackThreshold::
46-
If non-zero, all packs larger than this limit are kept when
47-
`git gc` is run. This is very similar to `--keep-largest-pack`
48-
except that all packs that meet the threshold are kept, not
49-
just the largest pack. Defaults to zero. Common unit suffixes of
50-
'k', 'm', or 'g' are supported.
46+
If non-zero, all non-cruft packs larger than this limit are kept
47+
when `git gc` is run. This is very similar to
48+
`--keep-largest-pack` except that all non-cruft packs that meet
49+
the threshold are kept, not just the largest pack. Defaults to
50+
zero. Common unit suffixes of 'k', 'm', or 'g' are supported.
5151
+
5252
Note that if the number of kept packs is more than gc.autoPackLimit,
5353
this configuration variable is ignored, all packs except the base pack

Documentation/git-gc.txt

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -77,9 +77,10 @@ be performed as well.
7777
instance running on this repository.
7878

7979
--keep-largest-pack::
80-
All packs except the largest pack and those marked with a
81-
`.keep` files are consolidated into a single pack. When this
82-
option is used, `gc.bigPackThreshold` is ignored.
80+
All packs except the largest non-cruft pack, any packs marked
81+
with a `.keep` file, and any cruft pack(s) are consolidated into
82+
a single pack. When this option is used, `gc.bigPackThreshold`
83+
is ignored.
8384

8485
AGGRESSIVE
8586
----------

builtin/gc.c

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -219,7 +219,7 @@ static struct packed_git *find_base_packs(struct string_list *packs,
219219
struct packed_git *p, *base = NULL;
220220

221221
for (p = get_all_packs(the_repository); p; p = p->next) {
222-
if (!p->pack_local)
222+
if (!p->pack_local || p->is_cruft)
223223
continue;
224224
if (limit) {
225225
if (p->pack_size >= limit)

t/t6500-gc.sh

Lines changed: 43 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -298,6 +298,49 @@ test_expect_success 'feature.experimental=false avoids cruft packs by default' '
298298
)
299299
'
300300

301+
test_expect_success '--keep-largest-pack ignores cruft packs' '
302+
test_when_finished "rm -fr repo" &&
303+
git init repo &&
304+
(
305+
cd repo &&
306+
307+
# Generate a pack for reachable objects (of which there
308+
# are 3), and one for unreachable objects (of which
309+
# there are 6).
310+
prepare_cruft_history &&
311+
git gc --cruft &&
312+
313+
mtimes="$(find .git/objects/pack -type f -name "pack-*.mtimes")" &&
314+
sz="$(test_file_size "${mtimes%.mtimes}.pack")" &&
315+
316+
# Ensure that the cruft pack gets removed (due to
317+
# `--prune=now`) despite it being the largest pack.
318+
git -c gc.bigPackThreshold=$sz gc --cruft --prune=now &&
319+
320+
assert_no_cruft_packs
321+
)
322+
'
323+
324+
test_expect_success 'gc.bigPackThreshold ignores cruft packs' '
325+
test_when_finished "rm -fr repo" &&
326+
git init repo &&
327+
(
328+
cd repo &&
329+
330+
# Generate a pack for reachable objects (of which there
331+
# are 3), and one for unreachable objects (of which
332+
# there are 6).
333+
prepare_cruft_history &&
334+
git gc --cruft &&
335+
336+
# Ensure that the cruft pack gets removed (due to
337+
# `--prune=now`) despite it being the largest pack.
338+
git gc --cruft --prune=now --keep-largest-pack &&
339+
340+
assert_no_cruft_packs
341+
)
342+
'
343+
301344
run_and_wait_for_auto_gc () {
302345
# We read stdout from gc for the side effect of waiting until the
303346
# background gc process exits, closing its fd 9. Furthermore, the

0 commit comments

Comments
 (0)