All of lore.kernel.org
 help / color / mirror / Atom feed
From: Siddharth Shrimali <r.siddharth.shrimali@gmail.com>
To: git@vger.kernel.org
Cc: gitster@pobox.com, christian.couder@gmail.com,
	siddharthasthana31@gmail.com, ttaylorr@openai.com, ps@pks.im,
	johannes.schindelin@gmx.de, l.s.r@web.de,
	r.siddharth.shrimali@gmail.com
Subject: [GSoC PATCH v5 4/6] builtin/repack: enumerate promisor blobs for --drop-filtered
Date: Fri, 14 Aug 2026 01:38:28 +0530	[thread overview]
Message-ID: <20260813200830.84348-5-r.siddharth.shrimali@gmail.com> (raw)
In-Reply-To: <20260813200830.84348-1-r.siddharth.shrimali@gmail.com>

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, so it is
recoverable from the promisor remote in the same sense as the rest of a
partial clone, as long as the remote still has it. This holds without a
separate is_promisor_object() check. A future implementation can verify
availability against the remote directly once a client-side
remote-object-info query exists.

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               | 82 +++++++++++++++++++++++++++++++
 repack.h                        |  4 ++
 t/t7706-repack-drop-filtered.sh | 85 ++++++++++++++++++++++++++++++++-
 4 files changed, 189 insertions(+), 2 deletions(-)

diff --git a/builtin/repack.c b/builtin/repack.c
index 2ad6358535..3633b17ce8 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
@@ -160,6 +162,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;
@@ -318,6 +321,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)
@@ -613,7 +630,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,
@@ -706,6 +723,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..869b9fc6e3 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,79 @@ 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 a
+	 * promisor object, so it is recoverable from the promisor remote
+	 * as long as the remote still has it, the same assumption the rest
+	 * of partial clone relies on.
+	 *
+	 * 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.
+	 * The caller has to_drop and is responsible for clearing it.
+	 */
+	ret = list_objects_filter__filter_oidset(repo,
+		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 07a976874a..6352f1fdce 100755
--- a/t/t7706-repack-drop-filtered.sh
+++ b/t/t7706-repack-drop-filtered.sh
@@ -1,9 +1,37 @@
 #!/bin/sh
 
-test_description='git repack --drop-filtered option validation'
+test_description='git repack --drop-filtered enumerates filtered promisor blobs'
 
 . ./test-lib.sh
 
+# Delete a loose or packed object from "repo".
+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"
+}
+
 # Check option validation before any promisor walk
 test_expect_success 'setup plain repo for validation' '
 	git init plain &&
@@ -52,4 +80,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


  parent reply	other threads:[~2026-08-13 20:09 UTC|newest]

