From: Siddharth Shrimali <r.siddharth.shrimali@gmail.com>
Date: Fri, 17 Jul 2026 18:00:33 GMT
Subject: Re: [RFC PATCH 1/7] builtin/repack.c: add --drop-filtered and --dry-run options
Message-ID: <CAGWgyh8qFiyrQtQDOH17jnkbDrZCVGX3ETjKLZ1dV4xkofRU0w@mail.gmail.com>
In-Reply-To: <xmqqh5lyej6f.fsf@gitster.g>

On Fri, 17 Jul 2026 at 02:38, Junio C Hamano <gitster@pobox.com> wrote:
>
> > +     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 ...
>
> > +             write_bitmaps = 0;
>
> ... way too late to drop the flag here?
>

right, thanks! The commit message claims I distinguish an explicit
-b/--write-bitmap-index from a config-provided default, but the code
only tests write_bitmaps > 0, which cannot tell the two apart at this point

For v2, alongside other changes, i'll distinguish the two, so that an explicit
-b on the command line errors out, while a config-provided default is
silently disabled for the duration of the command..

Thanks!
Siddharth Shrimali