From: Christian Couder Date: Wed, 02 Sep 2026 16:10:41 GMT Subject: [PATCH 0/6] Standardize early option scanning to fix argument parsing bugs 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 -- ` mistook the term name `--` for the revision/path separator, so was rejected as an invalid revision instead of being treated as a path. - `git rev-parse --default -- ` did the same, reporting "bad revision " 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 ` 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