Extract new build module webapp-shared.js from webapp-zip.js

RESOLVED FIXED in 2.0 S1 (9may)


6 years ago
5 years ago


(Reporter: yurenju, Assigned: gduan)


2.0 S1 (9may)
Dependency tree / graph

Firefox Tracking Flags



(Whiteboard: [p=5])


(7 attachments)

we do too much things in webapp-zip.js, so feature for picking files in shared directory should be extracted from webapp-zip.js

webapp-shared should pick files in shared into webapp.buildDirectoryFile for each app and webapp-zip will only compress files from webapp.buildDirectoryFile.
Posted file PR to master
Hi Yuren,
I'd like to have your feedback for the webapp-shared.js before writing unit test.
Please kindly check, thank.
Attachment #8377383 - Flags: feedback?(yurenju.mozilla)
Comment on attachment 8377383 [details] [review]
PR to master

offline discussed.
Attachment #8377383 - Flags: feedback?(yurenju.mozilla) → feedback+
Added integration tests and unit tests.

waiting for bug 897352.
Comment on attachment 8377383 [details] [review]
PR to master

Hi Yuren,
please kindly review this patch.
Attachment #8377383 - Flags: review?(yurenju.mozilla)
Comment on attachment 8377383 [details] [review]
PR to master

George, great job! please check my comments on github and set review flag to if have updated pr.

and please add GAIA_DEV_PIXELS_PER_PX integration test case
Attachment #8377383 - Flags: review?(yurenju.mozilla)
Comment on attachment 8377383 [details] [review]
PR to master

Hi Yuren,
I have updated based on the comments of github, please kindly review it again.
Attachment #8377383 - Flags: review?(yurenju.mozilla)
Comment on attachment 8377383 [details] [review]
PR to master

r=yurenju and please wait bug 897352 landed.
Attachment #8377383 - Flags: review?(yurenju.mozilla) → review+
Component: Gaia → Gaia::Build
Target Milestone: --- → 1.4 S5 (11apr)
travis is all green,
merge into master:
Closed: 5 years ago
Resolution: --- → FIXED
Backed out for causing test failures like: https://tbpl.mozilla.org/php/getParsedLog.php?id=37535315&tree=B2g-Inbound

Flags: needinfo?(gduan)
Resolution: FIXED → ---
Posted file PR to master 2
Resend PR,
waiting for tbpl.
Flags: needinfo?(gduan)
Previous test pass!
Merge to master,
Closed: 5 years ago5 years ago
Resolution: --- → FIXED
Reverted on suspicion of causing:
run-js-command gaia/webapp-shared
Exception: Error: Unable to add following file in stage: c:\builds\moz2_slave\b2g-in-w32_g-00000000000000000\build\gaia\shared\js\l10n.js
[Exception... "Component returned failure code: 0x80520012 (NS_ERROR_FILE_NOT_FOUND) [nsIFile.copyTo]"  nsresult: "0x80520012 (NS_ERROR_FILE_NOT_FOUND)"  location: "JS frame :: resource://gre/modules/commonjs/toolkit/loader.js -> file:///c://builds//moz2_slave//b2g-in-w32_g-00000000000000000//build//gaia/build/utils-xpc.js :: copyFileTo :: line 440"  data: no]
WebappShared.prototype.moveToBuildDir@resource://gre/modules/commonjs/toolkit/loader.js -> file:///c://builds//moz2_slave//b2g-in-w32_g-00000000000000000//build//gaia/build/webapp-shared.js:86:1
WebappShared.prototype.pushJS@resource://gre/modules/commonjs/toolkit/loader.js -> file:///c://builds//moz2_slave//b2g-in-w32_g-00000000000000000//build//gaia/build/webapp-shared.js:146:3
WebappShared.prototype.pushFileByType@resource://gre/modules/commonjs/toolkit/loader.js -> file:///c://builds//moz2_slave//b2g-in-w32_g-00000000000000000//build//gaia/build/webapp-shared.js:247:9
WebappShared.prototype.filterSharedUsage@resource://gre/modules/commonjs/toolkit/loader.js -> file:///c://builds//moz2_slave//b2g-in-w32_g-00000000000000000//build//gaia/build/webapp-shared.js:296:5
WebappShared.prototype.copyShared@resource://gre/modules/commonjs/toolkit/loader.js -> file:///c://builds//moz2_slave//b2g-in-w32_g-00000000000000000//build//gaia/build/webapp-shared.js:319:3
WebappShared.prototype.execute@resource://gre/modules/commonjs/toolkit/loader.js -> file:///c://builds//moz2_slave//b2g-in-w32_g-00000000000000000//build//gaia/build/webapp-shared.js:339:3
execute/<@resource://gre/modules/commonjs/toolkit/loader.js -> file:///c://builds//moz2_slave//b2g-in-w32_g-00000000000000000//build//gaia/build/webapp-shared.js:346:1
execute@resource://gre/modules/commonjs/toolkit/loader.js -> file:///c://builds//moz2_slave//b2g-in-w32_g-00000000000000000//build//gaia/build/webapp-shared.js:344:3

