977366 - Allow 'make node_modules' to optionally use the git.mozilla mirror

Status: RESOLVED FIXED

(Firefox OS Graveyard :: Gaia, defect)

Description

[Jonathan Griffin (:jgriffin)]

We've set up a git.mozilla.org mirror of https://github.com/mozilla-b2g/gaia-node-modules for use in TBPL; we should update 'make node_modules' so that it optionally uses this, probably based on an ENV variable.

Comment 1

[Jonathan Griffin (:jgriffin)]

Attachment: Link to Github pull-request: https://github.com/mozilla-b2g/gaia/pull/16678

Comment 2

[Gareth Aye [:gaye] (back from PTO)]

Comment on attachment 8382639 [details] [review]
Link to Github pull-request: https://github.com/mozilla-b2g/gaia/pull/16678

Sending review over to Kevin because I am scared of bash and make (and he did this : )

Comment 3

[Kevin Grandon :kgrandon]

Comment on attachment 8382639 [details] [review]
Link to Github pull-request: https://github.com/mozilla-b2g/gaia/pull/16678

I do wish we had tarball support which would make this a bit cleaner. Should be good for now, thanks!

Comment 4

[Julien Wajsberg [:julienw]]

I added a bunch of comments.

More generally, I think someone (probably Yuren and :ochameau) is owning the Makefile and should do the reviews for the Makefile.

Comment 5

[Kevin Grandon :kgrandon]

Comment on attachment 8382639 [details] [review]
Link to Github pull-request: https://github.com/mozilla-b2g/gaia/pull/16678

Right, that's probably true. Yuren should probably review this. Thanks!

Comment 6

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

Comment on attachment 8382639 [details] [review]
Link to Github pull-request: https://github.com/mozilla-b2g/gaia/pull/16678

Makefile rule |modules.tar| should be used to produce modules.tar file, so we shouldn't add |git clone| on rule modules.tar.

I suggest use |?=| define NODE_MODES_SRC with default value "modules.tar" and use a if condition to decide download tar or clone from a git server

here is a sample for it
https://gist.github.com/yurenju/9319829

make sure you have an integration test case to verify it.
https://github.com/mozilla-b2g/gaia/blob/master/build/test/integration/build.test.js

Comment 7

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

you can use |make build-test-integration| to run it.

Comment 8

[Jonathan Griffin (:jgriffin)]

Comment on attachment 8382639 [details] [review]
Link to Github pull-request: https://github.com/mozilla-b2g/gaia/pull/16678

Review comments addressed, tests added!

Comment 9

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

Comment on attachment 8382639 [details] [review]
Link to Github pull-request: https://github.com/mozilla-b2g/gaia/pull/16678

that's great, thank you! :D

Comment 10

[Jonathan Griffin (:jgriffin)]

https://github.com/mozilla-b2g/gaia/commit/66101d5a3c8bbb269966cb1c59ac7464f46657ba

Comment 11

[Hubert Figuiere [:hub]]

v1.3t uplift
https://github.com/mozilla-b2g/gaia/commit/d47f74b7284ef58170c19a2afe1f7825c41898ad