git/list[1] front-page[2] threads[3] people[4] search[5] about
 

[PATCH 03/10] builtin/gc.c: ignore cruft packs with `--keep-largest-pack`

From
Taylor Blau <me@ttaylorr.com>
Date
Apr 17, 2023, 20:54 UTC
Message-ID
<796df920ad6af0ee9101a0f3f80edbc793987336.1681764848.git.me@ttaylorr.com>
In-Reply-To
<cover.1681764848.git.me@ttaylorr.com>

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.

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>
---
 Documentation/config/gc.txt | 10 ++++-----
 Documentation/git-gc.txt    |  7 +++---
 builtin/gc.c                |  2 +-
 t/t6500-gc.sh               | 43 +++++++++++++++++++++++++++++++++++++
 4 files changed, 53 insertions(+), 9 deletions(-)
diff --git a/Documentation/config/gc.txt b/Documentation/config/gc.txt
index 38fea076a2..8d5353e9e0 100644
--- 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 a65c9aa62d..2427478314 100644
--- 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 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 edd98d35a5..53ef137e1d 100644
--- 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 d9acb63951..df6f2e6e52 100755
--- 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
-- 
2.38.1
Previous: Taylor BlauNext: Junio C Hamano
Message 6 of 48 in “gc: enable cruft packs by default”
  1. 00/10 gc: enable cruft packs by defaultTaylor Blau, Apr 17, 2023
  2. 01/10 pack-write.c: plug a leak in stage_tmp_packfiles()Taylor Blau, Apr 17, 2023
  3. Jeff KingApr 18, 2023
  4. Taylor BlauApr 18, 2023
  5. 02/10 builtin/repack.c: fix incorrect reference to '-C'Taylor Blau, Apr 17, 2023
  6. 03/10 builtin/gc.c: ignore cruft packs with `--keep-largest-pack`Taylor Blau, Apr 17, 2023
  7. Junio C HamanoApr 17, 2023
  8. Taylor BlauApr 17, 2023
  9. Jeff KingApr 18, 2023
  10. Derrick StoleeApr 18, 2023
  11. 04/10 t/t5304-prune.sh: prepare for `gc --cruft` by defaultTaylor Blau, Apr 17, 2023
  12. 05/10 t/t9300-fast-import.sh: prepare for `gc --cruft` by defaultTaylor Blau, Apr 17, 2023
  13. Jeff KingApr 18, 2023
  14. Taylor BlauApr 18, 2023
  15. 06/10 t/t6500-gc.sh: refactor cruft pack testsTaylor Blau, Apr 17, 2023
  16. 07/10 t/t6500-gc.sh: add additional test casesTaylor Blau, Apr 17, 2023
  17. Jeff KingApr 18, 2023
  18. Taylor BlauApr 18, 2023
  19. 08/10 t/t6501-freshen-objects.sh: prepare for `gc --cruft` by defaultTaylor Blau, Apr 17, 2023
  20. Jeff KingApr 18, 2023
  21. Taylor BlauApr 18, 2023
  22. Jeff KingApr 22, 2023
  23. 09/10 builtin/gc.c: make `gc.cruftPacks` enabled by defaultTaylor Blau, Apr 17, 2023
  24. Jeff KingApr 18, 2023
  25. Taylor BlauApr 18, 2023
  26. 10/10 repository.h: drop unused `gc_cruft_packs`Taylor Blau, Apr 17, 2023
  27. Jeff KingApr 18, 2023
  28. Jeff KingApr 18, 2023
  29. Taylor BlauApr 18, 2023
  30. 00/10 gc: enable cruft packs by defaultTaylor Blau, Apr 18, 2023
  31. 01/10 pack-write.c: plug a leak in stage_tmp_packfiles()Taylor Blau, Apr 18, 2023
  32. Junio C HamanoApr 19, 2023
  33. Taylor BlauApr 20, 2023
  34. Junio C HamanoApr 20, 2023
  35. 02/10 builtin/repack.c: fix incorrect reference to '-C'Taylor Blau, Apr 18, 2023
  36. 03/10 builtin/gc.c: ignore cruft packs with `--keep-largest-pack`Taylor Blau, Apr 18, 2023
  37. 04/10 t/t5304-prune.sh: prepare for `gc --cruft` by defaultTaylor Blau, Apr 18, 2023
  38. 05/10 t/t6501-freshen-objects.sh: prepare for `gc --cruft` by defaultTaylor Blau, Apr 18, 2023
  39. 06/10 t/t6500-gc.sh: refactor cruft pack testsTaylor Blau, Apr 18, 2023
  40. 07/10 t/t6500-gc.sh: add additional test casesTaylor Blau, Apr 18, 2023
  41. 08/10 t/t9300-fast-import.sh: prepare for `gc --cruft` by defaultTaylor Blau, Apr 18, 2023
  42. 09/10 builtin/gc.c: make `gc.cruftPacks` enabled by defaultTaylor Blau, Apr 18, 2023
  43. Junio C HamanoApr 19, 2023
  44. Taylor BlauApr 20, 2023
  45. Junio C HamanoApr 20, 2023
  46. Taylor BlauApr 20, 2023
  47. 10/10 repository.h: drop unused `gc_cruft_packs`Taylor Blau, Apr 18, 2023
  48. Junio C HamanoApr 19, 2023

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.