From ec51d9a5c3199656c068c27678129b1ffed987a9 Mon Sep 17 00:00:00 2001 From: khanxmetu Date: Sun, 15 Sep 2024 22:48:11 +0300 Subject: [PATCH 1/6] Ensure deterministic resolution of toctree --- sphinx/builders/__init__.py | 3 +++ 1 file changed, 3 insertions(+) diff --git a/sphinx/builders/__init__.py b/sphinx/builders/__init__.py index 76e7e230cdc..b3e5d559124 100644 --- a/sphinx/builders/__init__.py +++ b/sphinx/builders/__init__.py @@ -613,6 +613,9 @@ def write( docnames.add(tocdocname) docnames.add(self.config.root_doc) + # sort to ensure deterministic toctree generation + self.env.toctree_includes = dict(sorted(self.env.toctree_includes.items())) + with progress_message(__('preparing documents')): self.prepare_writing(docnames) From 909b1616b0c495e9a7ce42f2a233d7683b0647b9 Mon Sep 17 00:00:00 2001 From: khanxmetu Date: Thu, 19 Sep 2024 14:19:24 +0300 Subject: [PATCH 2/6] Add tests and update CHANGES --- CHANGES.rst | 4 ++++ .../test-toctree-multiple-parents/bar.rst | 6 +++++ .../test-toctree-multiple-parents/baz.rst | 6 +++++ .../test-toctree-multiple-parents/conf.py | 0 .../test-toctree-multiple-parents/foo.rst | 6 +++++ .../test-toctree-multiple-parents/index.rst | 7 ++++++ .../test-toctree-multiple-parents/qux.rst | 3 +++ .../relation_graph.txt | 7 ++++++ .../test_builders/test_build_html_toctree.py | 22 +++++++++++++++++++ 9 files changed, 61 insertions(+) create mode 100644 tests/roots/test-toctree-multiple-parents/bar.rst create mode 100644 tests/roots/test-toctree-multiple-parents/baz.rst create mode 100644 tests/roots/test-toctree-multiple-parents/conf.py create mode 100644 tests/roots/test-toctree-multiple-parents/foo.rst create mode 100644 tests/roots/test-toctree-multiple-parents/index.rst create mode 100644 tests/roots/test-toctree-multiple-parents/qux.rst create mode 100644 tests/roots/test-toctree-multiple-parents/relation_graph.txt diff --git a/CHANGES.rst b/CHANGES.rst index 1104eb529b6..4f4ea66e2f9 100644 --- a/CHANGES.rst +++ b/CHANGES.rst @@ -99,6 +99,10 @@ Bugs fixed * #12796: Enable parallel reading if requested, even if there are fewer than 6 documents. Patch by Matthias Geier. +* #12888: Ensure deterministic resolution of global toctree in parallel builds + when document is included in multiple toctrees by choosing lexicographically + greatest parent document. + Patch by A. Rafey Khan Testing diff --git a/tests/roots/test-toctree-multiple-parents/bar.rst b/tests/roots/test-toctree-multiple-parents/bar.rst new file mode 100644 index 00000000000..7a283eb86dc --- /dev/null +++ b/tests/roots/test-toctree-multiple-parents/bar.rst @@ -0,0 +1,6 @@ +Bar +=== +.. literalinclude:: relation_graph.txt + +.. toctree:: + qux diff --git a/tests/roots/test-toctree-multiple-parents/baz.rst b/tests/roots/test-toctree-multiple-parents/baz.rst new file mode 100644 index 00000000000..adbb3623325 --- /dev/null +++ b/tests/roots/test-toctree-multiple-parents/baz.rst @@ -0,0 +1,6 @@ +Baz +=== +.. literalinclude:: relation_graph.txt + +.. toctree:: + qux diff --git a/tests/roots/test-toctree-multiple-parents/conf.py b/tests/roots/test-toctree-multiple-parents/conf.py new file mode 100644 index 00000000000..e69de29bb2d diff --git a/tests/roots/test-toctree-multiple-parents/foo.rst b/tests/roots/test-toctree-multiple-parents/foo.rst new file mode 100644 index 00000000000..6218eb13878 --- /dev/null +++ b/tests/roots/test-toctree-multiple-parents/foo.rst @@ -0,0 +1,6 @@ +Foo +=== +.. literalinclude:: relation_graph.txt + +.. toctree:: + bar diff --git a/tests/roots/test-toctree-multiple-parents/index.rst b/tests/roots/test-toctree-multiple-parents/index.rst new file mode 100644 index 00000000000..672b06228c3 --- /dev/null +++ b/tests/roots/test-toctree-multiple-parents/index.rst @@ -0,0 +1,7 @@ +test-toctree-multiple-parents +============================= +.. literalinclude:: relation_graph.txt + +.. toctree:: + foo + baz diff --git a/tests/roots/test-toctree-multiple-parents/qux.rst b/tests/roots/test-toctree-multiple-parents/qux.rst new file mode 100644 index 00000000000..58e2a984e09 --- /dev/null +++ b/tests/roots/test-toctree-multiple-parents/qux.rst @@ -0,0 +1,3 @@ +Qux +=== +.. literalinclude:: relation_graph.txt diff --git a/tests/roots/test-toctree-multiple-parents/relation_graph.txt b/tests/roots/test-toctree-multiple-parents/relation_graph.txt new file mode 100644 index 00000000000..9cd43438dc7 --- /dev/null +++ b/tests/roots/test-toctree-multiple-parents/relation_graph.txt @@ -0,0 +1,7 @@ + index + / \ + foo baz + \ / + bar / + \ / + qux \ No newline at end of file diff --git a/tests/test_builders/test_build_html_toctree.py b/tests/test_builders/test_build_html_toctree.py index 7b9e5a49d13..59a4d794e3a 100644 --- a/tests/test_builders/test_build_html_toctree.py +++ b/tests/test_builders/test_build_html_toctree.py @@ -3,6 +3,7 @@ import re import pytest +from unittest.mock import patch from tests.test_builders.xpath_html_util import _intradocument_hyperlink_check from tests.test_builders.xpath_util import check_xpath @@ -63,3 +64,24 @@ def test_numbered_toctree(app): def test_singlehtml_hyperlinks(app, cached_etree_parse, expect): app.build() check_xpath(cached_etree_parse(app.outdir / 'index.html'), 'index.html', *expect) + + +@pytest.mark.sphinx( + 'html', + testroot='toctree-multiple-parents', + confoverrides={'html_theme': 'alabaster'}, +) +def test_toctree_multiple_parents(app, cached_etree_parse): + # Lexicographically greatest parent of the document in global toctree + # should be chosen regardless of the order in which files are read + with patch.object(app.builder, '_read_serial') as mock_read_serial: + # Read files in reversed order + _read_serial = app.builder.__class__._read_serial + mock_read_serial.side_effect = lambda docnames: _read_serial( + app.builder, list(reversed(docnames)) + ) + app.build() + # Check if qux is a child of baz in qux.html + xpath_baz_children = ".//ul[@class='current']//a[@href='baz.html']/../ul/li//a" + etree = cached_etree_parse(app.outdir / 'qux.html') + check_xpath(etree, 'qux.html', xpath=xpath_baz_children, check='Qux') From 1b08b66d16292b6fe200eaf6f45d3bd564a6d18b Mon Sep 17 00:00:00 2001 From: khanxmetu Date: Thu, 19 Sep 2024 15:45:42 +0300 Subject: [PATCH 3/6] Add warning for multiple toc parents found --- CHANGES.rst | 6 +++--- sphinx/environment/__init__.py | 20 ++++++++++++++++++++ tests/test_builders/test_build.py | 9 +++++++++ 3 files changed, 32 insertions(+), 3 deletions(-) diff --git a/CHANGES.rst b/CHANGES.rst index 4f4ea66e2f9..9c4fe29a91f 100644 --- a/CHANGES.rst +++ b/CHANGES.rst @@ -99,9 +99,9 @@ Bugs fixed * #12796: Enable parallel reading if requested, even if there are fewer than 6 documents. Patch by Matthias Geier. -* #12888: Ensure deterministic resolution of global toctree in parallel builds - when document is included in multiple toctrees by choosing lexicographically - greatest parent document. +* #12888: Add a warning when document is included in multiple toctrees + and ensure deterministic resolution of global toctree in parallel builds + by choosing lexicographically greatest parent document. Patch by A. Rafey Khan diff --git a/sphinx/environment/__init__.py b/sphinx/environment/__init__.py index 67087e61829..3ebd6c16ef6 100644 --- a/sphinx/environment/__init__.py +++ b/sphinx/environment/__init__.py @@ -758,6 +758,7 @@ def check_consistency(self) -> None: continue logger.warning(__("document isn't included in any toctree"), location=docname) + _check_toc_parents(self.toctree_includes) # call check-consistency for all extensions for domain in self.domains.values(): @@ -788,3 +789,22 @@ def _traverse_toctree( if sub_docname not in traversed: yield sub_parent, sub_docname traversed.add(sub_docname) + + +def _check_toc_parents(toctree_includes: dict[str, list[str]]): + toc_parents: dict[str, list[str]] = {} + for parent, children in toctree_includes.items(): + for child in children: + toc_parents.setdefault(child, []).append(parent) + + for doc, parents in sorted(toc_parents.items()): + if len(parents) > 1: + logger.warning( + __("document is referenced in multiple toctrees: %s, " + "selecting: %s <- %s"), + parents, + max(parents), + doc, + location=doc, type='toc', + subtype='multiple_toc_parents' + ) diff --git a/tests/test_builders/test_build.py b/tests/test_builders/test_build.py index 6a5ed0b6d26..9a7d8319d11 100644 --- a/tests/test_builders/test_build.py +++ b/tests/test_builders/test_build.py @@ -100,6 +100,15 @@ def test_numbered_circular_toctree(app): ) in warnings +@pytest.mark.sphinx('text', testroot='toctree-multiple-parents') +def test_multiple_parents_toctree(app): + app.build(force_all=True) + warnings = app.warning.getvalue() + assert ( + "document is referenced in multiple toctrees: ['bar', 'baz'], selecting: baz <- qux" + ) in warnings + + @pytest.mark.usefixtures('_http_teapot') @pytest.mark.sphinx('dummy', testroot='images') def test_image_glob(app): From e693e357b15e5fcd423dca4c09cf92d291775778 Mon Sep 17 00:00:00 2001 From: Adam Turner <9087854+aa-turner@users.noreply.github.com> Date: Sun, 6 Oct 2024 22:41:13 +0100 Subject: [PATCH 4/6] post-merge --- CHANGES.rst | 2 +- sphinx/environment/__init__.py | 15 ++++++++---- .../{bar.rst => alpha.rst} | 7 +++--- .../{baz.rst => bravo.rst} | 7 +++--- .../{qux.rst => charlie.rst} | 5 ++-- .../test-toctree-multiple-parents/conf.py | 1 + .../{foo.rst => delta.rst} | 7 +++--- .../test-toctree-multiple-parents/index.rst | 5 ++-- .../relation_graph.txt | 6 ++--- tests/test_builders/test_build.py | 2 +- .../test_builders/test_build_html_toctree.py | 24 +++++++++---------- 11 files changed, 46 insertions(+), 35 deletions(-) rename tests/roots/test-toctree-multiple-parents/{bar.rst => alpha.rst} (70%) rename tests/roots/test-toctree-multiple-parents/{baz.rst => bravo.rst} (68%) rename tests/roots/test-toctree-multiple-parents/{qux.rst => charlie.rst} (69%) rename tests/roots/test-toctree-multiple-parents/{foo.rst => delta.rst} (68%) diff --git a/CHANGES.rst b/CHANGES.rst index b582a0a89e9..ebce71d358a 100644 --- a/CHANGES.rst +++ b/CHANGES.rst @@ -129,7 +129,7 @@ Bugs fixed Patch by James Addison and Adam Turner. * #12888: Add a warning when document is included in multiple toctrees and ensure deterministic resolution of global toctree in parallel builds - by choosing lexicographically greatest parent document. + by choosing the lexicographically greatest parent document. Patch by A. Rafey Khan diff --git a/sphinx/environment/__init__.py b/sphinx/environment/__init__.py index c841eb06073..c205aebad28 100644 --- a/sphinx/environment/__init__.py +++ b/sphinx/environment/__init__.py @@ -750,6 +750,9 @@ def check_consistency(self) -> None: logger.warning( __("document isn't included in any toctree"), location=docname ) + # Call _check_toc_parents here rather than in _get_toctree_ancestors() + # because that method is called multiple times per document and would + # lead to duplicate warnings. _check_toc_parents(self.toctree_includes) # call check-consistency for all extensions @@ -785,7 +788,7 @@ def _traverse_toctree( traversed.add(sub_docname) -def _check_toc_parents(toctree_includes: dict[str, list[str]]): +def _check_toc_parents(toctree_includes: dict[str, list[str]]) -> None: toc_parents: dict[str, list[str]] = {} for parent, children in toctree_includes.items(): for child in children: @@ -794,11 +797,13 @@ def _check_toc_parents(toctree_includes: dict[str, list[str]]): for doc, parents in sorted(toc_parents.items()): if len(parents) > 1: logger.warning( - __("document is referenced in multiple toctrees: %s, " - "selecting: %s <- %s"), + __( + 'document is referenced in multiple toctrees: %s, selecting: %s <- %s' + ), parents, max(parents), doc, - location=doc, type='toc', - subtype='multiple_toc_parents' + location=doc, + type='toc', + subtype='multiple_toc_parents', ) diff --git a/tests/roots/test-toctree-multiple-parents/bar.rst b/tests/roots/test-toctree-multiple-parents/alpha.rst similarity index 70% rename from tests/roots/test-toctree-multiple-parents/bar.rst rename to tests/roots/test-toctree-multiple-parents/alpha.rst index 7a283eb86dc..c77c94122e1 100644 --- a/tests/roots/test-toctree-multiple-parents/bar.rst +++ b/tests/roots/test-toctree-multiple-parents/alpha.rst @@ -1,6 +1,7 @@ -Bar -=== +Alpha +===== + .. literalinclude:: relation_graph.txt .. toctree:: - qux + bravo diff --git a/tests/roots/test-toctree-multiple-parents/baz.rst b/tests/roots/test-toctree-multiple-parents/bravo.rst similarity index 68% rename from tests/roots/test-toctree-multiple-parents/baz.rst rename to tests/roots/test-toctree-multiple-parents/bravo.rst index adbb3623325..9f65ea6be10 100644 --- a/tests/roots/test-toctree-multiple-parents/baz.rst +++ b/tests/roots/test-toctree-multiple-parents/bravo.rst @@ -1,6 +1,7 @@ -Baz -=== +Bravo +===== + .. literalinclude:: relation_graph.txt .. toctree:: - qux + charlie diff --git a/tests/roots/test-toctree-multiple-parents/qux.rst b/tests/roots/test-toctree-multiple-parents/charlie.rst similarity index 69% rename from tests/roots/test-toctree-multiple-parents/qux.rst rename to tests/roots/test-toctree-multiple-parents/charlie.rst index 58e2a984e09..931979b62df 100644 --- a/tests/roots/test-toctree-multiple-parents/qux.rst +++ b/tests/roots/test-toctree-multiple-parents/charlie.rst @@ -1,3 +1,4 @@ -Qux -=== +Charlie +======= + .. literalinclude:: relation_graph.txt diff --git a/tests/roots/test-toctree-multiple-parents/conf.py b/tests/roots/test-toctree-multiple-parents/conf.py index e69de29bb2d..4db50a3c6ef 100644 --- a/tests/roots/test-toctree-multiple-parents/conf.py +++ b/tests/roots/test-toctree-multiple-parents/conf.py @@ -0,0 +1 @@ +html_theme = 'basic' diff --git a/tests/roots/test-toctree-multiple-parents/foo.rst b/tests/roots/test-toctree-multiple-parents/delta.rst similarity index 68% rename from tests/roots/test-toctree-multiple-parents/foo.rst rename to tests/roots/test-toctree-multiple-parents/delta.rst index 6218eb13878..f9887912a0e 100644 --- a/tests/roots/test-toctree-multiple-parents/foo.rst +++ b/tests/roots/test-toctree-multiple-parents/delta.rst @@ -1,6 +1,7 @@ -Foo -=== +Delta +===== + .. literalinclude:: relation_graph.txt .. toctree:: - bar + charlie diff --git a/tests/roots/test-toctree-multiple-parents/index.rst b/tests/roots/test-toctree-multiple-parents/index.rst index 672b06228c3..d642aa111b7 100644 --- a/tests/roots/test-toctree-multiple-parents/index.rst +++ b/tests/roots/test-toctree-multiple-parents/index.rst @@ -1,7 +1,8 @@ test-toctree-multiple-parents ============================= + .. literalinclude:: relation_graph.txt .. toctree:: - foo - baz + alpha + delta diff --git a/tests/roots/test-toctree-multiple-parents/relation_graph.txt b/tests/roots/test-toctree-multiple-parents/relation_graph.txt index 9cd43438dc7..a610eac2622 100644 --- a/tests/roots/test-toctree-multiple-parents/relation_graph.txt +++ b/tests/roots/test-toctree-multiple-parents/relation_graph.txt @@ -1,7 +1,7 @@ index / \ - foo baz + alpha delta \ / - bar / + bravo / \ / - qux \ No newline at end of file + charlie diff --git a/tests/test_builders/test_build.py b/tests/test_builders/test_build.py index 9a7d8319d11..65f3188423f 100644 --- a/tests/test_builders/test_build.py +++ b/tests/test_builders/test_build.py @@ -105,7 +105,7 @@ def test_multiple_parents_toctree(app): app.build(force_all=True) warnings = app.warning.getvalue() assert ( - "document is referenced in multiple toctrees: ['bar', 'baz'], selecting: baz <- qux" + "document is referenced in multiple toctrees: ['bravo', 'delta'], selecting: delta <- charlie" ) in warnings diff --git a/tests/test_builders/test_build_html_toctree.py b/tests/test_builders/test_build_html_toctree.py index 59a4d794e3a..a59de6328e6 100644 --- a/tests/test_builders/test_build_html_toctree.py +++ b/tests/test_builders/test_build_html_toctree.py @@ -1,9 +1,9 @@ """Test the HTML builder and check output against XPath.""" import re +from unittest.mock import patch import pytest -from unittest.mock import patch from tests.test_builders.xpath_html_util import _intradocument_hyperlink_check from tests.test_builders.xpath_util import check_xpath @@ -72,16 +72,16 @@ def test_singlehtml_hyperlinks(app, cached_etree_parse, expect): confoverrides={'html_theme': 'alabaster'}, ) def test_toctree_multiple_parents(app, cached_etree_parse): - # Lexicographically greatest parent of the document in global toctree - # should be chosen regardless of the order in which files are read - with patch.object(app.builder, '_read_serial') as mock_read_serial: + # The lexicographically greatest parent of the document in global toctree + # should be chosen, regardless of the order in which files are read + with patch.object(app.builder, '_read_serial') as m: # Read files in reversed order - _read_serial = app.builder.__class__._read_serial - mock_read_serial.side_effect = lambda docnames: _read_serial( - app.builder, list(reversed(docnames)) - ) + _read_serial = type(app.builder)._read_serial + m.side_effect = lambda docnames: _read_serial(app.builder, docnames[::-1]) app.build() - # Check if qux is a child of baz in qux.html - xpath_baz_children = ".//ul[@class='current']//a[@href='baz.html']/../ul/li//a" - etree = cached_etree_parse(app.outdir / 'qux.html') - check_xpath(etree, 'qux.html', xpath=xpath_baz_children, check='Qux') + # Check if charlie is a child of delta in charlie.html + xpath_delta_children = ( + ".//ul[@class='current']//a[@href='delta.html']/../ul/li//a" + ) + etree = cached_etree_parse(app.outdir / 'charlie.html') + check_xpath(etree, 'charlie.html', xpath=xpath_delta_children, check='Charlie') From 151f17a2a4def72a1d733a025e9e771227d6eeec Mon Sep 17 00:00:00 2001 From: Adam Turner <9087854+aa-turner@users.noreply.github.com> Date: Sun, 6 Oct 2024 23:07:17 +0100 Subject: [PATCH 5/6] INFO --- sphinx/environment/__init__.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/sphinx/environment/__init__.py b/sphinx/environment/__init__.py index c205aebad28..28e53aa7dec 100644 --- a/sphinx/environment/__init__.py +++ b/sphinx/environment/__init__.py @@ -796,7 +796,7 @@ def _check_toc_parents(toctree_includes: dict[str, list[str]]) -> None: for doc, parents in sorted(toc_parents.items()): if len(parents) > 1: - logger.warning( + logger.info( __( 'document is referenced in multiple toctrees: %s, selecting: %s <- %s' ), From c8b249f439daee95fdea8c8a38f83f4f5f1ffb0c Mon Sep 17 00:00:00 2001 From: Adam Turner <9087854+aa-turner@users.noreply.github.com> Date: Sun, 6 Oct 2024 23:11:50 +0100 Subject: [PATCH 6/6] INFO --- tests/test_builders/test_build.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/tests/test_builders/test_build.py b/tests/test_builders/test_build.py index 65f3188423f..5f396cc48af 100644 --- a/tests/test_builders/test_build.py +++ b/tests/test_builders/test_build.py @@ -103,10 +103,9 @@ def test_numbered_circular_toctree(app): @pytest.mark.sphinx('text', testroot='toctree-multiple-parents') def test_multiple_parents_toctree(app): app.build(force_all=True) - warnings = app.warning.getvalue() assert ( "document is referenced in multiple toctrees: ['bravo', 'delta'], selecting: delta <- charlie" - ) in warnings + ) in app.status.getvalue() @pytest.mark.usefixtures('_http_teapot')