[RFC PATCH 6/7] builtin/repack: actually drop filtered promisor blobs
- From
Siddharth Shrimali <r.siddharth.shrimali@gmail.com>
- Date
- Jul 16, 2026, 13:28 UTC
- Message-ID
- <20260716132848.95982-7-r.siddharth.shrimali@gmail.com>
- In-Reply-To
- <20260716132848.95982-1-r.siddharth.shrimali@gmail.com>
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