Finish converting git bisect into a built-in

After three GSoC/Outreachy students spent an incredible effort on this, it is finally time to put a neat little bow on it, or maybe more like a big bow, maybe even a very large one, seeing as it takes quite a while to tie (half a year at the time of writing)...

Changes since v5:

* Fixed the commit message of "bisect run: fix the error message" that had become stale as of v3 due to 80c2e9657f2 "overtaking" it and doing half of its job.
* Skipped translation usage strings in "bisect--helper: migrate to OPT_SUBCOMMAND()" that do not contain translatable parts.
* Minor white-space clean-up in "bisect--helper: migrate to OPT_SUBCOMMAND()".

Changes since v4:

* rebased onto sg/parse-options-subcommand
* migrated to OPT_SUBCOMMAND().
* As a consequence, this patch series is now unfortunately very large. And the range-diff is much less useful than I'd like because of the extensive changes that were de facto made a precondition to moving this patch series further. Junio, I would have liked to keep the scope (and burden for the reviewers) substantially smaller, maybe you can help with the review?

Changes since v3:

* Rebased because of merge conflicts with ab/plug-leak-in-revisions.
* Fixed the bug that git bisect --bisect-terms 1 2 wanted to auto-start a bisection if running with a git executable built at the in-between state at patch "bisect: move even the command-line parsing to bisect--helper". Since this bug was "fixed" in v3 by the very next patch, "bisect: teach the bisect--helper command to show the correct usage strings", v4 avoids introducing this bug simply by letting these two patches trade places. The range-diff admittedly looks quite awful because both patches overlap quite a bit in the lines they modify. The end result is the same, though, the diff between v3's and v4's builtin/bisect.c would be empty if I hadn't been forced to rebase.
* Added a test case to ensure that this bug won't be introduced again. This test case is the only actual difference relative to v3 of this patch series.

Changes since v2:

* We're now careful to provide identical usage strings upon git bisect -h and git bisect bogus.
* When a bogus command is provided, we now error out instead of trying to start a git bisect run.
* Rebased onto main to avoid plenty of merge conflicts with rs/bisect-executable-not-found, ac/usage-string-fixups and with cd/bisect-messages-from-pre-flight-states.

Changes since v1:

* Added a regression test to "bisect run: fix the error message".
* Added a patch to address an error message that double-single-quoted the command.
* Reworked the logic in "bisect--helper: make --bisect-state optional" to delay showing the usage upon an unknown command, which should make the code a lot less confusing.
* Split out the change that moved the BISECT_STATE case to the end of the switch block.
* Added a patch that replaces the return error() calls in cmd_bisect_helper() with die() calls, to avoid returning -1 as an exit code.
* Dropped the use of parse_options() for the single purpose of handling -h; This is now done explicitly.
* Simplified the diff of "bisect: move even the option parsing to bisect--helper" by modifying argc and argv instead of modifying all the function calls using those variables.
* In the "Turn git bisect into a full built-in" patch, changed the name of the variable holding the usage to use the builtin_ prefix used in other built-ins, too.
* Removed the trailing dot from the commit message of "Turn git bisect into a full built-in".

Johannes Schindelin (16):
bisect--helper: retire the --no-log option
bisect--helper: really retire --bisect-next-check
bisect--helper: really retire --bisect-autostart
bisect--helper: simplify exit code computation
bisect--helper: make terms an explicit singleton
bisect--helper: make the order consistently argc, argv
bisect--helper: migrate to OPT_SUBCOMMAND()
bisect: verify that a bogus option won't try to start a bisection bisect run: fix the error message
bisect: avoid double-quoting when printing the failed command
bisect--helper: calling bisect_state() without an argument is a bug
bisect--helper: make state optional
bisect: move even the command-line parsing to bisect--helper Turn git bisect into a full built-in
bisect: remove Cogito-related code
bisect: no longer try to clean up left-over .git/head-name files

 Makefile                               |   3 +-
 bisect.c                               |   3 -
 builtin.h                              |   2 +-
 builtin/{bisect--helper.c => bisect.c} | 679 ++++++++++++++-----------
 git-bisect.sh                          |  84 ---
 git.c                                  |   2 +-
 t/t6030-bisect-porcelain.sh            |  21 +-
 7 files changed, 406 insertions(+), 388 deletions(-)
 rename builtin/{bisect--helper.c => bisect.c} (67%)
 delete mode 100755 git-bisect.sh

