1556460 - |mach doc| broken with jsdoc >= 3.6: "No JSDoc documentation was found"

Status: RESOLVED FIXED

(Developer Infrastructure :: Source Documentation, task, P3)

Description

[Andrew Halberstadt [:ahal]]

We are currently pinned to sphinx-js==2.1 because the last time I tried to upgrade there were intermittents in the Doc task. I don't think I spent much time investigating as it was tangential to what I was working on at the time.

We should try to update it again, maybe the intermittents have solved themselves. If not, they are something we're going to need to fix sooner or later in order to pick up the (future) fix in https://github.com/mozilla/sphinx-js/issues/106 .

Comment 1

[Andrew Halberstadt [:ahal]]

Attachment: Bug 1556460 - [docs] Use compatible-release operator instead of * in tools/docs/Pipenv, r?davehunt

Comment 2

[Andrew Halberstadt [:ahal]]

Attachment: Bug 1556460 - [docs] Update to sphinx-js 2.7.1

Depends on D33555

Comment 3

[Andrew Halberstadt [:ahal]]

The above patches update |mach doc| to use sphinx-js==2.7.1. However I'm getting an error when running locally:

$ ./mach doc
...
WARNING: The config value `js_source_path' has type `list', defaults to `str'.
loading pickled environment... done
Cannot parse the config file tools/docs/jsdoc.json: Error: ENOENT: no such file or directory, open 'tools/docs/jsdoc.json'
Exiting JSDoc because an error occurred. See the previous log messages for details.

Sphinx error:
jsdoc found no JS files in the directories [u'/home/ahal/dev/mozilla-central/browser/components/extensions', u'/home/ahal/dev/mozilla-central/testing/marionette', u'/home/ahal/dev/mozilla-central/toolkit/components/extensions', u'/home/ahal/dev/mozilla-central/toolkit/components/extensions/parent', u'/home/ahal/dev/mozilla-central/toolkit/components/featuregates', u'/home/ahal/dev/mozilla-central/toolkit/mozapps/extensions']. Make sure js_source_path is set correctly in conf.py. It is also possible (though unlikely) that jsdoc emitted invalid JSON.
./mach: failed to generate documentation:
/home/ahal/dev/mozilla-central/tools: sphinx return code 2

Erik, any idea what's happening here? Looks like trying to run jsdoc on a dir without JS files is now an error. If it helps I could figure out exactly which version of sphinx-js this starts happening with.

Comment 4

[Andrew Halberstadt [:ahal]]

I think the first patch in that series is something we should do anyway and I see no reason not to land it.. so I'll get that pushed through here and leave-open to fix the sphinx-js update issues.

Comment 5

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/f795f0184e3e
[docs] Use compatible-release operator instead of * in tools/docs/Pipenv, r=davehunt

Comment 6

[Natalia Csoregi [:nataliaCs]]

https://hg.mozilla.org/mozilla-central/rev/f795f0184e3e

Comment 7

[Andrew Halberstadt [:ahal]]

Just putting a few things together here..

To solve this issue we need to:

1. Fix the root problem in sphinx-js
2. Upgrade to the new sphinx-js in ./mach doc (fixing any intermittents along the way)

In the meantime, we should:

1. Print an error message if jsdoc>=3.6 is detected.
2. Provide automatic bootstrapping of jsdoc (see also bug 1498604)

The workaround here is to downgrade jsdoc to version 3.5.x via npm.

Comment 9

[Henrik Skupin [:whimboo][⌚️UTC+2] (away 07/21 - 08/08)]

It's not true (at least for me) that jsdoc 3.6.3 is not working. After I got a similar error on bug 1579022 - which has a slightly different reason and I'm not sure if the dupe to this bug is actually correct - and I reinstalled jsdoc via npm, it's all working fine. So there must have been a broken dep or something else.

Comment 10

[Andrew Halberstadt [:ahal]]

Interesting. Erik is also reporting that the issue seems to be magically resolved as well:
https://github.com/mozilla/sphinx-js/issues/106#issuecomment-531388659

So maybe there's nothing to do here? Maybe we should just use this bug to upgrade sphinx-js since we should do that anyway?

Comment 11

[Erik Rose [:erik][:erikrose]]

Sgtm. I'll cut a 2.8 release with some additional fixes and features (hopefully today) and then put up a patch.

Comment 12

[Andrew Halberstadt [:ahal]]

Thanks! Just in case you missed it:

We are currently pinned to sphinx-js==2.1 because the last time I tried to upgrade there were intermittents in the Doc task.

Though who knows, maybe those issues are also magically solved :).

Comment 13

[Erik Rose [:erik][:erikrose]]

Spent the day getting CI passing with the latest jsdoc. So it's definitely working on 3.6.3 and will continue to. Did we ever finish switching moz-central over to Python 3? I'd like to make the release of sphinx-js after this one Python-3–only.

Comment 14

[Erik Rose [:erik][:erikrose]]

