godot-docs icon indicating copy to clipboard operation
godot-docs copied to clipboard

Update Sphinx and requirements

Open mhilbrunner opened this issue 2 years ago • 9 comments

In the vein of https://github.com/godotengine/godot-docs/pull/7436, this is a start on updating the version of Sphinx and its requirements we're using as everything is getting kinda old (we use Sphinx 4.x, current is 7.x; this PR updates it to 5.x at least).

Updating to Sphinx 6.x currently seems to break some things (like the sidebar menu toggling) which we need to investigate.

Checked in a local build:

  • General build and layout
  • Sidebar menu
  • Images
  • Code tabs
  • Code highlighting

Things that seem iffy/may need testing:

  • Search

Supersedes https://github.com/godotengine/godot-docs/pull/6575, https://github.com/godotengine/godot-docs/pull/6755, https://github.com/godotengine/godot-docs/pull/6749 Sphinx changelog: https://www.sphinx-doc.org/en/master/changes.html#release-5-0-2-released-jun-17-2022

mhilbrunner avatar May 30 '23 16:05 mhilbrunner

as everything is getting kinda old

It would be nice to document somewhere, preferably in this and similar PRs, what are the main benefits from updating to the newer versions. Not to sound like a luddite, but since it works right now and it is not a security concern, we could just keep it as is.

Updating would likely require us to rewrite our custom styles and scripts to keep working with newer versions (as you mention, sidebar is broken in 6+ as an example, every time we update the theme our style overrides break in unpredictable places). And such work doesn't come for free.

YuriSizov avatar May 30 '23 16:05 YuriSizov

but since it works right now and it is not a security concern, we could just keep it as is.

My feeling is that the longer we wait, the more effort updating is going to take (because everything will break, instead of a small breakage here or there that is easier to locate and fix), and eventually we will need to update.

Something like https://github.com/godotengine/godot-docs/pull/7289: we are forced to update a single component (like the OS), which may then require us to live with a newer Python, which requires a newer Sphinx, or which requires newer dependencies etc. Or a security issue is discovered, we now need to update a component, but doing that takes a lot of time as everything grew outdated.

That said, I agree this is a consideration, and currently there is nothing forcing us to update. I'd prefer to do it on our terms, instead of it coming back to haunt us :)

This PR is intended as "research how much work is needed to update dependencies", and if it takes too much work for now, we can postpone. My initial version updated to 6.x/7.x, but a lot broke; so I went with 5.x and so far not too much seems to break at first glance.

mhilbrunner avatar May 30 '23 16:05 mhilbrunner

Yeah, that's fair. If we don't update regularly, we may be in a bad place to do that by the time it's actually required. Though I don't see anything particularly critical that might arise in the future. It's just a PITA to adjust our stuff all the time. Perhaps we should implement it in a less hacky way. For example, we can actually fork the theme and make our adjustments a native part of our forked theme.

YuriSizov avatar May 30 '23 17:05 YuriSizov

  • the sphinx_rtd_theme is not compatible with sphinx 7 at the moment, the highest version we can choose is 6.2.1
  • The sidebar toggling is broken because sphinx 6.0 removed jquery, adding it manually restores the functionality. (https://www.sphinx-doc.org/en/master/changes.html#id91)

I did a complete build with 6.2.1 and got a lot of svg errors, not sure what this is about.

Piralein avatar May 30 '23 18:05 Piralein

We also extend templates, both core and from the RTD theme. So we need to double-check that our overrides don't erase anything added by newer versions.

YuriSizov avatar May 30 '23 19:05 YuriSizov

@Piralein Good call on JQuery, thats indeed an easy fix. From what I can see the SVG errors are more like warnings (they added social media preview card support, which also support showing images on the page, but SVG isn't supported for that so it generates warnings for every page where it would pick a SVG as the first image to preview and does nothing; which IMO is okay)

mhilbrunner avatar May 30 '23 22:05 mhilbrunner

Updating Sphinx could resolve the issue with make latexpdf not working due to https://github.com/sphinx-doc/sphinx/issues/10903. I'd like to add PDF builds to the offline CI workflow in the long run, as the large ePubs we currently build have many issues opening on old/low-end devices.

Calinou avatar Jun 06 '23 09:06 Calinou

I tested locally with:

sphinx==6.2.1
sphinx_rtd_theme==1.3.0
sphinx-tabs==3.4.1
sphinx-copybutton==0.5.2
sphinx-notfound-page==1.0.0
sphinxext-opengraph==0.8.2

All modules seem to be compatible. There are only 2 issues:

  1. The warnings with social cards - added by sphinxext-opengraph This can be disabled, see: https://github.com/wpilibsuite/sphinxext-opengraph/blob/main/docs/source/socialcards.md
ogp_social_cards = {
    "enable": False
}
  1. our config prevents the sphinx_rtd_theme to load the sphinxcontrib.jquery module. https://github.com/readthedocs/sphinx_rtd_theme/issues/1452 It works by removing html_theme_path = [sphinx_rtd_theme.get_html_theme_path()]. This seems to be some outdated configuration no longer used. The documentation of https://sphinx-rtd-theme.readthedocs.io/en/stable/installing.html doesn't mention it.

we could also add it to the extension ourself:

extensions = [
    "sphinxcontrib.jquery",
]

Piralein avatar Oct 02 '23 20:10 Piralein

pygments==2.18.0

sphinx==6.2.1
sphinx_rtd_theme==1.3.0

sphinx-tabs==3.4.5
sphinx-copybutton==0.5.2
sphinx-notfound-page==1.0.2
sphinxext-opengraph==0.9.1

sphinxcontrib-applehelp==1.0.8
sphinxcontrib-htmlhelp==2.0.5
sphinxcontrib-qthelp==1.0.7
sphinxcontrib-serializinghtml==1.1.10
sphinxcontrib-devhelp==1.0.6

sphinxcontrib-video==0.2.1

My current setup, I haven't discovered any problems yet. (with the changes above)

Piralein avatar Jun 27 '24 18:06 Piralein

Superseded by https://github.com/godotengine/godot-docs/pull/10158.

mhilbrunner avatar Nov 02 '24 14:11 mhilbrunner