base-commit: 8f9d80f6c06369b563c76ec46c462e740a1a2cf0
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-1132%2Fdscho%2Fbisect-in-c-v6
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1132/dscho/bisect-in-c-v6
Pull-Request: https://github.com/gitgitgadget/git/pull/1132

Range-diff vs v5:

  1:  05262b6a7d1 =  1:  05262b6a7d1 bisect--helper: retire the --no-log option
  2:  1e43148864a =  2:  1e43148864a bisect--helper: really retire --bisect-next-check
  3:  1a1649d9d0d =  3:  1a1649d9d0d bisect--helper: really retire `--bisect-autostart`
  4:  9ab30552c6a =  4:  9ab30552c6a bisect--helper: simplify exit code computation
  5:  92b3b116ef8 =  5:  92b3b116ef8 bisect--helper: make `terms` an explicit singleton
  6:  c9dc0281e38 =  6:  c9dc0281e38 bisect--helper: make the order consistently `argc, argv`
  7:  5b7a3d58b4f !  7:  e97e187bbec bisect--helper: migrate to OPT_SUBCOMMAND()
     @@ Commit message
          `bisect--helper` learns about it in preparation for replacing the
          `git-bisect.sh` script altogether.
      
     +    As a consequence, the usage strings are copied over from the scripted
     +    version of the `git bisect` command. To avoid regressing on 959d670d1a4
     +    (i18n: remove from i18n strings that do not hold translatable parts,
     +    2022-01-31), we specifically do not enclose usage strings in `N_(...)`
     +    that do not contain any translatable parts.
     +
          Note that a couple of `bisect_*()` functions are not converted into
          `cmd_bisect_*()` functions directly, as they have callers other than the
          `OPT_SUBCOMMAND()` one (and the original functions did not expect
     @@ builtin/bisect--helper.c: static GIT_PATH_FUNC(git_path_bisect_names, "BISECT_NA
      +	   "    [<pathspec>...]"),
      +	N_("git bisect (bad|new) [<rev>]"),
      +	N_("git bisect (good|old) [<rev>...]"),
     -+	N_("git bisect terms [--term-good | --term-bad]"),
     ++	"git bisect terms [--term-good | --term-bad]",
      +	N_("git bisect skip [(<rev>|<range>)...]"),
     -+	N_("git bisect next"),
     ++	"git bisect next",
      +	N_("git bisect reset [<commit>]"),
     -+	N_("git bisect (visualize|view)"),
     ++	"git bisect (visualize|view)",
      +	N_("git bisect replay <logfile>"),
     -+	N_("git bisect log"),
     ++	"git bisect log",
      +	N_("git bisect run <cmd>..."),
      +	NULL
      +};
     @@ builtin/bisect--helper.c: static GIT_PATH_FUNC(git_path_bisect_names, "BISECT_NA
      +};
      +
      +static const char * const bisect_terms_usage[] = {
     -+	N_("git bisect terms [--term-good | --term-bad]"),
     ++	"git bisect terms [--term-good | --term-bad]",
      +	NULL
      +};
      +
     @@ builtin/bisect--helper.c: static GIT_PATH_FUNC(git_path_bisect_names, "BISECT_NA
      +};
      +
      +static const char * const bisect_next_usage[] = {
     -+	N_("git bisect next"),
     ++	"git bisect next",
      +	NULL
      +};
      +
     @@ builtin/bisect--helper.c: static GIT_PATH_FUNC(git_path_bisect_names, "BISECT_NA
      +};
      +
      +static const char * const bisect_visualize_usage[] = {
     -+	N_("git bisect visualize"),
     ++	"git bisect visualize",
      +	NULL
      +};
      +
     @@ builtin/bisect--helper.c: static GIT_PATH_FUNC(git_path_bisect_names, "BISECT_NA
      +};
      +
      +static const char * const bisect_log_usage[] = {
     -+	N_("git bisect log"),
     ++	"git bisect log",
      +	NULL
      +};
      +
     @@ builtin/bisect--helper.c: static int bisect_reset(const char *commit)
      +		OPT_END()
      +	};
      +	argc = parse_options(argc, argv, prefix, options,
     -+			     bisect_reset_usage,  PARSE_OPT_STOP_AT_NON_OPTION);
     ++			     bisect_reset_usage, PARSE_OPT_STOP_AT_NON_OPTION);
      +
      +	if (argc > 1)
      +		usage_msg_opt(_("requires either no argument or a commit"),
     @@ builtin/bisect--helper.c: finish:
      +	};
      +
      +	argc = parse_options(argc, argv, prefix, options,
     -+			     bisect_terms_usage,  PARSE_OPT_STOP_AT_NON_OPTION);
     ++			     bisect_terms_usage, PARSE_OPT_STOP_AT_NON_OPTION);
      +
      +	if (argc > 1)
      +		usage_msg_opt(_("terms: requires 0 or 1 argument"),
     @@ builtin/bisect--helper.c: static enum bisect_error bisect_state(int argc, const
      +	get_terms();
      +
      +	argc = parse_options(argc, argv, prefix, options,
     -+			     bisect_state_usage,  PARSE_OPT_STOP_AT_NON_OPTION);
     ++			     bisect_state_usage, PARSE_OPT_STOP_AT_NON_OPTION);
      +
      +	if (!argc)
      +		usage_msg_opt(_("need at least one argument"),
     @@ builtin/bisect--helper.c: static enum bisect_error bisect_state(int argc, const
       	const char* filename = git_path_bisect_log();
       
      +	argc = parse_options(argc, argv, prefix, options,
     -+			     bisect_log_usage,  PARSE_OPT_STOP_AT_NON_OPTION);
     ++			     bisect_log_usage, PARSE_OPT_STOP_AT_NON_OPTION);
      +
      +	if (argc > 0)
      +		usage_msg_opt(_("require 0 argument"), bisect_log_usage,
     @@ builtin/bisect--helper.c: static int process_replay_line(struct strbuf *line)
       
      +	set_terms("bad", "good");
      +	argc = parse_options(argc, argv, prefix, options,
     -+			     bisect_reset_usage,  PARSE_OPT_STOP_AT_NON_OPTION);
     ++			     bisect_reset_usage, PARSE_OPT_STOP_AT_NON_OPTION);
      +
      +	if (argc != 1)
      +		usage_msg_opt(_("no logfile given"), bisect_replay_usage,
     @@ builtin/bisect--helper.c: static enum bisect_error bisect_replay(const char *fil
      +	set_terms("bad", "good");
      +	get_terms();
      +	argc = parse_options(argc, argv, prefix, options,
     -+			     bisect_skip_usage,  PARSE_OPT_STOP_AT_NON_OPTION);
     ++			     bisect_skip_usage, PARSE_OPT_STOP_AT_NON_OPTION);
      +
       	strvec_push(&argv_state, "skip");
       
     @@ builtin/bisect--helper.c: static int bisect_run(int argc, const char **argv)
       
      +	get_terms();
      +	argc = parse_options(argc, argv, prefix, options,
     -+			     bisect_run_usage,  PARSE_OPT_STOP_AT_NON_OPTION);
     ++			     bisect_run_usage, PARSE_OPT_STOP_AT_NON_OPTION);
      +
       	if (bisect_next_check(NULL))
       		return BISECT_FAILED;
  8:  ba537af7066 =  8:  30c87f2e92e bisect: verify that a bogus option won't try to start a bisection
  9:  409492ad830 !  9:  4696652b99c bisect run: fix the error message

@@ Commit message

However, the error message was supposed to print out whether the state was "good" or "bad", but used a bogus (because non-populated) args
- variable for it.
+ variable for it. This was fixed in 80c2e9657f2 (bisect--helper: report
+ actual bisect_state() argument on error, 2022-01-18), but the error
+ message still talks about bisect--helper, which is an implementation
+ detail that should not concern end users.
+
+ Fix that, and add a regression test to ensure that the intended form of
+ the error message.

 10:  bc5efc8fbfe = 10:  b202a0e386c bisect: avoid double-quoting when printing the failed command
 11:  8a0adfe3867 = 11:  3376b450867 bisect--helper: calling `bisect_state()` without an argument is a bug
 12:  189d2b3ba46 = 12:  e7623508f90 bisect--helper: make `state` optional
 13:  32bf74e3050 = 13:  3f052580c95 bisect: move even the command-line parsing to `bisect--helper`
 14:  a8f08f5e0cb = 14:  a83fe3dc3c2 Turn `git bisect` into a full built-in
 15:  a96489310d3 = 15:  f2132b61ff7 bisect: remove Cogito-related code
 16:  bfa7aa19f03 = 16:  4f93692e071 bisect: no longer try to clean up left-over `.git/head-name` files

gitgitgadget