[PATCH v4 0/7] Finish converting git bisect to C part 3

[PATCH v4 0/7] Finish converting git bisect to C part 3

[Miriam Rubio]

These patches correspond to a third part of patch series 
of Outreachy project "Finish converting `git bisect` from shell to C" 
started by Pranit Bauva and Tanushree Tumane
(https://public-inbox.org/git/pull.117.git.gitgitgadget@gmail.com) and
continued by me.

This third part is formed by reimplementations of some `git bisect` 
subcommands and removal of some temporary subcommands.

These patch series emails were generated from:
https://gitlab.com/mirucam/git/commits/git-bisect-work-part3_v3.

I would like to thank Rafael Silva and Junio Hamano for their
reviews and suggestions.

Specific changes
----------------

[1/7] bisect--helper: reimplement `bisect_log` shell function in C
* Add a missing space.
---

[2/7] bisect--helper: reimplement `bisect_replay` shell function in C
* Fix declaration after statement warning.


Pranit Bauva (7):
  bisect--helper: reimplement `bisect_log` shell function in C
  bisect--helper: reimplement `bisect_replay` shell function in C
  bisect--helper: retire `--bisect-write` subcommand
  bisect--helper: use `res` instead of return in BISECT_RESET case
    option
  bisect--helper: retire `--bisect-auto-next` subcommand
  bisect--helper: reimplement `bisect_skip` shell function in C
  bisect--helper: retire `--check-and-set-terms` subcommand

 builtin/bisect--helper.c | 179 ++++++++++++++++++++++++++++++++-------
 git-bisect.sh            |  58 +------------
 2 files changed, 151 insertions(+), 86 deletions(-)

-- 
2.29.2

[PATCH v4 1/7] bisect--helper: reimplement `bisect_log` shell function in C

[Miriam Rubio]

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>
---
 builtin/bisect--helper.c | 22 +++++++++++++++++++++-
 git-bisect.sh            |  7 +------
 2 files changed, 22 insertions(+), 7 deletions(-)

diff --git a/builtin/bisect--helper.c b/builtin/bisect--helper.c
index 709eb713a3..a65244a0f5 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_("list the bisection steps so far"), 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..05863cc142 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)
-- 
2.29.2

[PATCH v4 2/7] bisect--helper: reimplement `bisect_replay` shell function in C

[Miriam Rubio]

From: Pranit Bauva <pranit.bauva@gmail.com>

Reimplement the `bisect_replay` shell function in C and also add
`--bisect-replay` subcommand to `git bisect--helper` to call it from
git-bisect.sh

Using `--bisect-replay` 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>
---
 builtin/bisect--helper.c | 85 +++++++++++++++++++++++++++++++++++++++-
 git-bisect.sh            | 34 +---------------
 2 files changed, 85 insertions(+), 34 deletions(-)

diff --git a/builtin/bisect--helper.c b/builtin/bisect--helper.c
index a65244a0f5..d65b2f44c6 100644
--- a/builtin/bisect--helper.c
+++ b/builtin/bisect--helper.c
@@ -31,6 +31,7 @@ static const char * const git_bisect_helper_usage[] = {
 	N_("git bisect--helper --bisect-auto-next"),
 	N_("git bisect--helper --bisect-state (bad|new) [<rev>]"),
 	N_("git bisect--helper --bisect-state (good|old) [<rev>...]"),
+	N_("git bisect--helper --bisect-replay <filename>"),
 	NULL
 };
 
@@ -916,6 +917,79 @@ static enum bisect_error bisect_log(void)
 	return status ? BISECT_FAILED : BISECT_OK;
 }
 
+static int process_replay_line(struct bisect_terms *terms, struct strbuf *line)
+{
+	const char *p = line->buf + strspn(line->buf, " \t");
+	char *word_end, *rev;
+
+	if ((!skip_prefix(p, "git bisect", &p) &&
+	!skip_prefix(p, "git-bisect", &p)) || !isspace(*p))
+		return 0;
+	p += strspn(p, " \t");
+
+	word_end = (char *)p + strcspn(p, " \t");
+	rev = word_end + strspn(word_end, " \t");
+	*word_end = '\0'; /* NUL-terminate the word */
+
+	get_terms(terms);
+	if (check_and_set_terms(terms, p))
+		return -1;
+
+	if (!strcmp(p, "start")) {
+		struct strvec argv = STRVEC_INIT;
+		int res;
+		sq_dequote_to_strvec(rev, &argv);
+		res = bisect_start(terms, argv.v, argv.nr);
+		strvec_clear(&argv);
+		return res;
+	}
+
+	if (one_of(p, terms->term_good,
+	   terms->term_bad, "skip", NULL))
+		return bisect_write(p, rev, terms, 0);
+
+	if (!strcmp(p, "terms")) {
+		struct strvec argv = STRVEC_INIT;
+		int res;
+		sq_dequote_to_strvec(rev, &argv);
+		res = bisect_terms(terms, argv.nr == 1 ? argv.v[0] : NULL);
+		strvec_clear(&argv);
+		return res;
+	}
+	error(_("'%s'?? what are you talking about?"), p);
+
+	return -1;
+}
+
+static enum bisect_error bisect_replay(struct bisect_terms *terms, const char *filename)
+{
+	FILE *fp = NULL;
+	enum bisect_error res = BISECT_OK;
+	struct strbuf line = STRBUF_INIT;
+
+	if (is_empty_or_missing_file(filename))
+		return error(_("cannot read file '%s' for replaying"), filename);
+
+	if (bisect_reset(NULL))
+		return BISECT_FAILED;
+
+	fp = fopen(filename, "r");
+	if (!fp)
+		return BISECT_FAILED;
+
+	while ((strbuf_getline(&line, fp) != EOF) && !res){
+		res = process_replay_line(terms, &line);
+	}
+
+	strbuf_release(&line);
+	fclose(fp);
+
+	if (res)
+		return BISECT_FAILED;
+
+	return bisect_auto_next(terms, NULL);
+}
+
 int cmd_bisect__helper(int argc, const char **argv, const char *prefix)
 {
 	enum {
@@ -929,7 +1003,8 @@ int cmd_bisect__helper(int argc, const char **argv, const char *prefix)
 		BISECT_NEXT,
 		BISECT_AUTO_NEXT,
 		BISECT_STATE,
-		BISECT_LOG
+		BISECT_LOG,
+		BISECT_REPLAY
 	} cmdmode = 0;
 	int res = 0, nolog = 0;
 	struct option options[] = {
@@ -953,6 +1028,8 @@ int cmd_bisect__helper(int argc, const char **argv, const char *prefix)
 			 N_("mark the state of ref (or refs)"), BISECT_STATE),
 		OPT_CMDMODE(0, "bisect-log", &cmdmode,
 			 N_("list the bisection steps so far"), BISECT_LOG),
+		OPT_CMDMODE(0, "bisect-replay", &cmdmode,
+			 N_("replay the bisection process from the given file"), BISECT_REPLAY),
 		OPT_BOOL(0, "no-log", &nolog,
 			 N_("no log for BISECT_WRITE")),
 		OPT_END()
@@ -1020,6 +1097,12 @@ int cmd_bisect__helper(int argc, const char **argv, const char *prefix)
 			return error(_("--bisect-log requires 0 arguments"));
 		res = bisect_log();
 		break;
+	case BISECT_REPLAY:
+		if (argc != 1)
+			return error(_("no logfile given"));
+		set_terms(&terms, "bad", "good");
+		res = bisect_replay(&terms, argv[0]);
+		break;
 	default:
 		BUG("unknown subcommand %d", cmdmode);
 	}
