| Message ID | 20201221162743.96056-2-mirucam@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Finish converting git bisect to C part 3 | expand |
Hi Miriam, On Mon, 21 Dec 2020, Miriam Rubio wrote: > From: Pranit Bauva <pranit.bauva@gmail.com> > > Reimplement the `bisect_log()` shell function in C and also add > `--bisect-log` subcommand to `git bisect--helper` to call it from > git-bisect.sh . > > Using `--bisect-log` subcommand is a temporary measure to port shell > function to C so as to use the existing test suite. > > Mentored-by: Lars Schneider <larsxschneider@gmail.com> > Mentored-by: Christian Couder <chriscool@tuxfamily.org> > Mentored-by: Johannes Schindelin <Johannes.Schindelin@gmx.de> > Signed-off-by: Pranit Bauva <pranit.bauva@gmail.com> > Signed-off-by: Tanushree Tumane <tanushreetumane@gmail.com> > Signed-off-by: Miriam Rubio <mirucam@gmail.com> Good. I see this was originally sent as [PATCH 20/29] in https://lore.kernel.org/git/20200120143800.900-21-mirucam@gmail.com/, but this version contains improvements: - It returns `BISECT_FAILED` in `bisect_log()` instead of -1 (and `BISECT_OK` instead of 0) - Instead of checking for `argc > 1` (which was wrong), it now verifies that no arguments were passed via `if (argc)` - It exits from the shell script appropriately when the helper failed Just one nit: > diff --git a/builtin/bisect--helper.c b/builtin/bisect--helper.c > index 709eb713a3..1854377fa6 100644 > --- a/builtin/bisect--helper.c > +++ b/builtin/bisect--helper.c > @@ -938,6 +951,8 @@ int cmd_bisect__helper(int argc, const char **argv, const char *prefix) > N_("verify the next bisection state then checkout the next bisection commit"), BISECT_AUTO_NEXT), > OPT_CMDMODE(0, "bisect-state", &cmdmode, > N_("mark the state of ref (or refs)"), BISECT_STATE), > + OPT_CMDMODE(0, "bisect-log", &cmdmode, > + N_("output the contents of BISECT_LOG"), BISECT_LOG), If this is supposed to be a more permanent subcommand (and https://git-scm.com/docs/git-bisect#_bisect_log_and_bisect_replay suggests it might be), it would probably make more sense to describe the option in less implementation-specific detail. Maybe something like: list the bisection steps so far Ciao, Dscho
diff --git a/builtin/bisect--helper.c b/builtin/bisect--helper.c index 709eb713a3..1854377fa6 100644 --- a/builtin/bisect--helper.c +++ b/builtin/bisect--helper.c @@ -904,6 +904,18 @@ static enum bisect_error bisect_state(struct bisect_terms *terms, const char **a return bisect_auto_next(terms, NULL); } +static enum bisect_error bisect_log(void) +{ + int fd, status; + fd = open(git_path_bisect_log(), O_RDONLY); + if (fd < 0) + return BISECT_FAILED; + + status = copy_fd(fd, STDOUT_FILENO); + close(fd); + return status ? BISECT_FAILED : BISECT_OK; +} + int cmd_bisect__helper(int argc, const char **argv, const char *prefix) { enum { @@ -916,7 +928,8 @@ int cmd_bisect__helper(int argc, const char **argv, const char *prefix) BISECT_AUTOSTART, BISECT_NEXT, BISECT_AUTO_NEXT, - BISECT_STATE + BISECT_STATE, + BISECT_LOG } cmdmode = 0; int res = 0, nolog = 0; struct option options[] = { @@ -938,6 +951,8 @@ int cmd_bisect__helper(int argc, const char **argv, const char *prefix) N_("verify the next bisection state then checkout the next bisection commit"), BISECT_AUTO_NEXT), OPT_CMDMODE(0, "bisect-state", &cmdmode, N_("mark the state of ref (or refs)"), BISECT_STATE), + OPT_CMDMODE(0, "bisect-log", &cmdmode, + N_("output the contents of BISECT_LOG"), BISECT_LOG), OPT_BOOL(0, "no-log", &nolog, N_("no log for BISECT_WRITE")), OPT_END() @@ -1000,6 +1015,11 @@ int cmd_bisect__helper(int argc, const char **argv, const char *prefix) get_terms(&terms); res = bisect_state(&terms, argv, argc); break; + case BISECT_LOG: + if (argc) + return error(_("--bisect-log requires 0 arguments")); + res = bisect_log(); + break; default: BUG("unknown subcommand %d", cmdmode); } diff --git a/git-bisect.sh b/git-bisect.sh index 1f3f6e9fc5..c6149846ff 100755 --- a/git-bisect.sh +++ b/git-bisect.sh @@ -169,11 +169,6 @@ exit code \$res from '\$command' is < 0 or >= 128" >&2 done } -bisect_log () { - test -s "$GIT_DIR/BISECT_LOG" || die "$(gettext "We are not bisecting.")" - cat "$GIT_DIR/BISECT_LOG" -} - get_terms () { if test -s "$GIT_DIR/BISECT_TERMS" then @@ -210,7 +205,7 @@ case "$#" in replay) bisect_replay "$@" ;; log) - bisect_log ;; + git bisect--helper --bisect-log || exit;; run) bisect_run "$@" ;; terms)