diff mbox series

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

Message ID 20201221162743.96056-7-mirucam@gmail.com (mailing list archive)
State New, archived
Headers show
Series Finish converting git bisect to C part 3 | expand

Commit Message

Miriam R. Dec. 21, 2020, 4:27 p.m. UTC 
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 | 47 +++++++++++++++++++++++++++++++++++++++-
 git-bisect.sh            | 17 +--------------
 2 files changed, 47 insertions(+), 17 deletions(-)

Comments

Johannes Schindelin Jan. 18, 2021, 4:14 p.m. UTC | #1 
Hi Miriam,

On Mon, 21 Dec 2020, Miriam Rubio wrote:

> diff --git a/builtin/bisect--helper.c b/builtin/bisect--helper.c
> index d570a165de..1a6c75183a 100644
> --- a/builtin/bisect--helper.c
> +++ b/builtin/bisect--helper.c
> @@ -1030,6 +1031,43 @@ 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;
> +	const char *pattern = "*..*";
> +	struct strvec argv_state = STRVEC_INIT;
> +
> +	strvec_push(&argv_state, "skip");
> +
> +	for (i = 0; i < argc; i++) {
> +		if (!wildmatch(pattern, argv[i], 0)) {

Better use `strstr()`:

		const char *dotdot = strstr(argv[i], "..");

		if (dotdot) {
			[...]
		}

> +			struct rev_info revs;
> +			struct commit *commit;
> +			struct strvec rev_argv = STRVEC_INIT;
> +
> +			strvec_pushl(&rev_argv, "skipped_commits", argv[i], NULL);
> +			init_revisions(&revs, NULL);
> +			setup_revisions(rev_argv.nr, rev_argv.v, &revs, NULL);
> +			strvec_clear(&rev_argv);

Since the first argument passed to `setup_revisions()` is always ignored,
you can avoid all that `strvec` business altogether thusly:

			setup_revisions(2, argv + i - 1, &revs, NULL);

Other than that, this patch looks good to me.

Ciao,
Dscho

> +
> +			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 {
> @@ -1042,7 +1080,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[] = {
> @@ -1064,6 +1103,8 @@ int cmd_bisect__helper(int argc, const char **argv, const char *prefix)
>  			 N_("output the contents of BISECT_LOG"), 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()
> @@ -1126,6 +1167,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
>
>
diff mbox series

Patch

diff --git a/builtin/bisect--helper.c b/builtin/bisect--helper.c
index d570a165de..1a6c75183a 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
 };
 
@@ -1030,6 +1031,43 @@  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;
+	const char *pattern = "*..*";
+	struct strvec argv_state = STRVEC_INIT;
+
+	strvec_push(&argv_state, "skip");
+
+	for (i = 0; i < argc; i++) {
+		if (!wildmatch(pattern, argv[i], 0)) {
+			struct rev_info revs;
+			struct commit *commit;
+			struct strvec rev_argv = STRVEC_INIT;
+
+			strvec_pushl(&rev_argv, "skipped_commits", argv[i], NULL);
+			init_revisions(&revs, NULL);
+			setup_revisions(rev_argv.nr, rev_argv.v, &revs, NULL);
+			strvec_clear(&rev_argv);
+
+			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 {
@@ -1042,7 +1080,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[] = {
@@ -1064,6 +1103,8 @@  int cmd_bisect__helper(int argc, const char **argv, const char *prefix)
 			 N_("output the contents of BISECT_LOG"), 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()
@@ -1126,6 +1167,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 ;;