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

Status: RESOLVED FIXED

(Firefox OS Graveyard :: Gaia::Build, defect)

Description

[Yuren [:yurenju] (AFK ~ Oct. 12) (volunteer mode)]

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.

Comment 1

[(Inactive after June) George Duan [:gduan] [:喬智]]

WIP, https://github.com/cctuan/gaia/commit/c2285fd6936b0df9699027a00b65d6fe405b2e1e
remaining:
1. make it clean
2. unit test
3. integration test

Comment 2

[(Inactive after June) George Duan [:gduan] [:喬智]]

Attachment: 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.

Comment 3

[Yuren [:yurenju] (AFK ~ Oct. 12) (volunteer mode)]

Comment on attachment 8377383 [details] [review]
PR to master

offline discussed.

Comment 4

[(Inactive after June) George Duan [:gduan] [:喬智]]

Added integration tests and unit tests.

waiting for bug 897352.

Comment 5

[(Inactive after June) George Duan [:gduan] [:喬智]]

Comment on attachment 8377383 [details] [review]
PR to master

Hi Yuren,
please kindly review this patch.
Thanks.

Comment 6

[Yuren [:yurenju] (AFK ~ Oct. 12) (volunteer mode)]

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

Comment 7

[Yuren [:yurenju] (AFK ~ Oct. 12) (volunteer mode)]

WIP modification.
https://github.com/yurenju/gaia/commit/1133160e90585c2ea6585534c8608a2cee5b3ab0

Comment 8

[(Inactive after June) George Duan [:gduan] [:喬智]]

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.

Comment 9

[Yuren [:yurenju] (AFK ~ Oct. 12) (volunteer mode)]

Comment on attachment 8377383 [details] [review]
PR to master

r=yurenju and please wait bug 897352 landed.

Comment 10

[(Inactive after June) George Duan [:gduan] [:喬智]]

Conflict with bug 897352 is solved.
https://github.com/cctuan/gaia/commit/0eb054063d8754d31622011956504e4e1593fada

waiting for bug 897352 relanding.

Comment 11

[(Inactive after June) George Duan [:gduan] [:喬智]]

travis is all green,
merge into master:
https://github.com/mozilla-b2g/gaia/commit/8239e552c432ff6289037e7fdc9e235d634d4d74

Comment 12

[Kevin Grandon :kgrandon]

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

Comment 13

[(Inactive after June) George Duan [:gduan] [:喬智]]

Attachment: PR to master 2

Resend PR,
waiting for tbpl.
https://tbpl.mozilla.org/?tree=Try&rev=76f10b5925ce

Comment 14

[(Inactive after June) George Duan [:gduan] [:喬智]]

Previous test pass!
Merge to master,
https://github.com/mozilla-b2g/gaia/commit/b15c437c64fded0e0565aea05198486041e4372d

Comment 15

[Ed Morley [:emorley]]

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

Comment 16

[Ed Morley [:emorley]]

Plus also the same failures as reported in comment 12 for which this was backed out the first time :-(

https://tbpl.mozilla.org/php/getParsedLog.php?id=37633987&tree=B2g-Inbound
https://tbpl.mozilla.org/php/getParsedLog.php?id=37633902&tree=B2g-Inbound
https://tbpl.mozilla.org/php/getParsedLog.php?id=37633549&tree=B2g-Inbound

https://tbpl.mozilla.org/?tree=B2g-Inbound&rev=9778da56668b&jobname=emu

Note: The try run was builds only, no tests.

Comment 17

[Kevin Grandon :kgrandon]

Please push to gaia-try before re-landing.

Comment 18

[Jared Hirsch [:jhirsch] (he/him) (Needinfo please)]

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 :-)

Comment 20

[(Inactive after June) George Duan [:gduan] [:喬智]]

Attachment: PR to master 3

Comment 21

[(Inactive after June) George Duan [:gduan] [:喬智]]

push to tbpl
https://tbpl.mozilla.org/?tree=Try&rev=ef60b505f689

Comment 22

[(Inactive after June) George Duan [:gduan] [:喬智]]

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

Comment 23

[(Inactive after June) George Duan [:gduan] [:喬智]]

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

Comment 24

[(Inactive after June) George Duan [:gduan] [:喬智]]

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

Comment 25

[Kevin Grandon :kgrandon]

Strange indeed. I've retriggered the job and let's see what it does a second time around.

