Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Release testing with other documentation libraries: pinning Docutils #9088

Closed
felix-hilden opened this issue Apr 11, 2021 · 37 comments
Closed
Labels
type:enhancement enhance or introduce a new feature

Comments

@felix-hilden
Copy link

From #9056. There was discussion about pinning the version of Docutils due to recent problems with the 0.17 release. Eric Holscher proposed that new versions of Docutils would not automatically be accepted by Sphinx, but instead the versions would be tested by more experienced users. Issues would be fixed and then the new version would be added in the next Sphinx release.

Takeshi Komiya tended to agree but was worried about the added trouble it would cause to the already busy maintainers of Sphinx, and about determining who's responsibility it is to perform the tests and accept the versions. Crafting those tests would be hard. Still, many participants were willing to help.

The Sphinx RTD Theme and other libraries now already require docutils<0.17. However, Markus Heiser thought that if libraries should pin Docutils instead of Sphinx providing that, it should be documented in Sphinx's user guide. And as he put it: it's fine for individual libraries, but for the larger user base it doesn't address the potential of surprising errors they would encounter without pinning. It could be best to handle the problems at Sphinx's level.

Lastly, it seems that Takeshi agreed that pinning is the way to go. But a workflow should be established.

I tried my best to condense the discussion to these points. Is there anything I missed? If I may, I think everybody has raised good points and valid concerns. Hopefully the discussion will continue!

@tk0miya I really appreciate your perseverance and want to reach solutions despite the language barrier, as I'm sure many others do as well! Thank you for that.

@felix-hilden felix-hilden added the type:enhancement enhance or introduce a new feature label Apr 11, 2021
@tk0miya
Copy link
Member

tk0miya commented Apr 12, 2021

@felix-hilden Thank you for the summary.

Issues would be fixed and then the new version would be added in the next Sphinx release.

Yesterday, Sphinx-3.5.4 was released. It depends on docutils < 0.17. And the versioning pin has been merged into the 4.x branch. So Sphinx-4.0.0b1 depends on docutils < 0.17, too.

Lastly, it seems that Takeshi agreed that pinning is the way to go.

Yes, I surely agreed. Because many people will help testing!

@return42
Copy link

I tried my best to condense the discussion to these points. Is there anything I missed?

Thank you for the excellent summary!

First I want to gather the objects to test. In #9056 we talked about HTML (writer and) themes, I think we should also cover man pages and PDF (LateTex). . what do other suggest? Should we also cover some popular extensions? Personally I also like to have some tests of the c-domain (@jakobandersen).

@tk0miya
Copy link
Member

tk0miya commented Apr 12, 2021

Sphinx is built on docutils framework. So all features are related to docutils in any kind of way.

From the perspective of the (reST) parser components, almost Sphinx depends on docutils. Many kinds of extensions and domains provides directives and roles to enhance its syntax. And it modifies the intermediate document (a.k.a. doctree) via transform components. So we need to check whole of Sphinx features. But I believe testing scripts and GitHub CI helps this case (But we can't do tests with the latest release, because it's pinned!).

From the perspective of the writer components, Sphinx depends on the HTML4, HTML5, LaTeX, manpage, and XML writers of docutils. In our testing scripts does not check how they will be rendered in the viewer. So we can't find the CSS problems automatically. So this is an important point for the manual testing.

@ericholscher
Copy link
Contributor

ericholscher commented Apr 12, 2021

Agreed here. I think we can prioritize things in the order of what to focus on:

  • Highest usage
  • Most likely to break

At least on my side, the HTML output is the most important. It has the most usage, and also the most customization with users. In my experience, most people lightly theme PDF or Epub output, but HTML is often highly customized.

I don't have a great feeling for "Most likely to break", but perhaps we can think of some ways to consider this. I imagine it will map pretty closely to "Most complex code" and also "Code we maintain". So hopefully the docutils or pygments team will notice if their stuff breaks, but if we're extending or changing things, that is more important for us to test.

Proposed plan

The RTD theme is what the RTD team has most familiarity with, so that's a place that would be most obvious for us to focus on to start. I think we probably want something along the lines of:

  • An established test suite showing all the various HTML elements (This already exists)
  • A process for building and hosting these pages built against the pre-released version of Sphinx (4.0 at this point presumably)
    • I don't think we can currently use RTD for this, but it wouldn't be hard to set it up. We can either do this on RTD or something like GH pages.
  • A process & timeline for humans to do manual visual testing. Something like One week before release we will build the test docs and send a link to users on sphinx-dev & post a comment in the release plan ticket on GH

