Re: [PATCH 0/6] Standardize early option scanning to fix argument parsing bugs

On Wed, Sep 2, 2026 at 8:52 PM Junio C Hamano [off-list ref] wrote:

Christian Couder [off-list ref] writes:

quoted

To allow these commands to safely skip option values during their
early scans, this series introduces a new "early-scan" sub-API into
the existing "parse-options" API.

Yay.

quoted

This is deliberately implemented as a new simple and fast scan, which
has some limitations, instead of a full refactor and reuse of the
parse_options() code,

Sigh.  In other words, we hate these ad-hoc prescan that are buggy
badly enough to replace them all with yet another ad-hoc prescan
that is know to behave differently from the real thing?

Yes, because the limitations of the new scan are not very significant
in practice while refactoring the real thing (so that it can perform
an early scan without side effects) would be much more complex.

quoted

 - `git bisect start --term-good -- <not-a-rev>` mistook the term name
   `--` for the revision/path separator, so <not-a-rev> was rejected
   as an invalid revision instead of being treated as a path.

Sorry, I fail to see much practical value in this.

quoted

 - `git rev-parse --default -- <not-a-rev>` did the same, reporting
   "bad revision <notarev>" while any other default value gives the
   usual more helpful "ambiguous argument" error.

Neither in this one.

I have removed those from the series in the v2 I just sent.

quoted

 - `git fast-import --depth 5 --allow-unsafe-features` silently
   ignored `--allow-unsafe-features`, refusing unsafe features from
   the stream.

On the other hand, this may be a very good thing.

Is the reason why the ad-hoc pre-scan failed to see it was because
it did not realize 5 is a value to the --depth option?

Yes.

quoted

All of these commands call parse_options(), but for `git bisect` and
`git rev-parse`, the specific functions doing the early scan
(bisect_start() and cmd_rev_parse()'s main loop) parse their own
options by hand after the early scan and have no `struct option` array
for those options.

If bisect_start() and cmd_rev_parse() were converted to use
`struct option`, they could use early_scan_options_from_options() and
would not be affected by limitations 1), 2) and 3) above, as both use
the early scan only to locate `--`.

I imagine that in the long term we would rather see a properly
refactored parse-options machinery perform the prescan (perhaps with
some kind of "dry-run" option given to the machinery) than yet
another ad-hoc parser like this topic introduces.  It would be very
good if this interim solution at least took the same 'options[]'
array so that when we have the real thing in the future we do not
have to redo the conversion effort.

This is what is implemented in the v2 I just sent. So yeah, when a
refactored parse-options machinery will be able to perform the
prescan, we will be able to use it to replace the early-scan parser
without changing or converting the callers.

By the way, how does this interact with your other topic that has
been stalled for quite some time?  Would moving this one forward
help the other, or do they not have much relevance to each other?  I
would rather not see two topics of non-trivial size stalled on a
single author at the same time, so ...

They are separate topics and I alternate between them. I was recently
busy with travelling to the Git Merge and was a bit sick before that,
but hopefully I should be able to spend more time on them in the next
weeks. Also it seems to me that both topics have advanced to a point
where not a lot of big changes are needed. So they should move forward
quite fast now.