Re: [PATCH v14 07/27] bisect--helper: `bisect_reset` shell function in C

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Pranit Bauva <pranit.bauva@xxxxxxxxx> writes:

> +static int bisect_reset(const char *commit)
> +{
> +	struct strbuf branch = STRBUF_INIT;
> +
> +	if (!commit) {
> +		if (strbuf_read_file(&branch, git_path_bisect_start(), 0) < 1) {

Hmm, tricky but correct to do the "< 1" comparison.  If the file
does not exist, you'd get -1; if it fails to read it, you'd get -1;
if it turns out to be empty, you'd get 0.

> +			printf("We are not bisecting.\n");
> +			return 0;
> +		}
> +		strbuf_rtrim(&branch);
> +	} else {
> +		struct object_id oid;
> +		if (get_oid(commit, &oid))
> +			return error(_("'%s' is not a valid commit"), commit);

The original is

	rev-parse --quiet --verify "$1^{commit}"

Unless the caller of this function already appended "^{commit}" to
whatever the user gave "bisect--helper bisect-reset", this
conversion is not equivalent.  If you said

    git bisect reset HEAD:

get_oid() would tell you that the top-level tree object of the
current commit exists in the object store, but the original is
meant to catch "That's not a commit, it's a tree!" before attempting
to run "git checkout" on it.

I think get_sha1_committish() is what you want to use here.

> +		strbuf_addstr(&branch, commit);
> +	}

Also this version fails to catch "bisect reset a b c" as an error, I
suspect.

> @@ -627,7 +603,7 @@ case "$#" in
>  	visualize|view)
>  		bisect_visualize "$@" ;;
>  	reset)
> -		bisect_reset "$@" ;;
> +		git bisect--helper --bisect-reset "$@" ;;
>  	replay)
>  		bisect_replay "$@" ;;
>  	log)
>
> --
> https://github.com/git/git/pull/287
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html



[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]