flame-kk needs to query for updates differently from flame jb

RESOLVED FIXED in Firefox 34

Status

defect
RESOLVED FIXED
5 years ago
5 years ago

People

(Reporter: nthomas, Assigned: nthomas)

Tracking

unspecified
2.1 S5 (26sep)
x86
Gonk (Firefox OS)
Dependency tree / graph

Firefox Tracking Flags

(firefox33 unaffected, firefox34 fixed, firefox35 fixed, b2g-v2.0 fixed, b2g-v2.0M fixed, b2g-v2.1 fixed, b2g-v2.2 fixed)

Details

Attachments

(1 attachment, 2 obsolete attachments)

From bug 1055305 comment #40, the update query url is 
 https://aus4.mozilla.org/update/3/%PRODUCT%/%VERSION%/%BUILD_ID%/%PRODUCT_DEVICE%/...
(defined at http://mxr.mozilla.org/mozilla-central/source/b2g/app/b2g.js#568). Both flame and flame-kk subsitute 'flame' for PRODUCT_DEVICE, but the updates aren't compatible. This is blocking us turning on nightly builds for flame-kk.
Attachment #8484630 - Flags: review?(mwu)
Assignee: nobody → nthomas
Problem is that we'd also need to get the base images to report themselves as flame-kk. Can be done, but I wonder if we can't also check the underlying base version. Bug 1001542 seemed to give us access to ro.build.version.sdk, which is what we should ideally check against to determine if we need a JB or KK based image. No idea if it's reasonable to try to get a new thing in the update query url though. Maybe renaming the device to flame-kk is our only option?
Do you mean 'report themselves' in the sense of updates provided by Thundersoft, and/or in Settings -> Device Information ?

The current system for Mozilla updates expects the device name used with config.sh to show up as PRODUCT_DEVICE in the query, hence the patch here. There is an existing update/4/ url which we partially support, which is the same as update/3/ except it adds a %PLATFORM_VERSION% at the end, eg 
  .../%DIST_VERSION%/%PLATFORM_VERSION%/update.xml.
We'd need to change the update server a bit to fully support that.
I mean, when the vendor image is flashed, getprop ro.product.device should report flame-kk.

PLATFORM_VERSION isn't quite what we need to distinguish between flame-jb and flame-kk. We want sdk_version which was added in https://bug1001542.bugzilla.mozilla.org/attachment.cgi?id=8415598 . Doesn't look like it's hooked up to the update service though.
I'm not sure what the best way to move forward here is, but QA are asking for nightlies (as snapshots). So perhaps the short-term play is to 
* set B2G_UPDATER=0 for flame-kk, and possibly stop defining B2G_UPDATE_CHANNEL (both in b2g/config/flame-kk/config.json)
* enable nightly builds in buildbot
* figure out long term plan
(In reply to Nick Thomas [:nthomas] from comment #5)
> I'm not sure what the best way to move forward here is, but QA are asking
> for nightlies (as snapshots). So perhaps the short-term play is to 
> * set B2G_UPDATER=0 for flame-kk, and possibly stop defining
> B2G_UPDATE_CHANNEL (both in b2g/config/flame-kk/config.json)
> * enable nightly builds in buildbot
> * figure out long term plan

shoot, i remember this problem.   ni? Francis, can we get T2M to make the change of ro.product.device == flame-kk?  it's currently "flame"

$ adb shell getprop ro.product.device
flame
Flags: needinfo?(frlee)
(In reply to Tony Chung [:tchung] from comment #6)
> (In reply to Nick Thomas [:nthomas] from comment #5)
> > I'm not sure what the best way to move forward here is, but QA are asking
> > for nightlies (as snapshots). So perhaps the short-term play is to 
> > * set B2G_UPDATER=0 for flame-kk, and possibly stop defining
> > B2G_UPDATE_CHANNEL (both in b2g/config/flame-kk/config.json)
> > * enable nightly builds in buildbot
> > * figure out long term plan
> 
> shoot, i remember this problem.   ni? Francis, can we get T2M to make the
> change of ro.product.device == flame-kk?  it's currently "flame"
> 
> $ adb shell getprop ro.product.device
> flame

This isn't necessarily the right solution, and it won't help here, because our builds use https://github.com/mozilla-b2g/device-flame/blob/master/flame.mk. I strongly feel that we should strive to get the Android version number in the update URL so we can properly support these updates.
I'm going ahead with the disable-the-updater option in the meantime, at tchung's request.
hi Tony and all,

i can surely communicate with T2M to make following change: ro.product.device == flame-kk
but i would like to know, once this change is made, we can use this flag to distinguish if a specific Flame device should receive JB OTA (gaia/gecko) or KK OTA, correct?
how about shallow flash? is there any ways to prevent user flash a JB gaia/gecko on top of KK base image?
any suggestion?
Flags: needinfo?(frlee)
(In reply to Francis Lee [:frlee] from comment #9)
> hi Tony and all,
> 
> i can surely communicate with T2M to make following change:
> ro.product.device == flame-kk
> but i would like to know, once this change is made, we can use this flag to
> distinguish if a specific Flame device should receive JB OTA (gaia/gecko) or
> KK OTA, correct?
> how about shallow flash? is there any ways to prevent user flash a JB
> gaia/gecko on top of KK base image?
> any suggestion?

Sorry for missing your question.   i believe that is true.  any of our OTA infrastructure will just look for this ro.product.device name and apply the OTA changes appropriately.

cc'ing members Releng team to correct me if i'm wrong.
Flags: needinfo?(nthomas)
Flags: needinfo?(frlee)
Flags: needinfo?(catlee)
Flags: needinfo?(bhearsum)
(In reply to Tony Chung [:tchung] from comment #10)
> (In reply to Francis Lee [:frlee] from comment #9)
> > hi Tony and all,
> > 
> > i can surely communicate with T2M to make following change:
> > ro.product.device == flame-kk
> > but i would like to know, once this change is made, we can use this flag to
> > distinguish if a specific Flame device should receive JB OTA (gaia/gecko) or
> > KK OTA, correct?
> > how about shallow flash? is there any ways to prevent user flash a JB
> > gaia/gecko on top of KK base image?
> > any suggestion?
> 
> Sorry for missing your question.   i believe that is true.  any of our OTA
> infrastructure will just look for this ro.product.device name and apply the
> OTA changes appropriately.
> 
> cc'ing members Releng team to correct me if i'm wrong.

This won't help unless we make the same change to *our* builds. I still strongly advise including the android version somewhere in the update URL (this is a Gecko change) as the right fix here.
Flags: needinfo?(bhearsum)
ben, what do you suggest?   i'm not sure if the vendor is going to include the android version.   Given that we'll be talking to vendor to help with changes, can you highlight exactly what Mozilla needs to make this the right change?

ni? wayne to talk to Tams, as soon as Ben provides feedbac.
Flags: needinfo?(wchang)
Flags: needinfo?(bhearsum)
Tony, the Android version is *already* included in every build. We only need to fix the gecko side to support sending the Android version - the vendor doesn't need to do anything.
Flags: needinfo?(bhearsum)
(In reply to Michael Wu [:mwu] from comment #13)
> Tony, the Android version is *already* included in every build. We only need
> to fix the gecko side to support sending the Android version - the vendor
> doesn't need to do anything.

Yes, this. Sarry if I wasn't being clear enough earlier!
Flags: needinfo?(wchang)
Flags: needinfo?(frlee)
Flags: needinfo?(catlee)
Comment on attachment 8484630 [details] [review]
PR to set PRODUCT_DEVICE to flame-kk

Clearing review for now since it sounds like we want to fix gecko to send the sdk version info. Feel free to request review again if this changes.
Attachment #8484630 - Flags: review?(mwu)
Ben, are you proposing we change the update url to the 4/ style, using the sdk version as (say) %SDK_VERSION% at the position of %PLATFORM_VERSION% is for Android, along with changes to nsUpdateService.js to read ro.build.version.sdk (like we do for ro.product.device) and use it to replace %SDK_VERSION% at run time ?

How would that look on the Balrog side ?
Flags: needinfo?(nthomas) → needinfo?(bhearsum)
(In reply to Nick Thomas [:nthomas] from comment #16)
> Ben, are you proposing we change the update url to the 4/ style, using the
> sdk version as (say) %SDK_VERSION% at the position of %PLATFORM_VERSION% is
> for Android, along with changes to nsUpdateService.js to read
> ro.build.version.sdk (like we do for ro.product.device) and use it to
> replace %SDK_VERSION% at run time ?

I was thinking that Android version could go into the OS_VERSION field, at least in the short and medium term.

> How would that look on the Balrog side ?

The simplest thing to do would be to have an extra rule that only matches BUILD_TARGET=flame, OS_VERSION=kitkat (or whatever the string is), and point that to a different blob. If we did this we'd also need something extra on the balrog submitter side to submit to a different blob.

Another way, which would be cleaner but more work, would be to make it possible to delegate OS_VERSION into the blob like we do with BUILD_TARGET. We may want or need to create a Blob class for B2G if we do this.
Flags: needinfo?(bhearsum)
Queries look something like this at the moment:
https://aus4.mozilla.org/update/3/B2G/34.0a2/20140911012401/flame/en-US/nightly/Boot2Gecko%202.1.0.0-prerelease/default/default/update.xml

ie %OS_VERSION% is 'Boot2Gecko%202.1.0.0-prerelease'. It looks like that was set up in bug 1001542, are we saying it's not needed now ?
Oh you probably meant in addition, since the update server can match on anything in %OS_VERSION% string. So something like
  Boot2Gecko 2.1.0.0-prerelease (SDK 19)
I guess.

The secondaryLibrary properly gets used in nsUpdateService.js, appending 
' (<secondaryLibrary>)' to the end of %OS_VERSION%. We use the same mechanism for adding GTK version for desktop Firefox. Trying this out.
Comment on attachment 8491967 [details] [diff] [review]
[gecko] set secondaryLibrary property for B2G

Update query with this patch (on aurora):

https://aus4.mozilla.org/update/3/B2G/34.0a2/20140918232119/flame/en-US/aurora/Boot2Gecko%202.1.0.0-prerelease%20(SDK%2019)/default/default/update.xml

The '%20(SDK%2019)' part is new. A helpful person on IRC confirmed the JB builds have android_sdk_version of 18.
Attachment #8491967 - Flags: review?(dhylands)
Not sure how I coerce flame-kk builds to submit into a separate blob yet, will need to make changes at http://hg.mozilla.org/build/tools/file/default/lib/python/balrog/submitter/cli.py#l193 and 196.
JB actually has 16, 17, 18 corresponding to 4.1, 4.2, and 4.3
See: http://developer.android.com/reference/android/os/Build.VERSION_CODES.html#JELLY_BEAN
That should still be fine. We'll write an update rule using the 19 value from kitkat, everything else would fall through to another rule which wouldn't be specific SDK version.
Comment on attachment 8491967 [details] [diff] [review]
[gecko] set secondaryLibrary property for B2G

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

::: xpcom/base/nsSystemInfo.cpp
@@ +322,5 @@
>      SetPropertyAsInt32(NS_LITERAL_STRING("sdk_version"), android_sdk_version);
> +
> +    char* sdkver = PR_smprintf("SDK %u", android_sdk_version);
> +    SetPropertyAsACString(NS_LITERAL_STRING("secondaryLibrary"),
> +                          nsDependentCString(sdkver));

PR_smprint  will malloc a new string, and I don't see where you free it, so this look s like a memory leak to me.

Shouldn't you use nsPrintfCString instead?
Attachment #8491967 - Flags: review?(dhylands)
Thanks for the suggestion. I'm not a cpp guy so the previous patch was constructed from the similar gtk setup, and I missed the free call. This compiles for flame-kk, and yields the same result as before.
Attachment #8484630 - Attachment is obsolete: true
Attachment #8491967 - Attachment is obsolete: true
Attachment #8492820 - Flags: review?(dhylands)
Comment on attachment 8492820 [details] [diff] [review]
[gecko] set secondaryLibrary property for B2G, v2

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

Thanks - This version looks good to me.
Attachment #8492820 - Flags: review?(dhylands) → review+
https://hg.mozilla.org/mozilla-central/rev/7cc47db168dc
Status: NEW → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → 2.1 S5 (26sep)
Comment on attachment 8492820 [details] [diff] [review]
[gecko] set secondaryLibrary property for B2G, v2

Approval Request Comment
[Feature/regressing bug #]: Gives us distinguishable requests for OTA of flame-jb and flame-kk, so we can prevent updates to kk for jb builds (which would be bad). This is really a safety net rather than a blocker for flame-kk nightlies + OTA
[User impact if declined]: More risk of bricking flame-jb users
[Describe test coverage new/current, TBPL]: None added
[Risks and why]: Low risk, appends new information to a string already in the update url, fails gracefully in pathogical case of sdk version not being set.
[String/UUID change made/needed]: None
Attachment #8492820 - Flags: approval-mozilla-b2g32?
Attachment #8492820 - Flags: approval-mozilla-aurora?
Attachment #8492820 - Flags: approval-mozilla-b2g32?
Attachment #8492820 - Flags: approval-mozilla-b2g32+
Attachment #8492820 - Flags: approval-mozilla-aurora?
Attachment #8492820 - Flags: approval-mozilla-aurora+
(In reply to Ryan VanderMeulen [:RyanVM UTC-4] from comment #28)
> https://hg.mozilla.org/mozilla-central/rev/7cc47db168dc

FTR, this missed the nightly at 4am Pacific on the 22nd. It'll be in the 1600 nightly instead.
Hi Nick, this is new FFOS TAM Wesly, nice to meet you!

I'm taking over Flame from Francis (transition on-going) and seems missed some history here, so would like to get your advise.

Today's status is vendor already changed "product.device" name to "flame-kk" in latest Flame KK SW, but per above history your've done some mechanism in Gaia (detect android sdk version?) so looks to me we don't need the change of product.device anymore, right?

If that's the case then I will get vendor's help to roll back it to simply "flame".

Thanks!
Flags: needinfo?(nthomas)
Hi Ryan: 

Sorry for bothering, maybe you would also know the answer of my question in comment#33?
Thanks!
Flags: needinfo?(ryanvm)
Nick's the person to answer that, not me.
Flags: needinfo?(ryanvm)
First chance I'll have to respond is early next week.
Thanks Ryan and Nick's feedback.

Hi Nicki:

We are preparing QCT CS-based (also KK based) Flame SW for publishing and plan to code freeze later today, would you kindly confirm which is the right "product.device" we should set in order to receive OTA correctly? My previous information is "flame-kk" but per some comment history above we only need "flame" as before?

Thank you.
(In reply to Chris Cooper [:coop] from comment #37)
> kgrandon is reporting in IRC that this is preventing him from updating:
> 
> https://aus4.mozilla.org/update/3/B2G/35.0a1/20141006040204/flame-kk/en-US/
> nightly/Boot2Gecko%202.2.0.0-prerelease%20%28SDK%2019%29/default/default/
> update.xml?force=1
>
> Is this the expected state right now?

No, I'm not sure why that is happening. We still set PRODUCT_DEVICE to 'flame' at
   https://github.com/mozilla-b2g/device-flame/blob/kitkat/flame.mk#L43

kgrandon, what base image and gecko/gaia are you using ? If this is reproducible a new bug would be useful.

(In reply to Wesly Huang from comment #38)
> We are preparing QCT CS-based (also KK based) Flame SW for publishing and
> plan to code freeze later today, would you kindly confirm which is the right
> "product.device" we should set in order to receive OTA correctly? My
> previous information is "flame-kk" but per some comment history above we
> only need "flame" as before?

Sorry for the delayed response. Until kgrandon's comment above, I would have said it doesn't matter for Mozilla's OTA if product.device is flame or flame-kk. Now I'm not so sure. I'm passing the ni? on to mwu, because of his comment #2 here.

Per bug 1073399, it would be useful if the next base image we offer people is as slim as possible, ie not an eng build, no extra drivers/files etc.
Flags: needinfo?(nthomas) → needinfo?(mwu)
I think flame is still the expected product.device value, not flame-kk.
Flags: needinfo?(mwu)
Thanks Michael, now "product.device=flame" is the one we set for SW after v184. (also SW before v184, meaning only v184 has ever set product.device to flame-kk)
(In reply to Nick Thomas [:nthomas] from comment #39)
> (In reply to Chris Cooper [:coop] from comment #37)
> > kgrandon is reporting in IRC that this is preventing him from updating:
> > 
> > https://aus4.mozilla.org/update/3/B2G/35.0a1/20141006040204/flame-kk/en-US/
> > nightly/Boot2Gecko%202.2.0.0-prerelease%20%28SDK%2019%29/default/default/
> > update.xml?force=1
> >
> > Is this the expected state right now?
> 
> No, I'm not sure why that is happening. We still set PRODUCT_DEVICE to
> 'flame' at
>    https://github.com/mozilla-b2g/device-flame/blob/kitkat/flame.mk#L43
> 
> kgrandon, what base image and gecko/gaia are you using ? If this is
> reproducible a new bug would be useful.

I typically flash the bash image, then use the B2G nightly flame-kk builds, flashed from this tool: https://github.com/Mozilla-TWQA/B2G-flash-tool

Shouldn't we support upgrades coming from 'flame-kk' since we use this tool and encourage devs to use?
Flags: needinfo?(tchung)
Flags: needinfo?(nthomas)
(In reply to Kevin Grandon :kgrandon from comment #42)
> (In reply to Nick Thomas [:nthomas] from comment #39)
> > (In reply to Chris Cooper [:coop] from comment #37)
> > > kgrandon is reporting in IRC that this is preventing him from updating:
> > > 
> > > https://aus4.mozilla.org/update/3/B2G/35.0a1/20141006040204/flame-kk/en-US/
> > > nightly/Boot2Gecko%202.2.0.0-prerelease%20%28SDK%2019%29/default/default/
> > > update.xml?force=1
> > >
> > > Is this the expected state right now?
> > 
> > No, I'm not sure why that is happening. We still set PRODUCT_DEVICE to
> > 'flame' at
> >    https://github.com/mozilla-b2g/device-flame/blob/kitkat/flame.mk#L43
> > 
> > kgrandon, what base image and gecko/gaia are you using ? If this is
> > reproducible a new bug would be useful.
> 
> I typically flash the bash image, then use the B2G nightly flame-kk builds,
> flashed from this tool: https://github.com/Mozilla-TWQA/B2G-flash-tool
> 
> Shouldn't we support upgrades coming from 'flame-kk' since we use this tool
> and encourage devs to use?

i thought the comments earlier decided to base OTAs on Android version number, not device name?
Flags: needinfo?(tchung)
kgrandon, I suspect you have the v184 base image, which was the only one that queries with 'flame-kk' (due to earlier confusion in  this bug about what we wanted). The problem should go away with the later base images.

(In reply to Tony Chung [:tchung] from comment #43)
> i thought the comments earlier decided to base OTAs on Android version
> number, not device name?

That's right. This bug got extra confusing when we added discussion & trouble shooting after it was resolved.
Flags: needinfo?(nthomas)
Now that we're only shipping Flame builds for kk, I filed bug 1104954 to get rid of the "flame-kk" hack that was added.
You need to log in before you can comment on or make changes to this bug.