Makefile:552: recipe for target 'webapp-shared' failed
mozmake.exe[6]: Leaving directory 'c:/builds/moz2_slave/b2g-in-w32_g-00000000000000000/build/gaia'
Makefile:29: recipe for target 'libs' failed
mozmake.exe[5]: Leaving directory 'c:/builds/moz2_slave/b2g-in-w32_g-00000000000000000/build/obj-firefox/b2g/gaia'
c:/builds/moz2_slave/b2g-in-w32_g-00000000000000000/build/config/recurse.mk:95: recipe for target 'b2g/gaia/libs' failed
mozmake.exe[4]: Leaving directory 'c:/builds/moz2_slave/b2g-in-w32_g-00000000000000000/build/obj-firefox'
c:/builds/moz2_slave/b2g-in-w32_g-00000000000000000/build/config/recurse.mk:39: recipe for target 'libs' failed
mozmake.exe[3]: Leaving directory 'c:/builds/moz2_slave/b2g-in-w32_g-00000000000000000/build/obj-firefox'
c:/builds/moz2_slave/b2g-in-w32_g-00000000000000000/build/config/rules.mk:592: recipe for target 'default' failed
mozmake.exe[2]: Leaving directory 'c:/builds/moz2_slave/b2g-in-w32_g-00000000000000000/build/obj-firefox'
c:/builds/moz2_slave/b2g-in-w32_g-00000000000000000/build/client.mk:398: recipe for target 'realbuild' failed
mozmake.exe[1]: Leaving directory 'c:/builds/moz2_slave/b2g-in-w32_g-00000000000000000/build'
client.mk:185: recipe for target 'build' failed
mozmake.exe[6]: *** [webapp-shared] Error 3
mozmake.exe[5]: *** [libs] Error 2
mozmake.exe[4]: *** [b2g/gaia/libs] Error 2
mozmake.exe[3]: *** [libs] Error 2
mozmake.exe[2]: *** [default] Error 2
mozmake.exe[1]: *** [realbuild] Error 2
mozmake.exe: *** [build] Error 2
program finished with exit code 2
========= Finished compile failed (results: 2, elapsed: 49 mins, 50 secs) (at 2014-04-11 08:32:03.356449) =========

Resolution: FIXED → ---
Please push to gaia-try before re-landing.
FYI, it looks like this patch caused hamachi/buri devices to freeze at the blue firefox loading screen (bug 995292).

Maybe try flashing on a hamachi or buri device before re-landing.

I'm happy to try flashing this on my device as well, just set ni? for me if you want :-)
Target Milestone: 1.4 S5 (11apr) → 1.4 S6 (25apr)
Hi Jared,
I think I already fixed it in attachment 3 [details] [diff] [review], could you kindly help me to test it? It works on windows and mac with my hamachi device
Flags: needinfo?(6a68)
Hi Kevin,
I've pushed to tbpl, but there's still one fail test.
I have no idea why it failed. It's probably not due to my patch. What do you think?

Flags: needinfo?(kgrandon)
Sorry, it should be this link https://github.com/mozilla-b2g/gaia/pull/18308

(In reply to George Duan [:gduan] [:喬智] from comment #22)
> Hi Jared,
> I think I already fixed it in attachment 3 [details] [diff] [review], could
> you kindly help me to test it? It works on windows and mac with my hamachi
> device
Strange indeed. I've retriggered the job and let's see what it does a second time around.
Flags: needinfo?(kgrandon)
Hmm, same problems.

Here's what I did in detail:

1. Using the same gecko base from the last attempt, flash your branch (968661-3) onto the handset via "make reset-gaia".
2. I still get the phone stuck at the blue firefox screen (the tail is animated, but it sits there forever).
3. I tried a shallow gecko+gaia flash, Engineer build, using the latest build (2014-04-15-04-02-01), with the auto_flash_from_pvt tool.
4. I then flashed your branch (968661-3) onto my hamachi, again using `make reset-gaia`. Still freezing at the blue firefox screen.

If I can give you any useful debugging info, just let me know :-)
Flags: needinfo?(6a68)
Hi Jared,
I just flashed to 20140415160202 master-central build and reset gaia to my 968661-3 branch. But, I cannot reproduce what you saw on your device...

could you provide me the log? That'll be much helpful!
Flags: needinfo?(6a68)
OK, I am still getting the same symptoms.

I did "make really-clean", then "make reset-gaia" using the same gecko base from yesterday, and then let it sit for a minute or so at the blue screen, then did "adb logcat -d > output.txt".

