Issue 30487: DOC: automatically create a venv and install Sphinx when running make (original) (raw)

Created on 2017-05-26 16:49 by cjrh, last changed 2022-04-11 14:58 by admin. This issue is now closed.

Pull Requests
URL Status Linked Edit
PR 1743 closed cjrh,2017-05-26 16:49
PR 4346 merged cjrh,2017-11-09 10:47
PR 4592 merged ned.deily,2017-11-27 21:50
Messages (15)
msg294556 - (view) Author: Caleb Hattingh (cjrh) * Date: 2017-05-26 16:49
Under guidance from zware during Pycon sprints, I've changed the Doc/ Makefile to automatically create a virtual environment and install Sphinx, all as part of the `make html` command.
msg305932 - (view) Author: Caleb Hattingh (cjrh) * Date: 2017-11-09 03:58
I messed up the PR through a failed rebase (trying to rebase my PR on top of upstream). I closed the PR as a result. I have now fixed up my feature branch, but I have not resubmitted the PR. Since the PR was left alone for many months, I'm ok with leaving things as is, and close this issue?
msg305936 - (view) Author: Zachary Ware (zach.ware) * (Python committer) Date: 2017-11-09 05:04
You should be able to force-push your branch (`git push -f origin auto-venv-docbuilder`, or replace `origin` with the correct remote name) to fix the existing PR. Sorry I haven't gotten back to this previously; time to do a review and remembering that the PR exists have not coincided.
msg305955 - (view) Author: Caleb Hattingh (cjrh) * Date: 2017-11-09 10:50
No worries. I've made a new PR 4346. The old one was unsalvagable I'm afraid. Too many other people got added to the notifications list as a result of my incorrect rebase. The new one is fine.
msg307019 - (view) Author: Zachary Ware (zach.ware) * (Python committer) Date: 2017-11-26 21:18
New changeset d8d6b9122134f040cd5a4f15f40f6c9e3386db4d by Zachary Ware (Caleb Hattingh) in branch 'master': bpo-30487: automatically create a venv and install Sphinx when running make (GH-4346) https://github.com/python/cpython/commit/d8d6b9122134f040cd5a4f15f40f6c9e3386db4d
msg307082 - (view) Author: Ned Deily (ned.deily) * (Python committer) Date: 2017-11-27 21:12
I don't think this is a good idea. It has already caused problems with one buildbot (Issue32149) and will likely break other build scripts. As the Doc Makefile stood previous to this commit, the Doc builds could take advantage of either a system-installed or user-supplied Sphinx and blurb without having to use the venv step. Unless there are major objections, I am going to at least temporarily revert it.
msg307085 - (view) Author: Caleb Hattingh (cjrh) * Date: 2017-11-27 21:23
Hi Ned It's still supposed to allow both. It sounds like it's not working properly. I'll have a look. FYI, I worked on this for Zach Ware who is the primary stakeholder for this feature. Rgds Caleb > On 28 Nov 2017, at 7:12 AM, Ned Deily <report@bugs.python.org> wrote: > > > Ned Deily <nad@python.org> added the comment: > > I don't think this is a good idea. It has already caused problems with one buildbot (Issue32149) and will likely break other build scripts. As the Doc Makefile stood previous to this commit, the Doc builds could take advantage of either a system-installed or user-supplied Sphinx and blurb without having to use the venv step. Unless there are major objections, I am going to at least temporarily revert it. > > ---------- > nosy: +ned.deily > resolution: fixed -> > stage: resolved -> needs patch > status: closed -> open > > _______________________________________ > Python tracker <report@bugs.python.org> > <https://bugs.python.org/issue30487> > _______________________________________
msg307086 - (view) Author: Ned Deily (ned.deily) * (Python committer) Date: 2017-11-27 21:26
One problem is that the venv can't always be automatically built in all environments, as a recent Python 3 needs to be available in the right location.
msg307087 - (view) Author: Zachary Ware (zach.ware) * (Python committer) Date: 2017-11-27 21:40
Go ahead with the revert, Ned.
msg307090 - (view) Author: Caleb Hattingh (cjrh) * Date: 2017-11-27 21:58
It looks like the check for an existing sphinx-build passes, and so no new venv is made, but this also means that blurb doesn't get installed. I was concerned about this, but figured that at least the buildbots would create new envs each time, and this would only be an issue that a user with an odd configuration might have. Sorry for the trouble. The feature was simpler when it was only sphinx. Now that blurb is there too, the logic for checking what is and isn't already present becomes a bit complex to reason through.
msg307092 - (view) Author: Ned Deily (ned.deily) * (Python committer) Date: 2017-11-27 22:03
"Now that blurb is there too, the logic for checking what is and isn't already present becomes a bit complex to reason through." Yeah, it is a bit complicated. There's also the issue of trying to use a make recipe to ensure a "venv" exists and is up-to-date. I don't necessarily want to abandon the idea (and sorry I wasn't around to review the proposed change initially) but let's at least hold off until 370a3 is out the door.
msg307094 - (view) Author: Caleb Hattingh (cjrh) * Date: 2017-11-27 22:04
Yep, sounds good.
msg307097 - (view) Author: Ned Deily (ned.deily) * (Python committer) Date: 2017-11-27 22:07
New changeset 122fc136b34e11906466851e77bb6959946467ee by Ned Deily in branch 'master': Revert "bpo-30487: automatically create a venv and install Sphinx when running make (GH-4346)" (#4592) https://github.com/python/cpython/commit/122fc136b34e11906466851e77bb6959946467ee
msg333080 - (view) Author: Cheryl Sabella (cheryl.sabella) * (Python committer) Date: 2019-01-05 23:07
I believe this can be closed as resolved? Thanks.
msg333087 - (view) Author: Caleb Hattingh (cjrh) * Date: 2019-01-06 02:34
@cheryl.sabella I am ok with closing this, but the original motivation for this work was from @zack.ware so he should weigh in. I am not going to work on this any further for the forseeable future (I've got my hands full already with the asyncio docs I'm trying to write in #34831).
History
Date User Action Args
2022-04-11 14:58:46 admin set github: 74672
2019-01-06 16:56:25 ned.deily set status: open -> closedresolution: rejectedstage: patch review -> resolved
2019-01-06 02:34:45 cjrh set messages: +
2019-01-05 23:07:24 cheryl.sabella set nosy: + cheryl.sabellamessages: +
2017-11-27 22:07:34 ned.deily set messages: +
2017-11-27 22:04:19 cjrh set messages: +
2017-11-27 22:03:18 ned.deily set messages: +
2017-11-27 21:58:15 cjrh set messages: +
2017-11-27 21:50:18 ned.deily set stage: needs patch -> patch reviewpull_requests: + <pull%5Frequest4516>
2017-11-27 21:40:29 zach.ware set messages: +
2017-11-27 21:26:50 ned.deily set messages: +
2017-11-27 21:23:33 cjrh set messages: +
2017-11-27 21:12:46 ned.deily set status: closed -> opennosy: + ned.deilymessages: + resolution: fixed -> (no value)stage: resolved -> needs patch
2017-11-26 21:20:07 zach.ware set status: open -> closedresolution: fixedstage: patch review -> resolved
2017-11-26 21🔞32 zach.ware set messages: +
2017-11-09 10:50:19 cjrh set messages: +
2017-11-09 10:47:33 cjrh set keywords: + patchpull_requests: + <pull%5Frequest4303>
2017-11-09 05:04:05 zach.ware set messages: +
2017-11-09 03:58:32 cjrh set messages: +
2017-05-26 16:51:33 Mariatta set stage: patch review
2017-05-26 16:49:48 cjrh create