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

[PATCH v3 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_v2.

I would like to thank Rafael Silva and Johannes Schindelin for their
reviews and suggestions.

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

[1/7] bisect--helper: reimplement `bisect_log` shell function in C
* Change command description message.
---

[2/7] bisect--helper: reimplement `bisect_replay` shell function in C
* Remove get_next_word().
* Change process_replay_line()'s while loop implementation.
* Change error message.
---

[6/7] bisect--helper: reimplement `bisect_skip` shell function in C
* Use strstr() instead of wildmatch.
* Remove strvec use.
---

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 | 178 ++++++++++++++++++++++++++++++++-------
 git-bisect.sh            |  58 +------------
 2 files changed, 150 insertions(+), 86 deletions(-)

-- 
2.29.2

[PATCH v3 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..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)
-- 
2.29.2

[PATCH v3 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 | 84 +++++++++++++++++++++++++++++++++++++++-
 git-bisect.sh            | 34 +---------------
 2 files changed, 84 insertions(+), 34 deletions(-)

diff --git a/builtin/bisect--helper.c b/builtin/bisect--helper.c
index a65244a0f5..4d8dca3717 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,78 @@ 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");
+
+	if ((!skip_prefix(p, "git bisect", &p) &&
+	!skip_prefix(p, "git-bisect", &p)) || !isspace(*p))
+		return 0;
+	p += strspn(p, " \t");
+
+	char *word_end = (char*)p + strcspn(p, " \t");
+	char *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 +1002,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 +1027,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 +1096,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 c6149846ff..79bcd31bd7 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 v3 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 4d8dca3717..1f731167f6 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]"),
@@ -993,7 +992,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,
@@ -1009,8 +1007,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,
@@ -1047,11 +1043,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 v3 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 1f731167f6..1ce0399ad4 100644
--- a/builtin/bisect--helper.c
+++ b/builtin/bisect--helper.c
@@ -1042,7 +1042,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 v3 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 1ce0399ad4..1e53845e69 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>"),
@@ -998,7 +997,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
@@ -1017,8 +1015,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,
@@ -1071,12 +1067,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 v3 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 1e53845e69..61ddaa6b9c 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
 };
 
@@ -987,6 +988,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 {
@@ -999,7 +1035,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[] = {
@@ -1021,6 +1058,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()
@@ -1083,6 +1122,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 79bcd31bd7..016cc34e03 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 v3 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 61ddaa6b9c..3b0a6a7d0c 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>]"
@@ -1027,7 +1026,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,
@@ -1042,8 +1040,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,
@@ -1079,12 +1075,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 v3 2/7] bisect--helper: reimplement `bisect_replay` shell function in C

[Junio C Hamano]

Miriam Rubio <mirucam@gmail.com> writes:

