[PATCH 2/6] bisect: fix "--" detection when a term name is "--"
- From
Christian Couder <christian.couder@gmail.com>
- Date
- Sep 2, 2026, 16:10 UTC
- Message-ID
- <20260902161047.476753-3-christian.couder@gmail.com>
- In-Reply-To
- <20260902161047.476753-1-christian.couder@gmail.com>
`bisect_start()` walks its arguments twice. The second loop actually parses the options, and it knows that `--term-good`, `--term-old`, `--term-bad` and `--term-new` take their value as a separate argument, so it skips that value.
The first loop, which only looks for the "--" separating revisions from paths, doesn't know about these options. So when such an option is given "--" as its value, that "--" is mistaken for the separator and `has_double_dash` is wrongly set.
This matters because `has_double_dash` makes the second loop die on an argument that is not a valid revision, instead of treating it as the first path. So:
$ git bisect start --term-good -- notarev fatal: 'notarev' does not appear to be a valid revision
while the very same command line with any other term name happily takes "notarev" as a path.
Let's fix this by using early_scan_options(), telling it about the options taking their value as a separate argument, so that it can skip those values.
Note: One might argue that accepting a term name that looks like an option (such as "--") is a misfeature and should be forbidden entirely. However, whether we should tighten the validation rules for bisect terms is a separate UI issue that can be dealt with independently. For now, this commit simply ensures the parser correctly implements the existing rules.
Signed-off-by: Christian Couder <christian.couder@gmail.com> --- builtin/bisect.c | 27 +++++++++++++++++++++------ t/t6030-bisect-porcelain.sh | 8 ++++++++ 2 files changed, 29 insertions(+), 6 deletions(-)
diff --git a/builtin/bisect.c b/builtin/bisect.c index 1cfb8a794b..ad089b289f 100644 --- a/builtin/bisect.c +++ b/builtin/bisect.c @@ -803,6 +803,19 @@ static enum bisect_error bisect_auto_next(struct bisect_terms *terms, return bisect_next(terms, prefix); } +/* + * The options "git bisect start" accepts. Only the ones taking their + * value as a separate argument matter to the scan looking for "--" below, + * as their value has to be skipped along with them. + */ +static const struct early_scan_option bisect_start_early_options[] = { + EARLY_SCAN_SKIP_VALUE("term-good"), + EARLY_SCAN_SKIP_VALUE("term-old"), + EARLY_SCAN_SKIP_VALUE("term-bad"), + EARLY_SCAN_SKIP_VALUE("term-new"), + EARLY_SCAN_END() +}; + static enum bisect_error bisect_start(struct bisect_terms *terms, int argc, const char **argv) { @@ -825,13 +838,15 @@ static enum bisect_error bisect_start(struct bisect_terms *terms, int argc, /* * Check for one bad and then some good revisions + * + * The scan below has to know about the options taking their value + * as a separate argument, or such a value that happens to be "--" + * would be mistaken for the "--" separating revisions from paths. */ - for (i = 0; i < argc; i++) { - if (!strcmp(argv[i], "--")) { - has_double_dash = 1; - break; - } - } + i = early_scan_options(argc, argv, bisect_start_early_options, + EARLY_SCAN_STOP_AT_DASHDASH, NULL, NULL); + if (i < argc) + has_double_dash = 1; for (i = 0; i < argc; i++) { const char *arg = argv[i]; diff --git a/t/t6030-bisect-porcelain.sh b/t/t6030-bisect-porcelain.sh index a7588222a8..464ca53b42 100755 --- a/t/t6030-bisect-porcelain.sh +++ b/t/t6030-bisect-porcelain.sh @@ -1297,6 +1297,14 @@ test_expect_success 'bisect start takes options and revs in any order' ' test_cmp expected actual ' +test_expect_success 'bisect start with "--" as a term name' ' + git bisect reset && + git bisect start --term-good -- hello && + git bisect terms --term-good >actual && + echo -- >expected && + test_cmp expected actual +' + # Bisect is started with --term-new and --term-old arguments, # then skip. The HEAD should be changed. test_expect_success 'bisect skip works with --term*' '
-- 2.55.0.787.g3f9e2241eb.dirty