[GSoC PATCH v4 6/7] builtin/repack: add guards for --drop-filtered
- From
Siddharth Shrimali <r.siddharth.shrimali@gmail.com>
- Date
- Aug 10, 2026, 17:40 UTC
- Message-ID
- <20260810174047.6524-7-r.siddharth.shrimali@gmail.com>
- In-Reply-To
- <20260810174047.6524-1-r.siddharth.shrimali@gmail.com>
--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 | 49 +++++++++++++++++++++++++++++++++ t/t7706-repack-drop-filtered.sh | 36 ++++++++++++++++++++++++ 2 files changed, 85 insertions(+)
diff --git a/builtin/repack.c b/builtin/repack.c index e90016a33e..6ef4973038 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 @@ -317,6 +319,30 @@ int cmd_repack(int argc, if (!repo_has_promisor_remote(repo)) die(_("--drop-filtered requires a promisor remote")); + /* + * refuse to run while another operation is in progress. A + * dropped object would just be lazily re-fetched when the + * operation resumes, but triggering a network fetch in the + * middle of a half-finished + * merge/rebase/cherry-pick/revert/bisect is a poor + * experience, so this is a UX convenience rather than a + * safety measure. 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; /* @@ -332,6 +358,29 @@ int cmd_repack(int argc, if (ret) goto cleanup; + /* + * refuse to drop blobs that the current index references. + * such a blob would only be lazily re-fetched by the next + * command that touches the worktree, so dropping it reclaims + * nothing. This guard just avoids that churn. 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 ba00239c9d..05d58fa456 100755 --- a/t/t7706-repack-drop-filtered.sh +++ b/t/t7706-repack-drop-filtered.sh @@ -146,4 +146,40 @@ test_expect_success '--drop-filtered removes the promisor blob locally' ' test_grep "$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