[RFC PATCH 0/7] repack: add --drop-filtered to reclaim space in partial clones

[RFC PATCH 0/7] repack: add --drop-filtered to reclaim space in partial clones

[Siddharth Shrimali]

This is an RFC series seeking feedback on the design and approach.
Several pieces are still missing (noted below) and the commit
organization needs cleanup.

Partial clones let you work with large repositories without downloading
every blob up front and the missing blobs are lazily fetched from the promisor
remote on demand. Over time, though, these lazily-fetched blobs
accumulate locally and there is currently no safe, built-in way to
reclaim that disk space instead of re-cloning.

This series adds a "git repack --drop-filtered --filter=<spec>" command
that removes large, locally-held promisor blobs that are recoverable
from the promisor remote. The dropped blobs become absent locally but
remain lazily re-fetchable, making the partial-clone still reversible.

How it works:
  * Enumerate promisor objects directly (ODB_FOR_EACH_OBJECT_PROMISOR_ONLY)
    and select the blobs exceeding the filter threshold. Because every
    enumerated object is a promisor object, it is guaranteed recoverable and
    locally-created objects are never candidates.

  * Rebuild the promisor pack without the selected blobs, reusing the
    existing repack machinery, so the drop is crash-safe.

  * Record each dropped object in a drop log
    ($GIT_DIR/objects/info/promisor-dropped) so a later change can
    explain a failed lazy fetch (when it was dropped, which filter
    matched, which remotes) instead of a bare "could not fetch" error.

  * --dry-run lists the candidates and changes nothing.

Planned follow-ups:
  * Safety guards: refuse to run while a merge/rebase/cherry-pick is in
    progress, and refuse to drop blobs referenced by the current index.

  * Authoritative remote verification: the drop log currently lists all
    configured promisor remotes rather than the exact remote each object
    is recoverable from, because there is no client-side way to query a
    remote for object availability yet. A "remote-object-info" command
    is being added to the "git cat-file --batch" protocol for this. Once
    available, the exact remote can be recorded.

Known issues to address in v2:
  * There is churn between "enumerate promisor blobs" and "actually drop
    filtered promisor blobs". The former introduces
    enumerate_promisor_blobs() with an interim signature that the latter
    rewrites. These will be reorganized so the function is introduced
    in its final form.

  * The tests are in a standalone commit. They will instead be
    distributed into the commits that introduce the behavior they test.

Siddharth Shrimali (7):
  builtin/repack.c: add --drop-filtered and --dry-run options
  list-objects-filter: add list_objects_filter__filter_oidset()
  repack-promisor: allow excluding objects from the rebuilt promisor
    pack
  builtin/repack: enumerate promisor blobs for --drop-filtered
  t7706: test --drop-filtered enumeration and validation
  builtin/repack: actually drop filtered promisor blobs
  repack-promisor: record dropped objects in a drop log

 builtin/repack.c                |  76 ++++++++++++++++-
 list-objects-filter.c           |  45 ++++++++++
 list-objects-filter.h           |  16 ++++
 repack-filtered.c               |  81 ++++++++++++++++++
 repack-promisor.c               | 106 ++++++++++++++++++++++-
 repack.h                        |  12 ++-
 t/meson.build                   |   1 +
 t/t7706-repack-drop-filtered.sh | 145 ++++++++++++++++++++++++++++++++
 8 files changed, 478 insertions(+), 4 deletions(-)
 create mode 100755 t/t7706-repack-drop-filtered.sh

-- 
2.54.0

[RFC PATCH 1/7] builtin/repack.c: add --drop-filtered and --dry-run options

[Siddharth Shrimali]

Add two new command-line options to 'git-repack':

  --drop-filtered: intended to eventually delete objects that match
                   the filter specification. Requires --filter and -a,
                   and is incompatible with --filter-to.
  --dry-run: show which objects would be dropped without making any
             changes. Only meaningful with --drop-filtered.

--drop-filtered also requires a promisor remote to be configured,
since dropping objects without a remote to fetch them back from would
be permanent data loss.

--drop-filtered is incompatible with bitmap writing: filtering breaks
the "all objects in one pack" closure that bitmaps require. An explicit
-b is rejected with a clear error and a default-on bitmap configuration is
silently disabled for the duration of the command.

These options currently only perform validation. The actual enumeration
and deletion will be added in follow-up commits.

Mentored-by: Christian Couder <christian.couder@gmail.com>
Mentored-by: Siddharth Asthana <siddharthasthana31@gmail.com>
Signed-off-by: Siddharth Shrimali <r.siddharth.shrimali@gmail.com>
---
 builtin/repack.c | 44 ++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 44 insertions(+)

diff --git a/builtin/repack.c b/builtin/repack.c
index db504d673f..f4db0fc535 100644
--- a/builtin/repack.c
+++ b/builtin/repack.c
@@ -14,6 +14,7 @@
 #include "promisor-remote.h"
 #include "repack.h"
 #include "shallow.h"
+#include "list-objects-filter-options.h"
 
 #define ALL_INTO_ONE 1
 #define LOOSEN_UNREACHABLE 2
@@ -28,6 +29,8 @@ static int use_delta_islands;
 static int run_update_server_info = 1;
 static char *packdir, *packtmp_name, *packtmp;
 static int midx_must_contain_cruft = 1;
+static int drop_filtered;
+static int dry_run;
 
 static const char *const git_repack_usage[] = {
 	N_("git repack [-a] [-A] [-d] [-f] [-F] [-l] [-n] [-q] [-b] [-m]\n"
@@ -231,6 +234,10 @@ 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_BOOL(0, "drop-filtered", &drop_filtered,
+				N_("delete filtered out objects (requires --filter)")),
+		OPT_BOOL(0, "dry-run", &dry_run,
+				N_("only show which objects would be dropped")),
 		OPT_END()
 	};
 
@@ -252,6 +259,43 @@ 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_opt2(drop_filtered, "--drop-filtered",
+		write_bitmaps > 0, "--write-bitmap-index");
+
+	if (dry_run && !drop_filtered)
+		die(_("--dry-run only takes effect with --drop-filtered"));
+
+	if (drop_filtered) {
+		if (!dry_run)
+			die(_("--drop-filtered doesn't work without --dry-run yet"));
+
+		if (!po_args.filter_options.choice)
+			die(_("--drop-filtered requires --filter"));
+
+		if (!(pack_everything & ALL_INTO_ONE))
+			die(_("--drop-filtered requires -a"));
+
+		/*
+		 * Only blob:limit=<n> is supported for now. Reject other
+		 * filter choices early, before walking the object database.
+		 */
+		if (po_args.filter_options.choice != LOFC_BLOB_LIMIT)
+			die(_("--drop-filtered only supports --filter=blob:limit=<n> for now"));
+
+		/*
+		 * Without a promisor remote there is nowhere to re-fetch the
+		 * dropped objects from, so dropping them would be permanent
+		 * data loss.
+		 */
+		if (!repo_has_promisor_remote(repo))
+			die(_("--drop-filtered requires a promisor remote"));
+
+		write_bitmaps = 0;
+	}
+
 	if (delete_redundant && repo->repository_format_precious_objects)
 		die(_("cannot delete packs in a precious-objects repo"));
 
-- 
2.54.0

[RFC PATCH 2/7] list-objects-filter: add list_objects_filter__filter_oidset()

[Siddharth Shrimali]

The existing filter entry point, list_objects_filter__filter_object(),
is built around the object-walk path: it expects traversal context and
provisional omit sets, and is meant to be called as objects are
visited during a walk. A caller that already has a set of OIDs in hand
and only wants to know which ones a filter would select has no usable
entry point into the filter API.

--drop-filtered is exactly such a caller: it collects promisor blobs
into an oidset and needs to know which of them exceed the filter
threshold, without performing an object walk.

Add a helper, list_objects_filter__filter_oidset(), that takes a set
of OIDs and populates an "omitted" set with those that would be
filtered out by the given filter options. Only blob:limit=N filters
are supported for now.

This helper does not actually reuse the existing filter machinery.
It reimplements the blob:limit size check directly. That machinery
is tied to the object-walk path and cannot easily be driven
from a plain oidset. A NEEDSWORK comment marks this so the helper can
later be refactored to reuse the real filter logic instead of
duplicating it.

OBJECT_INFO_SKIP_FETCH_OBJECT is passed when reading object info so
the helper never triggers a lazy fetch.

Mentored-by: Christian Couder <christian.couder@gmail.com>
Mentored-by: Siddharth Asthana <siddharthasthana31@gmail.com>
Signed-off-by: Siddharth Shrimali <r.siddharth.shrimali@gmail.com>
---
 list-objects-filter.c | 45 +++++++++++++++++++++++++++++++++++++++++++
 list-objects-filter.h | 16 +++++++++++++++
 2 files changed, 61 insertions(+)

diff --git a/list-objects-filter.c b/list-objects-filter.c
index c912ff3079..6a2e9d5b24 100644
--- a/list-objects-filter.c
+++ b/list-objects-filter.c
@@ -828,3 +828,48 @@ void list_objects_filter__free(struct filter *filter)
 	filter->free_fn(filter->filter_data);
 	free(filter);
 }