diff --git a/git-bisect.sh b/git-bisect.sh
index 05863cc142..e834154e29 100755
--- a/git-bisect.sh
+++ b/git-bisect.sh
@@ -77,38 +77,6 @@ bisect_visualize() {
 	eval '"$@"' --bisect -- $(cat "$GIT_DIR/BISECT_NAMES")
 }
 
-bisect_replay () {
-	file="$1"
-	test "$#" -eq 1 || die "$(gettext "No logfile given")"
-	test -r "$file" || die "$(eval_gettext "cannot read \$file for replaying")"
-	git bisect--helper --bisect-reset || exit
-	oIFS="$IFS" IFS="$IFS$(printf '\015')"
-	while read git bisect command rev tail
-	do
-		test "$git $bisect" = "git bisect" || test "$git" = "git-bisect" || continue
-		if test "$git" = "git-bisect"
-		then
-			rev="$command"
-			command="$bisect"
-		fi
-		get_terms
-		git bisect--helper --check-and-set-terms "$command" "$TERM_GOOD" "$TERM_BAD" || exit
-		get_terms
-		case "$command" in
-		start)
-			eval "git bisect--helper --bisect-start $rev $tail" ;;
-		"$TERM_GOOD"|"$TERM_BAD"|skip)
-			git bisect--helper --bisect-write "$command" "$rev" "$TERM_GOOD" "$TERM_BAD" || exit;;
-		terms)
-			git bisect--helper --bisect-terms $rev || exit;;
-		*)
-			die "$(gettext "?? what are you talking about?")" ;;
-		esac
-	done <"$file"
-	IFS="$oIFS"
-	git bisect--helper --bisect-auto-next || exit
-}
-
 bisect_run () {
 	git bisect--helper --bisect-next-check $TERM_GOOD $TERM_BAD fail || exit
 
@@ -203,7 +171,7 @@ case "$#" in
 	reset)
 		git bisect--helper --bisect-reset "$@" ;;
 	replay)
-		bisect_replay "$@" ;;
+		git bisect--helper --bisect-replay "$@" || exit;;
 	log)
 		git bisect--helper --bisect-log || exit ;;
 	run)
-- 
2.29.2

[PATCH v4 3/7] bisect--helper: retire `--bisect-write` subcommand

[Miriam Rubio]

From: Pranit Bauva <pranit.bauva@gmail.com>

The `--bisect-write` subcommand is no longer used from the
git-bisect.sh shell script. Instead the function `bisect_write()`
is directly called from the C implementation.

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>
---
 builtin/bisect--helper.c | 9 ---------
 1 file changed, 9 deletions(-)

