1154585 - Refactoring webapp-shared.js for new build system

Status: RESOLVED FIXED

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

Description

[Ricky Chien [:rickychien] (inactive)]

See also new build system plan milestone 1 [1]

Refactoring webapp-shared.js and copyToStage: based on new build system architecture, we'll refactor webapp-shared module and copyToStage into config and execute task in order to save time when re-run make and check if any possible risk before Milestone 2 (refactoring all build modules and app/build.js).

[1] https://wiki.mozilla.org/Gaia/Build/NewBuildSystem

Comment 1

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

a prototype for copyToStage :
https://github.com/cctuan/gaia/commit/213a97c5a6adf0183efece83ae8c29e5fa43251c
it works fine.

Comment 2

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

1st make: 2m28
2nd make: 2m21
lol

Comment 3

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

forgot to remove clean-stage-app.
https://github.com/cctuan/gaia/commit/fd0685dbfc21b35d742dfdea7c4b20c5380cfd2f

Comment 4

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

after put -j8 in configure.js, 
1st make: 1m17.492s
2nd make: 1m15.414s

Comment 5

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

without configure/cp.js (simply cp all the app no matter what)
1st make: 1m9.525s
2nd make: 1m10.774s (should be not much different than 1st make)

with configure/cp.js (replace .PHONY with plain files dependency for copy app)
1st make: 1m17.492s
2nd make: 1m15.414s (should be less since we only copy file that we change)

I think bug 1154583 can save the configuring time if it has efficient way to scan changes.

Comment 6

[Ricky Chien [:rickychien] (inactive)]

WIP branch: https://github.com/RickyChien/gaia/tree/webapp-shared-configure

Comment 7

[Ricky Chien [:rickychien] (inactive)]

Here is some performance records after splitting configure and builder of webapp-shared base on WIP branch:

(Clobber build)
* Run webapp-shared's configure (generate webapp-shared.mk): 11.11s 
* Run webapp-shared.mk (copy a lot of files): 14.48s
** I guess the result of such bad performance indicates that we generate too many `cp` shell commands, and `make` process invokes `cp` processes too many times so it slows down our copy step. 

(Incremental build)
* Modify one shared file and 'time make -f webapp-shared.mk': 0.45s

As above result, we can see that we have a blazing fast incremental build in the new architecture, even though clobber build is slower than old one.

Comment 8

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

Based on you wip, I thought about a new step for each build module, let us name it 'ShouldReconfig' module.
Just as its naming, we let each build step to decide whether to regenerate makefile or not, each module's ShouldReconfig module will only watch what it need to watch,
for example, settings.js will only watch build/config/common-settings.json and GAIA_DEFAULT_LOCALE. Only if those value or file change, we need to rerun config step.

We only reconfig necessary component instead of all if any file or env change. I think that would save more time for incremental build.

Comment 9

[Ricky Chien [:rickychien] (inactive)]

I agree with your point. However, we still have same problem in webapp-shared's configure which should detect the changes of source files in apps. The webapp-shared.js should be rerun if there are some files added or deleted.

Comment 10

[GitHub Autolander Bot]

Attachment: [gaia] rickychien:webapp-shared > mozilla-b2g:master

Comment 11

[Ricky Chien [:rickychien] (inactive)]

Attachment: https://github.com/cctuan/gaia/pull/10

George, I'd like to merge this PR to your branch, please take a look if you feel free.

Comment 12

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

I reset my env and use your build to make, and found out an error of "error: Using inexistent shared page file: import/js/parameters.js from: communications.gaiamobile.org"
as discussed, it might be an issue that some configure steps depend on previous builder result. We probably need to rething the steps, like config -> build -> config -> build ... , or fix contacts-import-services because it generate file which does not exist in gaia tree and break your patch.

Comment 13

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

and also these two json files

/shared/resources/keyboard_layouts.json
/shared/js/search_providers.json

perhaps just remove them from .gitignore and check into gaia.

Comment 14

[Ricky Chien [:rickychien] (inactive)]

Patch has passed linux & mac Gb test on local machine, although there are some failures on treeherder since we've switched to node as run-time which could crash build.

I'm going to land it into your branch once I get r+.

Comment 15

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

Comment on attachment 8612088 [details] [review]
https://github.com/cctuan/gaia/pull/10

I've compared diff with/without your patch and it looks nice. 

Please kindly check my comment on github, if you agree my point, please add some comment in your code or perhaps modify the library, maybe use a list to set dependency automatically.

I still r+ this patch since it works just fine!

Comment 16

[Ricky Chien [:rickychien] (inactive)]

George, could you give me a hand to merge it into your branch because I don't have permission.

And also close this bug once you merge. Thanks!

Comment 17

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

merged!

Comment 18

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

Hi Ricky,
it looks like there's still a bug in below,
https://github.com/cctuan/gaia/blob/config-build/build/webapp-shared.js#L137
https://s3-us-west-2.amazonaws.com/taskcluster-public-artifacts/gHVQnDjzTiyMGG9dNXLiOA/1/public/logs/live_backing.log

Comment 19

[Ricky Chien [:rickychien] (inactive)]

I think we don't need to figure out totally the bustage from gaia-try since it probably take more time to investigate. As an experiment branch, patch passed on my local machine (both on Mac or Linux), so I'm fine to merge it into your branch first.

Comment 20

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

It's easy to reproduce if your repo is cloned from github at first time or done git clean -fdx before run make.

file bug 1170430 for this bug.