bisect--helper: parse subcommand with OPT_SUBCOMMAND

As of it is, we're parsing subcommand with OPT_CMDMODE, which will continue to parse more options even if the command has been found. When we're running "git bisect run" with a command that expecting a "--log" or "--no-log" arguments, or one of those "--bisect-..." arguments, bisect--helper may mistakenly think those options are bisect--helper's option. We may fix those problems by passing "--" when calling from git-bisect.sh, and skip that "--" in bisect--helper. However, it may interfere with user's "--". Let's parse subcommand with OPT_SUBCOMMAND since that API was born for this specific use-case. Reported-by: Lukáš Doktor <ldoktor@redhat.com> Signed-off-by: Đoàn Trần Công Danh <congdanhqx@gmail.com> Signed-off-by: Taylor Blau <me@ttaylorr.com>

Đoàn Trần Công Danh committed Nov 10, 2022 at 23:36 UTC e9011b6092411ca802907a1236d3137bf09081b6
3 files changed +30 -90
builtin/bisect--helper.c
+17 -70
@@ -1390,84 +1390,31 @@ static int cmd_bisect__run(int argc, const char **argv, const char *prefix UNUSE
1390
1391 int cmd_bisect__helper(int argc, const char **argv, const char *prefix)
1392 {
1393 - enum {
1394 - BISECT_RESET = 1,
1395 - BISECT_TERMS,
1396 - BISECT_START,
1397 - BISECT_AUTOSTART,
1398 - BISECT_NEXT,
1399 - BISECT_STATE,
1400 - BISECT_LOG,
1401 - BISECT_REPLAY,
1402 - BISECT_SKIP,
1403 - BISECT_VISUALIZE,
1404 - BISECT_RUN,
1405 - } cmdmode = 0;
1393 int res = 0;
1394 + parse_opt_subcommand_fn *fn = NULL;
1395 struct option options[] = {
1408 - OPT_CMDMODE(0, "bisect-reset", &cmdmode,
1409 - N_("reset the bisection state"), BISECT_RESET),
1410 - OPT_CMDMODE(0, "bisect-terms", &cmdmode,
1411 - N_("print out the bisect terms"), BISECT_TERMS),
1412 - OPT_CMDMODE(0, "bisect-start", &cmdmode,
1413 - N_("start the bisect session"), BISECT_START),
1414 - OPT_CMDMODE(0, "bisect-next", &cmdmode,
1415 - N_("find the next bisection commit"), BISECT_NEXT),
1416 - OPT_CMDMODE(0, "bisect-state", &cmdmode,
1417 - N_("mark the state of ref (or refs)"), BISECT_STATE),
1418 - OPT_CMDMODE(0, "bisect-log", &cmdmode,
1419 - N_("list the bisection steps so far"), BISECT_LOG),
1420 - OPT_CMDMODE(0, "bisect-replay", &cmdmode,
1421 - N_("replay the bisection process from the given file"), BISECT_REPLAY),
1422 - OPT_CMDMODE(0, "bisect-skip", &cmdmode,
1423 - N_("skip some commits for checkout"), BISECT_SKIP),
1424 - OPT_CMDMODE(0, "bisect-visualize", &cmdmode,
1425 - N_("visualize the bisection"), BISECT_VISUALIZE),
1426 - OPT_CMDMODE(0, "bisect-run", &cmdmode,
1427 - N_("use <cmd>... to automatically bisect"), BISECT_RUN),
1396 + OPT_SUBCOMMAND("reset", &fn, cmd_bisect__reset),
1397 + OPT_SUBCOMMAND("terms", &fn, cmd_bisect__terms),
1398 + OPT_SUBCOMMAND("start", &fn, cmd_bisect__start),
1399 + OPT_SUBCOMMAND("next", &fn, cmd_bisect__next),
1400 + OPT_SUBCOMMAND("state", &fn, cmd_bisect__state),
1401 + OPT_SUBCOMMAND("log", &fn, cmd_bisect__log),
1402 + OPT_SUBCOMMAND("replay", &fn, cmd_bisect__replay),
1403 + OPT_SUBCOMMAND("skip", &fn, cmd_bisect__skip),
1404 + OPT_SUBCOMMAND("visualize", &fn, cmd_bisect__visualize),
1405 + OPT_SUBCOMMAND("view", &fn, cmd_bisect__visualize),
1406 + OPT_SUBCOMMAND("run", &fn, cmd_bisect__run),
1407 OPT_END()
1408 };
1409 argc = parse_options(argc, argv, prefix, options,
1431 - git_bisect_helper_usage,
1432 - PARSE_OPT_KEEP_DASHDASH | PARSE_OPT_KEEP_UNKNOWN_OPT);
1410 + git_bisect_helper_usage, 0);
1411
1434 - if (!cmdmode)
1412 + if (!fn)
1413 usage_with_options(git_bisect_helper_usage, options);
1414 + argc--;
1415 + argv++;
1416
1437 - switch (cmdmode) {
1438 - case BISECT_RESET:
1439 - res = cmd_bisect__reset(argc, argv, prefix);
1440 - break;
1441 - case BISECT_TERMS:
1442 - res = cmd_bisect__terms(argc, argv, prefix);
1443 - break;
1444 - case BISECT_START:
1445 - res = cmd_bisect__start(argc, argv, prefix);
1446 - break;
1447 - case BISECT_NEXT:
1448 - res = cmd_bisect__next(argc, argv, prefix);
1449 - break;
1450 - case BISECT_STATE:
1451 - res = cmd_bisect__state(argc, argv, prefix);
1452 - break;
1453 - case BISECT_LOG:
1454 - res = cmd_bisect__log(argc, argv, prefix);
1455 - break;
1456 - case BISECT_REPLAY:
1457 - res = cmd_bisect__replay(argc, argv, prefix);
1458 - break;
1459 - case BISECT_SKIP:
1460 - res = cmd_bisect__skip(argc, argv, prefix);
1461 - break;
1462 - case BISECT_VISUALIZE:
1463 - res = cmd_bisect__visualize(argc, argv, prefix);
1464 - break;
1465 - case BISECT_RUN:
1466 - res = cmd_bisect__run(argc, argv, prefix);
1467 - break;
1468 - default:
1469 - BUG("unknown subcommand %d", cmdmode);
1470 - }
1417 + res = fn(argc, argv, prefix);
1418
1419 /*
1420 * Handle early success
git-bisect.sh
+3 -20
@@ -57,28 +57,11 @@ case "$#" in
57 case "$cmd" in
58 help)
59 git bisect -h ;;
60 - start)
61 - git bisect--helper --bisect-start "$@" ;;
60 bad|good|new|old|"$TERM_BAD"|"$TERM_GOOD")
63 - git bisect--helper --bisect-state "$cmd" "$@" ;;
64 - skip)
65 - git bisect--helper --bisect-skip "$@" || exit;;
66 - next)
67 - # Not sure we want "next" at the UI level anymore.
68 - git bisect--helper --bisect-next "$@" || exit ;;
69 - visualize|view)
70 - git bisect--helper --bisect-visualize "$@" || exit;;
71 - reset)
72 - git bisect--helper --bisect-reset "$@" ;;
73 - replay)
74 - git bisect--helper --bisect-replay "$@" || exit;;
61 + git bisect--helper state "$cmd" "$@" ;;
62 log)
76 - git bisect--helper --bisect-log || exit ;;
77 - run)
78 - git bisect--helper --bisect-run "$@" || exit;;
79 - terms)
80 - git bisect--helper --bisect-terms "$@" || exit;;
63 + git bisect--helper log || exit ;;
64 *)
82 - usage ;;
65 + git bisect--helper "$cmd" "$@" ;;
66 esac
67 esac
t/t6030-bisect-porcelain.sh
+10
@@ -266,6 +266,16 @@ test_expect_success '"git bisect run" simple case' '
266 git bisect reset
267 '
268
269 +# We want to make sure no arguments has been eaten
270 +test_expect_success '"git bisect run" simple case' '
271 + git bisect start &&
272 + git bisect good $HASH1 &&
273 + git bisect bad $HASH4 &&
274 + git bisect run printf "%s %s\n" reset --bisect-skip >my_bisect_log.txt &&
275 + grep -e "reset --bisect-skip" my_bisect_log.txt &&
276 + git bisect reset
277 +'
278 +
279 # We want to automatically find the commit that
280 # added "Ciao" into hello.
281 test_expect_success '"git bisect run" with more complex "git bisect start"' '