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

In ./Doc, "make html" and "make build" should depend on "make venv" #88919

Closed
jdevries3133 mannequin opened this issue Jul 28, 2021 · 19 comments
Closed

In ./Doc, "make html" and "make build" should depend on "make venv" #88919

jdevries3133 mannequin opened this issue Jul 28, 2021 · 19 comments
Labels
3.9 only security fixes 3.10 only security fixes 3.11 only security fixes docs Documentation in the Doc dir release-blocker type-feature A feature request or enhancement

Comments

@jdevries3133
Copy link
Mannequin

jdevries3133 mannequin commented Jul 28, 2021

BPO 44756
Nosy @ned-deily, @encukou, @ambv, @hroncok, @pablogsal, @miss-islington, @jdevries3133
PRs
  • bpo-44756: in ./Doc, make build depends on make html #27403
  • [3.10] bpo-44756: in ./Doc, make build depends on make html (GH-27403) #27410
  • [3.9] bpo-44756: in ./Doc, make build depends on make html (GH-27403) #27411
  • bpo-44756: [docs] revert automated virtual environment creation on make html #27635
  • [3.9] bpo-44756: [docs] revert automated virtual environment creation on make html (GH-27635) #27636
  • [3.10] bpo-44756: [docs] revert automated virtual environment creation on make html (GH-27635) #27637
  • bpo-44756: Remove misleading NEWS entries of a change that was reverted before release #28075
  • 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 2021-08-06.19:53:19.845>
    created_at = <Date 2021-07-28.04:47:25.985>
    labels = ['3.9', '3.10', '3.11', 'type-feature', 'release-blocker', 'docs']
    title = 'In ./Doc, "make html" and "make build" should depend on "make venv"'
    updated_at = <Date 2021-08-30.21:56:40.474>
    user = 'https://github.com/jdevries3133'

    bugs.python.org fields:

    activity = <Date 2021-08-30.21:56:40.474>
    actor = 'lukasz.langa'
    assignee = 'docs@python'
    closed = True
    closed_date = <Date 2021-08-06.19:53:19.845>
    closer = 'lukasz.langa'
    components = ['Demos and Tools', 'Documentation']
    creation = <Date 2021-07-28.04:47:25.985>
    creator = 'jack__d'
    dependencies = []
    files = []
    hgrepos = []
    issue_num = 44756
    keywords = ['patch']
    message_count = 19.0
    messages = ['398344', '398373', '398381', '398383', '398384', '398827', '398840', '398848', '398868', '398874', '398875', '398884', '399101', '399116', '399124', '399130', '399135', '400672', '400673']
    nosy_count = 8.0
    nosy_names = ['ned.deily', 'petr.viktorin', 'docs@python', 'lukasz.langa', 'hroncok', 'pablogsal', 'miss-islington', 'jack__d']
    pr_nums = ['27403', '27410', '27411', '27635', '27636', '27637', '28075']
    priority = 'release blocker'
    resolution = 'fixed'
    stage = 'resolved'
    status = 'closed'
    superseder = None
    type = 'enhancement'
    url = 'https://bugs.python.org/issue44756'
    versions = ['Python 3.9', 'Python 3.10', 'Python 3.11']

    @jdevries3133
    Copy link
    Mannequin Author

    jdevries3133 mannequin commented Jul 28, 2021

    In Doc/Makefile, all of the build rules should be dependent on the existence of a virtual environment. I could see this being controversial, because folks who have these tools installed elsewhere might prefer not to have a venv made for them, but my personal workflow is to strive to keep my system site-packages folder as empty as possible and to use virtual environments frequently. In any case, I'm interested to hear what we think.

    Momentarily, I will attach a PR where I went ahead and made these changes. Here is a summary of the changes:

    • venv rule is now conditional, and only does anything if $VENVDIR does
      not exist
    • add rule "clean-venv". This can be removed – what do you all think?
    • build rule is dependent on venv
    • as a result, rules dependent on build are dependent on venv
      • html
      • latex
      • etc.
    • update Doc/README.rst appropriately. Now users only need to type
      make html – nice!

    Let me know what you think. I may have a blind spot to others' workflows! Also, I'm not a Windows user, so I have no insight as to whether make.bat needs to be updated as well.

    @jdevries3133 jdevries3133 mannequin added the 3.11 only security fixes label Jul 28, 2021
    @jdevries3133 jdevries3133 mannequin assigned docspython Jul 28, 2021
    @jdevries3133 jdevries3133 mannequin added docs Documentation in the Doc dir type-feature A feature request or enhancement 3.11 only security fixes labels Jul 28, 2021
    @jdevries3133 jdevries3133 mannequin assigned docspython Jul 28, 2021
    @jdevries3133 jdevries3133 mannequin added docs Documentation in the Doc dir type-feature A feature request or enhancement labels Jul 28, 2021
    @ambv
    Copy link
    Contributor

    ambv commented Jul 28, 2021

    New changeset d22c876 by Jack DeVries in branch 'main':
    bpo-44756: in ./Doc, make build depends on make html (bpo-27403)
    d22c876

    @ambv
    Copy link
    Contributor

    ambv commented Jul 28, 2021

    New changeset f113195 by Miss Islington (bot) in branch '3.10':
    bpo-44756: in ./Doc, make build depends on make html (GH-27403) (GH-27410)
    f113195

    @ambv
    Copy link
    Contributor

    ambv commented Jul 28, 2021

    New changeset fd2c246 by Miss Islington (bot) in branch '3.9':
    bpo-44756: in ./Doc, make build depends on make html (GH-27403) (GH-27411)
    fd2c246

    @ambv
    Copy link
    Contributor

    ambv commented Jul 28, 2021

    Thanks, Jack! ✨ 🍰 ✨

    @ambv ambv closed this as completed Jul 28, 2021
    @ambv ambv closed this as completed Jul 28, 2021
    @encukou
    Copy link
    Member

    encukou commented Aug 3, 2021

    I'm one of those who disagree with "make html" suddenly downloading code from the internet and running it, but I guess I'm in a minority. I'll switch to using sphinx-build directly.

    Still, I'd have appreciated a heads-up notice.

    @jdevries3133
    Copy link
    Mannequin Author

    jdevries3133 mannequin commented Aug 3, 2021

    @petr.viktorin a whatsnew entry was added, what more notice could have been provided?

    I have an idea for an alternative that might be better. What if make clean deletes and restores the venv only if it already existed in the first place?

    @jdevries3133
    Copy link
    Mannequin Author

    jdevries3133 mannequin commented Aug 3, 2021

    Actually, I tested out that idea (main...jdevries3133:bpo-44756-doc-make), and I don't think its as nice of a solution. I think it is valuable for new contributors to be able to type "make html" and have it work. Look at how much the change simplifies the README for new contributors to setup their documentation build toolchain. Then, look at how many docs@python open issues there are. We need documentation contributors, and there is value in simplifying the process for them.

    Additionally, this is the extent of the "downloading code from the internet and running it" that occurs::

    pip install -U pip setuptools
    pip install sphynx blurb python-docs-theme
    

    If running that is ever unsafe, we have big problems!

    @hroncok
    Copy link
    Mannequin

    hroncok mannequin commented Aug 4, 2021

    The 3.10.0rc1 source tarballs contain the Docs/venv directory populated with pablogsal's venv: bpo-44824

    @encukou
    Copy link
    Member

    encukou commented Aug 4, 2021

    The issue this (or lack of communication about it) caused in rc1 is tracked in https://bugs.python.org/issue44823

    @petr.viktorin a whatsnew entry was added, what more notice could have been provided?

    Ideally, the python-dev mailing list (or Discourse).

    pip install sphinx blurb python-docs-theme
    If running that is ever unsafe, we have big problems!

    Who is "we"?
    We do have big problems. Anyone who can upload wheels for sphinx blurb python-docs-theme or any of their dependencies (or anyone who has their credentials) can now easily put code on machines of CPython developers.

    For example, PyPI doesn't guarantee that wheels correspond to sources. "Markupsafe" is particularly dangerous because the wheels are platform-specific and have compiled code, so tampering is nearly undetectable. (But if another dependency starts using platform-specific wheels, I don't think anyone would notice.)

    @encukou
    Copy link
    Member

    encukou commented Aug 4, 2021

    @pablogsal
    Copy link
    Member

    Note this issue has impacted negatively the release process of 3.10.0rc1:

    https://bugs.python.org/issue44823

    @ned-deily
    Copy link
    Member

    There was a previous attempt at adding a dependency on venv that ended up being reverted (bpo-30487 and 122fc136b34e11906466851e77bb6959946467ee0). Over the years we have had a number of iterations of tweaking Doc/Makefile to balance ease of use (by providing the venv) with those of more advanced users who build docs as part of their automated processeses. I think what we had prior to this most recent change worked reasonably well for all. The previous process is clearly documented, for example in the Dev Guide (https://devguide.python.org/documenting/?highlight=venv#building-the-documentation) and does not strike me as very onerous. I appreciate the motivation behind this change but I really think there isn't a problem here that needs to be solved and I would support reverting the change for 3.9 and 3.10 and *perhaps* reconsider something for 3.11.

    @ned-deily ned-deily reopened this Aug 6, 2021
    @ned-deily ned-deily reopened this Aug 6, 2021
    @ned-deily ned-deily added release-blocker 3.9 only security fixes 3.10 only security fixes labels Aug 6, 2021
    @ned-deily ned-deily added the 3.10 only security fixes label Aug 6, 2021
    @ambv
    Copy link
    Contributor

    ambv commented Aug 6, 2021

    New changeset 55fa87b by Łukasz Langa in branch 'main':
    bpo-44756: [docs] revert automated virtual environment creation on make html (GH-27635)
    55fa87b

    @ambv
    Copy link
    Contributor

    ambv commented Aug 6, 2021

    New changeset 91f6d38 by Miss Islington (bot) in branch '3.9':
    bpo-44756: [docs] revert automated virtual environment creation on make html (GH-27635) (GH-27636)
    91f6d38

    @miss-islington
    Copy link
    Contributor

    New changeset 1e9c9ca by Miss Islington (bot) in branch '3.10':
    bpo-44756: [docs] revert automated virtual environment creation on make html (GH-27635)
    1e9c9ca

    @ambv
    Copy link
    Contributor

    ambv commented Aug 6, 2021

    Revert done. Re-closing!

    @ambv ambv closed this as completed Aug 6, 2021
    @ambv ambv closed this as completed Aug 6, 2021
    @ambv
    Copy link
    Contributor

    ambv commented Aug 30, 2021

    New changeset 5246dbc by Łukasz Langa in branch 'main':
    bpo-44756: Remove misleading NEWS entries of a change that was reverted before release (GH-28075)
    5246dbc

    @ambv
    Copy link
    Contributor

    ambv commented Aug 30, 2021

    New changeset 9ef1843 by Łukasz Langa in branch '3.9':
    bpo-44756: Remove misleading NEWS entries of a change that was reverted before release (GH-28075)
    9ef1843

    @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.9 only security fixes 3.10 only security fixes 3.11 only security fixes docs Documentation in the Doc dir release-blocker type-feature A feature request or enhancement
    Projects
    None yet
    Development

    No branches or pull requests

    5 participants