1166522 - Change the update URL on client and build scripts to support device-buildtype-sdkversion

Status: RESOLVED WONTFIX

(Firefox OS Graveyard :: Gaia::System, defect, P2)

Description

[Naoki Hirata :nhirata (please use needinfo instead of cc)]

Based on some conversation, it appears that we should support doing an OTA url to support device-buildtype-sdkversion.  ie: flame-userdebug-kk

An example URL :  
https://aus4.mozilla.org/update/3/B2G/37.0/20150514162500/flame-userdebug-kk/en-US/nightly-b2g37/Boot2Gecko%202.2.0.0-prerelease%20%28SDK%2019%29/default/default/update.xml?force=1

Comment 1

[Naoki Hirata :nhirata (please use needinfo instead of cc)]

Attachment: bug1166522.diff

Patch for client side.

Comment 2

[Naoki Hirata :nhirata (please use needinfo instead of cc)]

I think we'll have to only push this to m-c and fix 2.2/2.1 OTA separately?  Need to check to see if that's possible.

Comment 3

[Wander Lairson Costa]

Comment on attachment 8607843 [details] [diff] [review]
bug1166522.diff

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

::: toolkit/mozapps/update/nsUpdateService.js
@@ +3500,3 @@
>        url = url.replace(/%PRODUCT_MODEL%/g,
>                          sysLibs.libcutils.property_get("ro.product.model"));
> +      if ( sdkVersion >= 14|| sdkVersion <= 19 ) {

is || or && operator to use here?

@@ +3505,5 @@
> +        if ( sdkVersion >=20 || sdkVersion <= 22 ) {
> +          url = url.replace(/%PRODUCT_DEVICE%/g, productDevice + "-" + buildTypei "-l" );
> +          } else {
> +          url = url.replace(/%PRODUCT_DEVICE%/g, productDevice);
> +        }

Maybe should be better to put the sdk logic in its own function, like so:

function getSdkSuffix(sdkversion) {
   if (sdkVersion >= 14 && sdkVersion <= 19) {
      return "-kk";
   } else if (sdkVersion >= 20 && sdkVersion <= 22) {
      reurn "-l";
   }
   return "";
}

Comment 4

[Eric Chang [:ericcc] [:echang]]

Hi Naoki, not sure if that is a typo, buildTypei or just buildType?

+          url = url.replace(/%PRODUCT_DEVICE%/g, productDevice + "-" + buildTypei "-l" );

Comment 5

[Naoki Hirata :nhirata (please use needinfo instead of cc)]

Thanks for the catch.  it's a typo.

Comment 6

[Naoki Hirata :nhirata (please use needinfo instead of cc)]

Attachment: Bug_1166522_final.patch

fixes based on nits and Eric's comment.

Comment 7

[Naoki Hirata :nhirata (please use needinfo instead of cc)]

Attachment: Bug_1166522_final.patch

Doh.  Realized why wcosta suggested the function and the return values.  Modified based on that.

Comment 8

[Wander Lairson Costa]

Comment on attachment 8610917 [details] [diff] [review]
Bug_1166522_final.patch

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

some nits, otherwise lgtm.

::: toolkit/mozapps/update/nsUpdateService.js
@@ +3431,5 @@
>    classID: Components.ID("{093C2356-4843-4C65-8709-D7DBCBBE7DFB}"),
>    QueryInterface: XPCOMUtils.generateQI([Ci.nsIUpdateManager, Ci.nsIObserver])
>  };
>  
> +/** 

nit: remove the trailing white space.

@@ +3437,5 @@
> + * returns the suffix based on the sdk version
> + * for kk or greater
> + *
> + */
> +

nit: remove the blank line.