diff --git a/builtin/bisect--helper.c b/builtin/bisect--helper.c
index d65b2f44c6..0b16653f13 100644
--- a/builtin/bisect--helper.c
+++ b/builtin/bisect--helper.c
@@ -21,7 +21,6 @@ static GIT_PATH_FUNC(git_path_bisect_first_parent, "BISECT_FIRST_PARENT")
 
 static const char * const git_bisect_helper_usage[] = {
 	N_("git bisect--helper --bisect-reset [<commit>]"),
-	N_("git bisect--helper --bisect-write [--no-log] <state> <revision> <good_term> <bad_term>"),
 	N_("git bisect--helper --bisect-check-and-set-terms <command> <good_term> <bad_term>"),
 	N_("git bisect--helper --bisect-next-check <good_term> <bad_term> [<term>]"),
 	N_("git bisect--helper --bisect-terms [--term-good | --term-old | --term-bad | --term-new]"),
@@ -994,7 +993,6 @@ int cmd_bisect__helper(int argc, const char **argv, const char *prefix)
 {
 	enum {
 		BISECT_RESET = 1,
-		BISECT_WRITE,
 		CHECK_AND_SET_TERMS,
 		BISECT_NEXT_CHECK,
 		BISECT_TERMS,
@@ -1010,8 +1008,6 @@ int cmd_bisect__helper(int argc, const char **argv, const char *prefix)
 	struct option options[] = {
 		OPT_CMDMODE(0, "bisect-reset", &cmdmode,
 			 N_("reset the bisection state"), BISECT_RESET),
-		OPT_CMDMODE(0, "bisect-write", &cmdmode,
-			 N_("write out the bisection state in BISECT_LOG"), BISECT_WRITE),
 		OPT_CMDMODE(0, "check-and-set-terms", &cmdmode,
 			 N_("check and set terms in a bisection state"), CHECK_AND_SET_TERMS),
 		OPT_CMDMODE(0, "bisect-next-check", &cmdmode,
@@ -1048,11 +1044,6 @@ int cmd_bisect__helper(int argc, const char **argv, const char *prefix)
 		if (argc > 1)
 			return error(_("--bisect-reset requires either no argument or a commit"));
 		return !!bisect_reset(argc ? argv[0] : NULL);
-	case BISECT_WRITE:
-		if (argc != 4 && argc != 5)
-			return error(_("--bisect-write requires either 4 or 5 arguments"));
-		set_terms(&terms, argv[3], argv[2]);
-		res = bisect_write(argv[0], argv[1], &terms, nolog);
 		break;
 	case CHECK_AND_SET_TERMS:
 		if (argc != 3)
-- 
2.29.2

[PATCH v4 4/7] bisect--helper: use `res` instead of return in BISECT_RESET case option

[Miriam Rubio]

From: Pranit Bauva <pranit.bauva@gmail.com>

Use `res` variable to store `bisect_reset()` output in BISECT_RESET
case option to make bisect--helper.c more consistent.

Mentored-by: Christian Couder <chriscool@tuxfamily.org>
Signed-off-by: Miriam Rubio <mirucam@gmail.com>
---
 builtin/bisect--helper.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/builtin/bisect--helper.c b/builtin/bisect--helper.c
index 0b16653f13..8232e795fa 100644
--- a/builtin/bisect--helper.c
+++ b/builtin/bisect--helper.c
@@ -1043,7 +1043,7 @@ int cmd_bisect__helper(int argc, const char **argv, const char *prefix)
 	case BISECT_RESET:
 		if (argc > 1)
 			return error(_("--bisect-reset requires either no argument or a commit"));
-		return !!bisect_reset(argc ? argv[0] : NULL);
+		res = bisect_reset(argc ? argv[0] : NULL);
 		break;
 	case CHECK_AND_SET_TERMS:
 		if (argc != 3)
-- 
2.29.2

[PATCH v4 5/7] bisect--helper: retire `--bisect-auto-next` subcommand

[Miriam Rubio]

From: Pranit Bauva <pranit.bauva@gmail.com>

The --bisect-auto-next subcommand is no longer used from the
git-bisect.sh shell script. Instead the function bisect_auto_next()
is directly called from the C implementation.

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>
---
 builtin/bisect--helper.c | 10 ----------
 1 file changed, 10 deletions(-)

diff --git a/builtin/bisect--helper.c b/builtin/bisect--helper.c
index 8232e795fa..3672994c19 100644
--- a/builtin/bisect--helper.c
+++ b/builtin/bisect--helper.c
@@ -27,7 +27,6 @@ static const char * const git_bisect_helper_usage[] = {
 	N_("git bisect--helper --bisect-start [--term-{new,bad}=<term> --term-{old,good}=<term>]"
 					    " [--no-checkout] [--first-parent] [<bad> [<good>...]] [--] [<paths>...]"),
 	N_("git bisect--helper --bisect-next"),
-	N_("git bisect--helper --bisect-auto-next"),
 	N_("git bisect--helper --bisect-state (bad|new) [<rev>]"),
 	N_("git bisect--helper --bisect-state (good|old) [<rev>...]"),
 	N_("git bisect--helper --bisect-replay <filename>"),
@@ -999,7 +998,6 @@ int cmd_bisect__helper(int argc, const char **argv, const char *prefix)
 		BISECT_START,
 		BISECT_AUTOSTART,
 		BISECT_NEXT,
-		BISECT_AUTO_NEXT,
 		BISECT_STATE,
 		BISECT_LOG,
 		BISECT_REPLAY
@@ -1018,8 +1016,6 @@ int cmd_bisect__helper(int argc, const char **argv, const char *prefix)
 			 N_("start the bisect session"), BISECT_START),
 		OPT_CMDMODE(0, "bisect-next", &cmdmode,
 			 N_("find the next bisection commit"), BISECT_NEXT),
-		OPT_CMDMODE(0, "bisect-auto-next", &cmdmode,
-			 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,
@@ -1072,12 +1068,6 @@ int cmd_bisect__helper(int argc, const char **argv, const char *prefix)
 		get_terms(&terms);
 		res = bisect_next(&terms, prefix);
 		break;
-	case BISECT_AUTO_NEXT:
-		if (argc)
-			return error(_("--bisect-auto-next requires 0 arguments"));
-		get_terms(&terms);
-		res = bisect_auto_next(&terms, prefix);
-		break;
 	case BISECT_STATE:
 		set_terms(&terms, "bad", "good");
 		get_terms(&terms);
-- 
2.29.2

[PATCH v4 6/7] bisect--helper: reimplement `bisect_skip` shell function in C

[Miriam Rubio]

From: Pranit Bauva <pranit.bauva@gmail.com>

Reimplement the `bisect_skip()` shell function in C and also add
`bisect-skip` subcommand to `git bisect--helper` to call it from
git-bisect.sh

Using `--bisect-skip` 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>
---
 builtin/bisect--helper.c | 45 +++++++++++++++++++++++++++++++++++++++-
 git-bisect.sh            | 17 +--------------
 2 files changed, 45 insertions(+), 17 deletions(-)

diff --git a/builtin/bisect--helper.c b/builtin/bisect--helper.c
index 3672994c19..145e1a6998 100644
--- a/builtin/bisect--helper.c
+++ b/builtin/bisect--helper.c
@@ -30,6 +30,7 @@ static const char * const git_bisect_helper_usage[] = {
 	N_("git bisect--helper --bisect-state (bad|new) [<rev>]"),
 	N_("git bisect--helper --bisect-state (good|old) [<rev>...]"),
 	N_("git bisect--helper --bisect-replay <filename>"),
+	N_("git bisect--helper --bisect-skip [(<rev>|<range>)...]"),
 	NULL
 };
 
@@ -988,6 +989,41 @@ static enum bisect_error bisect_replay(struct bisect_terms *terms, const char *f
 	return bisect_auto_next(terms, NULL);
 }
 
+static enum bisect_error bisect_skip(struct bisect_terms *terms, const char **argv, int argc)
+{
+	int i;
+	enum bisect_error res;
+	struct strvec argv_state = STRVEC_INIT;
+
+	strvec_push(&argv_state, "skip");
+
+	for (i = 0; i < argc; i++) {
+		const char *dotdot = strstr(argv[i], "..");
+
+		if (dotdot) {
+			struct rev_info revs;
+			struct commit *commit;
+
+			init_revisions(&revs, NULL);
+			setup_revisions(2, argv + i - 1, &revs, NULL);
+
+			if (prepare_revision_walk(&revs))
+				die(_("revision walk setup failed\n"));
+			while ((commit = get_revision(&revs)) != NULL)
+				strvec_push(&argv_state,
+						oid_to_hex(&commit->object.oid));
+
+			reset_revision_walk();
+		} else {
+			strvec_push(&argv_state, argv[i]);
+		}
+	}
+	res = bisect_state(terms, argv_state.v, argv_state.nr);
+
+	strvec_clear(&argv_state);
+	return res;
+}
+
 int cmd_bisect__helper(int argc, const char **argv, const char *prefix)
 {
 	enum {
@@ -1000,7 +1036,8 @@ int cmd_bisect__helper(int argc, const char **argv, const char *prefix)
 		BISECT_NEXT,
 		BISECT_STATE,
 		BISECT_LOG,
-		BISECT_REPLAY
+		BISECT_REPLAY,
+		BISECT_SKIP
 	} cmdmode = 0;
 	int res = 0, nolog = 0;
 	struct option options[] = {
@@ -1022,6 +1059,8 @@ int cmd_bisect__helper(int argc, const char **argv, const char *prefix)
 			 N_("list the bisection steps so far"), BISECT_LOG),
 		OPT_CMDMODE(0, "bisect-replay", &cmdmode,
 			 N_("replay the bisection process from the given file"), BISECT_REPLAY),
+		OPT_CMDMODE(0, "bisect-skip", &cmdmode,
+			 N_("skip some commits for checkout"), BISECT_SKIP),
 		OPT_BOOL(0, "no-log", &nolog,
 			 N_("no log for BISECT_WRITE")),
 		OPT_END()
@@ -1084,6 +1123,10 @@ int cmd_bisect__helper(int argc, const char **argv, const char *prefix)
 		set_terms(&terms, "bad", "good");
 		res = bisect_replay(&terms, argv[0]);
 		break;
+	case BISECT_SKIP:
+		set_terms(&terms, "bad", "good");
+		res = bisect_skip(&terms, argv, argc);
+		break;
 	default:
 		BUG("unknown subcommand %d", cmdmode);
 	}
diff --git a/git-bisect.sh b/git-bisect.sh
index e834154e29..6a7afaea8d 100755
--- a/git-bisect.sh
+++ b/git-bisect.sh
@@ -39,21 +39,6 @@ _x40="$_x40$_x40$_x40$_x40$_x40$_x40$_x40$_x40"
 TERM_BAD=bad
 TERM_GOOD=good
 
-bisect_skip() {
-	all=''
-	for arg in "$@"
-	do
-		case "$arg" in
-		*..*)
-			revs=$(git rev-list "$arg") || die "$(eval_gettext "Bad rev input: \$arg")" ;;
-		*)
-			revs=$(git rev-parse --sq-quote "$arg") ;;
-		esac
-		all="$all $revs"
-	done
-	eval git bisect--helper --bisect-state 'skip' $all
-}
-
 bisect_visualize() {
 	git bisect--helper --bisect-next-check $TERM_GOOD $TERM_BAD fail || exit
 
@@ -162,7 +147,7 @@ case "$#" in
 	bad|good|new|old|"$TERM_BAD"|"$TERM_GOOD")
 		git bisect--helper --bisect-state "$cmd" "$@" ;;
 	skip)
-		bisect_skip "$@" ;;
+		git bisect--helper --bisect-skip "$@" || exit;;
 	next)
 		# Not sure we want "next" at the UI level anymore.
 		git bisect--helper --bisect-next "$@" || exit ;;
-- 
2.29.2

[PATCH v4 7/7] bisect--helper: retire `--check-and-set-terms` subcommand

[Miriam Rubio]

From: Pranit Bauva <pranit.bauva@gmail.com>

The `--check-and-set-terms` subcommand is no longer from the
git-bisect.sh shell script. Instead the function
`check_and_set_terms()` is called from the C implementation.

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>
---
 builtin/bisect--helper.c | 10 ----------
 1 file changed, 10 deletions(-)

diff --git a/builtin/bisect--helper.c b/builtin/bisect--helper.c
index 145e1a6998..e26d9baa18 100644
--- a/builtin/bisect--helper.c
+++ b/builtin/bisect--helper.c
@@ -21,7 +21,6 @@ static GIT_PATH_FUNC(git_path_bisect_first_parent, "BISECT_FIRST_PARENT")
 
 static const char * const git_bisect_helper_usage[] = {
 	N_("git bisect--helper --bisect-reset [<commit>]"),
-	N_("git bisect--helper --bisect-check-and-set-terms <command> <good_term> <bad_term>"),
 	N_("git bisect--helper --bisect-next-check <good_term> <bad_term> [<term>]"),
 	N_("git bisect--helper --bisect-terms [--term-good | --term-old | --term-bad | --term-new]"),
 	N_("git bisect--helper --bisect-start [--term-{new,bad}=<term> --term-{old,good}=<term>]"
@@ -1028,7 +1027,6 @@ int cmd_bisect__helper(int argc, const char **argv, const char *prefix)
 {
 	enum {
 		BISECT_RESET = 1,
-		CHECK_AND_SET_TERMS,
 		BISECT_NEXT_CHECK,
 		BISECT_TERMS,
 		BISECT_START,
@@ -1043,8 +1041,6 @@ int cmd_bisect__helper(int argc, const char **argv, const char *prefix)
 	struct option options[] = {
 		OPT_CMDMODE(0, "bisect-reset", &cmdmode,
 			 N_("reset the bisection state"), BISECT_RESET),
-		OPT_CMDMODE(0, "check-and-set-terms", &cmdmode,
-			 N_("check and set terms in a bisection state"), CHECK_AND_SET_TERMS),
 		OPT_CMDMODE(0, "bisect-next-check", &cmdmode,
 			 N_("check whether bad or good terms exist"), BISECT_NEXT_CHECK),
 		OPT_CMDMODE(0, "bisect-terms", &cmdmode,
@@ -1080,12 +1076,6 @@ int cmd_bisect__helper(int argc, const char **argv, const char *prefix)
 			return error(_("--bisect-reset requires either no argument or a commit"));
 		res = bisect_reset(argc ? argv[0] : NULL);
 		break;
-	case CHECK_AND_SET_TERMS:
-		if (argc != 3)
-			return error(_("--check-and-set-terms requires 3 arguments"));
-		set_terms(&terms, argv[2], argv[1]);
-		res = check_and_set_terms(&terms, argv[0]);
-		break;
 	case BISECT_NEXT_CHECK:
 		if (argc != 2 && argc != 3)
 			return error(_("--bisect-next-check requires 2 or 3 arguments"));
-- 
2.29.2

Re: [PATCH v4 0/7] Finish converting git bisect to C part 3

[Junio C Hamano]

Miriam Rubio <mirucam@gmail.com> writes:

> These patches correspond to a third part of patch series 
> of Outreachy project "Finish converting `git bisect` from shell to C" 
> started by Pranit Bauva and Tanushree Tumane
> (https://public-inbox.org/git/pull.117.git.gitgitgadget@gmail.com) and
> continued by me.

Did we lose [2/7] somewhere in the mailing list?  I see the same
thing as what is shown in

https://lore.kernel.org/git/20210125191710.45161-1-mirucam@gmail.com/

i.e. a 7-patch series that lack its second part.

Thanks.

Re: [PATCH v4 0/7] Finish converting git bisect to C part 3

[Miriam R.]

Hi Junio,

El mar, 26 ene 2021 a las 18:55, Junio C Hamano (<gitster@pobox.com>) escribió:
>
> Miriam Rubio <mirucam@gmail.com> writes:
>
> > These patches correspond to a third part of patch series
> > of Outreachy project "Finish converting `git bisect` from shell to C"
> > started by Pranit Bauva and Tanushree Tumane
> > (https://public-inbox.org/git/pull.117.git.gitgitgadget@gmail.com) and
> > continued by me.
>
> Did we lose [2/7] somewhere in the mailing list?  I see the same
> thing as what is shown in
>
> https://lore.kernel.org/git/20210125191710.45161-1-mirucam@gmail.com/
>
> i.e. a 7-patch series that lack its second part.

I received the [2/7] patch:

-------------------------------------------
De: Miriam Rubio <mirucam@gmail.com>
Date: lun, 25 ene 2021 a las 20:17
Subject: [PATCH v4 2/7] bisect--helper: reimplement `bisect_replay`
shell function in C
To: <git@vger.kernel.org>
Cc: Pranit Bauva <pranit.bauva@gmail.com>, Lars Schneider
<larsxschneider@gmail.com>, Christian Couder
<chriscool@tuxfamily.org>, Johannes Schindelin
<Johannes.Schindelin@gmx.de>, Tanushree Tumane
<tanushreetumane@gmail.com>, Miriam Rubio <mirucam@gmail.com>


From: Pranit Bauva <pranit.bauva@gmail.com>

Reimplement the `bisect_replay` shell function in C and also add
`--bisect-replay` subcommand to `git bisect--helper` to call it from
git-bisect.sh

Using `--bisect-replay` 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>
---
 builtin/bisect--helper.c | 85 +++++++++++++++++++++++++++++++++++++++-
 git-bisect.sh            | 34 +---------------
 2 files changed, 85 insertions(+), 34 deletions(-)

diff --git a/builtin/bisect--helper.c b/builtin/bisect--helper.c
index a65244a0f5..d65b2f44c6 100644
--- a/builtin/bisect--helper.c
+++ b/builtin/bisect--helper.c
@@ -31,6 +31,7 @@ static const char * const git_bisect_helper_usage[] = {
        N_("git bisect--helper --bisect-auto-next"),
        N_("git bisect--helper --bisect-state (bad|new) [<rev>]"),
        N_("git bisect--helper --bisect-state (good|old) [<rev>...]"),
+       N_("git bisect--helper --bisect-replay <filename>"),
        NULL
 };

@@ -916,6 +917,79 @@ static enum bisect_error bisect_log(void)
        return status ? BISECT_FAILED : BISECT_OK;
 }

+static int process_replay_line(struct bisect_terms *terms, struct strbuf *line)
+{
+       const char *p = line->buf + strspn(line->buf, " \t");
+       char *word_end, *rev;
+
+       if ((!skip_prefix(p, "git bisect", &p) &&
+       !skip_prefix(p, "git-bisect", &p)) || !isspace(*p))
+               return 0;
+       p += strspn(p, " \t");
+
+       word_end = (char *)p + strcspn(p, " \t");
+       rev = word_end + strspn(word_end, " \t");
+       *word_end = '\0'; /* NUL-terminate the word */
+
+       get_terms(terms);
+       if (check_and_set_terms(terms, p))
+               return -1;
+
+       if (!strcmp(p, "start")) {
+               struct strvec argv = STRVEC_INIT;
+               int res;
+               sq_dequote_to_strvec(rev, &argv);
+               res = bisect_start(terms, argv.v, argv.nr);
+               strvec_clear(&argv);
+               return res;
+       }
+
+       if (one_of(p, terms->term_good,
+          terms->term_bad, "skip", NULL))
+               return bisect_write(p, rev, terms, 0);
+
+       if (!strcmp(p, "terms")) {
+               struct strvec argv = STRVEC_INIT;
+               int res;
+               sq_dequote_to_strvec(rev, &argv);
+               res = bisect_terms(terms, argv.nr == 1 ? argv.v[0] : NULL);
+               strvec_clear(&argv);
+               return res;
+       }
+       error(_("'%s'?? what are you talking about?"), p);
+
+       return -1;
+}
+
+static enum bisect_error bisect_replay(struct bisect_terms *terms,
const char *filename)
+{
+       FILE *fp = NULL;
+       enum bisect_error res = BISECT_OK;
+       struct strbuf line = STRBUF_INIT;
+
+       if (is_empty_or_missing_file(filename))
+               return error(_("cannot read file '%s' for replaying"),
filename);
+
+       if (bisect_reset(NULL))
+               return BISECT_FAILED;
+
+       fp = fopen(filename, "r");
+       if (!fp)
+               return BISECT_FAILED;
+
+       while ((strbuf_getline(&line, fp) != EOF) && !res){
+               res = process_replay_line(terms, &line);
+       }
+
+       strbuf_release(&line);
+       fclose(fp);
+
+       if (res)
+               return BISECT_FAILED;
+
+       return bisect_auto_next(terms, NULL);
+}
+
 int cmd_bisect__helper(int argc, const char **argv, const char *prefix)
 {
        enum {
@@ -929,7 +1003,8 @@ int cmd_bisect__helper(int argc, const char
**argv, const char *prefix)
                BISECT_NEXT,
                BISECT_AUTO_NEXT,
                BISECT_STATE,
-               BISECT_LOG
+               BISECT_LOG,
+               BISECT_REPLAY
        } cmdmode = 0;
        int res = 0, nolog = 0;
        struct option options[] = {
@@ -953,6 +1028,8 @@ int cmd_bisect__helper(int argc, const char
**argv, const char *prefix)
                         N_("mark the state of ref (or refs)"), BISECT_STATE),
                OPT_CMDMODE(0, "bisect-log", &cmdmode,
                         N_("list the bisection steps so far"), BISECT_LOG),
+               OPT_CMDMODE(0, "bisect-replay", &cmdmode,
+                        N_("replay the bisection process from the
given file"), BISECT_REPLAY),
                OPT_BOOL(0, "no-log", &nolog,
                         N_("no log for BISECT_WRITE")),
                OPT_END()
@@ -1020,6 +1097,12 @@ int cmd_bisect__helper(int argc, const char
**argv, const char *prefix)
                        return error(_("--bisect-log requires 0 arguments"));
                res = bisect_log();
                break;
+       case BISECT_REPLAY:
+               if (argc != 1)
+                       return error(_("no logfile given"));
+               set_terms(&terms, "bad", "good");
+               res = bisect_replay(&terms, argv[0]);
+               break;
        default:
                BUG("unknown subcommand %d", cmdmode);
        }
diff --git a/git-bisect.sh b/git-bisect.sh
index 05863cc142..e834154e29 100755
--- a/git-bisect.sh
+++ b/git-bisect.sh
@@ -77,38 +77,6 @@ bisect_visualize() {
        eval '"$@"' --bisect -- $(cat "$GIT_DIR/BISECT_NAMES")
 }

-bisect_replay () {
-       file="$1"
-       test "$#" -eq 1 || die "$(gettext "No logfile given")"
-       test -r "$file" || die "$(eval_gettext "cannot read \$file for
replaying")"
-       git bisect--helper --bisect-reset || exit
-       oIFS="$IFS" IFS="$IFS$(printf '\015')"
-       while read git bisect command rev tail
-       do
-               test "$git $bisect" = "git bisect" || test "$git" =
"git-bisect" || continue
-               if test "$git" = "git-bisect"
-               then
-                       rev="$command"
-                       command="$bisect"
-               fi
-               get_terms
-               git bisect--helper --check-and-set-terms "$command"
"$TERM_GOOD" "$TERM_BAD" || exit
-               get_terms
-               case "$command" in
-               start)
-                       eval "git bisect--helper --bisect-start $rev $tail" ;;
-               "$TERM_GOOD"|"$TERM_BAD"|skip)
-                       git bisect--helper --bisect-write "$command"
"$rev" "$TERM_GOOD" "$TERM_BAD" || exit;;
-               terms)
-                       git bisect--helper --bisect-terms $rev || exit;;
-               *)
-                       die "$(gettext "?? what are you talking about?")" ;;
-               esac
-       done <"$file"
-       IFS="$oIFS"
-       git bisect--helper --bisect-auto-next || exit
-}
-
 bisect_run () {
        git bisect--helper --bisect-next-check $TERM_GOOD $TERM_BAD fail || exit

@@ -203,7 +171,7 @@ case "$#" in
        reset)
                git bisect--helper --bisect-reset "$@" ;;
        replay)
-               bisect_replay "$@" ;;
+               git bisect--helper --bisect-replay "$@" || exit;;
        log)
                git bisect--helper --bisect-log || exit ;;
        run)
--
2.29.2
>
I have to resend only this patch to the mailing list? or what I have to do?
Thank you.
Miriam.
> Thanks.

Re: [PATCH v4 0/7] Finish converting git bisect to C part 3

[Junio C Hamano]

> > Did we lose [2/7] somewhere in the mailing list?  I see the same
> > thing as what is shown in
> >
> > https://lore.kernel.org/git/20210125191710.45161-1-mirucam@gmail.com/
> >
> > i.e. a 7-patch series that lack its second part.
>
> I received the [2/7] patch:
>
> -------------------------------------------
> De: Miriam Rubio <mirucam@gmail.com>
> Date: lun, 25 ene 2021 a las 20:17
> Subject: [PATCH v4 2/7] bisect--helper: reimplement `bisect_replay`
> shell function in C
> To: <git@vger.kernel.org>
> Cc: Pranit Bauva <pranit.bauva@gmail.com>, Lars Schneider
> <larsxschneider@gmail.com>, Christian Couder
> <chriscool@tuxfamily.org>, Johannes Schindelin
> <Johannes.Schindelin@gmx.de>, Tanushree Tumane
> <tanushreetumane@gmail.com>, Miriam Rubio <mirucam@gmail.com>

It does not count---you are on the direct path of the message
and are not relying on the list to relay it back to you.

It seems that today's one of those days that the mailing list is
hiccupping. The message I am responding to, which is sent both
to the list and directly to me, hasn't appeared either on the public-inbox
or the lore list archive, either.

Hopefully we'll see all 7 messages eventually ;-).

Re: [PATCH v4 0/7] Finish converting git bisect to C part 3

[Junio C Hamano]

Junio C Hamano <gitster@pobox.com> writes:

>> > Did we lose [2/7] somewhere in the mailing list?  I see the same
>> > thing as what is shown in
>> >
>> > https://lore.kernel.org/git/20210125191710.45161-1-mirucam@gmail.com/
>> >
>> > i.e. a 7-patch series that lack its second part.
>>
>> I received the [2/7] patch:
>>
>> -------------------------------------------
>> De: Miriam Rubio <mirucam@gmail.com>
>> Date: lun, 25 ene 2021 a las 20:17
>> Subject: [PATCH v4 2/7] bisect--helper: reimplement `bisect_replay`
>> shell function in C
>> To: <git@vger.kernel.org>
>> Cc: Pranit Bauva <pranit.bauva@gmail.com>, Lars Schneider
>> <larsxschneider@gmail.com>, Christian Couder
>> <chriscool@tuxfamily.org>, Johannes Schindelin
>> <Johannes.Schindelin@gmx.de>, Tanushree Tumane
>> <tanushreetumane@gmail.com>, Miriam Rubio <mirucam@gmail.com>
>
> It does not count---you are on the direct path of the message
> and are not relying on the list to relay it back to you.
>
> It seems that today's one of those days that the mailing list is
> hiccupping. The message I am responding to, which is sent both
> to the list and directly to me, hasn't appeared either on the public-inbox
> or the lore list archive, either.
>
> Hopefully we'll see all 7 messages eventually ;-).

Now I am seeing it on the lore NNTP server (so presumably it has
been relayed to list subscribers).

Re: [PATCH v4 0/7] Finish converting git bisect to C part 3

[Miriam R.]

Hi Junio,

El mar, 26 ene 2021 a las 23:32, Junio C Hamano (<gitster@pobox.com>) escribió:
>
> Junio C Hamano <gitster@pobox.com> writes:
>
> >> > Did we lose [2/7] somewhere in the mailing list?  I see the same
> >> > thing as what is shown in
> >> >
> >> > https://lore.kernel.org/git/20210125191710.45161-1-mirucam@gmail.com/
> >> >
> >> > i.e. a 7-patch series that lack its second part.
> >>
> >> I received the [2/7] patch:
> >>
> >> -------------------------------------------
> >> De: Miriam Rubio <mirucam@gmail.com>
> >> Date: lun, 25 ene 2021 a las 20:17
> >> Subject: [PATCH v4 2/7] bisect--helper: reimplement `bisect_replay`
> >> shell function in C
> >> To: <git@vger.kernel.org>
> >> Cc: Pranit Bauva <pranit.bauva@gmail.com>, Lars Schneider
> >> <larsxschneider@gmail.com>, Christian Couder
> >> <chriscool@tuxfamily.org>, Johannes Schindelin
> >> <Johannes.Schindelin@gmx.de>, Tanushree Tumane
> >> <tanushreetumane@gmail.com>, Miriam Rubio <mirucam@gmail.com>
> >
> > It does not count---you are on the direct path of the message
> > and are not relying on the list to relay it back to you.
> >
> > It seems that today's one of those days that the mailing list is
> > hiccupping. The message I am responding to, which is sent both
> > to the list and directly to me, hasn't appeared either on the public-inbox
> > or the lore list archive, either.
> >
> > Hopefully we'll see all 7 messages eventually ;-).
>
> Now I am seeing it on the lore NNTP server (so presumably it has
> been relayed to list subscribers).
Ok, perfect!
Best,
Miriam
>

Re: [PATCH v4 7/7] bisect--helper: retire `--check-and-set-terms` subcommand

[Junio C Hamano]

Miriam Rubio <mirucam@gmail.com> writes:

> From: Pranit Bauva <pranit.bauva@gmail.com>
>
> The `--check-and-set-terms` subcommand is no longer from the
> git-bisect.sh shell script. Instead the function
> `check_and_set_terms()` is called from the C implementation.

Nice to see these intermediate state going away.

Reviewers and mentors, how close do you folks think these patches
are to the completion?  It looks quite good to me (admittedly I gave
only a cursory look to these recent iterations).

Thanks.

Re: [PATCH v4 2/7] bisect--helper: reimplement `bisect_replay` shell function in C

[Christian Couder]

On Mon, Jan 25, 2021 at 8:17 PM Miriam Rubio <mirucam@gmail.com> wrote:

> +       while ((strbuf_getline(&line, fp) != EOF) && !res){

It looks like a space char is missing between ")" and "{"...

> +               res = process_replay_line(terms, &line);
> +       }

...but as there is only one line in the {...}, we could just get rid
of the block like this:

       while ((strbuf_getline(&line, fp) != EOF) && !res)
               res = process_replay_line(terms, &line);

Best,
Christian.

Re: [PATCH v4 3/7] bisect--helper: retire `--bisect-write` subcommand

[Christian Couder]

On Mon, Jan 25, 2021 at 8:17 PM Miriam Rubio <mirucam@gmail.com> wrote:

> @@ -1048,11 +1044,6 @@ int cmd_bisect__helper(int argc, const char **argv, const char *prefix)
>                 if (argc > 1)
>                         return error(_("--bisect-reset requires either no argument or a commit"));
>                 return !!bisect_reset(argc ? argv[0] : NULL);

Here we are returning so we never fall back into the BISECT_WRITE case below...

> -       case BISECT_WRITE:
> -               if (argc != 4 && argc != 5)
> -                       return error(_("--bisect-write requires either 4 or 5 arguments"));
> -               set_terms(&terms, argv[3], argv[2]);
> -               res = bisect_write(argv[0], argv[1], &terms, nolog);
>                 break;

...so the above "break;" could be removed too.

>         case CHECK_AND_SET_TERMS:
>                 if (argc != 3)

Re: [PATCH v4 4/7] bisect--helper: use `res` instead of return in BISECT_RESET case option

[Christian Couder]

On Mon, Jan 25, 2021 at 8:17 PM Miriam Rubio <mirucam@gmail.com> wrote:

> @@ -1043,7 +1043,7 @@ int cmd_bisect__helper(int argc, const char **argv, const char *prefix)
>         case BISECT_RESET:
>                 if (argc > 1)
>                         return error(_("--bisect-reset requires either no argument or a commit"));
> -               return !!bisect_reset(argc ? argv[0] : NULL);
> +               res = bisect_reset(argc ? argv[0] : NULL);
>                 break;

This "break;" was not necessary before but it becomes necessary when
the above "return ..." is replaced with something else.

>         case CHECK_AND_SET_TERMS:
>                 if (argc != 3)

Re: [PATCH v4 1/7] bisect--helper: reimplement `bisect_log` shell function in C

[Đoàn Trần Công Danh]

> +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;

The old code will write "We are not bisection." to stderr when, well,
we're not bisecting. This message suggested an alternative
 https://lore.kernel.org/git/gohp6kv9bml9qc.fsf@gmail.com

> +}
> +
>  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_("list the bisection steps so far"), 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..05863cc142 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 ;;

The original code was "die" when no bisect_log available.

I think we need to "exit 1" here to indicate a failure, i.e.

	git bisect--helper --bisect-log || exit 1 ;;

>  	run)
>  		bisect_run "$@" ;;
>  	terms)
> -- 
> 2.29.2
> 

-- 
Danh

Re: [PATCH v4 1/7] bisect--helper: reimplement `bisect_log` shell function in C

[Christian Couder]

On Sat, Jan 30, 2021 at 2:46 AM Đoàn Trần Công Danh
<congdanhqx@gmail.com> wrote:

> > @@ -210,7 +205,7 @@ case "$#" in
> >       replay)
> >               bisect_replay "$@" ;;
> >       log)
> > -             bisect_log ;;
> > +             git bisect--helper --bisect-log || exit ;;
>
> The original code was "die" when no bisect_log available.
>
> I think we need to "exit 1" here to indicate a failure, i.e.
>
>         git bisect--helper --bisect-log || exit 1 ;;

Actually:

             git bisect--helper --bisect-log || exit ;;

will indicate failure. See:

$ ( false || exit )
$ echo $?
1

The difference with what you suggest is that the exit code will be the
same instead of always 1:

$ ( ( exit 4 ) || exit )
echo $?
4

which is a good thing, as die() exits with code 128, so if `git
bisect--helper --bisect-log` die()s then we will get a 128 exit code
with `|| exit` instead of 1 with `|| exit 1`.

Thanks for your review,
Christian.