1120219 - Alert: configure errors: test Illegal number:

Status: RESOLVED DUPLICATE of bug 1119921

(Firefox Build System :: General, defect)

Description

[ISHIKAWA, Chiaki]

I noticed shell warning messages during recent build inside M-C tree
portion of the C-C source tree.

The warnings occur in three places: they all come M-C portion of the
sourc tree.

(a) /REF-COMM-CENTRAL/comm-central/mozilla/configure: 10193: test: Illegal number: 
(b) /REF-COMM-CENTRAL/comm-central/mozilla/configure: 20165: test: Illegal number: 
(c) js/src> /REF-COMM-CENTRAL/comm-central/mozilla/js/src/configure: 8870: test: Illegal number: 

I am building C-C TB locally using |make -f config.mk|
under Debian GNU/Linux 64-bit.

It is possible this issue has been fixed in the last few days, but I
think we should put "set -e" near the beginning of configure or
something (more on this later at the end.).

My update of C-C source tree was a few days ago.

hg log -R ./mozilla -r default -l 1
changeset:   222235:b42615e51c81
tag:         tip
parent:      222213:4d91c33b351c
parent:      222234:3580a90de286
user:        Ryan VanderMeulen <[e-mail]>
date:        Tue Jan 06 10:55:03 2015 -0500
summary:     Merge fx-team to m-c. a=merge

The warnings were buried in the lengthy configure output and so I have
not noticed them before, and the problems could have existed for
sometime.
But anyway the problems were in the above version.

The configure scripts have all similar constructs where the error occurs:

(a) 
if test -n "$gonkdir" -a "$ANDROID_VERSION" -ge 16; then
    MOZ_PIE=1
else
    MOZ_PIE=
fi

(b)
if test -n "$gonkdir" -a "$ANDROID_VERSION" -ge 21; then
    MOZ_WEBRTC=
fi

(c)
if test -n "$gonkdir" -a "$ANDROID_VERSION" -ge 16; then
    MOZ_PIE=1
else
    MOZ_PIE=
fi


Problem is under Debian GNU/Linux, I get
the variable settings of
gonkdir= ANDROID_VERSION=
when the control reaches above if-statement.

(I checked by inserting 
echo "gonkdir=$gonkdir ANDROID_VERSION=$ANDROID_VERSION"
immediately before if-statements.)
So after variable substition, the above construct, say (a), looks like

if test -n "" -a "" -ge 16; then
   ...
   
   
Naturally the "" -ge 16 is not right: Illegal number, indeed.

I initially tried to change the construct, say in (a) into something like

if test -n "$gonkdir" -a  x"$ANDROID_VERSION" != x -a "$ANDROID_VERSION" -ge 16; then
    MOZ_PIE=1
else
    MOZ_PIE=
fi

This does not work since the  shell, after the variable substition  sees

if test -n "" -a  x"" != x -a "" -ge 16; then

after all.

|test| does not seem to short-curcuit the evaluation and ignore what
comes aftrer

  -a x"" != x.

So are back to square 1.

I suspect that we have to use nested if constructs to avoid the
warning issue.
But since I don't know the semantics of the if condition, I will leave
the proper change to whoever understands it.
(I don't know, for example, if under Linux which setting should be
picked up, etc.)

I looked at TBPL to see if this problem is not in mozilla build.
Then I realize that it seems that |mach|
command hides the warnings of this kind from the log?

I think , the configure warning is a serious issue and should be
caught and configure should be aborted.  I looked at Thunderbird try
build and I did not seem to see the errors in there, too.  It could be
a patch landed to fix these issues, or the toolchain on my Debian
GNU/Linux 64-bit may have some issues ???

A suggestion to fix a this POSSIBLY A SERIOUS issue in configure.

This type of errors can have serious consequences.
Something like this can possibly end up in *incorrect/corrupt build*
for certain platforms
(since these warnings did not stop configure immediately),

I think I got lucky that MOZ_PIE and MOZ_WEBRTC are set
to proper values that work for me on my local PC this time around.
(Or are they indeed?)

You can see that a develoepr would have a tough time to figure out what
would have gone wrong if such setup of variables are incorrect due to
these type of errors and configure did not stop there.


So I would suggest we put

set -e

near the beginning of configure so that even a subtle issue causing a warning/erro
in configure.in will stop execution.

Well, OTOH, to be honest I am not sure if the particular construct
that causes error this time around can be detected by "set -e".

I just tested. configure proceeded happily :-(

It looks that the warning is printed by |test| prorgram itself(?). In
that case we have a much tougher time. 

Maybe the warnings from |test| and other warnings produced such as
these should be collected during the execution of configure.  Then
higher build command can decide to stop/continue based on known
problematic error/warning messages by using whitelist and blacklist.

At least, "test Illegal number" warnings have been noticed before
several times and fixed in due time from what I found in bugzilla.  So
the blacklist should contain this warning at least.
"binary operator expected" is another manifestation of undefined
shell variable issues. "integer expression expected" is another one. Hmm, maybe we need to collect a few more messages printed by "test" variants.

Considerations by build/config maintainers will be appreciated.

TIA

Comment 2

[ISHIKAWA, Chiaki]

Thank you.