1203800 - Allow for specifying the ANDROID_VERSION_CODE to use

Status: RESOLVED FIXED

(Firefox Build System :: Android Studio and Gradle Integration, defect)

Description

[Nathan Toone [:toonetown]]

The changes that were made in #1137898 cause a build failure when building for a min-sdk of greater than 11.  The description states:

The bit labelled 'g' is 1 if the build is targetting Android API 11+ and 0 otherwise

However, the check on line 110 of that file is:

elif min_sdk == 11:

Which causes the ValueError to be raised.  This check should probably be ">=" instead of "==".

Comment 1

[Nathan Toone [:toonetown]]

Attachment: bug-1203800.patch

Simple patch which just changes == to >=.  Flagging nalexander for review, since he's the one who did the initial code.

Comment 2

[Nathan Toone [:toonetown]]

Try server: https://treeherder.mozilla.org/#/jobs?repo=try&revision=216cb2e082bd

Comment 3

[Nathan Toone [:toonetown]]

(In reply to Nathan Toone from comment #0)
> The changes that were made in #1137898 cause a build failure when building
> for a min-sdk of greater than 11.  The description states:

Sorry - I prefixed with a pound sign, so it didn't recognize bug 1137898 in my first comment

Comment 4

[Nick Alexander :nalexander [he/him]]

(In reply to Nathan Toone from comment #0)
> The changes that were made in #1137898 cause a build failure when building
> for a min-sdk of greater than 11.  The description states:
> 
> The bit labelled 'g' is 1 if the build is targetting Android API 11+ and 0
> otherwise
> 
> However, the check on line 110 of that file is:
> 
> elif min_sdk == 11:
> 
> Which causes the ValueError to be raised.  This check should probably be
> ">=" instead of "==".

Nathan -- help me understand more about what you're doing.  This code is intentionally rejecting min_sdk values it doesn't recognize, since changing the SDK requirements changes the APK splits, and those need to update the android:versionCode computations.  I'm loathe to relax this.

If I understand your situation, you are building a single custom APK with a different min_sdk.  So you have no APK split at all (and no architecture split?), and could keep all the low-order bits free.

This ticket suggests that the versionCode computation somehow needs to be configured further in the mozconfig.  I guess we could add a flag to the Python script saying "do something else with the low order bits" for your builds.

You are not building with MOZILLA_OFFICIAL, right?  I'd be okay relaxing the script outside of MOZILLA_OFFICIAL.

Comment 5

[Nathan Toone [:toonetown]]

Yes - I am doing a custom build, but I only want to support (for example) API 15 and higher.  However, I get a failure when trying to build with this current comparison.  I am actually doing an architecture split, though.  But not an API split.

No - I'm not building with MOZILLA_OFFICIAL.  I agree, it would probably be best to just do the relaxation of checking outside of MOZILLA_OFFICIAL.

Comment 6

[Nick Alexander :nalexander [he/him]]

(In reply to Nathan Toone from comment #5)
> Yes - I am doing a custom build, but I only want to support (for example)
> API 15 and higher.  However, I get a failure when trying to build with this
> current comparison.  I am actually doing an architecture split, though.  But
> not an API split.
> 
> No - I'm not building with MOZILLA_OFFICIAL.  I agree, it would probably be
> best to just do the relaxation of checking outside of MOZILLA_OFFICIAL.

Do you even want rolling build versions based on the build ID?  Mozilla does that for automation reasons, and some history.  Most mainstream Android applications pin the version number and manually bump it as appropriate.  Would it work for you to manually bump?

If you look at https://dxr.mozilla.org/mozilla-central/source/mobile/android/confvars.sh, there's a MOZ_APP_VERSION=43.0a1 or whatever.  We could let you set ANDROID_VERSION_CODE in mozconfig or branding, and then fallback to our Mozilla-specific computation?

Comment 7

[Nathan Toone [:toonetown]]

That is exactly what I would like to do (set ANDROID_VERSION_CODE myself).  I was actually trying to dig in and figure out how to just use whatever got generated, but if I could specify it myself, that is even better.

Comment 8

[Nick Alexander :nalexander [he/him]]

(In reply to Nathan Toone from comment #7)
> That is exactly what I would like to do (set ANDROID_VERSION_CODE myself). 
> I was actually trying to dig in and figure out how to just use whatever got
> generated, but if I could specify it myself, that is even better.

I think we add a new MOZ_APP_ANDROID_VERSION_CODE variable around https://dxr.mozilla.org/mozilla-central/source/configure.in#8710, and AC_SUBST it like MOZ_APP_VERSION.  Comment it like, "android:versionCode to pack into any Android App produced.  If not set, falls back to a Mozilla-specific value derived from the build ID."

Then, around https://dxr.mozilla.org/mozilla-central/source/mobile/android/base/Makefile.in?from=mobile%2Fandroid%2Fbase%2FMakefile.in#7, make ANDROID_VERSION_CODE conditional on the new MOZ_APP_ANDROID_VERSION_CODE.

Be aware that you will need to handle the architecture yourself manually, although you must be doing so in your mozconfig.  I realize now that it's awkward to handle architecture in mozconfig and versionCode in branding.  Perhaps you can set that new variable in mozconfig, using `export` or `mk_add_options`?  I don't understand exactly how those work.

glandium: is this approach reasonable?

Comment 9

[Nick Alexander :nalexander [he/him]]

glandium: is this approach reasonable?

Comment 10

[Nathan Toone [:toonetown]]

Sounds good to me - I'll start working up a patch and submit it back here (I changed the title of the bug to better reflect what we will do)

Comment 11

[Nathan Toone [:toonetown]]

Attachment: MozReview Request: Bug 1203800 - Allow for specifying the ANDROID_VERSION_CODE to use; r?nalexander

Bug 1203800 - Allow for specifying the ANDROID_VERSION_CODE to use; r?nalexander

This patch allows you to set MOZ_APP_ANDROID_VERSION_CODE in a branding's configure.sh to specify the exact android:versionCode to use in the final (main) APK.  It does *not* modify the android:versionCode used in any other APKs.

Comment 12

[Nathan Toone [:toonetown]]

Comment on attachment 8660166 [details]
MozReview Request: Bug 1203800 - Allow for specifying the ANDROID_VERSION_CODE to use; r?nalexander

Bug 1203800 - Allow for specifying the ANDROID_VERSION_CODE to use; r?nalexander

This patch allows you to set MOZ_APP_ANDROID_VERSION_CODE in a branding's configure.sh to specify the exact android:versionCode to use in the final (main) APK.  It does *not* modify the android:versionCode used in any other APKs.

Comment 13

[Nick Alexander :nalexander [he/him]]

Comment on attachment 8660166 [details]
MozReview Request: Bug 1203800 - Allow for specifying the ANDROID_VERSION_CODE to use; r?nalexander

https://reviewboard.mozilla.org/r/19087/#review17017

If it works for you, it works for me.

Comment 14

[Mike Hommey [:glandium]]

wfm

Comment 15

[Nathan Toone [:toonetown]]

Try server: https://treeherder.mozilla.org/#/jobs?repo=try&revision=ba0100bb1720

Comment 16

[Pulsebot]

https://hg.mozilla.org/integration/fx-team/rev/87f8052b2b4a

Comment 17

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/87f8052b2b4a

Comment 18

[Nathan Toone [:toonetown]]

https://reviewboard.mozilla.org/r/19087/#review36639

Approved and already release in version 43