1232403 - |mach doc| is broken on Windows

Status: RESOLVED FIXED

(Firefox Build System :: General, defect)

Description

[Alessio Placitelli [:Dexter]]

It appears that ./mach doc is broken on Windows (at least on Windows 10): I wasn't able to generate the in-tree documentation on two machines.

--- Failure Log ---

Running Sphinx v1.3.3
making output directory...
Reading Sphinx metadata from build configuration
Staging static documentation

Exception occurred:
  File "c:\mozilla-build\python\lib\shutil.py", line 98, in copystat
    os.utime(dst, (st.st_atime, st.st_mtime))
WindowsError: [Error 3] Impossibile trovare il percorso specificato: u'c:\\mozilla-central\\obj-i686-pc-mingw32\\docs\\html\\Mozilla_Source_Tree_Docs\\_staging\\browser\\base\\sslerrorreport\\dataformat.rst'
The full traceback has been saved in c:\users\alessio\appdata\local\temp\sphinx-err-jgp690.log, if you want to report the issue to the developers.
Please also report this if it was a user error, so that a better error message can be provided next time.
A bug report can be filed in the tracker at <https://github.com/sphinx-doc/sphinx/issues>. Thanks!
./mach: failed to generate documentation:
c:\mozilla-central\tools: sphinx return code 1
--- End ---

I traced down the issue on the plane back home. Since Windows doesn't support symlinks, we end up in [0], which doesn't create the full path if the destination directory doesn't exist (as os.symlink seems to do instead). Copying silently fails as the path doesn't exist, and so does "copystat", triggering the error.

The ugly hack I resorted to was to add something like the following in [0]

if not os.path.exists(dest):
   os.makedirs(dest)

This works, and I'm able to generate an incomplete documentation (it's not picking up all the files, but that's another issue).

[0] - https://dxr.mozilla.org/mozilla-central/rev/99137d6d4061f408ae0869122649d8bdf489cc30/python/mozbuild/mozpack/files.py#305

Comment 1

[Alessio Placitelli [:Dexter]]

Attachment: sphinx-err-jgp690.log

The full log mentioned by the failing command.

Comment 2

[Alessio Placitelli [:Dexter]]

Attachment: test_doc.patch

I gave this bug another try, as ./mach doc is still broken on Windows. This patch moves things forward a bit, but now it fails differently.

> ./mach doc

- fails https://searchfox.org/mozilla-central/rev/b24e6342d744c5a83fab5c15972e11eeb69d68e6/python/mozbuild/mozpack/copier.py#341 because it tries to create

(Pdb) pp required_dirs
set([u'e:\\mozilla-unified\\obj-i686-pc-mingw32\\docs\\html\\main\\_staging',
     u'e:\\mozilla-unified\\obj-i686-pc-mingw32\\docs\\html\\main\\_staging\\browser',
     u'e:\\mozilla-unified\\obj-i686-pc-mingw32\\docs\\html\\main\\_staging\\browser\\extensions',
     u'e:\\mozilla-unified\\obj-i686-pc-mingw32\\docs\\html\\main\\_staging\\dom',
     u'e:\\mozilla-unified\\obj-i686-pc-mingw32\\docs\\html\\main\\_staging\\mobile',
     u'e:\\mozilla-unified\\obj-i686-pc-mingw32\\docs\\html\\main\\_staging\\testing',
     u'e:\\mozilla-unified\\obj-i686-pc-mingw32\\docs\\html\\main\\_staging\\toolkit',
     u'e:\\mozilla-unified\\obj-i686-pc-mingw32\\docs\\html\\main\\_staging\\toolkit\\components',
     u'e:\\mozilla-unified\\obj-i686-pc-mingw32\\docs\\html\\main\\_staging\\toolkit\\modules',
     u'e:\\mozilla-unified\\obj-i686-pc-mingw32\\docs\\html\\main\\_staging\\toolkit\\modules\\subprocess\\toolkit_modules', <--- FAILS HERE because 
     u'e:\\mozilla-unified\\obj-i686-pc-mingw32\\docs\\html\\main\\_staging\\toolkit\\mozapps'])
     
