1343624 - Install nodejs and run yarn install as part of Vagrant provision

Status: RESOLVED FIXED

(Tree Management :: Treeherder, enhancement, P2)

Description

[Ed Morley [:emorley]]

Bug 1336556 is switching from grunt to webpack, to help unblock the test-centric UI work (bug 1059770).

As part of that, the UI assets will no longer work without being built or else served via the neutrino dev server.

However at the moment nodejs isn't even installed in the Vagrant environment.

In addition to installing nodejs, we'll need to:
1) Run npm install, to save people having to do so themselves
2) Check whether everything still works on Windows, where virtualbox shared folders doesn't natively support symlinks (since node_modules will be in the shared folder)
3) Decide whether we should at the same time switch to newer nodejs and/or yarn, or save for later. (We might need to do this as part of #2, otherwise perhaps we should fix separately).

Comment 1

[Ed Morley [:emorley]]

As I thought may happen, the out of the box settings give symlink related errors when running Vagrant on a Windows host, since the npm install tries to create symlinks in node_modules, which is in the shared folder.

Options appear to be (http://kmile.nl/post/73956428426/npm-vagrant-and-symlinks-on-windows):
1) Run vagrant commands in an admin windows shell (a pain)
2) Use the Windows security policy editor to allow non-admin use of symlinks (allows them system wide which isn't ideal, requires extra setup steps)
3) Use the npm/yarn `--no-bin-links` option (though then means full path has to be specified eg in package.json lifecycle scripts)

Frustratingly, from inspecting both the npm and yarn sources, it seems that there is support for adding redirection shell scripts (instead of symlinks), however this is only used if `process.platform == 'win32'`. As-is `--no-bin-links` just skips creating them entirely. I'm going to see about reporting/fixing this upstream (if they'll accept it), though won't block this bug on it.

Ideally whatever solution is chosen will also allow the same node_modules to be used both inside and outside of the VM (eg if nodejs installed on the host and just wanting to do frontend development). However initial efforts to do that on Windows resulted in:
https://emorley.pastebin.mozilla.org/8980904

Eugh.

Comment 2

[GitHub Autolander Bot]

Attachment: [treeherder] mozilla:vagrant-nodejs-yarn > mozilla:master

Comment 3

[Ed Morley [:emorley]]

Comment on attachment 8845390 [details] [review]
[treeherder] mozilla:vagrant-nodejs-yarn > mozilla:master

The "npm/yarn install as part of provision" part is more involved for the reasons in comment 1, so I'll tackle that in a later PR.

At least now nodejs/yarn will exist in the Vagrant environment, which will let us all more easily try out Casey's Neutrino PR in the meantime.

Comment 4

[William Lachance (:wlach)]

Comment on attachment 8845390 [details] [review]
[treeherder] mozilla:vagrant-nodejs-yarn > mozilla:master

Great stuff, thank you

Comment 5

[Treeherder GitHub Bugbot]

Commit pushed to master at https://github.com/mozilla/treeherder

https://github.com/mozilla/treeherder/commit/a066f335b5f87a391d09eb1ec8dfd92b2b06bb93
Bug 1343624 - Vagrant: Install nodejs and yarn during provision

Since once we switch to Neutrino/webpack all UI and full-stack
development will require building the UI using nodejs.

Uses the steps here:
https://github.com/nodesource/distributions#manual-installation
https://yarnpkg.com/en/docs/install#linux-tab

Comment 6

[Ed Morley [:emorley]]

npm really is awful:

vagrant ~/treeherder $ npm install --no-bin-links
npm ERR! Linux 3.13.0-112-generic
npm ERR! argv "/usr/bin/nodejs" "/usr/bin/npm" "install" "--no-bin-links"
npm ERR! node v7.7.2
npm ERR! npm  v4.1.2

npm ERR! Maximum call stack size exceeded
npm ERR!
npm ERR! If you need help, you may report this error at:
npm ERR!     <https://github.com/npm/npm/issues>

npm ERR! Please include the following file with any support request:
npm ERR!     /home/vagrant/treeherder/npm-debug.log


The debug log has:

183615 verbose stack RangeError: Maximum call stack size exceeded
183615 verbose stack     at emitThree (events.js:116:13)
183615 verbose stack     at module.exports.emit (events.js:197:7)
183615 verbose stack     at module.exports.<anonymous> (/usr/lib/node_modules/npm/node_modules/npmlog/node_modules/are-we-there-yet/tracker-group.js:23:18)
183615 verbose stack     at emitThree (events.js:116:13)
183615 verbose stack     at module.exports.emit (events.js:197:7)
183615 verbose stack     at module.exports.<anonymous> (/usr/lib/node_modules/npm/node_modules/npmlog/node_modules/are-we-there-yet/tracker-group.js:23:18)
183615 verbose stack     at emitThree (events.js:116:13)
183615 verbose stack     at module.exports.emit (events.js:197:7)
183615 verbose stack     at module.exports.<anonymous> (/usr/lib/node_modules/npm/node_modules/npmlog/node_modules/are-we-there-yet/tracker-group.js:23:18)
183615 verbose stack     at emitThree (events.js:116:13)
183615 verbose stack     at module.exports.emit (events.js:197:7)


