Support docutils 0.18
Closes https://github.com/readthedocs/sphinx_rtd_theme/issues/1302
Build step is failing but I'm not sure how to fix that
docutils 0.19 has been released.
Possibly a related bug in this PR:
Two colons are still rendered with sphinx-rtd-theme and Sphinx 5.1.0.
https://github.com/readthedocs/sphinx_rtd_theme/issues/1302#issuecomment-1195290758
Please do say if you have any findings here :pray:
~I'd say that the "two colons" issue is important, but that it's a very little issue and shouldn't block us from the upgrade here, since it's causing issues for other important projects.~
Edit: Issue is fixed in Sphinx
Any chance to make new release? It is already quite long since last release has been made ..
@kloczek that's definitely the intention
I'll have to investigate these current build issues though:
Exception occurred:
File "/home/circleci/project/.tox/py27-sphinxlatest/lib/python2.7/site-packages/sphinx/ext/intersphinx.py", line 365, in missing_reference
contnode[0] = nodes.Text(newtarget, contnode[0].rawsource)
AttributeError: 'Text' object has no attribute 'rawsource'
Above error is fixed in https://github.com/sphinx-doc/sphinx/commit/961f5af0963055f0206a437765a087a2bf03bcca
This commmit is released with Sphinx 4.3.0. Hence, Sphinx releases prior to this version definitely have this issue and aren't compatible with docutils 0.18. What might be confusing is that the error isn't occurring 100% reliably.. but that's another story.
Should we handle this simply in our Tox matrix?
It turns out that the build error was because of a missing lower bound on Sphinx in tox.ini, meaning that for Python 2.7, we always got Sphinx 1.8 for everything intended to be 2.0+. In combination with docutils 0.18, this gave the error.
After specifying the versions correctly in tox.ini and .circleci/config.yml, we are fine :+1:
hey @benjaoming, I've been really busy so thanks for taking on the PR π
Not sure if https://github.com/readthedocs/sphinx_rtd_theme/pull/1338 might cause a need to solve conflicts, currently it has a clean cherry-pick.
I wanted to test this branch with
pip install git+https://github.com/edgarrmondragon/[email protected]
but somehow the docutils seems to be still set as < docutils 0.18
See pip error:
The conflict is caused by: sphinx-rtd-theme 1.0.1a1 depends on docutils<0.18
The branch from @benjaoming however installs correctly, but I do not need docutils 0.19 yet (due to incompatibility with some other packages)
pip install git+https://github.com/benjaoming/[email protected] works.
Any clue why?
@twodrops
I wanted to test this branch with pip install git+https://github.com/edgarrmondragon/[email protected] but somehow the docutils seems to be still set as < docutils 0.18
Can you verify the version of Sphinx and Python that was used for this?
Failing to reproduce conflict is caused by: sphinx-rtd-theme 1.0.1a1 depends on docutils<0.18 - docutils 0.18 is correctly installed in the tox environment.
Circle CI build output LGTM :+1:
py310-sphinx51 installed: alabaster==0.7.12,attrs==22.1.0,Babel==2.10.3,certifi==2022.9.24,charset-normalizer==2.1.1,docutils==0.18.1,idna==3.4,imagesize==1.4.1,iniconfig==1.1.1,Jinja2==3.0.3,MarkupSafe==2.1.1,packaging==21.3,pluggy==1.0.0,py==1.11.0,Pygments==2.13.0,pyparsing==3.0.9,pytest==7.1.3,pytz==2022.4,readthedocs-sphinx-ext==2.1.9,requests==2.28.1,six==1.16.0,snowballstemmer==2.2.0,Sphinx==5.1.1,sphinx-rtd-theme @ file:///home/circleci/project/.tox/.tmp/package/1/sphinx_rtd_theme-1.0.1a1.zip,sphinxcontrib-applehelp==1.0.2,sphinxcontrib-devhelp==1.0.2,sphinxcontrib-htmlhelp==2.0.0,sphinxcontrib-httpdomain==1.8.0,sphinxcontrib-jsmath==1.0.1,sphinxcontrib-qthelp==1.0.3,sphinxcontrib-serializinghtml==1.1.5,tomli==2.0.1,urllib3==1.26.12
#1348 adds missing Sphinx versions to the test matrix, i.e. currently only Python 3.10 environment has Sphinx 5.x.
Trying to move things forwards, so I'm going to propose the following assessment and if you agree @agjohnson I think we should merge both this PR and https://github.com/readthedocs/sphinx_rtd_theme/pull/1336 (because the reasoning for allowing docutils 0.19 is the same as below)
- sphinx_rtd_theme has an upper bound on docutils because of previous fallout, see: https://blog.readthedocs.com/build-errors-docutils-0-18/
- Previous incidents following the docutils 0.18 release are now largely handled since Sphinx is ultimately responsible for upper bounds on docutils - patched versions of Sphinx was released for 1.8.x and 2.4.x series, and subsequent Sphinx release series have upper bounds on docutils.
- Going into the future, sphinx_rtd_theme might maintain an upper bound on docutils since it may affect the HTML structure that Sphinx generates and thus what the theme is compatible with. We should discuss to completely remove our docutils dependency rely on the Sphinx dependency entirely in a separate issue.
Proposed solution: We bump the upper bound for docutils, i.e. merge this PR as-is, same with https://github.com/readthedocs/sphinx_rtd_theme/pull/1336
What happens then...
The upgrade to docutils<0.19 may negatively impact deployments that have pinned some variant of sphinx<1.8.6 or Sphinx>=2.0,<2.4.5.
Example breakage: A deployment with sphinx==1.8.5 (this version has no docutils upper bound and is incompatible with docutils 0.18) and sphinx_rtd_theme (no version pinning) will get docutils 0.18 and then builds will break.
How many projects are affected?
- Unaffected: Projects created or dependency-updated after April 11, 2021 are unaffected. On this date Sphinx==3.5.4 was released w/ docutils upper bound (oddly Sphinx 3.5.4 has
docutils>=0.12,<0.17whereas Sphinx 2.4.5 hasdocutils>=0.12,<0.18). - Projects that use
sphinx>=3are theoretically unaffected since they are compatible with docutils 0.18 (according to RTD blog post) and do not need the upper bound. This means that projects created after April 5 2020 are very likely unaffected. - Projects pinning
Sphinx==1.8.xneed to bump to 1.8.6 - Projects pinning
Sphinx==2.4.xneed to bump to 2.4.5 - Projects on Sphinx 1.6.x, 2.0.x, 2.1.x, 2.2.x, 2.3.x need to implement their own
docutils<0.18
Mitigation? It's possible that we release sphinx_rtd_theme with sphinx>=1.8.6,sphinx!=2.0.*,sphinx!=2.1.*,sphinx!=2.2.*,sphinx!=2.3.*,sphinx!=2.4.0,sphinx!=2.4.1,sphinx!=2.4.2,sphinx!=2.4.3,sphinx!=2.4.4, such that affected projects mentioned above won't see the 1.1 upgrade of sphinx_rtd_theme. However, since the pip resolver was changed and had issues with endless backtracking, I'm not at all comfortable with this. It will also introduce a complex version specifier that will make our future dependency headaches worse, also for users trying to understand their own derived dependency graphs. If the fallout is bad, we can decide to include this in a patch update.
Restating the benefits of bumping docutils version: This change is necessary (i.e. critical) to introduce because several projects have failing builds because of failure to satisfy dependencies. Docutils 0.18 is soon 1 year old, so it's due time to support it.
Refs: https://github.com/sphinx-doc/sphinx/issues/9807
@twodrops
I wanted to test this branch with pip install git+https://github.com/edgarrmondragon/[email protected] but somehow the docutils seems to be still set as < docutils 0.18
Can you verify the version of Sphinx and Python that was used for this?
@benjaoming I tested it with your branch again and I cannot reproduce it anymore. Sorry for the confusion. Looking forward to the quick merge of this PR which has been blocking us for a while now.
No worries! Thanks for weighing in! Looking forwards to resolving this!
@benjaoming A couple more notes for your input:
I'm going to propose the following assessment and if you agree @agjohnson I think we should merge both this PR
In the interest of sticking to our plan for faster release cadence, I don't think we should merge these just yet. These PRs will both require bug fixes for display bugs.
I noted on #1309 that the intention was to release 1.1 as a rather minimal release -- just pinning dependencies more accurately. We'd still try to iterate quicker and release 1.2/2.0 with new features like docutils 0.18/0.19 support however.
Previous incidents following the docutils 0.18 release are now largely handled since Sphinx is ultimately responsible for upper bounds on docutils
Mostly, however our concerns are tighter than the concerns that Sphinx has. We have display issues from docutils to worry about as well, and have to support multiple versions of docutils :disappointed:
Going into the future, sphinx_rtd_theme might maintain an upper bound on docutils
Because of above, we can say fairly concretely that we'll always have docutils specifiers in both directions.
Proposed solution: We bump the upper bound for docutils, i.e. merge this PR as-is, same with https://github.com/readthedocs/sphinx_rtd_theme/pull/1336
For next release (1.2/2.0), :+1: this sounds good. However, this is going to be more like "We bump the upper bounds for docutils to 0.19 and fix all display bugs from docutils 0.18 and docutils 0.19 changes."
How many projects are affected?
This list is probably even more limited than you describe, as the project would also have to pin docutils for there to be a dependency conflict. As its a transitive dependency, this isn't common. I suppose more-so, the project would have to pin an impossible dependency as well, docutils==0.17.1 will not introduce a build failure.
Mitigation?
We can probably be rather relaxed here, as it's only projects that have docutils pinned that we really need to worry about. We also have some deprecations to enforce, including old release of Sphinx, so we can take the position that Sphinx 1.8 support is phased out enough that we aren't going to worry about supporting modern docutils and crust Sphinx combinations. That is actually the point of releases like 1.1, to give a snapshot that users can use before we start culling our dependencies.
Going to look for more references about this issue - I would like to see the discussion about why docutils uses <aside> instead of <dl>, it seems wrong.
@pradyunsg re: https://github.com/pradyunsg/furo/discussions/447#discussioncomment-2863617 I see that you ran into this (footnotes are now in <aside> instead of <dl>) and already fixed it in Furo. Do you have a quick take on your decisions here? Did you drop support for docutils 0.17 at the same time?
https://github.com/pradyunsg/furo/commit/d955850a89310a932ecab4e84cba0c20371a6a33
I tweaked the look, preserved docutils 0.17 compat, and filed https://github.com/sphinx-doc/sphinx/issues/10531.
Iβve not had bandwidth to tweak the styles in Furo since then, but it seems like itβll work just fine to use the css in docutils with a tweak.
Thanks for getting things improved with Sphinx @pradyunsg - it's really nice that the docutils 0.19 behavior got backported to Sphinx 5.1, I think we can reduce the scope of footnote DOM variants to just 2 versions.. the old <dl> and the new <aside> from docutils 0.19.
@agjohnson the visual regression is now addressed in https://github.com/readthedocs/sphinx_rtd_theme/pull/1351
Support for docutils 0.18 has been added in #1381
In release notes, we might add some clarity on old versions of Sphinx potentially breaking because of docutils 0.18, see comment in https://github.com/readthedocs/sphinx_rtd_theme/pull/1304#issuecomment-1266924478
Released in 1.2.0rc1 :+1:
In mean time docutils 0.19 has been released π
It would be good to move to that new version as well π
@kloczek - definitely, and if we can get some help with verification and testing, we might also be able to add the support quite quickly - the release notes indicate that they have changed the DOM for footnotes again => https://docutils.sourceforge.io/RELEASE-NOTES.html#release-0-19-2022-07-05
There is an issue open for docutils 0.19 support here: #1323
Thank you for the update π π