Fails on the second to last because it tries to create "subprocess\\toolkit_modules" but "subprocess" doesn't exist. os.mkdir fails on Windows.

- Fails https://searchfox.org/mozilla-central/rev/b24e6342d744c5a83fab5c15972e11eeb69d68e6/python/mozbuild/mozpack/copier.py#484 because it cannot delete a non-empty directory. it's trying to remove the "subprocess" directory for some reason.

- jsdoc cannot be found by objdir/_virtualenv/Lib/site_packages/sphinx_js/jsdoc.py  (see https://github.com/erikrose/sphinx-js/blob/7ba9bf1d111b14bae85a1e16c143aa6ee5147d4e/sphinx_js/jsdoc.py#L31 ). The npm package itself is installed and I can run "jsdoc" from the same prompt where I run "./mach doc". The former works, the latter can't find "jsdoc".

This patch works around the first two problems, but I have no clue about the third. Any suggestion? 

@Erik, I see that you own sphinx_js (cool project!). Do you have any

Comment 3

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

> Do you have any

I have plenty! ;-) Unfortuantely, I don't have a copy of Windows handy, so maybe we can debug this by remote:

1. It's my understanding that the Windows PATH variable works pretty much like the Unix one: inherited by subprocesses and such. Your success at running jsdoc from the command prompt bodes well, but let's see what PATH is by the time it gets into sphinx-js. Perhaps mach or some other layer above is mucking with it. Right before the problematic Popen call, let's add...

    from os import environ
    raise ValueError(str(environ['PATH']))

Then run `mach doc` and it should crash with an informative PATH value. Let me know what pops out.

2. As a top-down way of reducing the bug, try replacing 'jsdoc' at https://github.com/erikrose/sphinx-js/blob/7ba9bf1d111b14bae85a1e16c143aa6ee5147d4e/sphinx_js/jsdoc.py#L24 with its full path. See if that makes it start working.

Thanks for spending the time to chase this!

Comment 4

[Alessio Placitelli [:Dexter]]

(In reply to Erik Rose [:erik][:erikrose] from comment #3)
> > Do you have any
> 
> I have plenty! ;-) Unfortuantely, I don't have a copy of Windows handy, so
> maybe we can debug this by remote:

Ah! Thanks :-D  I'll happily be the debugger-puppet :-D

> 2. As a top-down way of reducing the bug, try replacing 'jsdoc' at
> https://github.com/erikrose/sphinx-js/blob/
> 7ba9bf1d111b14bae85a1e16c143aa6ee5147d4e/sphinx_js/jsdoc.py#L24 with its
> full path. See if that makes it start working.
> 
> Thanks for spending the time to chase this!

Thank you for your suggestions! Turns out that the correct path was in "PATH": however, we were trying to execute "jsdoc" instead of "jsdoc.cmd". The latter is the "executable" for Windows provided in the mozilla-build package. Simply adding the extension makes it run.

There's another roadblock: the |popen| hangs forever here: https://github.com/erikrose/sphinx-js/blob/7ba9bf1d111b14bae85a1e16c143aa6ee5147d4e/sphinx_js/jsdoc.py#L32

It works if we change the previous line to |p = Popen(jsdoc_command, stdout=temp)|, i.e. removing |stderr=PIPE|. I'm not sure if that's a problem, but it seems to be related to https://groups.google.com/forum/#!topic/comp.lang.python/AyNcfwuAu3U

@Erik, any idea on how to properly change that popen line so that it doesn't get stuck?

With that solved it goes much further, but still fails with a weird "colorama" exception. I'm attaching the extended log.

Comment 5

[Alessio Placitelli [:Dexter]]

Attachment: latest_log.txt

Comment 6

[Alessio Placitelli [:Dexter]]