Attaching that file now.
Flags: needinfo?(6a68)

I forgot to refresh your branch before I posted my last comment.

Works for me. :beers:
Comment on attachment 8377383 [details] [review]
PR to master

Hi Yuren,

I've made some changes in 2nd commit.
It is to modify the order of app-makefiles and webapp-shared.js. The reason is, app-makefile will remove all the content of each stage app folder and execute r.js (requirejs), however, all the modules in shared are not copied into stage yet.
Attachment #8377383 - Flags: review+ → review?(yurenju.mozilla)
Attachment #8377383 - Flags: review?(yurenju.mozilla)
Comment on attachment 8406554 [details] [review]
PR to master 3

Sorry, should be this one for review.
Attachment #8406554 - Flags: review?(yurenju.mozilla)
Comment on attachment 8406554 [details] [review]
PR to master 3

looks you changed some makefiles in apps directory, make sure you comparing all files in profile directory with latest master.
Attachment #8406554 - Flags: review?(yurenju.mozilla) → review+
Thanks Yuren and Jared,

travis report is green: https://travis-ci.org/mozilla-b2g/gaia/builds/23253415
tbpl looks nice: https://tbpl.mozilla.org/?tree=Try&rev=39afcc3ca6d5

merge into master again,
Closed: 5 years ago5 years ago
Resolution: --- → FIXED
Georges, b2g-inbound is burning since that landed. Please backout.
Reverted with

[master acfa2cc] Revert "Merge pull request #18308 from cctuan/968661-3"
Resolution: FIXED → ---
Exception: Error: Using inexistent shared resource: keyboard_layouts.json from: settings.gaiamobile.org
21:13:33     INFO -  WebappShared.prototype.pushResource@resource://gre/modules/commonjs/toolkit/loader.js -> file:///builds/slave/b2g_b2g-in_emu-d_dep-000000000/build/gaia/build/webapp-shared.js:170:1
21:13:33     INFO -  WebappShared.prototype.pushFileByType@resource://gre/modules/commonjs/toolkit/loader.js -> file:///builds/slave/b2g_b2g-in_emu-d_dep-000000000/build/gaia/build/webapp-shared.js:261:9
21:13:33     INFO -  WebappShared.prototype.filterSharedUsage@resource://gre/modules/commonjs/toolkit/loader.js -> file:///builds/slave/b2g_b2g-in_emu-d_dep-000000000/build/gaia/build/webapp-shared.js:304:5
21:13:33     INFO -  WebappShared.prototype.copyShared@resource://gre/modules/commonjs/toolkit/loader.js -> file:///builds/slave/b2g_b2g-in_emu-d_dep-000000000/build/gaia/build/webapp-shared.js:327:3
21:13:33     INFO -  WebappShared.prototype.execute@resource://gre/modules/commonjs/toolkit/loader.js -> file:///builds/slave/b2g_b2g-in_emu-d_dep-000000000/build/gaia/build/webapp-shared.js:347:3
21:13:33     INFO -  execute/<@resource://gre/modules/commonjs/toolkit/loader.js -> file:///builds/slave/b2g_b2g-in_emu-d_dep-000000000/build/gaia/build/webapp-shared.js:354:1
21:13:33     INFO -  execute@resource://gre/modules/commonjs/toolkit/loader.js -> file:///builds/slave/b2g_b2g-in_emu-d_dep-000000000/build/gaia/build/webapp-shared.js:352:3
21:13:33     INFO -  CommonjsRunner.prototype.run@/builds/slave/b2g_b2g-in_emu-d_dep-000000000/build/gaia/build/xpcshell-commonjs.js:69:5
21:13:33     INFO -  run@/builds/slave/b2g_b2g-in_emu-d_dep-000000000/build/gaia/build/xpcshell-commonjs.js:84:3
21:13:33     INFO -  @-e:1:1
webapp-shared should depends on keyboard-layouts task.
Posted file PR to master 4
add dependency of keyboard-layout
Land it again.

tbpl report: all green

travis report: all green
Closed: 5 years ago5 years ago
Resolution: --- → FIXED
Backout needed - this caused bug 998954.

George - Can you backout?
Flags: needinfo?(gduan)
I went ahead and did the revert to make sure it was in for the next nightly.

