Re: [PATCH] configure: silence 'shift' error message in version_ge()

[Philippe Mathieu-Daudé]

On 8/21/20 5:26 PM, Stefano Garzarella wrote:
> On Fri, Aug 21, 2020 at 04:21:10PM +0100, Peter Maydell wrote:
>> On Fri, 21 Aug 2020 at 16:00, Stefano Garzarella <sgarzare@redhat.com> wrote:
>>>
>>> If there are less than 2 arguments in version_ge(), the second shift
>>> prints this error:
>>>     ../configure: line 232: shift: shift count out of range
>>>
>>> Let's shut it up, since we're expecting this situation.

Maybe s/shut up/silence/?

>>>
>>> Signed-off-by: Stefano Garzarella <sgarzare@redhat.com>
>>> ---
>>>  configure | 2 +-
>>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>>
>>> diff --git a/configure b/configure
>>> index 4e5fe33211..de4bd0df36 100755
>>> --- a/configure
>>> +++ b/configure
>>> @@ -229,7 +229,7 @@ version_ge () {
>>>          set x $local_ver1
>>>          local_first=${2-0}
>>>          # shift 2 does nothing if there are less than 2 arguments
>>> -        shift; shift
>>> +        shift; shift 2>/dev/null
>>
>> POSIX says
>> https://pubs.opengroup.org/onlinepubs/9699919799/utilities/V3_chap02.html#shift
>>
>> "If the n operand is invalid or is greater than "$#", this may be
>> considered a syntax error and a non-interactive shell may exit"
>>
>> so I think that we need to actually avoid the excess shift,
> 
> Maybe something like this:
> 
> diff --git a/configure b/configure
> index de4bd0df36..5f5f370e2c 100755
> --- a/configure
> +++ b/configure
> @@ -229,7 +229,7 @@ version_ge () {
>          set x $local_ver1
>          local_first=${2-0}
>          # shift 2 does nothing if there are less than 2 arguments
> -        shift; shift
> +        shift; test $# -gt 0 && shift

This looks better that mine indeed.

>          local_ver1=$*
>          set x $local_ver2
>          # the second argument finished, the first must be greater or equal
> 
>> not just suppress any warning it might print. (I'm not sure
>> Philippe's "shift || shift" patch can work for that, though,
>> as the exit status doesn't distinguish "valid shift but don't
>> do it again" from "valid shift and more args to come".)
> 
> I tried and also if I have meson 0.55.0, with the Philippe's patch
> applied it tries to download our internal meson, so maybe it is not
> working as expected.

=)

> 
> Thanks,
> Stefano
>