[PATCH v2] repack: do not rebuild packs on --dry-run

[Siddharth Shrimali <r.siddharth.shrimali@gmail.com>]

"git repack --drop-filtered --dry-run" is documented to list the
objects that would be dropped "without rebuilding any pack or
deleting anything", but it does both.

This is a bug in cmd_repack(): after printing the candidates, the
--dry-run block falls through into the regular repack code.
repack_promisor_objects() writes a new promisor pack, and with -d,
existing_packs_remove_redundant() deletes the old packs.

The existing guard only skips the implied "delete_redundant = 1", so
it does not stop an explicit -d, and it does not stop the new pack
from being written either.

This has been broken since --dry-run was introduced in 1e746b00aa
(builtin/repack: add --drop-filtered and --dry-run options). The
--dry-run block never returned early, so a dry run has always gone
on to do a regular repack. It was not broken by a later change.

Fix it by returning right after the candidates are listed, and add a
test that checks the pack directory is unchanged with and without -d.

Reported-by: Coy Geek <coygeek@gmail.com>
Helped-by: D. Ben Knoble <ben.knoble@gmail.com>
Signed-off-by: Siddharth Shrimali <r.siddharth.shrimali@gmail.com>
---
Bug report:
https://lore.kernel.org/git/CACgTecOm+=vbf50tZNXhcYvRi1ZTsQwbjVoJAbQqs2CmXdJCxg@mail.gmail.com/

Changes since v1:
 - reworded the comment
 - documented the bug in the log message

 builtin/repack.c                |  7 +++++++
 t/t7706-repack-drop-filtered.sh | 19 +++++++++++++++++++
 2 files changed, 26 insertions(+)

diff --git a/builtin/repack.c b/builtin/repack.c
index c4360382c1..7797972642 100644
--- a/builtin/repack.c
+++ b/builtin/repack.c
@@ -387,14 +387,21 @@ int cmd_repack(int argc,
 		if (dry_run) {
 			struct oidset_iter iter;
 			const struct object_id *oid;
 
 			oidset_iter_init(&drop_oids, &iter);
 			while ((oid = oidset_iter_next(&iter)))
 				printf("%s\n", oid_to_hex(oid));
+
+			/*
+			 * Only list the candidates to be dropped; skip
+			 * the actual repack
+			 */
+			ret = 0;
+			goto cleanup;
 		}
 	}
 
 	if (delete_redundant && repo->repository_format_precious_objects)
 		die(_("cannot delete packs in a precious-objects repo"));
 
 	die_for_incompatible_opt3(unpack_unreachable || (pack_everything & LOOSEN_UNREACHABLE), "-A",
diff --git a/t/t7706-repack-drop-filtered.sh b/t/t7706-repack-drop-filtered.sh
index cb36115834..a1c475e4ec 100755
--- a/t/t7706-repack-drop-filtered.sh
+++ b/t/t7706-repack-drop-filtered.sh
@@ -131,14 +131,33 @@ test_expect_success '--dry-run does not remove the filtered objects' '
 	git -C repo -c repack.writeBitmaps=false \
 		repack --drop-filtered --filter=blob:limit=1k --dry-run -a >out &&
 
 	# Candidate blob must still be present after a dry run.
 	git -C repo cat-file -e "$BIG"
 '
 
+test_expect_success '--dry-run leaves the pack directory untouched' '
+	BIG=$(cat big_oid) &&
+	packdir=repo/.git/objects/pack &&
+
+	for opt in "" -d
+	do
+		ls $packdir >before &&
+
+		git -C repo -c repack.writeBitmaps=false \
+			repack --drop-filtered --filter=blob:limit=1k \
+			--dry-run -a $opt >out &&
+
+		ls $packdir >after &&
+		test_cmp before after &&
+		test_grep "$BIG" out &&
+		git -C repo cat-file -e "$BIG" || return 1
+	done
+'
+
 test_expect_success '--drop-filtered removes the promisor blob locally' '
 	BIG=$(cat big_oid) &&
 	SMALL=$(cat small_oid) &&
 
 	git -C repo -c repack.writeBitmaps=false \
 		repack --drop-filtered --filter=blob:limit=1k -a &&
 
-- 
2.56.0