jbms / sphinx-immaterial

Adaptation of the popular mkdocs-material material design theme to the sphinx documentation system
https://jbms.github.io/sphinx-immaterial/
Other
177 stars 28 forks source link

incompatible with readthedocs builds and fail_on_warning option #283

Closed chrisjsewell closed 9 months ago

chrisjsewell commented 10 months ago

Heya,

This theme causes readthedocs to emit a warning, meaning that https://docs.readthedocs.io/en/stable/config-file/v2.html#sphinx-fail-on-warning will always fail (and this also means that it is not possible to properly use https://docs.readthedocs.io/en/stable/pull-requests.html)

You can see this in your own docs builds: https://readthedocs.org/projects/sphinx-immaterial/builds/21647542/

WARNING: Missing searchtools: /home/docs/checkouts/readthedocs.org/user_builds/sphinx-immaterial/checkouts/stable/_readthedocs/html/_static/searchtools.js
2bndy5 commented 10 months ago

Actually, I think this warning comes from the RTD sphinx extension (mandated to add the version drop down info). It could be argued that the RTD sphinx extension shouldn't assume the Sphinx basic theme's assets are present.

We could specifically not exclude that file when RTD env is detected https://github.com/jbms/sphinx-immaterial/blob/c61bad99139f17d535b9a287fac920f6d639b416/sphinx_immaterial/__init__.py#L61-L85 but IDK if that will have a conflict with the actual JS files being used.

chrisjsewell commented 10 months ago

Thanks for the quick response @2bndy5 .

Firstly, it would be good to clarify if this is actually breaking anything in the RTD builds?

Next, I think a minimal "fix" could be to ask the RTD guys to change this line: https://github.com/readthedocs/readthedocs-sphinx-ext/blob/7c60d1646c12ac0b83d61abfbdd5bcd77d324124/readthedocs_ext/readthedocs.py#L273

to something like

        log.warning('Missing searchtools: {}'.format(searchtools_file), type="readthedocs", subtype="searchtools")

this way one could at least use https://www.sphinx-doc.org/en/master/usage/configuration.html#confval-suppress_warnings, to ignore the warning (and you could document that here)

2bndy5 commented 10 months ago

Firstly, it would be good to clarify if this is actually breaking anything in the RTD builds?

The only thing I can think of is the RTD search feature scoped to the entire RTD project (all builds not just an individual build).

Next, I think a minimal fix could be to ask the RTD guys to change ...

I will escalate this as it does seem like they're very responsive. We might even get a better answer to the first question.

2bndy5 commented 9 months ago

This problem was resolved upstream.