Re: [RFC PATCH 1/7] builtin/repack.c: add --drop-filtered and --dry-run options
- From
Junio C Hamano <gitster@pobox.com>
- Date
- Jul 16, 2026, 21:08 UTC
- Message-ID
- <xmqqh5lyej6f.fsf@gitster.g>
- In-Reply-To
- <20260716132848.95982-2-r.siddharth.shrimali@gmail.com>
Siddharth Shrimali <r.siddharth.shrimali@gmail.com> writes:
> --drop-filtered is incompatible with bitmap writing: filtering breaks > the "all objects in one pack" closure that bitmaps require. An explicit > -b is rejected with a clear error and a default-on bitmap configuration is > silently disabled for the duration of the command.
That is very well intentioned.
Show 20 quoted lines
> @@ -231,6 +234,10 @@ int cmd_repack(int argc,
> N_("pack prefix to store a pack containing pruned objects")),
> OPT_STRING(0, "filter-to", &filter_to, N_("dir"),
> N_("pack prefix to store a pack containing filtered out objects")),
> + OPT_BOOL(0, "drop-filtered", &drop_filtered,
> + N_("delete filtered out objects (requires --filter)")),
> + OPT_BOOL(0, "dry-run", &dry_run,
> + N_("only show which objects would be dropped")),
> OPT_END()
> };
>
> @@ -252,6 +259,43 @@ int cmd_repack(int argc,
> po_args.depth = xstrdup_or_null(opt_depth);
> po_args.threads = xstrdup_or_null(opt_threads);
>
> + die_for_incompatible_opt2(drop_filtered, "--drop-filtered",
> + !!filter_to, "--filter-to");
> +
> + die_for_incompatible_opt2(drop_filtered, "--drop-filtered",
> + write_bitmaps > 0, "--write-bitmap-index");Hmph. Since this step does not change the parsing or configuration for write_bitmaps, we cannot tell if (write_bitmaps == 1) at this point in the execution came from the command line (e.g., an earlier call to parse_options() around line 247 of builtin/repack.c) or from the configuration files (e.g., a call to repo_config() around line 245). In other words, wouldn't it be ...
Show 29 quoted lines
> + if (dry_run && !drop_filtered)
> + 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"));
> +
> + if (!(pack_everything & ALL_INTO_ONE))
> + die(_("--drop-filtered requires -a"));
> +
> + /*
> + * Only blob:limit=<n> is supported for now. Reject other
> + * filter choices early, before walking the object database.
> + */
> + if (po_args.filter_options.choice != LOFC_BLOB_LIMIT)
> + die(_("--drop-filtered only supports --filter=blob:limit=<n> for now"));
> +
> + /*
> + * Without a promisor remote there is nowhere to re-fetch the
> + * dropped objects from, so dropping them would be permanent
> + * data loss.
> + */
> + if (!repo_has_promisor_remote(repo))
> + die(_("--drop-filtered requires a promisor remote"));
> +
> + write_bitmaps = 0;... way too late to drop the flag here?
> + }
> +
> if (delete_redundant && repo->repository_format_precious_objects)
> die(_("cannot delete packs in a precious-objects repo"));