Btw, tests also pass fine with the latest Sphinx, but that version is Python-3–only, so I can't update CI to it just yet. That's my motivation.

Comment 15

[Erik Rose [:erik][:erikrose]]

2.8 release is cut.

Comment 16

[Mike Kaply [:mkaply]]

Attachment: Bug 1556460 - Update min python version to get docs working. DONTBUILD

Comment 17

[Erik Rose [:erik][:erikrose]]

The fix to bug 1232403 fixes this as well.

Comment 18

[Erik Rose [:erik][:erikrose]]

This is really my problem. :-) ahal was just reviewing for me.

Comment 21

[Andrew Halberstadt [:ahal]]

Looks like this didn't get fixed after all. As per bug 1573471 comment 6 I'm still seeing this (and others are hitting it as well). I can reproduce with jsdoc==3.6.3, but downgrading to 3.5.5 fixes the issue. For others hitting this, run npm install -g jsdoc@3.5.5.

Any ideas Erik? The strange thing is I could have sworn it got fixed with your version bump, but now I can reproduce reliably again. Maybe there is some condition / state that needs to be met before the bug surfaces. Though it happens after a clobber as well.

Comment 22

[Andrew Halberstadt [:ahal]]

Not sure if this helps, but I added

+ print(unambiguous_segments)

here:
https://github.com/mozilla/sphinx-js/blob/master/sphinx_js/suffix_tree.py#L18

With jsdoc==3.5.5 I see the output:

[u'./', u'toolkit/', u'components/', u'extensions/', u'ExtensionCommon.', u'ExtensionAPI']

But with jsdoc==3.6.3 that line isn't present. I also managed to slightly simplify the STR to:

./mach doc toolkit/components/extensions/docs

The first problematic file seems to be:
https://searchfox.org/mozilla-central/source/toolkit/components/extensions/ExtensionCommon.jsm

Comment 23

[Andrew Halberstadt [:ahal]]

Ok, managed a simpler STR that doesn't involve sphinx-js at all. Looks like something changed in jsdoc such that APIs which were previously generated are no longer generated. Though there's nothing in the changelog that suggests anything intentional.

STR

$ echo '{"source":{"includePattern": ".+\\.js(doc|x|m)?$"}}' > conf.json
$ npm install -g jsdoc@3.5.5
$ jsdoc -c conf.json toolkit/components/extensions/ExtensionCommon.jsm -d out_3.5.5
$ npm install -g jsdoc@3.6.3
$ jsdoc -c conf.json toolkit/components/extensions/ExtensionCommon.jsm -d out_3.6.3
$ diff -U4 out_3.5.5/ExtensionCommon.jsm.html out_3.6.3/ExtensionCommon.jsm.html

Expected

No diff.

Actual

--- out_3.5.5/ExtensionCommon.jsm.html	2019-11-06 09:52:27.335916916 -0500
+++ out_3.6.0/ExtensionCommon.jsm.html	2019-11-06 09:51:57.156272366 -0500
@@ -2642,15 +2642,15 @@
 
 </div>
 
 <nav>
-    <h2><a href="index.html">Home</a></h2><h3>Classes</h3><ul><li><a href="APIModule.html">APIModule</a></li><li><a href="BaseContext.html">BaseContext</a></li><li><a href="CanOfAPIs.html">CanOfAPIs</a></li><li><a href="EventManager.html">EventManager</a></li><li><a href="ExtensionAPI.html">ExtensionAPI</a></li><li><a href="InnerWindowReference.html">InnerWindowReference</a></li><li><a href="LocalAPIImplementation.html">LocalAPIImplementation</a></li><li><a href="NoCloneSpreadArgs.html">NoCloneSpreadArgs</a></li><li><a href="SchemaAPIManager.html">SchemaAPIManager</a></li><li><a href="SpreadArgs.html">SpreadArgs</a></li></ul><h3>Interfaces</h3><ul><li><a href="SchemaAPIInterface.html">SchemaAPIInterface</a></li></ul><h3>Global</h3><ul><li><a href="global.html#defineLazyGetter">defineLazyGetter</a></li><li><a href="global.html#normalizeTime">normalizeTime</a></li></ul>
+    <h2><a href="index.html">Home</a></h2><h3>Classes</h3><ul><li><a href="APIModule.html">APIModule</a></li><li><a href="LocalAPIImplementation.html">LocalAPIImplementation</a></li><li><a href="SchemaAPIManager.html">SchemaAPIManager</a></li></ul><h3>Interfaces</h3><ul><li><a href="SchemaAPIInterface.html">SchemaAPIInterface</a></li></ul><h3>Global</h3><ul><li><a href="global.html#defineLazyGetter">defineLazyGetter</a></li><li><a href="global.html#normalizeTime">normalizeTime</a></li></ul>
 </nav>
 
 <br class="clear">
 
 <footer>
