bpo-30487: automatically create a venv and install Sphinx when running make by cjrh · Pull Request #1743 · python/cpython

cjrh · 2017-05-23T00:15:10Z

The changes have been made, and by removing venv from the .PHONY section in the Makefile, it now works reliably to only create the virtualenv if the venv/ directory doesn't exist.

I have also made some edits to the documentation to indicate that Sphinx installation (into a local venv) is now automatic.

Should we be doing more checking to deal with situations where the user overrides PYTHON and/or SPHINXBUILD? For example, if a user specifies their own SPHINXBUILD, they may not be too happy that an unnecessary venv is going to get created when they run $ make html...?

https://bugs.python.org/issue30487

mention-bot · 2017-05-23T00:15:11Z

@cjrh, thanks for your PR! By analyzing the history of the files in this pull request, we identified @birkenfeld, @tiran and @zware to be potential reviewers.

zware

I haven't tried it out yet, but this looks pretty good. Needs a couple of small tweaks, and should also have an issue on bpo.

zware · 2017-05-23T00:17:16Z

Doc/README.rst

Should probably specify that it needs to be Python 3.

zware · 2017-05-23T00:19:23Z

Doc/README.rst

s/virtualenv/virtual environment/. "virtualenv" suggests that you need to use the virtualenv tool, which is not true with Python 3. Similarly, "venv" suggests that you need to use the venv module, which won't work on Python 2. The spelled out "virtual environment" should imply that you can use whatever works for you.

willingc

Thanks @cjrh. I've added one small clarification about running make html from the Doc directory.

@zware I've tested both approaches locally and they work well on my MacOS system.

willingc · 2017-05-26T06:24:43Z

Doc/README.rst

Hi @cjrh, Similar to line 95, I would mention changing into the Doc directory before running make html to assist those folks that are newer to Sphinx.

the-knights-who-say-ni · 2017-05-26T16:55:53Z

Hello, and thanks for your contribution!

I'm a bot set up to make sure that the project can legally accept your contribution by verifying you have signed the PSF contributor agreement (CLA).

Unfortunately our records indicate you have not signed the CLA. For legal reasons we need you to sign this before we can look at your contribution. Please follow the steps outlined in the CPython devguide to rectify this issue.

Thanks again to your contribution and we look forward to looking at it!

cjrh · 2017-05-26T16:58:42Z

Something has gone wrong with a merge. I'll need to fix up my PR before we continue.

cjrh · 2017-05-26T17:07:49Z

Phew! Just needed some aggressive rebasing and force pushing. Should be good now.

willingc · 2017-05-26T17:13:45Z

@cjrh Looks like travis is unhappy with a virtual environment within a virtual environment.

cjrh · 2017-05-26T20:46:09Z

I could change it so that if make is being run within a virtual environment, then it doesn't try to make a new one? (Of course, that means that we will not be able to test that it can make a new one successfully in the case that make is being run from a non-virtual environment...🤔)

csabella · 2017-06-03T17:58:36Z

@cjrh - I followed the new make html instructions and received an error on the build. For reference, I also received the same error when building cherry_picker. It seems my venvs don't come with wheels.

Building wheels for collected packages: MarkupSafe
  Running setup.py bdist_wheel for MarkupSafe ... error
  Complete output from command /home/cheryl/cpython/Doc/venv/bin/python3 -u -c "import setuptools, tokenize;__file__='/tmp/pip-build-xha3qftv/MarkupSafe/setup.py';exec(compile(getattr(tokenize, 'open', open)(__file__).read().replace('\r\n', '\n'), __file__, 'exec'))" bdist_wheel -d /tmp/tmpow5wp4n8pip-wheel- --python-tag cp35:
  usage: -c [global_opts] cmd1 [cmd1_opts] [cmd2 [cmd2_opts] ...]
     or: -c --help [cmd1 cmd2 ...]
     or: -c --help-commands
     or: -c cmd --help
  
  error: invalid command 'bdist_wheel'
  
  ----------------------------------------
  Failed building wheel for MarkupSafe
  Running setup.py clean for MarkupSafe
