[PATCH 0/6] Standardize early option scanning to fix argument parsing bugs
- From
Christian Couder <christian.couder@gmail.com>
- Date
- Sep 2, 2026, 16:10 UTC
- Message-ID
- <20260902161047.476753-1-christian.couder@gmail.com>
A number of commands perform an early scan of their arguments to look for specific flags or structural separators (like `--`).
These hand-rolled early scans are often fragile. They especially fail to account for options that take their value as a separate argument. This leads to disagreements between the early scan and the actual parse_options() pass. For example, the early scanner might miss a special option entirely, or mistakenly treat an option's value as the `--` path separator.
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.
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, because the limitations are not very significant in practice, while a full refactor and reuse of the parse_options() code would be much more complex.
The current limitations of the new early scan code are:
1. short options are ignored,
2. options with PARSE_OPT_LASTARG_DEFAULT or PARSE_OPT_OPTARG are treated as not taking a separate value,
3. negated options ("--no-...") are not automatically generated,4. abbreviated options will not be matched.
Note that while the others could be real issues for some commands, "3. negated options" is not a practical issue because negated options never consume a separate argument.
The early scan is performed by a new early_scan_options() function which takes a `const struct early_scan_option *options` array as argument. That array can be built either by hand or by a new early_scan_options_from_options() function, which takes a `const struct option *options` array, when the command already uses `struct option`.
This allows us to use the new early-scan API even for commands that don't use the parse-options API yet, and which are the majority of commands performing an early scan.
In this series, only `git bisect`, `git rev-parse` and `git fast-import` are converted to the early-scan API, which fixes bugs in those commands:
- `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.
- `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.
- `git fast-import --depth 5 --allow-unsafe-features` silently ignored `--allow-unsafe-features`, refusing unsafe features from the stream.
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 `--`.
Note that using early_scan_options_from_options() rather than a hand-written table does not change how abbreviations are handled: the scan matches long names exactly either way. Limitation 4) would nevertheless become relevant to those commands, because such a conversion would also make parse_options() the parser for the options after the early scan has first inspected them, and parse_options() resolves abbreviations while their current hand-rolled loops do not.
`git diff`, `git column`, `git rev-list` and setup_revisions() in "revision.c" could also be converted to the early-scan API but aren't in this series for different reasons:
- `git diff` has a number of short options like `-S`, `-G`, `-O` taking separate values.
- `git column` scans `argv[1]` for `--command=` before reading the configuration. Because `--command` is an OPT_STRING, parse_options() also accepts `--command <name>` and abbreviations, so the two passes disagree. Converting it would fix that, but it changes user-visible behaviour in a command this series does not otherwise touch.
- `git rev-list` and "revision.c" are about converting setup_revisions(), but converting it to `struct option` first is likely the better way forward.
Overview of the patches: ========================
- Patch 1/6 introduces early_scan_options(), the early scanner that will be used instead of hand-rolled ones, along with its infrastructure.
- Patches 2/6 and 3/6 use this scanner to fix bugs in `git bisect` and `git rev-parse` respectively.
- Patch 4/6 refactors some existing code into a new parse_options_takes_argument() helper that will be used in the next patch.
- Patch 5/6 introduces the new early_scan_options_from_options() as a bridge between the parse-options API and the early-scan API.
- Patch 6/6 uses early_scan_options_from_options() to fix the early scan for `--allow-unsafe-features` in `git fast-import`.
CI tests: =========
They all pass, see:
https://github.com/chriscool/git/actions/runs/33612974808
Christian Couder (6): parse-options: add early_scan_options() bisect: fix "--" detection when a term name is "--" rev-parse: fix "--" detection when it is an option value parse-options: add parse_options_takes_argument() parse-options: build early scan options from a struct option array fast-import: use early_scan_options() for --allow-unsafe-features
Documentation/git-fast-import.adoc | 10 +- builtin/bisect.c | 27 ++++-- builtin/fast-import.c | 46 +++++---- builtin/rev-parse.c | 26 ++++-- parse-options.c | 144 ++++++++++++++++++++++++++--- parse-options.h | 92 ++++++++++++++++++ t/helper/test-parse-options.c | 71 ++++++++++++++ t/helper/test-tool.c | 2 + t/helper/test-tool.h | 2 + t/t0040-parse-options.sh | 103 +++++++++++++++++++++ t/t1500-rev-parse.sh | 5 + t/t6030-bisect-porcelain.sh | 8 ++ t/t9300-fast-import.sh | 14 +++ 13 files changed, 503 insertions(+), 47 deletions(-)
-- 2.55.0.787.g3f9e2241eb.dirty