| Message ID | 409492ad830828f2b5f341706ad9ad1c64f66d6e.1661604264.git.gitgitgadget@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Finish converting git bisect into a built-in | expand |
"Johannes Schindelin via GitGitGadget" <gitgitgadget@gmail.com> writes: > From: Johannes Schindelin <johannes.schindelin@gmx.de> > > In d1bbbe45df8 (bisect--helper: reimplement `bisect_run` shell function > in C, 2021-09-13), we ported the `bisect run` subcommand to C, including > the part that prints out an error message when the implicit `git bisect > bad` or `git bisect good` failed. > > 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. > > Helped-by: Elijah Newren <newren@gmail.com> > Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de> > --- > builtin/bisect--helper.c | 2 +- > t/t6030-bisect-porcelain.sh | 10 ++++++++++ > 2 files changed, 11 insertions(+), 1 deletion(-) > > diff --git a/builtin/bisect--helper.c b/builtin/bisect--helper.c > index d797cd1cef8..a00167f1373 100644 > --- a/builtin/bisect--helper.c > +++ b/builtin/bisect--helper.c > @@ -1430,7 +1430,7 @@ static int cmd_bisect_run(int argc, const char **argv, const char *prefix) > printf(_("bisect found first bad commit")); > res = BISECT_OK; > } else if (res) { > - error(_("bisect run failed: 'git bisect--helper --bisect-state" > + error(_("bisect run failed: 'git bisect" > " %s' exited with error code %d"), new_state, res); > } else { > continue; The change to retire "bisect--helper" from the end-user facing error message makes tons of sense, but I am not quite sure if the proposed log message describes the change correctly. Or is this a fallout of some "rebase -i" gotcha that a log message meant for one commit was applied to a different commit, or something?
Hi Junio, On Sun, 28 Aug 2022, Junio C Hamano wrote: > "Johannes Schindelin via GitGitGadget" <gitgitgadget@gmail.com> > writes: > > > From: Johannes Schindelin <johannes.schindelin@gmx.de> > > > > In d1bbbe45df8 (bisect--helper: reimplement `bisect_run` shell function > > in C, 2021-09-13), we ported the `bisect run` subcommand to C, including > > the part that prints out an error message when the implicit `git bisect > > bad` or `git bisect good` failed. > > > > 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. > > > > Helped-by: Elijah Newren <newren@gmail.com> > > Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de> > > --- > > builtin/bisect--helper.c | 2 +- > > t/t6030-bisect-porcelain.sh | 10 ++++++++++ > > 2 files changed, 11 insertions(+), 1 deletion(-) > > > > diff --git a/builtin/bisect--helper.c b/builtin/bisect--helper.c > > index d797cd1cef8..a00167f1373 100644 > > --- a/builtin/bisect--helper.c > > +++ b/builtin/bisect--helper.c > > @@ -1430,7 +1430,7 @@ static int cmd_bisect_run(int argc, const char **argv, const char *prefix) > > printf(_("bisect found first bad commit")); > > res = BISECT_OK; > > } else if (res) { > > - error(_("bisect run failed: 'git bisect--helper --bisect-state" > > + error(_("bisect run failed: 'git bisect" > > " %s' exited with error code %d"), new_state, res); > > } else { > > continue; > > The change to retire "bisect--helper" from the end-user facing error > message makes tons of sense, but I am not quite sure if the proposed > log message describes the change correctly. Or is this a fallout of > some "rebase -i" gotcha that a log message meant for one commit was > applied to a different commit, or something? This is indeed a fall-out from some interactive rebase. Thank you for catching it. You can actually see it in the thread history: v2 still had a diff that was aligned with the commit message, but <46fe0774-66e7-8947-cd79-d35229eec25a@web.de> graduated between v2 and v3 and partially addressed the issue fixed by this here patch. I'm glad that we caught this in time for v6. This is the commit message I plan on using: -- snip -- bisect run: fix the error message In d1bbbe45df8 (bisect--helper: reimplement `bisect_run` shell function in C, 2021-09-13), we ported the `bisect run` subcommand to C, including the part that prints out an error message when the implicit `git bisect bad` or `git bisect good` failed. 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. 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. -- snap -- Ciao, Dscho
diff --git a/builtin/bisect--helper.c b/builtin/bisect--helper.c index d797cd1cef8..a00167f1373 100644 --- a/builtin/bisect--helper.c +++ b/builtin/bisect--helper.c @@ -1430,7 +1430,7 @@ static int cmd_bisect_run(int argc, const char **argv, const char *prefix) printf(_("bisect found first bad commit")); res = BISECT_OK; } else if (res) { - error(_("bisect run failed: 'git bisect--helper --bisect-state" + error(_("bisect run failed: 'git bisect" " %s' exited with error code %d"), new_state, res); } else { continue; diff --git a/t/t6030-bisect-porcelain.sh b/t/t6030-bisect-porcelain.sh index 6d6e72276ae..7a76f204083 100755 --- a/t/t6030-bisect-porcelain.sh +++ b/t/t6030-bisect-porcelain.sh @@ -1063,4 +1063,14 @@ test_expect_success 'bisect state output with bad commit' ' grep -F "waiting for good commit(s), bad commit known" output ' +test_expect_success 'verify correct error message' ' + git bisect reset && + git bisect start $HASH4 $HASH1 && + write_script test_script.sh <<-\EOF && + rm .git/BISECT* + EOF + test_must_fail git bisect run ./test_script.sh 2>error && + grep "git bisect good.*exited with error code" error +' + test_done