Closed Bug 861073 Opened 9 years ago Closed 9 years ago

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

Categories

(Firefox Build System :: General, defect)

x86
macOS
defect
Not set
normal

Tracking

(firefox22 wontfix, firefox23 fixed)

RESOLVED FIXED
mozilla23
Tracking Status
firefox22 --- wontfix
firefox23 --- fixed

People

(Reporter: cpeterson, Assigned: cpeterson)

References

Details

Attachments

(1 file, 1 obsolete file)

Attached patch verify-android-version.patch (obsolete) — Splinter Review
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.
Attachment #736665 - Flags: review?(mh+mozilla)
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 ...
Attachment #736665 - Flags: review?(mh+mozilla) → review+
(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"?
(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
I incorporated your recommended changes and tested that both the `./configure --help` message and the configure-time warning print the correct MIN_ANDROID_VERSION.
Attachment #736665 - Attachment is obsolete: true
Attachment #736859 - Flags: review?(mh+mozilla)
Attachment #736859 - Flags: review?(mh+mozilla) → review+
https://hg.mozilla.org/mozilla-central/rev/43592a61a366
Status: ASSIGNED → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
Product: Core → Firefox Build System
You need to log in before you can comment on or make changes to this bug.