Merge branch 'tb/enable-cruft-packs-by-default'

When "gc" needs to retain unreachable objects, packing them into
cruft packs (instead of exploding them into loose object files) has
been offered as a more efficient option for some time.  Now the use
of cruft packs has been made the default and no longer considered
an experimental feature.

* tb/enable-cruft-packs-by-default:
  repository.h: drop unused `gc_cruft_packs`
  builtin/gc.c: make `gc.cruftPacks` enabled by default
  t/t9300-fast-import.sh: prepare for `gc --cruft` by default
  t/t6500-gc.sh: add additional test cases
  t/t6500-gc.sh: refactor cruft pack tests
  t/t6501-freshen-objects.sh: prepare for `gc --cruft` by default
  t/t5304-prune.sh: prepare for `gc --cruft` by default
  builtin/gc.c: ignore cruft packs with `--keep-largest-pack`
  builtin/repack.c: fix incorrect reference to '-C'
  pack-write.c: plug a leak in stage_tmp_packfiles()

4 files changed, 92 insertions, 92 deletions

diff --git a/t/t5304-prune.sh b/t/t5304-prune.sh
index 5500dd0842..662ae9b152 100755
--- a/t/t5304-prune.sh
+++ b/t/t5304-prune.sh
@@ -62,11 +62,11 @@ test_expect_success 'prune --expire' '
 test_expect_success 'gc: implicit prune --expire' '
 	add_blob &&
 	test-tool chmtime =-$((2*$week-30)) $BLOB_FILE &&
-	git gc &&
+	git gc --no-cruft &&
 	verbose test $((1 + $before)) = $(git count-objects | sed "s/ .*//") &&
 	test_path_is_file $BLOB_FILE &&
 	test-tool chmtime =-$((2*$week+1)) $BLOB_FILE &&
-	git gc &&
+	git gc --no-cruft &&
 	verbose test $before = $(git count-objects | sed "s/ .*//") &&
 	test_path_is_missing $BLOB_FILE
 '
@@ -86,7 +86,7 @@ test_expect_success 'gc: refuse to start with invalid gc.pruneExpire' '
 
 test_expect_success 'gc: start with ok gc.pruneExpire' '
 	git config gc.pruneExpire 2.days.ago &&
-	git gc
+	git gc --no-cruft
 '
 
 test_expect_success 'prune: prune nonsense parameters' '
@@ -137,44 +137,44 @@ test_expect_success 'gc --no-prune' '
 	add_blob &&
 	test-tool chmtime =-$((5001*$day)) $BLOB_FILE &&
 	git config gc.pruneExpire 2.days.ago &&
-	git gc --no-prune &&
+	git gc --no-prune --no-cruft &&
 	verbose test 1 = $(git count-objects | sed "s/ .*//") &&
 	test_path_is_file $BLOB_FILE
 '
 
 test_expect_success 'gc respects gc.pruneExpire' '
 	git config gc.pruneExpire 5002.days.ago &&
-	git gc &&
+	git gc --no-cruft &&
 	test_path_is_file $BLOB_FILE &&
 	git config gc.pruneExpire 5000.days.ago &&
-	git gc &&
+	git gc --no-cruft &&
 	test_path_is_missing $BLOB_FILE
 '
 
 test_expect_success 'gc --prune=<date>' '
 	add_blob &&
 	test-tool chmtime =-$((5001*$day)) $BLOB_FILE &&
-	git gc --prune=5002.days.ago &&
+	git gc --prune=5002.days.ago --no-cruft &&
 	test_path_is_file $BLOB_FILE &&
-	git gc --prune=5000.days.ago &&
+	git gc --prune=5000.days.ago --no-cruft &&
 	test_path_is_missing $BLOB_FILE
 '
 
 test_expect_success 'gc --prune=never' '
 	add_blob &&
-	git gc --prune=never &&
+	git gc --prune=never --no-cruft &&
 	test_path_is_file $BLOB_FILE &&
-	git gc --prune=now &&
+	git gc --prune=now --no-cruft &&
 	test_path_is_missing $BLOB_FILE
 '
 
 test_expect_success 'gc respects gc.pruneExpire=never' '
 	git config gc.pruneExpire never &&
 	add_blob &&
-	git gc &&
+	git gc --no-cruft &&
 	test_path_is_file $BLOB_FILE &&
 	git config gc.pruneExpire now &&
-	git gc &&
+	git gc --no-cruft &&
 	test_path_is_missing $BLOB_FILE
 '
 
@@ -194,7 +194,7 @@ test_expect_success 'gc: prune old objects after local clone' '
 		cd aclone &&
 		verbose test 1 = $(git count-objects | sed "s/ .*//") &&
 		test_path_is_file $BLOB_FILE &&