> 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
> ...
> +static int process_replay_line(struct bisect_terms *terms, struct strbuf *line)
> +{
> +	const char *p = line->buf + strspn(line->buf, " \t");
> +
> +	if ((!skip_prefix(p, "git bisect", &p) &&
> +	!skip_prefix(p, "git-bisect", &p)) || !isspace(*p))
> +		return 0;
> +	p += strspn(p, " \t");
> +
> +	char *word_end = (char*)p + strcspn(p, " \t");
> +	char *rev = word_end + strspn(word_end, " \t");

Are these lines new in this round?

These break builds with -Werror=declaration-after-statement.

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

[Junio C Hamano]

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

> Miriam Rubio <mirucam@gmail.com> writes:
>
>> 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
>> ...
>> +static int process_replay_line(struct bisect_terms *terms, struct strbuf *line)
>> +{
>> +	const char *p = line->buf + strspn(line->buf, " \t");
>> +
>> +	if ((!skip_prefix(p, "git bisect", &p) &&
>> +	!skip_prefix(p, "git-bisect", &p)) || !isspace(*p))
>> +		return 0;
>> +	p += strspn(p, " \t");
>> +
>> +	char *word_end = (char*)p + strcspn(p, " \t");
>> +	char *rev = word_end + strspn(word_end, " \t");
>
> Are these lines new in this round?
>
> These break builds with -Werror=declaration-after-statement.


The following fix-up is tentatively queued at the tip of the topic
in tonight's pushout.  There might be better way to do what this
function is doing (especially I am skeptical about the way the code
casts const away), so please don't blindly trust it, but at least it
should please the compiler.

(char*) should be spelled (char *), by the way.

Thanks.


diff --git a/builtin/bisect--helper.c b/builtin/bisect--helper.c
index 3b0a6a7d0c..c673f87795 100644
--- a/builtin/bisect--helper.c
+++ b/builtin/bisect--helper.c
@@ -918,14 +918,15 @@ static enum bisect_error bisect_log(void)
 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");
 
-	char *word_end = (char*)p + strcspn(p, " \t");
-	char *rev = word_end + strspn(word_end, " \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);
-- 
2.30.0-492-gde1138eff7

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

[Rafael Silva]

Nice work on this series.

I have one comment on this series regarding a behavior diff between the
C and shell version, and small comment about style, see below.

Miriam Rubio writes:

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

In the shell version, when we are not bisecting it the `git bisect log`
command will `die` with the text "We are not bisecting." which state to
the user that a bisect is not yet started. The shell version does that
by checking if the `$GIT_DIR/BISECT_LOG` file doesn't exists or it's
an empty file as the following code snippet copied from the shell
version that is remove by this patch:

   test -s "$GIT_DIR/BISECT_LOG" || die "$(gettext "We are not bisecting.")"

This seems to be "missing" from the new C version implemented by this
patch and perhaps we should add it. I'm not sure whether this change was
intentional and I'm missing some context here of why we are dropping
the message, if that's the case I apologize in advance. But, IMHO
outputting the error message provides a better user experience as it
would be obvious that the user forgot to `git bisect start` instead of
silently failing.

With that said, perhaps an obvious way of implementing is to use
`is_empty_or_missing_file()`, much like `bisect_replay()` does it in the
[2/7] patch on this series, and return the same error message from
the shell version:

-- >8 --
diff --git a/builtin/bisect--helper.c b/builtin/bisect--helper.c
index a65244a0f5..ce11383125 100644
--- a/builtin/bisect--helper.c
+++ b/builtin/bisect--helper.c
@@ -907,7 +907,12 @@ static enum bisect_error bisect_state(struct bisect_terms *terms, const char **a
 static enum bisect_error bisect_log(void)
 {
        int fd, status;
-       fd = open(git_path_bisect_log(), O_RDONLY);
+       const char* filename = git_path_bisect_log();
+
+       if (is_empty_or_missing_file(filename))
+               return error(_("We are not bisecting."));
+
+       fd = open(filename, O_RDONLY);
        if (fd < 0)
                return BISECT_FAILED;
-- >8 --

Although I compiled and did small test on the above code snippet, don't
trust it blindly and perform your own test and judge whether this is the
best way to implement this shortcoming.

>
> @@ -210,7 +205,7 @@ case "$#" in
>  	replay)
>  		bisect_replay "$@" ;;
>  	log)
> -		bisect_log ;;
> +		git bisect--helper --bisect-log || exit;;

Style: just a minor change to add a space between `exit` and `;;`.

-- 
Thanks
Rafael

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

[Miriam R.]

Hi Junio,

El dom, 24 ene 2021 a las 1:22, Junio C Hamano (<gitster@pobox.com>) escribió:
>
> Miriam Rubio <mirucam@gmail.com> writes:
>
> > 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
> > ...
> > +static int process_replay_line(struct bisect_terms *terms, struct strbuf *line)
> > +{
> > +     const char *p = line->buf + strspn(line->buf, " \t");
> > +
> > +     if ((!skip_prefix(p, "git bisect", &p) &&
> > +     !skip_prefix(p, "git-bisect", &p)) || !isspace(*p))
> > +             return 0;
> > +     p += strspn(p, " \t");
> > +
> > +     char *word_end = (char*)p + strcspn(p, " \t");
> > +     char *rev = word_end + strspn(word_end, " \t");
>
> Are these lines new in this round?
>
Yes, they are a reviewer's suggestion.

> These break builds with -Werror=declaration-after-statement.
>
Sorry about this, my compiler sometimes does not alert with some -Werror.
I have just sent a new version.
Best,
Miriam

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

[Miriam R.]

Hi Rafael,

El dom, 24 ene 2021 a las 14:56, Rafael Silva
(<rafaeloliveira.cs@gmail.com>) escribió:
>
>
> Nice work on this series.
>
> I have one comment on this series regarding a behavior diff between the
> C and shell version, and small comment about style, see below.
>
> Miriam Rubio writes:
>
> > 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;
> > +}
> > +
>
> In the shell version, when we are not bisecting it the `git bisect log`
> command will `die` with the text "We are not bisecting." which state to
> the user that a bisect is not yet started. The shell version does that
> by checking if the `$GIT_DIR/BISECT_LOG` file doesn't exists or it's
> an empty file as the following code snippet copied from the shell
> version that is remove by this patch:
>
>    test -s "$GIT_DIR/BISECT_LOG" || die "$(gettext "We are not bisecting.")"
>
> This seems to be "missing" from the new C version implemented by this
> patch and perhaps we should add it. I'm not sure whether this change was
> intentional and I'm missing some context here of why we are dropping
> the message, if that's the case I apologize in advance. But, IMHO
> outputting the error message provides a better user experience as it
> would be obvious that the user forgot to `git bisect start` instead of
> silently failing.
>
> With that said, perhaps an obvious way of implementing is to use
> `is_empty_or_missing_file()`, much like `bisect_replay()` does it in the
> [2/7] patch on this series, and return the same error message from
> the shell version:
>
> -- >8 --
> diff --git a/builtin/bisect--helper.c b/builtin/bisect--helper.c
> index a65244a0f5..ce11383125 100644
> --- a/builtin/bisect--helper.c
> +++ b/builtin/bisect--helper.c
> @@ -907,7 +907,12 @@ static enum bisect_error bisect_state(struct bisect_terms *terms, const char **a
>  static enum bisect_error bisect_log(void)
>  {
>         int fd, status;
> -       fd = open(git_path_bisect_log(), O_RDONLY);
> +       const char* filename = git_path_bisect_log();
> +
> +       if (is_empty_or_missing_file(filename))
> +               return error(_("We are not bisecting."));
> +
> +       fd = open(filename, O_RDONLY);
>         if (fd < 0)
>                 return BISECT_FAILED;
> -- >8 --
>
> Although I compiled and did small test on the above code snippet, don't
> trust it blindly and perform your own test and judge whether this is the
> best way to implement this shortcoming.
Ok, thank you.
I am not the original author of this subcommand reimplementation and I
don't know if there is a reason
 for the difference with the error message. Maybe we can wait for some
other reviewers opinion.
>
> >
> > @@ -210,7 +205,7 @@ case "$#" in
> >       replay)
> >               bisect_replay "$@" ;;
> >       log)
> > -             bisect_log ;;
> > +             git bisect--helper --bisect-log || exit;;
>
> Style: just a minor change to add a space between `exit` and `;;`.
Noted.
>
> --
> Thanks
> Rafael
Thank you for your suggestions.
Best,
Miriam

Re: [PATCH v3 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.
>
> 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_v2.

Unfortunately, with this series in the 'seen' branch, the CI jobs
seem to fail.

cf.
https://github.com/git/git/runs/1766239699?check_suite_focus=true#step:5:814

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

[Miriam R.]

Hi Junio,

El mar, 26 ene 2021 a las 2:58, 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.
> >
> > 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_v2.
>
> Unfortunately, with this series in the 'seen' branch, the CI jobs
> seem to fail.

With the last patch series sent:

https://public-inbox.org/git/20210125191710.45161-1-mirucam@gmail.com/
(branch: https://gitlab.com/mirucam/git/commits/git-bisect-work-part3_v3)

all tests pass (executed with: prove -j5 --shuffle t[0-9]*.sh):
--------
All tests successful.
Files=916, Tests=22661, 1628 wallclock secs ( 7.93 usr  3.16 sys +
1106.15 cusr 1387.58 csys = 2504.82 CPU)
Result: PASS
----------
>
> cf.
> https://github.com/git/git/runs/1766239699?check_suite_focus=true#step:5:814

I have seen that is "t3415-rebase-autosquash.sh" the test that fails in CI jobs.

I have also executed it independently with no errors:
./t3415-rebase-autosquash.sh
ok 1 - setup
ok 2 - auto fixup (option)
ok 3 - auto fixup (config)
ok 4 - auto squash (option)
ok 5 - auto squash (config)
ok 6 - misspelled auto squash
ok 7 - auto squash that matches 2 commits
ok 8 - auto squash that matches a commit after the squash
ok 9 - auto squash that matches a sha1
ok 10 - auto squash that matches longer sha1
ok 11 - use commit --fixup
ok 12 - use commit --squash
ok 13 - fixup! fixup!
ok 14 - fixup! squash!
ok 15 - squash! squash!
ok 16 - squash! fixup!
ok 17 - autosquash with custom inst format
ok 18 - autosquash with empty custom instructionFormat
ok 19 - autosquash with multiple empty patches
ok 20 - extra spaces after fixup!
ok 21 - wrapped original subject
ok 22 - abort last squash
ok 23 - fixup a fixup
# passed all 23 test(s)
1..23

If I am missing something or I should do any other test please tell me,
thank you,
Miriam.

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

[Junio C Hamano]

"Miriam R." <mirucam@gmail.com> writes:

>> Although I compiled and did small test on the above code snippet, don't
>> trust it blindly and perform your own test and judge whether this is the
>> best way to implement this shortcoming.
> Ok, thank you.
> I am not the original author of this subcommand reimplementation
> and I don't know if there is a reason for the difference with the
> error message. Maybe we can wait for some other reviewers opinion.

Sorry I missed this thread.

My understanding is that this topic is an attempt to "reimplement"
what is there in the scripted version, so any deviation of behaviour
obserbable from outside, which is *not* justified, should by
definition be treated as a bug.

If the original author did not explain why the behaviour difference
exists and defend why the new behaviour in the reimplementation is
better, and if you do not think of a good reason why the behaviour
should be different and the new behaviour is better, then let's
treat it in a bug and fix it.

Thanks.

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

[Junio C Hamano]

"Miriam R." <mirucam@gmail.com> writes:

>> cf.
>> https://github.com/git/git/runs/1766239699?check_suite_focus=true#step:5:814
>
> I have seen that is "t3415-rebase-autosquash.sh" the test that fails in CI jobs.

Sorry, a false alarm.  This I think was caused by another topic,
cm/rebase-i, and not the bisect topic.

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

[Miriam R.]

Hi,

El mar, 26 ene 2021 a las 19:32, Junio C Hamano (<gitster@pobox.com>) escribió:
>
> "Miriam R." <mirucam@gmail.com> writes:
>
> >> Although I compiled and did small test on the above code snippet, don't
> >> trust it blindly and perform your own test and judge whether this is the
> >> best way to implement this shortcoming.
> > Ok, thank you.
> > I am not the original author of this subcommand reimplementation
> > and I don't know if there is a reason for the difference with the
> > error message. Maybe we can wait for some other reviewers opinion.
>
> Sorry I missed this thread.
>
> My understanding is that this topic is an attempt to "reimplement"
> what is there in the scripted version, so any deviation of behaviour
> obserbable from outside, which is *not* justified, should by
> definition be treated as a bug.
>
> If the original author did not explain why the behaviour difference
> exists and defend why the new behaviour in the reimplementation is
> better, and if you do not think of a good reason why the behaviour
> should be different and the new behaviour is better, then let's
> treat it in a bug and fix it.
>
Ok, I will send another patch series adding this.
Thank you.
Miriam.
> Thanks.

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

[Miriam R.]

El mar, 26 ene 2021 a las 20:12, Junio C Hamano (<gitster@pobox.com>) escribió:
>
> "Miriam R." <mirucam@gmail.com> writes:
>
> >> cf.
> >> https://github.com/git/git/runs/1766239699?check_suite_focus=true#step:5:814
> >
> > I have seen that is "t3415-rebase-autosquash.sh" the test that fails in CI jobs.
>
> Sorry, a false alarm.  This I think was caused by another topic,
> cm/rebase-i, and not the bisect topic.
>
Ok.
Best,
Miriam.