@@ +3517,4 @@
>        let buildType = sysLibs.libcutils.property_get("ro.build.type");
>        url = url.replace(/%PRODUCT_MODEL%/g,
>                          sysLibs.libcutils.property_get("ro.product.model"));
> +      url = url.replace(/%PRODUCT_DEVICE%/g, 

nit: remove the trailing white space.

Comment 9

[Naoki Hirata :nhirata (please use needinfo instead of cc)]

Attachment: Bug_1166522_final.patch

Should be the final patch, cleaned the nits.  Wander, do you want to push your changes on the mozharness, taskcluster and balrog side along with this patch?

Comment 10

[Wander Lairson Costa]

Comment on attachment 8612119 [details] [diff] [review]
Bug_1166522_final.patch

lgtm. Please don't push it yet until I submit the server side of this.

Comment 11

[Wander Lairson Costa]

Attachment: Add target alias for flame and aries. r?bhearsum

As the effort to build unique BUILD_TARGET names for OTA updates, the
platform name will follow the pattern <device-name>-<variant>-<sdk>.

To keep backward compatiblity with existing images, we add apropriates
aliases for old platform names.

Comment 12

[[e-mail] (:bhearsum)]

Comment on attachment 8635739 [details] [diff] [review]
Add target alias for flame and aries. r?bhearsum

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

(In reply to Wander Lairson Costa [:wcosta] from comment #11)
> Created attachment 8635739 [details] [diff] [review]
> Add target alias for flame and aries. r?bhearsum
> 
> As the effort to build unique BUILD_TARGET names for OTA updates, the
> platform name will follow the pattern <device-name>-<variant>-<sdk>.
> 
> To keep backward compatiblity with existing images, we add apropriates
> aliases for old platform names.

\o/!

::: lib/python/release/platforms.py
@@ +33,5 @@
>                  # See bug 1071576 for details.
>                   'Darwin_x86-gcc3', 'Darwin_x86_64-gcc3'],
>      'win32': ['WINNT_x86-msvc'],
>      'win64': ['WINNT_x86_64-msvc'],
>      'flame-kk': ['flame-kk','flame'],

Is there a reason that this flame-kk needs to be kept around? I'm surprised to see flame-kk and flame here and below.

Comment 13

[Wander Lairson Costa]

(In reply to Ben Hearsum [:bhearsum] from comment #12)
> Comment on attachment 8635739 [details] [diff] [review]
> Add target alias for flame and aries. r?bhearsum
> 
> Review of attachment 8635739 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> (In reply to Wander Lairson Costa [:wcosta] from comment #11)
> > Created attachment 8635739 [details] [diff] [review]
> > Add target alias for flame and aries. r?bhearsum
> > 
> > As the effort to build unique BUILD_TARGET names for OTA updates, the
> > platform name will follow the pattern <device-name>-<variant>-<sdk>.
> > 
> > To keep backward compatiblity with existing images, we add apropriates
> > aliases for old platform names.
> 
> \o/!
> 
> ::: lib/python/release/platforms.py
> @@ +33,5 @@
> >                  # See bug 1071576 for details.
> >                   'Darwin_x86-gcc3', 'Darwin_x86_64-gcc3'],
> >      'win32': ['WINNT_x86-msvc'],
> >      'win64': ['WINNT_x86_64-msvc'],
> >      'flame-kk': ['flame-kk','flame'],
> 
> Is there a reason that this flame-kk needs to be kept around? I'm surprised
> to see flame-kk and flame here and below.

Initially I removed, but then I thought it could break buildbot.

Comment 14

[[e-mail] (:bhearsum)]

(In reply to Wander Lairson Costa [:wcosta] from comment #13)
> (In reply to Ben Hearsum [:bhearsum] from comment #12)
> > Comment on attachment 8635739 [details] [diff] [review]
> > Add target alias for flame and aries. r?bhearsum
> > 
> > Review of attachment 8635739 [details] [diff] [review]:
> > -----------------------------------------------------------------
> > 
> > (In reply to Wander Lairson Costa [:wcosta] from comment #11)
> > > Created attachment 8635739 [details] [diff] [review]
> > > Add target alias for flame and aries. r?bhearsum
> > > 
> > > As the effort to build unique BUILD_TARGET names for OTA updates, the
> > > platform name will follow the pattern <device-name>-<variant>-<sdk>.
> > > 
> > > To keep backward compatiblity with existing images, we add apropriates
> > > aliases for old platform names.
> > 
> > \o/!
> > 
> > ::: lib/python/release/platforms.py
> > @@ +33,5 @@
> > >                  # See bug 1071576 for details.
> > >                   'Darwin_x86-gcc3', 'Darwin_x86_64-gcc3'],
> > >      'win32': ['WINNT_x86-msvc'],
> > >      'win64': ['WINNT_x86_64-msvc'],
> > >      'flame-kk': ['flame-kk','flame'],
> > 
> > Is there a reason that this flame-kk needs to be kept around? I'm surprised
> > to see flame-kk and flame here and below.
> 
> Initially I removed, but then I thought it could break buildbot.

The only reason to keep it around would be if there's still builds that call the balrog submitter with "flame-kk" as the platform. If there is, we can keep this line, but we should add flame-user-kk to the alias list for consistency. If there isn't, it can just be removed.

Comment 15

[Wander Lairson Costa]

Attachment: Add target alias for flame and aries. r?bhearsum

As the effort to build unique BUILD_TARGET names for OTA updates, the
platform name will follow the pattern <device-name>-<variant>-<sdk>.

To keep backward compatiblity with existing images, we add proper
aliases for old platform names.

Comment 16

[Wander Lairson Costa]

url:        https://hg.mozilla.org/build/tools/rev/5d27f5b4176ace0e37638557c572645dd4de8a10
changeset:  5d27f5b4176ace0e37638557c572645dd4de8a10
user:       Wander Lairson Costa <[e-mail]>
date:       Mon Jul 20 14:39:02 2015 -0300
description:
Bug 1166522: Add target alias for flame and aries. r=bhearsum

As the effort to build unique BUILD_TARGET names for OTA updates, the
platform name will follow the pattern <device-name>-<variant>-<sdk>.

To keep backward compatiblity with existing images, we add proper
aliases for old platform names.

Comment 17

[Naoki Hirata :nhirata (please use needinfo instead of cc)]

This hasn't been pushed, so the changes won't take affect yet.

Comment 18

[Selena Deckelmann :selenamarie :selena]

Left to be done: 

* Update the tools repository with the new command-line arguments to support the new naming convention for blob name
* Patch mozharness to also receive a custom blob name related to the command-line parameters we updated in tools
* Update the build scripts intree related to these jobs

Dylan -- is this something your team can help with? O

Comment 19

[BMO Automation]

Firefox OS is not being worked on