Failed to build MarkupSafe

Here's the link to the PR and issue. (python/core-workflow#105)

Thanks!

csabella · 2017-06-03T18:04:26Z

Doc/README.rst

Is there an extra tick on Doc?

Well-spotted!

cjrh · 2017-06-04T04:35:39Z

@csabella Regarding the problem with wheels: do you mean that when you change into Doc/ and run $ make html in this PR, then you received an error because the wheel package did not get installed into the new venv that gets automatically created in this PR to install the Sphinx package? At which stage is wheel required to install Sphinx?

In the Makefile, I can easily change the venv section to read:

venv:
	$(PYTHON) -m venv venv
	./venv/bin/python3 -m pip install -U Sphinx wheel

but I'm unclear of the benefit of installing wheel here (my venv also doesn't get the wheel package installed automatically, and the Sphinx build runs fine).

csabella · 2017-06-04T09:33:59Z

@cjrh Thanks, yes, that's what I meant. My assumption was that I got the message because wheel wasn't installed, but there very well could be another reason. In the cherry_picker project, if I installed wheel inside the venv after creating it, then I didn't get the message, as you suggest for a fix. Perhaps this is something specific to me and my path? If it is, then I can work around it. I was just concerned that make html was one of the first things I tried to do (based on the DevGuide) and I would have been concerned if the make html command had given an error.

One point though is that the rest of the make works and the html is still created.

Here's is the entire log:

cpython/Doc$ make html
python3 -m venv venv
./venv/bin/python3 -m pip install -U Sphinx
Collecting Sphinx
  Using cached Sphinx-1.6.2-py2.py3-none-any.whl
Collecting babel!=2.0,>=1.3 (from Sphinx)
  Using cached Babel-2.4.0-py2.py3-none-any.whl
Collecting docutils>=0.11 (from Sphinx)
  Using cached docutils-0.13.1-py3-none-any.whl
Collecting alabaster<0.8,>=0.7 (from Sphinx)
  Using cached alabaster-0.7.10-py2.py3-none-any.whl
Collecting snowballstemmer>=1.1 (from Sphinx)
  Using cached snowballstemmer-1.2.1-py2.py3-none-any.whl
Collecting Jinja2>=2.3 (from Sphinx)
  Using cached Jinja2-2.9.6-py2.py3-none-any.whl
Collecting Pygments>=2.0 (from Sphinx)
  Using cached Pygments-2.2.0-py2.py3-none-any.whl
Collecting sphinxcontrib-websupport (from Sphinx)
  Using cached sphinxcontrib_websupport-1.0.1-py2.py3-none-any.whl
Collecting setuptools (from Sphinx)
  Using cached setuptools-36.0.1-py2.py3-none-any.whl
Collecting requests>=2.0.0 (from Sphinx)
  Using cached requests-2.17.3-py2.py3-none-any.whl
Collecting six>=1.5 (from Sphinx)
  Using cached six-1.10.0-py2.py3-none-any.whl
Collecting imagesize (from Sphinx)
  Using cached imagesize-0.7.1-py2.py3-none-any.whl
Collecting pytz>=0a (from babel!=2.0,>=1.3->Sphinx)
  Using cached pytz-2017.2-py2.py3-none-any.whl
Collecting MarkupSafe>=0.23 (from Jinja2>=2.3->Sphinx)
  Using cached MarkupSafe-1.0.tar.gz
Collecting certifi>=2017.4.17 (from requests>=2.0.0->Sphinx)
  Using cached certifi-2017.4.17-py2.py3-none-any.whl
Collecting idna<2.6,>=2.5 (from requests>=2.0.0->Sphinx)
  Using cached idna-2.5-py2.py3-none-any.whl
