[PATCH v4 2/3] pack-objects: add --ignore-pack-keep-from

["Qin ShiCheng via GitGitGadget" <gitgitgadget@gmail.com>]

From: Qin ShiCheng <qeesung@live.com>

repack decides which packs to delete before it runs pack-objects, but
pack-objects looks for ".keep" files on its own when it opens the
packs. A ".keep" that appears in between, for example from a push or
fetch whose index-pack wrote a pack identical to an existing one, makes
pack-objects treat as kept a pack that repack is about to delete. Its
objects are then left out of the new pack, or not loosened, or not
kept as unreachable, and they are gone once repack deletes the pack.

Not all of that goes through "--honor-pack-keep": loosening objects
for "--unpack-unreachable", and collecting them for
"--keep-unreachable" and "--cruft", skip packs with a ".keep" file
regardless of it.

Let the caller name such packs. "--ignore-pack-keep-from=<file>" reads
names of local packs, one per line, given and matched as for
"--keep-pack", and treats them as if they had no ".keep" file. All of
the above go by p->pack_keep, which is read from the disk once, when a
pack is first seen, so clearing it for the named packs before anything
looks at it covers every one of them. Other packs, including any that
show up later, keep whatever their ".keep" files say.

The names come from a file because there can be more of them than fit
on a command line (on Windows, the whole command line is limited to
32767 characters), and the standard input is already taken in every
mode repack uses.

Signed-off-by: Qin ShiCheng <qeesung@live.com>
---
 Documentation/git-pack-objects.adoc | 12 ++++++++++
 builtin/pack-objects.c              | 37 +++++++++++++++++++++++++++++
 t/t7700-repack.sh                   | 37 +++++++++++++++++++++++++++++
 3 files changed, 86 insertions(+)

diff --git a/Documentation/git-pack-objects.adoc b/Documentation/git-pack-objects.adoc
index ccad938c5b..43fb206ef0 100644
--- a/Documentation/git-pack-objects.adoc
+++ b/Documentation/git-pack-objects.adoc
@@ -13,6 +13,7 @@ SYNOPSIS
 		   [--no-reuse-delta] [--delta-base-offset] [--non-empty]
 		   [--local] [--incremental] [--window=<n>] [--depth=<n>]
 		   [--revs [--unpacked | --all]] [--keep-pack=<pack-name>]
+		   [--ignore-pack-keep-from=<file>]
 		   [--cruft] [--cruft-expiration=<time>]
 		   [--stdout [--filter=<filter-spec>] | <base-name>]
 		   [--shallow] [--keep-true-parents] [--[no-]sparse]
@@ -193,6 +194,17 @@ depth is 4095.
 	leading directory (e.g. `pack-123.pack`). The option could be
 	specified multiple times to keep multiple packs.
 
+--ignore-pack-keep-from=<file>::
+	Read names of local packs from `<file>`, one per line in the
+	same form as for `--keep-pack`, and treat them as if they had
+	no .keep file, both for `--honor-pack-keep` and when deciding
+	which objects to loosen or keep with `--unpack-unreachable`,
+	`--keep-unreachable` and `--cruft`.
+	This is for callers like linkgit:git-repack[1] that are about
+	to delete those packs: a .keep file that appears on one of them
+	after the caller decided to delete it must not cause its objects
+	to be left out.
+
 --incremental::
 	This flag causes an object already in a pack to be ignored
 	even if it would have otherwise been packed.
diff --git a/builtin/pack-objects.c b/builtin/pack-objects.c
index 5160e76842..f63baa7273 100644
--- a/builtin/pack-objects.c
+++ b/builtin/pack-objects.c
@@ -194,6 +194,7 @@ static const char *const pack_usage[] = {
 	   "                 [--no-reuse-delta] [--delta-base-offset] [--non-empty]\n"
 	   "                 [--local] [--incremental] [--window=<n>] [--depth=<n>]\n"
 	   "                 [--revs [--unpacked | --all]] [--keep-pack=<pack-name>]\n"
+	   "                 [--ignore-pack-keep-from=<file>]\n"
 	   "                 [--cruft] [--cruft-expiration=<time>]\n"
 	   "                 [--stdout [--filter=<filter-spec>] | <base-name>]\n"
 	   "                 [--shallow] [--keep-true-parents] [--[no-]sparse]\n"
@@ -5005,6 +5006,36 @@ static void get_object_list(struct rev_info *revs, struct strvec *argv)
 	oid_array_clear(&recent_objects);
 }
 
+/*
+ * Treat the local packs named in "path" as if they had no ".keep" file.
+ * The names are matched like those given with --keep-pack. This has to
+ * happen before anything looks at p->pack_keep, which is read from the
+ * disk only once, when the pack is first seen.
+ */
+static void ignore_pack_keep_from_file(const char *path)
+{
+	struct string_list names = STRING_LIST_INIT_DUP;
+	struct strbuf buf = STRBUF_INIT;
+	struct packed_git *p;
+	FILE *fp = xfopen(path, "r");
+
+	names.cmp = fspathcmp;
+	while (strbuf_getline(&buf, fp) != EOF)
+		string_list_append(&names, buf.buf);
+	if (ferror(fp))
+		die_errno(_("could not read '%s'"), path);
+	fclose(fp);
+	string_list_sort(&names);
+
+	repo_for_each_pack(the_repository, p)
+		if (p->pack_local &&
+		    string_list_has_string(&names, pack_basename(p)))
+			p->pack_keep = 0;
+
+	string_list_clear(&names, 0);
+	strbuf_release(&buf);
+}
+
 static void add_extra_kept_packs(const struct string_list *names)
 {
 	struct packed_git *p;
@@ -5138,6 +5169,7 @@ int cmd_pack_objects(int argc,
 	int rev_list_index = 0;
 	enum stdin_packs_mode stdin_packs = STDIN_PACKS_MODE_NONE;
 	struct string_list keep_pack_list = STRING_LIST_INIT_NODUP;
+	char *ignore_pack_keep_from = NULL;
 	struct list_objects_filter_options filter_options =
 		LIST_OBJECTS_FILTER_INIT;
 	struct repo_config_values *cfg = repo_config_values(the_repository);
@@ -5222,6 +5254,8 @@ int cmd_pack_objects(int argc,
 			 N_("ignore packs that have companion .keep file")),
 		OPT_STRING_LIST(0, "keep-pack", &keep_pack_list, N_("name"),
 				N_("ignore this pack")),
+		OPT_FILENAME(0, "ignore-pack-keep-from", &ignore_pack_keep_from,
+			     N_("ignore .keep files of the packs named in <file>")),
 		OPT_INTEGER(0, "compression", &cfg->pack_compression_level,
 			    N_("pack compression level")),
 		OPT_BOOL(0, "keep-true-parents", &grafts_keep_true_parents,
@@ -5449,6 +5483,8 @@ int cmd_pack_objects(int argc,
 	if (progress && all_progress_implied)
 		progress = 2;
 
+	if (ignore_pack_keep_from)
+		ignore_pack_keep_from_file(ignore_pack_keep_from);
 	add_extra_kept_packs(&keep_pack_list);
 	if (ignore_packed_keep_on_disk) {
 		struct packed_git *p;
@@ -5543,6 +5579,7 @@ cleanup:
 	clear_packing_data(&to_pack);
 	list_objects_filter_release(&filter_options);
 	string_list_clear(&keep_pack_list, 0);
+	free(ignore_pack_keep_from);
 	strvec_clear(&rp);
 
 	return 0;
diff --git a/t/t7700-repack.sh b/t/t7700-repack.sh
index f0a390e3c6..00fe259767 100755
--- a/t/t7700-repack.sh
+++ b/t/t7700-repack.sh
@@ -211,6 +211,43 @@ test_expect_success 'objects made unreachable by grafts only are kept' '
 	git cat-file -t $H1
 '
 
+test_expect_success 'pack-objects --ignore-pack-keep-from' '
+	test_when_finished "rm -fr ignore-keep" &&
+	git init ignore-keep &&
+	(
+		cd ignore-keep &&
+		test_commit base &&
+		unreachable=$(git commit-tree -m unreachable HEAD^{tree}) &&
+
+		git rev-list --objects --all $unreachable >objects &&
+		P=$(git pack-objects .git/objects/pack/pack <objects) &&
+		test_commit kept &&
+		git rev-list --objects base..kept >objects &&
+		K=$(git pack-objects .git/objects/pack/pack <objects) &&
+		git prune-packed &&
+		>.git/objects/pack/pack-$P.keep &&
+		>.git/objects/pack/pack-$K.keep &&
+		echo pack-$P.pack >ignore &&
+
+		# Both packs have a .keep file, but only P is named in
+		# "ignore": its reachable objects are packed and the others
+		# are loosened, while K is left alone.
+		git pack-objects --honor-pack-keep --all --reflog \
+			--indexed-objects --unpack-unreachable \
+			--ignore-pack-keep-from=ignore \
+			.git/objects/pack/pack </dev/null &&
+		test-tool find-pack --check-count 2 base &&
+		test-tool find-pack --check-count 1 kept &&
+		test_path_is_file .git/objects/$(test_oid_to_path $unreachable)
+	)
+'
+
+test_expect_success 'pack-objects --ignore-pack-keep-from with a missing file' '
+	test_must_fail git pack-objects --stdout \
+		--ignore-pack-keep-from=does-not-exist </dev/null 2>err &&
+	test_grep "could not open .does-not-exist. for reading" err
+'
+
 test_expect_success 'repack --keep-pack' '
 	test_create_repo keep-pack &&
 	(
-- 
gitgitgadget