Dan Mosedale (:dmosedale, :dmose) Assignee
	Description • 6 years ago

This is the first piece of getting NodeJS into the tree.  Landing it will pave the way for the next chunks of allowing uses of node from moz.build to be debugged in-tree, which will in turn make things easier on the devtools team to get their debugger patches (bug 1461714) in shape for landing.

Landing this should have no effect on the day-to-day workflow of developers.

	Gregory Szorc [:gps]
	Comment 5 • 6 years ago
mozreview-review

Comment on attachment 8995520 [details]
Bug 1478995 - Add node toolchain repack tasks for linux, windows, and mac,

https://reviewboard.mozilla.org/r/259922/#review267394

This is reasonable given existing tools. But I have patches in bugs 1467359 and 1479533 that will enable the script to be replaced by pure YAML. Depending on when this lands, those may have landed. But we can always nuke the script as a follow-up.

	Gregory Szorc [:gps]
	Comment 6 • 6 years ago
mozreview-review

Comment on attachment 8995521 [details]
Bug 1478995 - Add node toolchains to each automated build,

https://reviewboard.mozilla.org/r/259924/#review267398

This all seems pretty reasonable.

::: taskcluster/ci/build/android.yml:49
(Diff revision 1)
>          - android-sdk-linux
>          - linux64-clang
>          - linux64-rust-android
>          - linux64-rust-size
>          - linux64-sccache
> +        - linux64-node

I have a preference for the list being alphebetized. But unless there is a lint enforcing it (there isn't), I'm not inclined for make you change this if you don't want to.

	Gregory Szorc [:gps]
	Comment 7 • 6 years ago
mozreview-review

Comment on attachment 8995522 [details]
Bug 1478995 - Add node toolchain path to automation mozconfigs,

https://reviewboard.mozilla.org/r/259926/#review267400

::: browser/config/mozconfigs/linux32/l10n-mozconfig:4
(Diff revision 1)
>  ac_add_options --with-l10n-base=../../l10n
>  ac_add_options --enable-update-channel=${MOZ_UPDATE_CHANNEL}
>  ac_add_options --with-branding=browser/branding/nightly
> +ac_add_options --disable-nodejs

I wish we had a configure flag that indicated "this is an l10n build" because then we could imply --disable-nodejs. But we don't. So this workaround seems reasonable.

	Dan Mosedale (:dmosedale, :dmose) Assignee
	Comment 8 • 6 years ago
mozreview-review-reply

Comment on attachment 8995521 [details]
Bug 1478995 - Add node toolchains to each automated build,

https://reviewboard.mozilla.org/r/259924/#review267398

> I have a preference for the list being alphebetized. But unless there is a lint enforcing it (there isn't), I'm not inclined for make you change this if you don't want to.

This is a bunch of hand editing that feels like it could be error prone.  Given the length of time this has already taken, I'd rather just land it as- is.  

As far as getting a linter to do the right thing here, perhaps weighing in with this use case at https://github.com/adrienverge/yamllint/issues/123 would help move the ball forward.

	Pulsebot
	Comment 12 • 6 years ago

Pushed by dmosedale@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/8ced503eeb55
Add node toolchain repack tasks for linux, windows, and mac, r=gps
https://hg.mozilla.org/integration/autoland/rev/98578393d635
Add node toolchains to each automated build, r=gps
https://hg.mozilla.org/integration/autoland/rev/51795a3214ea
Add node toolchain path to automation mozconfigs, r=gps

	Daniel Varga [:dvarga]
	Comment 13 • 6 years ago
bugherder

https://hg.mozilla.org/mozilla-central/rev/8ced503eeb55
https://hg.mozilla.org/mozilla-central/rev/98578393d635
https://hg.mozilla.org/mozilla-central/rev/51795a3214ea

	Pulsebot
	Comment 14 • 6 years ago

Pushed by mozilla@jorgk.com:
https://hg.mozilla.org/comm-central/rev/43bfa39c5e32
Port bug 1478995 - added linux64-node and win64-node to toolchains. rs=bustage-fix

	Pulsebot
	Comment 15 • 6 years ago

Pushed by mozilla@hocat.ca:
https://hg.mozilla.org/mozilla-central/rev/3cb90f16402b
Add node toolchain to osx-nightly and win64-asan-reporter builds; r=me a=tomprince

	Pulsebot
	Comment 16 • 6 years ago

Pushed by mozilla@jorgk.com:
https://hg.mozilla.org/comm-central/rev/97c0d4f68e62
Port more of bug 1478995: add linux64-node/win64-node to macosx.yml, linux.yml and windows.yml. rs=bustage-fix

	Rob Lemley [:rjl]
	Comment 17 • 6 years ago

Add node as dependency to nightly TB builds

	Phabricator Automation
	Comment 18 • 6 years ago

Comment on attachment 8997179 [details]
More bug 1478995: add linux/win64-node to nightly configs. rs=bustage-fix

Tom Prince [:tomprince] (limited availability Jul 16-29) has approved the revision.

https://phabricator.services.mozilla.com/D2684

	Pulsebot
	Comment 19 • 6 years ago

Pushed by mozilla@jorgk.com:
https://hg.mozilla.org/comm-central/rev/d8ce3008ef77
Port even more of bug 1478995: add linux64-node/win64-node to nightly configs. r=tomprince DONTBUILD

	Mike Hommey [:glandium]
	Comment 20 • 6 years ago
mozreview-review

Comment on attachment 8995521 [details]
Bug 1478995 - Add node toolchains to each automated build,

https://reviewboard.mozilla.org/r/259924/#review268162

::: taskcluster/ci/toolchain/macosx.yml:37
(Diff revision 2)
>          toolchain-artifact: public/build/clang.tar.xz
>      toolchains:
>          - linux64-cctools-port
>          - linux64-clang-6
>          - linux64-gcc-4.9
> +        - linux64-node

huh? building macosx toolchains doesn't require node.

	Pulsebot
	Comment 21 • 6 years ago

Pushed by ncsoregi@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/228be4ef7ed1
Fix gradle-dependencies bustage for nodejs not found. CLOSED TREE

	Pulsebot
	Comment 22 • 6 years ago

Pushed by btara@mozilla.com:
https://hg.mozilla.org/mozilla-central/rev/3c1c7f965f0f
Fix gradle-dependencies bustage for nodejs not found. CLOSED TREE

	Bogdan Tara[:bogdan_tara | bogdant]
	Comment 23 • 6 years ago
bugherder

https://hg.mozilla.org/mozilla-central/rev/3c1c7f965f0f

	Dorel Luca [:dluca]
	Comment 24 • 6 years ago
bugherder

https://hg.mozilla.org/mozilla-central/rev/228be4ef7ed1

	Tom Prince [:tomprince]
	Comment 25 • 6 years ago

Comment on attachment 8997179 [details]
More bug 1478995: add linux/win64-node to nightly configs. rs=bustage-fix

Tom Prince [:tomprince] has been removed from the revision.

	Mihai Tabara [:mtabara]⌚️GMT
	Comment 27 • 6 years ago

Comment on attachment 9003176 [details]
Bug 1478995: Disable nodejs is source tasks.

Mihai Tabara [:mtabara]⌚️GMT has approved the revision.

	Pulsebot
	Comment 28 • 6 years ago

Pushed by mozilla@hocat.ca:
https://hg.mozilla.org/integration/mozilla-inbound/rev/6ad53d2f8d32
Disable nodejs in source tasks; r=Callek

	Raul Gurzau (:RaulG)
	Comment 29 • 6 years ago
bugherder

https://hg.mozilla.org/mozilla-central/rev/6ad53d2f8d32