-		git gc --prune &&
+		git gc --prune --no-cruft &&
 		verbose test 0 = $(git count-objects | sed "s/ .*//") &&
 		test_path_is_missing $BLOB_FILE
 	)
@@ -237,7 +237,7 @@ test_expect_success 'clean pack garbage with gc' '
 	>.git/objects/pack/fake2.keep &&
 	>.git/objects/pack/fake2.idx &&
 	>.git/objects/pack/fake3.keep &&
-	git gc &&
+	git gc --no-cruft &&
 	git count-objects -v 2>stderr &&
 	grep "^warning:" stderr | sort >actual &&
 	cat >expected <<\EOF &&
diff --git a/t/t6500-gc.sh b/t/t6500-gc.sh
index d9acb63951..69509d0c11 100755
--- a/t/t6500-gc.sh
+++ b/t/t6500-gc.sh
@@ -210,90 +210,95 @@ prepare_cruft_history () {
 	git reset HEAD^^
 }
 
-assert_cruft_packs () {
-	find .git/objects/pack -name "*.mtimes" >mtimes &&
-	sed -e 's/\.mtimes$/\.pack/g' mtimes >packs &&
-
-	test_file_not_empty packs &&
-	while read pack
-	do
-		test_path_is_file "$pack" || return 1
-	done <packs
-}
-
 assert_no_cruft_packs () {
 	find .git/objects/pack -name "*.mtimes" >mtimes &&
 	test_must_be_empty mtimes
 }
 
-test_expect_success 'gc --cruft generates a cruft pack' '
-	test_when_finished "rm -fr crufts" &&
-	git init crufts &&
+for argv in \
+	"gc" \
+	"-c gc.cruftPacks=true gc" \
+	"-c gc.cruftPacks=false gc --cruft"
+do
+	test_expect_success "git $argv generates a cruft pack" '
+		test_when_finished "rm -fr repo" &&
+		git init repo &&
+		(
+			cd repo &&
+
+			prepare_cruft_history &&
+			git $argv &&
+
+			find .git/objects/pack -name "*.mtimes" >mtimes &&
+			sed -e 's/\.mtimes$/\.pack/g' mtimes >packs &&
+
+			test_file_not_empty packs &&
+			while read pack
+			do
+				test_path_is_file "$pack" || return 1
+			done <packs
+		)
+	'
+done
+
+for argv in \
+	"gc --no-cruft" \
+	"-c gc.cruftPacks=false gc" \
+	"-c gc.cruftPacks=true gc --no-cruft"
+do
+	test_expect_success "git $argv does not generate a cruft pack" '
+		test_when_finished "rm -fr repo" &&
+		git init repo &&
+		(
+			cd repo &&
+
+			prepare_cruft_history &&
+			git $argv &&
+
+			assert_no_cruft_packs
+		)
+	'
+done
+
+test_expect_success '--keep-largest-pack ignores cruft packs' '
+	test_when_finished "rm -fr repo" &&
+	git init repo &&
 	(
-		cd crufts &&
+		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 &&
-		assert_cruft_packs
-	)
-'
-
-test_expect_success 'gc.cruftPacks=true generates a cruft pack' '
-	test_when_finished "rm -fr crufts" &&
-	git init crufts &&
-	(
-		cd crufts &&
-
-		prepare_cruft_history &&
-		git -c gc.cruftPacks=true gc &&
-		assert_cruft_packs
-	)
-'
-
-test_expect_success 'feature.experimental=true generates a cruft pack' '
-	git init crufts &&
-	test_when_finished "rm -fr crufts" &&
-	(
-		cd crufts &&
 
-		prepare_cruft_history &&
-		git -c feature.experimental=true gc &&
-		assert_cruft_packs
-	)
-'
+		mtimes="$(find .git/objects/pack -type f -name "pack-*.mtimes")" &&
+		sz="$(test_file_size "${mtimes%.mtimes}.pack")" &&
 
-test_expect_success 'feature.experimental=false allows explicit cruft packs' '
-	git init crufts &&
-	test_when_finished "rm -fr crufts" &&
-	(
-		cd crufts &&
+		# 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 &&
 
-		prepare_cruft_history &&
-		git -c gc.cruftPacks=true -c feature.experimental=false gc &&
-		assert_cruft_packs
+		assert_no_cruft_packs
 	)
 '
 
