Last Comment Bug 776185 - ANDROID_VERSION_CODE needs to be based off MOZ_BUILD_DATE
: ANDROID_VERSION_CODE needs to be based off MOZ_BUILD_DATE
Status: RESOLVED FIXED
:
Product: Firefox for Android
Classification: Client Software
Component: General (show other bugs)
: unspecified
: ARM Android
: -- normal (vote)
: Firefox 17
Assigned To: Mark Finkle (:mfinkle) (use needinfo?)
:
Mentors:
Depends on:
Blocks: 775232
  Show dependency treegraph
 
Reported: 2012-07-20 18:39 PDT by Aki Sasaki [:aki]
Modified: 2012-11-03 09:48 PDT (History)
6 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---
fixed
fixed


Attachments
patch (1.14 KB, patch)
2012-07-20 19:55 PDT, Mark Finkle (:mfinkle) (use needinfo?)
blassey.bugs: review+
aki: feedback+
akeybl: approval‑mozilla‑aurora+
blassey.bugs: approval‑mozilla‑beta-
Details | Diff | Review
use the buildid based on MOZ_BUILD_DATE for ANDROID_VERSION_CODE (1.36 KB, patch)
2012-08-08 16:45 PDT, Chris AtLee [:catlee]
mark.finkle: review+
Details | Diff | Review

Description Aki Sasaki [:aki] 2012-07-20 18:39:20 PDT
Android Native needs to be the first 10 digits of MOZ_BUILD_DATE (if set); Android native armv6 would have to be the first 10 digits of MOZ_BUILD_DATE decremented by one; and android-xul would be the first 10 digits of MOZ_BUILD_DATE, incremented by one.

Background:

1) we pass in MOZ_BUILD_DATE to release builds to make sure the buildids are the same, a la
    make -f client.mk build MOZ_BUILD_DATE=20120717110313

2) android versionCodes are very important to get right in relation to each other.  Until we fixed bug 760740, we had to upload android-xul after android native, and android-xul had to have a larger versionCode.

Android-xul may no longer apply, but android-armv6 now does.
https://bugzilla.mozilla.org/show_bug.cgi?id=775232#c15

3) Merely going by build timestamp may result in the builds being built in the wrong order, with overlapping versionCodes.  Since we're already passing in a shared MOZ_BUILD_DATE, let's use that instead of assuming the builds will build in the right order.

4) Since armv6 builds need to have a lower versionCode than android native armv7, I'm requesting that we also decrement that versionCode.

This blocks our ability to build beta release builds of armv6.
Comment 1 Mark Finkle (:mfinkle) (use needinfo?) 2012-07-20 19:55:20 PDT
Created attachment 644575 [details] [diff] [review]
patch

I stole most of this from the XUL change, but I decrement instead of increment. I assume MIN_CPU_VERSION is ok to use and I am testing it the right way.

This built ok for me in my armv7 build (I think... where can I see the version code?)

I am pushing to Try too.
Comment 2 Mark Finkle (:mfinkle) (use needinfo?) 2012-07-20 20:15:08 PDT
https://tbpl.mozilla.org/?tree=Try&rev=94d4e8c371f7
Comment 3 Aki Sasaki [:aki] 2012-07-20 21:03:00 PDT
Comment on attachment 644575 [details] [diff] [review]
patch

I think this will decrement.
However, I think this is also dependent on the computer time, rather than using MOZ_BUILD_DATE... I might be wrong.

This should be enough to let us build the two platforms at the same time, assuming android-armv6 starts before or in the same hour as android.
We can tie in MOZ_BUILD_DATE at a later time.
Comment 4 Nick Thomas [:nthomas] 2012-07-22 20:44:54 PDT
If we're unlucky and end up decrementing yyyymmdd00 or even yyyymm0100, does Google Play care ? aka does the buildID have to be valid
Comment 5 Nick Thomas [:nthomas] 2012-07-22 20:48:54 PDT
http://developer.android.com/tools/publishing/versioning.html says it just needs to be an integer, so never mind me. I'm trigger happy after bug 741688.
Comment 6 Brad Lassey [:blassey] (use needinfo?) 2012-07-23 07:16:48 PDT
Comment on attachment 644575 [details] [diff] [review]
patch

