Closed Bug 1458461 Opened 3 years ago Closed 2 years ago
Use pipenv to manage environment for mach doc command
59 bytes, text/x-review-board-request
Splitting this out from bug 1437593 as my patch is causing an intermittent failure, and we don't want to block on vendoring pipenv.
(In reply to Dave Hunt [:davehunt] ⌚️UTC+0 from comment #1) > Created attachment 8972667 [details] > Bug 1458461 - Use pipenv for |mach doc| environment; > > Review commit: https://reviewboard.mozilla.org/r/241208/diff/#index_header > See other reviews: https://reviewboard.mozilla.org/r/241208/ This patch was previously reviewed as part of bug 1437593 and was backed out due to an intermittent generating the docs. I've triggered a try run, which demonstrates the failure here: https://treeherder.mozilla.org/#/jobs?repo=try&revision=0b3c1362d940fd36b002474cb38f1525a9e8336a I believe we can ignore most of the errors output by the task as these also exist without this patch. The fatal issue appears to be "Resource temporarily unavailable". I haven't been able to replicate this locally, but we can probably investigate by adding debug or increasing the verbosity of Sphinx in a try run.
I did a little digging this morning, but I don't have any ideas yet. sphinx is getting `EAGAIN` writing a traceback to stderr (the fact that it's writing a traceback seems OK, the log is full of them): https://github.com/sphinx-doc/sphinx/blob/645044d7f03c7fcb4c39a8edf40aa66f6d6c2b64/sphinx/cmdline.py#L104 stderr is actually a codec writer because mach's `Mach.run` method reopens `sys.stderr` with `codecs.getwriter` so it will properly encode Unicode: https://dxr.mozilla.org/mozilla-central/rev/f877359308b17e691209e1afb7193b8e86f175ce/python/mach/mach/main.py#350 The traceback shows that the error is coming when writing to the file that the codec writer wraps, though, which is actually stderr: [task 2018-05-02T19:01:05.346Z] File "/usr/lib/python2.7/codecs.py", line 370, in write [task 2018-05-02T19:01:05.346Z] self.stream.write(data) Calling `write` on a `file` object in Python winds up just calling `fwrite` on the underlying `FILE*`: https://github.com/python/cpython/blob/6d3d02c69a65abcd64eb6d83baac5675f9208318/Objects/fileobject.c#L1871 `mach doc` in this task is invoked by way of `run-task`, which pipes the process output using `subprocess.Popen`: https://dxr.mozilla.org/mozilla-central/rev/f877359308b17e691209e1afb7193b8e86f175ce/taskcluster/docker/recipes/run-task#101 I checked, and that means that Python simply calls `os.pipe` to create the file descriptor: https://github.com/python/cpython/blob/6d3d02c69a65abcd64eb6d83baac5675f9208318/Lib/subprocess.py#L865 So fundamentally the mach process is erroring because `fwrite` is returning `EAGAIN` trying to write to the stdout pipe. The only way this should happen is if the pipe is set to non-blocking mode. I don't see anywhere in our code that that would be happening.
Comment on attachment 8972667 [details] Bug 1458461 - Use pipenv for |mach doc| environment; https://reviewboard.mozilla.org/r/241208/#review247242 This is fine once we sort out the failure.
Attachment #8972667 - Flags: review?(ted) → review+
Comment on attachment 8972667 [details] Bug 1458461 - Use pipenv for |mach doc| environment; https://reviewboard.mozilla.org/r/241208/#review254386 Sorry for the random drive-by review. I forgot this bug already existed and implemented almost this exact same patch before I realized. This will break doc uploading though, so giving an r- for now. ::: tools/docs/Pipfile:7 (Diff revision 1) > +backports-abc = "*" > +futures = "*" > +livereload = "*" > +recommonmark = "*" > +singledispatch = "*" > +sphinx = "<1.7.0" > +sphinx_rtd_theme = "*" > +sphinx-js = "*" > +typing = "*" This is going to break the DocUp task (I only know because I ran into the same problem). This is because when it tries to install boto3 with `install_pip_package` (which is apparently incompatible with the version of pip that pipenv uses): https://searchfox.org/mozilla-central/source/tools/docs/mach_commands.py#179 The `futures` and `typing` packages are dependencies of `boto3`, so they can be removed from here. ::: tools/docs/Pipfile:7 (Diff revision 1) > +url = "https://pypi.python.org/simple" > +verify_ssl = true > +name = "pypi" > + > +[packages] > +backports-abc = "*" When using \* for the specifiers, will it still be possible to update only a single package? Or would it be an all or nothing proposition? Say I wanted to *only* update `sphinx-rtd-theme` in the future, how could I do that?
Attachment #8972667 - Flags: review-
Btw, here's my change on try: https://treeherder.mozilla.org/#/jobs?repo=try&revision=3eb677e43e5c77f579bc8c54e562434d79327cde&filter-tier=1&filter-tier=2&filter-tier=3&selectedJob=181109418 The only difference is that I pinned the packages in Pipfile.. but using * might be better if there is a way to do what I'm asking in the second issue. The good news is that the intermittent Doc issue seems to have magically fixed itself!
Comment on attachment 8972667 [details] Bug 1458461 - Use pipenv for |mach doc| environment; https://reviewboard.mozilla.org/r/241208/#review254386 > When using \* for the specifiers, will it still be possible to update only a single package? Or would it be an all or nothing proposition? > > Say I wanted to *only* update `sphinx-rtd-theme` in the future, how could I do that? You should be able to run `pipenv update --outdated` to find out what updates are available, and `pipenv update <pkg>` to update a single package along with any transient dependencies. This will require you to have pipenv installed, unless we introduce a mach command to use pipenv in the initial virtualenv. Something like `mach pipenv /path/to/Pipfile [arg, ...]` where the args just get passed directly to pipenv.
Thanks, `pipenv update` is what I was missing (I was using `pipenv lock` before). In that case yes, let's not pin the dependencies in the Pipfile. I think a |mach pipenv| could be useful, though you could also just run |mach python -mpipenv update ...|. Since I have a patch that works with the DocUp task already, I'll take this bug over.
Attachment #8972667 - Attachment is obsolete: true
I decided to pin the packages because I think package updates should happen in their own bugs. There's already a bug on file to update sphinx (bug 1437526). Maybe we can update all the packages at the same time in that bug.
Comment on attachment 8982265 [details] Bug 1458461 - [docs] Use pipenv to manage |mach doc| python environment, https://reviewboard.mozilla.org/r/248212/#review254470
Attachment #8982265 - Flags: review?(dave.hunt) → review+
Pushed by email@example.com: https://hg.mozilla.org/integration/autoland/rev/4830f72e8297 [docs] Use pipenv to manage |mach doc| python environment, r=davehunt
Fyi looks like the intermittent happens when I upgrade all of the doc dependencies to their latest versions: https://treeherder.mozilla.org/#/jobs?repo=try&revision=510eb6ec7ca72918830aae1492ae62f419463f5b So we'll still likely need to address it at some point.
You need to log in before you can comment on or make changes to this bug.