Collecting chardet<3.1.0,>=3.0.2 (from requests>=2.0.0->Sphinx)
  Using cached chardet-3.0.3-py2.py3-none-any.whl
Collecting urllib3<1.22,>=1.21.1 (from requests>=2.0.0->Sphinx)
  Using cached urllib3-1.21.1-py2.py3-none-any.whl
Building wheels for collected packages: MarkupSafe
  Running setup.py bdist_wheel for MarkupSafe ... error
  Complete output from command /home/cheryl/cpython/Doc/venv/bin/python3 -u -c "import setuptools, tokenize;__file__='/tmp/pip-build-igej7bg5/MarkupSafe/setup.py';exec(compile(getattr(tokenize, 'open', open)(__file__).read().replace('\r\n', '\n'), __file__, 'exec'))" bdist_wheel -d /tmp/tmphble1qinpip-wheel- --python-tag cp35:
  usage: -c [global_opts] cmd1 [cmd1_opts] [cmd2 [cmd2_opts] ...]
     or: -c --help [cmd1 cmd2 ...]
     or: -c --help-commands
     or: -c cmd --help
  
  error: invalid command 'bdist_wheel'
  
  ----------------------------------------
  Failed building wheel for MarkupSafe
  Running setup.py clean for MarkupSafe
Failed to build MarkupSafe
Installing collected packages: pytz, babel, docutils, alabaster, snowballstemmer, MarkupSafe, Jinja2, Pygments, sphinxcontrib-websupport, setuptools, certifi, idna, chardet, urllib3, requests, six, imagesize, Sphinx
  Running setup.py install for MarkupSafe ... done
  Found existing installation: setuptools 20.7.0
    Uninstalling setuptools-20.7.0:
      Successfully uninstalled setuptools-20.7.0
Successfully installed Jinja2-2.9.6 MarkupSafe-1.0 Pygments-2.2.0 Sphinx-1.6.2 alabaster-0.7.10 babel-2.4.0 certifi-2017.4.17 chardet-3.0.3 docutils-0.13.1 idna-2.5 imagesize-0.7.1 pytz-2017.2 requests-2.17.3 setuptools-36.0.1 six-1.10.0 snowballstemmer-1.2.1 sphinxcontrib-websupport-1.0.1 urllib3-1.21.1
You are using pip version 8.1.1, however version 9.0.1 is available.
You should consider upgrading via the 'pip install --upgrade pip' command.
./venv/bin/sphinx-build -b html -d build/doctrees -D latex_elements.papersize=  . build/html 
Running Sphinx v1.6.2
loading pickled environment... done
building [mo]: targets for 0 po files that are out of date
building [html]: targets for 466 source files that are out of date
updating environment: 0 added, 0 changed, 0 removed
looking for now-outdated files... none found
preparing documents... done
writing output... [100%] whatsnew/index                                         
generating indices... genindex py-modindex
writing additional pages... download index search opensearch
copying images... [100%] library/pathlib-inheritance.png                        
copying downloadable files... [100%] includes/tzinfo_examples.py                
copying static files... done
copying extra files... done
dumping search index in English (code: en) ... done
dumping object inventory... done
build succeeded.

Build finished. The HTML pages are in build/html.

Requested changes made

zware · 2017-06-05T03:04:52Z

I think we should try to avoid building a venv if SPHINXBUILD is set to something other than ./venv/bin/sphinx-build, and we should set SPHINXBUILD to sphinx-build in .travis.yml so that we can set the Sphinx version used by Travis independently.

cjrh · 2017-06-05T03:23:12Z

@zware Agreed, I'll get those changes made sometime this week.

cjrh · 2017-06-24T04:28:45Z

Apologies for the delay on this.

@zware: I've changed it so that the auto-venv creation only runs if SPHINXBUILD does not match the path ./venv/bin/sphinx-build. I've also added SPHINXBUILD=sphinx-build to .travis.yml, so that the DOC test will use the sphinx already installed.

