Skip to content

make.py should not use os.system("sphinx-build ...") #8208

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

Closed
anntzer opened this issue Mar 6, 2017 · 7 comments
Closed

make.py should not use os.system("sphinx-build ...") #8208

anntzer opened this issue Mar 6, 2017 · 7 comments

Comments

@anntzer
Copy link
Contributor

anntzer commented Mar 6, 2017

I know the plan is to switch to a doc makefile, but the issue would be the same.

Calling os.system("sphinx-build ...") (or directly sphinx-build in the Makefile) will pick up a sphinx install outside of the current environment, which may or may not have an unrelated (e.g., older) version of matplotlib installed. Instead, the command should be something like python -msphinx build ... if sphinx offers that option, or we should use a wrapper script otherwise that makes sure that sphinx is used from the current environment only.

@matthew-brett
Copy link
Contributor

Looking at the contents of sphinx-build, you could probably get away with something like:

import sphinx
sphinx.build_main(['sphinx-build'] + input_args)

@QuLogic
Copy link
Member

QuLogic commented Mar 6, 2017

I don't understand your concern; why would it pick up sphinx-build outside the environment? If the environment didn't install the executable and modify PATH, then something is wrong with the environment.

There is a problem on Python 3 though if using system packages, because they generally install both sphinx-build and sphinx-build-3. This is handled by the Makefile using a modifiable variable SPHINXBUILD which can be set when calling make: make SPHINXBUILD=sphinx-build-3.

@matthew-brett
Copy link
Contributor

It's quite common in my experience for me to have sphinx-build installed on the system, or in my user installs, and for that to get picked up when I am working inside a virtualenv, on which I have not installed sphinx. That can be pretty confusing.

@QuLogic
Copy link
Member

QuLogic commented Mar 6, 2017

Oh yes, not having it installed at all, good point. We can fix this now, but I think there's not really much of a way to fix this when switching to the Makefile, though.

@anntzer
Copy link
Contributor Author

anntzer commented Mar 7, 2017

Actually, as of sphinx 1.5.3, you can just do

python -msphinx -bhtml $srcdir $destdir

Perhaps we can just check since when this form is supported and update the requirement. I will open an issue upstream to fix their Makefile/make.bat as well.

@anntzer
Copy link
Contributor Author

anntzer commented Mar 7, 2017

xref sphinx-doc/sphinx#3523

@anntzer
Copy link
Contributor Author

anntzer commented Mar 27, 2017

Closed by #8269.

@anntzer anntzer closed this as completed Mar 27, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants