From 23b1c30297384da1ccce67e92b50bb44ca7a1967 Mon Sep 17 00:00:00 2001 From: "Albert Y. Shih" Date: Tue, 22 Aug 2023 13:59:04 -0400 Subject: [PATCH 01/10] Fixed two more bugs with SVG inheritance-diagram links --- CHANGES | 3 +++ sphinx/ext/graphviz.py | 5 +++-- sphinx/ext/inheritance_diagram.py | 3 ++- tests/roots/test-ext-inheritance_diagram/subdir/index.rst | 2 ++ 4 files changed, 10 insertions(+), 3 deletions(-) diff --git a/CHANGES b/CHANGES index 4cd8ac3d335..2291738cdff 100644 --- a/CHANGES +++ b/CHANGES @@ -11,6 +11,9 @@ Bugs fixed * Fix regression in ``autodoc.Documenter.parse_name()``. * Fix regression in JSON serialisation. +* #11634: Fixed two more bugs in inheritance diagrams that resulted in + broken links. + Patch by Albert Shih. Release 7.2.2 (released Aug 17, 2023) ===================================== diff --git a/sphinx/ext/graphviz.py b/sphinx/ext/graphviz.py index 00c9e0b4164..cfab47efead 100644 --- a/sphinx/ext/graphviz.py +++ b/sphinx/ext/graphviz.py @@ -235,10 +235,11 @@ def fix_svg_relative_paths(self: SphinxTranslator, filepath: str) -> None: # not a relative link continue - old_path = path.join(self.builder.outdir, url) + docdir = path.dirname(path.join(self.builder.outdir, self.builder.current_docname)) + old_path = path.join(docdir, url) new_path = path.relpath( old_path, - start=path.join(self.builder.outdir, self.builder.imgpath), + start=path.join(docdir, self.builder.imgpath), ) modified_url = urlunsplit((scheme, hostname, new_path, query, fragment)) diff --git a/sphinx/ext/inheritance_diagram.py b/sphinx/ext/inheritance_diagram.py index 838fe40bc74..3a015a2b13d 100644 --- a/sphinx/ext/inheritance_diagram.py +++ b/sphinx/ext/inheritance_diagram.py @@ -36,6 +36,7 @@ class E(B): pass import re from collections.abc import Iterable, Sequence from importlib import import_module +from os import path from typing import TYPE_CHECKING, Any, cast from docutils import nodes @@ -417,7 +418,7 @@ def html_visit_inheritance_diagram(self: HTML5Translator, node: inheritance_diag # Create a mapping from fully-qualified class names to URLs. graphviz_output_format = self.builder.env.config.graphviz_output_format.upper() - current_filename = self.builder.current_docname + self.builder.out_suffix + current_filename = path.basename(self.builder.current_docname + self.builder.out_suffix) urls = {} pending_xrefs = cast(Iterable[addnodes.pending_xref], node) for child in pending_xrefs: diff --git a/tests/roots/test-ext-inheritance_diagram/subdir/index.rst b/tests/roots/test-ext-inheritance_diagram/subdir/index.rst index bfd4aa88a2d..a3b79730eb7 100644 --- a/tests/roots/test-ext-inheritance_diagram/subdir/index.rst +++ b/tests/roots/test-ext-inheritance_diagram/subdir/index.rst @@ -4,4 +4,6 @@ test-ext-inheritance_diagram subdirectory .. inheritance-diagram:: test.DocMainLevel +.. inheritance-diagram:: test.DocLowerLevel + .. py:class:: test.DocLowerLevel From 7511cf3e6c047ceda8ddb0c06d8e9d6a1ea40bb8 Mon Sep 17 00:00:00 2001 From: Adam Turner <9087854+aa-turner@users.noreply.github.com> Date: Mon, 28 Aug 2023 23:46:53 +0100 Subject: [PATCH 02/10] improve types --- sphinx/ext/graphviz.py | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/sphinx/ext/graphviz.py b/sphinx/ext/graphviz.py index cfab47efead..70f13741d18 100644 --- a/sphinx/ext/graphviz.py +++ b/sphinx/ext/graphviz.py @@ -21,7 +21,7 @@ from sphinx.errors import SphinxError from sphinx.locale import _, __ from sphinx.util import logging -from sphinx.util.docutils import SphinxDirective, SphinxTranslator +from sphinx.util.docutils import SphinxDirective from sphinx.util.i18n import search_image_for_language from sphinx.util.nodes import set_source_info from sphinx.util.osutil import ensuredir @@ -218,7 +218,8 @@ def run(self) -> list[Node]: return [figure] -def fix_svg_relative_paths(self: SphinxTranslator, filepath: str) -> None: +def fix_svg_relative_paths(self: HTML5Translator | LaTeXTranslator | TexinfoTranslator, + filepath: str) -> None: """Change relative links in generated svg files to be relative to imgpath.""" tree = ET.parse(filepath) # NoQA: S314 root = tree.getroot() @@ -250,7 +251,8 @@ def fix_svg_relative_paths(self: SphinxTranslator, filepath: str) -> None: tree.write(filepath) -def render_dot(self: SphinxTranslator, code: str, options: dict, format: str, +def render_dot(self: HTML5Translator | LaTeXTranslator | TexinfoTranslator, + code: str, options: dict, format: str, prefix: str = 'graphviz', filename: str | None = None, ) -> tuple[str | None, str | None]: """Render graphviz code into a PNG or PDF output file.""" From 33ded8898e265618f7e9b323d199061be73a3b06 Mon Sep 17 00:00:00 2001 From: Adam Turner <9087854+aa-turner@users.noreply.github.com> Date: Tue, 29 Aug 2023 00:39:09 +0100 Subject: [PATCH 03/10] refactor --- sphinx/ext/graphviz.py | 12 +++++------- 1 file changed, 5 insertions(+), 7 deletions(-) diff --git a/sphinx/ext/graphviz.py b/sphinx/ext/graphviz.py index 70f13741d18..6e92ed65f71 100644 --- a/sphinx/ext/graphviz.py +++ b/sphinx/ext/graphviz.py @@ -231,17 +231,15 @@ def fix_svg_relative_paths(self: HTML5Translator | LaTeXTranslator | TexinfoTran root.findall('.//svg:image[@xlink:href]', ns), root.findall('.//svg:a[@xlink:href]', ns), ): - scheme, hostname, url, query, fragment = urlsplit(element.attrib[href_name]) + scheme, hostname, rel_uri, query, fragment = urlsplit(element.attrib[href_name]) if hostname: # not a relative link continue - docdir = path.dirname(path.join(self.builder.outdir, self.builder.current_docname)) - old_path = path.join(docdir, url) - new_path = path.relpath( - old_path, - start=path.join(docdir, self.builder.imgpath), - ) + doc_dir = (self.builder.outdir / self.builder.current_docname).parent + old_path = doc_dir / rel_uri + img_path = doc_dir / self.builder.imgpath + new_path = path.relpath(old_path, start=img_path) modified_url = urlunsplit((scheme, hostname, new_path, query, fragment)) element.set(href_name, modified_url) From 8978d218f781f122e32c52b8cdec9547ad6c82d0 Mon Sep 17 00:00:00 2001 From: Adam Turner <9087854+aa-turner@users.noreply.github.com> Date: Tue, 29 Aug 2023 00:40:49 +0100 Subject: [PATCH 04/10] Ignores --- sphinx/ext/graphviz.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/sphinx/ext/graphviz.py b/sphinx/ext/graphviz.py index 6e92ed65f71..726be1720a1 100644 --- a/sphinx/ext/graphviz.py +++ b/sphinx/ext/graphviz.py @@ -295,8 +295,8 @@ def render_dot(self: HTML5Translator | LaTeXTranslator | TexinfoTranslator, logger.warning(__('dot command %r cannot be run (needed for graphviz ' 'output), check the graphviz_dot setting'), graphviz_dot) if not hasattr(self.builder, '_graphviz_warned_dot'): - self.builder._graphviz_warned_dot = {} # type: ignore[attr-defined] - self.builder._graphviz_warned_dot[graphviz_dot] = True # type: ignore[attr-defined] + self.builder._graphviz_warned_dot = {} # type: ignore[union-attr] + self.builder._graphviz_warned_dot[graphviz_dot] = True return None, None except CalledProcessError as exc: raise GraphvizError(__('dot exited with error:\n[stderr]\n%r\n' From af8a1f4dbf40499199ecb55963b7d94743f5126d Mon Sep 17 00:00:00 2001 From: Adam Turner <9087854+aa-turner@users.noreply.github.com> Date: Tue, 29 Aug 2023 01:04:36 +0100 Subject: [PATCH 05/10] Use fake absolute paths --- sphinx/ext/graphviz.py | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/sphinx/ext/graphviz.py b/sphinx/ext/graphviz.py index 726be1720a1..f5ccd758883 100644 --- a/sphinx/ext/graphviz.py +++ b/sphinx/ext/graphviz.py @@ -236,9 +236,8 @@ def fix_svg_relative_paths(self: HTML5Translator | LaTeXTranslator | TexinfoTran # not a relative link continue - doc_dir = (self.builder.outdir / self.builder.current_docname).parent - old_path = doc_dir / rel_uri - img_path = doc_dir / self.builder.imgpath + old_path = path.sep + rel_uri + img_path = path.sep + self.builder.imgpath new_path = path.relpath(old_path, start=img_path) modified_url = urlunsplit((scheme, hostname, new_path, query, fragment)) From 953f2f67c2d413786fadc73932ced5b62919649c Mon Sep 17 00:00:00 2001 From: Adam Turner <9087854+aa-turner@users.noreply.github.com> Date: Wed, 30 Aug 2023 02:38:43 +0100 Subject: [PATCH 06/10] Fix for relative documents etc --- sphinx/ext/graphviz.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/sphinx/ext/graphviz.py b/sphinx/ext/graphviz.py index f5ccd758883..4cea985bc5d 100644 --- a/sphinx/ext/graphviz.py +++ b/sphinx/ext/graphviz.py @@ -236,8 +236,8 @@ def fix_svg_relative_paths(self: HTML5Translator | LaTeXTranslator | TexinfoTran # not a relative link continue - old_path = path.sep + rel_uri - img_path = path.sep + self.builder.imgpath + old_path = path.basename(self.builder.current_docname) + rel_uri + img_path = path.basename(self.builder.current_docname) + self.builder.imgpath new_path = path.relpath(old_path, start=img_path) modified_url = urlunsplit((scheme, hostname, new_path, query, fragment)) From 6c10c768e3e109e93ed8337f21bf681d5397e2c5 Mon Sep 17 00:00:00 2001 From: Adam Turner <9087854+aa-turner@users.noreply.github.com> Date: Wed, 30 Aug 2023 02:55:20 +0100 Subject: [PATCH 07/10] fixup! Fix for relative documents etc --- sphinx/ext/graphviz.py | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/sphinx/ext/graphviz.py b/sphinx/ext/graphviz.py index 4cea985bc5d..90e2a3a11df 100644 --- a/sphinx/ext/graphviz.py +++ b/sphinx/ext/graphviz.py @@ -236,8 +236,11 @@ def fix_svg_relative_paths(self: HTML5Translator | LaTeXTranslator | TexinfoTran # not a relative link continue - old_path = path.basename(self.builder.current_docname) + rel_uri - img_path = path.basename(self.builder.current_docname) + self.builder.imgpath + docname = self.builder.env.path2doc(self.document["source"]) + doc_dir = self.builder.app.outdir.joinpath(docname).resolve().parent + + old_path = doc_dir / rel_uri + img_path = doc_dir / self.builder.imgpath new_path = path.relpath(old_path, start=img_path) modified_url = urlunsplit((scheme, hostname, new_path, query, fragment)) From 2b6593ea7875217087b895b4cfea2aeb65118e56 Mon Sep 17 00:00:00 2001 From: Adam Turner <9087854+aa-turner@users.noreply.github.com> Date: Wed, 30 Aug 2023 02:57:29 +0100 Subject: [PATCH 08/10] typing --- sphinx/ext/graphviz.py | 3 +++ 1 file changed, 3 insertions(+) diff --git a/sphinx/ext/graphviz.py b/sphinx/ext/graphviz.py index 90e2a3a11df..528bf30cbf6 100644 --- a/sphinx/ext/graphviz.py +++ b/sphinx/ext/graphviz.py @@ -237,6 +237,9 @@ def fix_svg_relative_paths(self: HTML5Translator | LaTeXTranslator | TexinfoTran continue docname = self.builder.env.path2doc(self.document["source"]) + if docname is None: + # This shouldn't happen! + continue doc_dir = self.builder.app.outdir.joinpath(docname).resolve().parent old_path = doc_dir / rel_uri From 66baae37b1d3fffb73e7be61e89fdceb4a80c370 Mon Sep 17 00:00:00 2001 From: "Albert Y. Shih" Date: Wed, 30 Aug 2023 09:42:04 -0400 Subject: [PATCH 09/10] Expanded unit test to cover linking between files in the same subdirectory --- .../{subdir => external}/other.py | 0 .../test-ext-inheritance_diagram/index.rst | 4 ++-- .../subdir/index.rst | 9 --------- .../subdir/page1.rst | 9 +++++++++ .../subdir/page2.rst | 5 +++++ .../test-ext-inheritance_diagram/test.py | 6 +++++- tests/test_ext_inheritance_diagram.py | 19 ++++++++++--------- 7 files changed, 31 insertions(+), 21 deletions(-) rename tests/roots/test-ext-inheritance_diagram/{subdir => external}/other.py (100%) delete mode 100644 tests/roots/test-ext-inheritance_diagram/subdir/index.rst create mode 100644 tests/roots/test-ext-inheritance_diagram/subdir/page1.rst create mode 100644 tests/roots/test-ext-inheritance_diagram/subdir/page2.rst diff --git a/tests/roots/test-ext-inheritance_diagram/subdir/other.py b/tests/roots/test-ext-inheritance_diagram/external/other.py similarity index 100% rename from tests/roots/test-ext-inheritance_diagram/subdir/other.py rename to tests/roots/test-ext-inheritance_diagram/external/other.py diff --git a/tests/roots/test-ext-inheritance_diagram/index.rst b/tests/roots/test-ext-inheritance_diagram/index.rst index 2e9283b7ad9..e694fb06988 100644 --- a/tests/roots/test-ext-inheritance_diagram/index.rst +++ b/tests/roots/test-ext-inheritance_diagram/index.rst @@ -7,12 +7,12 @@ test-ext-inheritance_diagram .. inheritance-diagram:: test.Foo :caption: Test Foo! -.. inheritance-diagram:: test.DocLowerLevel +.. inheritance-diagram:: test.DocSubDir2 .. py:class:: test.DocHere .. py:class:: test.DocMainLevel -.. inheritance-diagram:: subdir.other.Bob +.. inheritance-diagram:: external.other.Bob .. py:class:: test.Alice diff --git a/tests/roots/test-ext-inheritance_diagram/subdir/index.rst b/tests/roots/test-ext-inheritance_diagram/subdir/index.rst deleted file mode 100644 index a3b79730eb7..00000000000 --- a/tests/roots/test-ext-inheritance_diagram/subdir/index.rst +++ /dev/null @@ -1,9 +0,0 @@ -========================================= -test-ext-inheritance_diagram subdirectory -========================================= - -.. inheritance-diagram:: test.DocMainLevel - -.. inheritance-diagram:: test.DocLowerLevel - -.. py:class:: test.DocLowerLevel diff --git a/tests/roots/test-ext-inheritance_diagram/subdir/page1.rst b/tests/roots/test-ext-inheritance_diagram/subdir/page1.rst new file mode 100644 index 00000000000..3001b021929 --- /dev/null +++ b/tests/roots/test-ext-inheritance_diagram/subdir/page1.rst @@ -0,0 +1,9 @@ +================================================ +test-ext-inheritance_diagram subdirectory page 1 +================================================ + +.. inheritance-diagram:: test.DocMainLevel + +.. inheritance-diagram:: test.DocSubDir2 + +.. py:class:: test.DocSubDir1 diff --git a/tests/roots/test-ext-inheritance_diagram/subdir/page2.rst b/tests/roots/test-ext-inheritance_diagram/subdir/page2.rst new file mode 100644 index 00000000000..720e2d834ad --- /dev/null +++ b/tests/roots/test-ext-inheritance_diagram/subdir/page2.rst @@ -0,0 +1,5 @@ +================================================ +test-ext-inheritance_diagram subdirectory page 2 +================================================ + +.. py:class:: test.DocSubDir2 diff --git a/tests/roots/test-ext-inheritance_diagram/test.py b/tests/roots/test-ext-inheritance_diagram/test.py index dde7b21d1c4..efb1c2a7f6e 100644 --- a/tests/roots/test-ext-inheritance_diagram/test.py +++ b/tests/roots/test-ext-inheritance_diagram/test.py @@ -6,7 +6,11 @@ class DocHere(Foo): pass -class DocLowerLevel(DocHere): +class DocSubDir1(DocHere): + pass + + +class DocSubDir2(DocSubDir1): pass diff --git a/tests/test_ext_inheritance_diagram.py b/tests/test_ext_inheritance_diagram.py index 5329604310c..9ace5ad7956 100644 --- a/tests/test_ext_inheritance_diagram.py +++ b/tests/test_ext_inheritance_diagram.py @@ -138,13 +138,13 @@ def new_run(self): # An external inventory to test intersphinx links in inheritance diagrams -subdir_inventory = b'''\ +external_inventory = b'''\ # Sphinx inventory version 2 -# Project: subdir +# Project: external # Version: 1.0 # The remainder of this file is compressed using zlib. ''' + zlib.compress(b'''\ -subdir.other.Bob py:class 1 foo.html#subdir.other.Bob - +external.other.Bob py:class 1 foo.html#external.other.Bob - ''') @@ -152,7 +152,7 @@ def new_run(self): @pytest.mark.usefixtures('if_graphviz_found') def test_inheritance_diagram_png_html(tmp_path, app): inv_file = tmp_path / 'inventory' - inv_file.write_bytes(subdir_inventory) + inv_file.write_bytes(external_inventory) app.config.intersphinx_mapping = { 'https://example.org': str(inv_file), } @@ -173,7 +173,7 @@ def test_inheritance_diagram_png_html(tmp_path, app): 'title="Link to this image">\xb6

