Re: [PATCH v14 07/27] bisect--helper: `bisect_reset` shell function in C
- From
Junio C Hamano <gitster@pobox.com>
- Date
- Aug 24, 2016, 21:12 UTC
- Message-ID
- <xmqq4m69q3b2.fsf@gitster.mtv.corp.google.com>
- In-Reply-To
- <01020156b73fe69a-13136cfb-4daa-4f5d-9b56-537adf2c6942-000000@eu-west-1.amazonses.com>
Pranit Bauva <pranit.bauva@gmail.com> writes:
Show 6 quoted lines
> +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.
Show 8 quoted lines
> + 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.
Show 12 quoted lines
> @@ -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