1141356 - utils-xpc.js ls recursive skipped if pattern not matched

Status: RESOLVED FIXED

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

Description

[James Burke [:jrburke]]

I noticed in the email app, when getSharedJs in shared-utils.js was called, it did not include matching .js and .html files in nested folders under the email's js directory.

Tracing it to the ls function in utils-xpc.js, it seems like directories are only recursed if they match the pattern. However, given that the pattern is /\.(js|html)/, it will not recurse into subdirectories.

The fix is to move the `if (recursive && file.isDirectory())` test outside of the pattern match test. I will attach a pull request shortly.

This is particularly important because getting an accurate view of all the JS and HTML files allows the email app's build process to generate the correct cache digest. I will likely ask for 2.2 uplift if this lands in time on master.

Comment 1

[GitHub Autolander Bot]

Attachment: [gaia] jrburke:bug1141356-build-ls-xpc-recurse > mozilla-b2g:master

Comment 2

[Ricky Chien [:rickychien] (inactive)]

Comment on attachment 8574949 [details] [review]
[gaia] jrburke:bug1141356-build-ls-xpc-recurse > mozilla-b2g:master

Thank your for pointing out this bug in build system.
After apply your patch, it seems like breaking our build with exceptions:

Exception: L10nError: "contacts" not found in en-US in app://communications.gaiamobile.org
...
Exception: L10nError: "wait1" not found in en-US in app://ftu.gaiamobile.org
...
[failed] building communications app failed with exit code 1
[failed] building ftu app failed with exit code 1
make: *** [app] Error 1

Comment 3

[GitHub Autolander Bot]

Attachment: [gaia] RickyChien:fix-utils-ls > mozilla-b2g:master

Comment 4

[Ricky Chien [:rickychien] (inactive)]

Comment on attachment 8575135 [details] [review]
[gaia] RickyChien:fix-utils-ls > mozilla-b2g:master

utils.ls() has this issue for a long time, and patch has been submitted to fix this issue. For fixing it, I decided to remove pattern & include parameters and tweaked all utils.ls calls in source tree. For now, you should filter your file by doing Array.prototype.filter by yourself.

James Burke, please apply my patch and verify locally.

George, patch has passed my diff-compare.sh, result was same as last time.

Comment 5

[James Burke [:jrburke]]

Comment on attachment 8575135 [details] [review]
[gaia] RickyChien:fix-utils-ls > mozilla-b2g:master

Verified locally that email build process gets the full recursive set of files, thank you!

Comment 6

[GitHub Autolander Bot]

Attachment: [gaia] cctuan:1141356 > mozilla-b2g:master

Comment 7

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

steal it

Comment 8

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

in new utils.ls, we let dev to filter the file by file.path, however, some of original pattern might go wrong, for example, if dev try to filter /tests?/ , it will check the whole path string instead of leaf name of its children, so it might return a null array if gaia is under test folder (test/gaia/build_stage/....).

So, we need to add the parent folder path to the regex to make sure we only filter things we don't want.

Comment 9

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

Comment on attachment 8576438 [details] [review]
[gaia] cctuan:1141356 > mozilla-b2g:master

Hi Ricky,
I fixed comment 8. could you please check? Thanks.

Comment 10

[Ricky Chien [:rickychien] (inactive)]

Comment on attachment 8576438 [details] [review]
[gaia] cctuan:1141356 > mozilla-b2g:master

Gaia-try is green finally. In case of breaking our b2g-inbound, I suggest that we push to try for preventing path problem of Windows.

r=rickychien,

Comment 11

[Ricky Chien [:rickychien] (inactive)]

Hey George, are you going to land it?

Comment 12

[James Burke [:jrburke]]

I would be happy to see this land -- I am doing a larger email refactor, part of it is separating out pieces that might have been in the main build layer to other files.

This change would help me as I push changes, since right now a new push does not always recognize that a change has occurred and an old cache is used, sometimes causing confusion for me while debugging.

Comment 13

[GitHub Autolander Bot]

Attachment: [gaia] cctuan:1141356 > mozilla-b2g:master

Comment 14

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

Comment on attachment 8601341 [details] [review]
[gaia] cctuan:1141356 > mozilla-b2g:master

Hi Ricky, 
could you r+ it again? this patch is the same as previous one and tests are all green.

Comment 15

[Ricky Chien [:rickychien] (inactive)]