\n\n\n') assert re.search(pattern, content, re.M) - subdir_content = (app.outdir / 'subdir/index.html').read_text(encoding='utf8') + subdir_content = (app.outdir / 'subdir/page1.html').read_text(encoding='utf8') subdir_maps = re.findall('', subdir_content) subdir_maps = [re.sub('href="(\\S+)"', 'href="subdir/\\g<1>"', s) for s in subdir_maps] @@ -199,7 +199,7 @@ def test_inheritance_diagram_png_html(tmp_path, app): @pytest.mark.usefixtures('if_graphviz_found') def test_inheritance_diagram_svg_html(tmp_path, app): inv_file = tmp_path / 'inventory' - inv_file.write_bytes(subdir_inventory) + inv_file.write_bytes(external_inventory) app.config.intersphinx_mapping = { "subdir": ('https://example.org', str(inv_file)), } @@ -223,7 +223,7 @@ def test_inheritance_diagram_svg_html(tmp_path, app): assert re.search(pattern, content, re.M) - subdir_content = (app.outdir / 'subdir/index.html').read_text(encoding='utf8') + subdir_content = (app.outdir / 'subdir/page1.html').read_text(encoding='utf8') subdir_svgs = re.findall(' Date: Wed, 30 Aug 2023 09:43:16 -0400 Subject: [PATCH 10/10] Update CHANGES Co-authored-by: Adam Turner <9087854+AA-Turner@users.noreply.github.com> --- CHANGES | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/CHANGES b/CHANGES index ae9a2b1c353..8465a6bf7d3 100644 --- a/CHANGES +++ b/CHANGES @@ -20,8 +20,8 @@ Bugs fixed packages that make use of ``if typing.TYPE_CHECKING:`` to guard circular imports needed by type checkers. Patch by Matt Wozniski. -* #11634: Fixed two more bugs in inheritance diagrams that resulted in - broken links. +* #11634: Fixed inheritance diagram relative link resolution + for sibling files in a subdirectory. Patch by Albert Shih. Testing