Document the make venv documentation build target
#2953
Conversation
brettcannon
added
needs backport to 3.5
skip issue
docs
Doc/Makefile
Outdated
| @@ -5,6 +5,7 @@ | |||
|
|
|||
| # You can set these variables from the command line. | |||
| PYTHON = python3 | |||
| VENVDIR = env | |||
There was a problem hiding this comment.
What's your thinking about changing the name of the venv directory from venv to env? Making the change would introduce a compatibility issue; even though "make venv" wasn't documented, it's likely being used and, besides leaving a stray "venv" directory around, it would also require current doc builders to change their scripts.
There was a problem hiding this comment.
I've just seen more use of env instead than venv out in the wild is all. I can put it back.
|
|
||
| On Windows, set the PYTHON environment variable instead. | ||
| make html PYTHON=./env/bin/python |
There was a problem hiding this comment.
Alas, this is currently insufficient. The sphinx-build command in the venv won't be found without other Makefile changes (it doesn't fail if you already have sphinx-build installed on your PATH). Since we're going to have tweak this a bit once the blurb support is merged in, suggest for now just adding a shell source command prior to the make html, i.e. . ./env/bin/activate or (. ./venv/bin/activate).
There was a problem hiding this comment.
Price I pay for wrapping this up before rushing out the door. 😞
But I think I found a better solution than having to activate the venv blindly: setting SPHINXBUILD to $(PYTHON) -m sphinx. It's the same as sphinx-build but with the simpler setting of PYTHON being properly supported. Does that work for you, @ned-deily ?
There was a problem hiding this comment.
I know I suggested something similar for blurb. But now I'm not sure that was the best choice. It's more problematic with sphinx-build since existing doc builds may have been depending on using an already installed (e.g. system) sphinx-build and have never bothered before to run the make venv step (and before, even if they did run that step, it wouldn't have made a difference without tweaking PYTHON). With this change, those builds would now fail unless sphinx had happened to be installed in whatever python3 on PATH points to. I don't know how many users this would affect but it's not hypothetical: I ran into this myself. There are several ways around it; I was thinking the most straightforward way might be to play with PATH in the "build" rule; so something like:
PATH=./venv/bin:$PATH sphinx-build
Either that or we need to make sure this is documented as an incompatible change and then maybe only for 3.7?
There was a problem hiding this comment.
Another possibility would be to make the build rule depend on the venv rule but, as it stands, that causes venv to be run every time you do any doc build.
There was a problem hiding this comment.
My worry with the PATH=./$(VENVDIR)/bin:$PATH $(SPHINXBUILD) solution is won't this require being set for anything that might have an entry in bin? And that seems a bit more error-prone long-term than this current approach as I can see forgetting to set PATH in some rule.
I'm fine with this being a Python 3.7-only thing if that doesn't cause you grief in 3.6 releases. But if you would rather have the PATH solution I'm fine with it as well. Just let me know your preference and I'll update the PR.
|
@ned-deily any update on whether my proposed solution works for you? |
|
|
||
| On Windows, set the PYTHON environment variable instead. | ||
| make html PYTHON=./env/bin/python |
There was a problem hiding this comment.
I know I suggested something similar for blurb. But now I'm not sure that was the best choice. It's more problematic with sphinx-build since existing doc builds may have been depending on using an already installed (e.g. system) sphinx-build and have never bothered before to run the make venv step (and before, even if they did run that step, it wouldn't have made a difference without tweaking PYTHON). With this change, those builds would now fail unless sphinx had happened to be installed in whatever python3 on PATH points to. I don't know how many users this would affect but it's not hypothetical: I ran into this myself. There are several ways around it; I was thinking the most straightforward way might be to play with PATH in the "build" rule; so something like:
PATH=./venv/bin:$PATH sphinx-build
Either that or we need to make sure this is documented as an incompatible change and then maybe only for 3.7?
|
A Python core developer, ned-deily, has requested some changes be Once you have made the requested changes, please leave a comment |
To help with GH-2719.