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

[ANNOUNCEMENT] Potential Breaking Changes in Sphinx 7.2.x #11608

Closed
picnixz opened this issue Aug 17, 2023 · 14 comments
Closed

[ANNOUNCEMENT] Potential Breaking Changes in Sphinx 7.2.x #11608

picnixz opened this issue Aug 17, 2023 · 14 comments

Comments

@picnixz
Copy link
Member

picnixz commented Aug 17, 2023

Sphinx 7.2.5

Sphinx 7.2.5 has been released with fixes for all known outstanding issues

Please upgrade to Sphinx 7.2.5, and report any new failures

A


Original issue

I am opening this issue in order to track the issues related to the API changes that Sphinx 7.2 implemented. Since most of the issues are hard to hotfix, I would just recommend using an older version of Sphinx until this is fixed (probably in 7.2.1).

TocTree related API

In #11565, we decided to refactor the toctree adapter. This caused issues with pydata theme and other themes as well. Most of the issues are caused by the fact that the TocTree adapter exposed as a class contained public methods that extensions used. However, TocTree was never considered as a public API (it is not documented at all, see pydata/pydata-sphinx-theme#1404 (comment)), so we did not expect that this would lead to breaking changes (see pydata/pydata-sphinx-theme#1404 (comment)).

We are planning to something about this (maybe we could expose part of the private API).

Issues:

Javascript and CSS issues

Before, Javascript and CSS files were represented internally using classes that may also behave as strings. Many extensions relied on that and are broken (or may be broken). The string representation was originally intended for backward compatibility but way before these objects were introduced and should not have been used as such. It was now deprecated in #11582 (but we should have kept the str behaviour until removing completely the old objects).

