Re: [PATCH 02/16] bisect: add test for the bisect algorithm

From: Matthieu Moy <matthieu.moy@grenoble-inp.fr>
Date: Feb 27, 2016, 12:42 UTC
Message-ID: <vpqd1riqq7o.fsf@anie.imag.fr>
In-Reply-To: <CAP8UFD27f3zmrLrvyCuMfs6ijt7MtLB8rX0Ykvfar3kidpm6LQ@mail.gmail.com>

Christian Couder <christian.couder@gmail.com> writes:

Show 19 quoted lines

> Hi Stephan,
>
> On Fri, Feb 26, 2016 at 10:38 PM, Stephan Beyer <s-beyer@gmx.net> 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.

See Documentation/CodingGuidelines:

 - We do not write our "test" command with "-a" and "-o" and use "&&"
   or "||" to concatenate multiple "test" commands instead, because
   the use of "-a/-o" is often error-prone.  E.g.

     test -n "$x" -a "$a" = "$b"

   is buggy and breaks when $x is "=", but

     test -n "$x" && test "$a" = "$b"

   does not have such a problem.

Regarding portability, test -a/-o is not strictly POSIX (it's in the XSI extension), but AFAIK implemented by all reasonable shells.

-- 
Matthieu Moy
http://www-verimag.imag.fr/~moy/