[Approval Request Comment]
User impact if declined: won't be able to push ARMv6 and ARMv7 builds to google play as the same product
Testing completed (on m-c, etc.): try run, just landed on inbound
Risk to taking this patch (and alternatives if risky): should be zero risk. Alternative is for RelEng to do two seperate builds at least an hour apart
String or UUID changes made by this patch: none
Comment 7 Aki Sasaki [:aki] 2012-07-23 09:29:17 PDT
Grepping for MOZ_BUILD_DATE gives me this chunk of code: http://hg.mozilla.org/releases/mozilla-beta/file/327b951ff6a8/config/Makefile.in#l81 .

Maybe the long term fix is something like

ifeq (,$(ANDROID_VERSION_CODE))
ifeq ($(MIN_CPU_VERSION),7)
ifdef MOZ_BUILD_DATE
ANDROID_VERSION_CODE=$(shell echo $(MOZ_BUILD_DATE) | cut -c1-10)
else
ANDROID_VERSION_CODE=$(shell $(PYTHON) $(topsrcdir)/toolkit/xre/make-platformini.py --print-buildid | cut -c1-10)
endif
else
# decrement the version code by 1 for armv6 builds so armv7 builds will win any compatability ties
ifdef MOZ_BUILD_DATE
ANDROID_VERSION_CODE=$(shell echo `echo $(MOZ_BUILD_DATE) | cut -c1-10` - 1 | bc)
else
ANDROID_VERSION_CODE=$(shell echo `$(PYTHON) $(topsrcdir)/toolkit/xre/make-platformini.py --print-buildid | cut -c1-10` - 1 | bc)
endif
endif
endif

?
which is not very pretty, but will avoid release-day unpredictability.
Knowing Murphy, this will happen on a chemspill day.
Comment 8 Aki Sasaki [:aki] 2012-07-23 09:35:39 PDT
Maybe we should be going off buildid instead.
Comment 9 Aki Sasaki [:aki] 2012-07-23 09:38:40 PDT
ifeq (,$(ANDROID_VERSION_CODE))
ifeq ($(MIN_CPU_VERSION),7)
ANDROID_VERSION_CODE=$(shell cat $(DEPTH)/config/buildid | cut -c1-10)
else
# decrement the version code by 1 for armv6 builds so armv7 builds will win any compatability ties
ANDROID_VERSION_CODE=$(shell echo `cat $(DEPTH)/config/buildid | cut -c1-10` - 1 | bc)
endif
endif

?
Not sure if this will work, but if it does, I think that will solve this bug.
Comment 10 Alex Keybl [:akeybl] 2012-07-23 10:16:12 PDT
Comment on attachment 644575 [details] [diff] [review]
patch

[Triage Comment]
Let's only take this on Aurora for now, as we're still evaluating ARMv6 builds for Beta 15.
Comment 11 Ed Morley [:emorley] 2012-07-24 03:02:56 PDT
https://hg.mozilla.org/mozilla-central/rev/013625180b24
Comment 12 Aki Sasaki [:aki] 2012-07-24 10:00:25 PDT
Reopening until we use MOZ_BUILD_DATE or the buildid.
Comment 13 Brad Lassey [:blassey] (use needinfo?) 2012-07-25 16:07:46 PDT
Comment on attachment 644575 [details] [diff] [review]
patch

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

no longer needed on beta
Comment 14 Chris AtLee [:catlee] 2012-07-30 22:50:46 PDT
This seems to work on try: https://hg.mozilla.org/try/rev/d829418828bc (https://tbpl.mozilla.org/?tree=Try&rev=79f315f60aa4).

Any problems having the UA_BUILDID changed as well?
Comment 15 Matt Brubeck (:mbrubeck) 2012-07-31 10:54:39 PDT
(In reply to Chris AtLee [:catlee] from comment #14)
> Any problems having the UA_BUILDID changed as well?

No problem there.  UA_BUILDID is no longer used for anything on Android.
Comment 16 Chris AtLee [:catlee] 2012-08-08 16:45:58 PDT
Created attachment 650364 [details] [diff] [review]
use the buildid based on MOZ_BUILD_DATE for ANDROID_VERSION_CODE
Comment 17 Chris AtLee [:catlee] 2012-08-09 14:58:32 PDT
pushed as https://hg.mozilla.org/integration/mozilla-inbound/rev/87b84230f0ee
Comment 18 Ryan VanderMeulen [:RyanVM] 2012-08-09 19:56:08 PDT
https://hg.mozilla.org/mozilla-central/rev/87b84230f0ee
Comment 19 Chris AtLee [:catlee] 2012-08-28 06:58:16 PDT
Transplanted to mozilla-beta: https://hg.mozilla.org/releases/mozilla-beta/pushloghtml?changeset=3aa844edaa00

Note You need to log in before you can comment on or make changes to this bug.