Navigation Menu

Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

DOC: automatically create a venv and install Sphinx when running make #74672

Closed
cjrh mannequin opened this issue May 26, 2017 · 15 comments
Closed

DOC: automatically create a venv and install Sphinx when running make #74672

cjrh mannequin opened this issue May 26, 2017 · 15 comments
Labels
3.7 (EOL) end of life docs Documentation in the Doc dir

Comments

@cjrh
Copy link
Mannequin

cjrh mannequin commented May 26, 2017

BPO 30487
Nosy @ned-deily, @cjrh, @zware, @willingc, @csabella
PRs
  • bpo-30487: automatically create a venv and install Sphinx when running make #1743
  • bpo-30487: automatically create a venv and install Sphinx when running make #4346
  • bpo-30487: revert automatically create a venv and install Sphinx whe… #4592
  • Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.

    Show more details

    GitHub fields:

    assignee = None
    closed_at = <Date 2019-01-06.16:56:25.509>
    created_at = <Date 2017-05-26.16:49:48.410>
    labels = ['3.7', 'docs']
    title = 'DOC: automatically create a venv and install Sphinx when running make'
    updated_at = <Date 2019-01-06.16:56:25.508>
    user = 'https://github.com/cjrh'

    bugs.python.org fields:

    activity = <Date 2019-01-06.16:56:25.508>
    actor = 'ned.deily'
    assignee = 'docs@python'
    closed = True
    closed_date = <Date 2019-01-06.16:56:25.509>
    closer = 'ned.deily'
    components = ['Documentation']
    creation = <Date 2017-05-26.16:49:48.410>
    creator = 'cjrh'
    dependencies = []
    files = []
    hgrepos = []
    issue_num = 30487
    keywords = ['patch']
    message_count = 15.0
    messages = ['294556', '305932', '305936', '305955', '307019', '307082', '307085', '307086', '307087', '307090', '307092', '307094', '307097', '333080', '333087']
    nosy_count = 6.0
    nosy_names = ['ned.deily', 'cjrh', 'docs@python', 'zach.ware', 'willingc', 'cheryl.sabella']
    pr_nums = ['1743', '4346', '4592']
    priority = 'normal'
    resolution = 'rejected'
    stage = 'resolved'
    status = 'closed'
    superseder = None
    type = None
    url = 'https://bugs.python.org/issue30487'
    versions = ['Python 3.7']

    @cjrh
    Copy link
    Mannequin Author

    cjrh mannequin commented May 26, 2017

    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.

    @cjrh cjrh mannequin added the 3.7 (EOL) end of life label May 26, 2017
    @cjrh cjrh mannequin assigned docspython May 26, 2017
    @cjrh cjrh mannequin added the docs Documentation in the Doc dir label May 26, 2017
    @cjrh
    Copy link
    Mannequin Author

    cjrh mannequin commented Nov 9, 2017

    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?

    @zware
    Copy link
    Member

    zware commented Nov 9, 2017

    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.

    @cjrh
    Copy link
    Mannequin Author

    cjrh mannequin commented Nov 9, 2017

    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.

    @zware
    Copy link
    Member

    zware commented Nov 26, 2017

    New changeset d8d6b91 by Zachary Ware (Caleb Hattingh) in branch 'master':
    bpo-30487: automatically create a venv and install Sphinx when running make (GH-4346)
    d8d6b91

    @zware zware closed this as completed Nov 26, 2017
    @ned-deily
    Copy link
    Member

    I don't think this is a good idea. It has already caused problems with one buildbot (bpo-32149) 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.

    @ned-deily ned-deily reopened this Nov 27, 2017
    @cjrh
    Copy link
    Mannequin Author

    cjrh mannequin commented Nov 27, 2017

    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 (bpo-32149) 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\>


    @ned-deily
    Copy link
    Member

    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.

    @zware
    Copy link
    Member

    zware commented Nov 27, 2017

    Go ahead with the revert, Ned.

    @cjrh
    Copy link
    Mannequin Author

    cjrh mannequin commented Nov 27, 2017

    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.

    @ned-deily
    Copy link
    Member

    "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.

    @cjrh
    Copy link
    Mannequin Author

    cjrh mannequin commented Nov 27, 2017

    Yep, sounds good.

    @ned-deily
    Copy link
    Member

    New changeset 122fc13 by Ned Deily in branch 'master':
    Revert "bpo-30487: automatically create a venv and install Sphinx when running make (GH-4346)" (bpo-4592)
    122fc13

    @csabella
    Copy link
    Contributor

    csabella commented Jan 5, 2019

    I believe this can be closed as resolved? Thanks.

    @cjrh
    Copy link
    Mannequin Author

    cjrh mannequin commented Jan 6, 2019

    @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 bpo-34831).

    @ezio-melotti ezio-melotti transferred this issue from another repository Apr 10, 2022
    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
    Labels
    3.7 (EOL) end of life docs Documentation in the Doc dir
    Projects
    None yet
    Development

    No branches or pull requests

    3 participants