Re: [PATCH 1/2] bisect: parse revs before passing them to check_expected_revs()
- From
Junio C Hamano <gitster@pobox.com>
- Date
- Dec 29, 2014, 19:33 UTC
- Message-ID
- <xmqqegriz1wi.fsf@gitster.dls.corp.google.com>
- In-Reply-To
- <20141225182534.32540.83491.chriscool@tuxfamily.org>
Christian Couder <chriscool@tuxfamily.org> writes:
Show 5 quoted lines
> When running for example "git bisect bad HEAD" or > "git bisect good master", the parameter passed to > "git bisect (bad|good)" has to be parsed into a > commit hash before checking if it is the expected > commit or not.
Hmm, is that because you wrote commit object name in 40-hex in the EXPECTED_REV and you need to compare with what the user gave you which could be symbolic?
The conversion makes sense, but why is it a bad thing to say
git bisect bad maint
when 'maint' is not what you checked out in the current bisect run in the first place (perhaps you checked if it is good or bad manually before you started bisecting)?
Show 7 quoted lines
> diff --git a/git-bisect.sh b/git-bisect.sh
> index 6cda2b5..2fc07ac 100755
> --- a/git-bisect.sh
> +++ b/git-bisect.sh
> @@ -237,15 +237,18 @@ bisect_state() {
> check_expected_revs "$rev" ;;
> 2,bad|*,good|*,skip)This part accepts arbitrary number of revs when running good and skip, e.g.
git bisect good maint master next
and it loops
Show 10 quoted lines
> shift > - eval='' > + hash_list='' > for rev in "$@" > ... > + for rev in $hash_list > + do > + bisect_write "$state" "$rev" > + done > + check_expected_revs $hash_list ;;
But check_expected_revs loops and leaves the loop early when it finds anything that is not expected.
... goes and looks ...
Hmph, I think the logic in check_expected_revs is not wrong, but this helper function is grossly misnamed. It is not checking and rejecting the user input---it is checking to see if it can bypass check_good_are_ancestors_of_bad() which is expensive, so when it sees any one of the input is not what it checked out, it just disables the "optimization".
OK, will queue.
Thanks.