Ensure invoked Sphinx matches importable one

[eric-wieser]

This should fix crashes when both conda and pip have Sphinx installed. It's likely also negligibly faster since it avoids the binary wrappers.

[eric-wieser]

Note that this approach is already what you use for mkdocs.

[stsewd]

We need to test this across all sphinx versions (>=1.5).

But looks like it should work https://github.com/sphinx-doc/sphinx/blob/v1.5/sphinx/__main__.py

[eric-wieser]

We need to test this across all sphinx versions (>=1.5).

But looks like it should work https://github.com/sphinx-doc/sphinx/blob/v1.5/sphinx/__main__.py

It was added in 1.3: sphinx-doc/sphinx@74c7a52

[stsewd]

Tested this with a couple of versions, everything looks correct 👍

[ericholscher]

Interesting. Anything that changes how we do builds scares me, but this seems like a reasonable change. I wonder if we should put it behind a feature flag to test it, and then roll it out via historical_default=True, so it's easy to rollback if needed?

[stsewd]

@eric-wieser this is how we add feature flags https://github.com/readthedocs/readthedocs.org/pull/6729/files let me know if you want to give it a try o I can take it from here.

The flag could be called USE_SPHINX_FROM_VENV or FORCE_SPHINX_FROM_VENV or similar

[eric-wieser]

I'd prefer if you could take it from here, thanks.

I think a more suitable name for the flag might be DONT_MIX_PYTHONS_FOR_SPHINX - currently the system python is being used to invoke a script from the venv, which seems like a poor idea. The -m sphinx change here is probably far less relevant than the switch to using the venv python. If you're concerned, you could roll out the change in two pieces.

Whatever you decide, could you enable this new behavior on the cocotb.rtfd.io PR builds (and ping me when you've done so), so we can see if it fixes our issue? Thanks!

[ericholscher]

Looks good. 👍

[eric-wieser]

Should there be a test for this? There was one present in 1fa3dda, but it seems it was removed again.

[stsewd]

Should there be a test for this? There was one present in 1fa3dda, but it seems it was removed again.

I'll add it back when the feature flag is enabled by default.

[humitos]

@eric-wieser I enabled this feature flag in your project. Let me know if it does work as you expected. Thanks!

[eric-wieser]

Definitely doesn't break anything. Not sure if I can easily reproduce the original issue, since it relied on PyPI having a newer release of sphinx than conda, something that only happens immediately after a sphinx release.