+
+/*
+ * NEEDSWORK: this reimplements the blob:limit size check rather than
+ * reusing the existing filter machinery in
+ * list_objects_filter__filter_object(). That machinery is currently
+ * tied to the object-walk path and cannot easily be driven from a
+ * plain oidset. It would be nice to refactor the filter code so this
+ * helper can reuse it instead of duplicating the size check.
+ */
+int list_objects_filter__filter_oidset(struct repository *r,
+	struct list_objects_filter_options *opts,
+	const struct oidset *in,
+	struct oidset *omitted)
+{
+	struct oidset_iter iter;
+	const struct object_id *oid;
+
+	if (opts->choice != LOFC_BLOB_LIMIT)
+		return error(_("filter_oidset: only blob:limit filters are supported"));
+
+	oidset_iter_init(in, &iter);
+	while ((oid = oidset_iter_next(&iter))) {
+		struct object_info info = OBJECT_INFO_INIT;
+		enum object_type type;
+		unsigned long size;
+
+		info.typep = &type;
+		info.sizep = &size;
+
+		/*
+		 * Use OBJECT_INFO_SKIP_FETCH_OBJECT to avoid triggering
+		 * a lazy fetch while inspecting candidates for removal.
+		 */
+		if (odb_read_object_info_extended(r->objects, oid, &info,
+				OBJECT_INFO_SKIP_FETCH_OBJECT) < 0)
+			continue;
+
+		if (type != OBJ_BLOB)
+			continue;
+
+		if (size >= opts->blob_limit_value)
+			oidset_insert(omitted, oid);
+	}
+	return 0;
+}
diff --git a/list-objects-filter.h b/list-objects-filter.h
index 9e98814111..56a2d87aa0 100644
--- a/list-objects-filter.h
+++ b/list-objects-filter.h
@@ -94,4 +94,20 @@ enum list_objects_filter_result list_objects_filter__filter_object(
  */
 void list_objects_filter__free(struct filter *filter);
 
+/*
+ * Given a set of OIDs in 'in', populate 'omitted' with those that
+ * would be filtered by 'opts'. Currently only blob:limit=N is
+ * supported. Objects that cannot be read are silently skipped.
+ *
+ * NEEDSWORK: this reimplements the blob:limit size check rather than
+ * reusing the existing filter machinery. See the matching comment in
+ * list-objects-filter.c.
+ *
+ * Return 0 on success, -1 if the filter is not supported.
+ */
+int list_objects_filter__filter_oidset(struct repository *r,
+	struct list_objects_filter_options *opts,
+	const struct oidset *in,
+	struct oidset *omitted);
+
 #endif /* LIST_OBJECTS_FILTER_H */
-- 
2.54.0

[RFC PATCH 3/7] repack-promisor: allow excluding objects from the rebuilt promisor pack

[Siddharth Shrimali]

Add a to_drop oidset parameter to repack_promisor_objects(). When it is
non-NULL, write_oid() omits those objects from the rebuilt promisor
pack. This is the mechanism --drop-filtered will use to remove promisor
blobs, i.e. rebuild the promisor pack without them.

All existing callers pass NULL, so behavior is unchanged.

Mentored-by: Christian Couder <christian.couder@gmail.com>
Mentored-by: Siddharth Asthana <siddharthasthana31@gmail.com>
Signed-off-by: Siddharth Shrimali <r.siddharth.shrimali@gmail.com>
---
 builtin/repack.c  |  2 +-
 repack-promisor.c | 15 ++++++++++++++-
 repack.h          |  4 +++-
 3 files changed, 18 insertions(+), 3 deletions(-)

diff --git a/builtin/repack.c b/builtin/repack.c
index f4db0fc535..433b2c8205 100644
--- a/builtin/repack.c
+++ b/builtin/repack.c
@@ -406,7 +406,7 @@ int cmd_repack(int argc,
 		strvec_push(&cmd.args, "--delta-islands");
 
 	if (pack_everything & ALL_INTO_ONE) {
-		repack_promisor_objects(repo, &po_args, &names, packtmp);
+		repack_promisor_objects(repo, &po_args, &names, packtmp, NULL);
 
 		if (existing_packs_has_non_kept(&existing) &&
 		    delete_redundant &&
diff --git a/repack-promisor.c b/repack-promisor.c
index 90318ce150..fabfdc168a 100644
--- a/repack-promisor.c
+++ b/repack-promisor.c
@@ -6,10 +6,12 @@
 #include "path.h"
 #include "repository.h"
 #include "run-command.h"
+#include "oidset.h"
 
 struct write_oid_context {
 	struct child_process *cmd;
 	const struct git_hash_algo *algop;
+	const struct oidset *to_drop;
 };
 
 /*
@@ -23,6 +25,15 @@ static int write_oid(const struct object_id *oid,
 	struct write_oid_context *ctx = data;
 	struct child_process *cmd = ctx->cmd;
 
+	/*
+	 * Objects in to_drop are being removed from the repository, so
+	 * omit them from the rebuilt promisor pack. Each such object is a
+	 * promisor object and therefore remains recoverable from the
+	 * promisor remote.
+	 */
+	if (ctx->to_drop && oidset_contains(ctx->to_drop, oid))
+		return 0;
+
 	if (cmd->in == -1) {
 		if (start_command(cmd))
 			die(_("could not start pack-objects to repack promisor objects"));
@@ -81,7 +92,8 @@ static void finish_repacking_promisor_objects(struct repository *repo,
 
 void repack_promisor_objects(struct repository *repo,
 			     const struct pack_objects_args *args,
-			     struct string_list *names, const char *packtmp)
+			     struct string_list *names, const char *packtmp,
+			     const struct oidset *to_drop)
 {
 	struct write_oid_context ctx;
 	struct child_process cmd = CHILD_PROCESS_INIT;
@@ -98,6 +110,7 @@ void repack_promisor_objects(struct repository *repo,
 	 */
 	ctx.cmd = &cmd;
 	ctx.algop = repo->hash_algo;
+	ctx.to_drop = to_drop;
 	odb_for_each_object(repo->objects, NULL, write_oid, &ctx,
 			    ODB_FOR_EACH_OBJECT_PROMISOR_ONLY);
 
diff --git a/repack.h b/repack.h
index f9fbc895f0..a5a3f7c6ba 100644
--- a/repack.h
+++ b/repack.h
@@ -3,6 +3,7 @@
 
 #include "list-objects-filter-options.h"
 #include "string-list.h"
+#include "oidset.h"
 
 struct pack_objects_args {
 	char *window;
@@ -100,7 +101,8 @@ void generated_pack_install(struct generated_pack *pack, const char *name,
 
 void repack_promisor_objects(struct repository *repo,
 			     const struct pack_objects_args *args,
-			     struct string_list *names, const char *packtmp);
+			     struct string_list *names, const char *packtmp,
+			     const struct oidset *to_drop);
 
 struct pack_geometry {
 	struct packed_git **pack;
-- 
2.54.0

[RFC PATCH 4/7] builtin/repack: enumerate promisor blobs for --drop-filtered

[Siddharth Shrimali]

Add enumeration logic for --drop-filtered. In --dry-run mode, print
the OIDs of locally-held promisor blobs that exceed the filter
threshold, as candidates for removal.

Reading from write_filtered_pack() cannot work for partial clones.
git repack routes promisor objects through a separate path:
repack_promisor_objects() repacks them first, and the main
pack-objects run uses --exclude-promisor-objects. By the time
write_filtered_pack() runs, the promisor blobs are already consumed by
the main pack. The filtered pack is always empty on a partial clone.

Instead, walk promisor objects directly via odb_for_each_object() with
ODB_FOR_EACH_OBJECT_PROMISOR_ONLY, collecting all promisor blobs into
an oidset. The blobs exceeding the filter threshold are then selected
using list_objects_filter__filter_oidset().

Every object enumerated this way is a promisor object by construction,
so it is guaranteed to be recoverable from the promisor remote and is
safe to drop. No separate is_promisor_object() check is needed.

OBJECT_INFO_SKIP_FETCH_OBJECT is passed to every object info query so
enumeration never triggers a lazy fetch.

Deletion of the enumerated objects, together with the required
promisor-remote verification, will be added separately.

Mentored-by: Christian Couder <christian.couder@gmail.com>
Mentored-by: Siddharth Asthana <siddharthasthana31@gmail.com>
Signed-off-by: Siddharth Shrimali <r.siddharth.shrimali@gmail.com>
---
 builtin/repack.c  | 41 ++++++++++++++-------
 repack-filtered.c | 92 +++++++++++++++++++++++++++++++++++++++++++++++
 repack.h          |  4 +++
 3 files changed, 124 insertions(+), 13 deletions(-)

diff --git a/builtin/repack.c b/builtin/repack.c
index 433b2c8205..c2b07477d2 100644
--- a/builtin/repack.c
+++ b/builtin/repack.c
@@ -590,19 +590,34 @@ int cmd_repack(int argc,
 	}
 
 	if (po_args.filter_options.choice) {
-		struct write_pack_opts opts = {
-			.po_args = &po_args,
-			.destination = filter_to,
-			.packdir = packdir,
-			.packtmp = packtmp,
-		};
-
-		if (!opts.destination)
-			opts.destination = packtmp;
-
-		ret = write_filtered_pack(&opts, &existing, &names);
-		if (ret)
-			goto cleanup;
+		if (drop_filtered) {
+			/*
+			 * Enumerate promisor objects directly rather than
+			 * going through write_filtered_pack(). The filter
+			 * machinery cannot see promisor objects because
+			 * repack_promisor_objects() handles them separately
+			 * before the filter runs.
+			 */
+			ret = enumerate_promisor_blobs(repo,
+					&po_args.filter_options,
+					dry_run);
+			if (ret)
+				goto cleanup;
+		} else {
+			struct write_pack_opts opts = {
+				.po_args = &po_args,
+				.destination = filter_to,
+				.packdir = packdir,
+				.packtmp = packtmp,
+			};
+
+			if (!opts.destination)
+				opts.destination = packtmp;
+
+			ret = write_filtered_pack(&opts, &existing, &names);
+			if (ret)
+				goto cleanup;
+		}
 	}
 
 	string_list_sort(&names);
diff --git a/repack-filtered.c b/repack-filtered.c
index edcf7667c5..f5a1dae5b1 100644
--- a/repack-filtered.c
+++ b/repack-filtered.c
@@ -3,6 +3,12 @@
 #include "repository.h"
 #include "run-command.h"
 #include "string-list.h"
+#include "hex.h"
+#include "packfile.h"
+#include "list-objects-filter-options.h"
+#include "list-objects-filter.h"
+#include "odb.h"
+#include "promisor-remote.h"
 
 int write_filtered_pack(const struct write_pack_opts *opts,
 			struct existing_packs *existing,
@@ -49,3 +55,89 @@ int write_filtered_pack(const struct write_pack_opts *opts,
 	return finish_pack_objects_cmd(existing->repo->hash_algo, opts, &cmd,
 				       names);
 }
+
+struct collect_cb_data {
+	struct repository *repo;
+	struct oidset *set;
+};
+
+static int collect_promisor_blob(const struct object_id *oid,
+		struct object_info *oi UNUSED,
+		void *cb_data)
+{
+	struct collect_cb_data *data = cb_data;
+	struct object_info info = OBJECT_INFO_INIT;
+	enum object_type type;
+
+	info.typep = &type;
+
+	/*
+	 * Use OBJECT_INFO_SKIP_FETCH_OBJECT to avoid triggering a
+	 * lazy fetch while collecting promisor blobs.
+	 */
+	if (odb_read_object_info_extended(data->repo->objects, oid, &info,
+			OBJECT_INFO_SKIP_FETCH_OBJECT) < 0)
+		return 0;
+
+	if (type == OBJ_BLOB)
+		oidset_insert(data->set, oid);
+
+	return 0;
+}
+
+int enumerate_promisor_blobs(struct repository *repo,
+			const struct list_objects_filter_options *filter,
+			int dry_run)
+{
+	struct oidset all_promisor_blobs = OIDSET_INIT;
+	struct oidset to_drop = OIDSET_INIT;
+	struct collect_cb_data cb = {
+		.repo = repo,
+		.set = &all_promisor_blobs
+	};
+	struct oidset_iter iter;
+	const struct object_id *oid;
+	int ret = 0;
+
+	/*
+	 * The caller (cmd_repack) is responsible for validating that a
+	 * blob:limit filter and a promisor remote are present before
+	 * calling this function.
+	 *
+	 * Walk only promisor objects. Every object visited here is
+	 * guaranteed to be recoverable from the promisor remote, so
+	 * it is safe to drop.
+	 *
+	 * We do not use write_filtered_pack() here because git repack
+	 * routes promisor objects through repack_promisor_objects()
+	 * before the filter machinery runs, so the filtered pack never
+	 * contains promisor blobs. Direct enumeration via
+	 * ODB_FOR_EACH_OBJECT_PROMISOR_ONLY is the correct approach.
+	 */
+	ret = odb_for_each_object(repo->objects, NULL,
+			collect_promisor_blob, &cb,
+			ODB_FOR_EACH_OBJECT_PROMISOR_ONLY);
+	if (ret)
+		goto cleanup;
+
+	/*
+	 * Apply the filter to find which blobs exceed the threshold.
+	 */
+	ret = list_objects_filter__filter_oidset(repo,
+		(struct list_objects_filter_options *)filter,
+		&all_promisor_blobs,
+		&to_drop);
+	if (ret)
+		goto cleanup;
+
+	if (dry_run) {
+		oidset_iter_init(&to_drop, &iter);
+		while ((oid = oidset_iter_next(&iter)))
+			printf("%s\n", oid_to_hex(oid));
+	}
+
+cleanup:
+	oidset_clear(&all_promisor_blobs);
+	oidset_clear(&to_drop);
+	return ret;
+}
diff --git a/repack.h b/repack.h
index a5a3f7c6ba..d08e25b852 100644
--- a/repack.h
+++ b/repack.h
@@ -167,6 +167,10 @@ int write_filtered_pack(const struct write_pack_opts *opts,
 			struct existing_packs *existing,
 			struct string_list *names);
 
+int enumerate_promisor_blobs(struct repository *repo,
+			       const struct list_objects_filter_options *filter,
+			       int dry_run);
+
 int write_cruft_pack(const struct write_pack_opts *opts,
 		     const char *cruft_expiration,
 		     unsigned long combine_cruft_below_size,
-- 
2.54.0

[RFC PATCH 5/7] t7706: test --drop-filtered enumeration and validation

[Siddharth Shrimali]

Add tests for the --drop-filtered option.

  * Validation: --drop-filtered requires --filter and -a, is
    incompatible with --filter-to and --write-bitmap-index, --dry-run
    only takes effect with --drop-filtered, and --drop-filtered
    requires a promisor remote.

  * Enumeration: in a repository with a promisor remote, --dry-run
    lists promisor blobs above the filter threshold and excludes
    smaller ones. Promisor blobs are created with a synthetic promisor
    pack, following the helper pattern used in t0410.

  * Safety: a locally created large blob, which is not a promisor
    object and therefore not recoverable from the remote, is never
    listed as a drop candidate.

  * Non-destructiveness: --dry-run leaves the filtered objects intact
    in the repository.

Mentored-by: Christian Couder <christian.couder@gmail.com>
Mentored-by: Siddharth Asthana <siddharthasthana31@gmail.com>
Signed-off-by: Siddharth Shrimali <r.siddharth.shrimali@gmail.com>
---
 t/meson.build                   |   1 +
 t/t7706-repack-drop-filtered.sh | 139 ++++++++++++++++++++++++++++++++
 2 files changed, 140 insertions(+)
 create mode 100755 t/t7706-repack-drop-filtered.sh

diff --git a/t/meson.build b/t/meson.build
index 8ae6ab6c5f..37f272d7f4 100644
--- a/t/meson.build
+++ b/t/meson.build
@@ -962,6 +962,7 @@ integration_tests = [
   't7703-repack-geometric.sh',
   't7704-repack-cruft.sh',
   't7705-repack-incremental-midx.sh',
+  't7706-repack-drop-filtered.sh',
   't7800-difftool.sh',
   't7810-grep.sh',
   't7811-grep-open.sh',
diff --git a/t/t7706-repack-drop-filtered.sh b/t/t7706-repack-drop-filtered.sh
new file mode 100755
index 0000000000..b558807847
--- /dev/null
+++ b/t/t7706-repack-drop-filtered.sh
@@ -0,0 +1,139 @@
+#!/bin/sh
+
+test_description='git repack --drop-filtered enumerates
+filtered promisor blobs'
+
+. ./test-lib.sh
+
+delete_object () {
+	local repo="$1" &&
+	local obj="$2" &&
+	local path="$repo/.git/objects/$(test_oid_to_path "$obj")" &&
+	rm "$path"
+}
+
+# pack the objects into a promisor pack inside "repo",
+# it is a pack accompanied by an empty ".promisor" marker file. objects
+# in such a pack are treated as recoverable from the promisor remote.
+pack_as_from_promisor () {
+	HASH=$(git -C repo pack-objects .git/objects/pack/pack) &&
+	>repo/.git/objects/pack/pack-$HASH.promisor &&
+	echo $HASH
+}
+
+# write a blob of $1 bytes into "repo", record it as coming from the
+# promisor remote (promisor pack), and remove the loose copy so the
+# object is only present in the promisor pack.
+promisor_blob () {
+	test-tool genrandom "$1" "$2" >blob_content &&
+	OID=$(git -C repo hash-object -w --stdin <blob_content) &&
+	printf "%s\n" "$OID" | pack_as_from_promisor >/dev/null &&
+	delete_object repo "$OID" &&
+	echo "$OID"
+}
+
+# checks for options validations before any promisor walk
+test_expect_success 'setup plain repo for validation' '
+	git init plain &&
+	test_commit -C plain initial &&
+	git clone --bare plain plain.git &&
+	git -C plain.git repack -a -d
+'
+
+test_expect_success '--drop-filtered requires --filter' '
+	test_must_fail git -C plain.git repack --drop-filtered --dry-run -a 2>err &&
+	test_grep "drop-filtered requires --filter" err
+'
+
+test_expect_success '--drop-filtered cannot be used with --filter-to' '
+	test_must_fail git -C plain.git repack --drop-filtered \
+		--filter=blob:limit=1k --filter-to=./filter-out 2>err &&
+	test_grep "options .--drop-filtered. and .--filter-to. cannot be used together" err
+'
+
+test_expect_success '--dry-run only takes effect with --drop-filtered' '
+	test_must_fail git -C plain.git repack --dry-run 2>err &&
+	test_grep "dry-run only takes effect with --drop-filtered" err
+'
+
+test_expect_success '--drop-filtered without --dry-run is rejected' '
+	test_must_fail git -C plain.git repack --drop-filtered \
+		--filter=blob:limit=1k -a 2>err &&
+	test_grep "drop-filtered doesn.t work without --dry-run yet" err
+'
+
+test_expect_success '--drop-filtered requires -a' '
+	test_must_fail git -C plain.git repack --drop-filtered \
+		--filter=blob:limit=1k --dry-run 2>err &&
+	test_grep "drop-filtered requires -a" err
+'
+
+test_expect_success '--drop-filtered fails with --write-bitmap-index' '
+	test_must_fail git -C plain.git repack --drop-filtered \
+		--filter=blob:limit=1k --dry-run -a -b 2>err &&
+	test_grep "options .--drop-filtered. and .--write-bitmap-index. cannot be used together" err
+'
+
+test_expect_success '--drop-filtered fails without a promisor remote' '
+	test_must_fail git -C plain.git repack --drop-filtered \
+		--filter=blob:limit=1k --dry-run -a 2>err &&
+	test_grep "drop-filtered requires a promisor remote" err
+'
+
+# enumeration and safety tests using promisor packs
+test_expect_success 'setup repo with a promisor remote' '
+	rm -rf repo &&
+	test_create_repo repo &&
+	test_commit -C repo base &&
+
+	# mark the repo as a partial clone with a promisor remote so the
+	# promisor walk and the safety guard are satisfied.
+	git -C repo config core.repositoryformatversion 1 &&
+	git -C repo config extensions.partialclone origin &&
+	git -C repo config remote.origin.promisor true &&
+	git -C repo config remote.origin.url "." &&
+
+	BIG=$(promisor_blob big 3072) &&
+	SMALL=$(promisor_blob small 512) &&
+	echo "$BIG" >big_oid &&
+	echo "$SMALL" >small_oid
+'
+
+test_expect_success 'promisor blob over the threshold is listed' '
+	BIG=$(cat big_oid) &&
+	SMALL=$(cat small_oid) &&
+
+	git -C repo -c repack.writeBitmaps=false \
+		repack --drop-filtered --filter=blob:limit=1k --dry-run -a >out &&
+
+	test_grep "$BIG" out &&
+	test_grep ! "$SMALL" out
+'
+
+test_expect_success 'locally created blob is never listed' '
+	BIG=$(cat big_oid) &&
+
+	# large blob that exists only locally (no promisor pack) must
+	# never be a drop candidate: dropping it would be unrecoverable
+	# data loss.
+	test-tool genrandom local 4096 >local_content &&
+	LOCAL=$(git -C repo hash-object -w --stdin <local_content) &&
+
+	git -C repo -c repack.writeBitmaps=false \
+		repack --drop-filtered --filter=blob:limit=1k --dry-run -a >out &&
+
+	test_grep "$BIG" out &&
+	test_grep ! "$LOCAL" out
+'
+
+test_expect_success '--dry-run does not remove the filtered objects' '
+	BIG=$(cat big_oid) &&
+
+	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_done
-- 
2.54.0

[RFC PATCH 6/7] builtin/repack: actually drop filtered promisor blobs

[Siddharth Shrimali]

Make --drop-filtered remove the enumerated promisor blobs instead of
only listing them.

The drop set is computed before repack_promisor_objects() runs, and on
a real run it is passed in so the rebuilt promisor pack omits those
blobs. --drop-filtered implies -d so the old promisor packs, which
still contain the dropped blobs, are removed. Without this the blobs
would survive in the redundant packs. The existing repack machinery
performs the write-before-delete and fsync, so the drop is crash-safe.

The dropped blobs become absent locally but remain recoverable from the
promisor remote, so a later access lazy-fetches them back
transparently. --dry-run keeps its previous behavior, i.e. it lists the
candidates and changes nothing.

Mentored-by: Christian Couder <christian.couder@gmail.com>
Mentored-by: Siddharth Asthana <siddharthasthana31@gmail.com>
Signed-off-by: Siddharth Shrimali <r.siddharth.shrimali@gmail.com>
---
 builtin/repack.c                | 75 ++++++++++++++++++---------------
 repack-filtered.c               | 17 ++------
 repack.h                        |  4 +-
 t/t7706-repack-drop-filtered.sh | 18 +++++---
 4 files changed, 59 insertions(+), 55 deletions(-)

diff --git a/builtin/repack.c b/builtin/repack.c
index c2b07477d2..aa3257a98a 100644
--- a/builtin/repack.c
+++ b/builtin/repack.c
@@ -15,6 +15,8 @@
 #include "repack.h"
 #include "shallow.h"
 #include "list-objects-filter-options.h"
+#include "oidset.h"
+#include "hex.h"
 
 #define ALL_INTO_ONE 1
 #define LOOSEN_UNREACHABLE 2
@@ -143,6 +145,7 @@ int cmd_repack(int argc,
 	struct string_list_item *item;
 	struct string_list names = STRING_LIST_INIT_DUP;
 	struct existing_packs existing = EXISTING_PACKS_INIT;
+	struct oidset drop_oids = OIDSET_INIT;
 	struct pack_geometry geometry = { 0 };
 	struct tempfile *refs_snapshot = NULL;
 	int i, ret;
@@ -269,9 +272,6 @@ int cmd_repack(int argc,
 		die(_("--dry-run only takes effect with --drop-filtered"));
 
 	if (drop_filtered) {
-		if (!dry_run)
-			die(_("--drop-filtered doesn't work without --dry-run yet"));
-
 		if (!po_args.filter_options.choice)
 			die(_("--drop-filtered requires --filter"));
 
@@ -294,6 +294,28 @@ int cmd_repack(int argc,
 			die(_("--drop-filtered requires a promisor remote"));
 
 		write_bitmaps = 0;
+
+		/*
+		 * Dropping objects means rebuilding the promisor packs
+		 * without them and then removing the old packs, so the
+		 * redundant packs must be deleted. Imply -d on a real run.
+		 */
+		if (!dry_run)
+			delete_redundant = 1;
+
+		ret = enumerate_promisor_blobs(repo, &po_args.filter_options, &drop_oids);
+
+		if (ret)
+			goto cleanup;
+
+		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));
+		}
 	}
 
 	if (delete_redundant && repo->repository_format_precious_objects)
@@ -406,7 +428,8 @@ int cmd_repack(int argc,
 		strvec_push(&cmd.args, "--delta-islands");
 
 	if (pack_everything & ALL_INTO_ONE) {
-		repack_promisor_objects(repo, &po_args, &names, packtmp, NULL);
+		repack_promisor_objects(repo, &po_args, &names, packtmp,
+			(drop_filtered && !dry_run) ? &drop_oids : NULL);
 
 		if (existing_packs_has_non_kept(&existing) &&
 		    delete_redundant &&
@@ -589,35 +612,20 @@ int cmd_repack(int argc,
 		}
 	}
 
-	if (po_args.filter_options.choice) {
-		if (drop_filtered) {
-			/*
-			 * Enumerate promisor objects directly rather than
-			 * going through write_filtered_pack(). The filter
-			 * machinery cannot see promisor objects because
-			 * repack_promisor_objects() handles them separately
-			 * before the filter runs.
-			 */
-			ret = enumerate_promisor_blobs(repo,
-					&po_args.filter_options,
-					dry_run);
-			if (ret)
-				goto cleanup;
-		} else {
-			struct write_pack_opts opts = {
-				.po_args = &po_args,
-				.destination = filter_to,
-				.packdir = packdir,
-				.packtmp = packtmp,
-			};
-
-			if (!opts.destination)
-				opts.destination = packtmp;
-
-			ret = write_filtered_pack(&opts, &existing, &names);
-			if (ret)
-				goto cleanup;
-		}
+	if (po_args.filter_options.choice && !drop_filtered) {
+		struct write_pack_opts opts = {
+			.po_args = &po_args,
+			.destination = filter_to,
+			.packdir = packdir,
+			.packtmp = packtmp,
+		};
+
+		if (!opts.destination)
+			opts.destination = packtmp;
+
+		ret = write_filtered_pack(&opts, &existing, &names);
+		if (ret)
+			goto cleanup;
 	}
 
 	string_list_sort(&names);
@@ -697,6 +705,7 @@ int cmd_repack(int argc,
 cleanup:
 	string_list_clear(&keep_pack_list, 0);
 	string_list_clear(&names, 1);
+	oidset_clear(&drop_oids);
 	existing_packs_release(&existing);
 	pack_geometry_release(&geometry);
 	pack_objects_args_release(&po_args);
diff --git a/repack-filtered.c b/repack-filtered.c
index f5a1dae5b1..6f0cecca9b 100644
--- a/repack-filtered.c
+++ b/repack-filtered.c
@@ -87,16 +87,13 @@ static int collect_promisor_blob(const struct object_id *oid,
 
 int enumerate_promisor_blobs(struct repository *repo,
 			const struct list_objects_filter_options *filter,
-			int dry_run)
+			struct oidset *to_drop)
 {
 	struct oidset all_promisor_blobs = OIDSET_INIT;
-	struct oidset to_drop = OIDSET_INIT;
 	struct collect_cb_data cb = {
 		.repo = repo,
 		.set = &all_promisor_blobs
 	};
-	struct oidset_iter iter;
-	const struct object_id *oid;
 	int ret = 0;
 
 	/*
@@ -122,22 +119,14 @@ int enumerate_promisor_blobs(struct repository *repo,
 
 	/*
 	 * Apply the filter to find which blobs exceed the threshold.
+	 * The caller has to_drop and is responsible for clearing it.
 	 */
 	ret = list_objects_filter__filter_oidset(repo,
 		(struct list_objects_filter_options *)filter,
 		&all_promisor_blobs,
-		&to_drop);
-	if (ret)
-		goto cleanup;
-
-	if (dry_run) {
-		oidset_iter_init(&to_drop, &iter);
-		while ((oid = oidset_iter_next(&iter)))
-			printf("%s\n", oid_to_hex(oid));
-	}
+		to_drop);
 
 cleanup:
 	oidset_clear(&all_promisor_blobs);
-	oidset_clear(&to_drop);
 	return ret;
 }
diff --git a/repack.h b/repack.h
index d08e25b852..61e554e4ed 100644
--- a/repack.h
+++ b/repack.h
@@ -168,8 +168,8 @@ int write_filtered_pack(const struct write_pack_opts *opts,
 			struct string_list *names);
 
 int enumerate_promisor_blobs(struct repository *repo,
-			       const struct list_objects_filter_options *filter,
-			       int dry_run);
+			     const struct list_objects_filter_options *filter,
+			     struct oidset *to_drop);
 
 int write_cruft_pack(const struct write_pack_opts *opts,
 		     const char *cruft_expiration,
diff --git a/t/t7706-repack-drop-filtered.sh b/t/t7706-repack-drop-filtered.sh
index b558807847..41e7941799 100755
--- a/t/t7706-repack-drop-filtered.sh
+++ b/t/t7706-repack-drop-filtered.sh
@@ -56,12 +56,6 @@ test_expect_success '--dry-run only takes effect with --drop-filtered' '
 	test_grep "dry-run only takes effect with --drop-filtered" err
 '
 
-test_expect_success '--drop-filtered without --dry-run is rejected' '
-	test_must_fail git -C plain.git repack --drop-filtered \
-		--filter=blob:limit=1k -a 2>err &&
-	test_grep "drop-filtered doesn.t work without --dry-run yet" err
-'
-
 test_expect_success '--drop-filtered requires -a' '
 	test_must_fail git -C plain.git repack --drop-filtered \
 		--filter=blob:limit=1k --dry-run 2>err &&
@@ -136,4 +130,16 @@ test_expect_success '--dry-run does not remove the filtered objects' '
 	git -C repo cat-file -e "$BIG"
 '
 
+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 &&
+
+	git -C repo cat-file --batch-all-objects --batch-check="%(objectname)" >present &&
+	! grep -q "$BIG" present &&
+	grep -q "$SMALL" present
+'
+
 test_done
-- 
2.54.0

[RFC PATCH 7/7] repack-promisor: record dropped objects in a drop log

[Siddharth Shrimali]

After --drop-filtered removes promisor blobs, append a record of each
dropped object to $GIT_DIR/objects/info/promisor-dropped. Each line
records the object ID, a reflog-style timestamp (Unix seconds and
timezone), the filter spec, and the promisor remote it was attested
recoverable from like the following:

  <oid> <time> <tz> filter=<spec> remote=<name>

If a dropped object later becomes unrecoverable (for example, the
branch holding it is deleted on the promisor remote), a lazy fetch
fails with a generic error. This persistent record lets a later change
explain that the object was dropped deliberately, when, under which
filter, and from which remote it was expected to be recoverable.

The remote field lists all configured promisor remotes rather than the
specific one each dropped object is recoverable from. Determining the
exact remote would require asking the remote whether it has the object.
A "remote-object-info" command is being added to the "git cat-file
--batch" protocol for this kind of query, but it is not available yet.
A NEEDSWORK marks this for a follow-up.

The log is written only on a real run, i.e. --dry-run changes nothing.

Mentored-by: Christian Couder <christian.couder@gmail.com>
Mentored-by: Siddharth Asthana <siddharthasthana31@gmail.com>
Signed-off-by: Siddharth Shrimali <r.siddharth.shrimali@gmail.com>
---
 builtin/repack.c  |  4 +++
 repack-promisor.c | 91 +++++++++++++++++++++++++++++++++++++++++++++++
 repack.h          |  4 +++
 3 files changed, 99 insertions(+)

diff --git a/builtin/repack.c b/builtin/repack.c
index aa3257a98a..49dcbbc567 100644
--- a/builtin/repack.c
+++ b/builtin/repack.c
@@ -702,6 +702,10 @@ int cmd_repack(int argc,
 		write_midx_file(files->packed, NULL, NULL, flags);
 	}
 
+	if (drop_filtered && !dry_run)
+		append_drop_log(repo, &drop_oids,
+			expand_list_objects_filter_spec(&po_args.filter_options));
+
 cleanup:
 	string_list_clear(&keep_pack_list, 0);
 	string_list_clear(&names, 1);
diff --git a/repack-promisor.c b/repack-promisor.c
index fabfdc168a..60913a5150 100644
--- a/repack-promisor.c
+++ b/repack-promisor.c
@@ -7,6 +7,97 @@
 #include "repository.h"
 #include "run-command.h"
 #include "oidset.h"
+#include "date.h"
+#include "promisor-remote.h"
+#include "strbuf.h"
+
+/*
+ * Append the drop-log entries to the already-computed path.
+ * Returns -1 on any I/O failure so the caller can warn once.
+ * Keeping this in a separate helper avoids goto-based cleanup
+ * in append_drop_log();
+ */
+static int write_to_drop_log(struct repository *repo,
+			     const char *path,
+			     const struct oidset *dropped,
+			     const char *stamp,
+			     const char *filter_spec,
+			     const char *remotes)
+{
+	struct oidset_iter iter;
+	const struct object_id *oid;
+	FILE *fp;
+
+	if (safe_create_leading_directories(repo, (char *)path)) {
+		warning(_("could not create leading directories for '%s'"), path);
+		return -1;
+	}
+
+	fp = fopen(path, "a");
+	if (!fp) {
+		warning_errno(_("could not open '%s'"), path);
+		return -1;
+	}
+
+	oidset_iter_init(dropped, &iter);
+	while ((oid = oidset_iter_next(&iter))) {
+		if (fprintf(fp, "%s %s filter=%s remote=%s\n",
+				oid_to_hex(oid), stamp,
+				filter_spec ? filter_spec : "",
+				remotes) < 0) {
+			warning(_("could not write to '%s'"), path);
+			fclose(fp);
+			return -1;
+		}
+	}
+
+	if (fclose(fp)) {
+		warning_errno(_("could not close '%s'"), path);
+		return -1;
+	}
+
+	return 0;
+}
+
+void append_drop_log(struct repository *repo,
+		     const struct oidset *dropped,
+		     const char *filter_spec)
+{
+	char *path;
+	struct strbuf stamp = STRBUF_INIT;
+	struct strbuf remotes = STRBUF_INIT;
+	struct promisor_remote *pr;
+
+	if (!oidset_size(dropped))
+		return;
+
+	datestamp(&stamp);
+
+	/*
+	 * NEEDSWORK: we temporarily record all configured promisor remotes rather
+	 * than the specific one a given object is recoverable from because there
+	 * is currently no way to determine that locally. it would require
+	 * asking the remote whether it has the object. A "remote-object-info"
+	 * command is being added to the "git cat-file --batch" protocol for
+	 * this kind of query. Once it is merged in the codebase, this should
+	 * record the exact promisor remote that has each dropped object.
+	 */
+	for (pr = repo_promisor_remote_find(repo, NULL); pr; pr = pr->next) {
+		if (remotes.len)
+			strbuf_addch(&remotes, ',');
+		strbuf_addstr(&remotes, pr->name);
+	}
+
+	path = repo_git_path(repo, "objects/info/promisor-dropped");
+
+	if (write_to_drop_log(repo, path, dropped, stamp.buf,
+			filter_spec, remotes.buf))
+		warning(_("could not record all dropped objects in the drop log"));
+
+	strbuf_release(&stamp);
+	strbuf_release(&remotes);
+	free(path);
+}
 
 struct write_oid_context {
 	struct child_process *cmd;
diff --git a/repack.h b/repack.h
index 61e554e4ed..33309548ce 100644
--- a/repack.h
+++ b/repack.h
@@ -171,6 +171,10 @@ int enumerate_promisor_blobs(struct repository *repo,
 			     const struct list_objects_filter_options *filter,
 			     struct oidset *to_drop);
 
+void append_drop_log(struct repository *repo,
+		     const struct oidset *dropped,
+		     const char *filter_spec);
+
 int write_cruft_pack(const struct write_pack_opts *opts,
 		     const char *cruft_expiration,
 		     unsigned long combine_cruft_below_size,
-- 
2.54.0

Re: [RFC PATCH 1/7] builtin/repack.c: add --drop-filtered and --dry-run options

[Junio C Hamano]

Siddharth Shrimali <r.siddharth.shrimali@gmail.com> writes:

> --drop-filtered is incompatible with bitmap writing: filtering breaks
> the "all objects in one pack" closure that bitmaps require. An explicit
> -b is rejected with a clear error and a default-on bitmap configuration is
> silently disabled for the duration of the command.

That is very well intentioned.

> @@ -231,6 +234,10 @@ 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_BOOL(0, "drop-filtered", &drop_filtered,
> +				N_("delete filtered out objects (requires --filter)")),
> +		OPT_BOOL(0, "dry-run", &dry_run,
> +				N_("only show which objects would be dropped")),
>  		OPT_END()
>  	};
>  
> @@ -252,6 +259,43 @@ 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_opt2(drop_filtered, "--drop-filtered",
> +		write_bitmaps > 0, "--write-bitmap-index");

Hmph.  Since this step does not change the parsing or configuration
for write_bitmaps, we cannot tell if (write_bitmaps == 1) at this
point in the execution came from the command line (e.g., an earlier
call to parse_options() around line 247 of builtin/repack.c) or from
the configuration files (e.g., a call to repo_config() around
line 245).  In other words, wouldn't it be ...

> +	if (dry_run && !drop_filtered)
> +		die(_("--dry-run only takes effect with --drop-filtered"));
> +
> +	if (drop_filtered) {
> +		if (!dry_run)
> +			die(_("--drop-filtered doesn't work without --dry-run yet"));
> +
> +		if (!po_args.filter_options.choice)
> +			die(_("--drop-filtered requires --filter"));
> +
> +		if (!(pack_everything & ALL_INTO_ONE))
> +			die(_("--drop-filtered requires -a"));
> +
> +		/*
> +		 * Only blob:limit=<n> is supported for now. Reject other
> +		 * filter choices early, before walking the object database.
> +		 */
> +		if (po_args.filter_options.choice != LOFC_BLOB_LIMIT)
> +			die(_("--drop-filtered only supports --filter=blob:limit=<n> for now"));
> +
> +		/*
> +		 * Without a promisor remote there is nowhere to re-fetch the
> +		 * dropped objects from, so dropping them would be permanent
> +		 * data loss.
> +		 */
> +		if (!repo_has_promisor_remote(repo))
> +			die(_("--drop-filtered requires a promisor remote"));
> +
> +		write_bitmaps = 0;

... way too late to drop the flag here?

> +	}
> +
>  	if (delete_redundant && repo->repository_format_precious_objects)
>  		die(_("cannot delete packs in a precious-objects repo"));

Re: [RFC PATCH 1/7] builtin/repack.c: add --drop-filtered and --dry-run options

[Siddharth Shrimali]

On Fri, 17 Jul 2026 at 02:38, Junio C Hamano <gitster@pobox.com> wrote:
>
> > +     die_for_incompatible_opt2(drop_filtered, "--drop-filtered",
> > +             !!filter_to, "--filter-to");
> > +
> > +     die_for_incompatible_opt2(drop_filtered, "--drop-filtered",
> > +             write_bitmaps > 0, "--write-bitmap-index");
>
> Hmph.  Since this step does not change the parsing or configuration
> for write_bitmaps, we cannot tell if (write_bitmaps == 1) at this
> point in the execution came from the command line (e.g., an earlier
> call to parse_options() around line 247 of builtin/repack.c) or from
> the configuration files (e.g., a call to repo_config() around
> line 245).  In other words, wouldn't it be ...
>
> > +             write_bitmaps = 0;
>
> ... way too late to drop the flag here?
>

right, thanks! The commit message claims I distinguish an explicit
-b/--write-bitmap-index from a config-provided default, but the code
only tests write_bitmaps > 0, which cannot tell the two apart at this point

For v2, alongside other changes, i'll distinguish the two, so that an explicit
-b on the command line errors out, while a config-provided default is
silently disabled for the duration of the command..

Thanks!
Siddharth Shrimali

Re: [RFC PATCH 1/7] builtin/repack.c: add --drop-filtered and --dry-run options

[Christian Couder]

On Thu, Jul 16, 2026 at 3:29 PM Siddharth Shrimali
<r.siddharth.shrimali@gmail.com> wrote:
>
> Add two new command-line options to 'git-repack':
>
>   --drop-filtered: intended to eventually delete objects that match
>                    the filter specification. Requires --filter and -a,
>                    and is incompatible with --filter-to.
>   --dry-run: show which objects would be dropped without making any
>              changes. Only meaningful with --drop-filtered.

An alternative would be `--drop-filtered[=dry-run]`, which might be
extended with other `--drop-filtered` specific options later.

I think separating `--dry-run` from `--drop-filtered` like this patch
does makes sense though if we think that `--dry-run` could be useful
later without `--drop-filtered`. The fact that a number of other
commands already have a `--dry-run` option might be a good sign.

Anyway it would be nice if the commit message explained a bit the
choice to have a separate `--dry-run` option.

Re: [RFC PATCH 1/7] builtin/repack.c: add --drop-filtered and --dry-run options

[Siddharth Shrimali]

Hey Christian,

On Sat, 18 Jul 2026 at 18:01, Christian Couder
<christian.couder@gmail.com> wrote:
> An alternative would be `--drop-filtered[=dry-run]`, which might be
> extended with other `--drop-filtered` specific options later.
>
> I think separating `--dry-run` from `--drop-filtered` like this patch
> does makes sense though if we think that `--dry-run` could be useful
> later without `--drop-filtered`. The fact that a number of other
> commands already have a `--dry-run` option might be a good sign.

Agreed. I chose a separate --dry-run mainly for consistency with the many other
Git commands that already use it that way, and because it leaves room
for --dry-run
to apply to other repack behavior later rather than being tied to
--drop-filtered.

That said, I do like the --drop-filtered[=dry-run] form for keeping future
--drop-filtered-specific options together, and it might end up being the tidier
choice.

Since this is an RFC, I'd genuinely welcome others' thoughts on which one
holds up better in the long run, before settling on any one command-line.

>
> Anyway it would be nice if the commit message explained a bit the
> choice to have a separate `--dry-run` option.

Either way, for v2 I'll add a note to the commit message explaining
the final choice.

Thanks!

Re: [RFC PATCH 0/7] repack: add --drop-filtered to reclaim space in partial clones

[Siddharth Asthana]

On 16/07/26 18:58, Siddharth Shrimali wrote:
> This is an RFC series seeking feedback on the design and approach.
> Several pieces are still missing (noted below) and the commit
> organization needs cleanup.
> 
> Partial clones let you work with large repositories without downloading
> every blob up front and the missing blobs are lazily fetched from the promisor
> remote on demand. Over time, though, these lazily-fetched blobs
> accumulate locally and there is currently no safe, built-in way to
> reclaim that disk space instead of re-cloning.
> 
> This series adds a "git repack --drop-filtered --filter=<spec>" command
> that removes large, locally-held promisor blobs that are recoverable
> from the promisor remote. The dropped blobs become absent locally but
> remain lazily re-fetchable, making the partial-clone still reversible.
> 
> How it works:
>    * Enumerate promisor objects directly (ODB_FOR_EACH_OBJECT_PROMISOR_ONLY)
>      and select the blobs exceeding the filter threshold. Because every
>      enumerated object is a promisor object, it is guaranteed recoverable and
>      locally-created objects are never candidates.


This looks like the right approach to me. Going through the promisor 
repack path instead of write_filtered_pack() matches how repack already 
splits promisor objects out.


> 
>    * Rebuild the promisor pack without the selected blobs, reusing the
>      existing repack machinery, so the drop is crash-safe.
> 
>    * Record each dropped object in a drop log
>      ($GIT_DIR/objects/info/promisor-dropped) so a later change can
>      explain a failed lazy fetch (when it was dropped, which filter
>      matched, which remotes) instead of a bare "could not fetch" error.
> 
>    * --dry-run lists the candidates and changes nothing.
> 
> Planned follow-ups:
>    * Safety guards: refuse to run while a merge/rebase/cherry-pick is in
>      progress, and refuse to drop blobs referenced by the current index.


I think these matter before we present this as a real space-reclaim
tool. Without the index guard especially, users may drop blobs and then
immediately fetch them back on the next command that needs the worktree.

The drop log and remote-object-info can wait. I would not block the
next RFC round on them.

On the UI, I am fine with a separate --dry-run for now (same as
Christian). We can revisit a --drop-filtered=<mode> form later if we
grow more drop-specific options.

Thanks.
Siddharth


> 
>    * Authoritative remote verification: the drop log currently lists all
>      configured promisor remotes rather than the exact remote each object
>      is recoverable from, because there is no client-side way to query a
>      remote for object availability yet. A "remote-object-info" command
>      is being added to the "git cat-file --batch" protocol for this. Once
>      available, the exact remote can be recorded.
> 
> Known issues to address in v2:
>    * There is churn between "enumerate promisor blobs" and "actually drop
>      filtered promisor blobs". The former introduces
>      enumerate_promisor_blobs() with an interim signature that the latter
>      rewrites. These will be reorganized so the function is introduced
>      in its final form.
> 
>    * The tests are in a standalone commit. They will instead be
>      distributed into the commits that introduce the behavior they test.
> 
> Siddharth Shrimali (7):
>    builtin/repack.c: add --drop-filtered and --dry-run options
>    list-objects-filter: add list_objects_filter__filter_oidset()
>    repack-promisor: allow excluding objects from the rebuilt promisor
>      pack
>    builtin/repack: enumerate promisor blobs for --drop-filtered
>    t7706: test --drop-filtered enumeration and validation
>    builtin/repack: actually drop filtered promisor blobs
>    repack-promisor: record dropped objects in a drop log
> 
>   builtin/repack.c                |  76 ++++++++++++++++-
>   list-objects-filter.c           |  45 ++++++++++
>   list-objects-filter.h           |  16 ++++
>   repack-filtered.c               |  81 ++++++++++++++++++
>   repack-promisor.c               | 106 ++++++++++++++++++++++-
>   repack.h                        |  12 ++-
>   t/meson.build                   |   1 +
>   t/t7706-repack-drop-filtered.sh | 145 ++++++++++++++++++++++++++++++++
>   8 files changed, 478 insertions(+), 4 deletions(-)
>   create mode 100755 t/t7706-repack-drop-filtered.sh
> 

Re: [RFC PATCH 1/7] builtin/repack.c: add --drop-filtered and --dry-run options

[Siddharth Asthana]

On 17/07/26 02:38, Junio C Hamano wrote:
> Siddharth Shrimali <r.siddharth.shrimali@gmail.com> writes:
> 
>> --drop-filtered is incompatible with bitmap writing: filtering breaks
>> the "all objects in one pack" closure that bitmaps require. An explicit
>> -b is rejected with a clear error and a default-on bitmap configuration is
>> silently disabled for the duration of the command.
> 
> That is very well intentioned.
> 
>> @@ -231,6 +234,10 @@ 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_BOOL(0, "drop-filtered", &drop_filtered,
>> +				N_("delete filtered out objects (requires --filter)")),
>> +		OPT_BOOL(0, "dry-run", &dry_run,
>> +				N_("only show which objects would be dropped")),
>>   		OPT_END()
>>   	};
>>   
>> @@ -252,6 +259,43 @@ 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_opt2(drop_filtered, "--drop-filtered",
>> +		write_bitmaps > 0, "--write-bitmap-index");
> 
> Hmph.  Since this step does not change the parsing or configuration
> for write_bitmaps, we cannot tell if (write_bitmaps == 1) at this
> point in the execution came from the command line (e.g., an earlier
> call to parse_options() around line 247 of builtin/repack.c) or from
> the configuration files (e.g., a call to repo_config() around
> line 245).  In other words, wouldn't it be ...
> 
>> +	if (dry_run && !drop_filtered)
>> +		die(_("--dry-run only takes effect with --drop-filtered"));
>> +
>> +	if (drop_filtered) {
>> +		if (!dry_run)
>> +			die(_("--drop-filtered doesn't work without --dry-run yet"));
>> +
>> +		if (!po_args.filter_options.choice)
>> +			die(_("--drop-filtered requires --filter"));
>> +
>> +		if (!(pack_everything & ALL_INTO_ONE))
>> +			die(_("--drop-filtered requires -a"));
>> +
>> +		/*
>> +		 * Only blob:limit=<n> is supported for now. Reject other
>> +		 * filter choices early, before walking the object database.
>> +		 */
>> +		if (po_args.filter_options.choice != LOFC_BLOB_LIMIT)
>> +			die(_("--drop-filtered only supports --filter=blob:limit=<n> for now"));
>> +
>> +		/*
>> +		 * Without a promisor remote there is nowhere to re-fetch the
>> +		 * dropped objects from, so dropping them would be permanent
>> +		 * data loss.
>> +		 */
>> +		if (!repo_has_promisor_remote(repo))
>> +			die(_("--drop-filtered requires a promisor remote"));
>> +
>> +		write_bitmaps = 0;
> 
> ... way too late to drop the flag here?



Yes, I agree. At that point write_bitmaps > 0 can come from either
-b/--write-bitmap-index or repack.writeBitmaps, so we cannot both
error on an explicit -b and silently clear a config default with the
same check.

For v2 it would be nice to treat those two cases differently.

Thanks.
Siddharth


> 
>> +	}
>> +
>>   	if (delete_redundant && repo->repository_format_precious_objects)
>>   		die(_("cannot delete packs in a precious-objects repo"));

Re: [RFC PATCH 7/7] repack-promisor: record dropped objects in a drop log

[Siddharth Asthana]

On 16/07/26 18:58, Siddharth Shrimali wrote:
> After --drop-filtered removes promisor blobs, append a record of each
> dropped object to $GIT_DIR/objects/info/promisor-dropped. Each line
> records the object ID, a reflog-style timestamp (Unix seconds and
> timezone), the filter spec, and the promisor remote it was attested
> recoverable from like the following:
> 
>    <oid> <time> <tz> filter=<spec> remote=<name>
> 
> If a dropped object later becomes unrecoverable (for example, the
> branch holding it is deleted on the promisor remote), a lazy fetch
> fails with a generic error. This persistent record lets a later change
> explain that the object was dropped deliberately, when, under which
> filter, and from which remote it was expected to be recoverable.


I like the idea of better errors when a later lazy fetch fails.

An alternative would be to wait until we actually have that error-path
change in the same series, so we do not grow an on-disk format that
nothing reads yet. I think keeping the log in the RFC is fine though
if you find it useful while developing; I would not treat it as
required for the first mergeable version.



> 
> The remote field lists all configured promisor remotes rather than the
> specific one each dropped object is recoverable from. Determining the
> exact remote would require asking the remote whether it has the object.
> A "remote-object-info" command is being added to the "git cat-file
> --batch" protocol for this kind of query, but it is not available yet.
> A NEEDSWORK marks this for a follow-up.
> 
> The log is written only on a real run, i.e. --dry-run changes nothing.
> 
> Mentored-by: Christian Couder <christian.couder@gmail.com>
> Mentored-by: Siddharth Asthana <siddharthasthana31@gmail.com>
> Signed-off-by: Siddharth Shrimali <r.siddharth.shrimali@gmail.com>
> ---
>   builtin/repack.c  |  4 +++
>   repack-promisor.c | 91 +++++++++++++++++++++++++++++++++++++++++++++++
>   repack.h          |  4 +++
>   3 files changed, 99 insertions(+)
> 
> diff --git a/builtin/repack.c b/builtin/repack.c
> index aa3257a98a..49dcbbc567 100644
> --- a/builtin/repack.c
> +++ b/builtin/repack.c
> @@ -702,6 +702,10 @@ int cmd_repack(int argc,
>   		write_midx_file(files->packed, NULL, NULL, flags);
>   	}
>   
> +	if (drop_filtered && !dry_run)
> +		append_drop_log(repo, &drop_oids,
> +			expand_list_objects_filter_spec(&po_args.filter_options));
> +
>   cleanup:
>   	string_list_clear(&keep_pack_list, 0);
>   	string_list_clear(&names, 1);
> diff --git a/repack-promisor.c b/repack-promisor.c
> index fabfdc168a..60913a5150 100644
> --- a/repack-promisor.c
> +++ b/repack-promisor.c
> @@ -7,6 +7,97 @@
>   #include "repository.h"
>   #include "run-command.h"
>   #include "oidset.h"
> +#include "date.h"
> +#include "promisor-remote.h"
> +#include "strbuf.h"
> +
> +/*
> + * Append the drop-log entries to the already-computed path.
> + * Returns -1 on any I/O failure so the caller can warn once.
> + * Keeping this in a separate helper avoids goto-based cleanup
> + * in append_drop_log();
> + */
> +static int write_to_drop_log(struct repository *repo,
> +			     const char *path,
> +			     const struct oidset *dropped,
> +			     const char *stamp,
> +			     const char *filter_spec,
> +			     const char *remotes)
> +{
> +	struct oidset_iter iter;
> +	const struct object_id *oid;
> +	FILE *fp;
> +
> +	if (safe_create_leading_directories(repo, (char *)path)) {
> +		warning(_("could not create leading directories for '%s'"), path);
> +		return -1;
> +	}
> +
> +	fp = fopen(path, "a");
> +	if (!fp) {
> +		warning_errno(_("could not open '%s'"), path);
> +		return -1;
> +	}
> +
> +	oidset_iter_init(dropped, &iter);
> +	while ((oid = oidset_iter_next(&iter))) {
> +		if (fprintf(fp, "%s %s filter=%s remote=%s\n",
> +				oid_to_hex(oid), stamp,
> +				filter_spec ? filter_spec : "",
> +				remotes) < 0) {
> +			warning(_("could not write to '%s'"), path);
> +			fclose(fp);
> +			return -1;
> +		}
> +	}
> +
> +	if (fclose(fp)) {
> +		warning_errno(_("could not close '%s'"), path);
> +		return -1;
> +	}
> +
> +	return 0;
> +}
> +
> +void append_drop_log(struct repository *repo,
> +		     const struct oidset *dropped,
> +		     const char *filter_spec)
> +{
> +	char *path;
> +	struct strbuf stamp = STRBUF_INIT;
> +	struct strbuf remotes = STRBUF_INIT;
> +	struct promisor_remote *pr;
> +
> +	if (!oidset_size(dropped))
> +		return;
> +
> +	datestamp(&stamp);
> +
> +	/*
> +	 * NEEDSWORK: we temporarily record all configured promisor remotes rather
> +	 * than the specific one a given object is recoverable from because there


Recording all promisor remotes for now looks OK to me with that
NEEDSWORK. I would not block this on remote-object-info.


> +	 * is currently no way to determine that locally. it would require
> +	 * asking the remote whether it has the object. A "remote-object-info"
> +	 * command is being added to the "git cat-file --batch" protocol for
> +	 * this kind of query. Once it is merged in the codebase, this should
> +	 * record the exact promisor remote that has each dropped object.
> +	 */
> +	for (pr = repo_promisor_remote_find(repo, NULL); pr; pr = pr->next) {
> +		if (remotes.len)
> +			strbuf_addch(&remotes, ',');
> +		strbuf_addstr(&remotes, pr->name);
> +	}
> +
> +	path = repo_git_path(repo, "objects/info/promisor-dropped");


If we keep it, it would be nice to document this path (for example in
gitrepository-layout) and to have a small test that a real drop appends
a line.


Thanks
Siddharth


> +
> +	if (write_to_drop_log(repo, path, dropped, stamp.buf,
> +			filter_spec, remotes.buf))
> +		warning(_("could not record all dropped objects in the drop log"));
> +
> +	strbuf_release(&stamp);
> +	strbuf_release(&remotes);
> +	free(path);
> +}
>   
>   struct write_oid_context {
>   	struct child_process *cmd;
> diff --git a/repack.h b/repack.h
> index 61e554e4ed..33309548ce 100644
> --- a/repack.h
> +++ b/repack.h
> @@ -171,6 +171,10 @@ int enumerate_promisor_blobs(struct repository *repo,
>   			     const struct list_objects_filter_options *filter,
>   			     struct oidset *to_drop);
>   
> +void append_drop_log(struct repository *repo,
> +		     const struct oidset *dropped,
> +		     const char *filter_spec);
> +
>   int write_cruft_pack(const struct write_pack_opts *opts,
>   		     const char *cruft_expiration,
>   		     unsigned long combine_cruft_below_size,

Re: [RFC PATCH 6/7] builtin/repack: actually drop filtered promisor blobs

[Siddharth Asthana]

On 16/07/26 18:58, Siddharth Shrimali wrote:
> Make --drop-filtered remove the enumerated promisor blobs instead of
> only listing them.
> 
> The drop set is computed before repack_promisor_objects() runs, and on
> a real run it is passed in so the rebuilt promisor pack omits those
> blobs. --drop-filtered implies -d so the old promisor packs, which
> still contain the dropped blobs, are removed. Without this the blobs
> would survive in the redundant packs. The existing repack machinery
> performs the write-before-delete and fsync, so the drop is crash-safe.
> 
> The dropped blobs become absent locally but remain recoverable from the
> promisor remote, so a later access lazy-fetches them back
> transparently. --dry-run keeps its previous behavior, i.e. it lists the
> candidates and changes nothing.
> 
> Mentored-by: Christian Couder <christian.couder@gmail.com>
> Mentored-by: Siddharth Asthana <siddharthasthana31@gmail.com>
> Signed-off-by: Siddharth Shrimali <r.siddharth.shrimali@gmail.com>
> ---
>   builtin/repack.c                | 75 ++++++++++++++++++---------------
>   repack-filtered.c               | 17 ++------
>   repack.h                        |  4 +-
>   t/t7706-repack-drop-filtered.sh | 18 +++++---
>   4 files changed, 59 insertions(+), 55 deletions(-)
> 
> diff --git a/builtin/repack.c b/builtin/repack.c
> index c2b07477d2..aa3257a98a 100644
> --- a/builtin/repack.c
> +++ b/builtin/repack.c
> @@ -15,6 +15,8 @@
>   #include "repack.h"
>   #include "shallow.h"
>   #include "list-objects-filter-options.h"
> +#include "oidset.h"
> +#include "hex.h"
>   
>   #define ALL_INTO_ONE 1
>   #define LOOSEN_UNREACHABLE 2
> @@ -143,6 +145,7 @@ int cmd_repack(int argc,
>   	struct string_list_item *item;
>   	struct string_list names = STRING_LIST_INIT_DUP;
>   	struct existing_packs existing = EXISTING_PACKS_INIT;
> +	struct oidset drop_oids = OIDSET_INIT;
>   	struct pack_geometry geometry = { 0 };
>   	struct tempfile *refs_snapshot = NULL;
>   	int i, ret;
> @@ -269,9 +272,6 @@ int cmd_repack(int argc,
>   		die(_("--dry-run only takes effect with --drop-filtered"));
>   
>   	if (drop_filtered) {
> -		if (!dry_run)
> -			die(_("--drop-filtered doesn't work without --dry-run yet"));
> -
>   		if (!po_args.filter_options.choice)
>   			die(_("--drop-filtered requires --filter"));
>   
> @@ -294,6 +294,28 @@ int cmd_repack(int argc,
>   			die(_("--drop-filtered requires a promisor remote"));
>   
>   		write_bitmaps = 0;
> +
> +		/*
> +		 * Dropping objects means rebuilding the promisor packs
> +		 * without them and then removing the old packs, so the
> +		 * redundant packs must be deleted. Imply -d on a real run.
> +		 */
> +		if (!dry_run)
> +			delete_redundant = 1;


Yes, without that the drop would not actually reclaim space.

It would be nice if the documentation mentioned that a real
--drop-filtered run implies -d.


Thanks


> +
> +		ret = enumerate_promisor_blobs(repo, &po_args.filter_options, &drop_oids);
> +
> +		if (ret)
> +			goto cleanup;
> +
> +		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));
> +		}
>   	}
>   
>   	if (delete_redundant && repo->repository_format_precious_objects)
> @@ -406,7 +428,8 @@ int cmd_repack(int argc,
>   		strvec_push(&cmd.args, "--delta-islands");
>   
>   	if (pack_everything & ALL_INTO_ONE) {
> -		repack_promisor_objects(repo, &po_args, &names, packtmp, NULL);
> +		repack_promisor_objects(repo, &po_args, &names, packtmp,
> +			(drop_filtered && !dry_run) ? &drop_oids : NULL);
>   
>   		if (existing_packs_has_non_kept(&existing) &&
>   		    delete_redundant &&
> @@ -589,35 +612,20 @@ int cmd_repack(int argc,
>   		}
>   	}
>   
> -	if (po_args.filter_options.choice) {
> -		if (drop_filtered) {
> -			/*
> -			 * Enumerate promisor objects directly rather than
> -			 * going through write_filtered_pack(). The filter
> -			 * machinery cannot see promisor objects because
> -			 * repack_promisor_objects() handles them separately
> -			 * before the filter runs.
> -			 */
> -			ret = enumerate_promisor_blobs(repo,
> -					&po_args.filter_options,
> -					dry_run);
> -			if (ret)
> -				goto cleanup;
> -		} else {
> -			struct write_pack_opts opts = {
> -				.po_args = &po_args,
> -				.destination = filter_to,
> -				.packdir = packdir,
> -				.packtmp = packtmp,
> -			};
> -
> -			if (!opts.destination)
> -				opts.destination = packtmp;
> -
> -			ret = write_filtered_pack(&opts, &existing, &names);
> -			if (ret)
> -				goto cleanup;
> -		}
> +	if (po_args.filter_options.choice && !drop_filtered) {
> +		struct write_pack_opts opts = {
> +			.po_args = &po_args,
> +			.destination = filter_to,
> +			.packdir = packdir,
> +			.packtmp = packtmp,
> +		};
> +
> +		if (!opts.destination)
> +			opts.destination = packtmp;
> +
> +		ret = write_filtered_pack(&opts, &existing, &names);
> +		if (ret)
> +			goto cleanup;
>   	}
>   
>   	string_list_sort(&names);
> @@ -697,6 +705,7 @@ int cmd_repack(int argc,
>   cleanup:
>   	string_list_clear(&keep_pack_list, 0);
>   	string_list_clear(&names, 1);
> +	oidset_clear(&drop_oids);
>   	existing_packs_release(&existing);
>   	pack_geometry_release(&geometry);
>   	pack_objects_args_release(&po_args);
> diff --git a/repack-filtered.c b/repack-filtered.c
> index f5a1dae5b1..6f0cecca9b 100644
> --- a/repack-filtered.c
> +++ b/repack-filtered.c
> @@ -87,16 +87,13 @@ static int collect_promisor_blob(const struct object_id *oid,
>   
>   int enumerate_promisor_blobs(struct repository *repo,
>   			const struct list_objects_filter_options *filter,
> -			int dry_run)
> +			struct oidset *to_drop)
>   {
>   	struct oidset all_promisor_blobs = OIDSET_INIT;
> -	struct oidset to_drop = OIDSET_INIT;
>   	struct collect_cb_data cb = {
>   		.repo = repo,
>   		.set = &all_promisor_blobs
>   	};
> -	struct oidset_iter iter;
> -	const struct object_id *oid;
>   	int ret = 0;
>   
>   	/*
> @@ -122,22 +119,14 @@ int enumerate_promisor_blobs(struct repository *repo,
>   
>   	/*
>   	 * Apply the filter to find which blobs exceed the threshold.
> +	 * The caller has to_drop and is responsible for clearing it.
>   	 */
>   	ret = list_objects_filter__filter_oidset(repo,
>   		(struct list_objects_filter_options *)filter,
>   		&all_promisor_blobs,
> -		&to_drop);
> -	if (ret)
> -		goto cleanup;
> -
> -	if (dry_run) {
> -		oidset_iter_init(&to_drop, &iter);
> -		while ((oid = oidset_iter_next(&iter)))
> -			printf("%s\n", oid_to_hex(oid));
> -	}
> +		to_drop);
>   
>   cleanup:
>   	oidset_clear(&all_promisor_blobs);
> -	oidset_clear(&to_drop);
>   	return ret;
>   }
> diff --git a/repack.h b/repack.h
> index d08e25b852..61e554e4ed 100644
> --- a/repack.h
> +++ b/repack.h
> @@ -168,8 +168,8 @@ int write_filtered_pack(const struct write_pack_opts *opts,
>   			struct string_list *names);
>   
>   int enumerate_promisor_blobs(struct repository *repo,
> -			       const struct list_objects_filter_options *filter,
> -			       int dry_run);
> +			     const struct list_objects_filter_options *filter,
> +			     struct oidset *to_drop);
>   
>   int write_cruft_pack(const struct write_pack_opts *opts,
>   		     const char *cruft_expiration,
> diff --git a/t/t7706-repack-drop-filtered.sh b/t/t7706-repack-drop-filtered.sh
> index b558807847..41e7941799 100755
> --- a/t/t7706-repack-drop-filtered.sh
> +++ b/t/t7706-repack-drop-filtered.sh
> @@ -56,12 +56,6 @@ test_expect_success '--dry-run only takes effect with --drop-filtered' '
>   	test_grep "dry-run only takes effect with --drop-filtered" err
>   '
>   
> -test_expect_success '--drop-filtered without --dry-run is rejected' '
> -	test_must_fail git -C plain.git repack --drop-filtered \
> -		--filter=blob:limit=1k -a 2>err &&
> -	test_grep "drop-filtered doesn.t work without --dry-run yet" err
> -'
> -
>   test_expect_success '--drop-filtered requires -a' '
>   	test_must_fail git -C plain.git repack --drop-filtered \
>   		--filter=blob:limit=1k --dry-run 2>err &&
> @@ -136,4 +130,16 @@ test_expect_success '--dry-run does not remove the filtered objects' '
>   	git -C repo cat-file -e "$BIG"
>   '
>   
> +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 &&
> +
> +	git -C repo cat-file --batch-all-objects --batch-check="%(objectname)" >present &&
> +	! grep -q "$BIG" present &&
> +	grep -q "$SMALL" present
> +'
> +
>   test_done

Re: [RFC PATCH 0/7] repack: add --drop-filtered to reclaim space in partial clones

[Siddharth Shrimali]

Hi Siddharth,

On Fri, 24 Jul 2026 at 00:56, Siddharth Asthana
<siddharthasthana31@gmail.com> wrote:
> I think these matter before we present this as a real space-reclaim
> tool. Without the index guard especially, users may drop blobs and then
> immediately fetch them back on the next command that needs the worktree.

right, I will move the safety guards into v2; refuse to run
mid-merge/rebase/cherry-pick,
and refuse to drop blobs referenced by the current index

>
> The drop log and remote-object-info can wait. I would not block the
> next RFC round on them.

sounds good, I can pull the drop log out of the core series and keep
remote-object-info as a follow-up that upgrades the remote attribution
once it is available.


> On the UI, I am fine with a separate --dry-run for now (same as
> Christian). We can revisit a --drop-filtered=<mode> form later if we
> grow more drop-specific options.

i will keep the separate --dry-run for v2 and add a note to the commit message
explaining the choice, so the --drop-filtered=<mode> option stays open for later
without committing to it now.

Re: [RFC PATCH 6/7] builtin/repack: actually drop filtered promisor blobs

[Siddharth Shrimali]

On 24/07/26 01:12, Siddharth Asthana wrote:
> Yes, without that the drop would not actually reclaim space.
>
> It would be nice if the documentation mentioned that a real
> --drop-filtered run implies -d.

sounds good.. there is no git-repack documentation for --drop-filtered yet,
so when I add it in v2 I'll call out explicitly that a real
(non-dry-run) --drop-filtered run implies -d.

Re: [RFC PATCH 7/7] repack-promisor: record dropped objects in a drop log

[Siddharth Shrimali]

On Fri, 24 Jul 2026 at 01:11, Siddharth Asthana
<siddharthasthana31@gmail.com> wrote:
> Recording all promisor remotes for now looks OK to me with that
> NEEDSWORK. I would not block this on remote-object-info.
>
>
> > +      * is currently no way to determine that locally. it would require
> > +      * asking the remote whether it has the object. A "remote-object-info"
> > +      * command is being added to the "git cat-file --batch" protocol for
> > +      * this kind of query. Once it is merged in the codebase, this should
> > +      * record the exact promisor remote that has each dropped object.
> > +      */
> > +     for (pr = repo_promisor_remote_find(repo, NULL); pr; pr = pr->next) {
> > +             if (remotes.len)
> > +                     strbuf_addch(&remotes, ',');
> > +             strbuf_addstr(&remotes, pr->name);
> > +     }
> > +
> > +     path = repo_git_path(repo, "objects/info/promisor-dropped");
>
>
> If we keep it, it would be nice to document this path (for example in
> gitrepository-layout) and to have a small test that a real drop appends
> a line.

sounds good, when the log is finalized, whether as a min.  version now
or alongside the
error-path change later, I can document objects/info/promisor-dropped in
gitrepository-layout then

Thanks,
Siddharth Shrimali

[GSoC PATCH v2 0/7] repack: add --drop-filtered to reclaim space in partial clones

[Siddharth Shrimali]

This is v2 of the series adding "git repack --drop-filtered" to reclaim
disk space in partial clones by dropping large, locally-held promisor
blobs that remain recoverable from the promisor remote. v1 was sent as
an RFC [1].

Partial clones let you work with large repositories without downloading
every blob up front. Mising blobs are lazily fetched from the promisor
remote on demand. Over time these accumulate locally and there is
currently no safe, built-in way to reclaim that space short of
re-cloning. This series adds that reverse direction: enumerate promisor
blobs over a size threshold, drop them locally, and rely on the existing
lazy-fetch machinery to bring them back transparently when needed.

How it works:
  * Enumerate promisor objects directly (ODB_FOR_EACH_OBJECT_PROMISOR_ONLY)
    and select the blobs exceeding the filter threshold. Every enumerated
    object is a promisor object by construction, so it is guaranteed
    recoverable and locally-created objects are never candidates.
  * Rebuild the promisor pack without the selected blobs, reusing the
    existing repack machinery, so the drop is crash-safe (write, fsync,
    install, then delete the old pack).
  * --dry-run lists the candidates and changes nothing.

Safety guards refuse to run while a merge, rebase, am, cherry-pick,
revert, or bisect is in progress, and refuse to drop a blob referenced
by the current index (it would only be lazily re-fetched by the next
worktree command). Both are skipped for bare repositories.

Changes since v1:
  * distinguish an explicit -b/--write-bitmap-index on the command line
    (reported as a conflict) from a repack.writeBitmaps config value
    (silently disabled for the command). This addresses Junio's review
    that the previous check could not tell the two apart
  * documented the choice to keep --dry-run as a separate option rather
    than --drop-filtered=<mode>
  * implemented the safety guards
  * Added git-repack documentation for --drop-filtered and --dry-run
  * Reorganised so enumerate_promisor_blobs() is introduced in its final
    signature
  * Distributed the tests into the commits that introduce each behavior,
    instead of a single standalone test commit.
  * Dropped the drop-log commit from this series

To do:
  * Remote verification: verifying against the remote awaits the "remote-object-info"
    cat-file protocol command.
  * Drop log: introduce with the error-path change that reads it.
  * --verbose: space-reclaimed reporting.

[1] https://lore.kernel.org/git/20260716132848.95982-1-r.siddharth.shrimali@gmail.com/

Siddharth Shrimali (7):
  builtin/repack.c: add --drop-filtered and --dry-run options
  list-objects-filter: add list_objects_filter__filter_oidset()
  repack-promisor: allow excluding objects from the rebuilt promisor
    pack
  builtin/repack: enumerate promisor blobs for --drop-filtered
  builtin/repack: actually drop filtered promisor blobs
  builtin/repack: add safety guards for --drop-filtered
  Documentation/git-repack: document --drop-filtered and --dry-run

 Documentation/git-repack.adoc   |  35 +++++++
 builtin/repack.c                | 135 +++++++++++++++++++++++-
 list-objects-filter.c           |  45 ++++++++
 list-objects-filter.h           |  16 +++
 repack-filtered.c               |  81 +++++++++++++++
 repack-promisor.c               |  15 ++-
 repack.h                        |   8 +-
 t/meson.build                   |   1 +
 t/t7706-repack-drop-filtered.sh | 179 ++++++++++++++++++++++++++++++++
 9 files changed, 511 insertions(+), 4 deletions(-)
 create mode 100755 t/t7706-repack-drop-filtered.sh

-- 
2.54.0

[GSoC PATCH v2 1/7] builtin/repack.c: add --drop-filtered and --dry-run options

[Siddharth Shrimali]

Add two new command-line options to 'git-repack':

  --drop-filtered: intended to eventually delete objects that match
                   the filter specification. Requires --filter and -a,
                   and is incompatible with --filter-to.
  --dry-run: show which objects would be dropped without making any
             changes. Only meaningful with --drop-filtered.

Keep --dry-run as a separate option rather than folding it into
--drop-filtered (e.g --drop-filtered=dry-run), to stay consistent with
the --dry-run option other Git commands already provide and to leave
room for it to describe other repack behavior later. A
--drop-filtered=<mode> form can still be added later if more
drop-specific modes are needed.

--drop-filtered also requires a promisor remote to be configured, since
dropping objects without a remote to fetch them back from would be
permanent data loss.

--drop-filtered is incompatible with bitmap writing: filtering breaks
the "all objects in one pack" closure that bitmaps require. Snapshot
the bitmap setting after config but before option parsing so an
explicit -b/--write-bitmap-index on the command line can be told apart
from a repack.writeBitmaps configuration value. An explicit -b is
reported as a conflict, while a config-provided default is silently
disabled for the duration of the command.

These options currently only perform validation. The actual enumeration
and deletion will be added in follow-up commits.

Mentored-by: Christian Couder <christian.couder@gmail.com>
Mentored-by: Siddharth Asthana <siddharthasthana31@gmail.com>
Signed-off-by: Siddharth Shrimali <r.siddharth.shrimali@gmail.com>
---
 builtin/repack.c                | 63 +++++++++++++++++++++++++++++++++
 t/meson.build                   |  1 +
 t/t7706-repack-drop-filtered.sh | 49 +++++++++++++++++++++++++
 3 files changed, 113 insertions(+)
 create mode 100755 t/t7706-repack-drop-filtered.sh

diff --git a/builtin/repack.c b/builtin/repack.c
index db504d673f..322b01cb3e 100644
--- a/builtin/repack.c
+++ b/builtin/repack.c
@@ -14,6 +14,7 @@
 #include "promisor-remote.h"
 #include "repack.h"
 #include "shallow.h"
+#include "list-objects-filter-options.h"
 
 #define ALL_INTO_ONE 1
 #define LOOSEN_UNREACHABLE 2
@@ -28,6 +29,8 @@ static int use_delta_islands;
 static int run_update_server_info = 1;
 static char *packdir, *packtmp_name, *packtmp;
 static int midx_must_contain_cruft = 1;
+static int drop_filtered;
+static int dry_run;
 
 static const char *const git_repack_usage[] = {
 	N_("git repack [-a] [-A] [-d] [-f] [-F] [-l] [-n] [-q] [-b] [-m]\n"
@@ -148,6 +151,7 @@ int cmd_repack(int argc,
 	/* variables to be filled by option parsing */
 	struct repack_config_ctx config_ctx;
 	int delete_redundant = 0;
+	int write_bitmaps_before_parse;
 	const char *unpack_unreachable = NULL;
 	int keep_unreachable = 0;
 	struct string_list keep_pack_list = STRING_LIST_INIT_NODUP;
@@ -231,6 +235,10 @@ 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_BOOL(0, "drop-filtered", &drop_filtered,
+				N_("delete filtered out objects (requires --filter)")),
+		OPT_BOOL(0, "dry-run", &dry_run,
+				N_("only show which objects would be dropped")),
 		OPT_END()
 	};
 
@@ -244,6 +252,13 @@ int cmd_repack(int argc,
 
 	repo_config(repo, repack_config, &config_ctx);
 
+	/*
+	 * update the bitmap setting after config but before command line
+	 * parsing, so we can later tell whether -b/--write-bitmap-index was
+	 * given explicitly on the command line or not
+	 */
+	write_bitmaps_before_parse = write_bitmaps;
+
 	argc = parse_options(argc, argv, prefix, builtin_repack_options,
 				git_repack_usage, 0);
 
@@ -252,6 +267,54 @@ 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_opt2(drop_filtered, "--drop-filtered",
+		write_bitmaps > 0, "--write-bitmap-index");
+
+	if (dry_run && !drop_filtered)
+		die(_("--dry-run only takes effect with --drop-filtered"));
+
+	if (drop_filtered) {
+		int bitmaps_from_cmdline = (write_bitmaps != write_bitmaps_before_parse);
+
+		if (!dry_run)
+			die(_("--drop-filtered doesn't work without --dry-run yet"));
+
+		if (!po_args.filter_options.choice)
+			die(_("--drop-filtered requires --filter"));
+
+		if (!(pack_everything & ALL_INTO_ONE))
+			die(_("--drop-filtered requires -a"));
+
+		/*
+		 * Only blob:limit=<n> is supported for now. Reject other
+		 * filter choices early, before walking the object database.
+		 */
+		if (po_args.filter_options.choice != LOFC_BLOB_LIMIT)
+			die(_("--drop-filtered only supports --filter=blob:limit=<n> for now"));
+
+		/*
+		 * an explicit -b on the command line is a conflict we have to
+		 * report, a bitmap setting from config is silently overridden
+		 * for the duration of the command
+		 */
+		if (bitmaps_from_cmdline && write_bitmaps > 0)
+			die(_("options '%s' and '%s' cannot be used together"),
+				"--drop-filtered", "--write-bitmap-index");
+
+		/*
+		 * Without a promisor remote there is nowhere to re-fetch the
+		 * dropped objects from, so dropping them would be permanent
+		 * data loss.
+		 */
+		if (!repo_has_promisor_remote(repo))
+			die(_("--drop-filtered requires a promisor remote"));
+
+		write_bitmaps = 0;
+	}
+
 	if (delete_redundant && repo->repository_format_precious_objects)
 		die(_("cannot delete packs in a precious-objects repo"));
 
diff --git a/t/meson.build b/t/meson.build
index d8161c368b..c2bf60d129 100644
--- a/t/meson.build
+++ b/t/meson.build
@@ -963,6 +963,7 @@ integration_tests = [
   't7703-repack-geometric.sh',
   't7704-repack-cruft.sh',
   't7705-repack-incremental-midx.sh',
+  't7706-repack-drop-filtered.sh',
   't7800-difftool.sh',
   't7810-grep.sh',
   't7811-grep-open.sh',
diff --git a/t/t7706-repack-drop-filtered.sh b/t/t7706-repack-drop-filtered.sh
new file mode 100755
index 0000000000..65be756e33
--- /dev/null
+++ b/t/t7706-repack-drop-filtered.sh
@@ -0,0 +1,49 @@
+#!/bin/sh
+
+test_description='git repack --drop-filtered option validation'
+
+. ./test-lib.sh
+
+# checks for options validations before any promisor walk
+test_expect_success 'setup plain repo for validation' '
+	git init plain &&
+	test_commit -C plain initial &&
+	git clone --bare plain plain.git &&
+	git -C plain.git repack -a -d
+'
+
+test_expect_success '--drop-filtered requires --filter' '
+	test_must_fail git -C plain.git repack --drop-filtered --dry-run -a 2>err &&
+	test_grep "drop-filtered requires --filter" err
+'
+
+test_expect_success '--drop-filtered cannot be used with --filter-to' '
+	test_must_fail git -C plain.git repack --drop-filtered \
+		--filter=blob:limit=1k --filter-to=./filter-out 2>err &&
+	test_grep "options .--drop-filtered. and .--filter-to. cannot be used together" err
+'
+
+test_expect_success '--dry-run only takes effect with --drop-filtered' '
+	test_must_fail git -C plain.git repack --dry-run 2>err &&
+	test_grep "dry-run only takes effect with --drop-filtered" err
+'
+
+test_expect_success '--drop-filtered requires -a' '
+	test_must_fail git -C plain.git repack --drop-filtered \
+		--filter=blob:limit=1k --dry-run 2>err &&
+	test_grep "drop-filtered requires -a" err
+'
+
+test_expect_success '--drop-filtered fails with --write-bitmap-index' '
+	test_must_fail git -C plain.git repack --drop-filtered \
+		--filter=blob:limit=1k --dry-run -a -b 2>err &&
+	test_grep "options .--drop-filtered. and .--write-bitmap-index. cannot be used together" err
+'
+
+test_expect_success '--drop-filtered fails without a promisor remote' '
+	test_must_fail git -C plain.git repack --drop-filtered \
+		--filter=blob:limit=1k --dry-run -a 2>err &&
+	test_grep "drop-filtered requires a promisor remote" err
+'
+
+test_done
-- 
2.54.0

[GSoC PATCH v2 2/7] list-objects-filter: add list_objects_filter__filter_oidset()

[Siddharth Shrimali]

The existing filter entry point, list_objects_filter__filter_object(),
is built around the object-walk path: it expects traversal context and
provisional omit sets, and is meant to be called as objects are
visited during a walk. A caller that already has a set of OIDs in hand
and only wants to know which ones a filter would select has no usable
entry point into the filter API.

--drop-filtered is exactly such a caller: it collects promisor blobs
into an oidset and needs to know which of them exceed the filter
threshold, without performing an object walk.

Add a helper, list_objects_filter__filter_oidset(), that takes a set
of OIDs and populates an "omitted" set with those that would be
filtered out by the given filter options. Only blob:limit=N filters
are supported for now.

This helper does not actually reuse the existing filter machinery.
It reimplements the blob:limit size check directly. That machinery
is tied to the object-walk path and cannot easily be driven
from a plain oidset. A NEEDSWORK comment marks this so the helper can
later be refactored to reuse the real filter logic instead of
duplicating it.

OBJECT_INFO_SKIP_FETCH_OBJECT is passed when reading object info so
the helper never triggers a lazy fetch.

Mentored-by: Christian Couder <christian.couder@gmail.com>
Mentored-by: Siddharth Asthana <siddharthasthana31@gmail.com>
Signed-off-by: Siddharth Shrimali <r.siddharth.shrimali@gmail.com>
---
 list-objects-filter.c | 45 +++++++++++++++++++++++++++++++++++++++++++
 list-objects-filter.h | 16 +++++++++++++++
 2 files changed, 61 insertions(+)

diff --git a/list-objects-filter.c b/list-objects-filter.c
index c912ff3079..6a2e9d5b24 100644
--- a/list-objects-filter.c
+++ b/list-objects-filter.c
@@ -828,3 +828,48 @@ void list_objects_filter__free(struct filter *filter)
 	filter->free_fn(filter->filter_data);
 	free(filter);
 }
+
+/*
+ * NEEDSWORK: this reimplements the blob:limit size check rather than
+ * reusing the existing filter machinery in
+ * list_objects_filter__filter_object(). That machinery is currently
+ * tied to the object-walk path and cannot easily be driven from a
+ * plain oidset. It would be nice to refactor the filter code so this
+ * helper can reuse it instead of duplicating the size check.
+ */
+int list_objects_filter__filter_oidset(struct repository *r,
+	struct list_objects_filter_options *opts,
+	const struct oidset *in,
+	struct oidset *omitted)
+{
+	struct oidset_iter iter;
+	const struct object_id *oid;
+
+	if (opts->choice != LOFC_BLOB_LIMIT)
+		return error(_("filter_oidset: only blob:limit filters are supported"));
+
+	oidset_iter_init(in, &iter);
+	while ((oid = oidset_iter_next(&iter))) {
+		struct object_info info = OBJECT_INFO_INIT;
+		enum object_type type;
+		unsigned long size;
+
+		info.typep = &type;
+		info.sizep = &size;
+
+		/*
+		 * Use OBJECT_INFO_SKIP_FETCH_OBJECT to avoid triggering
+		 * a lazy fetch while inspecting candidates for removal.
+		 */
+		if (odb_read_object_info_extended(r->objects, oid, &info,
+				OBJECT_INFO_SKIP_FETCH_OBJECT) < 0)
+			continue;
+
+		if (type != OBJ_BLOB)
+			continue;
+
+		if (size >= opts->blob_limit_value)
+			oidset_insert(omitted, oid);
+	}
+	return 0;
+}
diff --git a/list-objects-filter.h b/list-objects-filter.h
index 9e98814111..56a2d87aa0 100644
--- a/list-objects-filter.h
+++ b/list-objects-filter.h
@@ -94,4 +94,20 @@ enum list_objects_filter_result list_objects_filter__filter_object(
  */
 void list_objects_filter__free(struct filter *filter);
 
+/*
+ * Given a set of OIDs in 'in', populate 'omitted' with those that
+ * would be filtered by 'opts'. Currently only blob:limit=N is
+ * supported. Objects that cannot be read are silently skipped.
+ *
+ * NEEDSWORK: this reimplements the blob:limit size check rather than
+ * reusing the existing filter machinery. See the matching comment in
+ * list-objects-filter.c.
+ *
+ * Return 0 on success, -1 if the filter is not supported.
+ */
+int list_objects_filter__filter_oidset(struct repository *r,
+	struct list_objects_filter_options *opts,
+	const struct oidset *in,
+	struct oidset *omitted);
+
 #endif /* LIST_OBJECTS_FILTER_H */
-- 
2.54.0

[GSoC PATCH v2 3/7] repack-promisor: allow excluding objects from the rebuilt promisor pack

[Siddharth Shrimali]

Add a to_drop oidset parameter to repack_promisor_objects(). When it is
non-NULL, write_oid() omits those objects from the rebuilt promisor
pack. This is the mechanism --drop-filtered will use to remove promisor
blobs, i.e. rebuild the promisor pack without them.

All existing callers pass NULL, so behavior is unchanged.

Mentored-by: Christian Couder <christian.couder@gmail.com>
Mentored-by: Siddharth Asthana <siddharthasthana31@gmail.com>
Signed-off-by: Siddharth Shrimali <r.siddharth.shrimali@gmail.com>
---
 builtin/repack.c  |  2 +-
 repack-promisor.c | 15 ++++++++++++++-
 repack.h          |  4 +++-
 3 files changed, 18 insertions(+), 3 deletions(-)

diff --git a/builtin/repack.c b/builtin/repack.c
index 322b01cb3e..f25d189b07 100644
--- a/builtin/repack.c
+++ b/builtin/repack.c
@@ -425,7 +425,7 @@ int cmd_repack(int argc,
 		strvec_push(&cmd.args, "--delta-islands");
 
 	if (pack_everything & ALL_INTO_ONE) {
-		repack_promisor_objects(repo, &po_args, &names, packtmp);
+		repack_promisor_objects(repo, &po_args, &names, packtmp, NULL);
 
 		if (existing_packs_has_non_kept(&existing) &&
 		    delete_redundant &&
diff --git a/repack-promisor.c b/repack-promisor.c
index 90318ce150..fabfdc168a 100644
--- a/repack-promisor.c
+++ b/repack-promisor.c
@@ -6,10 +6,12 @@
 #include "path.h"
 #include "repository.h"
 #include "run-command.h"
+#include "oidset.h"
 
 struct write_oid_context {
 	struct child_process *cmd;
 	const struct git_hash_algo *algop;
+	const struct oidset *to_drop;
 };
 
 /*
@@ -23,6 +25,15 @@ static int write_oid(const struct object_id *oid,
 	struct write_oid_context *ctx = data;
 	struct child_process *cmd = ctx->cmd;
 
+	/*
+	 * Objects in to_drop are being removed from the repository, so
+	 * omit them from the rebuilt promisor pack. Each such object is a
+	 * promisor object and therefore remains recoverable from the
+	 * promisor remote.
+	 */
+	if (ctx->to_drop && oidset_contains(ctx->to_drop, oid))
+		return 0;
+
 	if (cmd->in == -1) {
 		if (start_command(cmd))
 			die(_("could not start pack-objects to repack promisor objects"));
@@ -81,7 +92,8 @@ static void finish_repacking_promisor_objects(struct repository *repo,
 
 void repack_promisor_objects(struct repository *repo,
 			     const struct pack_objects_args *args,
-			     struct string_list *names, const char *packtmp)
+			     struct string_list *names, const char *packtmp,
+			     const struct oidset *to_drop)
 {
 	struct write_oid_context ctx;
 	struct child_process cmd = CHILD_PROCESS_INIT;
@@ -98,6 +110,7 @@ void repack_promisor_objects(struct repository *repo,
 	 */
 	ctx.cmd = &cmd;
 	ctx.algop = repo->hash_algo;
+	ctx.to_drop = to_drop;
 	odb_for_each_object(repo->objects, NULL, write_oid, &ctx,
 			    ODB_FOR_EACH_OBJECT_PROMISOR_ONLY);
 
diff --git a/repack.h b/repack.h
index f9fbc895f0..a5a3f7c6ba 100644
--- a/repack.h
+++ b/repack.h
@@ -3,6 +3,7 @@
 
 #include "list-objects-filter-options.h"
 #include "string-list.h"
+#include "oidset.h"
 
 struct pack_objects_args {
 	char *window;
@@ -100,7 +101,8 @@ void generated_pack_install(struct generated_pack *pack, const char *name,
 
 void repack_promisor_objects(struct repository *repo,
 			     const struct pack_objects_args *args,
-			     struct string_list *names, const char *packtmp);
+			     struct string_list *names, const char *packtmp,
+			     const struct oidset *to_drop);
 
 struct pack_geometry {
 	struct packed_git **pack;
-- 
2.54.0

[GSoC PATCH v2 4/7] builtin/repack: enumerate promisor blobs for --drop-filtered

[Siddharth Shrimali]

Add enumeration logic for --drop-filtered. In --dry-run mode, print
the OIDs of locally-held promisor blobs that exceed the filter
threshold, as candidates for removal.

Reading from write_filtered_pack() cannot work for partial clones.
git repack routes promisor objects through a separate path:
repack_promisor_objects() repacks them first, and the main
pack-objects run uses --exclude-promisor-objects. By the time
write_filtered_pack() runs, the promisor blobs are already consumed by
the main pack. The filtered pack is always empty on a partial clone.

Instead, walk promisor objects directly via odb_for_each_object() with
ODB_FOR_EACH_OBJECT_PROMISOR_ONLY, collecting all promisor blobs into
an oidset. The blobs exceeding the filter threshold are then selected
using list_objects_filter__filter_oidset().

Every object enumerated this way is a promisor object by construction,
so it is guaranteed to be recoverable from the promisor remote and is
safe to drop. No separate is_promisor_object() check is needed.

OBJECT_INFO_SKIP_FETCH_OBJECT is passed to every object info query so
enumeration never triggers a lazy fetch.

The enumeration collects candidates into a caller-provided oidset and
--dry-run prints them. Actually removing the objects, together with the
required promisor-remote verification, is written in a later commit.

Mentored-by: Christian Couder <christian.couder@gmail.com>
Mentored-by: Siddharth Asthana <siddharthasthana31@gmail.com>
Signed-off-by: Siddharth Shrimali <r.siddharth.shrimali@gmail.com>
---
 builtin/repack.c                | 20 +++++++-
 repack-filtered.c               | 80 +++++++++++++++++++++++++++++++
 repack.h                        |  4 ++
 t/t7706-repack-drop-filtered.sh | 84 ++++++++++++++++++++++++++++++++-
 4 files changed, 186 insertions(+), 2 deletions(-)

diff --git a/builtin/repack.c b/builtin/repack.c
index f25d189b07..8cb92d1a62 100644
--- a/builtin/repack.c
+++ b/builtin/repack.c
@@ -15,6 +15,8 @@
 #include "repack.h"
 #include "shallow.h"
 #include "list-objects-filter-options.h"
+#include "oidset.h"
+#include "hex.h"
 
 #define ALL_INTO_ONE 1
 #define LOOSEN_UNREACHABLE 2
@@ -143,6 +145,7 @@ int cmd_repack(int argc,
 	struct string_list_item *item;
 	struct string_list names = STRING_LIST_INIT_DUP;
 	struct existing_packs existing = EXISTING_PACKS_INIT;
+	struct oidset drop_oids = OIDSET_INIT;
 	struct pack_geometry geometry = { 0 };
 	struct tempfile *refs_snapshot = NULL;
 	int i, ret;
@@ -313,6 +316,20 @@ int cmd_repack(int argc,
 			die(_("--drop-filtered requires a promisor remote"));
 
 		write_bitmaps = 0;
+
+		ret = enumerate_promisor_blobs(repo, &po_args.filter_options, &drop_oids);
+
+		if (ret)
+			goto cleanup;
+
+		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));
+		}
 	}
 
 	if (delete_redundant && repo->repository_format_precious_objects)
@@ -608,7 +625,7 @@ int cmd_repack(int argc,
 		}
 	}
 
-	if (po_args.filter_options.choice) {
+	if (po_args.filter_options.choice && !drop_filtered) {
 		struct write_pack_opts opts = {
 			.po_args = &po_args,
 			.destination = filter_to,
@@ -701,6 +718,7 @@ int cmd_repack(int argc,
 cleanup:
 	string_list_clear(&keep_pack_list, 0);
 	string_list_clear(&names, 1);
+	oidset_clear(&drop_oids);
 	existing_packs_release(&existing);
 	pack_geometry_release(&geometry);
 	pack_objects_args_release(&po_args);
diff --git a/repack-filtered.c b/repack-filtered.c
index edcf7667c5..217fc54d7b 100644
--- a/repack-filtered.c
+++ b/repack-filtered.c
@@ -3,6 +3,12 @@
 #include "repository.h"
 #include "run-command.h"
 #include "string-list.h"
+#include "hex.h"
+#include "packfile.h"
+#include "list-objects-filter-options.h"
+#include "list-objects-filter.h"
+#include "odb.h"
+#include "promisor-remote.h"
 
 int write_filtered_pack(const struct write_pack_opts *opts,
 			struct existing_packs *existing,
@@ -49,3 +55,77 @@ int write_filtered_pack(const struct write_pack_opts *opts,
 	return finish_pack_objects_cmd(existing->repo->hash_algo, opts, &cmd,
 				       names);
 }
+
+struct collect_cb_data {
+	struct repository *repo;
+	struct oidset *set;
+};
+
+static int collect_promisor_blob(const struct object_id *oid,
+				 struct object_info *oi UNUSED,
+				 void *cb_data)
+{
+	struct collect_cb_data *data = cb_data;
+	struct object_info info = OBJECT_INFO_INIT;
+	enum object_type type;
+
+	info.typep = &type;
+
+	/*
+	 * Use OBJECT_INFO_SKIP_FETCH_OBJECT to avoid triggering a
+	 * lazy fetch while collecting promisor blobs.
+	 */
+	if (odb_read_object_info_extended(data->repo->objects, oid, &info,
+			OBJECT_INFO_SKIP_FETCH_OBJECT) < 0)
+		return 0;
+
+	if (type == OBJ_BLOB)
+		oidset_insert(data->set, oid);
+
+	return 0;
+}
+
+int enumerate_promisor_blobs(struct repository *repo,
+			     const struct list_objects_filter_options *filter,
+			     struct oidset *to_drop)
+{
+	struct oidset all_promisor_blobs = OIDSET_INIT;
+	struct collect_cb_data cb = {
+		.repo = repo,
+		.set = &all_promisor_blobs
+	};
+	int ret = 0;
+
+	/*
+	 * The caller (cmd_repack) is responsible for validating that a
+	 * blob:limit filter and a promisor remote are present before
+	 * calling this function.
+	 *
+	 * Walk only promisor objects. Every object visited here is
+	 * guaranteed to be recoverable from the promisor remote, so
+	 * it is safe to drop.
+	 *
+	 * We do not use write_filtered_pack() here because git repack
+	 * routes promisor objects through repack_promisor_objects()
+	 * before the filter machinery runs, so the filtered pack never
+	 * contains promisor blobs. Direct enumeration via
+	 * ODB_FOR_EACH_OBJECT_PROMISOR_ONLY is the correct approach.
+	 */
+	ret = odb_for_each_object(repo->objects, NULL,
+			collect_promisor_blob, &cb,
+			ODB_FOR_EACH_OBJECT_PROMISOR_ONLY);
+	if (ret)
+		goto cleanup;
+
+	/*
+	 * Apply the filter to find which blobs exceed the threshold.
+	 */
+	ret = list_objects_filter__filter_oidset(repo,
+		(struct list_objects_filter_options *)filter,
+		&all_promisor_blobs,
+		to_drop);
+
+cleanup:
+	oidset_clear(&all_promisor_blobs);
+	return ret;
+}
diff --git a/repack.h b/repack.h
index a5a3f7c6ba..61e554e4ed 100644
--- a/repack.h
+++ b/repack.h
@@ -167,6 +167,10 @@ int write_filtered_pack(const struct write_pack_opts *opts,
 			struct existing_packs *existing,
 			struct string_list *names);
 
+int enumerate_promisor_blobs(struct repository *repo,
+			     const struct list_objects_filter_options *filter,
+			     struct oidset *to_drop);
+
 int write_cruft_pack(const struct write_pack_opts *opts,
 		     const char *cruft_expiration,
 		     unsigned long combine_cruft_below_size,
diff --git a/t/t7706-repack-drop-filtered.sh b/t/t7706-repack-drop-filtered.sh
index 65be756e33..cbdb580702 100755
--- a/t/t7706-repack-drop-filtered.sh
+++ b/t/t7706-repack-drop-filtered.sh
@@ -1,9 +1,36 @@
 #!/bin/sh
 
-test_description='git repack --drop-filtered option validation'
+test_description='git repack --drop-filtered enumerates filtered promisor blobs'
 
 . ./test-lib.sh
 
+delete_object () {
+	local repo="$1" &&
+	local obj="$2" &&
+	local path="$repo/.git/objects/$(test_oid_to_path "$obj")" &&
+	rm "$path"
+}
+
+# pack the objects into a promisor pack inside "repo". it is a pack
+# accompanied by an empty ".promisor" marker file. objects
+# in such a pack are treated as recoverable from the promisor remote.
+pack_as_from_promisor () {
+	HASH=$(git -C repo pack-objects .git/objects/pack/pack) &&
+	>repo/.git/objects/pack/pack-$HASH.promisor &&
+	echo $HASH
+}
+
+# write a blob of $1 bytes into "repo", record it as coming from the
+# promisor remote, and remove the loose copy so the object is only
+# present in the promisor pack
+promisor_blob () {
+	test-tool genrandom "$1" "$2" >blob_content &&
+	OID=$(git -C repo hash-object -w --stdin <blob_content) &&
+	printf "%s\n" "$OID" | pack_as_from_promisor >/dev/null &&
+	delete_object repo "$OID" &&
+	echo "$OID"
+}
+
 # checks for options validations before any promisor walk
 test_expect_success 'setup plain repo for validation' '
 	git init plain &&
@@ -46,4 +73,59 @@ test_expect_success '--drop-filtered fails without a promisor remote' '
 	test_grep "drop-filtered requires a promisor remote" err
 '
 
+# enumeration tests using promisor pack
+test_expect_success 'setup repo with a promisor remote' '
+	rm -rf repo &&
+	test_create_repo repo &&
+	test_commit -C repo base &&
+
+	# mark the repo as a partial clone with a promisor remote so the
+	# promisor walk and the safety guard are satisfied
+	git -C repo config core.repositoryformatversion 1 &&
+	git -C repo config extensions.partialclone origin &&
+	git -C repo config remote.origin.promisor true &&
+	git -C repo config remote.origin.url "." &&
+
+	BIG=$(promisor_blob big 3072) &&
+	SMALL=$(promisor_blob small 512) &&
+	echo "$BIG" >big_oid &&
+	echo "$SMALL" >small_oid
+'
+
+test_expect_success 'promisor blob over the threshold is listed' '
+	BIG=$(cat big_oid) &&
+	SMALL=$(cat small_oid) &&
+
+	git -C repo -c repack.writeBitmaps=false \
+		repack --drop-filtered --filter=blob:limit=1k --dry-run -a >out &&
+
+	test_grep "$BIG" out &&
+	test_grep ! "$SMALL" out
+'
+
+test_expect_success 'locally created blob is never listed' '
+	BIG=$(cat big_oid) &&
+
+	# large blob that exists only locally must never be a drop candidate.
+	# dropping it would be unrecoverable
+	test-tool genrandom local 4096 >local_content &&
+	LOCAL=$(git -C repo hash-object -w --stdin <local_content) &&
+
+	git -C repo -c repack.writeBitmaps=false \
+		repack --drop-filtered --filter=blob:limit=1k --dry-run -a >out &&
+
+	test_grep "$BIG" out &&
+	test_grep ! "$LOCAL" out
+'
+
+test_expect_success '--dry-run does not remove the filtered objects' '
+	BIG=$(cat big_oid) &&
+
+	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_done
-- 
2.54.0

[GSoC PATCH v2 5/7] builtin/repack: actually drop filtered promisor blobs

[Siddharth Shrimali]

Make --drop-filtered remove the enumerated promisor blobs instead of
only listing them.

The drop set is computed before repack_promisor_objects() runs, and on
a real run it is passed in so the rebuilt promisor pack omits those
blobs. --drop-filtered implies -d so the old promisor packs, which
still contain the dropped blobs, are removed. Without this the blobs
would survive in the redundant packs. The existing repack machinery
performs the write-before-delete and fsync, so the drop is crash-safe.

The dropped blobs become absent locally but remain recoverable from the
promisor remote, so a later access lazy-fetches them back
transparently. --dry-run keeps its previous behavior, i.e. it lists the
candidates and changes nothing.

Mentored-by: Christian Couder <christian.couder@gmail.com>
Mentored-by: Siddharth Asthana <siddharthasthana31@gmail.com>
Signed-off-by: Siddharth Shrimali <r.siddharth.shrimali@gmail.com>
---
 builtin/repack.c                | 17 ++++++++++-------
 repack-filtered.c               |  1 +
 t/t7706-repack-drop-filtered.sh | 12 ++++++++++++
 3 files changed, 23 insertions(+), 7 deletions(-)

diff --git a/builtin/repack.c b/builtin/repack.c
index 8cb92d1a62..9a15ab1f2a 100644
--- a/builtin/repack.c
+++ b/builtin/repack.c
@@ -273,18 +273,12 @@ int cmd_repack(int argc,
 	die_for_incompatible_opt2(drop_filtered, "--drop-filtered",
 		!!filter_to, "--filter-to");
 
-	die_for_incompatible_opt2(drop_filtered, "--drop-filtered",
-		write_bitmaps > 0, "--write-bitmap-index");
-
 	if (dry_run && !drop_filtered)
 		die(_("--dry-run only takes effect with --drop-filtered"));
 
 	if (drop_filtered) {
 		int bitmaps_from_cmdline = (write_bitmaps != write_bitmaps_before_parse);
 
-		if (!dry_run)
-			die(_("--drop-filtered doesn't work without --dry-run yet"));
-
 		if (!po_args.filter_options.choice)
 			die(_("--drop-filtered requires --filter"));
 
@@ -317,6 +311,14 @@ int cmd_repack(int argc,
 
 		write_bitmaps = 0;
 
+		/*
+		 * Dropping objects means rebuilding the promisor packs
+		 * without them and then removing the old packs, so the
+		 * redundant packs must be deleted. Imply -d on a real run.
+		 */
+		if (!dry_run)
+			delete_redundant = 1;
+
 		ret = enumerate_promisor_blobs(repo, &po_args.filter_options, &drop_oids);
 
 		if (ret)
@@ -442,7 +444,8 @@ int cmd_repack(int argc,
 		strvec_push(&cmd.args, "--delta-islands");
 
 	if (pack_everything & ALL_INTO_ONE) {
-		repack_promisor_objects(repo, &po_args, &names, packtmp, NULL);
+		repack_promisor_objects(repo, &po_args, &names, packtmp,
+			(drop_filtered && !dry_run) ? &drop_oids : NULL);
 
 		if (existing_packs_has_non_kept(&existing) &&
 		    delete_redundant &&
diff --git a/repack-filtered.c b/repack-filtered.c
index 217fc54d7b..08796818d8 100644
--- a/repack-filtered.c
+++ b/repack-filtered.c
@@ -119,6 +119,7 @@ int enumerate_promisor_blobs(struct repository *repo,
 
 	/*
 	 * Apply the filter to find which blobs exceed the threshold.
+	 * The caller has to_drop and is responsible for clearing it.
 	 */
 	ret = list_objects_filter__filter_oidset(repo,
 		(struct list_objects_filter_options *)filter,
diff --git a/t/t7706-repack-drop-filtered.sh b/t/t7706-repack-drop-filtered.sh
index cbdb580702..b3e493e851 100755
--- a/t/t7706-repack-drop-filtered.sh
+++ b/t/t7706-repack-drop-filtered.sh
@@ -128,4 +128,16 @@ test_expect_success '--dry-run does not remove the filtered objects' '
 	git -C repo cat-file -e "$BIG"
 '
 
+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 &&
+
+	git -C repo cat-file --batch-all-objects --batch-check="%(objectname)" >present &&
+	! grep -q "$BIG" present &&
+	grep -q "$SMALL" present
+'
+
 test_done
-- 
2.54.0

[GSoC PATCH v2 6/7] builtin/repack: add safety guards for --drop-filtered

[Siddharth Shrimali]

--drop-filtered removes local promisor blobs. That is only safe when the
repository is not mid-operation and when the blobs are not actively in
use, so add two guards, both skipped for bare repositories which have
neither a worktree nor an index.

First, refuse to run while a merge, rebase, am, cherry-pick, revert, or
bisect is in progress. During these operations the working tree and
index are in an intermediate state, and rewriting packs and deleting
objects underneath a half-finished operation is unsafe.

Second, refuse to drop a blob that the current index references. Such a
blob is needed by the working tree, so dropping it would only cause the
next command that touches the worktree to lazy-fetch it straight back,
reclaiming nothing. The offending path is reported so the user can see
why the drop was refused.

Mentored-by: Christian Couder <christian.couder@gmail.com>
Mentored-by: Siddharth Asthana <siddharthasthana31@gmail.com>
Signed-off-by: Siddharth Shrimali <r.siddharth.shrimali@gmail.com>
---
 builtin/repack.c                | 47 +++++++++++++++++++++++++++++++++
 t/t7706-repack-drop-filtered.sh | 36 +++++++++++++++++++++++++
 2 files changed, 83 insertions(+)

diff --git a/builtin/repack.c b/builtin/repack.c
index 9a15ab1f2a..2339bcaac4 100644
--- a/builtin/repack.c
+++ b/builtin/repack.c
@@ -17,6 +17,8 @@
 #include "list-objects-filter-options.h"
 #include "oidset.h"
 #include "hex.h"
+#include "wt-status.h"
+#include "read-cache-ll.h"
 
 #define ALL_INTO_ONE 1
 #define LOOSEN_UNREACHABLE 2
@@ -309,6 +311,28 @@ int cmd_repack(int argc,
 		if (!repo_has_promisor_remote(repo))
 			die(_("--drop-filtered requires a promisor remote"));
 
+		/*
+		 * refuse to drop objects while another operation is in
+		 * progress. the working tree and index are in an
+		 * intermediate state, and rewriting packs in a half-finished
+		 * merge/rebase/cherry-pick/revert/bisect is unsafe
+		 * bare repositories have no such state, so the check
+		 * is skipped there
+		 */
+		if (!is_bare_repository(repo)) {
+			struct wt_status_state state = { 0 };
+
+			wt_status_get_state(repo, &state, 0);
+			if (state.merge_in_progress || state.revert_in_progress ||
+			    state.rebase_in_progress ||state.bisect_in_progress ||
+			    state.cherry_pick_in_progress ||state.am_in_progress||
+			    state.rebase_interactive_in_progress) {
+				wt_status_state_free_buffers(&state);
+				die(_("--drop-filtered cannot be used while another operation is in progress"));
+			}
+			wt_status_state_free_buffers(&state);
+		}
+
 		write_bitmaps = 0;
 
 		/*
@@ -324,6 +348,29 @@ int cmd_repack(int argc,
 		if (ret)
 			goto cleanup;
 
+		/*
+		 * refuse to drop blobs that the current index references.
+		 * dropping such a blob would cause the very next command
+		 * that touches the worktree to lazy-fetch it straight back, so
+		 * the drop would reclaim nothing. bare repositories have no
+		 * index, so the check is skipped there.
+		 */
+		if (!is_bare_repository(repo) && oidset_size(&drop_oids)) {
+			struct index_state *istate = repo->index;
+			unsigned int i;
+
+			if (repo_read_index(repo) < 0)
+				die(_("could not read the index"));
+
+			for (i = 0; i < istate->cache_nr; i++) {
+				const struct cache_entry *ce = istate->cache[i];
+
+				if (oidset_contains(&drop_oids, &ce->oid))
+					die(_("cannot drop '%s' (%s): it is referenced by the current index"),
+						ce->name, oid_to_hex(&ce->oid));
+			}
+		}
+
 		if (dry_run) {
 			struct oidset_iter iter;
 			const struct object_id *oid;
diff --git a/t/t7706-repack-drop-filtered.sh b/t/t7706-repack-drop-filtered.sh
index b3e493e851..dabed97541 100755
--- a/t/t7706-repack-drop-filtered.sh
+++ b/t/t7706-repack-drop-filtered.sh
@@ -140,4 +140,40 @@ test_expect_success '--drop-filtered removes the promisor blob locally' '
 	grep -q "$SMALL" present
 '
 
+test_expect_success '--drop-filtered refuses when a merge is in progress' '
+	test_when_finished "git -C repo merge --abort || :" &&
+
+	# creat a conflicting merge so wt_status reports it
+	git -C repo checkout -B mergebase base &&
+	echo one >repo/conflict.txt &&
+	git -C repo add conflict.txt &&
+	git -C repo commit -m one &&
+
+	git -C repo checkout -B mergeother base &&
+	echo two >repo/conflict.txt &&
+	git -C repo add conflict.txt &&
+	git -C repo commit -m two &&
+
+	test_must_fail git -C repo merge mergebase &&
+
+	test_must_fail git -C repo -c repack.writeBitmaps=false \
+		repack --drop-filtered --filter=blob:limit=1k --dry-run -a 2>err &&
+	test_grep "in progress" err
+'
+
+
+test_expect_success '--drop-filtered refuses to drop an index-referenced blob' '
+	# create a large blob, add it to the index and make it a promisor object
+	# so the index references it and enumeration picks it up
+	test-tool genrandom idx 4096 >repo/tracked-big.bin &&
+	git -C repo add tracked-big.bin &&
+	OID=$(git -C repo rev-parse :tracked-big.bin) &&
+	printf "%s\n" "$OID" | pack_as_from_promisor >/dev/null &&
+	delete_object repo "$OID" &&
+
+	test_must_fail git -C repo -c repack.writeBitmaps=false \
+		repack --drop-filtered --filter=blob:limit=1k --dry-run -a 2>err &&
+	test_grep "referenced by the current index" err
+'
+
 test_done
-- 
2.54.0

[GSoC PATCH v2 7/7] Documentation/git-repack: document --drop-filtered and --dry-run

[Siddharth Shrimali]

Describe the new --drop-filtered and --dry-run options: what they do,
only blob:limit filters are supported for now, a promisor remote is
required, --drop-filtered requires -a and implies -d so the redundant
packs are actually removed, its incompatibilities with --filter-to and
bitmap writing, and the safety guards that refuse to run mid-operation
or to drop index-referenced blobs.

Mentored-by: Christian Couder <christian.couder@gmail.com>
Mentored-by: Siddharth Asthana <siddharthasthana31@gmail.com>
Signed-off-by: Siddharth Shrimali <r.siddharth.shrimali@gmail.com>
---
 Documentation/git-repack.adoc | 35 +++++++++++++++++++++++++++++++++++
 1 file changed, 35 insertions(+)

diff --git a/Documentation/git-repack.adoc b/Documentation/git-repack.adoc
index 72c42015e2..9efff838f2 100644
--- a/Documentation/git-repack.adoc
+++ b/Documentation/git-repack.adoc
@@ -12,6 +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]]]
 
 DESCRIPTION
 -----------
@@ -182,6 +183,40 @@ depth is 4095.
 	`objects` and `objects/info/alternates` sections of
 	linkgit:gitrepository-layout[5].
 
+--drop-filtered::
+	Delete the local objects that match the `--filter` specification
+	instead of keeping them in a separate packfile, reclaiming the
+	disk space they occupy. This is intended for partial clones,
+	where the filtered objects are promisor objects that remain
+	recoverable from the promisor remote and are lazily re-fetched
+	on demand when they are next needed.
++
+Only large blobs are supported for now, so `--filter=blob:limit=<n>`
+is currently the only accepted filter. Because dropped objects must be
+recoverable, this option requires a promisor remote to be configured
+and refuses to run otherwise.
++
+This option requires `-a`, and implies `-d`: the objects are dropped by
+rebuilding the promisor pack without them and then removing the now
+redundant old packs, so the redundant packs must be deleted for the
+space to actually be reclaimed. It is incompatible with `--filter-to`
+and with bitmap writing (`-b`/`--write-bitmap-index`), since filtering
+breaks the single-pack closure that bitmaps require. A bitmap setting
+coming from configuration is silently disabled for the duration of the
+command.
++
+As a safety measure, `--drop-filtered` refuses to run while another
+operation (merge, rebase, am, cherry-pick, revert, or bisect) is in
+progress, and refuses to drop any blob that the current index
+references, since such a blob would only be lazily re-fetched by the
+next command that inspects the working tree. These checks are skipped
+in bare repositories, which have neither a working tree nor an index.
+
+--dry-run::
+	Only meaningful with `--drop-filtered`. List the objects that
+	would be dropped, one object ID per line, without rebuilding any
+	pack or deleting anything.
+
 -b::
 --write-bitmap-index::
 	Write a reachability bitmap index as part of the repack. This
-- 
2.54.0

Re: [GSoC PATCH v2 0/7] repack: add --drop-filtered to reclaim space in partial clones

[Junio C Hamano]

Siddharth Shrimali <r.siddharth.shrimali@gmail.com> writes:

> How it works:
>   * Enumerate promisor objects directly (ODB_FOR_EACH_OBJECT_PROMISOR_ONLY)
>     and select the blobs exceeding the filter threshold. Every enumerated
>     object is a promisor object by construction, so it is guaranteed
>     recoverable and locally-created objects are never candidates.

By 'by construction', do you mean 'It is guaranteed recoverable, as
long as ODB_FOR_EACH_OBJECT_PROMISOR_ONLY is working correctly'?
Since I do not use it, I do not personally trust promisor-based
traversal all that much, and it would be great if we could hear from
other practitioners that this really works well.

>   * Rebuild the promisor pack without the selected blobs, reusing the
>     existing repack machinery, so the drop is crash-safe (write, fsync,
>     install, then delete the old pack).

This is sensible, as long as this repacking is done only with
locally available data, without dynamically pulling in lazy objects
from the promisor (which would defeat the whole point ;-)).
Presumably, this rebuilding is done without an extra traversal,
driven instead by the list of enumerated promisor objects we
constructed above (excluding the unwanted ones)?

>   * --dry-run lists the candidates and changes nothing.

I wonder whether size is the only criterion we would want to use
when choosing what to discard among objects we know the promisor can
give us on-demand.  It is, of course, perfectly fine to make it the
only condition in this first effort, but it would help to imagine
what other criteria we might want in the future and how they would
fit into the framework you establish with this series.  Ensuring
that the framework is easily extensible with a future set of rules
will keep us from painting ourselves into a corner.

> Safety guards refuse to run while a merge, rebase, am, cherry-pick,
> revert, or bisect is in progress, and refuse to drop a blob referenced
> by the current index (it would only be lazily re-fetched by the next
> worktree command). Both are skipped for bare repositories.

I assume you do not mean a race where an operation wants to write a
blob, finds that an identical one that came from the promisor remote
already exists locally, refrains from writing another copy, and the
drop-filtered operation removes the blob at the right moment.
Rather, you likely have in mind an operation that stops, gives
control back to the user, and, while the user ponders the situation,
the drop-filtered operation kicks in and removes the blobs involved
in the operation in progress.  Am I reading you correctly?

Even in either of these situations, I do not quite see why the
safeguards are necessary.  The operation completes, or stays stopped
in the middle.  The user's next move (whether they issue a new
command after completion or resume the interrupted operation) will
automatically lazy-refetch what the drop-filtered operation
discarded as needed, will it not?