1498604 - './mach doc' should not require globally installed jsdoc (vendor it)

Status: RESOLVED FIXED

(Developer Infrastructure :: Source Documentation, enhancement, P2)

Description

[Mark Banner (:standard8)]

Currently, running `./mach doc` requires to have jsdoc installed globally. However there's nothing that tells the developer this.

It also doesn't give control over which version is used, so a developer could be on an out-of-date or too new version and get strange errors.

Lastly it is one thing that requires node to be installed globally in our docker images, rather than using the node that the build system will use.

https://searchfox.org/mozilla-central/rev/28fe656de6a022d1fc01bbd3811023fca99cf223/taskcluster/docker/lint/system-setup.sh#46-58


Given the current moves to have vendor node_modules, and node as part of the build system. I think we should either:

1) Add jsdoc as a vendored item in node_modules (after other bugs are done, e.g. bug 1491021 and bug 1491028).

2) Somehow install jsdoc into .mozbuild & use that version.

I'm leaning towards the first option here, as that seems easier to set up and more consistent with how we're running everything.

Comment 1

[Dan Mosedale (:dmosedale, :dmose)]

Vendoring it in sounds like the right plan to me.

Comment 2

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

Per conversation with :kmoir, I'm going through untriaged bugs in her components and marking the ones which look to be enhancements/tasks with the enhancement severity to get them out of the triage queue.

If this incorrect, please remove the tag.

Comment 3

[Nicholas Nethercote [inactive]]

I hit this today. I ran ./mach doc on my Linux box and it told me to run npm install -g jsdoc@3.5.5, so I did, and then it failed with lots of messages like this:

npm WARN npm npm does not support Node.js v10.15.2
npm WARN npm You should probably upgrade to a newer version of node as we
npm WARN npm can't make any promises that npm will work with this version.
npm WARN npm Supported releases of Node.js are the latest release of 4, 6, 7, 8, 9.
npm WARN npm You can find the latest version at https://nodejs.org/
npm WARN checkPermissions Missing write access to /usr/local/lib/node_modules/jsdoc/node_modules/@babel/parser
npm WARN checkPermissions Missing write access to /usr/local/lib/node_modules/jsdoc/node_modules/entities
... omit many more lines like this ...
npm WARN checkPermissions Missing write access to /usr/local/lib/node_modules/jsdoc/node_modules
npm WARN checkPermissions Missing write access to /usr/local/lib/node_modules
npm ERR! path /usr/local/lib/node_modules/jsdoc/node_modules/@babel/parser
npm ERR! code EACCES
npm ERR! errno -13
npm ERR! syscall access
npm ERR! Error: EACCES: permission denied, access '/usr/local/lib/node_modules/jsdoc/node_modules/@babel/parser'
npm ERR!  { [Error: EACCES: permission denied, access '/usr/local/lib/node_modules/jsdoc/node_modules/@babel/parser']
npm ERR!   stack:
npm ERR!    'Error: EACCES: permission denied, access \'/usr/local/lib/node_modules/jsdoc/node_modules/@babel/parser\'',
npm ERR!   errno: -13,
npm ERR!   code: 'EACCES',
npm ERR!   syscall: 'access',
npm ERR!   path:
npm ERR!    '/usr/local/lib/node_modules/jsdoc/node_modules/@babel/parser' }
npm ERR! 
npm ERR! The operation was rejected by your operating system.
npm ERR! It is likely you do not have the permissions to access this file as the current user
npm ERR! 
npm ERR! If you believe this might be a permissions issue, please double-check the
npm ERR! permissions of the file and its containing directories, or try running
npm ERR! the command again as root/Administrator (though this is not recommended).

And I'm not sure what to do next.

Comment 4

[Nicholas Nethercote [inactive]]

With Botond's help, I ended up running npm install jsdoc@3.5.5 (with no -g) in a directory $D and then added $D/node_modules/.bin to my PATH.

Comment 5

[Dan Mosedale (:dmosedale, :dmose)]

:njn Those error messages make it seem like you have a very old version of npm in your path. If you put the one in .mozbuild in your path, it shouldn't have the issue described above.

Comment 6

[Nicholas Nethercote [inactive]]

Thanks for the tip!

Given that we have npm in .mozbuild, I am surprised that mach doc has to find it via PATH instead of just using it directly. Is there a reason for that?

Comment 7

[Mark Banner (:standard8)]

./mach doc was written before we did all the node improvements. I suspect it just hasn't received the same intention.

My hope is to switch jsdoc to locally installed sometime soon (even if we haven't got the vendoring done), though realistically that's unlikely to happen for a couple of cycles. Though I will quite happily advise anyone else wanting to pick it up.

Comment 8

[Dan Mosedale (:dmosedale, :dmose)]

I've filed bug 1652173 and implemented mach node and mach npm there. Once that lands, we could change the message from suggesting npm install -g jsdoc@3.5.5 to suggesting mach npm install -g jsdoc@3.5.5 and people could avoid the problem that :njn had.

Comment 9

[Shivam Singhal [ :championshuttler ]]

Attachment: Bug 1498604 - Update the message to use mach npm now.r?#firefox-source-docs-reviewers

Comment 10

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/2d5fff240adc
Update the message to use mach npm now.r=dmose

Comment 11

[Dan Mosedale (:dmosedale, :dmose)]

Thanks, :championshuttler!

Comment 12

[Alexandru Michis [:malexandru]]

https://hg.mozilla.org/mozilla-central/rev/2d5fff240adc

Comment 13

[James Teh [:Jamie]]

I haven't updated Windows mozilla-build in a while and I don't have time to do this right now, but commenting here in case this is still a problem even with latest mozilla-build:

On Windows, when I run mach doc, I'm told to run mach npm install -g jsdoc@3.5.5. However, running mach doc again just tells me to run mach npm install -g jsdoc@3.5.5 again; it doesn't find jsdoc.

It seems jsdoc gets installed to ~/.mozbuild/node, but ~/.mozbuild/node isn't in my PATH. Running this gets around this (though then I get another error I haven't dealt with yet):
PATH=$PATH:~/.mozbuild/node mach doc

Comment 14

[Michael Comella (:mcomella) [NI reported issues only: ex-Mozilla]]

On macOS, I had a similar issue to comment 13. I ran the suggested command mach npm install -g jsdoc@3.5.5, which installed to homebrew /usr/local/Cellar/node/15.3.0/bin/jsdoc -> /usr/local/Cellar/node/15.3.0/lib/node_modules/jsdoc/jsdoc.js. Subsequently running mach doc gave me the same error. Adding that homebrew directory to my path fixed the error: export PATH=$PATH:/usr/local/Cellar/node/15.3.0/bin/.

Comment 15

[Dan Mosedale (:dmosedale, :dmose)]

It shouldn't have to be added to your path, we should just fix mach doc to the ~/.mozbuild version of node.

Comment 16

[Mark Banner (:standard8)]

Dan, I was holding off on this work because of the potential for the vendoring to happen, however I'm now thinking that we might as well get it done, and have this treated like the ESLint packages are currently.

I don't think this would be hard to do. Looking at the license graph, the only thing that's slightly suspect is TaffyDB, but reading the repo, it seems that there just isn't quite complete clarity there.

What do you think?

Comment 17

[Dan Mosedale (:dmosedale, :dmose)]

That seems reasonable, though my suggestion from comment 15 could be ok (though not as nice) too.

I wonder if it's depending on an old version of TaffyDB before the License file landed. If that's the case, and we can't get it upgraded to a newer version, last I heard, dnazer has taken over approval for these sorts of things from mhoye.

Comment 18

[Akshat [:baka]]

I faced same error today and doing mach npm install -g jsdoc@3.5.5 did not help. I had to do what was mentioned in comment 13

Comment 20

[Mark Banner (:standard8)]

I'm going to tentatively take this and talk to our license people to see if we have any issues with the TaffyDB license or including jsdoc (with potential for vendoring later).

Comment 21

[Mark Banner (:standard8)]

I've spoken with Daniel Nazer and we've agreed that it is reasonable to go with the one-clause BSD that's is in the taffy.js file. This is compatible with our licensing so we are clear to go ahead.

Comment 22

[Mark Banner (:standard8)]

Attachment: Bug 1498604 - Make './mach doc' use jsdoc installed into node_modules rather than the system. r?Mossop!,ahal!

Comment 23

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/53d4fb9f562f
Make './mach doc' use jsdoc installed into node_modules rather than the system. r=mossop,ahal

Comment 24

[Alexandru Michis [:malexandru]]

Backed out changeset 53d4fb9f562f (Bug 1498604) for causing doc generate failures.
Backout link: https://hg.mozilla.org/integration/autoland/rev/858cd731bce94fba0cf794c61a4cf3947470e30c
Push with failures, failure log.

Comment 25

[Mark Banner (:standard8)]

Unfortunately I'd missed running the actual generation on try :( I've now fixed that, so hopefully will be good.

Comment 26

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/3425029be980
Make './mach doc' use jsdoc installed into node_modules rather than the system. r=mossop,ahal

Comment 27

[Narcis Beleuzu [:NarcisB]]

https://hg.mozilla.org/mozilla-central/rev/3425029be980