[PATCH v4 00/16] Finish converting git bisect into a built-in
- From
Johannes Schindelin via GitGitGadget <gitgitgadget@gmail.com>
- Date
- Jun 27, 2022, 18:31 UTC
- Message-ID
- <pull.1132.v4.git.1656354677.gitgitgadget@gmail.com>
- In-Reply-To
- <pull.1132.v3.git.1653144546.gitgitgadget@gmail.com>
After three GSoC/Outreachy students spent an incredible effort on this, it is finally time to put a neat little bow on it.
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: 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: retire the --no-log option
bisect--helper: really retire --bisect-next-check
bisect--helper: really retire `--bisect-autostart`
bisect--helper: using `--bisect-state` without an argument is a bug
bisect--helper: align the sub-command order with git-bisect.sh
bisect--helper: make `--bisect-state` optional
bisect--helper: move the `BISECT_STATE` case to the end
bisect--helper: return only correct exit codes in `cmd_*()`
bisect: teach the `bisect--helper` command to show the correct usage
strings
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} | 202 +++++++++++--------------
git-bisect.sh | 84 ----------
git.c | 2 +-
t/t6030-bisect-porcelain.sh | 21 ++-
7 files changed, 110 insertions(+), 207 deletions(-)
rename builtin/{bisect--helper.c => bisect.c} (89%)
delete mode 100755 git-bisect.shbase-commit: 8168d5e9c23ed44ae3d604f392320d66556453c9 Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-1132%2Fdscho%2Fbisect-in-c-v4 Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1132/dscho/bisect-in-c-v4 Pull-Request: https://github.com/gitgitgadget/git/pull/1132
Range-diff vs v3:
-: ----------- > 1: 30ddbd7affc bisect: verify that a bogus option won't try to start a bisection
1: cf6034625dd = 2: 97dd2da8f89 bisect run: fix the error message
2: 955ccd4d8c8 = 3: 5571e0f76ff bisect: avoid double-quoting when printing the failed command
3: abcbc25966c = 4: 5bfaf0334c3 bisect--helper: retire the --no-log option
4: af60ef1b5a4 = 5: e85f236304b bisect--helper: really retire --bisect-next-check
5: 07a92c58f8e = 6: b94b7bb4fd0 bisect--helper: really retire `--bisect-autostart`
6: 04ba0950b85 = 7: aad3c9a0850 bisect--helper: using `--bisect-state` without an argument is a bug
7: 6847af9d485 = 8: 375a46dca9f bisect--helper: align the sub-command order with git-bisect.sh
8: b7bc53b9cb6 = 9: c57f63f6a61 bisect--helper: make `--bisect-state` optional
9: 1919237a819 = 10: 87f53469a72 bisect--helper: move the `BISECT_STATE` case to the end
10: 1236a731903 = 11: ce508583e45 bisect--helper: return only correct exit codes in `cmd_*()`
12: ac472aefb6a ! 12: 5dbe233e4ec bisect: teach the `bisect--helper` command to show the correct usage strings
@@ Commit message
`bisect` command, we hereby teach it to print the very same usage
strings as the scripted `git-bisect.sh` does.
- With this patch, the `bisect--helper` command is able to do everything
- that the `git-bisect.sh` script could, leaving as last step only to
- retire that script at long last, which we will do in the next commit.
-
- Note: Since we cannot use the `parse-options` API to handle the
- subcommands of `git bisect` anyway, we no longer use it even just to
- show the usage string anymore, either.
-
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
## builtin/bisect--helper.c ##
@@ builtin/bisect--helper.c: static GIT_PATH_FUNC(git_path_bisect_names, "BISECT_NA
- N_("git bisect--helper --bisect-skip [(<rev>|<range>)...]"),
- "git bisect--helper --bisect-visualize",
- N_("git bisect--helper --bisect-run <cmd>..."),
-- NULL
--};
+static const char *bisect_usage =
+ N_("git bisect [help|start|bad|good|new|old|terms|skip|next|reset|"
+ "visualize|view|replay|log|run]");
+
-+static const char *bisect_long_usage =
++static const char * const bisect_long_usage[] = {
+ N_("git bisect [help|start|bad|good|new|old|terms|skip|next|reset|"
+ "visualize|view|replay|log|run]\n"
+ "\n"
@@ builtin/bisect--helper.c: static GIT_PATH_FUNC(git_path_bisect_names, "BISECT_NA
+ "git bisect run <cmd>...\n"
+ "\tuse <cmd>... to automatically bisect.\n"
+ "\n"
-+ "Please use \"git help bisect\" to get the full man page.");
++ "Please use \"git help bisect\" to get the full man page."),
+ NULL
+ };
- struct add_bisect_ref_data {
- struct rev_info *revs;
-@@ builtin/bisect--helper.c: static int bisect_run(struct bisect_terms *terms, const char **argv, int argc)
- int cmd_bisect__helper(int argc, const char **argv, const char *prefix)
- {
- int res = 0;
-- struct option options[] = {
-- OPT_END()
-- };
+@@ builtin/bisect--helper.c: int cmd_bisect__helper(int argc, const char **argv, const char *prefix)
struct bisect_terms terms = { .term_good = NULL, .term_bad = NULL };
- const char *command = argc > 1 ? argv[1] : "help";
- if (!strcmp("-h", command) || !strcmp("help", command))
-- usage_with_options(git_bisect_helper_usage, options);
-+ usage(bisect_long_usage);
+ argc = parse_options(argc, argv, prefix, options,
+- git_bisect_helper_usage,
++ bisect_long_usage,
+ PARSE_OPT_KEEP_DASHDASH | PARSE_OPT_KEEP_UNKNOWN);
- argc -= 2;
- argv += 2;
+ switch (cmdmode ? cmdmode : BISECT_STATE) {
@@ builtin/bisect--helper.c: int cmd_bisect__helper(int argc, const char **argv, const char *prefix)
- get_terms(&terms);
res = bisect_run(&terms, argv, argc);
- } else {
-+ if (!file_is_not_empty(git_path_bisect_start()) &&
-+ !one_of(command, "bad", "good", "new", "old", NULL))
+ break;
+ case BISECT_STATE:
++ if (argc &&
++ !file_is_not_empty(git_path_bisect_start()) &&
++ !one_of(argv[0], "bad", "good", "new", "old", NULL))
+ usage(bisect_usage);
set_terms(&terms, "bad", "good");
get_terms(&terms);
-- if (check_and_set_terms(&terms, command)) {
-- char *msg = xstrfmt(_("unknown command: '%s'"), command);
+ if (!cmdmode &&
+- (!argc || check_and_set_terms(&terms, argv[0]))) {
+- char *msg = xstrfmt(_("unknown command: '%s'"), argv[0]);
- usage_msg_opt(msg, git_bisect_helper_usage, options);
- }
-+ if (check_and_set_terms(&terms, command))
++ (!argc || check_and_set_terms(&terms, argv[0])))
+ usage(bisect_usage);
- /* shift the `command` back in */
- argc++;
- argv--;
+ res = bisect_state(&terms, argv, argc);
+ break;
+ default:
11: 4ae78d37d04 ! 13: d56f2a14060 bisect: move even the command-line parsing to `bisect--helper`
@@ Commit message
bisect: move even the command-line parsing to `bisect--helper`
On our journey to a fully built-in `git bisect`, this is the
- one of the last steps.
+ last step.
Side note: The `parse-options` API is not at all set up to parse
subcommands such as `git bisect start`, `git bisect reset`, etc.
@@ Commit message
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
## builtin/bisect--helper.c ##
+@@ builtin/bisect--helper.c: static const char *bisect_usage =
+ N_("git bisect [help|start|bad|good|new|old|terms|skip|next|reset|"
+ "visualize|view|replay|log|run]");
+
+-static const char * const bisect_long_usage[] = {
++static const char *bisect_long_usage =
+ N_("git bisect [help|start|bad|good|new|old|terms|skip|next|reset|"
+ "visualize|view|replay|log|run]\n"
+ "\n"
+@@ builtin/bisect--helper.c: static const char * const bisect_long_usage[] = {
+ "git bisect run <cmd>...\n"
+ "\tuse <cmd>... to automatically bisect.\n"
+ "\n"
+- "Please use \"git help bisect\" to get the full man page."),
+- NULL
+-};
++ "Please use \"git help bisect\" to get the full man page.");
+
+ struct add_bisect_ref_data {
+ struct rev_info *revs;
@@ builtin/bisect--helper.c: static int bisect_run(struct bisect_terms *terms, const char **argv, int argc)
int cmd_bisect__helper(int argc, const char **argv, const char *prefix)
@@ builtin/bisect--helper.c: static int bisect_run(struct bisect_terms *terms, cons
- BISECT_RUN,
- } cmdmode = 0;
int res = 0;
- struct option options[] = {
+- struct option options[] = {
- OPT_CMDMODE(0, "bisect-start", &cmdmode,
- N_("start the bisect session"), BISECT_START),
- OPT_CMDMODE(0, "bisect-state", &cmdmode,
@@ builtin/bisect--helper.c: static int bisect_run(struct bisect_terms *terms, cons
- N_("list the bisection steps so far"), BISECT_LOG),
- OPT_CMDMODE(0, "bisect-run", &cmdmode,
- N_("use <cmd>... to automatically bisect"), BISECT_RUN),
- OPT_END()
- };
+- OPT_END()
+- };
struct bisect_terms terms = { .term_good = NULL, .term_bad = NULL };
+ const char *command = argc > 1 ? argv[1] : "help";
++
++ if (!strcmp("-h", command) || !strcmp("help", command))
++ usage(bisect_long_usage);
- argc = parse_options(argc, argv, prefix, options,
-- git_bisect_helper_usage,
+- bisect_long_usage,
- PARSE_OPT_KEEP_DASHDASH | PARSE_OPT_KEEP_UNKNOWN);
-+ if (!strcmp("-h", command) || !strcmp("help", command))
-+ usage_with_options(git_bisect_helper_usage, options);
++ argc -= 2;
++ argv += 2;
- switch (cmdmode ? cmdmode : BISECT_STATE) {
- case BISECT_START:
-+ argc -= 2;
-+ argv += 2;
-+
+ if (!strcmp("start", command)) {
set_terms(&terms, "bad", "good");
res = bisect_start(&terms, argv, argc);
@@ builtin/bisect--helper.c: static int bisect_run(struct bisect_terms *terms, cons
res = bisect_run(&terms, argv, argc);
- break;
- case BISECT_STATE:
+- if (argc &&
+- !file_is_not_empty(git_path_bisect_start()) &&
+- !one_of(argv[0], "bad", "good", "new", "old", NULL))
+ } else {
++ if (!file_is_not_empty(git_path_bisect_start()) &&
++ !one_of(command, "bad", "good", "new", "old", NULL))
+ usage(bisect_usage);
set_terms(&terms, "bad", "good");
get_terms(&terms);
- if (!cmdmode &&
-- (!argc || check_and_set_terms(&terms, argv[0]))) {
-- char *msg = xstrfmt(_("unknown command: '%s'"), argv[0]);
-+ if (check_and_set_terms(&terms, command)) {
-+ char *msg = xstrfmt(_("unknown command: '%s'"), command);
- usage_msg_opt(msg, git_bisect_helper_usage, options);
- }
+- (!argc || check_and_set_terms(&terms, argv[0])))
++ if (check_and_set_terms(&terms, command))
+ usage(bisect_usage);
+ /* shift the `command` back in */
+ argc++;
+ argv--;
13: 85f5c256ae3 = 14: 378d6d22737 Turn `git bisect` into a full built-in
14: 289917e96af = 15: 33566b86d77 bisect: remove Cogito-related code
15: 8f8d2ba0fe4 = 16: 334664f23a8 bisect: no longer try to clean up left-over `.git/head-name` files-- gitgitgadget