Another breaking change is app.add_js_file (everything I say about JS files is essentially valid for CSS files). Before app.add_js_file also injected the JS file into the builder's JS files, which was a reference to ctx['script_files'] in the HTML context that extensions could play with. However, if extensions call app.add_js_file in their own html-page-context handler, the JS files will NOT be added to the builder's files, and not to ctx['script_files'] neither. So no JS file will be injected (see #11586 (comment)).

We are planning to something about this (either we revert to the old behaviour or fix the current one).

Issues:

pathlib.Path objects vs plain strings

In #11526, we changed most of the str objects that behaved like a path to pathlib.Path objects. This breaks extensions or themes relying on the fact that the object was a string.

Issues:

cc @AA-Turner @humitos

@pllim

This comment was marked as outdated.

@picnixz

This comment was marked as outdated.

Eric-Arellano added a commit to Qiskit/qiskit_sphinx_theme that referenced this issue Aug 17, 2023
This works around sphinx-doc/sphinx#11608 and
pradyunsg/furo#693.

All the repos I've seen have an unbound pin on Sphinx like `>=6.0`, but
a bound pin on `qiskit-sphinx-theme` like `~=1.14.0`. That means that
when we release this hotfix, pip will merge the constraint from their
`requirements.txt` of `>=6.0` with our theme's constraint of
`>=6.0,<7.2` to use `<7.2`.

This approach allows us to easily fix all the Ecosystem projects without
needing to open a new PR for each of them. They will simply rebuild
their docs and use the newest patch version of `qiskit-sphinx-theme`.
Eric-Arellano added a commit to Qiskit/qiskit_sphinx_theme that referenced this issue Aug 17, 2023
This works around sphinx-doc/sphinx#11608 and
pradyunsg/furo#693.

All the repos I've seen have an unbound pin on Sphinx like `>=6.0`, but
a bound pin on `qiskit-sphinx-theme` like `~=1.14.0`. That means that
when we release this hotfix, pip will merge the constraint from their
`requirements.txt` of `>=6.0` with our theme's constraint of
`>=6.0,<7.2` to use `<7.2`.

This approach allows us to easily fix all the Ecosystem projects without
needing to open a new PR for each of them. They will simply rebuild
their docs and use the newest patch version of `qiskit-sphinx-theme`.
Eric-Arellano added a commit to Qiskit/qiskit_sphinx_theme that referenced this issue Aug 17, 2023
This works around sphinx-doc/sphinx#11608 and
pradyunsg/furo#693.

All the repos I've seen have an unbound pin on Sphinx like `>=6.0`, but
a bound pin on `qiskit-sphinx-theme` like `~=1.14.0`. That means that
when we release this hotfix, pip will merge the constraint from their
`requirements.txt` of `>=6.0` with our theme's constraint of
`>=6.0,<7.2` to use `<7.2`.

This approach allows us to easily fix all the Ecosystem projects without
needing to open a new PR for each of them. They will simply rebuild
their docs and use the newest patch version of `qiskit-sphinx-theme`.
Eric-Arellano added a commit to Qiskit/qiskit_sphinx_theme that referenced this issue Aug 17, 2023
This works around sphinx-doc/sphinx#11608 and
pradyunsg/furo#693.

All the repos I've seen have an unbound pin on Sphinx like `>=6.0`, but
a bound pin on `qiskit-sphinx-theme` like `~=1.14.0`. That means that
when we release this hotfix, pip will merge the constraint from their
`requirements.txt` of `>=6.0` with our theme's constraint of
`>=6.0,<7.2` to use `<7.2`.

This approach allows us to easily fix all the Ecosystem projects without
needing to open a new PR for each of them. They will simply rebuild
their docs and use the newest patch version of `qiskit-sphinx-theme`.
stefmolin added a commit to stefmolin/data-morph that referenced this issue Aug 17, 2023
@AA-Turner AA-Turner changed the title (READ THIS) Breaking Changes in Sphinx 7.2. [ANNOUNCEMENT] Potential Breaking Changes in Sphinx 7.2.0 Aug 17, 2023
@picnixz

This comment was marked as off-topic.

@AA-Turner AA-Turner pinned this issue Aug 17, 2023
donn added a commit to efabless/openlane2 that referenced this issue Aug 17, 2023
Sphinx 7.2 has a number of breaking changes: sphinx-doc/sphinx#11608

We've been running into one with `furo` specifically, where it thinks we're not using the `furo` CSS.
@stephenfin

This comment was marked as duplicate.

@kevalmorabia97

This comment was marked as resolved.

@AA-Turner
Copy link
Member

Sphinx 7.2.2 has been released, which contains fixes for Furo, and three other bugs.

A

@picnixz
Copy link
Member Author

picnixz commented Aug 18, 2023

@AA-Turner thank you very much ! (maybe you could edit the announcement, since it appears I cannot re-edit it?)

@oddbookworm

This comment was marked as off-topic.

@AA-Turner

This comment was marked as off-topic.

@pradyunsg
Copy link
Contributor

Adding 8e730ae to the list here -- it changed the behavior such that data-content_root needs to be set (it moved from a script without "use strict"; to a script that contains it).

Before:

document.getElementById("documentation_options").getAttribute('data-url_root')

After:

document.documentElement.dataset.content_root

It shouldn't affect themes that use base.html as-is from Sphinx; which... well, that's most theme. Furo doesn't and I'll be fixing that in a few minutes.

PS: Even though this release has been slightly disruptive, I do want to note that I appreciate the work that Sphinx's maintainers (notably @AA-Turner lately) have been doing! ^.^

pradyunsg added a commit to pradyunsg/furo that referenced this issue Aug 18, 2023
Search with dirhtml builds are broken when using Furo with Sphinx
7.2.{0,1,2}.

More details can be found in
sphinx-doc/sphinx#11608 (comment)
bearsh added a commit to bearsh/git-cola that referenced this issue Aug 22, 2023
so convert it to a string, the trailing os.sep is not needed.
os.path.join which is used to join paths, expects string and inserts the
os.sep.

See-also: sphinx-doc/sphinx#11526
See-also: sphinx-doc/sphinx#11608

Signed-off-by: Martin Gysel <[email protected]>
njzjz added a commit to njzjz/deepmodeling_sphinx that referenced this issue Aug 22, 2023
njzjz added a commit to deepmodeling/deepmodeling_sphinx that referenced this issue Aug 22, 2023
davvid added a commit to davvid/sphinx-to-github that referenced this issue Aug 23, 2023
Newer versions of sphinx use a pathlib.Path object instead
of a string for the "root" parameter. Add compatibility
by stringifying the value provided by Sphinx.

See-also: sphinx-doc/sphinx#11526
See-also: sphinx-doc/sphinx#11608
See-also: git-cola/git-cola#1336
Original-patch-by: Martin Gysel <[email protected]>
davvid added a commit to davvid/sphinx-to-github that referenced this issue Aug 23, 2023
Newer versions of sphinx use a pathlib.Path object instead
of a string for the "root" parameter. Add compatibility
by stringifying the value provided by Sphinx.

See-also: sphinx-doc/sphinx#11526
See-also: sphinx-doc/sphinx#11608
See-also: git-cola/git-cola#1336
Original-patch-by: Martin Gysel <[email protected]>
@AA-Turner
Copy link
Member

Sphinx 7.2.3 has been released.

A

davvid pushed a commit to davvid/git-cola that referenced this issue Aug 24, 2023
so convert it to a string, the trailing os.sep is not needed.
os.path.join which is used to join paths, expects string and inserts the
os.sep.

See-also: sphinx-doc/sphinx#11526
See-also: sphinx-doc/sphinx#11608

Signed-off-by: Martin Gysel <[email protected]>
(cherry picked from commit 1293e18)
@AA-Turner
Copy link
Member

Sphinx 7.2.4 has been released.

A

@AA-Turner
Copy link
Member

Sphinx 7.2.5 has been released.

A

@picnixz picnixz changed the title [ANNOUNCEMENT] Potential Breaking Changes in Sphinx 7.2.0 [ANNOUNCEMENT] Potential Breaking Changes in Sphinx 7.2.x Sep 9, 2023
@AA-Turner AA-Turner unpinned this issue Sep 13, 2023
@picnixz
Copy link
Member Author

picnixz commented Sep 21, 2023

With Sphinx 7.2.6 being released, I think this issue can be closed. Anyone is free to reopen if they think that a change introduced between 7.2.0 and 7.2.5 is breaking.

@picnixz picnixz closed this as completed Sep 21, 2023
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Oct 22, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

7 participants