861073 - Verify --with-android-version is at least 9

Status: RESOLVED FIXED

(Firefox Build System :: General, defect)

Description

[Chris Peterson [:cpeterson]]

Attachment: verify-android-version.patch

To avoid developer confusion about SLES/OpenSLES.h build errors after bug 850713, add a sanity check that --with-android-version >= 9 to Android's configure.

Comment 1

[Mike Hommey [:glandium]]

Comment on attachment 736665 [details] [diff] [review]
verify-android-version.patch

Review of attachment 736665 [details] [diff] [review]:
-----------------------------------------------------------------

::: build/autoconf/android.m4
@@ +31,4 @@
>  
>  MOZ_ARG_WITH_STRING(android-version,
>  [  --with-android-version=VER
> +                          android platform version, default $MIN_ANDROID_VERSION],

Help strings are put in a different place in configure, so this is not going to do what you intend.

@@ +35,4 @@
>      android_version=$withval)
>  
> +if test $android_version -lt $MIN_ANDROID_VERSION ; then
> +    AC_MSG_ERROR([--with-android-version=$android_version must be at least $MIN_ANDROID_VERSION.])

--with-android-version must be ...

Comment 2

[Chris Peterson [:cpeterson]]

(In reply to Mike Hommey [:glandium] from comment #1)
> >  MOZ_ARG_WITH_STRING(android-version,
> >  [  --with-android-version=VER
> > +                          android platform version, default $MIN_ANDROID_VERSION],
> 
> Help strings are put in a different place in configure, so this is not going
> to do what you intend.

So $MIN_ANDROID_VERSION would not be expanded in the help string? Should I just hard code the string "default 9"?

Comment 3

[Mike Hommey [:glandium]]

(In reply to Chris Peterson (:cpeterson) from comment #2)
> So $MIN_ANDROID_VERSION would not be expanded in the help string? Should I
> just hard code the string "default 9"?

that's option 1.

option 2 is to define MIN_ANDROID_VERSION as a m4 macro instead of a shell variable.

like:
define([MIN_ANDROID_VERSION], [9])
android_version=MIN_ANDROID_VERSION

MOZ_ARG_WITH_STRING(android-version,
[  --with-android-version=VER
                          android platform version, default] MIN_ANDROID_VERSION,
    android_version=$withval)

if test $android_version -lt MIN_ANDROID_VERSION ; then
    AC_MSG_ERROR([--with-android-version=$android_version must be at least] MIN_ANDROID_VERSION.)
fi

Comment 4

[Chris Peterson [:cpeterson]]

Attachment: verify-android-version-v2.patch

I incorporated your recommended changes and tested that both the `./configure --help` message and the configure-time warning print the correct MIN_ANDROID_VERSION.

Comment 5

[Chris Peterson [:cpeterson]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/43592a61a366

https://tbpl.mozilla.org/?tree=Try&rev=6b19f29de9e1

Comment 6

[(PTO June 19-July 7) Ryan VanderMeulen]

https://hg.mozilla.org/mozilla-central/rev/43592a61a366