Next steps

This seems like the "minimum viable testing plan" (MVP). I can see a few obvious places to expand it:

  • A suite of automated tests that alert us when HTML has changed between releases. This would let us know where to focus our manual attention when reviewing the outputted HTML.
  • Expanding the set of themes, extensions, and output formats to test against (as noted above)
    • Additional HTML themes should be pretty simple
    • PDF/Epub/etc. output would be a bit harder, but not terrible

Ownership

I think we'd want to assign ownership of the various testing areas to different people. For example, I can commit the RTD team to doing the testing against our theme, and likely the RTD build process as we expand the scope of this work.

It sounds like @return42 is interested in a different subset of functionality, so we should figure out your priority list, and see how we can integrate it into a similar testing process, so we only have 1 place for this.

I also imagine the Executable Books folks would be engaged in helping with this (Myst-Parser & pydata theme) would be other obvious places to test. /cc @chrisjsewell @choldgraf

Short-term steps

I'd like to see if we can get an initial version of this in time for the Sphinx 4.0 release, but that might be a bit optimistic given that it's coming up in 2 weeks. I will see what our availability is for the MVP. It wouldn't be a ton of work to do manually (build files locally, upload somewhere), so it should be doable I think.

I think the idea should be to do a small amount of work to see how it goes, then adjust the process as we learn more.

@pradyunsg
Copy link
Contributor

Would we benefit from automated tests, to make sure that the "context" provided to themes is consistent? In other words, to ensure that the variables going "into" the theme for Jinja2 rendering are consistent/constant.

I feel like that's realistically the main contract here, and having an established test suite showing all the various HTML elements would be really good to have.

Automated tests can help make it clear what HTML output changed between releases, which can help all downstream themes ensure that things look the exact same.

@humitos
Copy link
Contributor

humitos commented Apr 14, 2021

A process for building and hosting these pages built against the pre-released version of Sphinx (4.0 at this point presumably)
I don't think we can currently use RTD for this, but it wouldn't be hard to set it up

@ericholscher If you refer to not being able to install a pre-release on Read the Docs, I think this is possible by using a "hidden feature" of pip:

If a Requirement specifier includes a pre-release or development version (e.g. >=0.0.dev0) then pip will allow pre-release and development versions for that requirement. This does not include the != flag.

From https://pip.pypa.io/en/latest/cli/pip_install/#pre-release-versions

Example,

▶ pip install sphinx
Collecting sphinx
  Downloading Sphinx-3.5.4-py3-none-any.whl
...

▶ cat --plain requirements.txt
Sphinx>=4.0.dev0

▶ pip install -r requirements.txt
Collecting Sphinx>=4.0.dev0
  Downloading Sphinx-4.0.0b1-py3-none-any.whl (2.8 MB)
...

Edit: Example working on Read the Docs: https://sphinx-rtd-theme--1130.org.readthedocs.build/en/1130/demo/demo.html

In fact, I just found a small visual difference in how the math formulas are rendered.

@return42
Copy link

It sounds like @return42 is interested in a different subset of functionality, so we should figure out your priority list, and see how we can integrate it into a similar testing process, so we only have 1 place for this.

I'm contribute to a couple of projects and compared to RTD, the documentation build has not the same value / Sphinx is just one tool in a wider tool chain and not one of the core components of the enterprise. Most often it is only noticed when it generates some issues / Its sad, but is common in how projects are often managed. ... at least my experience. ;-)

@ericholscher The parts I want to cover are changing and some of them are exotic not only to RTD. For some of this task I need precompile steps which are also unusual for common sphinx projects. I don't want to burden this on a test suite we are talking here.

For me it will be perfect if we have a player like RTD who has the hat on and leads us through common use cases. For the more exotic things, I can build up my own test suites and see what is more general and has a value to the (RTD) test suite we are talking here.

I think we probably want something along the lines of:

  • An established test suite showing all the various HTML elements (This already exists)

How do you want to implement such a test suite practically? I think the repository https://github.com/readthedocs/sphinx_rtd_theme has some valuable documents for a test suite in its /doc tree, but I suppose we need a separated repository for such a suite, especially if we want to add other content to test.

We can either do this on RTD or something like GH pages.

I am looking for a test suite that has the ability to run tests locally and is not bind to dedicated hoster.

All the CI stuff (and may be later some kind of zero builds) can be done on top by the build chain, which is decoupled from local builds. I name this, because I can't estimate what are the limits when we bind the test suite to the RTD workflows (sorry I do not have experience with the workflow of RTD).

