Re: [GSoC PATCH v4 6/7] builtin/repack: add guards for --drop-filtered
From: Christian Couder <hidden>
Date: 2026-08-12 17:41:27
On Mon, Aug 10, 2026 at 7:41 PM Siddharth Shrimali [off-list ref] wrote: [...]
quoted hunk ↗ jump to hunk
@@ -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
s/refuse/Refuse/
+ * 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"));Nit: I wonder if something like die_if_some_operation_in_progress() from builtin/checkout.c could be used to improve on the error message.
quoted hunk ↗ jump to hunk
+ } + 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.
s/refuse/Refuse/
+ * 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
s/bare/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));
+ }
+ }
+quoted hunk ↗ jump to hunk
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
s/creat/Create/
+ 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 +'