Looks like:
https://github.com/npm/npm/issues/10776

Comment 7

[Ed Morley [:emorley]]

Too much breaks unless we switch to yarn at the same time, let's just do bug 1343928 as part of this.

Comment 8

[GitHub Autolander Bot]

Attachment: [treeherder] mozilla:yarn-install > mozilla:master

Comment 9

[William Lachance (:wlach)]

Comment on attachment 8846185 [details] [review]
[treeherder] mozilla:yarn-install > mozilla:master

This looks great! I assume you grepped the docs for any old instructions that talked about grunt. I'd also recommend a quick post to m.tools.treeherder telling people about this change.

Comment 10

[Ed Morley [:emorley]]

(In reply to William Lachance (:wlach) (use needinfo!) from comment #9)
> This looks great! I assume you grepped the docs for any old instructions
> that talked about grunt.

There are docs updates in the PR - if you mean did I double check there were no stragglers, yeah I checked for references to `grunt`, `karma and `grunt` and I believe I've got them all.

> I'd also recommend a quick post to m.tools.treeherder telling people about this change.

I'll probably post about this anyway, just as an intro to the changes coming up in bug 1336556, however this particular PR doesn't actually change most people's workflows, since previously unless someone went out of their way to manually install nodejs in their Vagrant environment, they wouldn't have touched grunt or karma anyway (I rarely bothered to do so). For those that use nodejs outside the VM, that workflow will continue to work with this PR, since they will still have grunt-cli installed globally outside the VM.

Bug 1336556 is going to be the significant change for everyone -- to the extent that I was going to get as many people to try it out before landing as possible (I'll set needinfos once this bug landed and the Neutrino PR rebased on top, so people didn't manually have to install node to try it).

Comment 11

[Ed Morley [:emorley]]

(In reply to Ed Morley [:emorley] from comment #10)
> I checked for references to `grunt`, `karma and `grunt`

`grunt`, `karma and `npm`, even :-)

Comment 12

[Treeherder GitHub Bugbot]

Commits pushed to master at https://github.com/mozilla/treeherder

https://github.com/mozilla/treeherder/commit/21abe6ed75eb20d046705f2233a3c1bc87bd0e7a
Bug 1343624 - Add `npm run build` as an alias of `grunt build`

Routing commands via npm/yarn is preferred, since it avoids
having to do global installs of grunt-cli, which simplifies contributor
setup, and means less effort when we switch to Yarn (since it requires
manual PATH setup for globally installed packages).

https://github.com/mozilla/treeherder/commit/245696ef5368ac8d3c1978c0796e685430b9b52f
Bug 1343624 - Add `npm run lint` as an alias of `grunt checkjs`

For the same reason as the previous commit.

Ideally we'd remove the grunt abstraction entirely and call eslint from
the `lint` command, but we might as well save that to the Neutrino PR.

https://github.com/mozilla/treeherder/commit/7b533c6b68d8c70486d6c5df9ae1d75a8befe5f2
Bug 1343624 - Use `npm test` alias instead of old Karma wrapper scripts

The `test` script entry in `package.json` (used by `npm test`) already
calls karma with the appropriate parameters, so the helper scripts are
unnecessary.

https://github.com/mozilla/treeherder/commit/a1bf9e19e2ad44072d03ccf47edfcc0194d3958d
Bug 1343624 - Make package.json scripts work with --no-bin-symlinks

Node packages that are intended to be called from the command line can
request that symlinks be added to `node_modules/.bin/` as part of their
installation. When `npm/yarn run` is used, it automatically adds that
directory to the PATH, so commands listed in `scripts` in `package.json`
can normally be specified without the full filepath.

However when npm/yarn is used inside a Linux Vagrant instance running on
a Windows host, even though the guest OS supports symlinks, errors will
occur if symlinks are created in a directory that is shared with the
host using Virtualbox shared folders.

In this case, the workaround is to prevent the creation of symlinks
using `--no-bin-links`. However unfortunately instead of having a
sensible fallback (eg a shell script that acts like a symlink) instead
no files are created in `node_modules/.bin/` at all.

As such, we have to use full paths in `package.json` after all.

https://github.com/mozilla/treeherder/commit/c77f8ff85c07da28ea01f725f6aaf8567db32f1e
Bug 1343624 - Vagrant: Run yarn install during provision

This will give a working environment out of the box, which will be
necessary once the Neutrino work lands in bug 1336556.

Comment 13

[Ed Morley [:emorley]]

Once https://github.com/mozilla/treeherder/pull/2159 is rebased against this, I'll send out a combined email to the mailing list about the yarn switch and requesting people try out the Neutrino PR.