Skip to content

Tests: unpick patch for Py314a7 no longer required with Py314b2 #13606

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

Conversation

jayaddison
Copy link
Contributor

Purpose

This changeset is intended to get the Sphinx unit tests for Python 3.14 passing again in pull requests. A recent test suite failure on Py3.14b2 can be found at: https://github.com/sphinx-doc/sphinx/actions/runs/15389148337/job/43294393665

References

@jayaddison
Copy link
Contributor Author

TODO: figure out why hyperlink label IDs in ./tests/roots/test-latex-labels/index.rst change and/or why a phantomlabel is created as a result of docutils b25db649a5d073d76424e6434e12cdba53ed57e8.

A diff of manually pretty-printed LaTeX output I've been comparing is:

100c100
< \caption{labeled figure}\label{\detokenize{index:id1}}\label{\detokenize{index:figure2}}\label{\detokenize{index:figure1}}\end{figure}
---
> \caption{labeled figure}\label{\detokenize{index:id2}}\label{\detokenize{index:figure2}}\label{\detokenize{index:figure1}}\end{figure}
137c137
< \sphinxcaption{table caption}\label{\detokenize{index:id2}}\label{\detokenize{index:table2}}\label{\detokenize{index:table1}}
---
> \sphinxcaption{table caption}\label{\detokenize{index:id3}}\label{\detokenize{index:table2}}\label{\detokenize{index:table1}}
202c202
< Embedded standalone hyperlink reference: {\hyperref[\detokenize{index:section1}]{\sphinxcrossref{subsection}}}.
---
> Embedded standalone hyperlink reference: {\hyperref[\detokenize{index:section1}]{\sphinxcrossref{subsection}}}\phantomsection\label{\detokenize{index:id1}}.

@jayaddison
Copy link
Contributor Author

NB @gmilde @jfbu: there is a change-in-behaviour for LaTeX build output as a result of a recent docutils commit - it's something to do with internal docutils state about node IDs, I think; it was detected by and results in a unit test failure.

@jfbu
Copy link
Contributor

jfbu commented Jun 2, 2025

The first assertion failure is due to expected output being:

\caption{labeled figure}\label{\detokenize{index:id1}}\label{\detokenize{index:figure2}}\label{\detokenize{index:figure1}}\end{figure}

and the obtained output being with index:id2 not index:id1.

\caption{labeled figure}\label{\detokenize{index:id2}}\label{\detokenize{index:figure2}}\label{\detokenize{index:figure1}}\end{figure}

I think what happens is that the "embedded standalone hyperlink reference" one finds in output at line 196 now is accompanied with an extra \phantomsection\label{\detokenize{index:id1}}:

Embedded standalone hyperlink reference: {\hyperref[\detokenize{index:section1}]{\sphinxcrossref{subsection}}}\phantomsection\label{\detokenize{index:id1}}.

So a new reference target is created on this branch + Docutils HEAD (at b25db649a5 in a git checkout, maybe perhaps trunk@10151 on sourceforge), which offsets previously existing targets. What was formerly id1 is now id2, and what was former id2 is now id3.

@jfbu
Copy link
Contributor

jfbu commented Jun 2, 2025

Ok, I used the exact same input but to produce html. The same s/id2/id3 and s/id1/id2 is observed (at the same matching locations) so an HTML test could have complained too. However there is no HTML equivalent to the LaTeX having changed to have an extra \phantomsection\label{\detokenize{index:id1}}. I found no id1 in any of the two index.html and other.html files produced by make html. The same un-modified mark-up is produced:

<ul class="simple">
<li><p>Embedded standalone hyperlink reference: <a class="reference internal" href="#section1">subsection</a>.</p>
</li>
</ul>

@jfbu
Copy link
Contributor

jfbu commented Jun 2, 2025

The used test is with the index.rst and other.rst files which are at https://github.com/sphinx-doc/sphinx/tree/master/tests/roots/test-latex-labels.

I have not tried to produce a minimal example, beyond checking that dropping the toctree directive in index.rst referring other.rst and using only index.rst is enough to display the "problem".

@jfbu
Copy link
Contributor

jfbu commented Jun 2, 2025

Coming back only to say that the phenomenon is triggered (with Python 3.13 I forgot to say I was using) by the current HEAD of Docutils Warn about duplicate name in references with embedded internal targets. (http://svn.code.sf.net/p/docutils/code/trunk@10151) (edited to give correct link: https://sourceforge.net/p/docutils/code/10151/). Testing with its parent commit, the problem with id1->id2->id3 is not seen whether in HTML or LaTeX. And the LaTeX output does not contain the extra \phantomsection\label{\detokenize{index:id1}}. Ping @gmilde

on reading the commit message I wonder if this commit is related to my remark at #13576 (comment), so that maybe I am the one ultimately causing this branch to fail ;-)

@jayaddison
Copy link
Contributor Author

Thanks @jfbu - I also think there is a possibility that the relevant commit/SVN-rev may help enable us to resolve an issue I filed at #13383; I will begin checking/testing that possibility in a few moments.

The identifier changes apparent in Sphinx's output do seem to be a side-effect of that specific docutils git commit / SVN rev though, yep - thanks for confirming that it also affects HTML builds and v3.13 of Python.

In the example .rst file where we encounter the change-in-behaviour, I note that there are multiple aliases declared for a figure. I have a sense that what is happening is that an incremental ID is now generated for each alias -- where previously only the first would. The second ID, once generated, also updates a mapping (e.g. Python dictionary) related to the child element -- and in combination, those could explain why the set of IDs across the output change (because: there is now one additional ID created) and why the first one emitted is 2 (because: both IDs 1 and 2 relate to the figure -- and the second one is overwriting the first in that dictionary).

@jayaddison
Copy link
Contributor Author

Thanks @jfbu - I also think there is a possibility that the relevant commit/SVN-rev may help enable us to resolve an issue I filed at #13383; I will begin checking/testing that possibility in a few moments.

Nope; I think I had misdiagnosed that issue report as related to the docutils r10151 changeset.

@jayaddison
Copy link
Contributor Author

One straightforward and backwards-compatible option to resolve the docutils HEAD test failures would be to use more flexible matching for the numeric IDs in the output of the test_latex_labels test case. I'll add that as a subsequent commit on this branch.

@jayaddison
Copy link
Contributor Author

One straightforward and backwards-compatible option to resolve the docutils HEAD test failures would be to use more flexible matching for the numeric IDs in the output of the test_latex_labels test case. I'll add that as a subsequent commit on this branch.

On second thoughts: it may be better to use a separate branch and PR for that, because we squash PRs on merge. I will reference the discussion from this thread, though.

@jfbu
Copy link
Contributor

jfbu commented Jun 2, 2025

Correcting the link to the Docutils revision, as the one I copied pasted from the git message commit is broken: https://sourceforge.net/p/docutils/code/10151/

The HEAD there is now at revision 10153.

@jayaddison jayaddison marked this pull request as ready for review June 2, 2025 20:58
@AA-Turner AA-Turner merged commit 55092e7 into sphinx-doc:master Jun 2, 2025
24 of 25 checks passed
@jayaddison jayaddison deleted the pr-13527-partial-revert/unpick-py314alpha-attrdoc-patching branch June 2, 2025 21:30
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Jul 12, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants