[PATCH 1/2] repack: add --filter-to-loose to write filtered-out objects as loose objects

["Adithya Chakilam via GitGitGadget" <gitgitgadget@gmail.com>]

From: Adithya Chakilam <achakilam@nvidia.com>

Some external object stores (like key-value) manage and retrieve git
objects individually. Although repack's --filter-to option can separate
objects from the main object store, it writes them into packs. Using
those objects individually then requires an additional unpacking step.

Add --filter-to-loose=<dir> to write filtered-out objects directly as
loose objects into <dir>, treating it as an object directory. Stream
the filtered pack into unpack-objects instead of writing an intermediate
pack file to disk.

The filtered pack is built with --stdin-packs, so this initially handles
only objects already stored in packs. Loose objects in the source
repository are left in place.

Signed-off-by: Adithya Chakilam <achakilam@nvidia.com>
---
 Documentation/git-repack.adoc |  10 ++-
 builtin/repack.c              |  21 ++++--
 repack-filtered.c             |  44 +++++++++++-
 repack.h                      |   1 +
 t/t7700-repack.sh             | 130 ++++++++++++++++++++++++++++++++++
 5 files changed, 199 insertions(+), 7 deletions(-)

diff --git a/Documentation/git-repack.adoc b/Documentation/git-repack.adoc
index a1f9e64f66..df1390154f 100644
--- a/Documentation/git-repack.adoc
+++ b/Documentation/git-repack.adoc
@@ -12,7 +12,7 @@ SYNOPSIS
 'git repack' [-a] [-A] [-d] [-f] [-F] [-l] [-n] [-q] [-b] [-m]
 	[--window=<n>] [--depth=<n>] [--threads=<n>] [--keep-pack=<pack-name>]
 	[--write-midx[=<mode>]] [--name-hash-version=<n>] [--path-walk]
-	[--filter=<filter-spec>] [--drop-filtered [--dry-run]]
+	[--filter=<filter-spec> [--filter-to-loose=<dir>]] [--drop-filtered [--dry-run]]
 
 DESCRIPTION
 -----------
@@ -183,6 +183,14 @@ depth is 4095.
 	`objects` and `objects/info/alternates` sections of
 	linkgit:gitrepository-layout[5].
 
+--filter-to-loose=<dir>::
+	Like `--filter-to`, but treat `<dir>` as an object directory
+	and write the filtered out objects there as loose objects
+	instead of as a pack. Cannot be combined with `--filter-to`
+	or `--max-pack-size`. Objects already present in `<dir>` are
+	not rewritten. All objects are written before any existing
+	pack is removed.
+
 --drop-filtered::
 	Delete the local objects that match the `--filter` specification
 	instead of keeping them in a separate packfile, reclaiming the
diff --git a/builtin/repack.c b/builtin/repack.c
index 21835453c6..140a992f1b 100644
--- a/builtin/repack.c
+++ b/builtin/repack.c
@@ -41,7 +41,7 @@ static const char *const git_repack_usage[] = {
 	N_("git repack [-a] [-A] [-d] [-f] [-F] [-l] [-n] [-q] [-b] [-m]\n"
 	   "[--window=<n>] [--depth=<n>] [--threads=<n>] [--keep-pack=<pack-name>]\n"
 	   "[--write-midx[=<mode>]] [--name-hash-version=<n>] [--path-walk]\n"
-	   "[--filter=<filter-spec>] [--drop-filtered [--dry-run]]"),
+	   "[--filter=<filter-spec> [--filter-to-loose=<dir>]] [--drop-filtered [--dry-run]]"),
 	NULL
 };
 