Comment 26

[Jared Hirsch [:jhirsch] (he/him) (Needinfo please)]

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 :-)

Comment 27

[(Inactive after June) George Duan [:gduan] [:喬智]]

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!

Comment 28

[(Inactive after June) George Duan [:gduan] [:喬智]]

push to tbpl again
https://tbpl.mozilla.org/?tree=Try&rev=6f4109406224

Comment 29

[Jared Hirsch [:jhirsch] (he/him) (Needinfo please)]

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.

Comment 30

[Jared Hirsch [:jhirsch] (he/him) (Needinfo please)]

LOL

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

Works for me. :beers:

Comment 31

[(Inactive after June) George Duan [:gduan] [:喬智]]

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.

Comment 32

[(Inactive after June) George Duan [:gduan] [:喬智]]

Comment on attachment 8406554 [details] [review]
PR to master 3

Sorry, should be this one for review.

Comment 33

[Yuren [:yurenju] (AFK ~ Oct. 12) (volunteer mode)]

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.

Comment 34

[(Inactive after June) George Duan [:gduan] [:喬智]]

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

Comment 35

[[:fabrice] Fabrice Desré]

Georges, b2g-inbound is burning since that landed. Please backout.

Comment 36

[John Ford [:jhford] CET/CEST Berlin Time]

Reverted with

[master acfa2cc] Revert "Merge pull request #18308 from cctuan/968661-3"

Comment 37

[(Inactive after June) George Duan [:gduan] [:喬智]]

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

Comment 38

[(Inactive after June) George Duan [:gduan] [:喬智]]

webapp-shared should depends on keyboard-layouts task.

Comment 39

[(Inactive after June) George Duan [:gduan] [:喬智]]

Attachment: PR to master 4

add dependency of keyboard-layout

Comment 40

[(Inactive after June) George Duan [:gduan] [:喬智]]

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

Comment 41

[(Inactive after June) George Duan [:gduan] [:喬智]]

in-bound:

https://tbpl.mozilla.org/?tree=B2g-Inbound&pusher=ffxbld

Comment 42

[(Inactive after June) George Duan [:gduan] [:喬智]]

Sorry, tbpl report should be this one.
https://tbpl.mozilla.org/?tree=B2g-Inbound&rev=347ffff6499b

Comment 43

[Jason Smith [:jsmith]]

Backout needed - this caused bug 998954.

George - Can you backout?

Comment 44

[Ryan VanderMeulen [:RyanVM]]

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

Comment 45

[(Inactive after June) George Duan [:gduan] [:喬智]]

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.

Comment 46

[:gerard-majax]

(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.

Comment 47

[(Inactive after June) George Duan [:gduan] [:喬智]]

Hi Alex,
may you upload the profile to dropbox or somewhere I can download and verify? That'll be much helpful!!

Comment 48

[:gerard-majax]

Attachment: profile.l10n.tar.bz2

The attached profile exposes the issue when flashing the system app.

Comment 49

[(Inactive after June) George Duan [:gduan] [:喬智]]

Attachment: 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.

Comment 50

[(Inactive after June) George Duan [:gduan] [:喬智]]

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

Comment 51

[(Inactive after June) George Duan [:gduan] [:喬智]]

Revert this patch since it cause profile 3~4 MB larger than before:

https://github.com/mozilla-b2g/gaia/commit/4da57191b7e3ca153a22d017068f3332f9dcf580

Comment 52

[(Inactive after June) George Duan [:gduan] [:喬智]]

Attachment: 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).

Comment 53

[(Inactive after June) George Duan [:gduan] [:喬智]]

Profile size on master: ~58MB
Profile size after my commit: ~54MB

Comment 54

[Yuren [:yurenju] (AFK ~ Oct. 12) (volunteer mode)]

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.

Comment 55

[Yuren [:yurenju] (AFK ~ Oct. 12) (volunteer mode)]

and pass on travis-ci and try server for all build & all tests

Comment 56

[(Inactive after June) George Duan [:gduan] [:喬智]]

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

Comment 57

[(Inactive after June) George Duan [:gduan] [:喬智]]

merge to master,
https://github.com/mozilla-b2g/gaia/commit/56f79456db5dc3ca010a56d09e1e8cc15a2408db

Comment 58

[(Inactive after June) George Duan [:gduan] [:喬智]]

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|.

Comment 59

[James Burke [:jrburke]]

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.