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

[Stefano Garzarella]

On Fri, Aug 21, 2020 at 06:14:25PM +0200, Philippe Mathieu-Daudé wrote:
> 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/?

Yeah, less aggressive ;-)

> 
> >>>
> >>> 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.

Okay, I'll send a v2 with this change.

Thanks,
Stefano

> 
> >          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
> > 
>