Allow for build-time specification of supported SDK ranges for the built application

RESOLVED FIXED in Firefox 34

Status

()

RESOLVED FIXED
4 years ago
4 years ago

People

(Reporter: rnewman, Assigned: rnewman)

Tracking

(Blocks: 2 bugs)

Trunk
Firefox 34
All
Android
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(2 attachments, 2 obsolete attachments)

(Assignee)

Description

4 years ago
Splitting out from Bug 1039789.

This bug is for the specific work of allowing min/max to be specified during build, spitting out the right constants and manifest entries accordingly.
(Assignee)

Updated

4 years ago
Blocks: 1042383
(Assignee)

Comment 1

4 years ago
Created attachment 8460624 [details] [diff] [review]
Part 1: configure.in changes to allow Android SDK ranges to be specified. v1

Nick deferred to you on how to do the enable flags. See Bug 1039789.
Attachment #8460624 - Flags: review?(mh+mozilla)
(Assignee)

Updated

4 years ago
Assignee: nobody → rnewman
Status: NEW → ASSIGNED
(Assignee)

Comment 2

4 years ago
Created attachment 8460625 [details] [diff] [review]
Part 2: define SDK ranges in AndroidManifest.xml. v1

Should be a rubber-stamp.
Attachment #8460625 - Flags: review?(mark.finkle)
Attachment #8460625 - Flags: review?(mark.finkle) → review+
Comment on attachment 8460624 [details] [diff] [review]
Part 1: configure.in changes to allow Android SDK ranges to be specified. v1

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

::: configure.in
@@ +4864,5 @@
> +fi
> +if test -n "$MOZ_ANDROID_MAX_SDK_VERSION"; then
> +    AC_DEFINE_UNQUOTED(MOZ_ANDROID_MAX_SDK_VERSION, $MOZ_ANDROID_MAX_SDK_VERSION)
> +fi
> +

That should go in build/autoconf/android.m4, and check that ANDROID_TARGET_SDK is less than or equal to MOZ_ANDROID_MIN_SDK_VERSION.
Attachment #8460624 - Flags: review?(mh+mozilla) → review-
Re the irc discussion, let's say I'm fine with this if and only if you make sure the build path on the build slave is going to be the exact same one for all those configurations.
Mmmm well, even that is a pita to get right. Meh. Whatever. Let's waste build resources.
(Assignee)

Comment 6

4 years ago
(In reply to Mike Hommey [:glandium] from comment #3)
> and check that
> ANDROID_TARGET_SDK is less than or equal to MOZ_ANDROID_MIN_SDK_VERSION.

The reverse, as far as I know. The min SDK version must be less than the target SDK, but that's almost a truism.

The target SDK is usually the highest SDK version that the app has been successfully built with -- e.g., SDK 19.

The minimum SDK version is the lowest Android release that is allowed to install the app -- ours defaults to 9.

The maximum SDK version is used to select from a family of APKs. This usually isn't present, but might be 9, or 13, or 15.

The valid sanity checks here would be:

  maximum SDK, if present <= target SDK
  minimum SDK <= target SDK
  minimum SDK <= maximum SDK, if present

Relevant docs:

http://developer.android.com/guide/topics/manifest/uses-sdk-element.html
Ah yes, I was thinking $android_min_api_level (which is $1 in MOZ_ANDROID_SDK)
(Assignee)

Comment 8

4 years ago
Created attachment 8461249 [details] [diff] [review]
Part 1: configure.in changes to allow Android SDK ranges to be specified. v2

Moved to android.m4, added basic sanity checking for range:

Setting max less than min:
 0:02.28 configure: error: --with-android-max-sdk must be at least the value of --with-android-min-sdk.

Setting --with-android-min-sdk=6:
 0:01.66 configure: error: --with-android-min-sdk must be at least 9.
Attachment #8461249 - Flags: review?(mh+mozilla)
(Assignee)

Updated

4 years ago
Attachment #8460624 - Attachment is obsolete: true
Comment on attachment 8461249 [details] [diff] [review]
Part 1: configure.in changes to allow Android SDK ranges to be specified. v2

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

MOZ_ANDROID_VERSION is the NDK platform version, and you're adding it to MOZ_ANDROID_NDK instead of MOZ_ANDROID_SDK
Attachment #8461249 - Flags: review?(mh+mozilla) → review-
(Assignee)

Comment 10

4 years ago
Created attachment 8461310 [details] [diff] [review]
Part 1: configure.in changes to allow Android SDK ranges to be specified. v3

Moved to correct section, used $1 instead of MIN_ANDROID_VERSION, added an additional check that min < target. Tested.
Attachment #8461310 - Flags: review?(mh+mozilla)
(Assignee)

Updated

4 years ago
Attachment #8461249 - Attachment is obsolete: true
Attachment #8461310 - Flags: review?(mh+mozilla) → review+
https://hg.mozilla.org/mozilla-central/rev/626b749ccb28
https://hg.mozilla.org/mozilla-central/rev/153a8b79e121
https://hg.mozilla.org/mozilla-central/rev/b2ce4cf640ef
Status: ASSIGNED → RESOLVED
Last Resolved: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 34
(Assignee)

Updated

4 years ago
Blocks: 1063873
You need to log in before you can comment on or make changes to this bug.