1491028 - (vendor-eslint) migrate eslint (mostly) from local packages to vendored node_modules

Status: NEW

(Developer Infrastructure :: Lint and Formatting, task, P2)

Description

[Dan Mosedale (:dmosedale, :dmose)]

Since eslint currently expects to use/control the node_modules space, it needs to be the first package migrated to the new node_modules structure.  

http://bit.ly/2x6gcaw has some basic notes.

Standard8, :ryanvm wondered if it would be possible to break out the work that makes eslint use the NODEJS found by configure and land that first (maybe as a dependent bug), so that he can remove NodeJS from MozillaBuild (bug remove-mozilla-build-node).  What are your thoughts?

Comment 1

[Mark Banner (:standard8)]

(In reply to Dan Mosedale (:dmose, :dmosedale) from comment #0)
> Standard8, :ryanvm wondered if it would be possible to break out the work
> that makes eslint use the NODEJS found by configure and land that first
> (maybe as a dependent bug), so that he can remove NodeJS from MozillaBuild
> (bug remove-mozilla-build-node).  What are your thoughts?

Discussed on irc and decided bug 1482435 would be the one to that work.

Comment 2

[Mark Banner (:standard8)]

I think the biggest question here, is do we have sufficient cross-platform & vcs support for symlinks in node_modules?

I can see two options: 1) check in the symlinks to source control. 2) Manually construct the symlinks when we need them (e.g. ESLint's setup routine).

Gergory, do you have any recommendations here?

Currently in an installed node_modules directory we have:

- symlinks in .bin/, e.g:

node_modules/.bin/eslint -> ../eslint/bin/eslint.js

We don't actually use this (we call the eslint.js script direct via node), but we might want to use similar links for other node/npm commands I guess?

- symlinks for the eslint-plugins, i.e.:

node_modules/eslint-plugin-mozilla -> ../tools/lint/eslint/eslint-plugin-mozilla
node_modules/eslint-plugin-spidermonkey-js -> ../tools/lint/eslint/eslint-plugin-spidermonkey-js

These are installed as a result of the file:/ urls. I think if we're vendoring node_modules, then we don't really want to move the sources for those into node_modules - though I'm welcome to opinions here.

Comment 3

[Mark Banner (:standard8)]

I think the hg hooks have just answered the symlink question this for me:

remote: ****************************** ERROR *******************************
remote: 3ae69eef7bad adds or modifies the following symlinks:
remote: 
remote:   node_modules/.bin/acorn
...
remote: Symlinks aren't allowed in this repo. Convert these paths to regular
remote: files and try your push again.
remote: ********************************************************************

So I think we'll still need ESLint to add the symlinks for eslint-plugin-* directories, unless there's better ideas.

Comment 4

[Mark Banner (:standard8)]

Experimental try push with symlinks created via support scripts:

https://treeherder.mozilla.org/#/jobs?repo=try&revision=423f8f2302db75f5d478e9e5376ef6a8caa284b4

I think this is probably going to be the way we need to go with this.

Comment 5

[Dan Mosedale (:dmosedale, :dmose)]

(In reply to Mark Banner (:standard8) from comment #3)
> I think the hg hooks have just answered the symlink question this for me:
> 
> remote: ****************************** ERROR *******************************
> remote: 3ae69eef7bad adds or modifies the following symlinks:
> remote: 
> remote:   node_modules/.bin/acorn
> ...
> remote: Symlinks aren't allowed in this repo. Convert these paths to regular
> remote: files and try your push again.
> remote: ********************************************************************
> 
> So I think we'll still need ESLint to add the symlinks for eslint-plugin-*
> directories, unless there's better ideas.

It would be interesting to know the rationale for this.  It's possible, after all, that the economics around this decision have changed since it was made, and that maybe it's worth undoing.  gps, it would be helpful to know your thoughts here...

Comment 6

[Gregory Szorc [:gps]]

The commit message for https://hg.mozilla.org/hgcustom/version-control-tools/rev/02675c286d0f explains the rationale.

I *really* don't want to open the flood gates and allow symlinks in the repo. We'll just be opening ourselves up to a lot of pain from Windows users not committing the right data into version control.

Comment 7

[Mark Banner (:standard8)]

Attachment: Bug 1491028 - Stop ignoring the top-level node_modules directory in preparation for vendoring node_modules.

Comment 8

[Mark Banner (:standard8)]

Attachment: Bug 1491028 - Vendor node_modules for ESLint into mozilla-central.

Depends on D9424

Comment 9

[Mark Banner (:standard8)]

Attachment: Bug 1491028 - Stop packaging node_modules for the ESLint builder, use the vendor node_modules instead.

Depends on D9425

Comment 10

[Mark Banner (:standard8)]

Attachment: Bug 1491028 - Simplify ESLint's setup to only install the Mozilla ESLint modules.

Depends on D9426

Comment 11

[Mark Banner (:standard8)]

Try push: https://bugzilla.mozilla.org/show_bug.cgi?id=1491028

Comment 12

[Dan Mosedale (:dmosedale, :dmose)]

Mark, do you have another link to that try push :-) ?

Comment 13

[Mark Banner (:standard8)]

That should be: https://treeherder.mozilla.org/#/jobs?repo=try&revision=ea4d7653fa66cbc9544d0dffe8472af1955992f8

Comment 14

[Emma Humphries ☕️🎸🧞‍♀️✨ (she/they) [:emceeaich] (Pacific Time) use needinfo]

This is blocking another bug, is this a P1 or P2 now?

Comment 15

[Mark Banner (:standard8)]

At the moment, this is blocked by non-bug work for getting approvals for having node_modules vendored in tree and sign-off on how we review security etc to make that happen.

Once that's done this is probably a P1, but I'll stick it as P2 for now.

Comment 16

[Mark Banner (:standard8)]

Quite happy to take this up again once we get the go ahead for vendoring, but at the moment dropping from my assigned list.