One challenge of such a test suite I think is to build up a test matrix which scales good over sphinx-themes, sphinx-writers and other components which might vary. The test matrix should work without the dependency to CI workflows of a dedicated hoster.

@tk0miya
Copy link
Member

tk0miya commented Apr 17, 2021

Is there any chance to unpin docutils-0.17 on Sphinx-4.0.0? Or we have to wait for Sphinx-5.0?

@astrojuanlu
Copy link
Contributor

FYI:

release 0.17.1 is on pypi

fixes two things

  • in python3.6 in an ascii environment reading the docutils.sty file
    fails, due to unicode characters in the file.
  • provide defaults (fallbacks) for settings (tab_width) if docutils is
    used by third party frontends.

@gmilde
Copy link

gmilde commented Apr 19, 2021

Is there any chance to unpin docutils-0.17 on Sphinx-4.0.0? Or we have to wait for Sphinx-5.0?

IMO, with the release of Docutils 0.17.1, docutils < 0.18 can/should be allowed (with a warning about the changes to HTML output in 0.17+).

@ericholscher
Copy link
Contributor

IMO, with the release of Docutils 0.17.1, docutils < 0.18 can/should be allowed (with a warning about the changes to HTML output in 0.17+).

I think the main question here is whether the sphinx basic theme and alabaster work well with the latest HTML. I tend to be more sympathetic to breaking changes in major versions (eg. 4.0) -- but we should be sure that Sphinx itself isn't broken with this release.

If we can confirm that Sphinx and alabaster are working well with 0.17, I'd be 👍 on pinning to docutils<0.18. We should add it to the test that @humitos did above and confirm this week.

Is there a test page with alabaster that shows all the elements similar to the RTD page? We could probably create one in a test repo for testing this, if there isn't (/cc @bitprophet)

@tk0miya
Copy link
Member

tk0miya commented Apr 19, 2021

Is there a test page with alabaster that shows all the elements similar to the RTD page?

We don't have it.

@pradyunsg
Copy link
Contributor

pradyunsg commented Apr 19, 2021

We don't have it.

We do actually! https://sphinx-themes.org exists!

I still have to trim the themes there, to make it actually usable for people "shopping" for themes (removing things with company logos and whatnot), but it's certainly usable today for this.

To that end, I had an idea for how to get what we want here quickly: sphinx-themes/sphinx-themes.org#70

Manual testing can be done based on the output of those builds, which is served on https://sphinx-themes.org. :)

@ericholscher
Copy link
Contributor

ericholscher commented Apr 19, 2021

@pradyunsg This is awesome! Thanks for that. I will give that issue a read today.

I think the main addition is that we need to either manually install the latest docutils, or raise the pin in the pre-released version.

@chrisjsewell
Copy link
Member

I also imagine the Executable Books folks would be engaged in helping with this (Myst-Parser & pydata theme) would be other obvious places to test.

FYI yep happy to help if needed, let me know if there is anything specific we can do

@ericholscher
Copy link
Contributor

@pradyunsg I looked a bit more, and I think my comment about latest docutils is mostly the main thing. Is it possible to get:

  • A version of alabaster rendered with the latest released sphinx
  • A version rendered with sphinx 4 & docutils 0.17

Then we can do a manual inspection of obvious things that might be broken.

@tk0miya
Copy link
Member

tk0miya commented Apr 21, 2021

I think the main addition is that we need to either manually install the latest docutils, or raise the pin in the pre-released version.

AFAIK, you need to modify the dependency list of Sphinx in setup.py before installing. If not, Sphinx will not be able to run. I'll post a PR for that. Please try it.

@tk0miya
Copy link
Member

tk0miya commented Apr 22, 2021

In this weekend, I'd like to release Sphinx-4.0 final as scheduled. But it seems the testing with docutils-0.17 has not been finished, right? Will it be finished if we'll postpone the release a few days? If so, I'll postpone it. If not, I'll go releasing and postpone supporting the new docutils to Sphinx-4.1.

@ericholscher
Copy link
Contributor

ericholscher commented Apr 23, 2021

@tk0miya I'd say go ahead and release 4.0 with the latest docutils, if that's what you're hoping to do. A lot of things should be pinned from the latest release, and I don't want to block the release on work we haven't figured out yet. We can get things better prepared for the next release, hopefully.

@tk0miya
Copy link
Member

tk0miya commented Apr 24, 2021

@ericholscher Thank you for your comment. But I'd not like to break our agreement in #9056. Let's confirm Sphinx surely works well with docutils-0.17.x until the next release.

@gmilde
Copy link

gmilde commented Apr 24, 2021 via email

