Closed Bug 1232403 Opened 4 years ago Closed 3 months ago
|mach doc| is broken on Windows
1.71 KB, text/x-log
31.68 KB, text/plain
8.20 KB, patch
|Details | Diff | Splinter Review|
47 bytes, text/x-phabricator-request
|Details | Review|
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 , 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  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).  - https://dxr.mozilla.org/mozilla-central/rev/99137d6d4061f408ae0869122649d8bdf489cc30/python/mozbuild/mozpack/files.py#305
The full log mentioned by the failing command.
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
> 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!
(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.
(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
> 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.
Flags: needinfo?(erik) → needinfo?(alessio.placitelli)
(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.
This fixes (hackily) the documentation being available only partially: the copy function in  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.  - https://searchfox.org/mozilla-central/rev/652fbd6270de0d3ec424d2b88f8375ff546f949f/python/mozbuild/mozpack/files.py#332
Attachment #8939841 - Attachment is obsolete: true
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.
(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).
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.
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?
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!
(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)
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.
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!
Attachment #8940272 - Attachment is obsolete: true
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.
Pushed by email@example.com: https://hg.mozilla.org/integration/autoland/rev/e206bc685749 Upgrade to sphinx-js 2.8. r=ahal
Flags: needinfo?(cmanchester) → needinfo?(shindli)
Backout by firstname.lastname@example.org: https://hg.mozilla.org/integration/autoland/rev/040c67886190 Backed out changeset e206bc685749 for causing a linting failure CLOSED TREE
Flags: needinfo?(shindli) → needinfo?(alessio.placitelli)
Flags: needinfo?(alessio.placitelli) → needinfo?(ahal)
Assignee: nobody → erik
Status: NEW → ASSIGNED
Pushed by email@example.com: https://hg.mozilla.org/integration/autoland/rev/c6a8078979c1 Upgrade to sphinx-js 2.8. r=ahal
You need to log in before you can comment on or make changes to this bug.