-    Documentation generated by <a href="https://github.com/jsdoc3/jsdoc">JSDoc 3.5.5</a> on Wed Nov 06 2019 09:52:27 GMT-0500 (Eastern Standard Time)
+    Documentation generated by <a href="https://github.com/jsdoc/jsdoc">JSDoc 3.6.0</a> on Wed Nov 06 2019 09:51:57 GMT-0500 (Eastern Standard Time)
 </footer>
 
 <script> prettyPrint(); </script>
 <script src="scripts/linenumber.js"> </script>

Note the former contains ExtensionAPI while the latter doesn't. It's unclear to me if this is a bug in jsdoc or if the difference is intentional and we need to update our API docs.

I'll leave the needinfo for you Erik in case you have any insight. Also Robert, as the person who initially integrated jsdoc into our docs, do you have any ideas? Or know who can help me figure this out?

Thanks!

Comment 24

[Andrew Halberstadt [:ahal]]

Forgot to post a link to the object:
https://searchfox.org/mozilla-central/source/toolkit/components/extensions/ExtensionCommon.jsm#348

Just a wild guess, but maybe the fact there aren't any @params in the docstring is relevant?

Comment 25

[Robert Helmer [:rhelmer]]

(In reply to Andrew Halberstadt [:ahal] from comment #24)

Forgot to post a link to the object:
https://searchfox.org/mozilla-central/source/toolkit/components/extensions/ExtensionCommon.jsm#348

Just a wild guess, but maybe the fact there aren't any @params in the docstring is relevant?

I think that's a red herring and happens because this happens to be the first js:autoclass encountered; if I remove https://searchfox.org/mozilla-central/source/toolkit/components/extensions/docs/reference.rst#7-10 then the same exception is thrown for the next.

Was this actually fixed in upstream sphinx-js? I thought we were still pinning jsdoc 3.5.5 for the moment.

It'd be nice if mach took care of installing the correct version of jsdoc, since the jsdoc --explain output that sphinx-js uses is not stable AFAIK so I'd expect this to come up in the future, and if we're just telling people to npm install -g jsdoc then it's going to cause some pain.

Comment 26

[Andrew Halberstadt [:ahal]]

There's a bug on file to vendor jsdoc, but we don't have the ability to easily vendor npm modules yet, so no ability to pin. That being said, I do have a patch pending review in bug 1592964 to do a version check and error out if we detect anything but jsdoc==3.5.x.

I guess that will "fix" most of the friction developers have been having here and make this a bit less urgent. It's still something we'll want to fix though.

Comment 27

[Andrew Halberstadt [:ahal]]

(In reply to Robert Helmer [:rhelmer] from comment #25)

I think that's a red herring and happens because this happens to be the first js:autoclass encountered; if I remove https://searchfox.org/mozilla-central/source/toolkit/components/extensions/docs/reference.rst#7-10 then the same exception is thrown for the next.

Yes I agree that there are many failures. But it's not everything as jsdoc is able to successfully process many of the objects in this file. So there's some commonality between the failures (maybe it's all js:autoclass objects).

Interestingly, if I rename ExtensionCommon.jsm to ExtensionCommon.js then everything works again. Plus one of the changes in jsdoc is updating their babel dependency.. Seems fishy. I also found these issues which may be related:
https://github.com/jsdoc/jsdoc/issues/1698
https://github.com/jsdoc/jsdoc/issues/1699

Comment 28

[Erik Rose [:erik][:erikrose]]

The next step I'd take is to diff the jsdoc -x output for 3.5.x and 3.6.x. If there's a difference we can "absorb" in sphinx-js and make things work, I'm not opposed.

But yes, given jsdoc's historic brittleness of both public and --explain output, pinning will ultimately be an important safety latch for moz-central.

Comment 29

[Mark Banner (:standard8)]

Following the landing of jsdoc into node_modules, I've just tried:

$ npm install --save-dev --save-exact jsdoc@3.6.6
$ ./mach doc

and that seemed to work fine. Andrew, does that work for you, or could there still be issues here?

Comment 30

[Andrew Halberstadt [:ahal]]

Yeah, that seems to work for me!

It's possible the issue was fixed when we switched to Python 3? I'm fuzzy on the details in this bug now, but the description mentions intermittents... So I guess we should just bump it, push a bunch of retriggers on try and land it if it looks good.

We'll also want to bump sphinx-js if this succeeds, but that can be a follow-up.

Comment 31

[Mark Banner (:standard8)]

Ok, I'll steal this and give that a run through next week hopefully.

Comment 32

[Andrew Halberstadt [:ahal]]

Actually just remembered there another bug open for sphinx-js and an outreachy contributor is taking it. So please just handle jsdoc here.

Comment 33

[Mark Banner (:standard8)]

I've just been running it through try, and it all seems good... so lets give it a go.

Comment 34

[Mark Banner (:standard8)]

Attachment: Bug 1556460 - Upgrade jsdoc version to 3.6.6. r?ahal!

Comment 35

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/150addefec08
Upgrade jsdoc version to 3.6.6. r=ahal

Comment 36

[Narcis Beleuzu [:NarcisB]]

https://hg.mozilla.org/mozilla-central/rev/150addefec08