616005 - "configure: line 11990: test: : integer expression expected"

Status: RESOLVED DUPLICATE of bug 591152

(Firefox Build System :: General, defect)

Description

[Daniel Holbert [:dholbert]]

Every fresh build, on Linux at least, has this bit of warning-spew amongst the initial configure lines:
> checking for clock_gettime(CLOCK_MONOTONIC) and -lrt... yes
> /builds/moz2_slave/mozilla-central-linux/build/configure: line 11990: test: : integer expression expected
> checking for res_ninit()... yes
http://tinderbox.mozilla.org/showlog.cgi?log=Firefox/1291231082.1291237625.23425.gz

This is a regression from this change:
http://hg.mozilla.org/mozilla-central/rev/ee594fb0fa3e#l1.121

The responsible line is:
> if test -z "$MACOS_DEPLOYMENT_TARGET" -o "$MACOS_DEPLOYMENT_TARGET" -ge "100300"; then
which is effectively equivalent to:
  if test -z "$FOO" -o "$FOO" -ge 5; then echo OHAI; fi
(that gives me the same warning, until I define FOO & set it to a numeric value)

Basically, it looks like test doesn't short-circuit "-o" or-operations -- it evaluates both arguments.  In contrast, it does short-circuit when "||" is used in place of "-o".  (or at least, no warning is spammed when I substitute in "||")  The line in question used to use "||", until the changeset noted above changed it to "-o".

Comment 1

[Daniel Holbert [:dholbert]]

ah, so it looks like "||" is evaluated by the shell -- not by 'test' -- so it's not correct either.  That is:
   FOO=6; if test -z $FOO || $FOO -ge 5; then echo OHAI; fi
prints out:
   6: command not found
which implies that I'm trying to run "$FOO -ge 5" as a command.

So neither the old code nor the new code is quite what we want.

Comment 2

[Daniel Holbert [:dholbert]]

"-a" doesn't short-circuit either -- that is:
> if test  -z  "$FOO" -o \( -n "$FOO" -a "$FOO" -ge 5 \); then echo OHAI; fi

I think we might want to just expand this into separate "if" statements, like so:
  
  DO_WCRTOMB_CHECK=0  # new helper variable

  if test -z "$MACOS_DEPLOYMENT_TARGET"; then
     DO_WCRTOMB_CHECK=1
  else
     if test "$MACOS_DEPLOYMENT_TARGET" -ge "100300"; then
       DO_WCRTOMB_CHECK=1
     fi
  fi

This seems to work locally, and provides the logic I'd expect from the original statement without any warnings.

khuey, does that look ok?  Or do you know of a more graceful way to do this?

Comment 3

[Daniel Holbert [:dholbert]]

(In reply to comment #2)
> "-a" doesn't short-circuit either -- that is:
> > if test  -z  "$FOO" -o \( -n "$FOO" -a "$FOO" -ge 5 \); then echo OHAI; fi

Sorry, I didn't finish that thought -- I meant to say that the above one-liner still produces an "integer expression expected" warning message, when FOO is unset or non-numeric.  (hence the need to expand into a multi-line check)

Comment 4

[Daniel Holbert [:dholbert]]

(In reply to comment #1)
> ah, so it looks like "||" is evaluated by the shell -- not by 'test'
[...]
> So neither the old code nor the new code is quite what we want.

Oh, I just noticed that the old code uses "test" twice -- it's not just a s/-o/||/ from the current code.

That's probably cleaner than expanding as in Comment 2.