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

[Stefano Garzarella]

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