(In reply to Alessio Placitelli [:Dexter] from comment #5)
> Created attachment 8940102 [details]
> latest_log.txt

@gps, given your experience with the build system and the doc generation (+ comment 4), do you have any idea on why generating the docs would fail as shown in this log (on Windows)?

I also just found out that running the command again doesn't fail, but rather generates a partial working documentation with the following topics:

>    mach package
>    mozbuild package
>    mozlint package
>    mozpack package
>    mozversioncontrol package
>    mozwebidlcodegen package
>    taskgraph package

Comment 7

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

> still fails with a weird "colorama" exception

You aren't using VSCode perchance, are you? The colorama crash looks a lot like https://github.com/tartley/colorama/issues/101, whose ultimate cause is a VSCode bug: https://github.com/Microsoft/vscode/issues/36630#issuecomment-350063413.

> we were trying to execute "jsdoc" instead of "jsdoc.cmd".

Brilliant. I've started collecting Windows fixes for sphinx-js at https://github.com/erikrose/sphinx-js/issues/37. Once we have it running all the way through, I'll implement them. I wish Travis CI supported Windows, but, alas, it does not.

> It works if we change the previous line to |p = Popen(jsdoc_command, stdout=temp)|, i.e. removing |stderr=PIPE|.

I can't think of a good reason to have stderr=PIPE there. Letting it default to None, as you're doing, should be fine, with the additional possible advantage that you can see jsdoc's error output, if any. (I'm not sure where the PIPE output ends up if we don't read and explicitly print it--probably not onscreen.) Added it to the sphinx-js ticket.

Comment 8

[Alessio Placitelli [:Dexter]]

(In reply to Erik Rose [:erik][:erikrose] from comment #7)
> > still fails with a weird "colorama" exception
> 
> You aren't using VSCode perchance, are you? The colorama crash looks a lot
> like https://github.com/tartley/colorama/issues/101, whose ultimate cause is
> a VSCode bug:
> https://github.com/Microsoft/vscode/issues/36630#issuecomment-350063413.

That's exactly it, thanks! Looks like there's nothing we can do for this right now then.

I think the only missing piece of the puzzle is figuring out why the thing is giving me only partial documentation when it succeeds. I bet it's a symlink-on-Windows problem.

Comment 9

[Alessio Placitelli [:Dexter]]

Attachment: test_doc.patch

This fixes (hackily) the documentation being available only partially: the copy function in [1] was failing because the directory didn't exist. I guess that this problem doesn't exist on Linux.

The whole thing still fails, but that's due to my machine having problems building the "psutil" dependency.

[1] - https://searchfox.org/mozilla-central/rev/652fbd6270de0d3ec424d2b88f8375ff546f949f/python/mozbuild/mozpack/files.py#332

Comment 10

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

Added the fixes to sphinx-js in https://github.com/erikrose/sphinx-js/commit/692ee1487d4f36d0d2ffe303fb70a829cb5c5c60. I'll cut a release once we get this working the rest of the way.

Comment 11

[Alessio Placitelli [:Dexter]]

(In reply to Erik Rose [:erik][:erikrose] from comment #10)
> Added the fixes to sphinx-js in
> https://github.com/erikrose/sphinx-js/commit/
> 692ee1487d4f36d0d2ffe303fb70a829cb5c5c60. I'll cut a release once we get
> this working the rest of the way.

Thank you! Let's wait for :gps to chime in. As far as I can see, the "putils" failure on my system is the only thing preventing me from building the docs on Windows (with the changes listed in this bug).

Comment 12

[Gregory Szorc [:gps]]

psutil on Windows is an historical PITA. Python 2.7 C extensions require the Visual Studio 2008 toolchain. Fortunately, Microsoft makes it easy to install one: https://www.microsoft.com/en-us/download/details.aspx?id=44266. But we don't want to force that dependency on people.

I would solve this by installing psutil from a wheel via pip+pypi. A quick hack is to add `self.virtualenv_manager.install_pip_package('psutil==3.1.1')` to tools/docs/mach_commands.py around the other calls to install_pip_package(). (3.1.1 is what is currently vendored and therefore what in-tree users of psutil should work against.)

A better solution is to use a pip requirements file for installing all the Python dependencies. And we should pin hashes for security reasons. That `virtualenv_manager` class has a `install_pip_requirements(self, path, require_hashes=True)` method. https://dxr.mozilla.org/mozilla-central/rev/739484451a6399c7f156a0d960335606aa6c1221/testing/mochitest/mach_commands.py#350 demonstrates how to use it.

An even better solution (which is out of scope for this bug) is to support installing psutil from a wheel for other tools that rely on it.

Comment 13

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

I like your "better solution", for a nice mix of expediency and correctness. Let's migrate the existing self.virtualenv_manager.install_pip_package call in mach_commands.py to a self.virtualenv_manager.install_pip_requirements(path, require_hashes=True) and make a new requirements file in tools/docs/ that has both sphinx_rtd_theme and psutil in it (and stick hashes in there). Any interest in making that patch Alessio, since you are all set up to test it easily?

Comment 14

[Chris H-C :chutten]

At :Dexter's request I tried this from a fresh-ish mozillabuild on my Windows laptop.

The short version is:

$ ./mach doc
<please install jsdoc from npm>

$ npm install -g jsdoc
<installs>

$ ./mach doc
<installs python stuff, tries to run Sphinx>
Exception (on os.mkdir)

$ <Apply the patch from this bug to mozbuild>
$ <Apply sha 692ee14 from sphinx_js to objdir/...>
$ ./mach doc
<Lots of warnings>
<At least one non-fatal error>


And then it "works". The Table of Contents on the left has only about 9 things in it, but all the Telemetry docs are actually built to HTML and can be found in the appropriate subdirs if you go hunting.

So from my POV the only things stopping this from working on Windows are:
1) Applying these patches to mozbuild and sphinx
2) Figuring out why not all of our topics are present in the global TOC

Good job!

Comment 15

[Alessio Placitelli [:Dexter]]

(In reply to Erik Rose [:erik][:erikrose] from comment #13)
> ... Any interest
> in making that patch Alessio, since you are all set up to test it easily?

Interest, yes! Unfortunately I can't guarantee to work on this and finish it as I'll be out on PTO soon-ish. I'll gladly try to work on it best-effort or as soon as I get back if no one fixes it before me :)

(Also, thanks Gregory)

Comment 16

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

I've released a new, Windows-supporting sphinx-js: 2.3.1, at https://pypi.python.org/pypi/sphinx-js/. Also, this is the first version with wheels built.

Comment 17

[Alessio Placitelli [:Dexter]]

Attachment: test_doc.patch

Ok, I gave the pip+hashing a stab. I still see an error building psutil though :(

> LINK : fatal error LNK1104: cannot open file 'MSVCRT.lib'
> error: command 'C:\\Program Files (x86)\\Microsoft Visual Studio 9.0\\VC\\Bin\\link.exe' failed with exit status 1104
> 
> Error processing command. Ignoring because optional. (optional:setup.py:third_party/python/psutil:build_ext:--inplace)
> Error processing command. Ignoring because optional. (optional:packages.txt:comm/build/virtualenv_packages.txt)

I'm not entirely sure why this is still happening, maybe I'm missing something trivial. The |third_party/python/psutil:build_ext:--inplace| part seems to imply it's not using the one I'm installing through pip?

@gps, any clue?

(In reply to Erik Rose [:erik][:erikrose] from comment #16)
> I've released a new, Windows-supporting sphinx-js: 2.3.1, at
> https://pypi.python.org/pypi/sphinx-js/. Also, this is the first version
> with wheels built.

Thank you!

Comment 18

[Gregory Szorc [:gps]]

Ahh, the reason it is attempting to to compile psutil is because the default virtualenv (defined by build/virtualenv_packages.txt) is installing it.

It does ignore failures, so you /might/ be OK. I'd have to poke around to see how we would fix things. The default virtualenv kinda insists it always exists. We really want support for installing multiple virtualenvs at different locations. There's a very old bug somewhere tracking that.

If things are actually broken and these aren't harmless warning messages, needinfo me to poke at things.

Comment 19

[Adam Gashlin (he/him) [:agashlin] (ex-moco)]

I tried mach doc on Windows 10 today, ran into this message:
[edit, somehow submitted by accident]

Exception occurred:
  File "c:\mozilla-build\python\lib\subprocess.py", line 644, in _execute_child
    startupinfo)
TypeError: environment can only contain strings

This seems related to bug 1473377, I think it's due to Sphinx setting up DOCUTILSCONFIG as a Unicode string, which _subprocess.CreateProcess doesn't like since it does PyString_Check(value).

But then I hit the jsdoc vs jsdoc.cmd thing from comment 4.

Comment 20

[Mark Hammond [:markh] [:mhammond]]

(In reply to Adam Gashlin (he/him) [:agashlin] from comment #19)

I tried mach doc on Windows 10 today, ran into this message:

I'm still seeing the same symptoms:

Exception occurred:
File "c:\mozilla-build\python\lib\subprocess.py", line 644, in _execute_child
startupinfo)
TypeError: environment can only contain strings

This seems related to bug 1473377, I think it's due to Sphinx setting up `DOCUTILSCONFIG` as a Unicode string,

Yep :( I'm guessing this is from sphinx's jsdoc.py, but I worked around it by doing os.environ["DOCUTILSCONFIG"] = os.environ["DOCUTILSCONFIG"].encode("mbcs") in mozbuild/sphinx.py.

But then I hit the jsdoc vs jsdoc.cmd thing from comment 4.

Yep - which I worked around by editing the jsdoc.py file installed into the build directory - I couldn't work out how to have the build process use a local copy of sphinx, so making a patch seems difficult and I've already blown enough time on this for now.

Erik, is there anything you can do here to help move things along? I'm surprised the .cmd thing reported a couple of years ago remains a problem, but working out how to use a locally patched version of sphinx would at least help me submit a PR to sphinx.

Comment 21

[Alessio Placitelli [:Dexter]]

(In reply to Mark Hammond [:markh] from comment #20)

(In reply to Adam Gashlin (he/him) [:agashlin] from comment #19)

I tried mach doc on Windows 10 today, ran into this message:

I'm still seeing the same symptoms:

FWIW I started building docs in the WSL in Windows 10. After you manually install jsdoc, it works great!

Comment 22

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

Hi, Mark! The .cmd thing was indeed fixed in sphinx-js in 2.3.1. We're at 2.7.1 now. What I need to do is update the version in moz-central. I will do so soon. Maybe you can review the patch.

Comment 23

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

Attachment: Bug 1232403 - Upgrade to sphinx-js 2.8. r=ahal

This should fix the doc builds on Windows, as sphinx-js added Windows support in 2.3.1 and 2.4. We also now get support for variadic args, @deprecated, and @see, along with other features.

sphinx-js 2.7.1 changed the default cwd to be the one containing conf.py, so I also had to twiddle jsdoc_config_path.

Let some other pipenv pinnings update themselves as well, as, if I don't, they'll just update themselves the next time somebody runs mach doc, dirtying their tree.

I suspect this also fixes bug 1556460, whose equivalent bug in sphinx-js is https://github.com/mozilla/sphinx-js/issues/106. IOW, it should no longer break with versions of jsdoc >= 3.6.

Comment 24

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/e206bc685749
Upgrade to sphinx-js 2.8. r=ahal

Comment 25

[Stefan Hindli [:stefan_hindli]]

Backed out changeset e206bc685749 (Bug 1232403) for causing a linting failure CLOSED TREE

https://treeherder.mozilla.org/#/jobs?repo=autoland&resultStatus=testfailed%2Cbusted%2Cexception&classifiedState=unclassified&selectedJob=269802618

https://treeherder.mozilla.org/logviewer.html#/jobs?job_id=269802618&repo=autoland&lineNumber=257

Comment 26

[Chris Manchester (limited bugmail, email directly)]

Stefan, did you intend to needinfo the patch author here?

Comment 27

[Pulsebot]

Backout by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/040c67886190
Backed out changeset e206bc685749 for causing a linting failure CLOSED TREE

Comment 28

[Stefan Hindli [:stefan_hindli]]

:chmanchester yes, sorry.

Comment 29

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

Should be fixed now. Ready for another look, ahal, if you have a moment.

Comment 30

[Andrew Halberstadt [:ahal]]

Thanks, changes look good. Not really sure why that happened, but also don't have the time to dig into it.

Comment 31

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/c6a8078979c1
Upgrade to sphinx-js 2.8. r=ahal

Comment 32

[Arthur Iakab [arthur_iakab]]

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