@csabella: I've also looked into the wheel issue. The problem you're seeing appear to happen because a wheel is being built for the package MarkupSafe. When I run make html, this is what happens with that package:

Collecting MarkupSafe>=0.23 (from Jinja2>=2.3->Sphinx)
  Downloading http://localhost:3141/root/pypi/+f/2fc/edc9284d50e57/MarkupSafe-1.0.tar.gz

<snip>

Installing collected packages: Pygments, setuptools, alabaster, six, snowballstemmer, docutils, MarkupSafe, Jinja2, pytz, babel, urllib3, idna, chardet, certifi, requests, sphinxcontrib-websupport, imagesize, Sphinx
  Found existing installation: setuptools 28.8.0
    Uninstalling setuptools-28.8.0:
      Successfully uninstalled setuptools-28.8.0
  Running setup.py install for MarkupSafe ... done

For me, the pip install Sphinx process does not try to build a wheel for MarkupSafe. In your output, it looks like there is an extra step where pip wants to build a wheel:

Building wheels for collected packages: MarkupSafe
  Running setup.py bdist_wheel for MarkupSafe ... error

<snip>

  Failed building wheel for MarkupSafe
  Running setup.py clean for MarkupSafe
Failed to build MarkupSafe
Installing collected packages: pytz, babel, docutils, alabaster, snowballstemmer, MarkupSafe, Jinja2, Pygments, sphinxcontrib-websupport, setuptools, certifi, idna, chardet, urllib3, requests, six, imagesize, Sphinx
  Running setup.py install for MarkupSafe ... done
  Found existing installation: setuptools 20.7.0
    Uninstalling setuptools-20.7.0:
      Successfully uninstalled setuptools-20.7.0

This leads me to think that you might have an option in your pip.conf that instructs pip to always build and add wheels to a local cache or wheelhouse?

In any case, if it's happening to you it will likely happen to other people, and there is very little cost to also installing wheel into our venv, so I think I should just add that to the Sphinx install. Agree @zware ?

cjrh · 2017-06-24T04:31:14Z

@csabella Perhaps options similar to these?

# pip.conf
[global]
use-wheel = true
wheel-dir = /Users/<username>/.pip/wheels
find-links = /Users/<username>/.pip/wheels

…thonGH-4343)

kB (*kilo* byte) unit means 1000 bytes, whereas KiB ("kibibyte") means 1024 bytes. KB was misused: replace kB or KB with KiB when appropriate. Same change for MB and GB which become MiB and GiB. Change the output of Tools/iobench/iobench.py. Round also the size of the documentation from 5.5 MB to 5 MiB.

…uto-venv-docbuilder

the-knights-who-say-ni · 2017-11-09T03:45:13Z

Hello, and thanks for your contribution!

I'm a bot set up to make sure that the project can legally accept your contribution by verifying you have signed the PSF contributor agreement (CLA).

Unfortunately we couldn't find an account corresponding to your GitHub username on bugs.python.org (b.p.o) to verify you have signed the CLA (this might be simply due to a missing "GitHub Name" entry in your b.p.o account settings). This is necessary for legal reasons before we can look at your contribution. Please follow the steps outlined in the CPython devguide to rectify this issue.

Thanks again to your contribution and we look forward to looking at it!

cjrh · 2017-11-09T03:47:29Z

Welp, that looks bad! My rebase went wrong sigh

cjrh · 2017-11-09T03:48:02Z

Closing this PR and starting again.

the-knights-who-say-ni added the CLA signed label May 23, 2017

zware previously requested changes May 25, 2017

View reviewed changes

willingc reviewed May 26, 2017

View reviewed changes

Mariatta changed the title ~~Auto venv docbuilder~~ bpo-30487: automatically create a venv and install Sphinx when running make May 26, 2017

Mariatta added docs Documentation in the Doc dir type-feature A feature request or enhancement and removed docs Documentation in the Doc dir labels May 26, 2017

