Yuren [:yurenju] (AFK ~ Oct. 12) (volunteer mode) Reporter
	Description • 11 years ago

In order to be able to build and preload app in gaia with a firefox addon, we need to implement variant.py in javascript.

	John Hu [:johnhu][:johu][:醬糊小弟] Assignee
	Comment 1 • 11 years ago

Hi Guys, 

This is my WIP: https://github.com/huchengtw-moz/gaia/commit/93e975fca7ed17dd4ad4c379ce256b0c9552c200. I met a strange problem here. The Downloads.simpleDownload always falls into failed callback. It gives the error like:

-*- variant.js download error...: 805a1ff3, false, false
-*- variant.js msg: [Exception... "2153390067"  nsresult: "0x80004005 (NS_ERROR_FAILURE)"  location: "JS frame :: resource://gre/modules/DownloadCore.jsm :: DownloadError :: line 952"  data: no]

I had checked the Doc[1] which says I can give string as the argument. But it failed. I wonder that if the https scheme affects. The arguments are followings:

url: https://owd.tid.es/store/manifest.webapp
target: /Users/hchu/gaia/firefoxos-gaia-spain/temp/apps/data/store/xhr-4.json

	John Hu [:johnhu][:johu][:醬糊小弟] Assignee
	Comment 2 • 11 years ago

Sorry, I forgot to add the reference url:
[1] https://developer.mozilla.org/en-US/docs/Mozilla/JavaScript_code_modules/Downloads.jsm#simpleDownload()

	John Hu [:johnhu][:johu][:醬糊小弟] Assignee
	Comment 3 • 11 years ago

Is simpleDownload an Old API??? I found another API called fetch with the same signature at:
http://mxr.mozilla.org/mozilla-central/source/toolkit/components/jsdownloads/src/Downloads.jsm#135

	John Hu [:johnhu][:johu][:醬糊小弟] Assignee
	Comment 4 • 11 years ago

A workable WIP:
https://github.com/huchengtw-moz/gaia/commit/84cc35ff39c559ba87916cbe1e8ca7c30c6f7946

This version works but with the following issues:
1. The exception can't be caught at out-side when throws exception at the callback of XHR request or Downloads.jsm.
2. Customized apps don't show up with qa-testcase-data reference configuration. But they shows up at firefox-os-span variants.

	John Hu [:johnhu][:johu][:醬糊小弟] Assignee
	Comment 5 • 11 years ago

Ok, I change the WIP to WIP-6:
https://github.com/huchengtw-moz/gaia/commit/8d0ea80ce1a0cc432c0b0b3f10638bed3b52c367

This WIP uses a global variable to host the thrown error and throws that at the end of module.

	John Hu [:johnhu][:johu][:醬糊小弟] Assignee
	Comment 6 • 11 years ago

Jason,

May you give me some information about the second issue in comment4: customization apps don't show up??

I had modified the variant.json to support my local telecom operator, 466-92. But it's still no apps shown. I added it to the t-mobile us record, or replace it in the mcc-mnc record, but not working.
ex:

      "id": "t-mobile us",
      "mcc-mnc": [
        "310-260",
        "466-92"
      ],

or

      "id": "t-mobile us",
      "mcc-mnc": [
        "466-92"
      ],

The apps had pushed to device at "/data/local/svoperapps/". And the svoperapps.json contains the record:

{"310-260":["Twitter","Wikipedia","Calculator"],"466-92":["Twitter","Wikipedia","Calculator"],"310-410":["Facebook","Accuweather","Poppit"]}

But, finally, no twitter, wikipedia, or calculator showed up.

	Albert [:albert]
	Comment 7 • 11 years ago