Comment on attachment 8601341 [details] [review]
[gaia] cctuan:1141356 > mozilla-b2g:master

Sure! Let's land it.

Comment 16

[Ricky Chien [:rickychien] (inactive)]

Land in master: https://github.com/mozilla-b2g/gaia/commit/ecf3b699dad846a444febeb296837032337b98b3

Comment 17

[James Burke [:jrburke]]

It is working for me now, thank you!

Comment 18

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

I had to revert this in https://github.com/mozilla-b2g/gaia/commit/dc52bcfb2439b8239744a47d101baedfe5eca7a4 because it probably broke b2g desktop windows builds:
https://treeherder.mozilla.org/logviewer.html#?job_id=1853036&repo=b2g-inbound

Comment 19

[Ricky Chien [:rickychien] (inactive)]

Steal this bug!

Comment 20

[GitHub Autolander Bot]

Attachment: [gaia] RickyChien:fix-utils-ls > mozilla-b2g:master

Comment 21

[Ricky Chien [:rickychien] (inactive)]

Root issue probably cause by a filter regexp doesn't support windows path.

I've submitted a fix for it

https://github.com/mozilla-b2g/gaia/pull/29913/files#diff-28c8971f32dbfa56726ec51760967c82R639

Comment 22

[Ricky Chien [:rickychien] (inactive)]

Push to try:

https://treeherder.mozilla.org/#/jobs?repo=try&revision=3f7776609bf4

Comment 23

[Ricky Chien [:rickychien] (inactive)]

Push to try #2:

https://treeherder.mozilla.org/#/jobs?repo=try&revision=17831b85c980

Comment 24

[Ricky Chien [:rickychien] (inactive)]

I guess there are some try problems causing try job crash. 

Push to try #3:

https://treeherder.mozilla.org/#/jobs?repo=try&revision=6ffe995b6442

Comment 25

[Ricky Chien [:rickychien] (inactive)]

Comment on attachment 8602038 [details] [review]
[gaia] RickyChien:fix-utils-ls > mozilla-b2g:master

Windows path issue has been fixed, please review it again =)

Comment 26

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

Comment on attachment 8602038 [details] [review]
[gaia] RickyChien:fix-utils-ls > mozilla-b2g:master

Thank you to pick this bug , r+!

Comment 27

[Ricky Chien [:rickychien] (inactive)]

Landed in master: https://github.com/mozilla-b2g/gaia/commit/9ef6f94298277d656a1048c7b752685b16167515

Comment 28

[Carsten Book [:Tomcat]]

sorry had to revert this for bustage like https://treeherder.mozilla.org/logviewer.html#?job_id=1867423&repo=b2g-inbound

Comment 29

[GitHub Autolander Bot]

Attachment: [gaia] RickyChien:fix-utils-ls > mozilla-b2g:master

Comment 30

[Ricky Chien [:rickychien] (inactive)]

Since try doesn't output any path errors on Windows build, I want to land it directly and back-out immediately if we meet build bustage again.

Land in master: https://github.com/mozilla-b2g/gaia/commit/4d793fb125bd44be0bc5e924b5251b01db005116

Comment 31

[GitHub Autolander Bot]

Attachment: [gaia] mozilla-b2g:revert-29982-fix-utils-ls > mozilla-b2g:master

Comment 32

[Ricky Chien [:rickychien] (inactive)]

Revert it again https://github.com/mozilla-b2g/gaia/commit/5b2a150f6f5d29bddfaac13fcbbf099376f2f275

Comment 33

[GitHub Autolander Bot]

Attachment: [gaia] RickyChien:fix-utils-ls > mozilla-b2g:master

Comment 34

[Ricky Chien [:rickychien] (inactive)]

Land in master and see b2g-inbound status:

https://github.com/mozilla-b2g/gaia/commit/0974a80e81d76f4e8c5b5e39f1e983c5d5dad3be

I will back-out it again once it crash windows build.

Comment 35

[GitHub Autolander Bot]

Attachment: [gaia] mozilla-b2g:revert-29991-fix-utils-ls > mozilla-b2g:master

Comment 36

[GitHub Autolander Bot]

Attachment: [gaia] RickyChien:fix-utils-ls > mozilla-b2g:master

Comment 37

[Ricky Chien [:rickychien] (inactive)]

Land in master since issue has been reproduced on Windows and fixed.

https://github.com/mozilla-b2g/gaia/commit/3b06584f664746c99eff10d69256838a567c66bc