@tk0miya
Copy link
Member

tk0miya commented Apr 24, 2021

I don't understand what we should do. Somebody requests pinning, and somebody requests unpinning. Please let me know what we should do. Anyway, I'll stop working for a while. Bye.

@ericholscher
Copy link
Contributor

ericholscher commented Apr 24, 2021

@tk0miya @gmilde I think we all agree that Sphinx 4.0 should support the latest version of docutils if possible. The current issue is:

We haven't built a good way to test that Sphinx 4.0 themes (including alabaster, the default theme) don't break on docutils 0.17

This is the current problem we're trying to solve. We don't want to release Sphinx 4.0 that is broken, and we're trying to design the system for testing it.

So far:

  • We have the work @humitos did on Sphinx 4.0b, docutils<0.17, and sphinx_rtd_theme
  • We have the work @pradyunsg did on Sphinx 4.0b, docutils<0.17, and alabaster (and other themes?)

My suggestion would be:

  • Let's do a beta release of Sphinx 4 (sphinx 4.0b2) with a docutils<0.18 pin
  • Wait 1 additional week to release Sphinx 4.0, so the tests above can be run with the latest docutils, and users can try it out as well by pinning in requirements.txt

This should let us do more testing with the latest docutils & sphinx, and only delay the release a week. We can also try and share this a bit on twitter to get more user-testing, and see if that has any meaningful results.