-test_expect_success 'feature.experimental=true can be overridden' '
-	git init crufts &&
-	test_when_finished "rm -fr crufts" &&
+test_expect_success 'gc.bigPackThreshold ignores cruft packs' '
+	test_when_finished "rm -fr repo" &&
+	git init repo &&
 	(
-		cd crufts &&
+		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 -c feature.expiremental=true -c gc.cruftPacks=false gc &&
-		assert_no_cruft_packs
-	)
-'
+		git gc --cruft &&
 
-test_expect_success 'feature.experimental=false avoids cruft packs by default' '
-	git init crufts &&
-	test_when_finished "rm -fr crufts" &&
-	(
-		cd crufts &&
+		# 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 &&
 
-		prepare_cruft_history &&
-		git -c feature.experimental=false gc &&
 		assert_no_cruft_packs
 	)
 '
diff --git a/t/t6501-freshen-objects.sh b/t/t6501-freshen-objects.sh
index 3968b47ed5..dbfa8a4d4c 100755
--- a/t/t6501-freshen-objects.sh
+++ b/t/t6501-freshen-objects.sh
@@ -101,7 +101,7 @@ do
 	'
 
 	test_expect_success "simultaneous gc ($title)" '
-		git gc --prune=12.hours.ago
+		git gc --no-cruft --prune=12.hours.ago
 	'
 
 	test_expect_success "finish writing out commit ($title)" '
@@ -131,7 +131,7 @@ do
 	'
 
 	test_expect_success "simultaneous gc ($title)" '
-		git gc --prune=12.hours.ago
+		git gc --no-cruft --prune=12.hours.ago
 	'
 
 	# tree should have been refreshed by write-tree
@@ -151,7 +151,7 @@ test_expect_success 'do not complain about existing broken links (commit)' '
 	some message
 	EOF
 	commit=$(git hash-object -t commit -w broken-commit) &&
-	git gc -q 2>stderr &&
+	git gc --no-cruft -q 2>stderr &&
 	verbose git cat-file -e $commit &&
 	test_must_be_empty stderr
 '
@@ -161,7 +161,7 @@ test_expect_success 'do not complain about existing broken links (tree)' '
 	100644 blob $(test_oid 003)	foo
 	EOF
 	tree=$(git mktree --missing <broken-tree) &&
-	git gc -q 2>stderr &&
+	git gc --no-cruft -q 2>stderr &&
 	git cat-file -e $tree &&
 	test_must_be_empty stderr
 '
@@ -176,7 +176,7 @@ test_expect_success 'do not complain about existing broken links (tag)' '
 	this is a broken tag
 	EOF
 	tag=$(git hash-object -t tag -w broken-tag) &&
-	git gc -q 2>stderr &&
+	git gc --no-cruft -q 2>stderr &&
 	git cat-file -e $tag &&
 	test_must_be_empty stderr
 '
diff --git a/t/t9300-fast-import.sh b/t/t9300-fast-import.sh
index aa55b41b9a..ac237a1f90 100755
--- a/t/t9300-fast-import.sh
+++ b/t/t9300-fast-import.sh
@@ -388,9 +388,7 @@ test_expect_success 'B: accept branch name "TEMP_TAG"' '
 
 	INPUT_END
 
-	test_when_finished "rm -f .git/TEMP_TAG
-		git gc
-		git prune" &&
+	test_when_finished "rm -f .git/TEMP_TAG && git gc --prune=now" &&
 	git fast-import <input &&
 	test $(test-tool ref-store main resolve-ref TEMP_TAG 0 | cut -f1 -d " " ) != "$ZERO_OID" &&
 	test $(git rev-parse main) = $(git rev-parse TEMP_TAG^)
@@ -406,8 +404,7 @@ test_expect_success 'B: accept empty committer' '
 	INPUT_END
 
 	test_when_finished "git update-ref -d refs/heads/empty-committer-1
-		git gc
-		git prune" &&
+		git gc --prune=now" &&
 	git fast-import <input &&
 	out=$(git fsck) &&
 	echo "$out" &&
@@ -452,8 +449,7 @@ test_expect_success 'B: accept and fixup committer with no name' '
 	INPUT_END
 
 	test_when_finished "git update-ref -d refs/heads/empty-committer-2
-		git gc
-		git prune" &&
+		git gc --prune=now" &&
 	git fast-import <input &&
 	out=$(git fsck) &&
 	echo "$out" &&
@@ -1778,8 +1774,7 @@ test_expect_success 'P: verbatim SHA gitlinks' '
 	INPUT_END
 
 	git branch -D sub &&
-	git gc &&
-	git prune &&
+	git gc --prune=now &&
 	git fast-import <input &&
 	test $(git rev-parse --verify subuse2) = $(git rev-parse --verify subuse1)
 '