Master: https://github.com/mozilla-b2g/gaia/commit/22bf3bca1113082c672e5f67127f205ff1c27ff8
Flags: needinfo?(gduan)
Resolution: FIXED → ---
Target Milestone: 1.4 S6 (25apr) → ---
Hi Alex,
I cannot reproduce Bug 998954 on gaia 6b7a10e5d026cf031b2f3fc0fe5f671ea02dd4f9. System app works fine and no l10n error...
could you provide your build id and device as well? I have not much idea to fix it now.
Flags: needinfo?(lissyx+mozillians)
(In reply to George Duan [:gduan] [:喬智] from comment #45)
> Hi Alex,
> I cannot reproduce Bug 998954 on gaia
> 6b7a10e5d026cf031b2f3fc0fe5f671ea02dd4f9. System app works fine and no l10n
> error...
> could you provide your build id and device as well? I have not much idea to
> fix it now.

I don't have much idea either, but the issue was 100% reproductible. Build ID would match anything built yesterday by myself. It may be triggerred by multilocale build, though, as documented in bug 998954 per the command line given to reproduce.
Flags: needinfo?(lissyx+mozillians)
Hi Alex,
may you upload the profile to dropbox or somewhere I can download and verify? That'll be much helpful!!
Flags: needinfo?(lissyx+mozillians)
The attached profile exposes the issue when flashing the system app.
Flags: needinfo?(lissyx+mozillians)
Posted file PR to master 5
As https://bugzilla.mozilla.org/show_bug.cgi?id=998954#c13 ,
I open another bug 999413 to solve it.

send pr to master again,
waiting for travis and tbpl.
tbpl is all green,
travis is all green,

merge to master again,
Closed: 5 years ago5 years ago
Resolution: --- → FIXED
Depends on: 1000617
Revert this patch since it cause profile 3~4 MB larger than before:

Resolution: FIXED → ---
Posted file PR to master 6
Compared with original code, images inside styles have not been compressed, so it extend the size of profile.

This patch will try to compress all files except jpg and jpeg (they might be larger after compressed).
Attachment #8412438 - Flags: review?(yurenju.mozilla)
Profile size on master: ~58MB
Profile size after my commit: ~54MB
Comment on attachment 8412438 [details] [review]
PR to master 6

r=yurenju for one line patch for getCompression(). George, please make sure this patch can solve the problem.
Attachment #8412438 - Flags: review?(yurenju.mozilla) → review+
and pass on travis-ci and try server for all build & all tests
Target Milestone: --- → 2.0 S1 (9may)
Whiteboard: [ETA:4/29]
With new patch,

TBPL: all green
Travis: all green
Profile size: 
|make| 53.8MB 
Push to hamachi:  |make PRODUCTION=1 MOZILLA_OFFICIAL=1 reset-gaia|
/dev                    89M    48K    89M   4096
/mnt/asec               89M     0K    89M   4096
/mnt/obb                89M     0K    89M   4096
/system                200M   152M    47M   4096
/data                  161M     2M   158M   4096
/persist                 4M   908K     3M   4096
/cache                  40M     1M    38M   4096
/mnt/sdcard              1G   245M     1G   32768
/mnt/secure/asec         1G   245M     1G   32768
merge to master,
Closed: 5 years ago5 years ago
Resolution: --- → FIXED
This patch will not override the original shared content of each app. If someone's source tree is not clean with any shared content in apps, that properly cause pb.

Please make sure you have run |git clean -fd|.
Whiteboard: [ETA:4/29] → [p=5]
This bug seems to have misunderstood the purpose of looking for a gaia_shared.json file, at least in the email case.

The gaia_shared.json file was introduced because the email app did not use link and script tags in the index.html to indicate dependencies, but instead used a module system and @import for CSS. In email's Makefile, it would generate the gaia_shared.json file so that the rest of the build system.

The patch for this bug seems to have inserted commented out link/script tags into the email/index.html so that the build would work because now the way the build is structured, the webapp-shared runs before the app's Makefile could generate a gaia_shared.json.

However, bug 992994 introduced some new @import lines in the mail.css file, but those CSS files are not found now due to the change in order of operations because of this bug and possibly an earlier one.

For now I suppose we will have to duplicate path info on those link comments in the index.html, but the whole point of the gaia_shared.json support was to avoid this duplication, as it is prone to failure (see bug 992994), and it is just redundant if the app's Makefile can generate that list itself.

Maybe the longer term fix is to allow webapp-shared.js to be smarter for apps that don't specify all file dependencies in the top level HTML file, as this is becoming more common in apps, whether they use a module system or the LazyLoader. Maybe parts of email/build/make_gaia_shared.js could be ported, but not all apps use the same staged loading technique, so not sure it makes sense. 

That was one of the goals of gaia_shared.json though, it allowed the app's Makefile to generate the shared list, however it thought appropriate. 

I can appreciate wanting to pull out as much common code, in this code change instance, from apps having to manually copy in the shared/ area (as email's Makefile was doing), particularly now that all apps are assembled in build_stage, and copying all of shared/ into them may seem wasteful.

So I do not have a solid course for resolution given the other build changes. I just wanted to document what I have found so far.
feature-b2g: --- → 2.0
You need to log in before you can comment on or make changes to this bug.