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

["Đoàn Trần Công Danh" <congdanhqx@gmail.com>]

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