Hi Stephan, On Fri, Feb 26, 2016 at 10:38 PM, Stephan Beyer <s-beyer@xxxxxxx> wrote: > Hi Christian, > > On 02/26/2016 07:53 AM, Christian Couder wrote: >>> +test_expect_success 'bisect algorithm works in linear history with an odd number of commits' ' >>> + git bisect start A7 && >>> + git bisect next && >>> + test "$(git rev-parse HEAD)" = "$(git rev-parse A3)" \ >>> + -o "$(git rev-parse HEAD)" = "$(git rev-parse A4)" >> >> I thought that we should not use "-o" and "-a" but instead "|| test" >> and "&& test". > > Why is this? I think it is because it might not be very portable, but I am not sure I remember well the previous discussions about this. > I understand the && instead of -a thing (test atomicity), > however, for || this results in an ugly > > + git bisect next && > + ( test "$(git rev-parse HEAD)" = "$(git rev-parse A3)" || > + test "$(git rev-parse HEAD)" = "$(git rev-parse A4)" ) > > Right? (Otherwise a failure of e.g. "git bisect start A7" would run > the command after || (which may still be fine in some cases but is wrong > in most of the other cases). Yeah. By the way maybe t6030-bisect-porcelain.sh is paranoid, but it does some of those checks like: HASH4=$(git rev-parse --verify HEAD) and later: rev_hash4=$(git rev-parse --verify HEAD) && test "$rev_hash4" = "$HASH4" && So it uses "--verify" and also often puts the "git rev-parse" on a separate line... > However, what do you think about this? > > diff --git a/t/t8010-bisect-algorithm.sh b/t/t8010-bisect-algorithm.sh > index bda59da..ae50e7c 100755 > --- a/t/t8010-bisect-algorithm.sh > +++ b/t/t8010-bisect-algorithm.sh > @@ -8,6 +8,16 @@ exec </dev/null > > . ./test-lib.sh > > +test_compare_rev () { > + arg="$(git rev-parse "$1")" ...hence I would suggest: arg="$(git rev-parse --verify "$1")" || return > + shift > + for rev > + do > + test "$arg" = "$(git rev-parse "$rev")" && return 0 ...and: hash="$(git rev-parse --verify "$rev")" || return test "$arg" = "$hash" && return 0 > + done > + return 1 > +} Otherwise I like test_compare_rev(). Thanks, Christian. -- 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