Thread overview: 74+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-07-16 13:28 [RFC PATCH 0/7] repack: add --drop-filtered to reclaim space in partial clones Siddharth Shrimali
2026-07-16 13:28 ` [RFC PATCH 1/7] builtin/repack.c: add --drop-filtered and --dry-run options Siddharth Shrimali
2026-07-16 21:08   ` Junio C Hamano
2026-07-17 18:00     ` Siddharth Shrimali
2026-07-23 19:31     ` Siddharth Asthana
2026-07-18 12:30   ` Christian Couder
2026-07-20 10:19     ` Siddharth Shrimali
2026-07-16 13:28 ` [RFC PATCH 2/7] list-objects-filter: add list_objects_filter__filter_oidset() Siddharth Shrimali
2026-07-16 13:28 ` [RFC PATCH 3/7] repack-promisor: allow excluding objects from the rebuilt promisor pack Siddharth Shrimali
2026-07-16 13:28 ` [RFC PATCH 4/7] builtin/repack: enumerate promisor blobs for --drop-filtered Siddharth Shrimali
2026-07-16 13:28 ` [RFC PATCH 5/7] t7706: test --drop-filtered enumeration and validation Siddharth Shrimali
2026-07-16 13:28 ` [RFC PATCH 6/7] builtin/repack: actually drop filtered promisor blobs Siddharth Shrimali
2026-07-23 19:42   ` Siddharth Asthana
2026-07-25 19:30     ` Siddharth Shrimali
2026-07-16 13:28 ` [RFC PATCH 7/7] repack-promisor: record dropped objects in a drop log Siddharth Shrimali
2026-07-23 19:41   ` Siddharth Asthana
2026-07-25 19:35     ` Siddharth Shrimali
2026-07-23 19:26 ` [RFC PATCH 0/7] repack: add --drop-filtered to reclaim space in partial clones Siddharth Asthana
2026-07-25 19:23   ` Siddharth Shrimali
2026-07-30 17:41 ` [GSoC PATCH v2 " Siddharth Shrimali
2026-07-30 17:41   ` [GSoC PATCH v2 1/7] builtin/repack.c: add --drop-filtered and --dry-run options Siddharth Shrimali
2026-08-04 21:13     ` Siddharth Asthana
2026-07-30 17:41   ` [GSoC PATCH v2 2/7] list-objects-filter: add list_objects_filter__filter_oidset() Siddharth Shrimali
2026-07-30 17:41   ` [GSoC PATCH v2 3/7] repack-promisor: allow excluding objects from the rebuilt promisor pack Siddharth Shrimali
2026-07-30 17:41   ` [GSoC PATCH v2 4/7] builtin/repack: enumerate promisor blobs for --drop-filtered Siddharth Shrimali
2026-07-30 17:41   ` [GSoC PATCH v2 5/7] builtin/repack: actually drop filtered promisor blobs Siddharth Shrimali
2026-07-30 17:41   ` [GSoC PATCH v2 6/7] builtin/repack: add safety guards for --drop-filtered Siddharth Shrimali
2026-08-04 21:13     ` Siddharth Asthana
2026-07-30 17:41   ` [GSoC PATCH v2 7/7] Documentation/git-repack: document --drop-filtered and --dry-run Siddharth Shrimali
2026-07-31 15:33   ` [GSoC PATCH v2 0/7] repack: add --drop-filtered to reclaim space in partial clones Junio C Hamano
2026-08-01 18:19     ` Siddharth Shrimali
2026-08-02  2:18       ` Junio C Hamano
2026-08-02 11:28         ` Siddharth Shrimali
2026-08-04 21:12   ` Siddharth Asthana
2026-08-06 11:21   ` [GSoC PATCH v3 " Siddharth Shrimali
2026-08-06 11:21     ` [GSoC PATCH v3 1/7] builtin/repack.c: add --drop-filtered and --dry-run options Siddharth Shrimali
2026-08-06 11:21     ` [GSoC PATCH v3 2/7] list-objects-filter: add list_objects_filter__filter_oidset() Siddharth Shrimali
2026-08-06 11:21     ` [GSoC PATCH v3 3/7] repack-promisor: allow excluding objects from the rebuilt promisor pack Siddharth Shrimali
2026-08-06 11:21     ` [GSoC PATCH v3 4/7] builtin/repack: enumerate promisor blobs for --drop-filtered Siddharth Shrimali
2026-08-06 11:22     ` [GSoC PATCH v3 5/7] builtin/repack: actually drop filtered promisor blobs Siddharth Shrimali
2026-08-06 11:22     ` [GSoC PATCH v3 6/7] builtin/repack: add guards for --drop-filtered Siddharth Shrimali
2026-08-06 11:22     ` [GSoC PATCH v3 7/7] Documentation/git-repack: document --drop-filtered and --dry-run Siddharth Shrimali
2026-08-06 21:34     ` [GSoC PATCH v3 0/7] repack: add --drop-filtered to reclaim space in partial clones Junio C Hamano
2026-08-06 22:19     ` Junio C Hamano
2026-08-07  9:06       ` Siddharth Shrimali
2026-08-07 20:52         ` Junio C Hamano
2026-08-08 20:07           ` Siddharth Shrimali
2026-08-10 17:40     ` [GSoC PATCH v4 " Siddharth Shrimali
2026-08-10 17:40       ` [GSoC PATCH v4 1/7] builtin/repack.c: add --drop-filtered and --dry-run options Siddharth Shrimali
2026-08-10 17:40       ` [GSoC PATCH v4 2/7] list-objects-filter: add list_objects_filter__filter_oidset() Siddharth Shrimali
2026-08-12 17:09         ` Christian Couder
2026-08-12 18:04         ` Junio C Hamano
2026-08-12 19:22           ` Siddharth Shrimali
2026-08-10 17:40       ` [GSoC PATCH v4 3/7] repack-promisor: allow excluding objects from the rebuilt promisor pack Siddharth Shrimali
2026-08-10 17:40       ` [GSoC PATCH v4 4/7] builtin/repack: enumerate promisor blobs for --drop-filtered Siddharth Shrimali
2026-08-12 17:21         ` Christian Couder
2026-08-10 17:40       ` [GSoC PATCH v4 5/7] builtin/repack: actually drop filtered promisor blobs Siddharth Shrimali
2026-08-10 17:40       ` [GSoC PATCH v4 6/7] builtin/repack: add guards for --drop-filtered Siddharth Shrimali
2026-08-12 17:41         ` Christian Couder
2026-08-12 19:45           ` Siddharth Shrimali
2026-08-10 17:40       ` [GSoC PATCH v4 7/7] Documentation/git-repack: document --drop-filtered and --dry-run Siddharth Shrimali
2026-08-12 17:51         ` Christian Couder
2026-08-11 17:50       ` [GSoC PATCH v4 0/7] repack: add --drop-filtered to reclaim space in partial clones Junio C Hamano
2026-08-12  7:35         ` Siddharth Shrimali
2026-08-13 20:08       ` [GSoC PATCH v5 0/6] " Siddharth Shrimali
2026-08-13 20:08         ` [GSoC PATCH v5 1/6] builtin/repack: add --drop-filtered and --dry-run options Siddharth Shrimali
2026-08-13 20:08         ` [GSoC PATCH v5 2/6] list-objects-filter: add list_objects_filter__filter_oidset() Siddharth Shrimali
2026-08-13 20:08         ` [GSoC PATCH v5 3/6] repack-promisor: allow excluding objects from the rebuilt promisor pack Siddharth Shrimali
2026-08-13 20:08         ` Siddharth Shrimali [this message]
2026-08-13 20:08         ` [GSoC PATCH v5 5/6] builtin/repack: actually drop filtered promisor blobs Siddharth Shrimali
2026-08-13 20:08         ` [GSoC PATCH v5 6/6] builtin/repack: add guards for --drop-filtered Siddharth Shrimali
2026-08-14 17:12         ` [GSoC PATCH v5 0/6] repack: add --drop-filtered to reclaim space in partial clones Christian Couder
2026-08-14 17:17           ` Junio C Hamano
2026-08-14 19:32             ` Siddharth Shrimali

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20260813200830.84348-5-r.siddharth.shrimali@gmail.com \
    --to=r.siddharth.shrimali@gmail.com \
    --cc=christian.couder@gmail.com \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=johannes.schindelin@gmx.de \
    --cc=l.s.r@web.de \
    --cc=ps@pks.im \
    --cc=siddharthasthana31@gmail.com \
    --cc=ttaylorr@openai.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.