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

Sphinx 7.2.0 problems with pathlib.Path instead of string paths #11605

Closed
tronical opened this issue Aug 17, 2023 · 7 comments · Fixed by #11619
Closed

Sphinx 7.2.0 problems with pathlib.Path instead of string paths #11605

tronical opened this issue Aug 17, 2023 · 7 comments · Fixed by #11619

Comments

@tronical
Copy link
Contributor

Describe the bug

When using breathe with the 7.2.0 release, we get this error:

      File "/Users/simon/.local/share/virtualenvs/docs-SEK-9aTg/lib/python3.11/site-packages/breathe/project.py", line 116, in __init__
        self._default_build_dir = os.path.dirname(app.doctreedir.rstrip(os.sep))
                                                  ^^^^^^^^^^^^^^^^^^^^^
    AttributeError: 'PosixPath' object has no attribute 'rstrip'
    The full traceback has been saved in /var/folders/x6/97fjm0c540s7yn9zlv7fp9k00000gn/T/sphinx-err-sbf8ny4n.log, if you want to report the issue to the developers.
    Please also report this if it was a user error, so that a better error message can be provided next time.
    A bug report can be filed in the tracker at <https://github.com/sphinx-doc/sphinx/issues>. Thanks!

I suspect that breathe relied on doctreedir being a string, and now it's a PosixPath.

If you believe that this is in error and that breathe should be changed instead to not rely on this, let me know and I'll file an issue there :)

How to Reproduce

$ echo "Test" > index.rst
$ echo "extensions = ['breathe']" > conf.py
$ sphinx-build -M html . _build
Running Sphinx v7.2.0

Exception occurred:
  File "/Users/simon/.local/share/virtualenvs/fob-HUh8Hd7H/lib/python3.11/site-packages/breathe/project.py", line 116, in __init__
    self._default_build_dir = os.path.dirname(app.doctreedir.rstrip(os.sep))
                                              ^^^^^^^^^^^^^^^^^^^^^
AttributeError: 'PosixPath' object has no attribute 'rstrip'
The full traceback has been saved in /var/folders/x6/97fjm0c540s7yn9zlv7fp9k00000gn/T/sphinx-err-0d2v5eaa.log, if you want to report the issue to the developers.
Please also report this if it was a user error, so that a better error message can be provided next time.
A bug report can be filed in the tracker at <https://github.com/sphinx-doc/sphinx/issues>. Thanks!

Environment Information

Platform:              darwin; (macOS-13.5-x86_64-i386-64bit)
Python version:        3.11.4 (main, Jun 20 2023, 16:59:59) [Clang 14.0.3 (clang-1403.0.22.14.1)])
Python implementation: CPython
Sphinx version:        7.2.0
Docutils version:      0.20.1
Jinja2 version:        3.1.2
Pygments version:      2.16.1

Sphinx extensions

['breathe']

Additional context

No response

tronical added a commit to slint-ui/slint that referenced this issue Aug 17, 2023
Pin sphinx to the previous release due to

    sphinx-doc/sphinx#11605
@picnixz
Copy link
Member

picnixz commented Aug 17, 2023

Since we changed most of the strings to use pathlib.Path whenever possible (#11526), I think Breathe should update their codebase.

Now, I agree that communication was perhaps not detailed enough. @AA-Turner maybe you should have updated https://www.sphinx-doc.org/en/master/extdev/appapi.html#sphinx.application.Sphinx.doctreedir to indicate the change of types.

Personally, I think it's reasonable to assume that an attribute representing a path may either be a string or an os.PathLike object. As such, if people want to use string methods, they need to enforce the type first.

@jmckenna
Copy link

@picnixz Have you filed this in the Breathe repo as an issue? I have found them very receptive to any reports that I have made there in the past regarding Sphinx versions support. Please report as an issue at https://github.com/breathe-doc/breathe/issues Thanks

@AA-Turner
Copy link
Member

Reported to breathe in breathe-doc/breathe#943.

A

@AA-Turner AA-Turner closed this as not planned Won't fix, can't repro, duplicate, stale Aug 17, 2023
@stephenfin
Copy link
Contributor

I think this should be reopened. This is a breaking API change that affects multiple plugins (see: https://review.opendev.org/c/openstack/openstackdocstheme/+/891678, https://github.com/click-contrib/sphinx-click/actions/runs/5893388282/job/15984823090). At a minimum, this should have pushed to a major release. Better, there should have been a translation shim and a deprecation period.

Can we revert 49d8304 and wait until 8.0 to re-merge this, please?

stephenfin added a commit to click-contrib/sphinx-click that referenced this issue Aug 17, 2023
sphinx-doc/sphinx#11605

Signed-off-by: Stephen Finucane <stephen@that.guru>
@AA-Turner
Copy link
Member

Part of the challenge is that until recently, Path couldn't be subclassed, and so creating the deprecation pass-through would be fairly challenging. The other is that you extensions should really be using os.fspath, though I appreciate that as Sphinx used string paths internally the need perhaps was not felt.

A

@AA-Turner AA-Turner changed the title Sphinx 7.2.0 breaks breathe Sphinx 7.2.0 problems with pathlib.Path instead of string paths Aug 17, 2023
@lucyleeow

This comment was marked as resolved.

@AA-Turner AA-Turner linked a pull request Aug 18, 2023 that will close this issue
cosmin added a commit to cosmin/shaka-packager that referenced this issue Aug 19, 2023
…to 7.1.2 until either of them resolve this issue
cosmin added a commit to cosmin/shaka-packager that referenced this issue Aug 21, 2023
…to 7.1.2 until either of them resolve this issue
joeyparrish pushed a commit to shaka-project/shaka-packager that referenced this issue Aug 21, 2023
Sphinx 7.2.0 breaks Breathe, see sphinx-doc/sphinx#11605 for now pin to 7.1.2 until either of them resolve this issue
@AA-Turner
Copy link
Member

I've released Sphinx 7.2.3 which should fix these issues.

A

tronical added a commit to slint-ui/slint that referenced this issue Aug 24, 2023
sphinx-doc/sphinx#11605 is fixed and Sphinx
7.2.3 should fix the issue worked around in commit d918bc2
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Sep 23, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants