Closed Bug 921417 Opened 6 years ago Closed 6 years ago

rewrite build/variant.py in javascript

Categories

(Firefox OS Graveyard :: Gaia, defect)

x86_64
Linux
defect
Not set

Tracking

(blocking-b2g:1.3+)

RESOLVED FIXED
1.3 Sprint 4 - 11/8
blocking-b2g 1.3+

People

(Reporter: yurenju, Assigned: johnhu)

References

Details

(Whiteboard: ft:system-platform])

Attachments

(2 files)

In order to be able to build and preload app in gaia with a firefox addon, we need to implement variant.py in javascript.
Assignee: yurenju.mozilla → johu
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
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
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.
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.
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.
Flags: needinfo?(jsmith)
(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
See comment 7 - check to see if that works for you.
Flags: needinfo?(jsmith)
Jason, you are so cool!! I never found that should always be 3 digitals. It works.
Oh, sorry. That's Albert in comment 7. Thank you, Albert.
blocking-b2g: --- → 1.3?
Target Milestone: --- → 1.3 Sprint 4 - 11/8
blocking-b2g: 1.3? → 1.3+
Whiteboard: ft:system-platform]
Blocks: 931457
See Also: → 929572
See Also: → 929602
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
Attachment #823760 - Flags: review?(yurenju.mozilla)
Blocks: 929572
Comment on attachment 823760 [details]
rewrite variant.py as variant.js

also set review flag to Albert who is the author of variant.py.
Attachment #823760 - Flags: review?(acperez)
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!
Attachment #823760 - Flags: review?(yurenju.mozilla) → review-
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.
Attachment #823760 - Flags: review?(acperez) → review-
Thanks guys for reviewing. I will change the patch ASAP.
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.
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.
Attachment #823760 - Flags: review?(yurenju.mozilla)
Attachment #823760 - Flags: review?(acperez)
Attachment #823760 - Flags: review-
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
Comment on attachment 823760 [details]
rewrite variant.py as variant.js

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

Nice work!
Attachment #823760 - Flags: review?(acperez) → review+
John, please use |git am 0001-Fixed-issues-for-Windows.patch| to apply this commit into your pull request.
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!
Attachment #823760 - Flags: review?(yurenju.mozilla) → review+
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 :-)
Flags: needinfo?(ryanvm)
Status: NEW → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
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 :)
Flags: needinfo?(ryanvm)
Okay I got it, I'll CC sheriffs@mozilla.bugs next time, thank you!
You need to log in before you can comment on or make changes to this bug.