From 982fecf7c195d47c50cd397db303d262d2af88f6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=86var=20Arnfj=C3=B6r=C3=B0=20Bjarmason?= Date: Thu, 10 Nov 2022 23:36:36 +0700 Subject: [PATCH 01/12] bisect tests: test for v2.30.0 "bisect run" regressions MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Add three failing tests which succeed on v2.29.0, but due to the topic merged at [1] (specifically [2]) have been failing since then. We'll address those regressions in subsequent commits. There was also a "regression" where: git bisect run ./missing-script.sh Would count a non-existing script as "good", as the shell would exit with 127. That edge case is a bit too insane to preserve, so let's not add it to these regression tests. There was another regression that 'git bisect' consumed some options that was meant to passed down to program run with 'git bisect run'. Since that regression is breaking user's expectation, it has been fixed earlier without this patch queued. 1. 0a4cb1f1f2f (Merge branch 'mr/bisect-in-c-4', 2021-09-23) 2. d1bbbe45df8 (bisect--helper: reimplement `bisect_run` shell function in C, 2021-09-13) Signed-off-by: Ævar Arnfjörð Bjarmason Signed-off-by: Đoàn Trần Công Danh Signed-off-by: Taylor Blau --- t/t6030-bisect-porcelain.sh | 79 +++++++++++++++++++++++++++++++++++++ 1 file changed, 79 insertions(+) diff --git a/t/t6030-bisect-porcelain.sh b/t/t6030-bisect-porcelain.sh index 6dbbe62eb2..6c2c57cadf 100755 --- a/t/t6030-bisect-porcelain.sh +++ b/t/t6030-bisect-porcelain.sh @@ -252,6 +252,85 @@ test_expect_success 'bisect skip: with commit both bad and skipped' ' grep $HASH4 my_bisect_log.txt ' +test_bisect_run_args () { + test_when_finished "rm -f run.sh actual" && + >actual && + cat >expect.args && + cat <&6 >expect.out && + cat <&7 >expect.err && + write_script run.sh <<-\EOF && + while test $# != 0 + do + echo "<$1>" && + shift + done >actual.args + EOF + + test_when_finished "git bisect reset" && + git bisect start && + git bisect good $HASH1 && + git bisect bad $HASH4 && + git bisect run ./run.sh $@ >actual.out.raw 2>actual.err && + # Prune just the log output + sed -n \ + -e '/^Author:/d' \ + -e '/^Date:/d' \ + -e '/^$/d' \ + -e '/^commit /d' \ + -e '/^ /d' \ + -e 'p' \ + actual.out && + test_cmp expect.out actual.out && + test_cmp expect.err actual.err && + test_cmp expect.args actual.args +} + +test_expect_failure 'git bisect run: args, stdout and stderr with no arguments' " + test_bisect_run_args <<-'EOF_ARGS' 6<<-EOF_OUT 7<<-'EOF_ERR' + EOF_ARGS + running ./run.sh + $HASH4 is the first bad commit + bisect run success + EOF_OUT + EOF_ERR +" + +test_expect_failure 'git bisect run: args, stdout and stderr: "--" argument' " + test_bisect_run_args -- <<-'EOF_ARGS' 6<<-EOF_OUT 7<<-'EOF_ERR' + <--> + EOF_ARGS + running ./run.sh -- + $HASH4 is the first bad commit + bisect run success + EOF_OUT + EOF_ERR +" + +test_expect_failure 'git bisect run: args, stdout and stderr: "--log foo --no-log bar" arguments' " + test_bisect_run_args --log foo --no-log bar <<-'EOF_ARGS' 6<<-EOF_OUT 7<<-'EOF_ERR' + <--log> + + <--no-log> + + EOF_ARGS + running ./run.sh --log foo --no-log bar + $HASH4 is the first bad commit + bisect run success + EOF_OUT + EOF_ERR +" + +test_expect_failure 'git bisect run: args, stdout and stderr: "--bisect-start" argument' " + test_bisect_run_args --bisect-start <<-'EOF_ARGS' 6<<-EOF_OUT 7<<-'EOF_ERR' + <--bisect-start> + EOF_ARGS + running ./run.sh --bisect-start + $HASH4 is the first bad commit + bisect run success + EOF_OUT + EOF_ERR +" + # We want to automatically find the commit that # added "Another" into hello. test_expect_success '"git bisect run" simple case' ' From bdd2aa8a8bc46efce4a300d3cd6e169d89e99bc8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=86var=20Arnfj=C3=B6r=C3=B0=20Bjarmason?= Date: Thu, 10 Nov 2022 23:36:37 +0700 Subject: [PATCH 02/12] bisect: refactor bisect_run() to match CodingGuidelines MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit We didn't add "{}" to all "if/else" branches, and one "error" was mis-indented. Let's fix that first, which makes subsequent commits smaller. In the case of the "if" we can simply early return instead. Signed-off-by: Ævar Arnfjörð Bjarmason Signed-off-by: Đoàn Trần Công Danh Signed-off-by: Taylor Blau --- builtin/bisect--helper.c | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/builtin/bisect--helper.c b/builtin/bisect--helper.c index 6e41cbdb2d..08d83e6867 100644 --- a/builtin/bisect--helper.c +++ b/builtin/bisect--helper.c @@ -1191,13 +1191,12 @@ static int bisect_run(struct bisect_terms *terms, const char **argv, int argc) if (bisect_next_check(terms, NULL)) return BISECT_FAILED; - if (argc) - sq_quote_argv(&command, argv); - else { + if (!argc) { error(_("bisect run failed: no command provided.")); return BISECT_FAILED; } + sq_quote_argv(&command, argv); while (1) { res = do_bisect_run(command.buf); @@ -1268,7 +1267,7 @@ static int bisect_run(struct bisect_terms *terms, const char **argv, int argc) res = BISECT_OK; } else if (res) { error(_("bisect run failed: 'git bisect--helper --bisect-state" - " %s' exited with error code %d"), new_state, res); + " %s' exited with error code %d"), new_state, res); } else { continue; } From f37d0bdd42d08602204760f42f48cb4d77b251bf Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C4=90o=C3=A0n=20Tr=E1=BA=A7n=20C=C3=B4ng=20Danh?= Date: Thu, 10 Nov 2022 23:36:38 +0700 Subject: [PATCH 03/12] bisect: fix output regressions in v2.30.0 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When d1bbbe45df8 (bisect--helper: reimplement `bisect_run` shell function in C, 2021-09-13) reimplemented parts of "git bisect run" in C it changed the output we emitted so that: - The "running ..." line was now quoted - We lost the \n after our output - We started saying "bisect found ..." instead of "bisect run success" Arguably some of this is better now, but as d1bbbe45df8 did not advocate for changing the output, let's revert this for now. It'll be easy to change it back if that's what we'd prefer. This does not change the one remaining use of "command.buf" to emit the quoted argument, as that's new in d1bbbe45df8. Some of these cases were not tested for in the tests added in the preceding commit, I didn't have time to fleshen those out, but a look at f1de981e8b6 will show that the other output being adjusted here is now equivalent to what it was before d1bbbe45df8. Signed-off-by: Ævar Arnfjörð Bjarmason Signed-off-by: Đoàn Trần Công Danh Signed-off-by: Taylor Blau --- builtin/bisect--helper.c | 27 +++++++++++++++------------ t/t6030-bisect-porcelain.sh | 8 ++++---- 2 files changed, 19 insertions(+), 16 deletions(-) diff --git a/builtin/bisect--helper.c b/builtin/bisect--helper.c index 08d83e6867..05cab468e3 100644 --- a/builtin/bisect--helper.c +++ b/builtin/bisect--helper.c @@ -1141,17 +1141,17 @@ static int get_first_good(const char *refname UNUSED, return 1; } -static int do_bisect_run(const char *command) +static int do_bisect_run(const char *command, const char *unquoted_cmd) { struct child_process cmd = CHILD_PROCESS_INIT; - printf(_("running %s\n"), command); + printf(_("running %s\n"), unquoted_cmd); cmd.use_shell = 1; strvec_push(&cmd.args, command); return run_command(&cmd); } -static int verify_good(const struct bisect_terms *terms, const char *command) +static int verify_good(const struct bisect_terms *terms, const char *command, const char *unquoted_cmd) { int rc; enum bisect_error res; @@ -1171,7 +1171,7 @@ static int verify_good(const struct bisect_terms *terms, const char *command) if (res != BISECT_OK) return -1; - rc = do_bisect_run(command); + rc = do_bisect_run(command, unquoted_cmd); res = bisect_checkout(¤t_rev, no_checkout); if (res != BISECT_OK) @@ -1184,6 +1184,7 @@ static int bisect_run(struct bisect_terms *terms, const char **argv, int argc) { int res = BISECT_OK; struct strbuf command = STRBUF_INIT; + struct strbuf unquoted = STRBUF_INIT; const char *new_state; int temporary_stdout_fd, saved_stdout; int is_first_run = 1; @@ -1197,8 +1198,9 @@ static int bisect_run(struct bisect_terms *terms, const char **argv, int argc) } sq_quote_argv(&command, argv); + strbuf_join_argv(&unquoted, argc, argv,' '); while (1) { - res = do_bisect_run(command.buf); + res = do_bisect_run(command.buf, unquoted.buf); /* * Exit code 126 and 127 can either come from the shell @@ -1208,11 +1210,11 @@ static int bisect_run(struct bisect_terms *terms, const char **argv, int argc) * missing or non-executable script. */ if (is_first_run && (res == 126 || res == 127)) { - int rc = verify_good(terms, command.buf); + int rc = verify_good(terms, command.buf, unquoted.buf); is_first_run = 0; if (rc < 0) { error(_("unable to verify '%s' on good" - " revision"), command.buf); + " revision"), unquoted.buf); res = BISECT_FAILED; break; } @@ -1226,7 +1228,7 @@ static int bisect_run(struct bisect_terms *terms, const char **argv, int argc) if (res < 0 || 128 <= res) { error(_("bisect run failed: exit code %d from" - " '%s' is < 0 or >= 128"), res, command.buf); + " '%s' is < 0 or >= 128"), res, unquoted.buf); break; } @@ -1260,20 +1262,21 @@ static int bisect_run(struct bisect_terms *terms, const char **argv, int argc) if (res == BISECT_ONLY_SKIPPED_LEFT) error(_("bisect run cannot continue any more")); else if (res == BISECT_INTERNAL_SUCCESS_MERGE_BASE) { - printf(_("bisect run success")); + puts(_("bisect run success")); res = BISECT_OK; } else if (res == BISECT_INTERNAL_SUCCESS_1ST_BAD_FOUND) { - printf(_("bisect found first bad commit")); + puts(_("bisect run success")); res = BISECT_OK; } else if (res) { - error(_("bisect run failed: 'git bisect--helper --bisect-state" - " %s' exited with error code %d"), new_state, res); + error(_("bisect run failed: 'bisect-state %s'" + " exited with error code %d"), new_state, res); } else { continue; } break; } + strbuf_release(&unquoted); strbuf_release(&command); return res; } diff --git a/t/t6030-bisect-porcelain.sh b/t/t6030-bisect-porcelain.sh index 6c2c57cadf..a3dc5c8140 100755 --- a/t/t6030-bisect-porcelain.sh +++ b/t/t6030-bisect-porcelain.sh @@ -285,7 +285,7 @@ test_bisect_run_args () { test_cmp expect.args actual.args } -test_expect_failure 'git bisect run: args, stdout and stderr with no arguments' " +test_expect_success 'git bisect run: args, stdout and stderr with no arguments' " test_bisect_run_args <<-'EOF_ARGS' 6<<-EOF_OUT 7<<-'EOF_ERR' EOF_ARGS running ./run.sh @@ -295,7 +295,7 @@ test_expect_failure 'git bisect run: args, stdout and stderr with no arguments' EOF_ERR " -test_expect_failure 'git bisect run: args, stdout and stderr: "--" argument' " +test_expect_success 'git bisect run: args, stdout and stderr: "--" argument' " test_bisect_run_args -- <<-'EOF_ARGS' 6<<-EOF_OUT 7<<-'EOF_ERR' <--> EOF_ARGS @@ -306,7 +306,7 @@ test_expect_failure 'git bisect run: args, stdout and stderr: "--" argument' " EOF_ERR " -test_expect_failure 'git bisect run: args, stdout and stderr: "--log foo --no-log bar" arguments' " +test_expect_success 'git bisect run: args, stdout and stderr: "--log foo --no-log bar" arguments' " test_bisect_run_args --log foo --no-log bar <<-'EOF_ARGS' 6<<-EOF_OUT 7<<-'EOF_ERR' <--log> @@ -320,7 +320,7 @@ test_expect_failure 'git bisect run: args, stdout and stderr: "--log foo --no-lo EOF_ERR " -test_expect_failure 'git bisect run: args, stdout and stderr: "--bisect-start" argument' " +test_expect_success 'git bisect run: args, stdout and stderr: "--bisect-start" argument' " test_bisect_run_args --bisect-start <<-'EOF_ARGS' 6<<-EOF_OUT 7<<-'EOF_ERR' <--bisect-start> EOF_ARGS From 461fec41fa79803c367ccaf11f713d6c67ba709f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C4=90o=C3=A0n=20Tr=E1=BA=A7n=20C=C3=B4ng=20Danh?= Date: Thu, 10 Nov 2022 23:36:39 +0700 Subject: [PATCH 04/12] bisect run: keep some of the post-v2.30.0 output MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Preceding commits fixed output and behavior regressions in d1bbbe45df8 (bisect--helper: reimplement `bisect_run` shell function in C, 2021-09-13), which did not claim to be changing the output of "git bisect run". But some of the output it emitted was subjectively better, so once we've asserted that we're back on v2.29.0 behavior, let's change some of it back: - We now quote the arguments again, but omit the first " " when printing the "running" line. - Ditto for other cases where we emitted the argument - We say "found first bad commit" again, not just "run success" Signed-off-by: Ævar Arnfjörð Bjarmason Based-on-patch-by: Ævar Arnfjörð Bjarmason Signed-off-by: Đoàn Trần Công Danh Signed-off-by: Taylor Blau --- builtin/bisect--helper.c | 24 ++++++++-------- t/t6030-bisect-porcelain.sh | 55 +++++++++++++++++++++++++++++++------ 2 files changed, 58 insertions(+), 21 deletions(-) diff --git a/builtin/bisect--helper.c b/builtin/bisect--helper.c index 05cab468e3..180c2faa7f 100644 --- a/builtin/bisect--helper.c +++ b/builtin/bisect--helper.c @@ -1141,17 +1141,17 @@ static int get_first_good(const char *refname UNUSED, return 1; } -static int do_bisect_run(const char *command, const char *unquoted_cmd) +static int do_bisect_run(const char *command) { struct child_process cmd = CHILD_PROCESS_INIT; - printf(_("running %s\n"), unquoted_cmd); + printf(_("running %s\n"), command); cmd.use_shell = 1; strvec_push(&cmd.args, command); return run_command(&cmd); } -static int verify_good(const struct bisect_terms *terms, const char *command, const char *unquoted_cmd) +static int verify_good(const struct bisect_terms *terms, const char *command) { int rc; enum bisect_error res; @@ -1171,7 +1171,7 @@ static int verify_good(const struct bisect_terms *terms, const char *command, co if (res != BISECT_OK) return -1; - rc = do_bisect_run(command, unquoted_cmd); + rc = do_bisect_run(command); res = bisect_checkout(¤t_rev, no_checkout); if (res != BISECT_OK) @@ -1184,7 +1184,6 @@ static int bisect_run(struct bisect_terms *terms, const char **argv, int argc) { int res = BISECT_OK; struct strbuf command = STRBUF_INIT; - struct strbuf unquoted = STRBUF_INIT; const char *new_state; int temporary_stdout_fd, saved_stdout; int is_first_run = 1; @@ -1198,9 +1197,9 @@ static int bisect_run(struct bisect_terms *terms, const char **argv, int argc) } sq_quote_argv(&command, argv); - strbuf_join_argv(&unquoted, argc, argv,' '); + strbuf_ltrim(&command); while (1) { - res = do_bisect_run(command.buf, unquoted.buf); + res = do_bisect_run(command.buf); /* * Exit code 126 and 127 can either come from the shell @@ -1210,11 +1209,11 @@ static int bisect_run(struct bisect_terms *terms, const char **argv, int argc) * missing or non-executable script. */ if (is_first_run && (res == 126 || res == 127)) { - int rc = verify_good(terms, command.buf, unquoted.buf); + int rc = verify_good(terms, command.buf); is_first_run = 0; if (rc < 0) { - error(_("unable to verify '%s' on good" - " revision"), unquoted.buf); + error(_("unable to verify %s on good" + " revision"), command.buf); res = BISECT_FAILED; break; } @@ -1228,7 +1227,7 @@ static int bisect_run(struct bisect_terms *terms, const char **argv, int argc) if (res < 0 || 128 <= res) { error(_("bisect run failed: exit code %d from" - " '%s' is < 0 or >= 128"), res, unquoted.buf); + " %s is < 0 or >= 128"), res, command.buf); break; } @@ -1265,7 +1264,7 @@ static int bisect_run(struct bisect_terms *terms, const char **argv, int argc) puts(_("bisect run success")); res = BISECT_OK; } else if (res == BISECT_INTERNAL_SUCCESS_1ST_BAD_FOUND) { - puts(_("bisect run success")); + puts(_("bisect found first bad commit")); res = BISECT_OK; } else if (res) { error(_("bisect run failed: 'bisect-state %s'" @@ -1276,7 +1275,6 @@ static int bisect_run(struct bisect_terms *terms, const char **argv, int argc) break; } - strbuf_release(&unquoted); strbuf_release(&command); return res; } diff --git a/t/t6030-bisect-porcelain.sh b/t/t6030-bisect-porcelain.sh index a3dc5c8140..34fd45a48e 100755 --- a/t/t6030-bisect-porcelain.sh +++ b/t/t6030-bisect-porcelain.sh @@ -288,9 +288,9 @@ test_bisect_run_args () { test_expect_success 'git bisect run: args, stdout and stderr with no arguments' " test_bisect_run_args <<-'EOF_ARGS' 6<<-EOF_OUT 7<<-'EOF_ERR' EOF_ARGS - running ./run.sh + running './run.sh' $HASH4 is the first bad commit - bisect run success + bisect found first bad commit EOF_OUT EOF_ERR " @@ -299,9 +299,9 @@ test_expect_success 'git bisect run: args, stdout and stderr: "--" argument' " test_bisect_run_args -- <<-'EOF_ARGS' 6<<-EOF_OUT 7<<-'EOF_ERR' <--> EOF_ARGS - running ./run.sh -- + running './run.sh' '--' $HASH4 is the first bad commit - bisect run success + bisect found first bad commit EOF_OUT EOF_ERR " @@ -313,9 +313,9 @@ test_expect_success 'git bisect run: args, stdout and stderr: "--log foo --no-lo <--no-log> EOF_ARGS - running ./run.sh --log foo --no-log bar + running './run.sh' '--log' 'foo' '--no-log' 'bar' $HASH4 is the first bad commit - bisect run success + bisect found first bad commit EOF_OUT EOF_ERR " @@ -324,13 +324,52 @@ test_expect_success 'git bisect run: args, stdout and stderr: "--bisect-start" a test_bisect_run_args --bisect-start <<-'EOF_ARGS' 6<<-EOF_OUT 7<<-'EOF_ERR' <--bisect-start> EOF_ARGS - running ./run.sh --bisect-start + running './run.sh' '--bisect-start' $HASH4 is the first bad commit - bisect run success + bisect found first bad commit EOF_OUT EOF_ERR " +test_expect_success 'git bisect run: negative exit code' " + write_script fail.sh <<-'EOF' && + exit 255 + EOF + cat <<-'EOF' >expect && + bisect run failed: exit code -1 from './fail.sh' is < 0 or >= 128 + EOF + test_when_finished 'git bisect reset' && + git bisect start && + git bisect good $HASH1 && + git bisect bad $HASH4 && + ! git bisect run ./fail.sh 2>err && + sed -En 's/.*(bisect.*code) (-?[0-9]+) (from.*)/\1 -1 \3/p' err >actual && + test_cmp expect actual +" + +test_expect_failure 'git bisect run: unable to verify on good' " + write_script fail.sh <<-'EOF' && + head=\$(git rev-parse --verify HEAD) + good=\$(git rev-parse --verify $HASH1) + if test "\$head" = "\$good" + then + exit 255 + else + exit 127 + fi + EOF + cat <<-'EOF' >expect && + unable to verify './fail.sh' on good revision + EOF + test_when_finished 'git bisect reset' && + git bisect start && + git bisect good $HASH1 && + git bisect bad $HASH4 && + ! git bisect run ./fail.sh 2>err && + sed -n 's/.*\(unable to verify.*\)/\1/p' err >actual && + test_cmp expect actual +" + # We want to automatically find the commit that # added "Another" into hello. test_expect_success '"git bisect run" simple case' ' From 8962f8f8887c15b3e55ebc348a2490290a55d8a5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C4=90o=C3=A0n=20Tr=E1=BA=A7n=20C=C3=B4ng=20Danh?= Date: Thu, 10 Nov 2022 23:36:40 +0700 Subject: [PATCH 05/12] bisect-run: verify_good: account for non-negative exit status MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Some system never reports negative exit code at all, they reports them as bigger-than-128 instead. We take extra care for those systems in the later check for normal 'do_bisect_run' loop. Let's check it here, too. Signed-off-by: Đoàn Trần Công Danh Signed-off-by: Taylor Blau --- builtin/bisect--helper.c | 2 +- t/t6030-bisect-porcelain.sh | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/builtin/bisect--helper.c b/builtin/bisect--helper.c index 180c2faa7f..e214190599 100644 --- a/builtin/bisect--helper.c +++ b/builtin/bisect--helper.c @@ -1211,7 +1211,7 @@ static int bisect_run(struct bisect_terms *terms, const char **argv, int argc) if (is_first_run && (res == 126 || res == 127)) { int rc = verify_good(terms, command.buf); is_first_run = 0; - if (rc < 0) { + if (rc < 0 || 128 <= rc) { error(_("unable to verify %s on good" " revision"), command.buf); res = BISECT_FAILED; diff --git a/t/t6030-bisect-porcelain.sh b/t/t6030-bisect-porcelain.sh index 34fd45a48e..03d99b22f1 100755 --- a/t/t6030-bisect-porcelain.sh +++ b/t/t6030-bisect-porcelain.sh @@ -347,7 +347,7 @@ test_expect_success 'git bisect run: negative exit code' " test_cmp expect actual " -test_expect_failure 'git bisect run: unable to verify on good' " +test_expect_success 'git bisect run: unable to verify on good' " write_script fail.sh <<-'EOF' && head=\$(git rev-parse --verify HEAD) good=\$(git rev-parse --verify $HASH1) From 252060be77f1d9ba7696bb3540f61451e147bf8b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C4=90o=C3=A0n=20Tr=E1=BA=A7n=20C=C3=B4ng=20Danh?= Date: Thu, 10 Nov 2022 23:36:41 +0700 Subject: [PATCH 06/12] bisect--helper: identify as bisect when report error MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit In a later change, we will convert the bisect--helper to be builtin bisect. Let's start by self-identifying it's the real bisect when reporting error. This change is safe since 'git bisect--helper' is an implementation detail, users aren't expected to call 'git bisect--helper'. Signed-off-by: Đoàn Trần Công Danh Signed-off-by: Taylor Blau --- builtin/bisect--helper.c | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) diff --git a/builtin/bisect--helper.c b/builtin/bisect--helper.c index e214190599..f28bedac6a 100644 --- a/builtin/bisect--helper.c +++ b/builtin/bisect--helper.c @@ -1282,7 +1282,8 @@ static int bisect_run(struct bisect_terms *terms, const char **argv, int argc) static int cmd_bisect__reset(int argc, const char **argv, const char *prefix UNUSED) { if (argc > 1) - return error(_("--bisect-reset requires either no argument or a commit")); + return error(_("'%s' requires either no argument or a commit"), + "git bisect reset"); return bisect_reset(argc ? argv[0] : NULL); } @@ -1292,7 +1293,8 @@ static int cmd_bisect__terms(int argc, const char **argv, const char *prefix UNU struct bisect_terms terms = { 0 }; if (argc > 1) - return error(_("--bisect-terms requires 0 or 1 argument")); + return error(_("'%s' requires 0 or 1 argument"), + "git bisect terms"); res = bisect_terms(&terms, argc == 1 ? argv[0] : NULL); free_terms(&terms); return res; @@ -1315,7 +1317,8 @@ static int cmd_bisect__next(int argc, const char **argv UNUSED, const char *pref struct bisect_terms terms = { 0 }; if (argc) - return error(_("--bisect-next requires 0 arguments")); + return error(_("'%s' requires 0 arguments"), + "git bisect next"); get_terms(&terms); res = bisect_next(&terms, prefix); free_terms(&terms); @@ -1337,7 +1340,7 @@ static int cmd_bisect__state(int argc, const char **argv, const char *prefix UNU static int cmd_bisect__log(int argc, const char **argv UNUSED, const char *prefix UNUSED) { if (argc) - return error(_("--bisect-log requires 0 arguments")); + return error(_("'%s' requires 0 arguments"), "git bisect log"); return bisect_log(); } @@ -1383,7 +1386,7 @@ static int cmd_bisect__run(int argc, const char **argv, const char *prefix UNUSE struct bisect_terms terms = { 0 }; if (!argc) - return error(_("bisect run failed: no command provided.")); + return error(_("'%s' failed: no command provided."), "git bisect run"); get_terms(&terms); res = bisect_run(&terms, argv, argc); free_terms(&terms); From 929bf9db28e617e55b52bdd307f3d44ceeb3e978 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=86var=20Arnfj=C3=B6r=C3=B0=20Bjarmason?= Date: Thu, 10 Nov 2022 23:36:42 +0700 Subject: [PATCH 07/12] bisect test: test exit codes on bad usage MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Address a test blindspot, the "log" command is the odd one out because "git-bisect.sh" ignores any arguments it receives. Let's test both the exit codes we expect, and the stderr and stdout we're emitting. Signed-off-by: Ævar Arnfjörð Bjarmason Signed-off-by: Đoàn Trần Công Danh Signed-off-by: Taylor Blau --- t/t6030-bisect-porcelain.sh | 30 ++++++++++++++++++++++++++++++ 1 file changed, 30 insertions(+) diff --git a/t/t6030-bisect-porcelain.sh b/t/t6030-bisect-porcelain.sh index 03d99b22f1..98a72ff78a 100755 --- a/t/t6030-bisect-porcelain.sh +++ b/t/t6030-bisect-porcelain.sh @@ -34,6 +34,36 @@ HASH2= HASH3= HASH4= +test_bisect_usage () { + local code="$1" && + shift && + cat >expect && + test_expect_code $code "$@" >out 2>actual && + test_must_be_empty out && + test_cmp expect actual +} + +test_expect_success 'bisect usage' " + test_bisect_usage 1 git bisect reset extra1 extra2 <<-\EOF && + error: 'git bisect reset' requires either no argument or a commit + EOF + test_bisect_usage 1 git bisect terms extra1 extra2 <<-\EOF && + error: 'git bisect terms' requires 0 or 1 argument + EOF + test_bisect_usage 1 git bisect next extra1 <<-\EOF && + error: 'git bisect next' requires 0 arguments + EOF + test_bisect_usage 1 git bisect log extra1 <<-\EOF && + error: We are not bisecting. + EOF + test_bisect_usage 1 git bisect replay <<-\EOF && + error: no logfile given + EOF + test_bisect_usage 1 git bisect run <<-\EOF + error: 'git bisect run' failed: no command provided. + EOF +" + test_expect_success 'set up basic repo with 1 file (hello) and 4 commits' ' add_line_into_file "1: Hello World" hello && HASH1=$(git rev-parse --verify HEAD) && From 5512376ae14b07bb3c71c3ea7b4a181dd0fb4172 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=86var=20Arnfj=C3=B6r=C3=B0=20Bjarmason?= Date: Thu, 10 Nov 2022 23:36:43 +0700 Subject: [PATCH 08/12] bisect--helper: emit usage for "git bisect" MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit In subsequent commits we'll be removing "git-bisect.sh" in favor of promoting "bisect--helper" to a "bisect" built-in. In doing that we'll first need to have it support "git bisect--helper " rather than "git bisect--helper --", and then finally have its "-h" output claim to be "bisect" rather than "bisect--helper". Instead of suffering that churn let's start claiming to be "git bisect" now. In just a few commits this will be true, and in the meantime emitting the "wrong" usage information from the helper is a small price to pay to avoid the churn. Let's also declare "BUILTIN_*" macros, when we eventually migrate the sub-commands themselves to parse_options() we'll be able to re-use the strings. See 0afd556b2e1 (worktree: define subcommand -h in terms of command -h, 2022-10-13) for a recent example. Signed-off-by: Ævar Arnfjörð Bjarmason Signed-off-by: Đoàn Trần Công Danh Signed-off-by: Taylor Blau --- builtin/bisect--helper.c | 51 ++++++++++++++++++++++++++++------------ 1 file changed, 36 insertions(+), 15 deletions(-) diff --git a/builtin/bisect--helper.c b/builtin/bisect--helper.c index f28bedac6a..1ff2d4ea3f 100644 --- a/builtin/bisect--helper.c +++ b/builtin/bisect--helper.c @@ -20,18 +20,40 @@ static GIT_PATH_FUNC(git_path_bisect_names, "BISECT_NAMES") static GIT_PATH_FUNC(git_path_bisect_first_parent, "BISECT_FIRST_PARENT") static GIT_PATH_FUNC(git_path_bisect_run, "BISECT_RUN") -static const char * const git_bisect_helper_usage[] = { - N_("git bisect--helper --bisect-reset []"), - "git bisect--helper --bisect-terms [--term-good | --term-old | --term-bad | --term-new]", - N_("git bisect--helper --bisect-start [--term-{new,bad}= --term-{old,good}=]" - " [--no-checkout] [--first-parent] [ [...]] [--] [...]"), - "git bisect--helper --bisect-next", - N_("git bisect--helper --bisect-state (bad|new) []"), - N_("git bisect--helper --bisect-state (good|old) [...]"), - N_("git bisect--helper --bisect-replay "), - N_("git bisect--helper --bisect-skip [(|)...]"), - "git bisect--helper --bisect-visualize", - N_("git bisect--helper --bisect-run ..."), +#define BUILTIN_GIT_BISECT_START_USAGE \ + N_("git bisect start [--term-{new,bad}= --term-{old,good}=]" \ + " [--no-checkout] [--first-parent] [ [...]] [--]" \ + " [...]") +#define BUILTIN_GIT_BISECT_STATE_USAGE \ + N_("git bisect (good|bad) [...]") +#define BUILTIN_GIT_BISECT_TERMS_USAGE \ + "git bisect terms [--term-good | --term-bad]" +#define BUILTIN_GIT_BISECT_SKIP_USAGE \ + N_("git bisect skip [(|)...]") +#define BUILTIN_GIT_BISECT_NEXT_USAGE \ + "git bisect next" +#define BUILTIN_GIT_BISECT_RESET_USAGE \ + N_("git bisect reset []") +#define BUILTIN_GIT_BISECT_VISUALIZE_USAGE \ + "git bisect visualize" +#define BUILTIN_GIT_BISECT_REPLAY_USAGE \ + N_("git bisect replay ") +#define BUILTIN_GIT_BISECT_LOG_USAGE \ + "git bisect log" +#define BUILTIN_GIT_BISECT_RUN_USAGE \ + N_("git bisect run ...") + +static const char * const git_bisect_usage[] = { + BUILTIN_GIT_BISECT_START_USAGE, + BUILTIN_GIT_BISECT_STATE_USAGE, + BUILTIN_GIT_BISECT_TERMS_USAGE, + BUILTIN_GIT_BISECT_SKIP_USAGE, + BUILTIN_GIT_BISECT_NEXT_USAGE, + BUILTIN_GIT_BISECT_RESET_USAGE, + BUILTIN_GIT_BISECT_VISUALIZE_USAGE, + BUILTIN_GIT_BISECT_REPLAY_USAGE, + BUILTIN_GIT_BISECT_LOG_USAGE, + BUILTIN_GIT_BISECT_RUN_USAGE, NULL }; @@ -1411,11 +1433,10 @@ int cmd_bisect__helper(int argc, const char **argv, const char *prefix) OPT_SUBCOMMAND("run", &fn, cmd_bisect__run), OPT_END() }; - argc = parse_options(argc, argv, prefix, options, - git_bisect_helper_usage, 0); + argc = parse_options(argc, argv, prefix, options, git_bisect_usage, 0); if (!fn) - usage_with_options(git_bisect_helper_usage, options); + usage_with_options(git_bisect_usage, options); argc--; argv++; From df63421be927c689bbd9384d503768c0515063c4 Mon Sep 17 00:00:00 2001 From: Johannes Schindelin Date: Thu, 10 Nov 2022 23:36:44 +0700 Subject: [PATCH 09/12] bisect--helper: handle states directly MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit In preparation for making `git bisect` a real built-in, let's prepare the `bisect--helper` built-in to handle `git bisect--helper good` and `git bisect--helper bad`, i.e. eliminate the need of `state` subcommand. Signed-off-by: Johannes Schindelin Signed-off-by: Ævar Arnfjörð Bjarmason Signed-off-by: Đoàn Trần Công Danh Signed-off-by: Taylor Blau --- builtin/bisect--helper.c | 38 +++++++++++++++++++------------------- git-bisect.sh | 2 -- 2 files changed, 19 insertions(+), 21 deletions(-) diff --git a/builtin/bisect--helper.c b/builtin/bisect--helper.c index 1ff2d4ea3f..29d5a26c64 100644 --- a/builtin/bisect--helper.c +++ b/builtin/bisect--helper.c @@ -1347,18 +1347,6 @@ static int cmd_bisect__next(int argc, const char **argv UNUSED, const char *pref return res; } -static int cmd_bisect__state(int argc, const char **argv, const char *prefix UNUSED) -{ - int res; - struct bisect_terms terms = { 0 }; - - set_terms(&terms, "bad", "good"); - get_terms(&terms); - res = bisect_state(&terms, argv, argc); - free_terms(&terms); - return res; -} - static int cmd_bisect__log(int argc, const char **argv UNUSED, const char *prefix UNUSED) { if (argc) @@ -1424,7 +1412,6 @@ int cmd_bisect__helper(int argc, const char **argv, const char *prefix) OPT_SUBCOMMAND("terms", &fn, cmd_bisect__terms), OPT_SUBCOMMAND("start", &fn, cmd_bisect__start), OPT_SUBCOMMAND("next", &fn, cmd_bisect__next), - OPT_SUBCOMMAND("state", &fn, cmd_bisect__state), OPT_SUBCOMMAND("log", &fn, cmd_bisect__log), OPT_SUBCOMMAND("replay", &fn, cmd_bisect__replay), OPT_SUBCOMMAND("skip", &fn, cmd_bisect__skip), @@ -1433,14 +1420,27 @@ int cmd_bisect__helper(int argc, const char **argv, const char *prefix) OPT_SUBCOMMAND("run", &fn, cmd_bisect__run), OPT_END() }; - argc = parse_options(argc, argv, prefix, options, git_bisect_usage, 0); + argc = parse_options(argc, argv, prefix, options, git_bisect_usage, + PARSE_OPT_SUBCOMMAND_OPTIONAL); - if (!fn) - usage_with_options(git_bisect_usage, options); - argc--; - argv++; + if (!fn) { + struct bisect_terms terms = { 0 }; - res = fn(argc, argv, prefix); + if (!argc) + usage_msg_opt(_("need a command"), git_bisect_usage, options); + + set_terms(&terms, "bad", "good"); + get_terms(&terms); + if (check_and_set_terms(&terms, argv[0])) + usage_msg_optf(_("unknown command: '%s'"), git_bisect_usage, + options, argv[0]); + res = bisect_state(&terms, argv, argc); + free_terms(&terms); + } else { + argc--; + argv++; + res = fn(argc, argv, prefix); + } /* * Handle early success diff --git a/git-bisect.sh b/git-bisect.sh index dfce4b4f44..9f6c8cc093 100755 --- a/git-bisect.sh +++ b/git-bisect.sh @@ -57,8 +57,6 @@ case "$#" in case "$cmd" in help) git bisect -h ;; - bad|good|new|old|"$TERM_BAD"|"$TERM_GOOD") - git bisect--helper state "$cmd" "$@" ;; log) git bisect--helper log || exit ;; *) From 0da4b538e40f38f2ee7adc625cac18ae0d8df4d1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C4=90o=C3=A0n=20Tr=E1=BA=A7n=20C=C3=B4ng=20Danh?= Date: Thu, 10 Nov 2022 23:36:45 +0700 Subject: [PATCH 10/12] bisect--helper: log: allow arbitrary number of arguments MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit In a later change, we would like to turn bisect into a builtin by renaming bisect--helper. However, there's an oddity that "git bisect log" accepts any number of arguments and it will just ignore them all. Let's prepare for the next step by ignoring any arguments passed to "git bisect--helper log" Signed-off-by: Đoàn Trần Công Danh Signed-off-by: Taylor Blau --- builtin/bisect--helper.c | 4 +--- git-bisect.sh | 2 -- 2 files changed, 1 insertion(+), 5 deletions(-) diff --git a/builtin/bisect--helper.c b/builtin/bisect--helper.c index 29d5a26c64..6066f553fd 100644 --- a/builtin/bisect--helper.c +++ b/builtin/bisect--helper.c @@ -1347,10 +1347,8 @@ static int cmd_bisect__next(int argc, const char **argv UNUSED, const char *pref return res; } -static int cmd_bisect__log(int argc, const char **argv UNUSED, const char *prefix UNUSED) +static int cmd_bisect__log(int argc UNUSED, const char **argv UNUSED, const char *prefix UNUSED) { - if (argc) - return error(_("'%s' requires 0 arguments"), "git bisect log"); return bisect_log(); } diff --git a/git-bisect.sh b/git-bisect.sh index 9f6c8cc093..f95b8103a9 100755 --- a/git-bisect.sh +++ b/git-bisect.sh @@ -57,8 +57,6 @@ case "$#" in case "$cmd" in help) git bisect -h ;; - log) - git bisect--helper log || exit ;; *) git bisect--helper "$cmd" "$@" ;; esac From 73fce29427071e53d2650d7f201cf44d3c941e3b Mon Sep 17 00:00:00 2001 From: Johannes Schindelin Date: Thu, 10 Nov 2022 23:36:46 +0700 Subject: [PATCH 11/12] Turn `git bisect` into a full built-in MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Now that the shell script hands off to the `bisect--helper` to do _anything_ (except to show the help), it is but a tiny step to let the helper implement the actual `git bisect` command instead. This retires `git-bisect.sh`, concluding a multi-year journey that many hands helped with, in particular Pranit Bauna, Tanushree Tumane and Miriam Rubio. Signed-off-by: Johannes Schindelin Signed-off-by: Junio C Hamano Signed-off-by: Đoàn Trần Công Danh Signed-off-by: Taylor Blau --- Makefile | 3 +-- builtin.h | 2 +- builtin/{bisect--helper.c => bisect.c} | 2 +- git.c | 2 +- 4 files changed, 4 insertions(+), 5 deletions(-) rename builtin/{bisect--helper.c => bisect.c} (99%) diff --git a/Makefile b/Makefile index 4927379184..78785c6b84 100644 --- a/Makefile +++ b/Makefile @@ -627,7 +627,6 @@ THIRD_PARTY_SOURCES = # interactive shell sessions without exporting it. unexport CDPATH -SCRIPT_SH += git-bisect.sh SCRIPT_SH += git-difftool--helper.sh SCRIPT_SH += git-filter-branch.sh SCRIPT_SH += git-merge-octopus.sh @@ -1137,7 +1136,7 @@ BUILTIN_OBJS += builtin/am.o BUILTIN_OBJS += builtin/annotate.o BUILTIN_OBJS += builtin/apply.o BUILTIN_OBJS += builtin/archive.o -BUILTIN_OBJS += builtin/bisect--helper.o +BUILTIN_OBJS += builtin/bisect.o BUILTIN_OBJS += builtin/blame.o BUILTIN_OBJS += builtin/branch.o BUILTIN_OBJS += builtin/bugreport.o diff --git a/builtin.h b/builtin.h index 8901a34d6b..aa955466b4 100644 --- a/builtin.h +++ b/builtin.h @@ -116,7 +116,7 @@ int cmd_am(int argc, const char **argv, const char *prefix); int cmd_annotate(int argc, const char **argv, const char *prefix); int cmd_apply(int argc, const char **argv, const char *prefix); int cmd_archive(int argc, const char **argv, const char *prefix); -int cmd_bisect__helper(int argc, const char **argv, const char *prefix); +int cmd_bisect(int argc, const char **argv, const char *prefix); int cmd_blame(int argc, const char **argv, const char *prefix); int cmd_branch(int argc, const char **argv, const char *prefix); int cmd_bugreport(int argc, const char **argv, const char *prefix); diff --git a/builtin/bisect--helper.c b/builtin/bisect.c similarity index 99% rename from builtin/bisect--helper.c rename to builtin/bisect.c index 6066f553fd..cc9483e851 100644 --- a/builtin/bisect--helper.c +++ b/builtin/bisect.c @@ -1401,7 +1401,7 @@ static int cmd_bisect__run(int argc, const char **argv, const char *prefix UNUSE return res; } -int cmd_bisect__helper(int argc, const char **argv, const char *prefix) +int cmd_bisect(int argc, const char **argv, const char *prefix) { int res = 0; parse_opt_subcommand_fn *fn = NULL; diff --git a/git.c b/git.c index 6662548986..a2deb15e46 100644 --- a/git.c +++ b/git.c @@ -492,7 +492,7 @@ static struct cmd_struct commands[] = { { "annotate", cmd_annotate, RUN_SETUP }, { "apply", cmd_apply, RUN_SETUP_GENTLY }, { "archive", cmd_archive, RUN_SETUP_GENTLY }, - { "bisect--helper", cmd_bisect__helper, RUN_SETUP }, + { "bisect", cmd_bisect, RUN_SETUP }, { "blame", cmd_blame, RUN_SETUP }, { "branch", cmd_branch, RUN_SETUP | DELAY_PAGER_CONFIG }, { "bugreport", cmd_bugreport, RUN_SETUP_GENTLY }, From 049141dce971bdbb85b3c6d12aae7254e7ddbe68 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=86var=20Arnfj=C3=B6r=C3=B0=20Bjarmason?= Date: Tue, 15 Nov 2022 10:32:42 +0100 Subject: [PATCH 12/12] bisect; remove unused "git-bisect.sh" and ".gitignore" entry MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Since 73fce29427 (Turn `git bisect` into a full built-in, 2022-11-10) we've used builtin/bisect.c instead of git-bisect.sh to implement the "bisect" command. Let's remove the unused leftover script, and the ".gitignore" entry for the "git-bisect--helper", which also hasn't been built since 73fce29427. Signed-off-by: Ævar Arnfjörð Bjarmason Signed-off-by: Taylor Blau --- .gitignore | 1 - git-bisect.sh | 63 --------------------------------------------------- 2 files changed, 64 deletions(-) delete mode 100755 git-bisect.sh diff --git a/.gitignore b/.gitignore index cb0231fb40..fe234cfa19 100644 --- a/.gitignore +++ b/.gitignore @@ -20,7 +20,6 @@ /git-archimport /git-archive /git-bisect -/git-bisect--helper /git-blame /git-branch /git-bugreport diff --git a/git-bisect.sh b/git-bisect.sh deleted file mode 100755 index f95b8103a9..0000000000 --- a/git-bisect.sh +++ /dev/null @@ -1,63 +0,0 @@ -#!/bin/sh - -USAGE='[help|start|bad|good|new|old|terms|skip|next|reset|visualize|view|replay|log|run]' -LONG_USAGE='git bisect help - print this long help message. -git bisect start [--term-{new,bad}= --term-{old,good}=] - [--no-checkout] [--first-parent] [ [...]] [--] [...] - reset bisect state and start bisection. -git bisect (bad|new) [] - mark a known-bad revision/ - a revision after change in a given property. -git bisect (good|old) [...] - mark ... known-good revisions/ - revisions before change in a given property. -git bisect terms [--term-good | --term-bad] - show the terms used for old and new commits (default: bad, good) -git bisect skip [(|)...] - mark ... untestable revisions. -git bisect next - find next bisection to test and check it out. -git bisect reset [] - finish bisection search and go back to commit. -git bisect (visualize|view) - show bisect status in gitk. -git bisect replay - replay bisection log. -git bisect log - show bisect log. -git bisect run ... - use ... to automatically bisect. - -Please use "git help bisect" to get the full man page.' - -OPTIONS_SPEC= -. git-sh-setup - -TERM_BAD=bad -TERM_GOOD=good - -get_terms () { - if test -s "$GIT_DIR/BISECT_TERMS" - then - { - read TERM_BAD - read TERM_GOOD - } <"$GIT_DIR/BISECT_TERMS" - fi -} - -case "$#" in -0) - usage ;; -*) - cmd="$1" - get_terms - shift - case "$cmd" in - help) - git bisect -h ;; - *) - git bisect--helper "$cmd" "$@" ;; - esac -esac