(In reply to John Hu [:johnhu] from comment #6)
> Jason,
> 
> May you give me some information about the second issue in comment4:
> customization apps don't show up??
> 
> I had modified the variant.json to support my local telecom operator,
> 466-92. But it's still no apps shown. I added it to the t-mobile us record,
> or replace it in the mcc-mnc record, but not working.
> ex:
> 
>       "id": "t-mobile us",
>       "mcc-mnc": [
>         "310-260",
>         "466-92"
>       ],
> 
> or
> 
>       "id": "t-mobile us",
>       "mcc-mnc": [
>         "466-92"
>       ],
> 
> The apps had pushed to device at "/data/local/svoperapps/". And the
> svoperapps.json contains the record:
> 
> {"310-260":["Twitter","Wikipedia","Calculator"],"466-92":["Twitter",
> "Wikipedia","Calculator"],"310-410":["Facebook","Accuweather","Poppit"]}
> 
> But, finally, no twitter, wikipedia, or calculator showed up.

John, mcc-mnc pairs must have 3 digits. Change 466-92 to 466-092

	Jason Smith [:jsmith]
	Comment 8 • 11 years ago

See comment 7 - check to see if that works for you.

	John Hu [:johnhu][:johu][:醬糊小弟] Assignee
	Comment 9 • 11 years ago

Jason, you are so cool!! I never found that should always be 3 digitals. It works.

	John Hu [:johnhu][:johu][:醬糊小弟] Assignee
	Comment 10 • 11 years ago

Oh, sorry. That's Albert in comment 7. Thank you, Albert.

	John Hu [:johnhu][:johu][:醬糊小弟] Assignee
	Comment 11 • 11 years ago

1. add some node.js compatible functions to utils 
2. add download_manager.js
3. rewrite the variant.py as variant.js
4. change the makefile to run variant.js

	Yuren [:yurenju] (AFK ~ Oct. 12) (volunteer mode) Reporter
	Comment 12 • 11 years ago

Comment on attachment 823760 [details]
rewrite variant.py as variant.js

also set review flag to Albert who is the author of variant.py.

	Yuren [:yurenju] (AFK ~ Oct. 12) (volunteer mode) Reporter
	Comment 13 • 11 years ago

Comment on attachment 823760 [details]
rewrite variant.py as variant.js

Hi John, I left comments on github, please set review flag to me again if you have updated pull request, thank you!

	Albert [:albert]
	Comment 14 • 11 years ago

Comment on attachment 823760 [details]
rewrite variant.py as variant.js

I think you missed a feature that is implemented in python script. You have the logic to keep a cache of downloaded apps but then the first thing you do is to delete all folders where apps are stored, so cache is not used. You shouldn't remove tmp folders, in that way build will be faster (you won't need to download the whole application, just manifests) and you can launch make instead you are offline.

	John Hu [:johnhu][:johu][:醬糊小弟] Assignee
	Comment 15 • 11 years ago

Thanks guys for reviewing. I will change the patch ASAP.

	John Hu [:johnhu][:johu][:醬糊小弟] Assignee
	Comment 16 • 11 years ago

Albert,

Thanks for this hint. I think I misunderstood the first comment of original python version: Create and clear temporal output folders. I just felt I should clear all temp folder and recreate them. I should change the code to remove "conf" folder only.

	John Hu [:johnhu][:johu][:醬糊小弟] Assignee
	Comment 17 • 11 years ago

Comment on attachment 823760 [details]
rewrite variant.py as variant.js

Hi Yuren and Alert,

I had changed the patch according to your comments. Please review it again. Thanks.

	Yuren [:yurenju] (AFK ~ Oct. 12) (volunteer mode) Reporter
	Comment 18 • 11 years ago

Houston, we have a problem. Testing it on Windows and get some error, I'll take a look tomorrow.

> $ GAIA_DISTRIBUTION_DIR=../firefoxos-gaia-spain-master/ make
> XULrunner directory: xulrunner-sdk-26/xulrunner-sdk
> run-js-command  applications-data
> run-js-command  preferences
> run-js-command  variant
> Exception: TypeError: this is null
> join@resource://gre/modules/osfile/ospath_win_back.jsm:148
> joinPath@resource://gre/modules/commonjs/toolkit/loader.js -> file:///C:/MinGW/msys/1.0/home/yurenju/gaia/build/utils.js:319
> execute@resource://gre/modules/commonjs/toolkit/loader.js -> file:///C:/MinGW/msys/1.0/home/yurenju/gaia/build/variant.js:310
> @-e:1
> 
> make: *** [local-apps] Error 3

	Albert [:albert]
	Comment 19 • 11 years ago

Comment on attachment 823760 [details]
rewrite variant.py as variant.js

Once Yuren's comments are fixed, for me is ok.

Nice work!

	Yuren [:yurenju] (AFK ~ Oct. 12) (volunteer mode) Reporter
	Comment 20 • 11 years ago

John, please use |git am 0001-Fixed-issues-for-Windows.patch| to apply this commit into your pull request.

	Yuren [:yurenju] (AFK ~ Oct. 12) (volunteer mode) Reporter
	Comment 21 • 11 years ago

Comment on attachment 823760 [details]
rewrite variant.py as variant.js

Looks good, r+ if all comments on github are addressed and apply the patch on comment 20, thanks for your effort!

	John Hu [:johnhu][:johu][:醬糊小弟] Assignee
	Comment 22 • 11 years ago

merged to master:
https://github.com/mozilla-b2g/gaia/commit/ebe4c42e97d7cee1acaf6afae20886bd2463e011

	Yuren [:yurenju] (AFK ~ Oct. 12) (volunteer mode) Reporter
	Comment 23 • 11 years ago

RyanVM, this is a huge change for gaia build system and it has a little bit chance to break TPBL based on my recently experience although we tested it on Mac and Windows with variant configuration and we don't have try server for gaia. just needinfo you to let you know if you find something break TPBL, please check this first :-)

	Ryan VanderMeulen [:RyanVM]
	Comment 24 • 11 years ago

I guess it went OK :)

In the future, you're better off CCing sheriffs@mozilla.bugs rather than needinfo'ing me, however. You made this change at 10:30pm for me, so I wasn't going to see any bustage from it for quite some time :)

	Yuren [:yurenju] (AFK ~ Oct. 12) (volunteer mode) Reporter
	Comment 25 • 11 years ago

Okay I got it, I'll CC sheriffs@mozilla.bugs next time, thank you!