Closed Bug 1170120 Opened 7 years ago Closed 7 years ago

Move dependency declarations from jsmarionette packages which landed in gaia to their appropriate fs location

Categories

(Testing Graveyard :: JSMarionette, defect)

defect
Not set
normal

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: gaye, Assigned: gaye)

References

Details

Attachments

(4 files, 1 obsolete file)

As a step towards doing local npm modules in gaia, we moved dependencies of various jsmarionette packages into gaia's package.json. We should move them back into the manifests of the modules that use them directly.
Blocks: 1165496
Assignee: nobody → gaye
Comment on attachment 8613480 [details] [review]
[gaia] gaye:bug-1170120 > mozilla-b2g:master

r=self
Attachment #8613480 - Flags: review+
Landed on master https://github.com/mozilla-b2g/gaia/commit/f0e0afa570f1572de100a141191ec0d719c34e16
Status: NEW → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Backed out for causing errors after landing: https://treeherder.mozilla.org/#/jobs?repo=b2g-inbound&revision=fccbe388d4d1

Unfortunately this only broke on b-i and not gaia-try, so you might have to use gaia.json to debug this:  https://wiki.mozilla.org/ReleaseEngineering/TryServer#Using_a_custom_Gaia

https://github.com/mozilla-b2g/gaia/commit/6d477a7884273886605049b20f60af5c1583a150
Status: RESOLVED → REOPENED
Flags: needinfo?(gaye)
Resolution: FIXED → ---
:gaye, I believe this is exactly the thing that :lightsofapollo and I were talking to you about when we rolled out that hack to get local in tree module support in the cache.

npm isn't very smart, and when it sees a top level node_modules folder, it assumes things are super duper awesome. Which in this case they aren't. Running npm install again doesn't always work (most often it fails to catch the fact that deps are missing).

We're planning on correcting these kinds of issues with exhibition which will enforce exact dependencies. It won't just trust npm to say things are great. It does it's own thing which is a lot more automake like.

It's on my Q2 goals to get this running. We should be able to land this... just not yet. Sorry. :(
There's one option though! Eureka!

We could make the npm-cache Makefile target in gaia rebuild the local in-tree modules, ensuring that their dependencies are fulfilled. I'll attach a patch that you can try.
Attachment #8613915 - Flags: review?(gaye)
Going to send :aus patch to try. If it works I'll probably fiddle with it a bit so that it automagically detects all of the local packages specified in gaia's package.json.
Flags: needinfo?(gaye)
Attached patch bug-1170120-try.patch (obsolete) — Splinter Review
:kgrandon or :jgriffin - Would one of you mind pushing to try for me? My hg access depends on bug 925081 since I lost my old ssh key pair... Thanks a million!
Attachment #8614031 - Flags: feedback?(kgrandon)
Attachment #8614031 - Flags: feedback?(jgriffin)
Comment on attachment 8613915 [details] [review]
[gaia] nullaus:bug1170120 > mozilla-b2g:master

Thanks :auswerk! I cleaned this up a little bit and folded into my pr
Attachment #8613915 - Flags: review?(gaye)
Comment on attachment 8613480 [details] [review]
[gaia] gaye:bug-1170120 > mozilla-b2g:master

Hey :aus do you want to take a quick look at bin/rebuild_local_modules.js?
Attachment #8613480 - Flags: review+ → review?(aus)
Comment on attachment 8614031 [details] [diff] [review]
bug-1170120-try.patch

Review of attachment 8614031 [details] [diff] [review]:
-----------------------------------------------------------------

On try:  https://treeherder.mozilla.org/#/jobs?repo=try&revision=53e3aa7f2b2e
Attachment #8614031 - Flags: feedback?(kgrandon)
Attachment #8614031 - Flags: feedback?(jgriffin)
Thanks Kevin! It looks like it worked, so as soon as I get a review from :aus I will land.
Comment on attachment 8614024 [details] [review]
[gaia] gaye:bug-1170120 > mozilla-b2g:master

lgtm :)
Attachment #8614024 - Flags: review+
https://github.com/mozilla-b2g/gaia/commit/2fbadd586461010cabc2f08191f62090172b127e landed on master
Status: REOPENED → RESOLVED
Closed: 7 years ago7 years ago
Resolution: --- → FIXED
Comment on attachment 8613480 [details] [review]
[gaia] gaye:bug-1170120 > mozilla-b2g:master

Review was really for a different PR.
Attachment #8613480 - Flags: review?(aus)
Thing got backed out again. The issue was that although the thing worked on try, it wasn't running my gaia branch since I didn't have a revision specified. The docs said you could either specify a branch OR a revision, but this was not right. We've decided to roll with just running npm install to hydrate the local dependencies for the time being.
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
"Aus! Push me to try!" - the patch
Attachment #8614031 - Attachment is obsolete: true
Flags: needinfo?(aus)
https://github.com/mozilla-b2g/gaia/commit/641edbbe75f6ea85b94153bd58c63b2f49eebc41 landed on master (hopefully for the last time :). Our last push to try looked good.
Status: REOPENED → RESOLVED
Closed: 7 years ago7 years ago
Resolution: --- → FIXED
I definitely remember pushing that patch. :)
Flags: needinfo?(aus)
Product: Testing → Testing Graveyard
You need to log in before you can comment on or make changes to this bug.