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

[WIP?] C, add compatibility flag for parsing some pre-v3 input #7905

Merged
merged 4 commits into from Jul 14, 2020

Conversation

jakobandersen
Copy link
Contributor

@jakobandersen jakobandersen commented Jul 2, 2020

The C domain got much stricter in its parsing in version 3 and added additional directives and roles to provide better markup. This PR adds the configuration variable c_allow_pre_v3. When set to True the type directive and role will try to convert some previously recognized input to the v3 equivalent.

Feature or Bugfix

  • Feature

Detail

@jakobandersen
Copy link
Contributor Author

@vstinner, I believe this should take care of most cases that can not be changed to work in both v2 and v3. Let me know if there are more cases.

@vstinner
Copy link

vstinner commented Jul 2, 2020

In short, the PR works as expected, thanks! I just had to remove -W option from sphinx-build command used by Python.


I built the Python documentation with this PR.

make check SPHINXOPTS="-q -W -j4" succeed.

make suspicious SPHINXOPTS="-q -W -j4" fails with:

Doc/extending/extending.rst.rst:160:RemovedInSphinx40Warning: Pre-v3 C type role ':c:type:`PyErr_*`' converted to ':c:expr:`PyErr_*`'.
The original parsing error was:
Invalid C declaration: Expected end of definition. [error at 6]
  PyErr_*
  ------^

That's because sphinx-build is run with -W to treat warnings as errors.

Without -W, make suspicious SPHINXOPTS="-q -j4" SPHINXERRORHANDLING="" (check for suspicious syntax) and make html SPHINXOPTS="-q -j4" SPHINXERRORHANDLING="" (render as HTML) commands succeed (exit code 0), but log tons of warnings (which is acceptable for now).

sphinx/domains/c.py Outdated Show resolved Hide resolved
@jakobandersen
Copy link
Contributor Author

There should now be a c_warn_on_allowed_pre_v3 option you can set to False to disable these warnings. The behaviour is still deprecated though.
Would v4 be late enough for removal, or should we bump it to v5?

@vstinner
Copy link

vstinner commented Jul 2, 2020

FYI the issue to track the Sphinx 3 compatibility in Python is: https://bugs.python.org/issue40204

@vstinner
Copy link

vstinner commented Jul 2, 2020

There should now be a c_warn_on_allowed_pre_v3 option you can set to False to disable these warnings. The behaviour is still deprecated though. Would v4 be late enough for removal, or should we bump it to v5?

FYI in Python, we try to wait at least 2 years between a feature is deprecated and its removal. I have no idea when v4 and v5 are planned. All I know is that so far, I'm only aware of Fedora Rawhide which ugraded to Sphinx 3, and there are multiple packages broken by this upgrade:
https://bugzilla.redhat.com/showdependencytree.cgi?id=SPHINX3&hide_resolved=1

@jakobandersen
Copy link
Contributor Author

If I remember correctly the major versions are released once per year, so v4 would be in spring 2021.

@jakobandersen jakobandersen added this to the 3.2.0 milestone Jul 5, 2020
@vstinner
Copy link

vstinner commented Jul 9, 2020

What's the next step for this PEP?

@jakobandersen
Copy link
Contributor Author

Basically it can be merged (after I add docs and changelog entry), but how does it work? Are there more cases where we should try to do something?
I tried compiling the Python docs, but there were other warnings that prevented me from quickly seeing if every case I expected were handled.

@vstinner
Copy link

but how does it work?

As I wrote, this PR makes possible to build the Python documentation. For the warnings, I don't have the bandwidth to go through the huge pile of warnings. Some are real issues in the doc, some are deprecation warnings that nobody tried to address, etc.

@jakobandersen
Copy link
Contributor Author

Great, and yes, there were indeed a lot of warnings. Some still related to the C domain changes, but should be fixed on the Python side, and can be in a v2 compatible manner.
I'll get this merged when as soon as I get the docs updated.

@jakobandersen
Copy link
Contributor Author

This should be in the upcomming v3.2, and is scheduled to be removed in v5.0.

@jakobandersen jakobandersen deleted the c_compat branch July 14, 2020 11:26
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Jul 26, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
domains:c type:enhancement enhance or introduce a new feature
Projects
None yet
Development

Successfully merging this pull request may close these issues.

C domain changes of Sphinx 3 prevent to write doc compatible with Sphinx 2 and Sphinx 3
2 participants