From: Justin Tobler <jltobler@gmail.com>
Date: Mon, 15 Sep 2025 17:08:26 GMT
Subject: Re: [PATCH 2/6] builtin/update-index: end ODB transaction when --verbose is specified
Message-ID: <273dcsi6xmsdb356adxrlhh3renuimnl5wopfzkxxnryi5fmwd@xud2pvwdz57g>
In-Reply-To: <aMetaZxCN8CDsr6g@pks.im>

On 25/09/15 08:08AM, Patrick Steinhardt wrote:
> On Thu, Sep 11, 2025 at 10:34:42AM -0500, Justin Tobler wrote:
> > We only start reporting updates when the --verbose option is first
> > encountered. Options are not all processed upfront. This means in the
> > follow example:
> > 
> >   $ git update-index --add foo --add bar --verbose --stdin
> > 
> > both "foo" and "bar" are silently added via a transaction. After the
> > --verbose option, subsequent updates are reported. At this point there
> > is no reason for the transaction to continue as all subsequent object
> > writes must be fully written before being reported. Thus the transaction
> > is ended early.
> 
> That's... huh. I really have no idea, but is this design intentional or
> an accident?

I think it is likely that the behavior of the verbose flag here is just
a side effect of how options parsing is handled by git-update-index(1).
Instead of just using parse_options() to parse the options in one go,
parse_options_step() is used to handle filename arguments as the come.

Looking into this a bit further, I originally thought this was so you
could do something like:

  $ git update-index --remove foo --add bar

but this doesn't work because as soon as the --remove option is
encountered, all subsequent file arguments are treated as a removed.
This does mean though something like the following _does_ work:

  $ git update-index --add foo --remove bar

This is probably unintentional though and rather awkward. Due to the
nature of argument parsing here, this interface has several other order
related quirks like this.

Looking at the history, this appears to come from 309be813c9
(update-index: migrate to parse-options API, 2010-12-01). I might be
missing something, but I'm not entirely sure why though we need to
process the filename arguments as the come instead of waiting to end to
process them all. We could explore tighening this interface by updating
to use the normal parse_options(), but I do have some concerns though
that there may be users out there relying on this quirky interface.

-Justin