the-knights-who-say-ni added CLA not signed and removed CLA signed labels May 26, 2017

cjrh force-pushed the auto-venv-docbuilder branch from 14e72b1 to 6802b54 Compare May 26, 2017 17:04

the-knights-who-say-ni added CLA signed and removed CLA not signed labels May 26, 2017

willingc approved these changes May 26, 2017

View reviewed changes

csabella reviewed Jun 3, 2017

View reviewed changes

Mariatta added the sprint label Jun 12, 2017

cjrh force-pushed the auto-venv-docbuilder branch from 265b12b to 1b9306a Compare June 24, 2017 04:12

berkerpeksag and others added 14 commits November 9, 2017 00:43

bpo-11063: Use more reliable way to check if uuid function exists (py…

0e163d2

…thonGH-4343)

Remove venv from .PHONY to avoid unnecessary venv creation attempts

e0405c2

Update docs to mention that Sphinx dependency handling is automatic

90ba95c

Remove invalid comment, since Makefile already specifies "python3"

f740066

Updated PR from review comments.

b9b75f3

Update docs to mention that Sphinx dependency handling is automatic

9e730c9

Fix merge conflict comments from a previous merge

a3daba7

Remove extraneous backtick spotted by @csabella

06f0523

Only create a venv if SPHINXBUILD option matches the default.

935195a

Make travis doc test use the sphinx already installed.

d737281

Remove bad merge artifacts.

968c471

Fix README for recent addition of blurb.

3715329

Merge branch 'auto-venv-docbuilder' of github.com:cjrh/cpython into a…

d77c676

…uto-venv-docbuilder

cjrh requested review from a team, 1st1, gpshead, ncoghlan, rhettinger, skrah and terryjreedy as code owners November 9, 2017 03:45

cjrh requested a review from a team November 9, 2017 03:45

cjrh requested a review from a team as a code owner November 9, 2017 03:45

the-knights-who-say-ni added CLA not signed and removed CLA signed labels Nov 9, 2017

cjrh closed this Nov 9, 2017

cjrh mentioned this pull request Nov 9, 2017

bpo-30487: automatically create a venv and install Sphinx when running make #4346

Merged

Uh oh!

Conversation

cjrh commented May 23, 2017 • edited by bedevere-bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

mention-bot commented May 23, 2017

Uh oh!

zware left a comment

Choose a reason for hiding this comment

Uh oh!

zware May 23, 2017

Choose a reason for hiding this comment

Uh oh!

zware May 23, 2017

Choose a reason for hiding this comment

Uh oh!

willingc left a comment

Choose a reason for hiding this comment

Uh oh!

willingc May 26, 2017

Choose a reason for hiding this comment

Uh oh!

the-knights-who-say-ni commented May 26, 2017

Uh oh!

cjrh commented May 26, 2017

Uh oh!

cjrh commented May 26, 2017

Uh oh!

willingc commented May 26, 2017

Uh oh!

cjrh commented May 26, 2017

Uh oh!

csabella commented Jun 3, 2017

Uh oh!

csabella Jun 3, 2017

Choose a reason for hiding this comment

Uh oh!

cjrh Jun 4, 2017

Choose a reason for hiding this comment

Uh oh!

cjrh commented Jun 4, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

csabella commented Jun 4, 2017

Uh oh!

zware commented Jun 5, 2017

Uh oh!

cjrh commented Jun 5, 2017

Uh oh!

cjrh commented Jun 24, 2017

Uh oh!

cjrh commented Jun 24, 2017

Uh oh!

the-knights-who-say-ni commented Nov 9, 2017

Uh oh!

cjrh commented Nov 9, 2017

Uh oh!

cjrh commented Nov 9, 2017

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

19 participants

cjrh commented May 23, 2017 •

edited by bedevere-bot

Loading

cjrh commented Jun 4, 2017 •

edited

Loading