Re: [GSoC PATCH v4 6/7] builtin/repack: add guards for --drop-filtered
- From
Christian Couder <christian.couder@gmail.com>
- Date
- Aug 12, 2026, 17:41 UTC
- Message-ID
- <CAP8UFD2ii8C77jWpyHRYG=H7y4t-PFtR5FQB3on4bmB5FQyG4A@mail.gmail.com>
- In-Reply-To
- <20260810174047.6524-7-r.siddharth.shrimali@gmail.com>
On Mon, Aug 10, 2026 at 7:41 PM Siddharth Shrimali <r.siddharth.shrimali@gmail.com> wrote:
[...]
Show 6 quoted lines
> @@ -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. As/refuse/Refuse/
Show 18 quoted lines
> + * 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.
Show 13 quoted lines
> + } > + 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/
Show 18 quoted lines
> + * 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));
> + }
> + }
> +Show 12 quoted lines
> 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/
Show 16 quoted lines
> + 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 > +'