Closed
Bug 776185
Opened 12 years ago
Closed 12 years ago
ANDROID_VERSION_CODE needs to be based off MOZ_BUILD_DATE
Categories
(Firefox for Android Graveyard :: General, defect)
Tracking
(firefox16 fixed, firefox17 fixed)
RESOLVED
FIXED
Firefox 17
People
(Reporter: mozilla, Assigned: mfinkle)
References
Details
Attachments
(2 files)
1.14 KB,
patch
|
blassey
:
review+
mozilla
:
feedback+
akeybl
:
approval-mozilla-aurora+
blassey
:
approval-mozilla-beta-
|
Details | Diff | Splinter Review |
1.36 KB,
patch
|
mfinkle
:
review+
|
Details | Diff | Splinter Review |
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.
Assignee | ||
Comment 1•12 years ago
|
||
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.
Assignee: nobody → mark.finkle
Attachment #644575 -
Flags: review?(blassey.bugs)
Attachment #644575 -
Flags: feedback?(aki)
Assignee | ||
Comment 2•12 years ago
|
||
https://tbpl.mozilla.org/?tree=Try&rev=94d4e8c371f7
Reporter | ||
Comment 3•12 years ago
|
||
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.
Attachment #644575 -
Flags: feedback?(aki) → feedback+
Updated•12 years ago
|
Attachment #644575 -
Flags: review?(blassey.bugs) → review+
Comment 4•12 years ago
|
||
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•12 years ago
|
||
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•12 years ago
|
||
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
Attachment #644575 -
Flags: approval-mozilla-beta?
Attachment #644575 -
Flags: approval-mozilla-aurora?
Reporter | ||
Comment 7•12 years ago
|
||
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.
Reporter | ||
Comment 8•12 years ago
|
||
Maybe we should be going off buildid instead.
Reporter | ||
Comment 9•12 years ago
|
||
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•12 years ago
|
||
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.
Attachment #644575 -
Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Comment 11•12 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/013625180b24
Status: NEW → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 17
Reporter | ||
Comment 12•12 years ago
|
||
Reopening until we use MOZ_BUILD_DATE or the buildid.
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Comment 13•12 years ago
|
||
Comment on attachment 644575 [details] [diff] [review] patch Review of attachment 644575 [details] [diff] [review]: ----------------------------------------------------------------- no longer needed on beta
Attachment #644575 -
Flags: approval-mozilla-beta? → approval-mozilla-beta-
Comment 14•12 years ago
|
||
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•12 years ago
|
||
(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•12 years ago
|
||
Attachment #650364 -
Flags: review?(mark.finkle)
Assignee | ||
Updated•12 years ago
|
Attachment #650364 -
Flags: review?(mark.finkle) → review+
Comment 17•12 years ago
|
||
pushed as https://hg.mozilla.org/integration/mozilla-inbound/rev/87b84230f0ee
Comment 18•12 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/87b84230f0ee
Status: REOPENED → RESOLVED
Closed: 12 years ago → 12 years ago
Resolution: --- → FIXED
Comment 19•12 years ago
|
||
Transplanted to mozilla-beta: https://hg.mozilla.org/releases/mozilla-beta/pushloghtml?changeset=3aa844edaa00
Updated•12 years ago
|
status-firefox16:
--- → fixed
status-firefox17:
--- → fixed
Updated•3 years ago
|
Product: Firefox for Android → Firefox for Android Graveyard
You need to log in
before you can comment on or make changes to this bug.
Description
•