Re: [LTP] [PATCH v4 2/5] doc: Add sphinx to requirements.txt

Andrea Cervesato via ltp
2025-04-01T08:18:43Z

On 3/31/25 19:53, Petr Vorel wrote:
>> On 3/31/25 18:37, Petr Vorel wrote:
>>> Wait, one thing: we use doc/requirements.txt in .readthedocs.yml. Is it really a
>>> good idea to add there sphinx, when it uses containers? I can test it in a
>>> separate branch, but we discussed some time ago that we should avoid adding
>>> sphinx for readthedocs.
>>> Wouldn't be in the end better to keep 'pip install sphinx' in Makefile instead
>>> (the previous proposal).
>>> Kind regards,
>>> Petr
>> I forgot about it. Thanks for remembering. At the moment readthedocs
>> installs sphinx via pip, so if we add just sphinx ro requirements.txt
>> without version, pip should reinstall the same version which is already
>> available before building the website.
>> We should give a try and eventually get back to the old way.
> To be honest I would prefer to avoid experimenting with readthedoc as much as
> possible. How about just forcing 'pip install sphinx' only in Makefile?
> It can be the simplest version - without checking if it's installed.
The thing is that readthedoc is already doing "pip install sphinx"
before building the website, so any other "pip install sphinx" or "pip
install requirements.txt" (with sphinx in it) won't create issues. pip
will check the cache, finding sphinx already installed and it will skip
its installation.

The only thing we should avoid is the usage of sphinx version. So we
just keep "sphinx" as a generic package inside requirements.txt and pip
will take care about the version.

> Because I don't see any benefit to add sphinx to readthedocs where it's not
> needed - in the best case it will takes time to reinstall, in worst it just take
> time to experiment to find it breaks things. We are going to diverge from
> readthedocs anyway, unless we manually sync with the sphinx version it uses.
> You asked for newest version (which is 8.2.3, if I don't count broken 8.3),
> but in readthedocs we use old ubuntu-22.04, which uses sphinx 4.3.2 [1].
>
> We could update to noble (24.04LTS) (if supported by readthedocs) to get newer
> 7.2.6 (as I can test it as a separate effort.
>
> [1] https://packages.ubuntu.com/jammy/python3-sphinx
> [2] https://packages.ubuntu.com/noble/python3-sphinx
>
> I guess controlling the version is important, therefore I would prefer b) or c).
>
> a) simple version in Makefile, without version:
>
> $(VENV_DIR):
> $(PYTHON) -m virtualenv $(VENV_DIR)
> $(VENV_CMD) && pip install sphinx && pip install -r requirements.txt
>
> b) simple version with specifying sphinx version:
>
> SPHINX_VERSION := 8.2.3 # or 7.2.6 or 4.3.2?
> ...
> $(VENV_DIR):
> $(PYTHON) -m virtualenv $(VENV_DIR)
> $(VENV_CMD) && pip install sphinx==$(SPHINX_VERSION) && pip install -r requirements.txt
>
> c) more complicated version (you did not like) which install sphinx only when
> needed:
>
> SPHINX_VERSION := 8.2.3
> INSTALL_SPHINX := $(shell $(PYTHON) -c "import sphinx" 2>/dev/null && echo ":" || echo "pip install sphinx==$(SPHINX_VERSION)")
> ...
> $(VENV_DIR):
> $(PYTHON) -m virtualenv $(VENV_DIR)
> $(VENV_CMD) && pip install -r requirements.txt && $(INSTALL_SPHINX)
>
> Kind regards,
> Petr
>
>> Andrea
Andrea