Re: [PATCH] bisect: fix handling of `help` and invalid subcommands

From: Ben Knoble <ben.knoble@gmail.com>
Date: Oct 22, 2025, 17:52 UTC
Message-ID: <3DA38465-7636-4EEF-B074-53E4628F5355@gmail.com>
In-Reply-To: <pull.2078.git.git.1761122173126.gitgitgadget@gmail.com>

Show 11 quoted lines

> Le 22 oct. 2025 à 04:37, Ruoyu Zhong via GitGitGadget <gitgitgadget@gmail.com> a écrit :
> 
> From: Ruoyu Zhong <zhongruoyu@outlook.com>
> 
> As documented in git-bisect(1), `git bisect help` should display usage
> information. However, since the migration of `git bisect` to a full
> builtin command in 73fce29427 (Turn `git bisect` into a full built-in,
> 2022-11-10), this behavior was broken. Running `git bisect help` would,
> instead of showing usage, either fail silently if already in a bisect
> session, or otherwise trigger an interactive autostart prompt asking "Do
> you want me to do it for you [Y/n]?".

Good catch!

FWIW, in this project we describe the buggy behavior in the present tense (« is broken », « Running git bisect shows », etc.)

Show 42 quoted lines

> 
> Similarly, since df63421be9 (bisect--helper: handle states directly,
> 2022-11-10), running invalid subcommands like `git bisect foobar` also
> led to the same behavior.
> 
> This occurred because `help` and other unrecognized subcommands were
> being unconditionally passed to `bisect_state`, which then called
> `bisect_autostart`, triggering the interactive prompt.
> 
> Fix this by:
> 1. Adding explicit handling for the `help` subcommand to show usage;
> 2. Validating that unrecognized commands are actually valid state
>   commands before calling `bisect_state`;
> 3. Showing an error with usage for truly invalid commands.
> 
> This ensures that `git bisect help` displays the usage as documented,
> and invalid commands fail cleanly without entering interactive mode.
> Alternate terms are still handled correctly through
> `check_and_set_terms`.
> 
> Signed-off-by: Ruoyu Zhong <zhongruoyu@outlook.com>
> ---
>    bisect: fix handling of help and invalid subcommands
> 
> Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-2078%2FZhongRuoyu%2Fgit-bisect-subcommands-v1
> Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-2078/ZhongRuoyu/git-bisect-subcommands-v1
> Pull-Request: https://github.com/git/git/pull/2078
> 
> builtin/bisect.c | 6 +++++-
> 1 file changed, 5 insertions(+), 1 deletion(-)
> 
> diff --git a/builtin/bisect.c b/builtin/bisect.c
> index 8b8d870cd1..993caf545d 100644
> --- a/builtin/bisect.c
> +++ b/builtin/bisect.c
> @@ -1453,9 +1453,13 @@ int cmd_bisect(int argc,
>        if (!argc)
>            usage_msg_opt(_("need a command"), git_bisect_usage, options);
> 
> +        if (!strcmp(argv[0], "help"))
> +            usage_with_options(git_bisect_usage, options);
> +

From an extremely quick look at the code, this might be better handled with a new OPT_SUBCOMMAND, though that might mean making the options array statically scoped to this file rather than the function.

It would also be nice to update the usage to match the manual while we’re here, which presumably in turn affects the test between command usage and manuals.

Show 12 quoted lines

>        set_terms(&terms, "bad", "good");
>        get_terms(&terms);
> -        if (check_and_set_terms(&terms, argv[0]))
> +        if (check_and_set_terms(&terms, argv[0]) ||
> +            !one_of(argv[0], terms.term_good, terms.term_bad, NULL))
>            usage_msg_optf(_("unknown command: '%s'"), git_bisect_usage,
>                       options, argv[0]);
>        res = bisect_state(&terms, argc, argv);
> 
> base-commit: 81f86aacc4eb74cdb9c2c8082d36d2070c666045
> --
> gitgitgadget

I think this part is OK, since we only intend to check this when using the « git bisect <term> » form.