We can also test it in some of our public-facing docs (https://docs.readthedocs.io/en/stable/) -- and perhaps Sphinx should also build its docs (https://www.sphinx-doc.org/en/master/) with the sphinx==4.0b2, docutils==0.17 pin to make sure we find any obvious issues by using it ourselves prior to release?

@ericholscher
Copy link
Contributor

ericholscher commented Apr 24, 2021

Updated:

I tried to build the primary RTD docs against it but we hit a conflict with sphinx-tabs:

The conflict is caused by:
    The user requested sphinx==4.0b1
    recommonmark 0.7.1 depends on sphinx>=1.3.1
    sphinx-autobuild 2021.3.14 depends on sphinx
    sphinx-intl 2.0.1 depends on sphinx
    sphinx-prompt 1.4.0 depends on Sphinx
    sphinx-rtd-theme 0.5.2 depends on sphinx
    sphinx-tabs 2.1.0 depends on sphinx<4 and >=2

Will try and figure that out later, but it shows the complexity of pinning things. Probably good that it's pinned though, since we aren't sure if it supports Sphinx 4 or not!

@ericholscher
Copy link
Contributor

Just poking around, it seems on the RTD theme at least bulleted lists are broken:

They seem fine on the Sphinx theme though:

Not sure if that's a docutils issue or not though:

Anyway -- I think a bit more looking around, some diff'ing between versions, and getting more users to install Sphinx 4 & docutils 0.17 is probably a good goal for the next week.

@mitya57
Copy link
Contributor

mitya57 commented Apr 24, 2021

My 2¢: if Sphinx' built-in themes and the default theme (Alabaster) work fine with docutils 0.17.1, then that version should be allowed. I don't think we should stick to the old version just because of sphinx-rtd-theme, because it's a third-party theme.

@ericholscher
Copy link
Contributor

ericholscher commented Apr 24, 2021

Agreed. The RTD theme is already pinned to docutils <0.17, so it shouldn't effect us. I'm mostly just using it as a test case to see what could be broken, since we have a pretty large test suite of elements.

@return42
Copy link

The RTD theme is already pinned to docutils <0.17,

Same here, I also pinned to docutils <0.17 in my projects. BTW: sorry for my absent, ATM I have timelines in other projects .. I'm looking forward to assemble up a suite with mine use cases one day .. hopefully soon :-)

Thanks! .. to all here for your efforts in making sphinx-doc more stable.

@pradyunsg
Copy link
Contributor

we hit a conflict with sphinx-tabs:

Yay actually functional dependency resolver!

@Daltz333
Copy link

Daltz333 commented Apr 24, 2021

@pradyunsg @ericholscher one of the maintainers of sphinx-tabs here. We are just waiting on a release to bump the version, to not have to pin testing on a beta version of sphinx.

See executablebooks/sphinx-tabs#110

@astrojuanlu
Copy link
Contributor

Our tests with Sphinx 4.0, alabaster, and docutils 0.17 were positive: #9056 (comment)

@tk0miya
Copy link
Member

tk0miya commented Apr 29, 2021

I'm back now. Sorry for my absence. I'm perfectly recovered.

@astrojuanlu @humitos @ericholscher Thank you for testing! Good to hear. I adopted @ericholscher 's comment in #9088 (comment). So I'll release 4.0.0b2 from now on.

@tk0miya
Copy link
Member

tk0miya commented May 3, 2021

Just FYI: I noticed jinja2 and markupsafe will release maror version soon. So I'll pin their versions on 4.0.0, and will unpin it on 4.1.0.
#9161

yuhan added a commit to dagster-io/dagster that referenced this issue May 7, 2021
Summary:
sphinx 4.0 release got delayed bc of sphinx-doc/sphinx#9088
maybe we should spend some time upgrading it to 4.0 once it's released and resolve deps on our end all at once (e.g. pin whats needed) -- (may 8th according to sphinx-doc/sphinx#9056)

Test Plan: manually re-indexed search so api doc shows up

Reviewers: sashank

Reviewed By: sashank

Differential Revision: https://dagster.phacility.com/D7785
return42 added a commit to return42/searxng that referenced this issue May 14, 2021
@return42
Copy link

Thanks to all who helped & tested .. just updated to 4.0 and unpinned docutils in my project .. without any flaw!

Thanks a lot ..can't say often enough :)

return42 added a commit to searxng/searxng that referenced this issue May 15, 2021
…ja2-3.0.0"

This reverts commit 9e9a1ac, reversing
changes made to 5b1f04f.

We can't upgrade to jinja2==3.0.0 until Sphinx 4.1.0 has been released [1][2]

ERROR: Cannot install -r /share/searx/requirements-dev.txt (line 11) and jinja2==3.0.0 because these package versions have conflicting dependencies.

The conflict is caused by:
    The user requested jinja2==3.0.0
    sphinx 4.0.1 depends on Jinja2<3.0 and >=2.3

[1] sphinx-doc/sphinx#9088 (comment)
[2] sphinx-doc/sphinx#9161

Signed-off-by: Markus Heiser <markus.heiser@darmarit.de>
@astrojuanlu
Copy link
Contributor

I see that @tk0miya is adding a cron job to test sphinx against the HEAD of docutils at #9702 🎉 and some dependencies are pinning the docutils version, including Sphinx itself. What is left to be done here?

@gmilde
Copy link

gmilde commented Nov 18, 2021

The difficulties after the 0.18 release showed, that the current test strategy is still insufficient. An unexpected high number of users still depend on Sphinx 1.8.5, as this was the default pin on RTD for new projects until October 2020. Also many users pinning Sphinx for "reproducible builds" fail to pin the implicit dependency Docutils.
How can we improve?

  • The new backport releases for the Sphinx 1.x and 2.x series help users that pin to Sphinx < 1.2 and similar but not with Sphinx == x.x.x
  • The upcoming bugfix release Docutils 0.18.1 avoids the errors with Sphinx 1.8.5 and later (HTML styling problems may remain).

But we also need a better test strategy to avoid repetition with the next Docutils release:

  • Sphinx already tests its development version with Docutils HEAD.
  • As long as there are a considerable amount of RTD installations pinned to Sphinx 1.8.5 but without Docutils pin, RTD should test this old default installation against Docutils beta releases to be aware of problems before they hit again with the next full release.

All problems should be reported to Docutils, either via mail to Docutils-develop@lists.sourceforge.net or opening a ticket at https://sourceforge.net/p/docutils/bugs/.
Deprecation warnings should be worked around or reported for up-to-date Sphinx. If there is no way to avoid a warning, we may turn it in a PendingDeprecationWarning for some time.
We want to keep Docutils compatible to Sphinx and downstream users at until version 1.2, but we need your input.

For the future, it is also important, that users know about the implicit dependency of Sphinx releases from before May 2021 on Docutils < 0.17. This should be stated somewhere in the documentation, e.g. on the reproducible builds page via the additon:

✅ Good:

  # File: docs/environment.yaml

  name: docs
  channels:
    - conda-forge
    - defaults
  dependencies:
    - sphinx==4.2.0
    - docutils=0.17 # not required for Sphinx releases after May 2021
    - nbsphinx==0.8.1

@astrojuanlu
Copy link
Contributor

As long as there are a considerable amount of RTD installations pinned to Sphinx 1.8.5 but without Docutils pin, RTD should test this old default installation against Docutils beta releases to be aware of problems before they hit again with the next full release.

Noted @gmilde , thanks. Will address this these days.

@AA-Turner AA-Turner added this to the some future version milestone Sep 29, 2022
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Sep 12, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
type:enhancement enhance or introduce a new feature
Projects
None yet
Development

No branches or pull requests