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

RESOLVED FIXED in 2.0 S1 (9may)

Status

Firefox OS
Gaia::Build
RESOLVED FIXED
4 years ago
4 years ago

People

(Reporter: yurenju, Assigned: gduan)

Tracking

unspecified
2.0 S1 (9may)
x86
Mac OS X
Dependency tree / graph

Firefox Tracking Flags

(feature-b2g:2.0)

Details

(Whiteboard: [p=5])

Attachments

(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.
Depends on: 897352
WIP, https://github.com/cctuan/gaia/commit/c2285fd6936b0df9699027a00b65d6fe405b2e1e
remaining:
1. make it clean
2. unit test
3. integration test
Created attachment 8377383 [details] [review]
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.
Depends on: 920431
Comment on attachment 8377383 [details] [review]
PR to master

Hi Yuren,
please kindly review this patch.
Thanks.
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.
Thanks.
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
Status: NEW → ASSIGNED
Target Milestone: --- → 1.4 S5 (11apr)
travis is all green,
merge into master:
https://github.com/mozilla-b2g/gaia/commit/8239e552c432ff6289037e7fdc9e235d634d4d74
Status: ASSIGNED → RESOLVED
Last Resolved: 4 years ago
Resolution: --- → FIXED
Backed out for causing test failures like: https://tbpl.mozilla.org/php/getParsedLog.php?id=37535315&tree=B2g-Inbound

https://github.com/mozilla-b2g/gaia/commit/e354619da2a5ba8804f2a233e9f1c309d80b2378
Status: RESOLVED → REOPENED
Flags: needinfo?(gduan)
Resolution: FIXED → ---
Created attachment 8405248 [details] [review]
PR to master 2

Resend PR,
waiting for tbpl.
https://tbpl.mozilla.org/?tree=Try&rev=76f10b5925ce
Flags: needinfo?(gduan)
Previous test pass!
Merge to master,
https://github.com/mozilla-b2g/gaia/commit/b15c437c64fded0e0565aea05198486041e4372d
Status: REOPENED → RESOLVED
Last Resolved: 4 years ago4 years ago
Resolution: --- → FIXED
Reverted on suspicion of causing:
https://tbpl.mozilla.org/php/getParsedLog.php?id=37631651&tree=B2g-Inbound
{
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
CommonjsRunner.prototype.run@c:\builds\moz2_slave\b2g-in-w32_g-00000000000000000\build\gaia/build/xpcshell-commonjs.js:69:5
run@c:\builds\moz2_slave\b2g-in-w32_g-00000000000000000\build\gaia/build/xpcshell-commonjs.js:84:3
@-e:1:1

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
elapsedTime=2988.893000
========= Finished compile failed (results: 2, elapsed: 49 mins, 50 secs) (at 2014-04-11 08:32:03.356449) =========
}

https://github.com/mozilla-b2g/gaia/commit/aa898163f415fcc05e18b202a13dc8b8c141dd5e
Status: RESOLVED → REOPENED
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 :-)
Duplicate of this bug: 995292
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?

https://tbpl.mozilla.org/?tree=Try&rev=ef60b505f689
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)
LOL

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,
https://github.com/mozilla-b2g/gaia/commit/5888971a38db832241e0cf40e4f9d5e32333db51
Status: REOPENED → RESOLVED
Last Resolved: 4 years ago4 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"
Status: RESOLVED → REOPENED
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.
Created attachment 8408909 [details] [review]
PR to master 4

add dependency of keyboard-layout
Land it again.
https://github.com/mozilla-b2g/gaia/commit/6b7a10e5d026cf031b2f3fc0fe5f671ea02dd4f9

tbpl report: all green
https://tbpl.mozilla.org/?tree=Try&rev=3d3a7359d204

travis report: all green
https://travis-ci.org/mozilla-b2g/gaia/builds/23324902
Status: REOPENED → RESOLVED
Last Resolved: 4 years ago4 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
Status: RESOLVED → REOPENED
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)
Created attachment 8410107 [details]
profile.l10n.tar.bz2

The attached profile exposes the issue when flashing the system app.
Flags: needinfo?(lissyx+mozillians)
Created attachment 8410246 [details] [review]
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,
https://tbpl.mozilla.org/?tree=Try&rev=0d848dd78449
travis is all green,
https://github.com/mozilla-b2g/gaia/pull/18519

merge to master again,
https://github.com/mozilla-b2g/gaia/commit/33b5bcb1a092172c983ef178766869aba738ad4d
Status: REOPENED → RESOLVED
Last Resolved: 4 years ago4 years ago
Resolution: --- → FIXED

Updated

4 years ago
Depends on: 1000617
Revert this patch since it cause profile 3~4 MB larger than before:

https://github.com/mozilla-b2g/gaia/commit/4da57191b7e3ca153a22d017068f3332f9dcf580
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Created attachment 8412438 [details] [review]
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
https://tbpl.mozilla.org/?tree=Try&rev=4788d6c52026
Travis: all green
https://travis-ci.org/mozilla-b2g/gaia/builds/23737658
Profile size: 
|make| 53.8MB 
|make PRODUCTION=1 MOZILLA_OFFICIAL=1| 35MB
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,
https://github.com/mozilla-b2g/gaia/commit/56f79456db5dc3ca010a56d09e1e8cc15a2408db
Status: REOPENED → RESOLVED
Last Resolved: 4 years ago4 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.

Updated

4 years ago
feature-b2g: --- → 2.0
You need to log in before you can comment on or make changes to this bug.