@@ -182,6 +182,7 @@ int cmd_repack(int argc,
 	const char *cruft_expiration = NULL;
 	const char *expire_to = NULL;
 	const char *filter_to = NULL;
+	const char *filter_to_loose = NULL;
 	const char *opt_window = NULL;
 	const char *opt_window_memory = NULL;
 	const char *opt_depth = NULL;
@@ -257,6 +258,8 @@ int cmd_repack(int argc,
 			   N_("pack prefix to store a pack containing pruned objects")),
 		OPT_STRING(0, "filter-to", &filter_to, N_("dir"),
 			   N_("pack prefix to store a pack containing filtered out objects")),
+		OPT_STRING(0, "filter-to-loose", &filter_to_loose, N_("dir"),
+			   N_("object directory to write filtered out objects into as loose objects")),
 		OPT_BOOL(0, "drop-filtered", &drop_filtered,
 				N_("delete filtered out objects (requires --filter)")),
 		OPT_BOOL(0, "dry-run", &dry_run,
@@ -282,8 +285,16 @@ int cmd_repack(int argc,
 	po_args.depth = xstrdup_or_null(opt_depth);
 	po_args.threads = xstrdup_or_null(opt_threads);
 
-	die_for_incompatible_opt2(drop_filtered, "--drop-filtered",
-		!!filter_to, "--filter-to");
+	die_for_incompatible_opt3(drop_filtered, "--drop-filtered",
+			!!filter_to, "--filter-to",
+			!!filter_to_loose, "--filter-to-loose");
+	die_for_incompatible_opt2(!!filter_to, "--filter-to",
+			!!filter_to_loose, "--filter-to-loose");
+	die_for_incompatible_opt2(!!filter_to_loose, "--filter-to-loose",
+			!!po_args.max_pack_size, "--max-pack-size");
+
+	if (filter_to_loose)
+		filter_to = filter_to_loose;
 
 	if (dry_run && !drop_filtered)
 		die(_("--dry-run only takes effect with --drop-filtered"));
@@ -553,7 +564,8 @@ int cmd_repack(int argc,
 		strvec_pushf(&cmd.args, "--filter=%s",
 			     expand_list_objects_filter_spec(&po_args.filter_options));
 	else if (filter_to)
-		die(_("option '%s' can only be used along with '%s'"), "--filter-to", "--filter");
+		die(_("option '%s' can only be used along with '%s'"),
+		    filter_to_loose ? "--filter-to-loose" : "--filter-to", "--filter");
 
 	if (geometry.split_factor)
 		cmd.in = -1;
@@ -698,6 +710,7 @@ int cmd_repack(int argc,
 		struct write_pack_opts opts = {
 			.po_args = &po_args,
 			.destination = filter_to,
+			.loose = !!filter_to_loose,
 			.packdir = packdir,
 			.packtmp = packtmp,
 		};
diff --git a/repack-filtered.c b/repack-filtered.c
index 869b9fc6e3..51c9dfeb13 100644
--- a/repack-filtered.c
+++ b/repack-filtered.c
@@ -9,19 +9,40 @@
 #include "list-objects-filter.h"
 #include "odb.h"
 #include "promisor-remote.h"
+#include "sigchain.h"
+
+static void start_unpack_objects(struct child_process *unpack,
+				 const char *objdir)
+{
+	strvec_push(&unpack->args, "unpack-objects");
+	strvec_push(&unpack->args, "-q");
+	strvec_pushf(&unpack->env, "GIT_OBJECT_DIRECTORY=%s", objdir);
+	strvec_push(&unpack->env, "GIT_ALTERNATE_OBJECT_DIRECTORIES");
+	unpack->git_cmd = 1;
+	unpack->in = -1;
+	if (start_command(unpack))
+		die(_("could not start unpack-objects for '%s'"), objdir);
+}
 
 int write_filtered_pack(const struct write_pack_opts *opts,
 			struct existing_packs *existing,
 			struct string_list *names)
 {
 	struct child_process cmd = CHILD_PROCESS_INIT;
+	struct child_process unpack = CHILD_PROCESS_INIT;
 	struct string_list_item *item;
 	FILE *in;
 	int ret;
 	const char *caret;
 	const char *pack_prefix = write_pack_opts_pack_prefix(opts);
 
-	prepare_pack_objects(&cmd, opts->po_args, opts->destination);
+	if (opts->loose) {
+		sigchain_push(SIGPIPE, SIG_IGN);
+		start_unpack_objects(&unpack, opts->destination);
+		prepare_pack_objects(&cmd, opts->po_args, "--stdout");
+	} else {
+		prepare_pack_objects(&cmd, opts->po_args, opts->destination);
+	}
 
 	strvec_push(&cmd.args, "--stdin-packs");
 
@@ -29,10 +50,18 @@ int write_filtered_pack(const struct write_pack_opts *opts,
 		strvec_pushf(&cmd.args, "--keep-pack=%s", item->string);
 
 	cmd.in = -1;
+	if (opts->loose)
+		cmd.out = unpack.in;
 
 	ret = start_command(&cmd);
-	if (ret)
+	if (ret) {
+		if (opts->loose) {
+			close(unpack.in);
+			finish_command(&unpack);
+			sigchain_pop(SIGPIPE);
+		}
 		return ret;
+	}
 
 	/*
 	 * Here 'names' contains only the pack(s) that were just
@@ -52,6 +81,17 @@ int write_filtered_pack(const struct write_pack_opts *opts,
 		fprintf(in, "%s%s.pack\n", caret, item->string);
 	fclose(in);
 
+	if (opts->loose) {
+		ret = finish_command(&cmd);
+		if (finish_command(&unpack))
+			ret = -1;
+		sigchain_pop(SIGPIPE);
+		if (ret)
+			ret = error(_("failed to write filtered out objects to '%s'"),
+				    opts->destination);
+		return ret;
+	}
+
 	return finish_pack_objects_cmd(existing->repo->hash_algo, opts, &cmd,
 				       names);
 }
diff --git a/repack.h b/repack.h
index 61e554e4ed..d5528745ec 100644
--- a/repack.h
+++ b/repack.h
@@ -41,6 +41,7 @@ void repack_remove_redundant_pack(struct repository *repo, const char *dir_name,
 struct write_pack_opts {
 	struct pack_objects_args *po_args;
 	const char *destination;
+	bool loose;
 	const char *packdir;
 	const char *packtmp;
 };
diff --git a/t/t7700-repack.sh b/t/t7700-repack.sh
index f0a390e3c6..5aa4b698f1 100755
--- a/t/t7700-repack.sh
+++ b/t/t7700-repack.sh
@@ -489,6 +489,136 @@ test_expect_success '--filter-to stores filtered out objects' '
 	test "$blob_content" = "content1"
 '
 
+test_expect_success '--filter-to-loose writes filtered out objects loose' '
+	rm -rf loose-work loose-src.git loose-dst.git &&
+	git init --bare loose-dst.git &&
+	git init loose-work &&
+	test_commit -C loose-work base file1 content1 &&
+	git clone --bare loose-work loose-src.git &&
+	git -C loose-src.git repack -a -d &&
+
+	blob=$(git -C loose-src.git rev-parse HEAD:file1) &&
+	git -C loose-src.git -c repack.writebitmaps=false repack -a -d \
+		--filter=blob:none --filter-to-loose=../loose-dst.git/objects &&
+
+	test_path_is_file loose-dst.git/objects/$(test_oid_to_path $blob) &&
+	find loose-dst.git/objects/pack -name "*.pack" >actual-packs &&
+	test_must_be_empty actual-packs &&
+	test_must_fail git -C loose-src.git cat-file -e $blob &&
+
+	echo $(pwd)/loose-dst.git/objects >loose-src.git/objects/info/alternates &&
+	echo content1 >expect &&
+	git -C loose-src.git cat-file blob $blob >actual &&
+	test_cmp expect actual
+'
+
+test_expect_success '--filter-to-loose writes objects that also exist in the source' '
+	rm -rf loose-work loose-src.git loose-dst.git &&
+	git init --bare loose-dst.git &&
+	git init loose-work &&
+	test_commit -C loose-work base file1 content1 &&
+	git clone --bare loose-work loose-src.git &&
+	git -C loose-src.git repack -a -d &&
+
+	blob=$(git -C loose-src.git rev-parse HEAD:file1) &&
+	git -C loose-src.git -c repack.writebitmaps=false repack -a -d \
+		--filter=blob:none --filter-to-loose=../loose-dst.git/objects &&
+
+	test_path_is_file loose-dst.git/objects/$(test_oid_to_path $blob)
+'
+
+# Create loose-work/loose-src.git/loose-dst.git; $big is a 2k blob on
+# refs/heads/big in loose-src.git, packed unless $1 is "loose".
+setup_loose_src () {
+	rm -rf loose-work loose-src.git loose-dst.git &&
+	git init --bare loose-dst.git &&
+	git init loose-work &&
+	test_commit -C loose-work base small small &&
+	git clone --bare loose-work loose-src.git &&
+
+	big=$(test-tool genrandom big 2k | git -C loose-src.git hash-object -w --stdin) &&
+	tree=$(printf "100644 blob %s\tbig\n" $big | git -C loose-src.git mktree) &&
+	commit=$(git -C loose-src.git commit-tree -m big $tree) &&
+	git -C loose-src.git update-ref refs/heads/big $commit &&
+
+	if test "$1" != loose
+	then
+		git -C loose-src.git repack -a -d
+	fi
+}
+
+test_expect_success '--filter-to-loose failure to write leaves the source intact' '
+	setup_loose_src &&
+	>not-a-dir &&
+
+	ls loose-src.git/objects/pack/*.pack >packs-before &&
+	test_must_fail git -C loose-src.git -c repack.writebitmaps=false \
+		repack -a -d --filter=blob:limit=1k \
+		--filter-to-loose=../not-a-dir &&
+
+	ls loose-src.git/objects/pack/*.pack >packs-after &&
+	test_cmp packs-before packs-after &&
+
+	find loose-src.git/objects/pack -name ".tmp-*" >tmp-leftovers &&
+	test_must_be_empty tmp-leftovers &&
+	git -C loose-src.git cat-file -e $big
+'
+
+test_expect_success '--filter-to-loose does not move objects from kept packs' '
+	setup_loose_src &&
+
+	for p in loose-src.git/objects/pack/pack-*.pack
+	do
+		>${p%.pack}.keep || return 1
+	done &&
+
+	git -C loose-src.git -c repack.writebitmaps=false repack -a -d \
+		--filter=blob:limit=1k --filter-to-loose=../loose-dst.git/objects &&
+
+	test_path_is_missing loose-dst.git/objects/$(test_oid_to_path $big) &&
+	git -C loose-src.git cat-file -e $big
+'
+
+test_expect_success '--filter-to-loose leaves unreachable objects in cruft packs alone' '
+	setup_loose_src loose &&
+	git -C loose-src.git update-ref -d refs/heads/big &&
+	git -C loose-src.git repack --cruft -d &&
+
+	git -C loose-src.git -c repack.writebitmaps=false repack -a -d --cruft \
+		--filter=blob:limit=1k --filter-to-loose=../loose-dst.git/objects &&
+
+	test_path_is_missing loose-dst.git/objects/$(test_oid_to_path $big) &&
+	git -C loose-src.git cat-file -e $big
+'
+
+test_expect_success '--filter-to-loose output survives gc and prune of the source' '
+	setup_loose_src &&
+
+	git -C loose-src.git -c repack.writebitmaps=false repack -a -d \
+		--filter=blob:limit=1k --filter-to-loose=../loose-dst.git/objects &&
+
+	echo $(pwd)/loose-dst.git/objects >loose-src.git/objects/info/alternates &&
+	git -C loose-src.git gc --prune=now &&
+	git -C loose-src.git prune --expire=now &&
+
+	git -C loose-src.git fsck &&
+	git -C loose-src.git cat-file -e $big &&
+	test_path_is_file loose-dst.git/objects/$(test_oid_to_path $big)
+'
+
+test_expect_success '--filter-to-loose requires --filter, rejects --filter-to and --max-pack-size' '
+	test_must_fail git -C bare.git repack -a --filter-to-loose=../x 2>err &&
+	test_grep "can only be used along with" err &&
+
+	test_must_fail git -C bare.git repack -a --filter=blob:none \
+		--filter-to=../y --filter-to-loose=../x 2>err &&
+	test_grep "cannot be used together" err &&
+
+	test_must_fail git -C bare.git repack -a --filter=blob:none \
+		--filter-to-loose=../x --max-pack-size=1M 2>err &&
+	test_grep "cannot be used together" err
+'
+
 test_expect_success '--filter works with --max-pack-size' '
 	rm -rf filtered.git &&
 	git init --bare filtered.git &&
-- 
gitgitgadget