From e50c662e205068aa845f4eb53349b3f0645944ca Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?B=C3=A9n=C3=A9dikt=20Tran?= <10796600+picnixz@users.noreply.github.com> Date: Wed, 4 Oct 2023 16:19:53 +0200 Subject: [PATCH 01/52] fix intersphinx cache loading in incremental builds --- CHANGES.rst | 3 ++ sphinx/ext/intersphinx.py | 45 ++++++++++++++++++++++-------- sphinx/util/typing.py | 1 + tests/test_ext_intersphinx.py | 52 +++++++++++++++++++++++++++++++++++ 4 files changed, 90 insertions(+), 11 deletions(-) diff --git a/CHANGES.rst b/CHANGES.rst index ca5fe1bc482..f3a072316bb 100644 --- a/CHANGES.rst +++ b/CHANGES.rst @@ -36,6 +36,9 @@ Bugs fixed Patch by Bénédikt Tran. * #11697: HTML Search: add 'noindex' meta robots tag. Patch by James Addison. +* #11466: intersphinx: fix cache loading when the inventory URI is changed + in an incremental build. + Patch by Bénédikt Tran. Testing ------- diff --git a/sphinx/ext/intersphinx.py b/sphinx/ext/intersphinx.py index 453bb6e986d..796df6a992c 100644 --- a/sphinx/ext/intersphinx.py +++ b/sphinx/ext/intersphinx.py @@ -24,6 +24,7 @@ import re import sys import time +from itertools import chain from os import path from typing import TYPE_CHECKING, cast from urllib.parse import urlsplit, urlunsplit @@ -229,6 +230,7 @@ def fetch_inventory_group( logger.info(__('loading intersphinx inventory from %s...'), safe_inv_url) try: invdata = fetch_inventory(app, uri, inv) + print(invdata) except Exception as err: failures.append(err.args) continue @@ -237,7 +239,7 @@ def fetch_inventory_group( return True return False finally: - if failures == []: + if not failures: pass elif len(failures) < len(invs): logger.info(__("encountered some issues with some of the inventories," @@ -273,22 +275,43 @@ def load_mappings(app: Sphinx) -> None: # Duplicate values in different inventories will shadow each # other; which one will override which can vary between builds # since they are specified using an unordered dict. To make - # it more consistent, we sort the named inventories and then - # add the unnamed inventories last. This means that the - # unnamed inventories will shadow the named ones but the named - # ones can still be accessed when the name is specified. - named_vals = [] - unnamed_vals = [] + # it more consistent, we sort the named inventories by their + # name and then add the unnamed inventories last. This means + # that the unnamed inventories will shadow the named ones but + # the named ones can still be accessed when the name is specified. + # + # When we encounter a named inventory that already exists, + # this means that we had two entries for the same inventory, + # but with different URIs (e.g., the remote documentation was + # updated). In particular, the newest URI is inserted in the + # cache *after* the one that existed from a previous build. + # + # Example: assume that we use A to generate the inventory of "foo", + # so that we have + # + # intersphinx_cache = {A: ('foo', timeout, D1)} + # + # If we rebuild the project, the cache becomes + # + # intersphinx_cache = {A: ('foo', ..., D1), B: ('foo', ..., D2)} + # + # So if we iterate the values of ``intersphinx_cache``, we correctly + # replace old inventory cache data with the same name but different + # URIs. If we have the same URI or if we force-reload the cache, the + # data is already updated. + named_vals: dict[str, Inventory] = {} + unnamed_vals: list[tuple[str, Inventory]] = [] for name, _expiry, invdata in intersphinx_cache.values(): if name: - named_vals.append((name, invdata)) + named_vals[name] = invdata else: unnamed_vals.append((name, invdata)) - for name, invdata in sorted(named_vals) + unnamed_vals: + + for name, invdata in chain(named_vals.items(), unnamed_vals): if name: inventories.named_inventory[name] = invdata - for type, objects in invdata.items(): - inventories.main_inventory.setdefault(type, {}).update(objects) + for objtype, objects in invdata.items(): + inventories.main_inventory.setdefault(objtype, {}).update(objects) def _create_element_from_result(domain: Domain, inv_name: str | None, diff --git a/sphinx/util/typing.py b/sphinx/util/typing.py index 171420df58b..76bf7aa4baa 100644 --- a/sphinx/util/typing.py +++ b/sphinx/util/typing.py @@ -61,6 +61,7 @@ def is_invalid_builtin_class(obj: Any) -> bool: str, # URL str, # display name ] +# referencable role => (reference name => inventory item) Inventory = dict[str, dict[str, InventoryItem]] diff --git a/tests/test_ext_intersphinx.py b/tests/test_ext_intersphinx.py index 912cc4e6395..4cece1a91e8 100644 --- a/tests/test_ext_intersphinx.py +++ b/tests/test_ext_intersphinx.py @@ -1,9 +1,12 @@ """Test the intersphinx extension.""" +from __future__ import annotations + import http.server from unittest import mock import pytest +import zlib from docutils import nodes from sphinx import addnodes @@ -427,6 +430,55 @@ def test_load_mappings_fallback(tmp_path, app, status, warning): assert isinstance(rn, nodes.reference) +@pytest.mark.sphinx('dummy', testroot='basic') +def test_load_mappings_cache_update(make_app, app_params): + def make_invdata(i: int) -> bytes: + headers = f'''\ +# Sphinx inventory version 2 +# Project: foo +# Version: {i} +# The remainder of this file is compressed with zlib. +'''.encode() + line = f'module{i} py:module 0 foo.html#module-$ -\n'.encode() + return headers + zlib.compress(line) + + class InventoryHandler(http.server.BaseHTTPRequestHandler): + def do_GET(self): + self.send_response(200, 'OK') + if self.path.startswith('/old/'): + data = make_invdata(1) + else: + data = make_invdata(2) + self.send_header('Content-Length', str(len(data))) + self.end_headers() + self.wfile.write(data) + + def log_message(*args, **kwargs): + pass + + # clean build + args, kwargs = app_params + _ = make_app(*args, freshenv=True, **kwargs) + _.build() + + confoverrides = {'extensions': ['sphinx.ext.intersphinx']} + + with http_server(InventoryHandler): + url1 = f'http://localhost:7777/old' + confoverrides1 = confoverrides | {'intersphinx_mapping': {'foo': (url1, None)}} + app1 = make_app(*args, confoverrides=confoverrides1, **kwargs) + app1.build() + entry1 = {'module1': ('foo', '1', f'{url1}/foo.html#module-module1', '-')} + assert app1.env.intersphinx_named_inventory == {'foo': {'py:module': entry1}} + + url2 = f'http://localhost:7777/new' + confoverrides2 = confoverrides | {'intersphinx_mapping': {'foo': (url2, None)}} + app2 = make_app(*args, confoverrides=confoverrides2, **kwargs) + app2.build() + entry2 = {'module2': ('foo', '2', f'{url2}/foo.html#module-module2', '-')} + assert app2.env.intersphinx_named_inventory == {'foo': {'py:module': entry2}} + + class TestStripBasicAuth: """Tests for sphinx.ext.intersphinx._strip_basic_auth()""" def test_auth_stripped(self): From ca545a322a1813644deaec9a9dbdd593e307cb44 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?B=C3=A9n=C3=A9dikt=20Tran?= <10796600+picnixz@users.noreply.github.com> Date: Wed, 4 Oct 2023 16:27:55 +0200 Subject: [PATCH 02/52] fix lint0 --- sphinx/ext/intersphinx.py | 2 +- tests/test_ext_intersphinx.py | 6 +++--- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/sphinx/ext/intersphinx.py b/sphinx/ext/intersphinx.py index 796df6a992c..ed0edf972f4 100644 --- a/sphinx/ext/intersphinx.py +++ b/sphinx/ext/intersphinx.py @@ -300,7 +300,7 @@ def load_mappings(app: Sphinx) -> None: # URIs. If we have the same URI or if we force-reload the cache, the # data is already updated. named_vals: dict[str, Inventory] = {} - unnamed_vals: list[tuple[str, Inventory]] = [] + unnamed_vals: list[tuple[str | None, Inventory]] = [] for name, _expiry, invdata in intersphinx_cache.values(): if name: named_vals[name] = invdata diff --git a/tests/test_ext_intersphinx.py b/tests/test_ext_intersphinx.py index 4cece1a91e8..b90fabd4b90 100644 --- a/tests/test_ext_intersphinx.py +++ b/tests/test_ext_intersphinx.py @@ -3,10 +3,10 @@ from __future__ import annotations import http.server +import zlib from unittest import mock import pytest -import zlib from docutils import nodes from sphinx import addnodes @@ -464,14 +464,14 @@ def log_message(*args, **kwargs): confoverrides = {'extensions': ['sphinx.ext.intersphinx']} with http_server(InventoryHandler): - url1 = f'http://localhost:7777/old' + url1 = 'http://localhost:7777/old' confoverrides1 = confoverrides | {'intersphinx_mapping': {'foo': (url1, None)}} app1 = make_app(*args, confoverrides=confoverrides1, **kwargs) app1.build() entry1 = {'module1': ('foo', '1', f'{url1}/foo.html#module-module1', '-')} assert app1.env.intersphinx_named_inventory == {'foo': {'py:module': entry1}} - url2 = f'http://localhost:7777/new' + url2 = 'http://localhost:7777/new' confoverrides2 = confoverrides | {'intersphinx_mapping': {'foo': (url2, None)}} app2 = make_app(*args, confoverrides=confoverrides2, **kwargs) app2.build() From cfcb4f59522445535346ecfe86dd14f59fb0923a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?B=C3=A9n=C3=A9dikt=20Tran?= <10796600+picnixz@users.noreply.github.com> Date: Thu, 5 Oct 2023 10:31:56 +0200 Subject: [PATCH 03/52] Remove debug print --- sphinx/ext/intersphinx.py | 1 - 1 file changed, 1 deletion(-) diff --git a/sphinx/ext/intersphinx.py b/sphinx/ext/intersphinx.py index ed0edf972f4..b116c331f1f 100644 --- a/sphinx/ext/intersphinx.py +++ b/sphinx/ext/intersphinx.py @@ -230,7 +230,6 @@ def fetch_inventory_group( logger.info(__('loading intersphinx inventory from %s...'), safe_inv_url) try: invdata = fetch_inventory(app, uri, inv) - print(invdata) except Exception as err: failures.append(err.args) continue From 92243a44dd6e9304d1290cdf0266c8bf5c3d0b77 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?B=C3=A9n=C3=A9dikt=20Tran?= <10796600+picnixz@users.noreply.github.com> Date: Thu, 5 Oct 2023 14:01:21 +0200 Subject: [PATCH 04/52] save --- sphinx/ext/intersphinx.py | 44 ++++++++++++++++++++++++++--------- tests/test_ext_intersphinx.py | 40 ++++++++++++++++++++++++------- 2 files changed, 65 insertions(+), 19 deletions(-) diff --git a/sphinx/ext/intersphinx.py b/sphinx/ext/intersphinx.py index b116c331f1f..4a1d8f911f0 100644 --- a/sphinx/ext/intersphinx.py +++ b/sphinx/ext/intersphinx.py @@ -234,6 +234,26 @@ def fetch_inventory_group( failures.append(err.args) continue if invdata: + old_uris = set() + # Find the URIs of the projects that are currently stored + # but should be removed because they are being overwritten; + # + # If the current cache contains a project named "foo" with + # some URI_FOO, and if the new intersphinx mapping value + # uses URI_FOO for another project, we need to be sure that + # either the + # to first remove + # all the URL + for old_uri, (project, _, _) in cache.items(): + if project == name: + old_uris.add(old_uri) + + for old_uri in old_uris: + del cache[old_uri] + if uri in cache: + # ensure that the cache is removed now + del cache[uri] + cache[uri] = name, now, invdata return True return False @@ -273,26 +293,25 @@ def load_mappings(app: Sphinx) -> None: # Duplicate values in different inventories will shadow each # other; which one will override which can vary between builds - # since they are specified using an unordered dict. To make - # it more consistent, we sort the named inventories by their - # name and then add the unnamed inventories last. This means - # that the unnamed inventories will shadow the named ones but - # the named ones can still be accessed when the name is specified. + # since they are specified using an unordered dict. Nevertheless, + # we can ensure that the build is using the latest inventory data. # # When we encounter a named inventory that already exists, # this means that we had two entries for the same inventory, # but with different URIs (e.g., the remote documentation was - # updated). In particular, the newest URI is inserted in the + # updated). In particular, the newest URI is inserted in the # cache *after* the one that existed from a previous build. # - # Example: assume that we use A to generate the inventory of "foo", + # Example: assume that we use URI1 to generate the inventory of "foo", # so that we have # - # intersphinx_cache = {A: ('foo', timeout, D1)} + # intersphinx_cache = {URI1: ('foo', timeout, DATA_FROM_URI1)} # - # If we rebuild the project, the cache becomes + # If we rebuild the project but change URI1 to URI2 while keeping + # the build directory (i.e. incremental build), the cache becomes # - # intersphinx_cache = {A: ('foo', ..., D1), B: ('foo', ..., D2)} + # intersphinx_cache = {URI1: ('foo', ..., DATA_FROM_URI1), + # URI2: ('foo', ..., DATA_FROM_URI2)} # # So if we iterate the values of ``intersphinx_cache``, we correctly # replace old inventory cache data with the same name but different @@ -306,7 +325,10 @@ def load_mappings(app: Sphinx) -> None: else: unnamed_vals.append((name, invdata)) - for name, invdata in chain(named_vals.items(), unnamed_vals): + # we do not sort the named inventories by their name so that two builds + # with same intersphinx_mapping value but ordered differently reflect + # the order correctly for reproducible builds + for name, invdata in chain(sorted(named_vals.items()), unnamed_vals): if name: inventories.named_inventory[name] = invdata for objtype, objects in invdata.items(): diff --git a/tests/test_ext_intersphinx.py b/tests/test_ext_intersphinx.py index b90fabd4b90..9bae437fc2c 100644 --- a/tests/test_ext_intersphinx.py +++ b/tests/test_ext_intersphinx.py @@ -445,10 +445,14 @@ def make_invdata(i: int) -> bytes: class InventoryHandler(http.server.BaseHTTPRequestHandler): def do_GET(self): self.send_response(200, 'OK') + if self.path.startswith('/old/'): data = make_invdata(1) - else: + elif self.path.startswith('/new/'): data = make_invdata(2) + else: + data = b'' + self.send_header('Content-Length', str(len(data))) self.end_headers() self.wfile.write(data) @@ -461,23 +465,43 @@ def log_message(*args, **kwargs): _ = make_app(*args, freshenv=True, **kwargs) _.build() - confoverrides = {'extensions': ['sphinx.ext.intersphinx']} + baseconfig = {'extensions': ['sphinx.ext.intersphinx']} + url_old = 'http://localhost:7777/old' + url_new = 'http://localhost:7777/new' with http_server(InventoryHandler): - url1 = 'http://localhost:7777/old' - confoverrides1 = confoverrides | {'intersphinx_mapping': {'foo': (url1, None)}} + confoverrides1 = baseconfig | {'intersphinx_mapping': {'foo': (url_old, None)}} app1 = make_app(*args, confoverrides=confoverrides1, **kwargs) app1.build() - entry1 = {'module1': ('foo', '1', f'{url1}/foo.html#module-module1', '-')} + entry1 = {'module1': ('foo', '1', f'{url_old}/foo.html#module-module1', '-')} + + assert list(app1.env.intersphinx_cache) == [url_old] + assert app1.env.intersphinx_cache[url_old][0] == 'foo' + assert app1.env.intersphinx_cache[url_old][2] == {'py:module': entry1} assert app1.env.intersphinx_named_inventory == {'foo': {'py:module': entry1}} - url2 = 'http://localhost:7777/new' - confoverrides2 = confoverrides | {'intersphinx_mapping': {'foo': (url2, None)}} + # switch to new url and assert that the old URL is no more stored + confoverrides2 = baseconfig | {'intersphinx_mapping': {'foo': (url_new, None)}} app2 = make_app(*args, confoverrides=confoverrides2, **kwargs) app2.build() - entry2 = {'module2': ('foo', '2', f'{url2}/foo.html#module-module2', '-')} + entry2 = {'module2': ('foo', '2', f'{url_new}/foo.html#module-module2', '-')} + + assert list(app2.env.intersphinx_cache) == [url_new] + assert app2.env.intersphinx_cache[url_new][0] == 'foo' + assert app2.env.intersphinx_cache[url_new][2] == {'py:module': entry2} assert app2.env.intersphinx_named_inventory == {'foo': {'py:module': entry2}} + # switch back to old url + confoverrides3 = baseconfig | {'intersphinx_mapping': {'foo': (url_old, None)}} + app3 = make_app(*args, confoverrides=confoverrides3, **kwargs) + app3.build() + # same as entry 1 + entry3 = {'module1': ('foo', '1', f'{url_old}/foo.html#module-module1', '-')} + + assert list(app3.env.intersphinx_cache) == [url_old] + assert app3.env.intersphinx_cache[url_old][0] == 'foo' + assert app3.env.intersphinx_cache[url_old][2] == {'py:module': entry3} + assert app3.env.intersphinx_named_inventory == {'foo': {'py:module': entry3}} class TestStripBasicAuth: """Tests for sphinx.ext.intersphinx._strip_basic_auth()""" From 56f553301a2a0a1f13c1c4f3c74c347fb57fae6e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?B=C3=A9n=C3=A9dikt=20Tran?= <10796600+picnixz@users.noreply.github.com> Date: Sat, 3 Feb 2024 12:31:40 +0100 Subject: [PATCH 05/52] update implementation and comments --- sphinx/ext/intersphinx.py | 53 ++++++++++--------- tests/test_extensions/test_ext_intersphinx.py | 2 + 2 files changed, 29 insertions(+), 26 deletions(-) diff --git a/sphinx/ext/intersphinx.py b/sphinx/ext/intersphinx.py index 5e3e6acb36d..0262f94899b 100644 --- a/sphinx/ext/intersphinx.py +++ b/sphinx/ext/intersphinx.py @@ -235,23 +235,23 @@ def fetch_inventory_group( continue if invdata: old_uris = set() - # Find the URIs of the projects that are currently stored - # but should be removed because they are being overwritten; + # Find the URIs of the *existing* projects that are to + # be removed because there are being overwritten. # - # If the current cache contains a project named "foo" with - # some URI_FOO, and if the new intersphinx mapping value - # uses URI_FOO for another project, we need to be sure that - # either the - # to first remove - # all the URL - for old_uri, (project, _, _) in cache.items(): - if project == name: + # If the current cache contains some (project, uri) pair + # say ("foo", "foo.com") and if the new intersphinx dict + # contains the pair ("bar", "foo.com"), we need to remove + # the first entry and use the second one. + for old_uri, (old_name, _expiry, _data) in cache.items(): + if old_name == name: old_uris.add(old_uri) for old_uri in old_uris: del cache[old_uri] + if uri in cache: - # ensure that the cache is removed now + # ensure that the cache data is moved to the end + # when setting `cache[uri] = ...` del cache[uri] cache[uri] = name, now, invdata @@ -292,31 +292,32 @@ def load_mappings(app: Sphinx) -> None: inventories.clear() # Duplicate values in different inventories will shadow each - # other; which one will override which can vary between builds - # since they are specified using an unordered dict. Nevertheless, - # we can ensure that the build is using the latest inventory data. + # other; which one will override which may vary between builds, + # but we can ensure using the latest inventory data. # # When we encounter a named inventory that already exists, # this means that we had two entries for the same inventory, - # but with different URIs (e.g., the remote documentation was - # updated). In particular, the newest URI is inserted in the - # cache *after* the one that existed from a previous build. + # but with different URIs (e.g., the remote URL was updated). + # + # In particular, the newest URI is inserted in the cache *after* + # the one that existed from a previous build (dict are ordered + # by insertion time since Python 3.6). # - # Example: assume that we use URI1 to generate the inventory of "foo", + # Example: assume that we use URI_A to generate the inventory of "foo", # so that we have # - # intersphinx_cache = {URI1: ('foo', timeout, DATA_FROM_URI1)} + # intersphinx_cache = {URI_A: ('foo', timeout, DATA_FROM_URI1)} # - # If we rebuild the project but change URI1 to URI2 while keeping + # If we rebuild the project but change URI_A to URI_B while keeping # the build directory (i.e. incremental build), the cache becomes # - # intersphinx_cache = {URI1: ('foo', ..., DATA_FROM_URI1), - # URI2: ('foo', ..., DATA_FROM_URI2)} + # intersphinx_cache = {URI_A: ('foo', ..., DATA_FROM_URI_A), + # URI_B: ('foo', ..., DATA_FROM_URI_B)} # - # So if we iterate the values of ``intersphinx_cache``, we correctly - # replace old inventory cache data with the same name but different - # URIs. If we have the same URI or if we force-reload the cache, the - # data is already updated. + # So if we iterate the *values* of ``intersphinx_cache`` and not + # the keys, we correctly replace old (in time) inventory cache data + # with the same name but different URIs (and/or data). If the URI is + # the same or if we force-reload the cache, only the data is updated. named_vals: dict[str, Inventory] = {} unnamed_vals: list[tuple[str | None, Inventory]] = [] for name, _expiry, invdata in intersphinx_cache.values(): diff --git a/tests/test_extensions/test_ext_intersphinx.py b/tests/test_extensions/test_ext_intersphinx.py index 03eaa518db1..afc79fdd3a4 100644 --- a/tests/test_extensions/test_ext_intersphinx.py +++ b/tests/test_extensions/test_ext_intersphinx.py @@ -473,6 +473,7 @@ def log_message(*args, **kwargs): confoverrides1 = baseconfig | {'intersphinx_mapping': {'foo': (url_old, None)}} app1 = make_app(*args, confoverrides=confoverrides1, **kwargs) app1.build() + # the inventory when querying the 'old' URL entry1 = {'module1': ('foo', '1', f'{url_old}/foo.html#module-module1', '-')} assert list(app1.env.intersphinx_cache) == [url_old] @@ -503,6 +504,7 @@ def log_message(*args, **kwargs): assert app3.env.intersphinx_cache[url_old][2] == {'py:module': entry3} assert app3.env.intersphinx_named_inventory == {'foo': {'py:module': entry3}} + class TestStripBasicAuth: """Tests for sphinx.ext.intersphinx._strip_basic_auth()""" From ac22d65ba2cc24536a376ffadf18ad0a1e5387d4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?B=C3=A9n=C3=A9dikt=20Tran?= <10796600+picnixz@users.noreply.github.com> Date: Tue, 13 Feb 2024 14:21:55 +0100 Subject: [PATCH 06/52] update logic and refactor --- tests/test_extensions/test_ext_intersphinx.py | 210 ++++++++++++++---- 1 file changed, 172 insertions(+), 38 deletions(-) diff --git a/tests/test_extensions/test_ext_intersphinx.py b/tests/test_extensions/test_ext_intersphinx.py index afc79fdd3a4..fed5b761f8d 100644 --- a/tests/test_extensions/test_ext_intersphinx.py +++ b/tests/test_extensions/test_ext_intersphinx.py @@ -3,7 +3,11 @@ from __future__ import annotations import http.server +import posixpath +import re import zlib +from io import BytesIO +from typing import TYPE_CHECKING from unittest import mock import pytest @@ -25,6 +29,126 @@ from tests.test_util.test_util_inventory import inventory_v2, inventory_v2_not_having_version from tests.utils import http_server +if TYPE_CHECKING: + from collections.abc import Iterable + from typing import Any, BinaryIO + + from sphinx.util.typing import InventoryItem + + +class InventoryEntry: + __slots__ = ( + 'name', 'display_name', 'domain_name', 'object_type', 'uri', 'anchor', 'priority', + ) + + def __init__( + self, + name: str, + *, + display_name: str | None = None, + domain_name: str = 'py', + object_type: str = 'obj', + uri: str = 'index.html', + anchor: str = '', + priority: int = 0, + ): + if anchor.endswith(name): + anchor = anchor[:-len(name)] + '$' + + if anchor: + uri += '#' + anchor + + if display_name is None or display_name == name: + display_name = '-' + + self.name = name + self.display_name = display_name + self.domain_name = domain_name + self.object_type = object_type + self.uri = uri + self.anchor = anchor + self.priority = priority + + def format(self) -> str: + return (f'{self.name} {self.domain_name}:{self.object_type} ' + f'{self.priority} {self.uri} {self.display_name}\n') + + def __repr__(self) -> str: + fields = (f'{attr}={getattr(self, attr)!r}' for attr in self.__slots__) + return f"{self.__class__.__name__}({', '.join(fields)})" + + +class IntersphinxProject: + def __init__( + self, + name: str, + version: Any, + baseurl: str = '', + baseuri: str = '', + file: str | None = None, + ) -> None: + #: The project name. + self.name = name + #: The escaped project name. + self.safe_name = re.sub(r'\\s+', ' ', name) + + #: The project version as a string. + self.version = version = str(version) + #: The escaped project version. + self.safe_version = re.sub(r'\\s+', ' ', version) + + #: The project base URL (e.g., http://localhost:8080). + self.baseurl = baseurl + #: The project base URI, relative to *baseurl* (e.g., 'foo'). + self.uri = baseuri + #: The project URL, as specified in :confval:`intersphinx_mapping`. + self.url = posixpath.join(baseurl, baseuri) + #: The project local file, if any. + self.file = file + + @property + def record(self) -> dict[str, tuple[str | None, str | None]]: + """The :confval:`intersphinx_mapping` record for this project.""" + return {self.name: (self.url, self.file)} + + def normalize(self, entry: InventoryEntry) -> tuple[str, InventoryItem]: + url = posixpath.join(self.url, entry.uri) + return entry.name, (self.safe_name, self.safe_version, url, entry.display_name) + + +class FakeInventory: + protocol_version: int + + def __init__(self, project: IntersphinxProject) -> None: + self.project = project + + def serialize(self, entries: Iterable[InventoryEntry]) -> bytes: + buffer = BytesIO() + self._write_headers(buffer) + self._write_body(buffer, (item.format().encode() for item in entries)) + return buffer.getvalue() + + def _write_headers(self, buffer: BinaryIO) -> None: + buffer.write((f'# Sphinx inventory version {self.protocol_version}\n' + f'# Project: {self.project.safe_name}\n' + f'# Version: {self.project.safe_version}\n').encode()) + + def _write_body(self, buffer: BinaryIO, lines: Iterable[bytes]) -> None: + raise NotImplementedError + + +class FakeInventoryV2(FakeInventory): + protocol_version = 2 + + def _write_headers(self, buffer: BinaryIO) -> None: + super()._write_headers(buffer) + buffer.write(b'# The remainder of this file is compressed using zlib.\n') + + def _write_body(self, buffer: BinaryIO, lines: Iterable[bytes]) -> None: + compressor = zlib.compressobj(9) + buffer.writelines(map(compressor.compress, lines)) + buffer.write(compressor.flush()) + def fake_node(domain, type, target, content, **attrs): contnode = nodes.emphasis(content, content) @@ -432,26 +556,36 @@ def test_load_mappings_fallback(tmp_path, app, status, warning): @pytest.mark.sphinx('dummy', testroot='basic') def test_load_mappings_cache_update(make_app, app_params): - def make_invdata(i: int) -> bytes: - headers = f'''\ -# Sphinx inventory version 2 -# Project: foo -# Version: {i} -# The remainder of this file is compressed with zlib. -'''.encode() - line = f'module{i} py:module 0 foo.html#module-$ -\n'.encode() - return headers + zlib.compress(line) + ITEM_NAME = 'bar' + DOMAIN_NAME = 'py' + OBJECT_TYPE = 'module' + REFTYPE = f'{DOMAIN_NAME}:{OBJECT_TYPE}' + + PROJECT_NAME, PROJECT_BASEURL = 'foo', 'http://localhost:7777' + old_project = IntersphinxProject(PROJECT_NAME, 1337, PROJECT_BASEURL, 'old') + assert old_project.name == PROJECT_NAME + assert old_project.url == 'http://localhost:7777/old' + + new_project = IntersphinxProject(PROJECT_NAME, 1701, PROJECT_BASEURL, 'new') + assert new_project.name == PROJECT_NAME + assert new_project.url == 'http://localhost:7777/new' + + def make_entry(project: IntersphinxProject) -> InventoryEntry: + name = f'{ITEM_NAME}_{project.version}' + return InventoryEntry(name, domain_name=DOMAIN_NAME, object_type=OBJECT_TYPE) + + def make_invdata(project: IntersphinxProject) -> bytes: + return FakeInventoryV2(project).serialize([make_entry(project)]) class InventoryHandler(http.server.BaseHTTPRequestHandler): def do_GET(self): self.send_response(200, 'OK') - if self.path.startswith('/old/'): - data = make_invdata(1) - elif self.path.startswith('/new/'): - data = make_invdata(2) - else: - data = b'' + data = b'' + for project in (old_project, new_project): + if self.path.startswith(f'/{project.uri}/'): + data = make_invdata(project) + break self.send_header('Content-Length', str(len(data))) self.end_headers() @@ -466,43 +600,43 @@ def log_message(*args, **kwargs): _.build() baseconfig = {'extensions': ['sphinx.ext.intersphinx']} - url_old = 'http://localhost:7777/old' - url_new = 'http://localhost:7777/new' - with http_server(InventoryHandler): - confoverrides1 = baseconfig | {'intersphinx_mapping': {'foo': (url_old, None)}} + with (http_server(InventoryHandler)): + confoverrides1 = baseconfig | {'intersphinx_mapping': old_project.record} app1 = make_app(*args, confoverrides=confoverrides1, **kwargs) app1.build() + # the inventory when querying the 'old' URL - entry1 = {'module1': ('foo', '1', f'{url_old}/foo.html#module-module1', '-')} + old_entry = make_entry(old_project) + old_item = dict([old_project.normalize(old_entry)]) - assert list(app1.env.intersphinx_cache) == [url_old] - assert app1.env.intersphinx_cache[url_old][0] == 'foo' - assert app1.env.intersphinx_cache[url_old][2] == {'py:module': entry1} - assert app1.env.intersphinx_named_inventory == {'foo': {'py:module': entry1}} + assert list(app1.env.intersphinx_cache) == [old_project.url] + assert app1.env.intersphinx_cache[old_project.url][0] == old_project.name + assert app1.env.intersphinx_cache[old_project.url][2] == {REFTYPE: old_item} + assert app1.env.intersphinx_named_inventory == {old_project.name: {REFTYPE: old_item}} # switch to new url and assert that the old URL is no more stored - confoverrides2 = baseconfig | {'intersphinx_mapping': {'foo': (url_new, None)}} + confoverrides2 = baseconfig | {'intersphinx_mapping': new_project.record} app2 = make_app(*args, confoverrides=confoverrides2, **kwargs) app2.build() - entry2 = {'module2': ('foo', '2', f'{url_new}/foo.html#module-module2', '-')} - assert list(app2.env.intersphinx_cache) == [url_new] - assert app2.env.intersphinx_cache[url_new][0] == 'foo' - assert app2.env.intersphinx_cache[url_new][2] == {'py:module': entry2} - assert app2.env.intersphinx_named_inventory == {'foo': {'py:module': entry2}} + new_entry = make_entry(new_project) + new_item = dict([new_project.normalize(new_entry)]) + + assert list(app2.env.intersphinx_cache) == [new_project.url] + assert app2.env.intersphinx_cache[new_project.url][0] == PROJECT_NAME + assert app2.env.intersphinx_cache[new_project.url][2] == {REFTYPE: new_item} + assert app2.env.intersphinx_named_inventory == {PROJECT_NAME: {REFTYPE: new_item}} - # switch back to old url - confoverrides3 = baseconfig | {'intersphinx_mapping': {'foo': (url_old, None)}} + # switch back to old url (re-use 'old_item') + confoverrides3 = baseconfig | {'intersphinx_mapping': old_project.record} app3 = make_app(*args, confoverrides=confoverrides3, **kwargs) app3.build() - # same as entry 1 - entry3 = {'module1': ('foo', '1', f'{url_old}/foo.html#module-module1', '-')} - assert list(app3.env.intersphinx_cache) == [url_old] - assert app3.env.intersphinx_cache[url_old][0] == 'foo' - assert app3.env.intersphinx_cache[url_old][2] == {'py:module': entry3} - assert app3.env.intersphinx_named_inventory == {'foo': {'py:module': entry3}} + assert list(app3.env.intersphinx_cache) == [old_project.url] + assert app3.env.intersphinx_cache[old_project.url][0] == PROJECT_NAME + assert app3.env.intersphinx_cache[old_project.url][2] == {REFTYPE: old_item} + assert app3.env.intersphinx_named_inventory == {PROJECT_NAME: {REFTYPE: old_item}} class TestStripBasicAuth: From 822aa881cd2afe255a6b8015816897c01869cfa1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?B=C3=A9n=C3=A9dikt=20Tran?= <10796600+picnixz@users.noreply.github.com> Date: Wed, 14 Feb 2024 17:27:13 +0100 Subject: [PATCH 07/52] remove CHANGELOG entry until 8.x --- CHANGES.rst | 3 --- 1 file changed, 3 deletions(-) diff --git a/CHANGES.rst b/CHANGES.rst index 5bdfde97df9..2652542d4ad 100644 --- a/CHANGES.rst +++ b/CHANGES.rst @@ -77,9 +77,6 @@ Bugs fixed * #11925: Blacklist the ``sphinxprettysearchresults`` extension; the functionality it provides was merged into Sphinx v2.0.0. Patch by James Addison. -* #11466: intersphinx: fix cache loading when the inventory URI is changed - in an incremental build. - Patch by Bénédikt Tran. Testing ------- From 6d606652c2c5a8bc00052e78eddc54e479018fe6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?B=C3=A9n=C3=A9dikt=20Tran?= <10796600+picnixz@users.noreply.github.com> Date: Wed, 14 Feb 2024 17:41:34 +0100 Subject: [PATCH 08/52] implement intersphinx new format --- sphinx/ext/intersphinx.py | 314 ++++++++++-------- tests/test_domains/test_domain_c.py | 2 +- tests/test_domains/test_domain_cpp.py | 2 +- .../test_ext_inheritance_diagram.py | 4 +- tests/test_extensions/test_ext_intersphinx.py | 157 +++++++-- 5 files changed, 314 insertions(+), 165 deletions(-) diff --git a/sphinx/ext/intersphinx.py b/sphinx/ext/intersphinx.py index 0262f94899b..ff5ec9e919b 100644 --- a/sphinx/ext/intersphinx.py +++ b/sphinx/ext/intersphinx.py @@ -24,8 +24,9 @@ import re import sys import time -from itertools import chain +from operator import itemgetter from os import path +from threading import RLock from typing import TYPE_CHECKING, cast from urllib.parse import urlsplit, urlunsplit @@ -45,10 +46,11 @@ if TYPE_CHECKING: from collections.abc import Iterable from types import ModuleType - from typing import IO, Any, Union + from typing import IO, Any from docutils.nodes import Node, TextElement, system_message from docutils.utils import Reporter + from typing_extensions import TypeAlias from sphinx.application import Sphinx from sphinx.config import Config @@ -56,10 +58,24 @@ from sphinx.environment import BuildEnvironment from sphinx.util.typing import Inventory, InventoryItem, RoleFunction - InventoryCacheEntry = tuple[Union[str, None], int, Inventory] + #: The inventory external URL. + #: + #: This value is unique in :confval:`intersphinx_mapping`. + InventoryURI: TypeAlias = str + + #: The inventory name. It is unique and in one-to-one correspondence + #: with an inventory remote URL. + InventoryName: TypeAlias = str + + #: The different targets containing the inventory data. When falsy, + #: this indicates the default inventory file. + InventoryLocation: TypeAlias = str | None + + InventoryCacheEntry: TypeAlias = tuple[InventoryName, int, Inventory] logger = logging.getLogger(__name__) +_THREAD_RLOCK = RLock() class InventoryAdapter: """Inventory adapter for environment""" @@ -75,13 +91,13 @@ def __init__(self, env: BuildEnvironment) -> None: self.env.intersphinx_named_inventory = {} # type: ignore[attr-defined] @property - def cache(self) -> dict[str, InventoryCacheEntry]: + def cache(self) -> dict[InventoryURI, InventoryCacheEntry]: """Intersphinx cache. - Key is the URI of the remote inventory - Element one is the key given in the Sphinx intersphinx_mapping configuration value - - Element two is a time value for cache invalidation, a float + - Element two is a time value for cache invalidation, an integer. - Element three is the loaded remote inventory, type Inventory """ return self.env.intersphinx_cache # type: ignore[attr-defined] @@ -210,128 +226,149 @@ def fetch_inventory(app: Sphinx, uri: str, inv: str) -> Inventory: def fetch_inventory_group( - name: str | None, - uri: str, - invs: tuple[str | None, ...], - cache: dict[str, InventoryCacheEntry], + name: InventoryName, + uri: InventoryURI, + locations: tuple[InventoryLocation, ...], + cache: dict[InventoryURI, InventoryCacheEntry], app: Sphinx, now: int, ) -> bool: cache_time = now - app.config.intersphinx_cache_limit * 86400 + + def should_store(uri: str, inv: str) -> bool: + # decide whether the inventory must be read: always read local + # files; remote ones only if the cache time is expired + return '://' not in inv or uri not in cache or cache[uri][1] < cache_time + + updated = False failures = [] - try: - for inv in invs: - if not inv: - inv = posixpath.join(uri, INVENTORY_FILENAME) - # decide whether the inventory must be read: always read local - # files; remote ones only if the cache time is expired - if '://' not in inv or uri not in cache or cache[uri][1] < cache_time: - safe_inv_url = _get_safe_url(inv) - logger.info(__('loading intersphinx inventory from %s...'), safe_inv_url) - try: - invdata = fetch_inventory(app, uri, inv) - except Exception as err: - failures.append(err.args) - continue - if invdata: - old_uris = set() - # Find the URIs of the *existing* projects that are to - # be removed because there are being overwritten. - # - # If the current cache contains some (project, uri) pair - # say ("foo", "foo.com") and if the new intersphinx dict - # contains the pair ("bar", "foo.com"), we need to remove - # the first entry and use the second one. - for old_uri, (old_name, _expiry, _data) in cache.items(): - if old_name == name: - old_uris.add(old_uri) - - for old_uri in old_uris: - del cache[old_uri] - - if uri in cache: - # ensure that the cache data is moved to the end - # when setting `cache[uri] = ...` - del cache[uri] - - cache[uri] = name, now, invdata - return True - return False - finally: - if not failures: - pass - elif len(failures) < len(invs): - logger.info(__("encountered some issues with some of the inventories," - " but they had working alternatives:")) - for fail in failures: - logger.info(*fail) - else: - issues = '\n'.join(f[0] % f[1:] for f in failures) - logger.warning(__("failed to reach any of the inventories " - "with the following issues:") + "\n" + issues) + + for location in locations: + inv: str = location or posixpath.join(uri, INVENTORY_FILENAME) + with _THREAD_RLOCK: + if not should_store(uri, inv): + continue + + safe_inv_url = _get_safe_url(inv) + logger.info(__('loading intersphinx inventory from %s...'), safe_inv_url) + + try: + invdata = fetch_inventory(app, uri, inv) + except Exception as err: + failures.append(err.args) + continue + + # If the current cache contains some (project, uri) pair + # say ("foo", "foo.com") and if the new intersphinx dict + # contains the pair ("foo", "bar.com"), we need to remove + # the ("foo", "foo.com") entry and use ("foo", "bar.com"). + # + # While URIs are unique in :confval:`intersphinx_mapping`, + # they might be unique when we compare them to the cached + # version. Consider the following :confval:`intersphinx_mapping`: + # + # (1) {"foo": (URI_FOO, None)} + # (2) {"foo": (URI_BAR, None), "bar": (URI_FOO, NONE)} + # (3) {"foo": (URI_FOO, None)} + # + # We expect the cache to evolve as follows: + # + # t=0: cache(conf=1) = {URI_FOO: "foo"} + # t=1: cache(conf=2) = {URI_BAR: "foo", URI_FOO: "bar"} + # t=2: cache(conf=3) = {URI_BAR: "foo"} + # t=3: cache(conf=1) = {URI_FOO: "foo"} + # + # If we use a naive update, we might have the following execution: + # + # * t = 0, conf = 1 + # + # initial cache = {} + # after foo: cache = {URI_FOO: "foo"} + # + # * t = 1, conf = 2 + # + # initial cache = {URI_FOO: "foo"} + # after foo: cache = {URI_FOO: "foo", URI_BAR: "foo"} + # after bar: cache = {URI_FOO: "bar", URI_BAR: "foo"} + # + # * t = 2, conf = 3 + # + # initial cache = {URI_FOO: "bar", URI_BAR: "foo"} + # + # expect result: cache = {URI_FOO: "foo", URI_BAR: "foo"} + # actual result: cache = {URI_FOO: "bar", URI_BAR: "foo"} + # + # In this last case, the issue is because URI_FOO was already + # updated by the previous build, and thus might not considered + # to be expired. + # + # A viable solution is as follows: + # + # * Before doing anything, we store a copy of the current cache. + # * We only keep the URIs that are known to be updated for this + # build, namely, only those in :confval:`intersphinx_mapping`. + # * If some URIs have been associated with an other project, then + # we remove that entry from the cache but will restore it if + # a failure occurs. + + if invdata: + cache[uri] = name, now, invdata + updated = True + break + + if not failures: + pass + elif len(failures) < len(locations): + logger.info(__("encountered some issues with some of the inventories," + " but they had working alternatives:")) + for fail in failures: + logger.info(*fail) + else: + issues = '\n'.join(f[0] % f[1:] for f in failures) + logger.warning(__("failed to reach any of the inventories " + "with the following issues:") + "\n" + issues) + return updated def load_mappings(app: Sphinx) -> None: - """Load all intersphinx mappings into the environment.""" + """Load the (normalized) intersphinx mappings into the environment.""" now = int(time.time()) inventories = InventoryAdapter(app.builder.env) - intersphinx_cache: dict[str, InventoryCacheEntry] = inventories.cache + intersphinx_cache: dict[InventoryURI, InventoryCacheEntry] = inventories.cache + intersphinx_mapping = app.config.intersphinx_mapping + + expected_uris = {uri for uri, _invs in app.config.intersphinx_mapping.values()} + + for uri in frozenset(intersphinx_cache): + if intersphinx_cache[uri][0] not in intersphinx_mapping or uri not in expected_uris: + # remove a cached inventory if the latter is no more used by intersphinx + del intersphinx_cache[uri] with concurrent.futures.ThreadPoolExecutor() as pool: - futures = [] - name: str | None - uri: str - invs: tuple[str | None, ...] - for name, (uri, invs) in app.config.intersphinx_mapping.values(): - futures.append(pool.submit( - fetch_inventory_group, name, uri, invs, intersphinx_cache, app, now, - )) + futures = [ + pool.submit(fetch_inventory_group, name, uri, invs, intersphinx_cache, app, now) + for name, (uri, invs) in app.config.intersphinx_mapping.values() + ] updated = [f.result() for f in concurrent.futures.as_completed(futures)] if any(updated): + # clear the local inventories inventories.clear() # Duplicate values in different inventories will shadow each - # other; which one will override which may vary between builds, - # but we can ensure using the latest inventory data. - # - # When we encounter a named inventory that already exists, - # this means that we had two entries for the same inventory, - # but with different URIs (e.g., the remote URL was updated). - # - # In particular, the newest URI is inserted in the cache *after* - # the one that existed from a previous build (dict are ordered - # by insertion time since Python 3.6). - # - # Example: assume that we use URI_A to generate the inventory of "foo", - # so that we have + # other and which one will override which varies between builds. # - # intersphinx_cache = {URI_A: ('foo', timeout, DATA_FROM_URI1)} - # - # If we rebuild the project but change URI_A to URI_B while keeping - # the build directory (i.e. incremental build), the cache becomes - # - # intersphinx_cache = {URI_A: ('foo', ..., DATA_FROM_URI_A), - # URI_B: ('foo', ..., DATA_FROM_URI_B)} - # - # So if we iterate the *values* of ``intersphinx_cache`` and not - # the keys, we correctly replace old (in time) inventory cache data - # with the same name but different URIs (and/or data). If the URI is - # the same or if we force-reload the cache, only the data is updated. - named_vals: dict[str, Inventory] = {} - unnamed_vals: list[tuple[str | None, Inventory]] = [] - for name, _expiry, invdata in intersphinx_cache.values(): - if name: - named_vals[name] = invdata - else: - unnamed_vals.append((name, invdata)) - - # we do not sort the named inventories by their name so that two builds - # with same intersphinx_mapping value but ordered differently reflect - # the order correctly for reproducible builds - for name, invdata in chain(sorted(named_vals.items()), unnamed_vals): - if name: - inventories.named_inventory[name] = invdata + # We can however order the cache by URIs for reproducibility. + intersphinx_cache_values = sorted(intersphinx_cache.values(), key=itemgetter(0, 1)) + for name, _timeout, invdata in intersphinx_cache_values: + if not name: + logger.warning( + __('intersphinx cache seems corrupted, please rebuild ' + 'the project with the "-E" option (see sphinx --help)') + ) + continue + + inventories.named_inventory[name] = invdata for objtype, objects in invdata.items(): inventories.main_inventory.setdefault(objtype, {}).update(objects) @@ -692,36 +729,41 @@ def install_dispatcher(app: Sphinx, docname: str, source: list[str]) -> None: def normalize_intersphinx_mapping(app: Sphinx, config: Config) -> None: - for key, value in config.intersphinx_mapping.copy().items(): - try: - if isinstance(value, (list, tuple)): - # new format - name, (uri, inv) = key, value - if not isinstance(name, str): - logger.warning(__('intersphinx identifier %r is not string. Ignored'), - name) - config.intersphinx_mapping.pop(key) - continue - else: - # old format, no name - # xref RemovedInSphinx80Warning - name, uri, inv = None, key, value - msg = ( - "The pre-Sphinx 1.0 'intersphinx_mapping' format is " - "deprecated and will be removed in Sphinx 8. Update to the " - "current format as described in the documentation. " - f"Hint: \"intersphinx_mapping = {{'': {(uri, inv)!r}}}\"." - "https://www.sphinx-doc.org/en/master/usage/extensions/intersphinx.html#confval-intersphinx_mapping" # NoQA: E501 - ) - logger.warning(msg) + # URIs should NOT be duplicated, otherwise different builds may use + # different project names (and thus, the build are no more reproducible) + # depending on which one is inserted last in the cache. + seen: dict[InventoryURI, InventoryName] = {} + + for name, value in config.intersphinx_mapping.copy().items(): + if not isinstance(name, str): + logger.warning(__('intersphinx identifier %r is not string. Ignored'), name) + del config.intersphinx_mapping[name] + continue - if not isinstance(inv, tuple): - config.intersphinx_mapping[key] = (name, (uri, (inv,))) - else: - config.intersphinx_mapping[key] = (name, (uri, inv)) + if not isinstance(value, (tuple, list)): + logger.warning(__('intersphinx_mapping[%r]: invalid format. Ignored'), name) + del config.intersphinx_mapping[name] + continue + + try: + uri, inv = value except Exception as exc: - logger.warning(__('Failed to read intersphinx_mapping[%s], ignored: %r'), key, exc) - config.intersphinx_mapping.pop(key) + logger.warning(__('Failed to read intersphinx_mapping[%s], ignored: %r'), name, exc) + del config.intersphinx_mapping[name] + continue + + if (name_for_uri := seen.setdefault(uri, name)) != name: + logger.warning( + __('conflicting URI %r for intersphinx_mapping[%r] and intersphinx_mapping[%r]'), + uri, name, name_for_uri, + ) + del config.intersphinx_mapping[name] + continue + + if isinstance(inv, (tuple, list)): + config.intersphinx_mapping[name] = (name, (uri, tuple(inv))) + else: + config.intersphinx_mapping[name] = (name, (uri, (inv,))) def setup(app: Sphinx) -> dict[str, Any]: diff --git a/tests/test_domains/test_domain_c.py b/tests/test_domains/test_domain_c.py index 5839bac12ff..3f807997f50 100644 --- a/tests/test_domains/test_domain_c.py +++ b/tests/test_domains/test_domain_c.py @@ -771,7 +771,7 @@ def test_domain_c_build_intersphinx(tmp_path, app, status, warning): _var c:member 1 index.html#c.$ - ''')) # NoQA: W291 app.config.intersphinx_mapping = { - 'https://localhost/intersphinx/c/': str(inv_file), + 'c': ('https://localhost/intersphinx/c/', str(inv_file)), } app.config.intersphinx_cache_limit = 0 # load the inventory and check if it's done correctly diff --git a/tests/test_domains/test_domain_cpp.py b/tests/test_domains/test_domain_cpp.py index 23a134b9d3f..d89093c6bbe 100644 --- a/tests/test_domains/test_domain_cpp.py +++ b/tests/test_domains/test_domain_cpp.py @@ -1426,7 +1426,7 @@ def test_domain_cpp_build_intersphinx(tmp_path, app, status, warning): _var cpp:member 1 index.html#_CPPv44$ - ''')) # NoQA: W291 app.config.intersphinx_mapping = { - 'https://localhost/intersphinx/cpp/': str(inv_file), + 'cpp': ('https://localhost/intersphinx/cpp/', str(inv_file)), } app.config.intersphinx_cache_limit = 0 # load the inventory and check if it's done correctly diff --git a/tests/test_extensions/test_ext_inheritance_diagram.py b/tests/test_extensions/test_ext_inheritance_diagram.py index c13ccea9247..e82b5cc8cfc 100644 --- a/tests/test_extensions/test_ext_inheritance_diagram.py +++ b/tests/test_extensions/test_ext_inheritance_diagram.py @@ -153,9 +153,7 @@ def new_run(self): def test_inheritance_diagram_png_html(tmp_path, app): inv_file = tmp_path / 'inventory' inv_file.write_bytes(external_inventory) - app.config.intersphinx_mapping = { - 'https://example.org': str(inv_file), - } + app.config.intersphinx_mapping = {'example': ('https://example.com', str(inv_file))} app.config.intersphinx_cache_limit = 0 normalize_intersphinx_mapping(app, app.config) load_mappings(app) diff --git a/tests/test_extensions/test_ext_intersphinx.py b/tests/test_extensions/test_ext_intersphinx.py index fed5b761f8d..2e660248c61 100644 --- a/tests/test_extensions/test_ext_intersphinx.py +++ b/tests/test_extensions/test_ext_intersphinx.py @@ -2,12 +2,13 @@ from __future__ import annotations +import contextlib import http.server import posixpath import re import zlib from io import BytesIO -from typing import TYPE_CHECKING +from typing import TYPE_CHECKING, NamedTuple from unittest import mock import pytest @@ -30,7 +31,7 @@ from tests.utils import http_server if TYPE_CHECKING: - from collections.abc import Iterable + from collections.abc import Generator, Iterable from typing import Any, BinaryIO from sphinx.util.typing import InventoryItem @@ -43,7 +44,7 @@ class InventoryEntry: def __init__( self, - name: str, + name: str = 'this', *, display_name: str | None = None, domain_name: str = 'py', @@ -81,8 +82,8 @@ def __repr__(self) -> str: class IntersphinxProject: def __init__( self, - name: str, - version: Any, + name: str = 'foo', + version: str | int = 1, baseurl: str = '', baseuri: str = '', file: str | None = None, @@ -119,12 +120,13 @@ def normalize(self, entry: InventoryEntry) -> tuple[str, InventoryItem]: class FakeInventory: protocol_version: int - def __init__(self, project: IntersphinxProject) -> None: - self.project = project + def __init__(self, project: IntersphinxProject | None = None) -> None: + self.project = project or IntersphinxProject() - def serialize(self, entries: Iterable[InventoryEntry]) -> bytes: + def serialize(self, entries: Iterable[InventoryEntry] | None = None) -> bytes: buffer = BytesIO() self._write_headers(buffer) + entries = entries or [InventoryEntry()] self._write_body(buffer, (item.format().encode() for item in entries)) return buffer.getvalue() @@ -172,6 +174,32 @@ def set_config(app, mapping): app.config.intersphinx_disabled_reftypes = [] +# TODO(picnixz): investigate why 'caplog' fixture does not work +@mock.patch("sphinx.ext.intersphinx.logger") +def test_normalize_intersphinx_mapping(logger): + app = mock.Mock() + app.config.intersphinx_mapping = { + 'uri': None, # invalid format + 'foo': ('foo.com', None), # valid format + 'dup': ('foo.com', None), # duplicated URI + 'bar': ['bar.com', None], # uses [...] instead of (...) (OK) + } + + normalize_intersphinx_mapping(app, app.config) + + assert app.config.intersphinx_mapping == { + 'foo': ('foo', ('foo.com', (None,))), + 'bar': ('bar', ('bar.com', (None,))), + } + + assert len(logger.method_calls) == 2 + args = logger.method_calls[0].args + assert "intersphinx_mapping['uri']: invalid format. Ignored" == args[0] % args[1:] + args = logger.method_calls[1].args + assert ("conflicting URI 'foo.com' for intersphinx_mapping['dup'] " + "and intersphinx_mapping['foo']") == args[0] % args[1:] + + @mock.patch('sphinx.ext.intersphinx.InventoryFile') @mock.patch('sphinx.ext.intersphinx._read_from_url') def test_fetch_inventory_redirection(_read_from_url, InventoryFile, app, status, warning): # NoQA: PT019 @@ -220,7 +248,7 @@ def test_missing_reference(tmp_path, app, status, warning): inv_file = tmp_path / 'inventory' inv_file.write_bytes(inventory_v2) set_config(app, { - 'https://docs.python.org/': str(inv_file), + 'python': ('https://docs.python.org/', str(inv_file)), 'py3k': ('https://docs.python.org/py3k/', str(inv_file)), 'py3krel': ('py3k', str(inv_file)), # relative path 'py3krelparent': ('../../py3k', str(inv_file)), # relative path, parent dir @@ -297,9 +325,7 @@ def test_missing_reference(tmp_path, app, status, warning): def test_missing_reference_pydomain(tmp_path, app, status, warning): inv_file = tmp_path / 'inventory' inv_file.write_bytes(inventory_v2) - set_config(app, { - 'https://docs.python.org/': str(inv_file), - }) + set_config(app, {'python': ('https://docs.python.org/', str(inv_file))}) # load the inventory and check if it's done correctly normalize_intersphinx_mapping(app, app.config) @@ -368,9 +394,7 @@ def test_missing_reference_stddomain(tmp_path, app, status, warning): def test_missing_reference_cppdomain(tmp_path, app, status, warning): inv_file = tmp_path / 'inventory' inv_file.write_bytes(inventory_v2) - set_config(app, { - 'https://docs.python.org/': str(inv_file), - }) + set_config(app, {'python': ('https://docs.python.org/', str(inv_file))}) # load the inventory and check if it's done correctly normalize_intersphinx_mapping(app, app.config) @@ -394,9 +418,7 @@ def test_missing_reference_cppdomain(tmp_path, app, status, warning): def test_missing_reference_jsdomain(tmp_path, app, status, warning): inv_file = tmp_path / 'inventory' inv_file.write_bytes(inventory_v2) - set_config(app, { - 'https://docs.python.org/': str(inv_file), - }) + set_config(app, {'python': ('https://docs.python.org/', str(inv_file))}) # load the inventory and check if it's done correctly normalize_intersphinx_mapping(app, app.config) @@ -480,9 +502,7 @@ def assert_(rn, expected): def test_inventory_not_having_version(tmp_path, app, status, warning): inv_file = tmp_path / 'inventory' inv_file.write_bytes(inventory_v2_not_having_version) - set_config(app, { - 'https://docs.python.org/': str(inv_file), - }) + set_config(app, {'python': ('https://docs.python.org/', str(inv_file))}) # load the inventory and check if it's done correctly normalize_intersphinx_mapping(app, app.config) @@ -503,7 +523,7 @@ def test_load_mappings_warnings(tmp_path, app, status, warning): inv_file = tmp_path / 'inventory' inv_file.write_bytes(inventory_v2) set_config(app, { - 'https://docs.python.org/': str(inv_file), + 'http://example.com': str(inv_file), 'py3k': ('https://docs.python.org/py3k/', str(inv_file)), 'repoze.workflow': ('https://docs.repoze.org/workflow/', str(inv_file)), 'django-taggit': ('https://django-taggit.readthedocs.org/en/latest/', @@ -516,7 +536,7 @@ def test_load_mappings_warnings(tmp_path, app, status, warning): load_mappings(app) warnings = warning.getvalue().splitlines() assert len(warnings) == 2 - assert "The pre-Sphinx 1.0 'intersphinx_mapping' format is " in warnings[0] + assert "intersphinx_mapping['http://example.com']: invalid format. Ignored" in warnings[0] assert 'intersphinx identifier 12345 is not string. Ignored' in warnings[1] @@ -601,7 +621,7 @@ def log_message(*args, **kwargs): baseconfig = {'extensions': ['sphinx.ext.intersphinx']} - with (http_server(InventoryHandler)): + with http_server(InventoryHandler): confoverrides1 = baseconfig | {'intersphinx_mapping': old_project.record} app1 = make_app(*args, confoverrides=confoverrides1, **kwargs) app1.build() @@ -639,6 +659,95 @@ def log_message(*args, **kwargs): assert app3.env.intersphinx_named_inventory == {PROJECT_NAME: {REFTYPE: old_item}} +@pytest.mark.sphinx('dummy', testroot='basic', confoverrides={ + 'extensions': ['sphinx.ext.intersphinx'], + 'intersphinx_cache_limit': 0, + 'intersphinx_mapping': { + 'a': ('http://localhost:7777/', None), + 'b': ('http://localhost:7777/', None), + 'c': ('http://localhost:7777/', None), + }, +}) +def test_intersphinx_race_condition(monkeypatch, make_app, app_params): + # group 'a', query 'a', done 'a' + wait = {'a': 0, 'b': 3, 'c': 1} + + class Operation(NamedTuple): + name: str + stack: list[str] + + def init(self, source: str) -> str: + return f'+{self.name}({source})' + + def done(self, source: str) -> str: + return f'-{self.name}({source})' + + @contextlib.contextmanager + def __call__(self, source: str) -> Generator[None, None, None]: + self.stack.append(self.init(source)) + yield + self.stack.append(self.done(source)) + + messages = [] + GROUP = Operation('group', messages) + STORE = Operation('store', messages) + + def mock_fetch_inventory_group( + name: str | None, + uri: str, + invs: tuple[str | None, ...], + cache: dict[str, Any], + app: Any, + now: int, + ) -> bool: + assert name is not None, 'old intersphinx format' + + with GROUP(name): + failures = [] + for inv in invs: + if not inv: + inv = posixpath.join(uri, INVENTORY_FILENAME) + # decide whether the inventory must be read: always read local + # files; remote ones only if the cache time is expired + if '://' not in inv or uri not in cache: + safe_inv_url = _get_safe_url(inv) + try: + invdata = fetch_inventory(app, uri, inv) + except Exception as err: + failures.append(err.args) + continue + + if invdata: + with STORE(name): + cache[uri] = name, now, invdata + return True + return False + + class InventoryHandler(http.server.BaseHTTPRequestHandler): + def do_GET(self): + self.send_response(200, 'OK') + + data = FakeInventoryV2().serialize() + self.send_header('Content-Length', str(len(data))) + self.end_headers() + self.wfile.write(data) + + def log_message(*args, **kwargs): + pass + + with http_server(InventoryHandler): + with monkeypatch.context() as m: + m.setattr('os.cpu_count', lambda: 2) + m.setattr('sphinx.ext.intersphinx.fetch_inventory_group', mock_fetch_inventory_group) + + args, kwargs = app_params + app = make_app(*args, **kwargs) + app.build() + + print(app.env.intersphinx_cache) + + + class TestStripBasicAuth: """Tests for sphinx.ext.intersphinx._strip_basic_auth()""" From 760e4fc21c4fbb47c3b0ca7d26b6fbff45f39647 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?B=C3=A9n=C3=A9dikt=20Tran?= <10796600+picnixz@users.noreply.github.com> Date: Wed, 14 Feb 2024 17:49:35 +0100 Subject: [PATCH 09/52] cleanup --- sphinx/ext/intersphinx.py | 21 ++-- tests/test_extensions/test_ext_intersphinx.py | 102 ++---------------- 2 files changed, 15 insertions(+), 108 deletions(-) diff --git a/sphinx/ext/intersphinx.py b/sphinx/ext/intersphinx.py index ff5ec9e919b..81e4b15c15b 100644 --- a/sphinx/ext/intersphinx.py +++ b/sphinx/ext/intersphinx.py @@ -26,7 +26,6 @@ import time from operator import itemgetter from os import path -from threading import RLock from typing import TYPE_CHECKING, cast from urllib.parse import urlsplit, urlunsplit @@ -75,8 +74,6 @@ logger = logging.getLogger(__name__) -_THREAD_RLOCK = RLock() - class InventoryAdapter: """Inventory adapter for environment""" @@ -245,9 +242,8 @@ def should_store(uri: str, inv: str) -> bool: for location in locations: inv: str = location or posixpath.join(uri, INVENTORY_FILENAME) - with _THREAD_RLOCK: - if not should_store(uri, inv): - continue + if not should_store(uri, inv): + continue safe_inv_url = _get_safe_url(inv) logger.info(__('loading intersphinx inventory from %s...'), safe_inv_url) @@ -364,7 +360,7 @@ def load_mappings(app: Sphinx) -> None: if not name: logger.warning( __('intersphinx cache seems corrupted, please rebuild ' - 'the project with the "-E" option (see sphinx --help)') + 'the project with the "-E" option (see sphinx --help)'), ) continue @@ -748,15 +744,16 @@ def normalize_intersphinx_mapping(app: Sphinx, config: Config) -> None: try: uri, inv = value except Exception as exc: - logger.warning(__('Failed to read intersphinx_mapping[%s], ignored: %r'), name, exc) + logger.warning( + __('Failed to read intersphinx_mapping[%s], ignored: %r'), name, exc, + ) del config.intersphinx_mapping[name] continue if (name_for_uri := seen.setdefault(uri, name)) != name: - logger.warning( - __('conflicting URI %r for intersphinx_mapping[%r] and intersphinx_mapping[%r]'), - uri, name, name_for_uri, - ) + logger.warning(__( + 'conflicting URI %r for intersphinx_mapping[%r] and intersphinx_mapping[%r]', + ), uri, name, name_for_uri) del config.intersphinx_mapping[name] continue diff --git a/tests/test_extensions/test_ext_intersphinx.py b/tests/test_extensions/test_ext_intersphinx.py index 2e660248c61..551509099cc 100644 --- a/tests/test_extensions/test_ext_intersphinx.py +++ b/tests/test_extensions/test_ext_intersphinx.py @@ -2,13 +2,12 @@ from __future__ import annotations -import contextlib import http.server import posixpath import re import zlib from io import BytesIO -from typing import TYPE_CHECKING, NamedTuple +from typing import TYPE_CHECKING from unittest import mock import pytest @@ -31,8 +30,8 @@ from tests.utils import http_server if TYPE_CHECKING: - from collections.abc import Generator, Iterable - from typing import Any, BinaryIO + from collections.abc import Iterable + from typing import BinaryIO from sphinx.util.typing import InventoryItem @@ -194,10 +193,10 @@ def test_normalize_intersphinx_mapping(logger): assert len(logger.method_calls) == 2 args = logger.method_calls[0].args - assert "intersphinx_mapping['uri']: invalid format. Ignored" == args[0] % args[1:] + assert args[0] % args[1:] == "intersphinx_mapping['uri']: invalid format. Ignored" args = logger.method_calls[1].args - assert ("conflicting URI 'foo.com' for intersphinx_mapping['dup'] " - "and intersphinx_mapping['foo']") == args[0] % args[1:] + assert args[0] % args[1:] == ("conflicting URI 'foo.com' for intersphinx_mapping['dup'] " + "and intersphinx_mapping['foo']") @mock.patch('sphinx.ext.intersphinx.InventoryFile') @@ -659,95 +658,6 @@ def log_message(*args, **kwargs): assert app3.env.intersphinx_named_inventory == {PROJECT_NAME: {REFTYPE: old_item}} -@pytest.mark.sphinx('dummy', testroot='basic', confoverrides={ - 'extensions': ['sphinx.ext.intersphinx'], - 'intersphinx_cache_limit': 0, - 'intersphinx_mapping': { - 'a': ('http://localhost:7777/', None), - 'b': ('http://localhost:7777/', None), - 'c': ('http://localhost:7777/', None), - }, -}) -def test_intersphinx_race_condition(monkeypatch, make_app, app_params): - # group 'a', query 'a', done 'a' - wait = {'a': 0, 'b': 3, 'c': 1} - - class Operation(NamedTuple): - name: str - stack: list[str] - - def init(self, source: str) -> str: - return f'+{self.name}({source})' - - def done(self, source: str) -> str: - return f'-{self.name}({source})' - - @contextlib.contextmanager - def __call__(self, source: str) -> Generator[None, None, None]: - self.stack.append(self.init(source)) - yield - self.stack.append(self.done(source)) - - messages = [] - GROUP = Operation('group', messages) - STORE = Operation('store', messages) - - def mock_fetch_inventory_group( - name: str | None, - uri: str, - invs: tuple[str | None, ...], - cache: dict[str, Any], - app: Any, - now: int, - ) -> bool: - assert name is not None, 'old intersphinx format' - - with GROUP(name): - failures = [] - for inv in invs: - if not inv: - inv = posixpath.join(uri, INVENTORY_FILENAME) - # decide whether the inventory must be read: always read local - # files; remote ones only if the cache time is expired - if '://' not in inv or uri not in cache: - safe_inv_url = _get_safe_url(inv) - try: - invdata = fetch_inventory(app, uri, inv) - except Exception as err: - failures.append(err.args) - continue - - if invdata: - with STORE(name): - cache[uri] = name, now, invdata - return True - return False - - class InventoryHandler(http.server.BaseHTTPRequestHandler): - def do_GET(self): - self.send_response(200, 'OK') - - data = FakeInventoryV2().serialize() - self.send_header('Content-Length', str(len(data))) - self.end_headers() - self.wfile.write(data) - - def log_message(*args, **kwargs): - pass - - with http_server(InventoryHandler): - with monkeypatch.context() as m: - m.setattr('os.cpu_count', lambda: 2) - m.setattr('sphinx.ext.intersphinx.fetch_inventory_group', mock_fetch_inventory_group) - - args, kwargs = app_params - app = make_app(*args, **kwargs) - app.build() - - print(app.env.intersphinx_cache) - - - class TestStripBasicAuth: """Tests for sphinx.ext.intersphinx._strip_basic_auth()""" From eea6a9ddfee136a961a649fb46374bd1b6975122 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?B=C3=A9n=C3=A9dikt=20Tran?= <10796600+picnixz@users.noreply.github.com> Date: Wed, 14 Feb 2024 17:51:32 +0100 Subject: [PATCH 10/52] cleanup 3.9 --- sphinx/ext/intersphinx.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/sphinx/ext/intersphinx.py b/sphinx/ext/intersphinx.py index 81e4b15c15b..893b51b166b 100644 --- a/sphinx/ext/intersphinx.py +++ b/sphinx/ext/intersphinx.py @@ -45,7 +45,7 @@ if TYPE_CHECKING: from collections.abc import Iterable from types import ModuleType - from typing import IO, Any + from typing import IO, Any, Optional from docutils.nodes import Node, TextElement, system_message from docutils.utils import Reporter @@ -68,12 +68,13 @@ #: The different targets containing the inventory data. When falsy, #: this indicates the default inventory file. - InventoryLocation: TypeAlias = str | None + InventoryLocation: TypeAlias = Optional[str] InventoryCacheEntry: TypeAlias = tuple[InventoryName, int, Inventory] logger = logging.getLogger(__name__) + class InventoryAdapter: """Inventory adapter for environment""" From 9cdd37395ac6ec073e62627fe35eb008128f4a32 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?B=C3=A9n=C3=A9dikt=20Tran?= <10796600+picnixz@users.noreply.github.com> Date: Wed, 14 Feb 2024 17:53:23 +0100 Subject: [PATCH 11/52] remove typing_extensions dependency --- sphinx/ext/intersphinx.py | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/sphinx/ext/intersphinx.py b/sphinx/ext/intersphinx.py index 893b51b166b..c4f70c19271 100644 --- a/sphinx/ext/intersphinx.py +++ b/sphinx/ext/intersphinx.py @@ -49,7 +49,6 @@ from docutils.nodes import Node, TextElement, system_message from docutils.utils import Reporter - from typing_extensions import TypeAlias from sphinx.application import Sphinx from sphinx.config import Config @@ -60,17 +59,17 @@ #: The inventory external URL. #: #: This value is unique in :confval:`intersphinx_mapping`. - InventoryURI: TypeAlias = str + InventoryURI = str #: The inventory name. It is unique and in one-to-one correspondence #: with an inventory remote URL. - InventoryName: TypeAlias = str + InventoryName = str #: The different targets containing the inventory data. When falsy, #: this indicates the default inventory file. - InventoryLocation: TypeAlias = Optional[str] + InventoryLocation = Optional[str] - InventoryCacheEntry: TypeAlias = tuple[InventoryName, int, Inventory] + InventoryCacheEntry = tuple[InventoryName, int, Inventory] logger = logging.getLogger(__name__) From ae80634d634e65fd2a622bbf5391a556bf7fd4b9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?B=C3=A9n=C3=A9dikt=20Tran?= <10796600+picnixz@users.noreply.github.com> Date: Wed, 14 Feb 2024 17:56:49 +0100 Subject: [PATCH 12/52] cleanup comment --- sphinx/ext/intersphinx.py | 57 +++------------------------------------ 1 file changed, 4 insertions(+), 53 deletions(-) diff --git a/sphinx/ext/intersphinx.py b/sphinx/ext/intersphinx.py index c4f70c19271..cebb6af2c99 100644 --- a/sphinx/ext/intersphinx.py +++ b/sphinx/ext/intersphinx.py @@ -254,59 +254,6 @@ def should_store(uri: str, inv: str) -> bool: failures.append(err.args) continue - # If the current cache contains some (project, uri) pair - # say ("foo", "foo.com") and if the new intersphinx dict - # contains the pair ("foo", "bar.com"), we need to remove - # the ("foo", "foo.com") entry and use ("foo", "bar.com"). - # - # While URIs are unique in :confval:`intersphinx_mapping`, - # they might be unique when we compare them to the cached - # version. Consider the following :confval:`intersphinx_mapping`: - # - # (1) {"foo": (URI_FOO, None)} - # (2) {"foo": (URI_BAR, None), "bar": (URI_FOO, NONE)} - # (3) {"foo": (URI_FOO, None)} - # - # We expect the cache to evolve as follows: - # - # t=0: cache(conf=1) = {URI_FOO: "foo"} - # t=1: cache(conf=2) = {URI_BAR: "foo", URI_FOO: "bar"} - # t=2: cache(conf=3) = {URI_BAR: "foo"} - # t=3: cache(conf=1) = {URI_FOO: "foo"} - # - # If we use a naive update, we might have the following execution: - # - # * t = 0, conf = 1 - # - # initial cache = {} - # after foo: cache = {URI_FOO: "foo"} - # - # * t = 1, conf = 2 - # - # initial cache = {URI_FOO: "foo"} - # after foo: cache = {URI_FOO: "foo", URI_BAR: "foo"} - # after bar: cache = {URI_FOO: "bar", URI_BAR: "foo"} - # - # * t = 2, conf = 3 - # - # initial cache = {URI_FOO: "bar", URI_BAR: "foo"} - # - # expect result: cache = {URI_FOO: "foo", URI_BAR: "foo"} - # actual result: cache = {URI_FOO: "bar", URI_BAR: "foo"} - # - # In this last case, the issue is because URI_FOO was already - # updated by the previous build, and thus might not considered - # to be expired. - # - # A viable solution is as follows: - # - # * Before doing anything, we store a copy of the current cache. - # * We only keep the URIs that are known to be updated for this - # build, namely, only those in :confval:`intersphinx_mapping`. - # * If some URIs have been associated with an other project, then - # we remove that entry from the cache but will restore it if - # a failure occurs. - if invdata: cache[uri] = name, now, invdata updated = True @@ -335,6 +282,10 @@ def load_mappings(app: Sphinx) -> None: expected_uris = {uri for uri, _invs in app.config.intersphinx_mapping.values()} + # If the current cache contains some (project, uri) pair + # say ("foo", "foo.com") and if the new intersphinx dict + # contains the pair ("foo", "bar.com"), we need to remove + # the ("foo", "foo.com") entry and use ("foo", "bar.com"). for uri in frozenset(intersphinx_cache): if intersphinx_cache[uri][0] not in intersphinx_mapping or uri not in expected_uris: # remove a cached inventory if the latter is no more used by intersphinx From 14836d40e0dd836be3dbccffd878fbc3cb4c4cd2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?B=C3=A9n=C3=A9dikt=20Tran?= <10796600+picnixz@users.noreply.github.com> Date: Mon, 11 Mar 2024 10:23:08 +0100 Subject: [PATCH 13/52] Add typing --- sphinx/ext/intersphinx.py | 17 +++++++++++++++-- sphinx/util/typing.py | 2 ++ 2 files changed, 17 insertions(+), 2 deletions(-) diff --git a/sphinx/ext/intersphinx.py b/sphinx/ext/intersphinx.py index 5eb8f07597d..3d044b1cd4b 100644 --- a/sphinx/ext/intersphinx.py +++ b/sphinx/ext/intersphinx.py @@ -44,7 +44,7 @@ if TYPE_CHECKING: from collections.abc import Iterable from types import ModuleType - from typing import IO, Any, Union + from typing import IO, Any, Optional from docutils.nodes import Node, TextElement, system_message from docutils.utils import Reporter @@ -55,7 +55,20 @@ from sphinx.environment import BuildEnvironment from sphinx.util.typing import Inventory, InventoryItem, RoleFunction - InventoryCacheEntry = tuple[Union[str, None], int, Inventory] + #: The inventory external URL. + #: + #: This value is unique in :confval:`intersphinx_mapping`. + InventoryURI = str + + #: The inventory name. It is unique and in one-to-one correspondence + #: with an inventory remote URL. + InventoryName = str + + #: The different targets containing the inventory data. When falsy, + #: this indicates the default inventory file. + InventoryLocation = Optional[str] + + InventoryCacheEntry = tuple[InventoryName, int, Inventory] logger = logging.getLogger(__name__) diff --git a/sphinx/util/typing.py b/sphinx/util/typing.py index 67e7fc1d3cf..67e30e3e849 100644 --- a/sphinx/util/typing.py +++ b/sphinx/util/typing.py @@ -87,6 +87,8 @@ def is_invalid_builtin_class(obj: Any) -> bool: str, # URL str, # display name ] + +# referencable role => (reference name => inventory item) Inventory = dict[str, dict[str, InventoryItem]] From bb09f20a82367c883df43a8f52f136a9835d1918 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?B=C3=A9n=C3=A9dikt=20Tran?= <10796600+picnixz@users.noreply.github.com> Date: Mon, 11 Mar 2024 10:27:40 +0100 Subject: [PATCH 14/52] apply formatter --- sphinx/ext/intersphinx.py | 81 +++++++++++++++++++++------------------ 1 file changed, 44 insertions(+), 37 deletions(-) diff --git a/sphinx/ext/intersphinx.py b/sphinx/ext/intersphinx.py index 3d044b1cd4b..9651398f675 100644 --- a/sphinx/ext/intersphinx.py +++ b/sphinx/ext/intersphinx.py @@ -250,7 +250,7 @@ def fetch_inventory_group( return True return False finally: - if failures == []: + if not failures: pass elif len(failures) < len(invs): logger.info(__("encountered some issues with some of the inventories," @@ -304,9 +304,11 @@ def load_mappings(app: Sphinx) -> None: inventories.main_inventory.setdefault(type, {}).update(objects) -def _create_element_from_result(domain: Domain, inv_name: str | None, - data: InventoryItem, - node: pending_xref, contnode: TextElement) -> nodes.reference: +def _create_element_from_result( + domain: Domain, inv_name: str | None, + data: InventoryItem, + node: pending_xref, contnode: TextElement, +) -> nodes.reference: proj, version, uri, dispname = data if '://' not in uri and node.get('refdoc'): # get correct path in case of subdirectories @@ -319,8 +321,7 @@ def _create_element_from_result(domain: Domain, inv_name: str | None, if node.get('refexplicit'): # use whatever title was given newnode.append(contnode) - elif dispname == '-' or \ - (domain.name == 'std' and node['reftype'] == 'keyword'): + elif dispname == '-' or (domain.name == 'std' and node['reftype'] == 'keyword'): # use whatever title was given, but strip prefix title = contnode.astext() if inv_name is not None and title.startswith(inv_name + ':'): @@ -335,10 +336,11 @@ def _create_element_from_result(domain: Domain, inv_name: str | None, def _resolve_reference_in_domain_by_target( - inv_name: str | None, inventory: Inventory, - domain: Domain, objtypes: Iterable[str], - target: str, - node: pending_xref, contnode: TextElement) -> nodes.reference | None: + inv_name: str | None, inventory: Inventory, + domain: Domain, objtypes: Iterable[str], + target: str, + node: pending_xref, contnode: TextElement, +) -> nodes.reference | None: for objtype in objtypes: if objtype not in inventory: # Continue if there's nothing of this kind in the inventory @@ -368,12 +370,13 @@ def _resolve_reference_in_domain_by_target( return None -def _resolve_reference_in_domain(env: BuildEnvironment, - inv_name: str | None, inventory: Inventory, - honor_disabled_refs: bool, - domain: Domain, objtypes: Iterable[str], - node: pending_xref, contnode: TextElement, - ) -> nodes.reference | None: +def _resolve_reference_in_domain( + env: BuildEnvironment, + inv_name: str | None, inventory: Inventory, + honor_disabled_refs: bool, + domain: Domain, objtypes: Iterable[str], + node: pending_xref, contnode: TextElement, +) -> nodes.reference | None: obj_types: dict[str, None] = {}.fromkeys(objtypes) # we adjust the object types for backwards compatibility @@ -411,20 +414,22 @@ def _resolve_reference_in_domain(env: BuildEnvironment, full_qualified_name, node, contnode) -def _resolve_reference(env: BuildEnvironment, inv_name: str | None, inventory: Inventory, - honor_disabled_refs: bool, - node: pending_xref, contnode: TextElement) -> nodes.reference | None: +def _resolve_reference( + env: BuildEnvironment, inv_name: str | None, inventory: Inventory, + honor_disabled_refs: bool, + node: pending_xref, contnode: TextElement, +) -> nodes.reference | None: # disabling should only be done if no inventory is given honor_disabled_refs = honor_disabled_refs and inv_name is None + intersphinx_disabled_reftypes = env.config.intersphinx_disabled_reftypes - if honor_disabled_refs and '*' in env.config.intersphinx_disabled_reftypes: + if honor_disabled_refs and '*' in intersphinx_disabled_reftypes: return None typ = node['reftype'] if typ == 'any': for domain_name, domain in env.domains.items(): - if (honor_disabled_refs - and (domain_name + ":*") in env.config.intersphinx_disabled_reftypes): + if honor_disabled_refs and f'{domain_name}:*' in intersphinx_disabled_reftypes: continue objtypes: Iterable[str] = domain.object_types.keys() res = _resolve_reference_in_domain(env, inv_name, inventory, @@ -439,8 +444,7 @@ def _resolve_reference(env: BuildEnvironment, inv_name: str | None, inventory: I if not domain_name: # only objects in domains are in the inventory return None - if honor_disabled_refs \ - and (domain_name + ":*") in env.config.intersphinx_disabled_reftypes: + if honor_disabled_refs and f'{domain_name}:*' in intersphinx_disabled_reftypes: return None domain = env.get_domain(domain_name) objtypes = domain.objtypes_for_role(typ) or () @@ -456,10 +460,11 @@ def inventory_exists(env: BuildEnvironment, inv_name: str) -> bool: return inv_name in InventoryAdapter(env).named_inventory -def resolve_reference_in_inventory(env: BuildEnvironment, - inv_name: str, - node: pending_xref, contnode: TextElement, - ) -> nodes.reference | None: +def resolve_reference_in_inventory( + env: BuildEnvironment, + inv_name: str, + node: pending_xref, contnode: TextElement, +) -> nodes.reference | None: """Attempt to resolve a missing reference via intersphinx references. Resolution is tried in the given inventory with the target as is. @@ -471,10 +476,11 @@ def resolve_reference_in_inventory(env: BuildEnvironment, False, node, contnode) -def resolve_reference_any_inventory(env: BuildEnvironment, - honor_disabled_refs: bool, - node: pending_xref, contnode: TextElement, - ) -> nodes.reference | None: +def resolve_reference_any_inventory( + env: BuildEnvironment, + honor_disabled_refs: bool, + node: pending_xref, contnode: TextElement, +) -> nodes.reference | None: """Attempt to resolve a missing reference via intersphinx references. Resolution is tried with the target as is in any inventory. @@ -484,9 +490,9 @@ def resolve_reference_any_inventory(env: BuildEnvironment, node, contnode) -def resolve_reference_detect_inventory(env: BuildEnvironment, - node: pending_xref, contnode: TextElement, - ) -> nodes.reference | None: +def resolve_reference_detect_inventory( + env: BuildEnvironment, node: pending_xref, contnode: TextElement, +) -> nodes.reference | None: """Attempt to resolve a missing reference via intersphinx references. Resolution is tried first with the target as is in any inventory. @@ -512,8 +518,9 @@ def resolve_reference_detect_inventory(env: BuildEnvironment, return res_inv -def missing_reference(app: Sphinx, env: BuildEnvironment, node: pending_xref, - contnode: TextElement) -> nodes.reference | None: +def missing_reference( + app: Sphinx, env: BuildEnvironment, node: pending_xref, contnode: TextElement, +) -> nodes.reference | None: """Attempt to resolve a missing reference via intersphinx references.""" return resolve_reference_detect_inventory(env, node, contnode) From 3466db0b0886e730b06c3dcf28426a1052d08dd5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?B=C3=A9n=C3=A9dikt=20Tran?= <10796600+picnixz@users.noreply.github.com> Date: Mon, 11 Mar 2024 12:01:29 +0100 Subject: [PATCH 15/52] deprecate intersphinx alpha format --- sphinx/ext/intersphinx.py | 163 +++++++++++++++++++++++++------------- 1 file changed, 108 insertions(+), 55 deletions(-) diff --git a/sphinx/ext/intersphinx.py b/sphinx/ext/intersphinx.py index 9651398f675..65fc39869d0 100644 --- a/sphinx/ext/intersphinx.py +++ b/sphinx/ext/intersphinx.py @@ -55,21 +55,31 @@ from sphinx.environment import BuildEnvironment from sphinx.util.typing import Inventory, InventoryItem, RoleFunction - #: The inventory external URL. + #: The inventory project URL to which links are resolved. #: #: This value is unique in :confval:`intersphinx_mapping`. InventoryURI = str - #: The inventory name. It is unique and in one-to-one correspondence - #: with an inventory remote URL. + #: The inventory (non-empty) name. + #: + #: It is unique and in bijection with an inventory remote URL. InventoryName = str - #: The different targets containing the inventory data. When falsy, - #: this indicates the default inventory file. + #: A target (local or remote) containing the inventory data to fetch. + #: + #: Empty strings are not expected and ``None`` indicates the default + #: inventory file name :data:`~sphinx.builder.html.INVENTORY_FILENAME`. InventoryLocation = Optional[str] + #: Inventory cache entry. The integer field is the cache expiration time. InventoryCacheEntry = tuple[InventoryName, int, Inventory] + #: The type of :confval:`intersphinx_mapping` *after* normalization. + IntersphinxMapping = dict[ + InventoryName, + tuple[InventoryName, tuple[InventoryURI, tuple[InventoryLocation, ...]]], + ] + logger = logging.getLogger(__name__) @@ -87,14 +97,13 @@ def __init__(self, env: BuildEnvironment) -> None: self.env.intersphinx_named_inventory = {} # type: ignore[attr-defined] @property - def cache(self) -> dict[str, InventoryCacheEntry]: + def cache(self) -> dict[InventoryURI, InventoryCacheEntry]: """Intersphinx cache. - - Key is the URI of the remote inventory - - Element one is the key given in the Sphinx intersphinx_mapping - configuration value - - Element two is a time value for cache invalidation, a float - - Element three is the loaded remote inventory, type Inventory + - Key is the URI of the remote inventory. + - Element one is the key given in the Sphinx :confval:`intersphinx_mapping`. + - Element two is a time value for cache invalidation, an integer. + - Element three is the loaded remote inventory of type :class:`!Inventory`. """ return self.env.intersphinx_cache # type: ignore[attr-defined] @@ -103,7 +112,7 @@ def main_inventory(self) -> Inventory: return self.env.intersphinx_inventory # type: ignore[attr-defined] @property - def named_inventory(self) -> dict[str, Inventory]: + def named_inventory(self) -> dict[InventoryName, Inventory]: return self.env.intersphinx_named_inventory # type: ignore[attr-defined] def clear(self) -> None: @@ -184,7 +193,7 @@ def _get_safe_url(url: str) -> str: return urlunsplit(frags) -def fetch_inventory(app: Sphinx, uri: str, inv: str) -> Inventory: +def fetch_inventory(app: Sphinx, uri: InventoryURI, inv: str) -> Inventory: """Fetch, parse and return an intersphinx inventory file.""" # both *uri* (base URI of the links to generate) and *inv* (actual # location of the inventory file) can be local or remote URIs @@ -222,10 +231,10 @@ def fetch_inventory(app: Sphinx, uri: str, inv: str) -> Inventory: def fetch_inventory_group( - name: str | None, - uri: str, - invs: tuple[str | None, ...], - cache: dict[str, InventoryCacheEntry], + name: InventoryName, + uri: InventoryURI, + invs: tuple[InventoryLocation, ...], + cache: dict[InventoryURI, InventoryCacheEntry], app: Sphinx, now: int, ) -> bool: @@ -264,17 +273,18 @@ def fetch_inventory_group( def load_mappings(app: Sphinx) -> None: - """Load all intersphinx mappings into the environment.""" + """Load all intersphinx mappings into the environment. + + The intersphinx mappings are expected to be normalized. + """ now = int(time.time()) inventories = InventoryAdapter(app.builder.env) - intersphinx_cache: dict[str, InventoryCacheEntry] = inventories.cache + intersphinx_cache: dict[InventoryURI, InventoryCacheEntry] = inventories.cache + intersphinx_mapping: IntersphinxMapping = app.config.intersphinx_mapping with concurrent.futures.ThreadPoolExecutor() as pool: futures = [] - name: str | None - uri: str - invs: tuple[str | None, ...] - for name, (uri, invs) in app.config.intersphinx_mapping.values(): + for name, (uri, invs) in intersphinx_mapping.values(): futures.append(pool.submit( fetch_inventory_group, name, uri, invs, intersphinx_cache, app, now, )) @@ -305,7 +315,7 @@ def load_mappings(app: Sphinx) -> None: def _create_element_from_result( - domain: Domain, inv_name: str | None, + domain: Domain, inv_name: InventoryName | None, data: InventoryItem, node: pending_xref, contnode: TextElement, ) -> nodes.reference: @@ -336,7 +346,7 @@ def _create_element_from_result( def _resolve_reference_in_domain_by_target( - inv_name: str | None, inventory: Inventory, + inv_name: InventoryName | None, inventory: Inventory, domain: Domain, objtypes: Iterable[str], target: str, node: pending_xref, contnode: TextElement, @@ -372,7 +382,7 @@ def _resolve_reference_in_domain_by_target( def _resolve_reference_in_domain( env: BuildEnvironment, - inv_name: str | None, inventory: Inventory, + inv_name: InventoryName | None, inventory: Inventory, honor_disabled_refs: bool, domain: Domain, objtypes: Iterable[str], node: pending_xref, contnode: TextElement, @@ -415,7 +425,8 @@ def _resolve_reference_in_domain( def _resolve_reference( - env: BuildEnvironment, inv_name: str | None, inventory: Inventory, + env: BuildEnvironment, + inv_name: InventoryName | None, inventory: Inventory, honor_disabled_refs: bool, node: pending_xref, contnode: TextElement, ) -> nodes.reference | None: @@ -456,13 +467,13 @@ def _resolve_reference( node, contnode) -def inventory_exists(env: BuildEnvironment, inv_name: str) -> bool: +def inventory_exists(env: BuildEnvironment, inv_name: InventoryName) -> bool: return inv_name in InventoryAdapter(env).named_inventory def resolve_reference_in_inventory( env: BuildEnvironment, - inv_name: str, + inv_name: InventoryName, node: pending_xref, contnode: TextElement, ) -> nodes.reference | None: """Attempt to resolve a missing reference via intersphinx references. @@ -669,36 +680,78 @@ def install_dispatcher(app: Sphinx, docname: str, source: list[str]) -> None: def normalize_intersphinx_mapping(app: Sphinx, config: Config) -> None: - for key, value in config.intersphinx_mapping.copy().items(): + # URIs should NOT be duplicated, otherwise different builds may use + # different project names (and thus, the build are no more reproducible) + # depending on which one is inserted last in the cache. + seen: dict[InventoryURI, InventoryName] = {} + + for name, value in config.intersphinx_mapping.copy().items(): + if not isinstance(name, str): + logger.warning( + __('intersphinx identifier %r is not string. Ignored'), + name, type='intersphinx', subtype='config', + ) + del config.intersphinx_mapping[name] + continue + + # ensure that intersphinx projects are always named + if not name: + logger.warning( + __('intersphinx identifier %r must be a non-empty string; ignoring.'), + name, type='intersphinx', subtype='config', + ) + del config.intersphinx_mapping[name] + continue + + if not isinstance(value, (tuple, list)): + logger.warning( + __('intersphinx_mapping[%r]: expecting a tuple or a list, got: %r; ignoring.'), + name, type(value), type='intersphinx', subtype='config', + ) + del config.intersphinx_mapping[name] + continue + try: - if isinstance(value, (list, tuple)): - # new format - name, (uri, inv) = key, value - if not isinstance(name, str): - logger.warning(__('intersphinx identifier %r is not string. Ignored'), - name) - config.intersphinx_mapping.pop(key) - continue + uri, inv = value + except ValueError as exc: + # Since ``value`` is a list or a tuple and an unpack failure raises + # a ValueError, there is no need to catch a broader exception (if + # a broader exception is raised, this is likely a bug to report). + logger.warning( + __('Failed to read intersphinx_mapping[%s], ignored: %r'), + name, exc, type='intersphinx', subtype='config', + ) + del config.intersphinx_mapping[name] + continue + + if not uri or not isinstance(uri, str): + logger.warning( + __('intersphinx_mapping[%r]: URI %r must be a non-empty string; ignoring.'), + name, uri, type='intersphinx', subtype='config', + ) + del config.intersphinx_mapping[name] + continue + + if (name_for_uri := seen.setdefault(uri, name)) != name: + logger.warning( + __('intersphinx_mapping[%r]: URI %r shadows URI for intersphinx_mapping[%r]; ' + 'ignoring.'), name, uri, name_for_uri, type='intersphinx', subtype='config', + ) + del config.intersphinx_mapping[name] + continue + + targets: list[InventoryLocation] = [] + for target in (inv if isinstance(inv, (tuple, list)) else (inv,)): + if target is None or isinstance(target, str): + targets.append(target) else: - # old format, no name - # xref RemovedInSphinx80Warning - name, uri, inv = None, key, value - msg = ( - "The pre-Sphinx 1.0 'intersphinx_mapping' format is " - "deprecated and will be removed in Sphinx 8. Update to the " - "current format as described in the documentation. " - f"Hint: \"intersphinx_mapping = {{'': {(uri, inv)!r}}}\"." - "https://www.sphinx-doc.org/en/master/usage/extensions/intersphinx.html#confval-intersphinx_mapping" # NoQA: E501 + logger.warning( + __('intersphinx_mapping[%r]: inventory location must be None or ' + 'a non-empty string, got: %r; ignoring.'), + name, target, type='intersphinx', subtype='config', ) - logger.warning(msg) - if not isinstance(inv, tuple): - config.intersphinx_mapping[key] = (name, (uri, (inv,))) - else: - config.intersphinx_mapping[key] = (name, (uri, inv)) - except Exception as exc: - logger.warning(__('Failed to read intersphinx_mapping[%s], ignored: %r'), key, exc) - config.intersphinx_mapping.pop(key) + config.intersphinx_mapping[name] = (name, (uri, tuple(targets))) def setup(app: Sphinx) -> dict[str, Any]: From 7277f719c16b570c7259f82555f7a947d52887d9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?B=C3=A9n=C3=A9dikt=20Tran?= <10796600+picnixz@users.noreply.github.com> Date: Mon, 11 Mar 2024 12:57:46 +0100 Subject: [PATCH 16/52] deprecate intersphinx alpha format --- sphinx/ext/intersphinx.py | 21 +++++++++------------ 1 file changed, 9 insertions(+), 12 deletions(-) diff --git a/sphinx/ext/intersphinx.py b/sphinx/ext/intersphinx.py index 65fc39869d0..b0faad76fd9 100644 --- a/sphinx/ext/intersphinx.py +++ b/sphinx/ext/intersphinx.py @@ -697,8 +697,8 @@ def normalize_intersphinx_mapping(app: Sphinx, config: Config) -> None: # ensure that intersphinx projects are always named if not name: logger.warning( - __('intersphinx identifier %r must be a non-empty string; ignoring.'), - name, type='intersphinx', subtype='config', + __('ignoring empty intersphinx identifier'), + type='intersphinx', subtype='config', ) del config.intersphinx_mapping[name] continue @@ -706,17 +706,14 @@ def normalize_intersphinx_mapping(app: Sphinx, config: Config) -> None: if not isinstance(value, (tuple, list)): logger.warning( __('intersphinx_mapping[%r]: expecting a tuple or a list, got: %r; ignoring.'), - name, type(value), type='intersphinx', subtype='config', + name, value, type='intersphinx', subtype='config', ) del config.intersphinx_mapping[name] continue try: uri, inv = value - except ValueError as exc: - # Since ``value`` is a list or a tuple and an unpack failure raises - # a ValueError, there is no need to catch a broader exception (if - # a broader exception is raised, this is likely a bug to report). + except Exception as exc: logger.warning( __('Failed to read intersphinx_mapping[%s], ignored: %r'), name, exc, type='intersphinx', subtype='config', @@ -726,7 +723,7 @@ def normalize_intersphinx_mapping(app: Sphinx, config: Config) -> None: if not uri or not isinstance(uri, str): logger.warning( - __('intersphinx_mapping[%r]: URI %r must be a non-empty string; ignoring.'), + __('intersphinx_mapping[%r]: URI must be a non-empty string, got: %r; ignoring.'), name, uri, type='intersphinx', subtype='config', ) del config.intersphinx_mapping[name] @@ -734,7 +731,7 @@ def normalize_intersphinx_mapping(app: Sphinx, config: Config) -> None: if (name_for_uri := seen.setdefault(uri, name)) != name: logger.warning( - __('intersphinx_mapping[%r]: URI %r shadows URI for intersphinx_mapping[%r]; ' + __('intersphinx_mapping[%r]: URI %r shadows URI from intersphinx_mapping[%r]; ' 'ignoring.'), name, uri, name_for_uri, type='intersphinx', subtype='config', ) del config.intersphinx_mapping[name] @@ -742,12 +739,12 @@ def normalize_intersphinx_mapping(app: Sphinx, config: Config) -> None: targets: list[InventoryLocation] = [] for target in (inv if isinstance(inv, (tuple, list)) else (inv,)): - if target is None or isinstance(target, str): + if target is None or target and isinstance(target, str): targets.append(target) else: logger.warning( - __('intersphinx_mapping[%r]: inventory location must be None or ' - 'a non-empty string, got: %r; ignoring.'), + __('intersphinx_mapping[%r]: inventory location must ' + 'be a non-empty string or None, got: %r; ignoring.'), name, target, type='intersphinx', subtype='config', ) From a6fdcec162179f798b1462f782aefa139a3f1b14 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?B=C3=A9n=C3=A9dikt=20Tran?= <10796600+picnixz@users.noreply.github.com> Date: Mon, 11 Mar 2024 12:58:14 +0100 Subject: [PATCH 17/52] upgrade tests --- tests/test_domains/test_domain_c.py | 2 +- tests/test_domains/test_domain_cpp.py | 2 +- tests/test_extensions/test_ext_intersphinx.py | 81 +++++++++++++------ 3 files changed, 59 insertions(+), 26 deletions(-) diff --git a/tests/test_domains/test_domain_c.py b/tests/test_domains/test_domain_c.py index 5839bac12ff..0b3533d832b 100644 --- a/tests/test_domains/test_domain_c.py +++ b/tests/test_domains/test_domain_c.py @@ -771,7 +771,7 @@ def test_domain_c_build_intersphinx(tmp_path, app, status, warning): _var c:member 1 index.html#c.$ - ''')) # NoQA: W291 app.config.intersphinx_mapping = { - 'https://localhost/intersphinx/c/': str(inv_file), + 'local': ('https://localhost/intersphinx/c/', str(inv_file)), } app.config.intersphinx_cache_limit = 0 # load the inventory and check if it's done correctly diff --git a/tests/test_domains/test_domain_cpp.py b/tests/test_domains/test_domain_cpp.py index 23a134b9d3f..6900d9ee611 100644 --- a/tests/test_domains/test_domain_cpp.py +++ b/tests/test_domains/test_domain_cpp.py @@ -1426,7 +1426,7 @@ def test_domain_cpp_build_intersphinx(tmp_path, app, status, warning): _var cpp:member 1 index.html#_CPPv44$ - ''')) # NoQA: W291 app.config.intersphinx_mapping = { - 'https://localhost/intersphinx/cpp/': str(inv_file), + 'test': ('https://localhost/intersphinx/cpp/', str(inv_file)), } app.config.intersphinx_cache_limit = 0 # load the inventory and check if it's done correctly diff --git a/tests/test_extensions/test_ext_intersphinx.py b/tests/test_extensions/test_ext_intersphinx.py index 51ca7f5ed17..a7aba781e88 100644 --- a/tests/test_extensions/test_ext_intersphinx.py +++ b/tests/test_extensions/test_ext_intersphinx.py @@ -1,6 +1,8 @@ """Test the intersphinx extension.""" +from __future__ import annotations import http.server +from typing import TYPE_CHECKING from unittest import mock import pytest @@ -18,10 +20,14 @@ normalize_intersphinx_mapping, ) from sphinx.ext.intersphinx import setup as intersphinx_setup +from sphinx.testing.util import strip_escseq from tests.test_util.test_util_inventory import inventory_v2, inventory_v2_not_having_version from tests.utils import http_server +if TYPE_CHECKING: + from typing import NoReturn + def fake_node(domain, type, target, content, **attrs): contnode = nodes.emphasis(content, content) @@ -40,7 +46,8 @@ def reference_check(app, *args, **kwds): def set_config(app, mapping): - app.config.intersphinx_mapping = mapping + # copy *mapping* so that normalization does not alter it + app.config.intersphinx_mapping = dict(mapping) app.config.intersphinx_cache_limit = 0 app.config.intersphinx_disabled_reftypes = [] @@ -93,7 +100,7 @@ def test_missing_reference(tmp_path, app, status, warning): inv_file = tmp_path / 'inventory' inv_file.write_bytes(inventory_v2) set_config(app, { - 'https://docs.python.org/': str(inv_file), + 'python': ('https://docs.python.org/', str(inv_file)), 'py3k': ('https://docs.python.org/py3k/', str(inv_file)), 'py3krel': ('py3k', str(inv_file)), # relative path 'py3krelparent': ('../../py3k', str(inv_file)), # relative path, parent dir @@ -171,7 +178,7 @@ def test_missing_reference_pydomain(tmp_path, app, status, warning): inv_file = tmp_path / 'inventory' inv_file.write_bytes(inventory_v2) set_config(app, { - 'https://docs.python.org/': str(inv_file), + 'python': ('https://docs.python.org/', str(inv_file)), }) # load the inventory and check if it's done correctly @@ -252,7 +259,7 @@ def test_missing_reference_cppdomain(tmp_path, app, status, warning): inv_file = tmp_path / 'inventory' inv_file.write_bytes(inventory_v2) set_config(app, { - 'https://docs.python.org/': str(inv_file), + 'python': ('https://docs.python.org/', str(inv_file)), }) # load the inventory and check if it's done correctly @@ -278,7 +285,7 @@ def test_missing_reference_jsdomain(tmp_path, app, status, warning): inv_file = tmp_path / 'inventory' inv_file.write_bytes(inventory_v2) set_config(app, { - 'https://docs.python.org/': str(inv_file), + 'python': ('https://docs.python.org/', str(inv_file)), }) # load the inventory and check if it's done correctly @@ -364,7 +371,7 @@ def test_inventory_not_having_version(tmp_path, app, status, warning): inv_file = tmp_path / 'inventory' inv_file.write_bytes(inventory_v2_not_having_version) set_config(app, { - 'https://docs.python.org/': str(inv_file), + 'python': ('https://docs.python.org/', str(inv_file)), }) # load the inventory and check if it's done correctly @@ -378,29 +385,55 @@ def test_inventory_not_having_version(tmp_path, app, status, warning): assert rn[0].astext() == 'Long Module desc' -def test_load_mappings_warnings(tmp_path, app, status, warning): - """ - load_mappings issues a warning if new-style mapping - identifiers are not string - """ +def test_normalize_intersphinx_mapping_warnings(tmp_path, app, warning): + """Check warnings in :func:`sphinx.ext.intersphinx.normalize_intersphinx_mapping`.""" inv_file = tmp_path / 'inventory' inv_file.write_bytes(inventory_v2) - set_config(app, { - 'https://docs.python.org/': str(inv_file), - 'py3k': ('https://docs.python.org/py3k/', str(inv_file)), - 'repoze.workflow': ('https://docs.repoze.org/workflow/', str(inv_file)), - 'django-taggit': ('https://django-taggit.readthedocs.org/en/latest/', - str(inv_file)), - 12345: ('https://www.sphinx-doc.org/en/stable/', str(inv_file)), + targets = (str(inv_file),) + + class FakeList(list): + def __iter__(self) -> NoReturn: + raise NotImplementedError + + set_config(app, bad_intersphinx_mapping := { + # fmt: off + '': ('67890.net', targets), # invalid project name (value) + 12345: ('12345.net', targets), # invalid project name (type) + 'bad-dict-item': 0, # invalid dict item type + 'unpack-except-1': [0], # invalid dict item size (native ValueError) + 'unpack-except-2': FakeList(), # invalid dict item size (custom exception) + 'bad-uri-type-1': (123456789, targets), # invalid URI type + 'bad-uri-type-2': (None, targets), # invalid URI type + 'bad-uri-value': ('', targets), # invalid URI value + 'good': ('foo.com', targets), # duplicated URI (good entry) + 'dedup-good': ('foo.com', targets), # duplicated URI + 'bad-target-1': ('a.com', 1), # invalid URI type (single input, bad type) + 'bad-target-2': ('b.com', ''), # invalid URI type (single input, bad string) + 'bad-target-3': ('c.com', [2, 'x']), # invalid URI type (sequence input, bad type) + 'bad-target-4': ('d.com', ['y', '']), # invalid URI type (sequence input, bad string) + # fmt: on }) - # load the inventory and check if it's done correctly + # normalize the inventory and check if it's done correctly normalize_intersphinx_mapping(app, app.config) - load_mappings(app) - warnings = warning.getvalue().splitlines() - assert len(warnings) == 2 - assert "The pre-Sphinx 1.0 'intersphinx_mapping' format is " in warnings[0] - assert 'intersphinx identifier 12345 is not string. Ignored' in warnings[1] + warnings = strip_escseq(warning.getvalue()).splitlines() + assert len(warnings) == len(bad_intersphinx_mapping) - 1 + for index, messages in enumerate(( + "ignoring empty intersphinx identifier", + 'intersphinx identifier 12345 is not string. Ignored', + "intersphinx_mapping['bad-dict-item']: expecting a tuple or a list, got: 0; ignoring.", + "Failed to read intersphinx_mapping[unpack-except-1], ignored: ValueError('not enough values to unpack (expected 2, got 1)')", + "Failed to read intersphinx_mapping[unpack-except-2], ignored: NotImplementedError()", + "intersphinx_mapping['bad-uri-type-1']: URI must be a non-empty string, got: 123456789; ignoring.", + "intersphinx_mapping['bad-uri-type-2']: URI must be a non-empty string, got: None; ignoring.", + "intersphinx_mapping['bad-uri-value']: URI must be a non-empty string, got: ''; ignoring.", + "intersphinx_mapping['dedup-good']: URI 'foo.com' shadows URI from intersphinx_mapping['good']; ignoring.", + "intersphinx_mapping['bad-target-1']: inventory location must be a non-empty string or None, got: 1; ignoring.", + "intersphinx_mapping['bad-target-2']: inventory location must be a non-empty string or None, got: ''; ignoring.", + "intersphinx_mapping['bad-target-3']: inventory location must be a non-empty string or None, got: 2; ignoring.", + "intersphinx_mapping['bad-target-4']: inventory location must be a non-empty string or None, got: ''; ignoring.", + )): + assert messages in warnings[index] def test_load_mappings_fallback(tmp_path, app, status, warning): From 479a3fe71fc94027076e14e9ce12887b0126c10e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?B=C3=A9n=C3=A9dikt=20Tran?= <10796600+picnixz@users.noreply.github.com> Date: Mon, 11 Mar 2024 13:02:45 +0100 Subject: [PATCH 18/52] Update doc --- doc/usage/extensions/intersphinx.rst | 22 ---------------------- 1 file changed, 22 deletions(-) diff --git a/doc/usage/extensions/intersphinx.rst b/doc/usage/extensions/intersphinx.rst index 5aaaf9f6c30..ec2caaa13cc 100644 --- a/doc/usage/extensions/intersphinx.rst +++ b/doc/usage/extensions/intersphinx.rst @@ -126,28 +126,6 @@ linking: ('../../otherbook/build/html/objects.inv', None)), } - **Old format for this config value** - - .. deprecated:: 6.2 - - .. RemovedInSphinx80Warning - - .. caution:: This is the format used before Sphinx 1.0. - It is deprecated and will be removed in Sphinx 8.0. - - A dictionary mapping URIs to either ``None`` or an URI. The keys are the - base URI of the foreign Sphinx documentation sets and can be local paths or - HTTP URIs. The values indicate where the inventory file can be found: they - can be ``None`` (at the same location as the base URI) or another local or - HTTP URI. - - Example: - - .. code:: python - - intersphinx_mapping = {'https://docs.python.org/': None} - - .. confval:: intersphinx_cache_limit The maximum number of days to cache remote inventories. The default is From f90631478b316498d2d6dac684afe7f49e323beb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?B=C3=A9n=C3=A9dikt=20Tran?= <10796600+picnixz@users.noreply.github.com> Date: Thu, 14 Mar 2024 10:42:30 +0100 Subject: [PATCH 19/52] fixup --- sphinx/ext/intersphinx.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/sphinx/ext/intersphinx.py b/sphinx/ext/intersphinx.py index b0faad76fd9..c204680cf4f 100644 --- a/sphinx/ext/intersphinx.py +++ b/sphinx/ext/intersphinx.py @@ -723,7 +723,8 @@ def normalize_intersphinx_mapping(app: Sphinx, config: Config) -> None: if not uri or not isinstance(uri, str): logger.warning( - __('intersphinx_mapping[%r]: URI must be a non-empty string, got: %r; ignoring.'), + __('intersphinx_mapping[%r]: URI must be a non-empty string, ' + 'got: %r; ignoring.'), name, uri, type='intersphinx', subtype='config', ) del config.intersphinx_mapping[name] From 8c7894b73ac4f7835b11cedaec6d7673f1864fd4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?B=C3=A9n=C3=A9dikt=20Tran?= <10796600+picnixz@users.noreply.github.com> Date: Mon, 11 Mar 2024 13:02:45 +0100 Subject: [PATCH 20/52] Update doc --- doc/usage/extensions/intersphinx.rst | 22 ---------------------- 1 file changed, 22 deletions(-) diff --git a/doc/usage/extensions/intersphinx.rst b/doc/usage/extensions/intersphinx.rst index 5aaaf9f6c30..ec2caaa13cc 100644 --- a/doc/usage/extensions/intersphinx.rst +++ b/doc/usage/extensions/intersphinx.rst @@ -126,28 +126,6 @@ linking: ('../../otherbook/build/html/objects.inv', None)), } - **Old format for this config value** - - .. deprecated:: 6.2 - - .. RemovedInSphinx80Warning - - .. caution:: This is the format used before Sphinx 1.0. - It is deprecated and will be removed in Sphinx 8.0. - - A dictionary mapping URIs to either ``None`` or an URI. The keys are the - base URI of the foreign Sphinx documentation sets and can be local paths or - HTTP URIs. The values indicate where the inventory file can be found: they - can be ``None`` (at the same location as the base URI) or another local or - HTTP URI. - - Example: - - .. code:: python - - intersphinx_mapping = {'https://docs.python.org/': None} - - .. confval:: intersphinx_cache_limit The maximum number of days to cache remote inventories. The default is From 128c3249b888e5e8f56ec7bdc78ec97922545ab4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?B=C3=A9n=C3=A9dikt=20Tran?= <10796600+picnixz@users.noreply.github.com> Date: Thu, 14 Mar 2024 11:11:26 +0100 Subject: [PATCH 21/52] fixup --- tests/test_extensions/test_ext_intersphinx.py | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/tests/test_extensions/test_ext_intersphinx.py b/tests/test_extensions/test_ext_intersphinx.py index c07015ca6e3..be2f43f20b0 100644 --- a/tests/test_extensions/test_ext_intersphinx.py +++ b/tests/test_extensions/test_ext_intersphinx.py @@ -193,10 +193,11 @@ def test_normalize_intersphinx_mapping(logger): assert len(logger.method_calls) == 2 args = logger.method_calls[0].args - assert args[0] % args[1:] == "intersphinx_mapping['uri']: invalid format. Ignored" + assert args[0] % args[1:] == ("intersphinx_mapping['uri']: expecting a tuple or a list, " + "got: None; ignoring.") args = logger.method_calls[1].args - assert args[0] % args[1:] == ("conflicting URI 'foo.com' for intersphinx_mapping['dup'] " - "and intersphinx_mapping['foo']") + assert args[0] % args[1:] == ("intersphinx_mapping['dup']: URI 'foo.com' shadows URI " + "from intersphinx_mapping['foo']; ignoring.") @mock.patch('sphinx.ext.intersphinx.InventoryFile') From f824dc2fcdd92c2f870ac278a2de24e07c6f8ed7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?B=C3=A9n=C3=A9dikt=20Tran?= <10796600+picnixz@users.noreply.github.com> Date: Mon, 11 Mar 2024 10:23:08 +0100 Subject: [PATCH 22/52] deprecate intersphinx alpha format --- sphinx/ext/intersphinx/_load.py | 123 ++++++++++++++++++++--------- sphinx/ext/intersphinx/_resolve.py | 23 +++--- sphinx/ext/intersphinx/_shared.py | 42 +++++++--- sphinx/util/typing.py | 2 + 4 files changed, 131 insertions(+), 59 deletions(-) diff --git a/sphinx/ext/intersphinx/_load.py b/sphinx/ext/intersphinx/_load.py index 8e951ecd4ae..1d607adc5fa 100644 --- a/sphinx/ext/intersphinx/_load.py +++ b/sphinx/ext/intersphinx/_load.py @@ -21,55 +21,100 @@ from sphinx.application import Sphinx from sphinx.config import Config - from sphinx.ext.intersphinx._shared import InventoryCacheEntry + from sphinx.ext.intersphinx._shared import ( + IntersphinxMapping, + InventoryCacheEntry, + InventoryLocation, + InventoryName, + InventoryURI, + ) from sphinx.util.typing import Inventory def normalize_intersphinx_mapping(app: Sphinx, config: Config) -> None: - for key, value in config.intersphinx_mapping.copy().items(): + # URIs should NOT be duplicated, otherwise different builds may use + # different project names (and thus, the build are no more reproducible) + # depending on which one is inserted last in the cache. + seen: dict[InventoryURI, InventoryName] = {} + + for name, value in config.intersphinx_mapping.copy().items(): + if not isinstance(name, str): + LOGGER.warning(__('intersphinx identifier %r is not string. Ignored'), + name, type='intersphinx', subtype='config') + del config.intersphinx_mapping[name] + continue + + # ensure that intersphinx projects are always named + if not name: + LOGGER.warning( + __('ignoring empty intersphinx identifier'), + type='intersphinx', subtype='config', + ) + del config.intersphinx_mapping[name] + continue + + if not isinstance(value, (tuple, list)): + LOGGER.warning( + __('intersphinx_mapping[%r]: expecting a tuple or a list, got: %r; ignoring.'), + name, value, type='intersphinx', subtype='config', + ) + del config.intersphinx_mapping[name] + continue + try: - if isinstance(value, (list, tuple)): - # new format - name, (uri, inv) = key, value - if not isinstance(name, str): - LOGGER.warning(__('intersphinx identifier %r is not string. Ignored'), - name) - config.intersphinx_mapping.pop(key) - continue + uri, inv = value + except Exception as exc: + LOGGER.warning( + __('Failed to read intersphinx_mapping[%s], ignored: %r'), + name, exc, type='intersphinx', subtype='config', + ) + del config.intersphinx_mapping[name] + continue + + if not uri or not isinstance(uri, str): + LOGGER.warning( + __('intersphinx_mapping[%r]: URI must be a non-empty string, ' + 'got: %r; ignoring.'), + name, uri, type='intersphinx', subtype='config', + ) + del config.intersphinx_mapping[name] + continue + + if (name_for_uri := seen.setdefault(uri, name)) != name: + LOGGER.warning( + __('intersphinx_mapping[%r]: URI %r shadows URI from intersphinx_mapping[%r]; ' + 'ignoring.'), name, uri, name_for_uri, type='intersphinx', subtype='config', + ) + del config.intersphinx_mapping[name] + continue + + targets: list[InventoryLocation] = [] + for target in (inv if isinstance(inv, (tuple, list)) else (inv,)): + if target is None or target and isinstance(target, str): + targets.append(target) else: - # old format, no name - # xref RemovedInSphinx80Warning - name, uri, inv = None, key, value - msg = ( - "The pre-Sphinx 1.0 'intersphinx_mapping' format is " - 'deprecated and will be removed in Sphinx 8. Update to the ' - 'current format as described in the documentation. ' - f"Hint: `intersphinx_mapping = {{'': {(uri, inv)!r}}}`." - 'https://www.sphinx-doc.org/en/master/usage/extensions/intersphinx.html#confval-intersphinx_mapping' # NoQA: E501 + LOGGER.warning( + __('intersphinx_mapping[%r]: inventory location must ' + 'be a non-empty string or None, got: %r; ignoring.'), + name, target, type='intersphinx', subtype='config', ) - LOGGER.warning(msg) - if not isinstance(inv, tuple): - config.intersphinx_mapping[key] = (name, (uri, (inv,))) - else: - config.intersphinx_mapping[key] = (name, (uri, inv)) - except Exception as exc: - LOGGER.warning(__('Failed to read intersphinx_mapping[%s], ignored: %r'), key, exc) - config.intersphinx_mapping.pop(key) + config.intersphinx_mapping[name] = (name, (uri, tuple(targets))) def load_mappings(app: Sphinx) -> None: - """Load all intersphinx mappings into the environment.""" + """Load all intersphinx mappings into the environment. + + The intersphinx mappings are expected to be normalized. + """ now = int(time.time()) inventories = InventoryAdapter(app.builder.env) - intersphinx_cache: dict[str, InventoryCacheEntry] = inventories.cache + intersphinx_cache: dict[InventoryURI, InventoryCacheEntry] = inventories.cache + intersphinx_mapping: IntersphinxMapping = app.config.intersphinx_mapping with concurrent.futures.ThreadPoolExecutor() as pool: futures = [] - name: str | None - uri: str - invs: tuple[str | None, ...] - for name, (uri, invs) in app.config.intersphinx_mapping.values(): + for name, (uri, invs) in intersphinx_mapping.values(): futures.append(pool.submit( fetch_inventory_group, name, uri, invs, intersphinx_cache, app, now, )) @@ -100,10 +145,10 @@ def load_mappings(app: Sphinx) -> None: def fetch_inventory_group( - name: str | None, - uri: str, - invs: tuple[str | None, ...], - cache: dict[str, InventoryCacheEntry], + name: InventoryName, + uri: InventoryURI, + invs: tuple[InventoryLocation, ...], + cache: dict[InventoryURI, InventoryCacheEntry], app: Sphinx, now: int, ) -> bool: @@ -128,7 +173,7 @@ def fetch_inventory_group( return True return False finally: - if failures == []: + if not failures: pass elif len(failures) < len(invs): LOGGER.info(__('encountered some issues with some of the inventories,' @@ -141,7 +186,7 @@ def fetch_inventory_group( 'with the following issues:') + '\n' + issues) -def fetch_inventory(app: Sphinx, uri: str, inv: str) -> Inventory: +def fetch_inventory(app: Sphinx, uri: InventoryURI, inv: str) -> Inventory: """Fetch, parse and return an intersphinx inventory file.""" # both *uri* (base URI of the links to generate) and *inv* (actual # location of the inventory file) can be local or remote URIs diff --git a/sphinx/ext/intersphinx/_resolve.py b/sphinx/ext/intersphinx/_resolve.py index 08961e071b4..8482fd7ade3 100644 --- a/sphinx/ext/intersphinx/_resolve.py +++ b/sphinx/ext/intersphinx/_resolve.py @@ -28,10 +28,11 @@ from sphinx.application import Sphinx from sphinx.domains import Domain from sphinx.environment import BuildEnvironment + from sphinx.ext.intersphinx._shared import InventoryName from sphinx.util.typing import Inventory, InventoryItem, RoleFunction -def _create_element_from_result(domain: Domain, inv_name: str | None, +def _create_element_from_result(domain: Domain, inv_name: InventoryName | None, data: InventoryItem, node: pending_xref, contnode: TextElement) -> nodes.reference: proj, version, uri, dispname = data @@ -61,7 +62,7 @@ def _create_element_from_result(domain: Domain, inv_name: str | None, def _resolve_reference_in_domain_by_target( - inv_name: str | None, inventory: Inventory, + inv_name: InventoryName | None, inventory: Inventory, domain: Domain, objtypes: Iterable[str], target: str, node: pending_xref, contnode: TextElement) -> nodes.reference | None: @@ -95,7 +96,7 @@ def _resolve_reference_in_domain_by_target( def _resolve_reference_in_domain(env: BuildEnvironment, - inv_name: str | None, inventory: Inventory, + inv_name: InventoryName | None, inventory: Inventory, honor_disabled_refs: bool, domain: Domain, objtypes: Iterable[str], node: pending_xref, contnode: TextElement, @@ -137,20 +138,21 @@ def _resolve_reference_in_domain(env: BuildEnvironment, full_qualified_name, node, contnode) -def _resolve_reference(env: BuildEnvironment, inv_name: str | None, inventory: Inventory, +def _resolve_reference(env: BuildEnvironment, + inv_name: InventoryName | None, inventory: Inventory, honor_disabled_refs: bool, node: pending_xref, contnode: TextElement) -> nodes.reference | None: # disabling should only be done if no inventory is given honor_disabled_refs = honor_disabled_refs and inv_name is None + intersphinx_disabled_reftypes = env.config.intersphinx_disabled_reftypes - if honor_disabled_refs and '*' in env.config.intersphinx_disabled_reftypes: + if honor_disabled_refs and '*' in intersphinx_disabled_reftypes: return None typ = node['reftype'] if typ == 'any': for domain_name, domain in env.domains.items(): - if (honor_disabled_refs - and (domain_name + ':*') in env.config.intersphinx_disabled_reftypes): + if honor_disabled_refs and f'{domain_name}:*' in intersphinx_disabled_reftypes: continue objtypes: Iterable[str] = domain.object_types.keys() res = _resolve_reference_in_domain(env, inv_name, inventory, @@ -165,8 +167,7 @@ def _resolve_reference(env: BuildEnvironment, inv_name: str | None, inventory: I if not domain_name: # only objects in domains are in the inventory return None - if (honor_disabled_refs - and (domain_name + ':*') in env.config.intersphinx_disabled_reftypes): + if honor_disabled_refs and f'{domain_name}:*' in intersphinx_disabled_reftypes: return None domain = env.get_domain(domain_name) objtypes = domain.objtypes_for_role(typ) or () @@ -178,12 +179,12 @@ def _resolve_reference(env: BuildEnvironment, inv_name: str | None, inventory: I node, contnode) -def inventory_exists(env: BuildEnvironment, inv_name: str) -> bool: +def inventory_exists(env: BuildEnvironment, inv_name: InventoryName) -> bool: return inv_name in InventoryAdapter(env).named_inventory def resolve_reference_in_inventory(env: BuildEnvironment, - inv_name: str, + inv_name: InventoryName, node: pending_xref, contnode: TextElement, ) -> nodes.reference | None: """Attempt to resolve a missing reference via intersphinx references. diff --git a/sphinx/ext/intersphinx/_shared.py b/sphinx/ext/intersphinx/_shared.py index f2f52444b99..fed5844947d 100644 --- a/sphinx/ext/intersphinx/_shared.py +++ b/sphinx/ext/intersphinx/_shared.py @@ -2,15 +2,40 @@ from __future__ import annotations -from typing import TYPE_CHECKING, Final, Union +from typing import TYPE_CHECKING, Final from sphinx.util import logging if TYPE_CHECKING: + from typing import Optional + from sphinx.environment import BuildEnvironment from sphinx.util.typing import Inventory - InventoryCacheEntry = tuple[Union[str, None], int, Inventory] + #: The inventory project URL to which links are resolved. + #: + #: This value is unique in :confval:`intersphinx_mapping`. + InventoryURI = str + + #: The inventory (non-empty) name. + #: + #: It is unique and in bijection with an inventory remote URL. + InventoryName = str + + #: A target (local or remote) containing the inventory data to fetch. + #: + #: Empty strings are not expected and ``None`` indicates the default + #: inventory file name :data:`~sphinx.builder.html.INVENTORY_FILENAME`. + InventoryLocation = Optional[str] + + #: Inventory cache entry. The integer field is the cache expiration time. + InventoryCacheEntry = tuple[InventoryName, int, Inventory] + + #: The type of :confval:`intersphinx_mapping` *after* normalization. + IntersphinxMapping = dict[ + InventoryName, + tuple[InventoryName, tuple[InventoryURI, tuple[InventoryLocation, ...]]], + ] LOGGER: Final[logging.SphinxLoggerAdapter] = logging.getLogger('sphinx.ext.intersphinx') @@ -29,14 +54,13 @@ def __init__(self, env: BuildEnvironment) -> None: self.env.intersphinx_named_inventory = {} # type: ignore[attr-defined] @property - def cache(self) -> dict[str, InventoryCacheEntry]: + def cache(self) -> dict[InventoryURI, InventoryCacheEntry]: """Intersphinx cache. - - Key is the URI of the remote inventory - - Element one is the key given in the Sphinx intersphinx_mapping - configuration value - - Element two is a time value for cache invalidation, a float - - Element three is the loaded remote inventory, type Inventory + - Key is the URI of the remote inventory. + - Element one is the key given in the Sphinx :confval:`intersphinx_mapping`. + - Element two is a time value for cache invalidation, an integer. + - Element three is the loaded remote inventory of type :class:`!Inventory`. """ return self.env.intersphinx_cache # type: ignore[attr-defined] @@ -45,7 +69,7 @@ def main_inventory(self) -> Inventory: return self.env.intersphinx_inventory # type: ignore[attr-defined] @property - def named_inventory(self) -> dict[str, Inventory]: + def named_inventory(self) -> dict[InventoryName, Inventory]: return self.env.intersphinx_named_inventory # type: ignore[attr-defined] def clear(self) -> None: diff --git a/sphinx/util/typing.py b/sphinx/util/typing.py index c8e89ee0ad6..d57d1ea5c57 100644 --- a/sphinx/util/typing.py +++ b/sphinx/util/typing.py @@ -111,6 +111,8 @@ def is_invalid_builtin_class(obj: Any) -> bool: str, # URL str, # display name ] + +# referencable role => (reference name => inventory item) Inventory = dict[str, dict[str, InventoryItem]] From 55effd8e9485dc4da0469cacaff9a71af8511827 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?B=C3=A9n=C3=A9dikt=20Tran?= <10796600+picnixz@users.noreply.github.com> Date: Mon, 11 Mar 2024 12:58:14 +0100 Subject: [PATCH 23/52] upgrade tests --- tests/test_domains/test_domain_c.py | 2 +- tests/test_domains/test_domain_cpp.py | 2 +- tests/test_extensions/test_ext_intersphinx.py | 80 +++++++++++++------ 3 files changed, 58 insertions(+), 26 deletions(-) diff --git a/tests/test_domains/test_domain_c.py b/tests/test_domains/test_domain_c.py index a8a92cb82ae..43a3e44cc68 100644 --- a/tests/test_domains/test_domain_c.py +++ b/tests/test_domains/test_domain_c.py @@ -771,7 +771,7 @@ def test_domain_c_build_intersphinx(tmp_path, app, status, warning): _var c:member 1 index.html#c.$ - ''')) # NoQA: W291 app.config.intersphinx_mapping = { - 'https://localhost/intersphinx/c/': str(inv_file), + 'local': ('https://localhost/intersphinx/c/', str(inv_file)), } app.config.intersphinx_cache_limit = 0 # load the inventory and check if it's done correctly diff --git a/tests/test_domains/test_domain_cpp.py b/tests/test_domains/test_domain_cpp.py index abd0f822d7e..0619c091cef 100644 --- a/tests/test_domains/test_domain_cpp.py +++ b/tests/test_domains/test_domain_cpp.py @@ -1426,7 +1426,7 @@ def test_domain_cpp_build_intersphinx(tmp_path, app, status, warning): _var cpp:member 1 index.html#_CPPv44$ - ''')) # NoQA: W291 app.config.intersphinx_mapping = { - 'https://localhost/intersphinx/cpp/': str(inv_file), + 'test': ('https://localhost/intersphinx/cpp/', str(inv_file)), } app.config.intersphinx_cache_limit = 0 # load the inventory and check if it's done correctly diff --git a/tests/test_extensions/test_ext_intersphinx.py b/tests/test_extensions/test_ext_intersphinx.py index c9d53787b57..ee758343ecc 100644 --- a/tests/test_extensions/test_ext_intersphinx.py +++ b/tests/test_extensions/test_ext_intersphinx.py @@ -1,6 +1,8 @@ """Test the intersphinx extension.""" +from __future__ import annotations import http.server +from typing import TYPE_CHECKING from unittest import mock import pytest @@ -22,6 +24,9 @@ from tests.test_util.intersphinx_data import INVENTORY_V2, INVENTORY_V2_NO_VERSION from tests.utils import http_server +if TYPE_CHECKING: + from typing import NoReturn + def fake_node(domain, type, target, content, **attrs): contnode = nodes.emphasis(content, content) @@ -40,7 +45,8 @@ def reference_check(app, *args, **kwds): def set_config(app, mapping): - app.config.intersphinx_mapping = mapping + # copy *mapping* so that normalization does not alter it + app.config.intersphinx_mapping = dict(mapping) app.config.intersphinx_cache_limit = 0 app.config.intersphinx_disabled_reftypes = [] @@ -93,7 +99,7 @@ def test_missing_reference(tmp_path, app, status, warning): inv_file = tmp_path / 'inventory' inv_file.write_bytes(INVENTORY_V2) set_config(app, { - 'https://docs.python.org/': str(inv_file), + 'python': ('https://docs.python.org/', str(inv_file)), 'py3k': ('https://docs.python.org/py3k/', str(inv_file)), 'py3krel': ('py3k', str(inv_file)), # relative path 'py3krelparent': ('../../py3k', str(inv_file)), # relative path, parent dir @@ -171,7 +177,7 @@ def test_missing_reference_pydomain(tmp_path, app, status, warning): inv_file = tmp_path / 'inventory' inv_file.write_bytes(INVENTORY_V2) set_config(app, { - 'https://docs.python.org/': str(inv_file), + 'python': ('https://docs.python.org/', str(inv_file)), }) # load the inventory and check if it's done correctly @@ -252,7 +258,7 @@ def test_missing_reference_cppdomain(tmp_path, app, status, warning): inv_file = tmp_path / 'inventory' inv_file.write_bytes(INVENTORY_V2) set_config(app, { - 'https://docs.python.org/': str(inv_file), + 'python': ('https://docs.python.org/', str(inv_file)), }) # load the inventory and check if it's done correctly @@ -278,7 +284,7 @@ def test_missing_reference_jsdomain(tmp_path, app, status, warning): inv_file = tmp_path / 'inventory' inv_file.write_bytes(INVENTORY_V2) set_config(app, { - 'https://docs.python.org/': str(inv_file), + 'python': ('https://docs.python.org/', str(inv_file)), }) # load the inventory and check if it's done correctly @@ -364,7 +370,7 @@ def test_inventory_not_having_version(tmp_path, app, status, warning): inv_file = tmp_path / 'inventory' inv_file.write_bytes(INVENTORY_V2_NO_VERSION) set_config(app, { - 'https://docs.python.org/': str(inv_file), + 'python': ('https://docs.python.org/', str(inv_file)), }) # load the inventory and check if it's done correctly @@ -378,29 +384,55 @@ def test_inventory_not_having_version(tmp_path, app, status, warning): assert rn[0].astext() == 'Long Module desc' -def test_load_mappings_warnings(tmp_path, app, status, warning): - """ - load_mappings issues a warning if new-style mapping - identifiers are not string - """ +def test_normalize_intersphinx_mapping_warnings(tmp_path, app, warning): + """Check warnings in :func:`sphinx.ext.intersphinx.normalize_intersphinx_mapping`.""" inv_file = tmp_path / 'inventory' inv_file.write_bytes(INVENTORY_V2) - set_config(app, { - 'https://docs.python.org/': str(inv_file), - 'py3k': ('https://docs.python.org/py3k/', str(inv_file)), - 'repoze.workflow': ('https://docs.repoze.org/workflow/', str(inv_file)), - 'django-taggit': ('https://django-taggit.readthedocs.org/en/latest/', - str(inv_file)), - 12345: ('https://www.sphinx-doc.org/en/stable/', str(inv_file)), + targets = (str(inv_file),) + + class FakeList(list): + def __iter__(self) -> NoReturn: + raise NotImplementedError + + set_config(app, bad_intersphinx_mapping := { + # fmt: off + '': ('67890.net', targets), # invalid project name (value) + 12345: ('12345.net', targets), # invalid project name (type) + 'bad-dict-item': 0, # invalid dict item type + 'unpack-except-1': [0], # invalid dict item size (native ValueError) + 'unpack-except-2': FakeList(), # invalid dict item size (custom exception) + 'bad-uri-type-1': (123456789, targets), # invalid URI type + 'bad-uri-type-2': (None, targets), # invalid URI type + 'bad-uri-value': ('', targets), # invalid URI value + 'good': ('foo.com', targets), # duplicated URI (good entry) + 'dedup-good': ('foo.com', targets), # duplicated URI + 'bad-target-1': ('a.com', 1), # invalid URI type (single input, bad type) + 'bad-target-2': ('b.com', ''), # invalid URI type (single input, bad string) + 'bad-target-3': ('c.com', [2, 'x']), # invalid URI type (sequence input, bad type) + 'bad-target-4': ('d.com', ['y', '']), # invalid URI type (sequence input, bad string) + # fmt: on }) - # load the inventory and check if it's done correctly + # normalize the inventory and check if it's done correctly normalize_intersphinx_mapping(app, app.config) - load_mappings(app) - warnings = warning.getvalue().splitlines() - assert len(warnings) == 2 - assert "The pre-Sphinx 1.0 'intersphinx_mapping' format is " in warnings[0] - assert 'intersphinx identifier 12345 is not string. Ignored' in warnings[1] + warnings = strip_colors(warning.getvalue()).splitlines() + assert len(warnings) == len(bad_intersphinx_mapping) - 1 + for index, messages in enumerate(( + "ignoring empty intersphinx identifier", + 'intersphinx identifier 12345 is not string. Ignored', + "intersphinx_mapping['bad-dict-item']: expecting a tuple or a list, got: 0; ignoring.", + "Failed to read intersphinx_mapping[unpack-except-1], ignored: ValueError('not enough values to unpack (expected 2, got 1)')", + "Failed to read intersphinx_mapping[unpack-except-2], ignored: NotImplementedError()", + "intersphinx_mapping['bad-uri-type-1']: URI must be a non-empty string, got: 123456789; ignoring.", + "intersphinx_mapping['bad-uri-type-2']: URI must be a non-empty string, got: None; ignoring.", + "intersphinx_mapping['bad-uri-value']: URI must be a non-empty string, got: ''; ignoring.", + "intersphinx_mapping['dedup-good']: URI 'foo.com' shadows URI from intersphinx_mapping['good']; ignoring.", + "intersphinx_mapping['bad-target-1']: inventory location must be a non-empty string or None, got: 1; ignoring.", + "intersphinx_mapping['bad-target-2']: inventory location must be a non-empty string or None, got: ''; ignoring.", + "intersphinx_mapping['bad-target-3']: inventory location must be a non-empty string or None, got: 2; ignoring.", + "intersphinx_mapping['bad-target-4']: inventory location must be a non-empty string or None, got: ''; ignoring.", + )): + assert messages in warnings[index] def test_load_mappings_fallback(tmp_path, app, status, warning): From 472eeab96b603ecfc48af33c447e36932cad8c2e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?B=C3=A9n=C3=A9dikt=20Tran?= <10796600+picnixz@users.noreply.github.com> Date: Mon, 11 Mar 2024 13:02:45 +0100 Subject: [PATCH 24/52] Update doc --- doc/usage/extensions/intersphinx.rst | 22 ---------------------- 1 file changed, 22 deletions(-) diff --git a/doc/usage/extensions/intersphinx.rst b/doc/usage/extensions/intersphinx.rst index e81719f7e39..db4c31ab92b 100644 --- a/doc/usage/extensions/intersphinx.rst +++ b/doc/usage/extensions/intersphinx.rst @@ -126,28 +126,6 @@ linking: ('../../otherbook/build/html/objects.inv', None)), } - **Old format for this config value** - - .. deprecated:: 6.2 - - .. RemovedInSphinx80Warning - - .. caution:: This is the format used before Sphinx 1.0. - It is deprecated and will be removed in Sphinx 8.0. - - A dictionary mapping URIs to either ``None`` or an URI. The keys are the - base URI of the foreign Sphinx documentation sets and can be local paths or - HTTP URIs. The values indicate where the inventory file can be found: they - can be ``None`` (at the same location as the base URI) or another local or - HTTP URI. - - Example: - - .. code:: python - - intersphinx_mapping = {'https://docs.python.org/': None} - - .. confval:: intersphinx_cache_limit The maximum number of days to cache remote inventories. The default is From 7655ba56d2dd3ea54e80a21d3c99b1baaeb8dde5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?B=C3=A9n=C3=A9dikt=20Tran?= <10796600+picnixz@users.noreply.github.com> Date: Sat, 20 Jul 2024 15:44:22 +0200 Subject: [PATCH 25/52] Merge remote-tracking branch 'upstream/master' into fix/11466-intersphinx-inventory-consistency # Conflicts: # doc/usage/extensions/intersphinx.rst # sphinx/ext/intersphinx/_resolve.py # tests/test_extensions/test_ext_intersphinx.py --- sphinx/ext/intersphinx/_load.py | 246 +++++++----- sphinx/ext/intersphinx/_resolve.py | 377 +----------------- sphinx/ext/intersphinx/_shared.py | 30 +- tests/test_extensions/test_ext_intersphinx.py | 11 +- 4 files changed, 191 insertions(+), 473 deletions(-) diff --git a/sphinx/ext/intersphinx/_load.py b/sphinx/ext/intersphinx/_load.py index b458d6a7e18..2a21c1d329f 100644 --- a/sphinx/ext/intersphinx/_load.py +++ b/sphinx/ext/intersphinx/_load.py @@ -6,6 +6,7 @@ import functools import posixpath import time +from operator import itemgetter from os import path from typing import TYPE_CHECKING from urllib.parse import urlsplit, urlunsplit @@ -21,129 +22,190 @@ from sphinx.application import Sphinx from sphinx.config import Config - from sphinx.ext.intersphinx._shared import InventoryCacheEntry + from sphinx.ext.intersphinx._shared import ( + InventoryCacheEntry, + InventoryLocation, + InventoryName, + InventoryURI, + ) from sphinx.util.typing import Inventory def normalize_intersphinx_mapping(app: Sphinx, config: Config) -> None: - for key, value in config.intersphinx_mapping.copy().items(): + # URIs should NOT be duplicated, otherwise different builds may use + # different project names (and thus, the build are no more reproducible) + # depending on which one is inserted last in the cache. + seen: dict[InventoryURI, InventoryName] = {} + + for name, value in config.intersphinx_mapping.copy().items(): + if not isinstance(name, str): + LOGGER.warning( + __('intersphinx identifier %r is not string. Ignored'), + name, type='intersphinx', subtype='config', + ) + del config.intersphinx_mapping[name] + continue + + # ensure that intersphinx projects are always named + if not name: + LOGGER.warning( + __('ignoring empty intersphinx identifier'), + type='intersphinx', subtype='config', + ) + del config.intersphinx_mapping[name] + continue + + if not isinstance(value, (tuple, list)): + LOGGER.warning( + __('intersphinx_mapping[%r]: expecting a tuple or a list, got: %r; ignoring.'), + name, value, type='intersphinx', subtype='config', + ) + del config.intersphinx_mapping[name] + continue + try: - if isinstance(value, (list, tuple)): - # new format - name, (uri, inv) = key, value - if not isinstance(name, str): - LOGGER.warning(__('intersphinx identifier %r is not string. Ignored'), - name) - config.intersphinx_mapping.pop(key) - continue + uri, inv = value + except Exception as exc: + LOGGER.warning( + __('Failed to read intersphinx_mapping[%s], ignored: %r'), + name, exc, type='intersphinx', subtype='config', + ) + del config.intersphinx_mapping[name] + continue + + if not uri or not isinstance(uri, str): + LOGGER.warning( + __('intersphinx_mapping[%r]: URI must be a non-empty string, ' + 'got: %r; ignoring.'), + name, uri, type='intersphinx', subtype='config', + ) + del config.intersphinx_mapping[name] + continue + + if (name_for_uri := seen.setdefault(uri, name)) != name: + LOGGER.warning( + __('intersphinx_mapping[%r]: URI %r shadows URI from intersphinx_mapping[%r]; ' + 'ignoring.'), name, uri, name_for_uri, type='intersphinx', subtype='config', + ) + del config.intersphinx_mapping[name] + continue + + targets: list[InventoryLocation] = [] + for target in (inv if isinstance(inv, (tuple, list)) else (inv,)): + if target is None or target and isinstance(target, str): + targets.append(target) else: - # old format, no name - # xref RemovedInSphinx80Warning - name, uri, inv = None, key, value - msg = ( - "The pre-Sphinx 1.0 'intersphinx_mapping' format is " - 'deprecated and will be removed in Sphinx 8. Update to the ' - 'current format as described in the documentation. ' - f"Hint: `intersphinx_mapping = {{'': {(uri, inv)!r}}}`." - 'https://www.sphinx-doc.org/en/master/usage/extensions/intersphinx.html#confval-intersphinx_mapping' # NoQA: E501 + LOGGER.warning( + __('intersphinx_mapping[%r]: inventory location must ' + 'be a non-empty string or None, got: %r; ignoring.'), + name, target, type='intersphinx', subtype='config', ) - LOGGER.warning(msg) - if not isinstance(inv, tuple): - config.intersphinx_mapping[key] = (name, (uri, (inv,))) - else: - config.intersphinx_mapping[key] = (name, (uri, inv)) - except Exception as exc: - LOGGER.warning(__('Failed to read intersphinx_mapping[%s], ignored: %r'), key, exc) - config.intersphinx_mapping.pop(key) + config.intersphinx_mapping[name] = (name, (uri, tuple(targets))) def load_mappings(app: Sphinx) -> None: - """Load all intersphinx mappings into the environment.""" + """Load the (normalized) intersphinx mappings into the environment.""" now = int(time.time()) inventories = InventoryAdapter(app.builder.env) - intersphinx_cache: dict[str, InventoryCacheEntry] = inventories.cache + intersphinx_cache: dict[InventoryURI, InventoryCacheEntry] = inventories.cache + intersphinx_mapping = app.config.intersphinx_mapping + + expected_uris = {uri for uri, _invs in app.config.intersphinx_mapping.values()} + + # If the current cache contains some (project, uri) pair + # say ("foo", "foo.com") and if the new intersphinx dict + # contains the pair ("foo", "bar.com"), we need to remove + # the ("foo", "foo.com") entry and use ("foo", "bar.com"). + for uri in frozenset(intersphinx_cache): + if intersphinx_cache[uri][0] not in intersphinx_mapping or uri not in expected_uris: + # remove a cached inventory if the latter is no more used by intersphinx + del intersphinx_cache[uri] with concurrent.futures.ThreadPoolExecutor() as pool: - futures = [] - name: str | None - uri: str - invs: tuple[str | None, ...] - for name, (uri, invs) in app.config.intersphinx_mapping.values(): - futures.append(pool.submit( - fetch_inventory_group, name, uri, invs, intersphinx_cache, app, now, - )) + futures = [ + pool.submit(fetch_inventory_group, name, uri, invs, intersphinx_cache, app, now) + for name, (uri, invs) in app.config.intersphinx_mapping.values() + ] updated = [f.result() for f in concurrent.futures.as_completed(futures)] if any(updated): + # clear the local inventories inventories.clear() # Duplicate values in different inventories will shadow each - # other; which one will override which can vary between builds - # since they are specified using an unordered dict. To make - # it more consistent, we sort the named inventories and then - # add the unnamed inventories last. This means that the - # unnamed inventories will shadow the named ones but the named - # ones can still be accessed when the name is specified. - named_vals = [] - unnamed_vals = [] - for name, _expiry, invdata in intersphinx_cache.values(): - if name: - named_vals.append((name, invdata)) - else: - unnamed_vals.append((name, invdata)) - for name, invdata in sorted(named_vals) + unnamed_vals: - if name: - inventories.named_inventory[name] = invdata - for type, objects in invdata.items(): - inventories.main_inventory.setdefault(type, {}).update(objects) + # other and which one will override which varies between builds. + # + # We can however order the cache by URIs for reproducibility. + intersphinx_cache_values = sorted(intersphinx_cache.values(), key=itemgetter(0, 1)) + for name, _timeout, invdata in intersphinx_cache_values: + if not name: + LOGGER.warning( + __('intersphinx cache seems corrupted, please rebuild ' + 'the project with the "-E" option (see sphinx --help)'), + ) + continue + + inventories.named_inventory[name] = invdata + for objtype, objects in invdata.items(): + inventories.main_inventory.setdefault(objtype, {}).update(objects) def fetch_inventory_group( - name: str | None, - uri: str, - invs: tuple[str | None, ...], - cache: dict[str, InventoryCacheEntry], + name: InventoryName, + uri: InventoryURI, + invs: tuple[InventoryLocation, ...], + cache: dict[InventoryURI, InventoryCacheEntry], app: Sphinx, now: int, ) -> bool: cache_time = now - app.config.intersphinx_cache_limit * 86400 + + def should_store(uri: str, inv: str) -> bool: + # decide whether the inventory must be read: always read local + # files; remote ones only if the cache time is expired + return '://' not in inv or uri not in cache or cache[uri][1] < cache_time + + updated = False failures = [] - try: - for inv in invs: - if not inv: - inv = posixpath.join(uri, INVENTORY_FILENAME) - # decide whether the inventory must be read: always read local - # files; remote ones only if the cache time is expired - if '://' not in inv or uri not in cache or cache[uri][1] < cache_time: - safe_inv_url = _get_safe_url(inv) - inv_descriptor = name or 'main_inventory' - LOGGER.info(__("loading intersphinx inventory '%s' from %s..."), - inv_descriptor, safe_inv_url) - try: - invdata = fetch_inventory(app, uri, inv) - except Exception as err: - failures.append(err.args) - continue - if invdata: - cache[uri] = name, now, invdata - return True - return False - finally: - if failures == []: - pass - elif len(failures) < len(invs): - LOGGER.info(__('encountered some issues with some of the inventories,' - ' but they had working alternatives:')) - for fail in failures: - LOGGER.info(*fail) - else: - issues = '\n'.join(f[0] % f[1:] for f in failures) - LOGGER.warning(__('failed to reach any of the inventories ' - 'with the following issues:') + '\n' + issues) + + for location in invs: + inv: str = location or posixpath.join(uri, INVENTORY_FILENAME) + if not should_store(uri, inv): + print("nope", uri, inv) + continue + + safe_inv_url = _get_safe_url(inv) + inv_descriptor = name or 'main_inventory' + LOGGER.info(__("loading intersphinx inventory '%s' from %s..."), + inv_descriptor, safe_inv_url) + + try: + invdata = fetch_inventory(app, uri, inv) + except Exception as err: + failures.append(err.args) + continue + + if invdata: + cache[uri] = name, now, invdata + updated = True + break + + if not failures: + pass + elif len(failures) < len(invs): + LOGGER.info(__("encountered some issues with some of the inventories," + " but they had working alternatives:")) + for fail in failures: + LOGGER.info(*fail) + else: + issues = '\n'.join(f[0] % f[1:] for f in failures) + LOGGER.warning(__("failed to reach any of the inventories " + "with the following issues:") + "\n" + issues) + return updated -def fetch_inventory(app: Sphinx, uri: str, inv: str) -> Inventory: +def fetch_inventory(app: Sphinx, uri: InventoryURI, inv: str) -> Inventory: """Fetch, parse and return an intersphinx inventory file.""" # both *uri* (base URI of the links to generate) and *inv* (actual # location of the inventory file) can be local or remote URIs diff --git a/sphinx/ext/intersphinx/_resolve.py b/sphinx/ext/intersphinx/_resolve.py index 05eb18868c5..6c671d0f732 100644 --- a/sphinx/ext/intersphinx/_resolve.py +++ b/sphinx/ext/intersphinx/_resolve.py @@ -3,10 +3,7 @@ from __future__ import annotations import posixpath -import sys -import time -from operator import itemgetter -from os import path +import re from typing import TYPE_CHECKING, cast from docutils import nodes @@ -23,7 +20,7 @@ if TYPE_CHECKING: from collections.abc import Iterable from types import ModuleType - from typing import IO, Any, Optional + from typing import Any from docutils.nodes import Node, TextElement, system_message from docutils.utils import Reporter @@ -31,243 +28,9 @@ from sphinx.application import Sphinx from sphinx.domains import Domain from sphinx.environment import BuildEnvironment + from sphinx.ext.intersphinx._shared import InventoryName from sphinx.util.typing import Inventory, InventoryItem, RoleFunction - #: The inventory project URL to which links are resolved. - #: - #: This value is unique in :confval:`intersphinx_mapping`. - InventoryURI = str - - #: The inventory (non-empty) name. - #: - #: It is unique and in bijection with an inventory remote URL. - InventoryName = str - - #: A target (local or remote) containing the inventory data to fetch. - #: - #: Empty strings are not expected and ``None`` indicates the default - #: inventory file name :data:`~sphinx.builder.html.INVENTORY_FILENAME`. - InventoryLocation = Optional[str] - - #: Inventory cache entry. The integer field is the cache expiration time. - InventoryCacheEntry = tuple[InventoryName, int, Inventory] - - #: The type of :confval:`intersphinx_mapping` *after* normalization. - IntersphinxMapping = dict[ - InventoryName, - tuple[InventoryName, tuple[InventoryURI, tuple[InventoryLocation, ...]]], - ] - - - - -def _strip_basic_auth(url: str) -> str: - """Returns *url* with basic auth credentials removed. Also returns the - basic auth username and password if they're present in *url*. - - E.g.: https://user:pass@example.com => https://example.com - - *url* need not include basic auth credentials. - - :param url: url which may or may not contain basic auth credentials - :type url: ``str`` - - :return: *url* with any basic auth creds removed - :rtype: ``str`` - """ - frags = list(urlsplit(url)) - # swap out "user[:pass]@hostname" for "hostname" - if '@' in frags[1]: - frags[1] = frags[1].split('@')[1] - return urlunsplit(frags) - - -def _read_from_url(url: str, *, config: Config) -> IO: - """Reads data from *url* with an HTTP *GET*. - - This function supports fetching from resources which use basic HTTP auth as - laid out by RFC1738 § 3.1. See § 5 for grammar definitions for URLs. - - .. seealso: - - https://www.ietf.org/rfc/rfc1738.txt - - :param url: URL of an HTTP resource - :type url: ``str`` - - :return: data read from resource described by *url* - :rtype: ``file``-like object - """ - r = requests.get(url, stream=True, timeout=config.intersphinx_timeout, - _user_agent=config.user_agent, - _tls_info=(config.tls_verify, config.tls_cacerts)) - r.raise_for_status() - r.raw.url = r.url - # decode content-body based on the header. - # ref: https://github.com/psf/requests/issues/2155 - r.raw.read = functools.partial(r.raw.read, decode_content=True) - return r.raw - - -def _get_safe_url(url: str) -> str: - """Gets version of *url* with basic auth passwords obscured. This function - returns results suitable for printing and logging. - - E.g.: https://user:12345@example.com => https://user@example.com - - :param url: a url - :type url: ``str`` - - :return: *url* with password removed - :rtype: ``str`` - """ - parts = urlsplit(url) - if parts.username is None: - return url - else: - frags = list(parts) - if parts.port: - frags[1] = f'{parts.username}@{parts.hostname}:{parts.port}' - else: - frags[1] = f'{parts.username}@{parts.hostname}' - - return urlunsplit(frags) - - -def fetch_inventory(app: Sphinx, uri: InventoryURI, inv: str) -> Inventory: - """Fetch, parse and return an intersphinx inventory file.""" - # both *uri* (base URI of the links to generate) and *inv* (actual - # location of the inventory file) can be local or remote URIs - if '://' in uri: - # case: inv URI points to remote resource; strip any existing auth - uri = _strip_basic_auth(uri) - try: - if '://' in inv: - f = _read_from_url(inv, config=app.config) - else: - f = open(path.join(app.srcdir, inv), 'rb') # NoQA: SIM115 - except Exception as err: - err.args = ('intersphinx inventory %r not fetchable due to %s: %s', - inv, err.__class__, str(err)) - raise - try: - if hasattr(f, 'url'): - newinv = f.url - if inv != newinv: - logger.info(__('intersphinx inventory has moved: %s -> %s'), inv, newinv) - - if uri in (inv, path.dirname(inv), path.dirname(inv) + '/'): - uri = path.dirname(newinv) - with f: - try: - invdata = InventoryFile.load(f, uri, posixpath.join) - except ValueError as exc: - raise ValueError('unknown or unsupported inventory version: %r' % exc) from exc - except Exception as err: - err.args = ('intersphinx inventory %r not readable due to %s: %s', - inv, err.__class__.__name__, str(err)) - raise - else: - return invdata - - -def fetch_inventory_group( - name: InventoryName, - uri: InventoryURI, - invs: tuple[InventoryLocation, ...], - cache: dict[InventoryURI, InventoryCacheEntry], - app: Sphinx, - now: int, -) -> bool: - cache_time = now - app.config.intersphinx_cache_limit * 86400 - - def should_store(uri: str, inv: str) -> bool: - # decide whether the inventory must be read: always read local - # files; remote ones only if the cache time is expired - return '://' not in inv or uri not in cache or cache[uri][1] < cache_time - - updated = False - failures = [] - - for location in invs: - inv: str = location or posixpath.join(uri, INVENTORY_FILENAME) - if not should_store(uri, inv): - continue - - safe_inv_url = _get_safe_url(inv) - logger.info(__('loading intersphinx inventory from %s...'), safe_inv_url) - - try: - invdata = fetch_inventory(app, uri, inv) - except Exception as err: - failures.append(err.args) - continue - - if invdata: - cache[uri] = name, now, invdata - updated = True - break - - if not failures: - pass - elif len(failures) < len(invs): - logger.info(__("encountered some issues with some of the inventories," - " but they had working alternatives:")) - for fail in failures: - logger.info(*fail) - else: - issues = '\n'.join(f[0] % f[1:] for f in failures) - logger.warning(__("failed to reach any of the inventories " - "with the following issues:") + "\n" + issues) - return updated - - -def load_mappings(app: Sphinx) -> None: - """Load the (normalized) intersphinx mappings into the environment.""" - now = int(time.time()) - inventories = InventoryAdapter(app.builder.env) - intersphinx_cache: dict[InventoryURI, InventoryCacheEntry] = inventories.cache - intersphinx_mapping = app.config.intersphinx_mapping - - expected_uris = {uri for uri, _invs in app.config.intersphinx_mapping.values()} - - # If the current cache contains some (project, uri) pair - # say ("foo", "foo.com") and if the new intersphinx dict - # contains the pair ("foo", "bar.com"), we need to remove - # the ("foo", "foo.com") entry and use ("foo", "bar.com"). - for uri in frozenset(intersphinx_cache): - if intersphinx_cache[uri][0] not in intersphinx_mapping or uri not in expected_uris: - # remove a cached inventory if the latter is no more used by intersphinx - del intersphinx_cache[uri] - - with concurrent.futures.ThreadPoolExecutor() as pool: - futures = [ - pool.submit(fetch_inventory_group, name, uri, invs, intersphinx_cache, app, now) - for name, (uri, invs) in app.config.intersphinx_mapping.values() - ] - updated = [f.result() for f in concurrent.futures.as_completed(futures)] - - if any(updated): - # clear the local inventories - inventories.clear() - - # Duplicate values in different inventories will shadow each - # other and which one will override which varies between builds. - # - # We can however order the cache by URIs for reproducibility. - intersphinx_cache_values = sorted(intersphinx_cache.values(), key=itemgetter(0, 1)) - for name, _timeout, invdata in intersphinx_cache_values: - if not name: - logger.warning( - __('intersphinx cache seems corrupted, please rebuild ' - 'the project with the "-E" option (see sphinx --help)'), - ) - continue - - inventories.named_inventory[name] = invdata - for objtype, objects in invdata.items(): - inventories.main_inventory.setdefault(objtype, {}).update(objects) - def _create_element_from_result( domain: Domain, inv_name: InventoryName | None, @@ -753,137 +516,3 @@ def install_dispatcher(app: Sphinx, docname: str, source: list[str]) -> None: """ dispatcher = IntersphinxDispatcher() dispatcher.enable() - - -def normalize_intersphinx_mapping(app: Sphinx, config: Config) -> None: - # URIs should NOT be duplicated, otherwise different builds may use - # different project names (and thus, the build are no more reproducible) - # depending on which one is inserted last in the cache. - seen: dict[InventoryURI, InventoryName] = {} - - for name, value in config.intersphinx_mapping.copy().items(): - if not isinstance(name, str): - logger.warning( - __('intersphinx identifier %r is not string. Ignored'), - name, type='intersphinx', subtype='config', - ) - del config.intersphinx_mapping[name] - continue - - # ensure that intersphinx projects are always named - if not name: - logger.warning( - __('ignoring empty intersphinx identifier'), - type='intersphinx', subtype='config', - ) - del config.intersphinx_mapping[name] - continue - - if not isinstance(value, (tuple, list)): - logger.warning( - __('intersphinx_mapping[%r]: expecting a tuple or a list, got: %r; ignoring.'), - name, value, type='intersphinx', subtype='config', - ) - del config.intersphinx_mapping[name] - continue - - try: - uri, inv = value - except Exception as exc: - logger.warning( - __('Failed to read intersphinx_mapping[%s], ignored: %r'), - name, exc, type='intersphinx', subtype='config', - ) - del config.intersphinx_mapping[name] - continue - - if not uri or not isinstance(uri, str): - logger.warning( - __('intersphinx_mapping[%r]: URI must be a non-empty string, ' - 'got: %r; ignoring.'), - name, uri, type='intersphinx', subtype='config', - ) - del config.intersphinx_mapping[name] - continue - - if (name_for_uri := seen.setdefault(uri, name)) != name: - logger.warning( - __('intersphinx_mapping[%r]: URI %r shadows URI from intersphinx_mapping[%r]; ' - 'ignoring.'), name, uri, name_for_uri, type='intersphinx', subtype='config', - ) - del config.intersphinx_mapping[name] - continue - - targets: list[InventoryLocation] = [] - for target in (inv if isinstance(inv, (tuple, list)) else (inv,)): - if target is None or target and isinstance(target, str): - targets.append(target) - else: - logger.warning( - __('intersphinx_mapping[%r]: inventory location must ' - 'be a non-empty string or None, got: %r; ignoring.'), - name, target, type='intersphinx', subtype='config', - ) - - config.intersphinx_mapping[name] = (name, (uri, tuple(targets))) - - -def setup(app: Sphinx) -> dict[str, Any]: - app.add_config_value('intersphinx_mapping', {}, 'env') - app.add_config_value('intersphinx_cache_limit', 5, '') - app.add_config_value('intersphinx_timeout', None, '') - app.add_config_value('intersphinx_disabled_reftypes', ['std:doc'], 'env') - app.connect('config-inited', normalize_intersphinx_mapping, priority=800) - app.connect('builder-inited', load_mappings) - app.connect('source-read', install_dispatcher) - app.connect('missing-reference', missing_reference) - app.add_post_transform(IntersphinxRoleResolver) - return { - 'version': sphinx.__display_version__, - 'env_version': 1, - 'parallel_read_safe': True, - } - - -def inspect_main(argv: list[str], /) -> int: - """Debug functionality to print out an inventory""" - if len(argv) < 1: - print("Print out an inventory file.\n" - "Error: must specify local path or URL to an inventory file.", - file=sys.stderr) - return 1 - - class MockConfig: - intersphinx_timeout: int | None = None - tls_verify = False - tls_cacerts: str | dict[str, str] | None = None - user_agent: str = '' - - class MockApp: - srcdir = '' - config = MockConfig() - - try: - filename = argv[0] - inv_data = fetch_inventory(MockApp(), '', filename) # type: ignore[arg-type] - for key in sorted(inv_data or {}): - print(key) - inv_entries = sorted(inv_data[key].items()) - for entry, (_proj, _ver, url_path, display_name) in inv_entries: - display_name = display_name * (display_name != '-') - print(f' {entry:<40} {display_name:<40}: {url_path}') - except ValueError as exc: - print(exc.args[0] % exc.args[1:], file=sys.stderr) - return 1 - except Exception as exc: - print(f'Unknown error: {exc!r}', file=sys.stderr) - return 1 - else: - return 0 - - -if __name__ == '__main__': - import logging as _logging - _logging.basicConfig() - - raise SystemExit(inspect_main(sys.argv[1:])) diff --git a/sphinx/ext/intersphinx/_shared.py b/sphinx/ext/intersphinx/_shared.py index aee5e8d4758..98f9b13c410 100644 --- a/sphinx/ext/intersphinx/_shared.py +++ b/sphinx/ext/intersphinx/_shared.py @@ -2,15 +2,41 @@ from __future__ import annotations -from typing import TYPE_CHECKING, Final, Union +from typing import TYPE_CHECKING from sphinx.util import logging if TYPE_CHECKING: + from typing import Final, Optional + from sphinx.environment import BuildEnvironment from sphinx.util.typing import Inventory - InventoryCacheEntry = tuple[Union[str, None], int, Inventory] + #: The inventory project URL to which links are resolved. + #: + #: This value is unique in :confval:`intersphinx_mapping`. + InventoryURI = str + + #: The inventory (non-empty) name. + #: + #: It is unique and in bijection with an inventory remote URL. + InventoryName = str + + #: A target (local or remote) containing the inventory data to fetch. + #: + #: Empty strings are not expected and ``None`` indicates the default + #: inventory file name :data:`~sphinx.builder.html.INVENTORY_FILENAME`. + InventoryLocation = Optional[str] + + #: Inventory cache entry. The integer field is the cache expiration time. + InventoryCacheEntry = tuple[InventoryName, int, Inventory] + + #: The type of :confval:`intersphinx_mapping` *after* normalization. + IntersphinxMapping = dict[ + InventoryName, + tuple[InventoryName, tuple[InventoryURI, tuple[InventoryLocation, ...]]], + ] + LOGGER: Final[logging.SphinxLoggerAdapter] = logging.getLogger('sphinx.ext.intersphinx') diff --git a/tests/test_extensions/test_ext_intersphinx.py b/tests/test_extensions/test_ext_intersphinx.py index 09c517e7639..37f5b17f73a 100644 --- a/tests/test_extensions/test_ext_intersphinx.py +++ b/tests/test_extensions/test_ext_intersphinx.py @@ -178,7 +178,7 @@ def set_config(app, mapping): # TODO(picnixz): investigate why 'caplog' fixture does not work -@mock.patch("sphinx.ext.intersphinx.LOGGER") +@mock.patch("sphinx.ext.intersphinx._load.LOGGER") def test_normalize_intersphinx_mapping(logger): app = mock.Mock() app.config.intersphinx_mapping = { @@ -646,15 +646,16 @@ def test_load_mappings_cache_update(make_app, app_params): DOMAIN_NAME = 'py' OBJECT_TYPE = 'module' REFTYPE = f'{DOMAIN_NAME}:{OBJECT_TYPE}' + PORT = 7777 - PROJECT_NAME, PROJECT_BASEURL = 'foo', 'http://localhost:7777' + PROJECT_NAME, PROJECT_BASEURL = 'foo', f'http://localhost:{PORT}' old_project = IntersphinxProject(PROJECT_NAME, 1337, PROJECT_BASEURL, 'old') assert old_project.name == PROJECT_NAME - assert old_project.url == 'http://localhost:7777/old' + assert old_project.url == f'http://localhost:{PORT}/old' new_project = IntersphinxProject(PROJECT_NAME, 1701, PROJECT_BASEURL, 'new') assert new_project.name == PROJECT_NAME - assert new_project.url == 'http://localhost:7777/new' + assert new_project.url == f'http://localhost:{PORT}/new' def make_entry(project: IntersphinxProject) -> InventoryEntry: name = f'{ITEM_NAME}_{project.version}' @@ -687,7 +688,7 @@ def log_message(*args, **kwargs): baseconfig = {'extensions': ['sphinx.ext.intersphinx']} - with http_server(InventoryHandler): + with http_server(InventoryHandler, port=PORT): confoverrides1 = baseconfig | {'intersphinx_mapping': old_project.record} app1 = make_app(*args, confoverrides=confoverrides1, **kwargs) app1.build() From 47c3383f5846418fb0576d0cbced32988b6812de Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?B=C3=A9n=C3=A9dikt=20Tran?= <10796600+picnixz@users.noreply.github.com> Date: Sat, 20 Jul 2024 15:44:53 +0200 Subject: [PATCH 26/52] remove debug --- sphinx/ext/intersphinx/_load.py | 1 - 1 file changed, 1 deletion(-) diff --git a/sphinx/ext/intersphinx/_load.py b/sphinx/ext/intersphinx/_load.py index 2a21c1d329f..7a9521f884a 100644 --- a/sphinx/ext/intersphinx/_load.py +++ b/sphinx/ext/intersphinx/_load.py @@ -172,7 +172,6 @@ def should_store(uri: str, inv: str) -> bool: for location in invs: inv: str = location or posixpath.join(uri, INVENTORY_FILENAME) if not should_store(uri, inv): - print("nope", uri, inv) continue safe_inv_url = _get_safe_url(inv) From ad86c96b7af7c39e54dce5ef361b4418c3dde615 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?B=C3=A9n=C3=A9dikt=20Tran?= <10796600+picnixz@users.noreply.github.com> Date: Sat, 20 Jul 2024 15:51:37 +0200 Subject: [PATCH 27/52] fixups --- sphinx/ext/intersphinx/_load.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/sphinx/ext/intersphinx/_load.py b/sphinx/ext/intersphinx/_load.py index 7a9521f884a..e9c2324b500 100644 --- a/sphinx/ext/intersphinx/_load.py +++ b/sphinx/ext/intersphinx/_load.py @@ -23,6 +23,7 @@ from sphinx.application import Sphinx from sphinx.config import Config from sphinx.ext.intersphinx._shared import ( + IntersphinxMapping, InventoryCacheEntry, InventoryLocation, InventoryName, @@ -109,7 +110,7 @@ def load_mappings(app: Sphinx) -> None: now = int(time.time()) inventories = InventoryAdapter(app.builder.env) intersphinx_cache: dict[InventoryURI, InventoryCacheEntry] = inventories.cache - intersphinx_mapping = app.config.intersphinx_mapping + intersphinx_mapping: IntersphinxMapping = app.config.intersphinx_mapping expected_uris = {uri for uri, _invs in app.config.intersphinx_mapping.values()} From 3912820c9ef3597015f4e4faf2b61ed964c85f51 Mon Sep 17 00:00:00 2001 From: Adam Turner <9087854+aa-turner@users.noreply.github.com> Date: Sun, 21 Jul 2024 10:52:29 +0100 Subject: [PATCH 28/52] raise a ConfigError --- sphinx/ext/intersphinx/_load.py | 63 ++++++------ sphinx/util/typing.py | 2 +- tests/test_extensions/test_ext_intersphinx.py | 99 ++++++++++--------- 3 files changed, 85 insertions(+), 79 deletions(-) diff --git a/sphinx/ext/intersphinx/_load.py b/sphinx/ext/intersphinx/_load.py index aebbc7f5738..985c7887e16 100644 --- a/sphinx/ext/intersphinx/_load.py +++ b/sphinx/ext/intersphinx/_load.py @@ -11,6 +11,7 @@ from urllib.parse import urlsplit, urlunsplit from sphinx.builders.html import INVENTORY_FILENAME +from sphinx.errors import ConfigError from sphinx.ext.intersphinx._shared import LOGGER, InventoryAdapter from sphinx.locale import __ from sphinx.util import requests @@ -37,70 +38,66 @@ def normalize_intersphinx_mapping(app: Sphinx, config: Config) -> None: # depending on which one is inserted last in the cache. seen: dict[InventoryURI, InventoryName] = {} + errors = [] for name, value in config.intersphinx_mapping.copy().items(): + # ensure that intersphinx projects are always named if not isinstance(name, str): - LOGGER.warning(__('intersphinx identifier %r is not string. Ignored'), - name, type='intersphinx', subtype='config') + msg = __('project identifier must be a string') + errors.append((msg, name, name)) del config.intersphinx_mapping[name] continue - - # ensure that intersphinx projects are always named if not name: - LOGGER.warning( - __('ignoring empty intersphinx identifier'), - type='intersphinx', subtype='config', - ) + msg = __('expected an intersphinx project identifier') + errors.append((msg, name, name)) del config.intersphinx_mapping[name] continue + # ensure values are properly formatted if not isinstance(value, (tuple, list)): - LOGGER.warning( - __('intersphinx_mapping[%r]: expecting a tuple or a list, got: %r; ignoring.'), - name, value, type='intersphinx', subtype='config', - ) + msg = __('expected a tuple or a list') + errors.append((msg, name, value)) del config.intersphinx_mapping[name] continue - try: uri, inv = value - except Exception as exc: - LOGGER.warning( - __('Failed to read intersphinx_mapping[%s], ignored: %r'), - name, exc, type='intersphinx', subtype='config', - ) + except Exception: + msg = __('values must be a (target URI, inventory locations) pair') + errors.append((msg, name, value)) del config.intersphinx_mapping[name] continue + # ensure target URIs are non-empty and unique if not uri or not isinstance(uri, str): - LOGGER.warning( - __('intersphinx_mapping[%r]: URI must be a non-empty string, ' - 'got: %r; ignoring.'), - name, uri, type='intersphinx', subtype='config', - ) + msg = __('target URI must be a non-empty string') + errors.append((msg, name, uri)) del config.intersphinx_mapping[name] continue - if (name_for_uri := seen.setdefault(uri, name)) != name: - LOGGER.warning( - __('intersphinx_mapping[%r]: URI %r shadows URI from intersphinx_mapping[%r]; ' - 'ignoring.'), name, uri, name_for_uri, type='intersphinx', subtype='config', - ) + msg = __('target URI must be unique (other instance in `intersphinx_mapping[%r]`)') + errors.append((msg % name_for_uri, name, uri)) del config.intersphinx_mapping[name] continue + # ensure inventory locations are None or non-empty targets: list[InventoryLocation] = [] for target in (inv if isinstance(inv, (tuple, list)) else (inv,)): if target is None or target and isinstance(target, str): targets.append(target) else: - LOGGER.warning( - __('intersphinx_mapping[%r]: inventory location must ' - 'be a non-empty string or None, got: %r; ignoring.'), - name, target, type='intersphinx', subtype='config', - ) + msg = __('inventory location must be a non-empty string or None') + errors.append((msg, name, target)) + del config.intersphinx_mapping[name] + continue config.intersphinx_mapping[name] = (name, (uri, tuple(targets))) + if len(errors) > 0: + for (msg, name, value) in errors: + error_msg = __('Invalid value %r in intersphinx_mapping[%r]: %s') + LOGGER.error(error_msg % (value, name, msg)) + msg = __('Invalid `intersphinx_mapping` configuration (%s errors).') + raise ConfigError(msg % len(errors)) + def load_mappings(app: Sphinx) -> None: """Load all intersphinx mappings into the environment. diff --git a/sphinx/util/typing.py b/sphinx/util/typing.py index 9dccc50a89a..49d071d9c0f 100644 --- a/sphinx/util/typing.py +++ b/sphinx/util/typing.py @@ -128,7 +128,7 @@ def __call__( str, # display name ] -# referencable role => (reference name => inventory item) +# referencable role -> (reference name -> inventory item) Inventory = dict[str, dict[str, InventoryItem]] diff --git a/tests/test_extensions/test_ext_intersphinx.py b/tests/test_extensions/test_ext_intersphinx.py index cf4402ac8e3..529315d8f55 100644 --- a/tests/test_extensions/test_ext_intersphinx.py +++ b/tests/test_extensions/test_ext_intersphinx.py @@ -1,4 +1,5 @@ """Test the intersphinx extension.""" + from __future__ import annotations import http.server @@ -10,6 +11,7 @@ from sphinx import addnodes from sphinx.builders.html import INVENTORY_FILENAME +from sphinx.errors import ConfigError from sphinx.ext.intersphinx import ( fetch_inventory, inspect_main, @@ -32,6 +34,11 @@ from typing import NoReturn +class FakeList(list): + def __iter__(self) -> NoReturn: + raise NotImplementedError + + def fake_node(domain, type, target, content, **attrs): contnode = nodes.emphasis(content, content) node = addnodes.pending_xref('') @@ -406,55 +413,57 @@ def test_inventory_not_having_version(tmp_path, app, status, warning): assert rn[0].astext() == 'Long Module desc' -def test_normalize_intersphinx_mapping_warnings(tmp_path, app, warning): +def test_normalize_intersphinx_mapping_warnings(app, warning): """Check warnings in :func:`sphinx.ext.intersphinx.normalize_intersphinx_mapping`.""" - inv_file = tmp_path / 'inventory' - inv_file.write_bytes(INVENTORY_V2) - targets = (str(inv_file),) - - class FakeList(list): - def __iter__(self) -> NoReturn: - raise NotImplementedError - - set_config(app, bad_intersphinx_mapping := { + bad_intersphinx_mapping = { # fmt: off - '': ('67890.net', targets), # invalid project name (value) - 12345: ('12345.net', targets), # invalid project name (type) - 'bad-dict-item': 0, # invalid dict item type - 'unpack-except-1': [0], # invalid dict item size (native ValueError) - 'unpack-except-2': FakeList(), # invalid dict item size (custom exception) - 'bad-uri-type-1': (123456789, targets), # invalid URI type - 'bad-uri-type-2': (None, targets), # invalid URI type - 'bad-uri-value': ('', targets), # invalid URI value - 'good': ('foo.com', targets), # duplicated URI (good entry) - 'dedup-good': ('foo.com', targets), # duplicated URI - 'bad-target-1': ('a.com', 1), # invalid URI type (single input, bad type) - 'bad-target-2': ('b.com', ''), # invalid URI type (single input, bad string) - 'bad-target-3': ('c.com', [2, 'x']), # invalid URI type (sequence input, bad type) - 'bad-target-4': ('d.com', ['y', '']), # invalid URI type (sequence input, bad string) + '': ('789.example', None), # invalid project name (value) + 12345: ('456.example', None), # invalid project name (type) + None: ('123.example', None), # invalid project name (type) + 'https://example/': 'inventory', # Sphinx 0.x style value + 'bad-dict-item': 0, # invalid dict item type + 'unpack-except-1': [0], # invalid dict item size (native ValueError) + 'unpack-except-2': FakeList(), # invalid dict item size (custom exception) + 'bad-uri-type-1': (123456789, None), # invalid target URI type + 'bad-uri-type-2': (None, None), # invalid target URI type + 'bad-uri-value': ('', None), # invalid target URI value + 'good': ('example.org', None), # duplicated target URI (good entry) + 'dedup-good': ('example.org', None), # duplicated target URI + 'bad-location-1': ('a.example', 1), # invalid inventory location (single input, bad type) + 'bad-location-2': ('b.example', ''), # invalid inventory location (single input, bad string) + 'bad-location-3': ('c.example', [2, 'x']), # invalid inventory location (sequence input, bad type) + 'bad-location-4': ('d.example', ['y', '']), # invalid inventory location (sequence input, bad string) + 'good-target-1': ('e.example', None), # valid inventory location (None) + 'good-target-2': ('f.example', ('x',)), # valid inventory location (sequence input) # fmt: on - }) - - # normalize the inventory and check if it's done correctly - normalize_intersphinx_mapping(app, app.config) + } + set_config(app, bad_intersphinx_mapping) + + # normalise the inventory and check if it's done correctly + with pytest.raises( + ConfigError, + match=r'Invalid `intersphinx_mapping` configuration \(15 errors\).', + ): + normalize_intersphinx_mapping(app, app.config) warnings = strip_colors(warning.getvalue()).splitlines() - assert len(warnings) == len(bad_intersphinx_mapping) - 1 - for index, messages in enumerate(( - "ignoring empty intersphinx identifier", - 'intersphinx identifier 12345 is not string. Ignored', - "intersphinx_mapping['bad-dict-item']: expecting a tuple or a list, got: 0; ignoring.", - "Failed to read intersphinx_mapping[unpack-except-1], ignored: ValueError('not enough values to unpack (expected 2, got 1)')", - "Failed to read intersphinx_mapping[unpack-except-2], ignored: NotImplementedError()", - "intersphinx_mapping['bad-uri-type-1']: URI must be a non-empty string, got: 123456789; ignoring.", - "intersphinx_mapping['bad-uri-type-2']: URI must be a non-empty string, got: None; ignoring.", - "intersphinx_mapping['bad-uri-value']: URI must be a non-empty string, got: ''; ignoring.", - "intersphinx_mapping['dedup-good']: URI 'foo.com' shadows URI from intersphinx_mapping['good']; ignoring.", - "intersphinx_mapping['bad-target-1']: inventory location must be a non-empty string or None, got: 1; ignoring.", - "intersphinx_mapping['bad-target-2']: inventory location must be a non-empty string or None, got: ''; ignoring.", - "intersphinx_mapping['bad-target-3']: inventory location must be a non-empty string or None, got: 2; ignoring.", - "intersphinx_mapping['bad-target-4']: inventory location must be a non-empty string or None, got: ''; ignoring.", - )): - assert messages in warnings[index] + assert len(warnings) == len(bad_intersphinx_mapping) - 3 + assert list(enumerate(warnings)) == list(enumerate(( + "ERROR: Invalid value '' in intersphinx_mapping['']: expected an intersphinx project identifier", + "ERROR: Invalid value 12345 in intersphinx_mapping[12345]: project identifier must be a string", + "ERROR: Invalid value None in intersphinx_mapping[None]: project identifier must be a string", + "ERROR: Invalid value 'inventory' in intersphinx_mapping['https://example/']: expected a tuple or a list", + "ERROR: Invalid value 0 in intersphinx_mapping['bad-dict-item']: expected a tuple or a list", + "ERROR: Invalid value [0] in intersphinx_mapping['unpack-except-1']: values must be a (target URI, inventory locations) pair", + "ERROR: Invalid value [] in intersphinx_mapping['unpack-except-2']: values must be a (target URI, inventory locations) pair", + "ERROR: Invalid value 123456789 in intersphinx_mapping['bad-uri-type-1']: target URI must be a non-empty string", + "ERROR: Invalid value None in intersphinx_mapping['bad-uri-type-2']: target URI must be a non-empty string", + "ERROR: Invalid value '' in intersphinx_mapping['bad-uri-value']: target URI must be a non-empty string", + "ERROR: Invalid value 'example.org' in intersphinx_mapping['dedup-good']: target URI must be unique (other instance in `intersphinx_mapping['good']`)", + "ERROR: Invalid value 1 in intersphinx_mapping['bad-location-1']: inventory location must be a non-empty string or None", + "ERROR: Invalid value '' in intersphinx_mapping['bad-location-2']: inventory location must be a non-empty string or None", + "ERROR: Invalid value 2 in intersphinx_mapping['bad-location-3']: inventory location must be a non-empty string or None", + "ERROR: Invalid value '' in intersphinx_mapping['bad-location-4']: inventory location must be a non-empty string or None", + ))) def test_load_mappings_fallback(tmp_path, app, status, warning): From a28674a78aaa2ca3174cbd9993b2420d80d23314 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?B=C3=A9n=C3=A9dikt=20Tran?= <10796600+picnixz@users.noreply.github.com> Date: Sun, 21 Jul 2024 12:19:16 +0200 Subject: [PATCH 29/52] Update test_ext_inheritance_diagram.py --- tests/test_extensions/test_ext_inheritance_diagram.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/test_extensions/test_ext_inheritance_diagram.py b/tests/test_extensions/test_ext_inheritance_diagram.py index 45a5ff0dda9..1aa78d285da 100644 --- a/tests/test_extensions/test_ext_inheritance_diagram.py +++ b/tests/test_extensions/test_ext_inheritance_diagram.py @@ -154,7 +154,7 @@ def test_inheritance_diagram_png_html(tmp_path, app): inv_file = tmp_path / 'inventory' inv_file.write_bytes(external_inventory) app.config.intersphinx_mapping = { - 'https://example.org': str(inv_file), + 'example': ("https://example.org", str(inv_file)), } app.config.intersphinx_cache_limit = 0 normalize_intersphinx_mapping(app, app.config) From 864713b97aa28786def88197cd19604bb9cdd399 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?B=C3=A9n=C3=A9dikt=20Tran?= <10796600+picnixz@users.noreply.github.com> Date: Sun, 21 Jul 2024 12:19:52 +0200 Subject: [PATCH 30/52] Update sphinx/ext/intersphinx/_load.py --- sphinx/ext/intersphinx/_load.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/sphinx/ext/intersphinx/_load.py b/sphinx/ext/intersphinx/_load.py index 985c7887e16..3bcfd3d8726 100644 --- a/sphinx/ext/intersphinx/_load.py +++ b/sphinx/ext/intersphinx/_load.py @@ -92,7 +92,7 @@ def normalize_intersphinx_mapping(app: Sphinx, config: Config) -> None: config.intersphinx_mapping[name] = (name, (uri, tuple(targets))) if len(errors) > 0: - for (msg, name, value) in errors: + for msg, name, value in errors: error_msg = __('Invalid value %r in intersphinx_mapping[%r]: %s') LOGGER.error(error_msg % (value, name, msg)) msg = __('Invalid `intersphinx_mapping` configuration (%s errors).') From cfe69ba7579452ee5a04a38cfaae841bdbc57357 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?B=C3=A9n=C3=A9dikt=20Tran?= <10796600+picnixz@users.noreply.github.com> Date: Sun, 21 Jul 2024 12:19:58 +0200 Subject: [PATCH 31/52] Update sphinx/ext/intersphinx/_load.py --- sphinx/ext/intersphinx/_load.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/sphinx/ext/intersphinx/_load.py b/sphinx/ext/intersphinx/_load.py index 3bcfd3d8726..ce42e2d7965 100644 --- a/sphinx/ext/intersphinx/_load.py +++ b/sphinx/ext/intersphinx/_load.py @@ -91,7 +91,7 @@ def normalize_intersphinx_mapping(app: Sphinx, config: Config) -> None: config.intersphinx_mapping[name] = (name, (uri, tuple(targets))) - if len(errors) > 0: + if errors: for msg, name, value in errors: error_msg = __('Invalid value %r in intersphinx_mapping[%r]: %s') LOGGER.error(error_msg % (value, name, msg)) From bea91dc9cc878cc11cd4d7441f43c75d5f36a534 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?B=C3=A9n=C3=A9dikt=20Tran?= <10796600+picnixz@users.noreply.github.com> Date: Sun, 21 Jul 2024 12:20:31 +0200 Subject: [PATCH 32/52] Update tests/test_extensions/test_ext_inheritance_diagram.py --- tests/test_extensions/test_ext_inheritance_diagram.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/test_extensions/test_ext_inheritance_diagram.py b/tests/test_extensions/test_ext_inheritance_diagram.py index 1aa78d285da..19423c2c5c1 100644 --- a/tests/test_extensions/test_ext_inheritance_diagram.py +++ b/tests/test_extensions/test_ext_inheritance_diagram.py @@ -154,7 +154,7 @@ def test_inheritance_diagram_png_html(tmp_path, app): inv_file = tmp_path / 'inventory' inv_file.write_bytes(external_inventory) app.config.intersphinx_mapping = { - 'example': ("https://example.org", str(inv_file)), + 'example': ('https://example.org', str(inv_file)), } app.config.intersphinx_cache_limit = 0 normalize_intersphinx_mapping(app, app.config) From 8ada5cd299b29208a149ef9372e7963b29b2d3b8 Mon Sep 17 00:00:00 2001 From: Adam Turner <9087854+aa-turner@users.noreply.github.com> Date: Sun, 21 Jul 2024 22:19:09 +0100 Subject: [PATCH 33/52] Better error messages --- sphinx/ext/intersphinx/_load.py | 49 +++++++++++-------- tests/test_extensions/test_ext_intersphinx.py | 40 ++++++++------- 2 files changed, 49 insertions(+), 40 deletions(-) diff --git a/sphinx/ext/intersphinx/_load.py b/sphinx/ext/intersphinx/_load.py index ce42e2d7965..3d0a4722f5e 100644 --- a/sphinx/ext/intersphinx/_load.py +++ b/sphinx/ext/intersphinx/_load.py @@ -38,43 +38,49 @@ def normalize_intersphinx_mapping(app: Sphinx, config: Config) -> None: # depending on which one is inserted last in the cache. seen: dict[InventoryURI, InventoryName] = {} - errors = [] + errors = 0 for name, value in config.intersphinx_mapping.copy().items(): # ensure that intersphinx projects are always named if not isinstance(name, str): - msg = __('project identifier must be a string') - errors.append((msg, name, name)) + errors += 1 + msg = __('Invalid intersphinx project identifier `%r` in intersphinx_mapping. Project identifiers must be non-empty strings.') + LOGGER.error(msg % name) del config.intersphinx_mapping[name] continue if not name: - msg = __('expected an intersphinx project identifier') - errors.append((msg, name, name)) + errors += 1 + msg = __('Invalid intersphinx project identifier `%r` in intersphinx_mapping. Project identifiers must be non-empty strings.') + LOGGER.error(msg % name) del config.intersphinx_mapping[name] continue # ensure values are properly formatted if not isinstance(value, (tuple, list)): - msg = __('expected a tuple or a list') - errors.append((msg, name, value)) + errors += 1 + msg = __('Invalid value `%r` in intersphinx_mapping[%r]. Expected a two-element tuple or list.') + LOGGER.error(msg % (value, name)) del config.intersphinx_mapping[name] continue try: uri, inv = value - except Exception: - msg = __('values must be a (target URI, inventory locations) pair') - errors.append((msg, name, value)) + except (TypeError, ValueError, Exception): + errors += 1 + msg = __('Invalid value `%r` in intersphinx_mapping[%r]. Values must be a (target URI, inventory locations) pair.') + LOGGER.error(msg % (value, name)) del config.intersphinx_mapping[name] continue # ensure target URIs are non-empty and unique if not uri or not isinstance(uri, str): - msg = __('target URI must be a non-empty string') - errors.append((msg, name, uri)) + errors += 1 + msg = __('Invalid target URI value `%r` in intersphinx_mapping[%r][0]. Target URIs must be unique non-empty strings.') + LOGGER.error(msg % (uri, name)) del config.intersphinx_mapping[name] continue if (name_for_uri := seen.setdefault(uri, name)) != name: - msg = __('target URI must be unique (other instance in `intersphinx_mapping[%r]`)') - errors.append((msg % name_for_uri, name, uri)) + errors += 1 + msg = __('Invalid target URI value `%r` in intersphinx_mapping[%r][0]. Target URIs must be unique (other instance in intersphinx_mapping[%r]).') + LOGGER.error(msg % (uri, name, name_for_uri)) del config.intersphinx_mapping[name] continue @@ -84,19 +90,20 @@ def normalize_intersphinx_mapping(app: Sphinx, config: Config) -> None: if target is None or target and isinstance(target, str): targets.append(target) else: - msg = __('inventory location must be a non-empty string or None') - errors.append((msg, name, target)) + errors += 1 + msg = __('Invalid inventory location value `%r` in intersphinx_mapping[%r][1]. Inventory locations must be non-empty strings or None.') + LOGGER.error(msg % (target, name)) del config.intersphinx_mapping[name] continue config.intersphinx_mapping[name] = (name, (uri, tuple(targets))) - if errors: - for msg, name, value in errors: - error_msg = __('Invalid value %r in intersphinx_mapping[%r]: %s') - LOGGER.error(error_msg % (value, name, msg)) + if errors == 1: + msg = __('Invalid `intersphinx_mapping` configuration (1 error).') + raise ConfigError(msg) + if errors > 1: msg = __('Invalid `intersphinx_mapping` configuration (%s errors).') - raise ConfigError(msg % len(errors)) + raise ConfigError(msg % errors) def load_mappings(app: Sphinx) -> None: diff --git a/tests/test_extensions/test_ext_intersphinx.py b/tests/test_extensions/test_ext_intersphinx.py index 529315d8f55..21500a31fb1 100644 --- a/tests/test_extensions/test_ext_intersphinx.py +++ b/tests/test_extensions/test_ext_intersphinx.py @@ -420,7 +420,8 @@ def test_normalize_intersphinx_mapping_warnings(app, warning): '': ('789.example', None), # invalid project name (value) 12345: ('456.example', None), # invalid project name (type) None: ('123.example', None), # invalid project name (type) - 'https://example/': 'inventory', # Sphinx 0.x style value + 'https://example/': 'inventory', # Sphinx 0.x style value (str) + 'https://server/': None, # Sphinx 0.x style value (None) 'bad-dict-item': 0, # invalid dict item type 'unpack-except-1': [0], # invalid dict item size (native ValueError) 'unpack-except-2': FakeList(), # invalid dict item size (custom exception) @@ -442,28 +443,29 @@ def test_normalize_intersphinx_mapping_warnings(app, warning): # normalise the inventory and check if it's done correctly with pytest.raises( ConfigError, - match=r'Invalid `intersphinx_mapping` configuration \(15 errors\).', + match=r'Invalid `intersphinx_mapping` configuration \(16 errors\).', ): normalize_intersphinx_mapping(app, app.config) warnings = strip_colors(warning.getvalue()).splitlines() assert len(warnings) == len(bad_intersphinx_mapping) - 3 - assert list(enumerate(warnings)) == list(enumerate(( - "ERROR: Invalid value '' in intersphinx_mapping['']: expected an intersphinx project identifier", - "ERROR: Invalid value 12345 in intersphinx_mapping[12345]: project identifier must be a string", - "ERROR: Invalid value None in intersphinx_mapping[None]: project identifier must be a string", - "ERROR: Invalid value 'inventory' in intersphinx_mapping['https://example/']: expected a tuple or a list", - "ERROR: Invalid value 0 in intersphinx_mapping['bad-dict-item']: expected a tuple or a list", - "ERROR: Invalid value [0] in intersphinx_mapping['unpack-except-1']: values must be a (target URI, inventory locations) pair", - "ERROR: Invalid value [] in intersphinx_mapping['unpack-except-2']: values must be a (target URI, inventory locations) pair", - "ERROR: Invalid value 123456789 in intersphinx_mapping['bad-uri-type-1']: target URI must be a non-empty string", - "ERROR: Invalid value None in intersphinx_mapping['bad-uri-type-2']: target URI must be a non-empty string", - "ERROR: Invalid value '' in intersphinx_mapping['bad-uri-value']: target URI must be a non-empty string", - "ERROR: Invalid value 'example.org' in intersphinx_mapping['dedup-good']: target URI must be unique (other instance in `intersphinx_mapping['good']`)", - "ERROR: Invalid value 1 in intersphinx_mapping['bad-location-1']: inventory location must be a non-empty string or None", - "ERROR: Invalid value '' in intersphinx_mapping['bad-location-2']: inventory location must be a non-empty string or None", - "ERROR: Invalid value 2 in intersphinx_mapping['bad-location-3']: inventory location must be a non-empty string or None", - "ERROR: Invalid value '' in intersphinx_mapping['bad-location-4']: inventory location must be a non-empty string or None", - ))) + assert warnings == [ + "ERROR: Invalid intersphinx project identifier `''` in intersphinx_mapping. Project identifiers must be non-empty strings.", + "ERROR: Invalid intersphinx project identifier `12345` in intersphinx_mapping. Project identifiers must be non-empty strings.", + "ERROR: Invalid intersphinx project identifier `None` in intersphinx_mapping. Project identifiers must be non-empty strings.", + "ERROR: Invalid value `'inventory'` in intersphinx_mapping['https://example/']. Expected a two-element tuple or list.", + "ERROR: Invalid value `None` in intersphinx_mapping['https://server/']. Expected a two-element tuple or list.", + "ERROR: Invalid value `0` in intersphinx_mapping['bad-dict-item']. Expected a two-element tuple or list.", + "ERROR: Invalid value `[0]` in intersphinx_mapping['unpack-except-1']. Values must be a (target URI, inventory locations) pair.", + "ERROR: Invalid value `[]` in intersphinx_mapping['unpack-except-2']. Values must be a (target URI, inventory locations) pair.", + "ERROR: Invalid target URI value `123456789` in intersphinx_mapping['bad-uri-type-1'][0]. Target URIs must be unique non-empty strings.", + "ERROR: Invalid target URI value `None` in intersphinx_mapping['bad-uri-type-2'][0]. Target URIs must be unique non-empty strings.", + "ERROR: Invalid target URI value `''` in intersphinx_mapping['bad-uri-value'][0]. Target URIs must be unique non-empty strings.", + "ERROR: Invalid target URI value `'example.org'` in intersphinx_mapping['dedup-good'][0]. Target URIs must be unique (other instance in intersphinx_mapping['good']).", + "ERROR: Invalid inventory location value `1` in intersphinx_mapping['bad-location-1'][1]. Inventory locations must be non-empty strings or None.", + "ERROR: Invalid inventory location value `''` in intersphinx_mapping['bad-location-2'][1]. Inventory locations must be non-empty strings or None.", + "ERROR: Invalid inventory location value `2` in intersphinx_mapping['bad-location-3'][1]. Inventory locations must be non-empty strings or None.", + "ERROR: Invalid inventory location value `''` in intersphinx_mapping['bad-location-4'][1]. Inventory locations must be non-empty strings or None." + ] def test_load_mappings_fallback(tmp_path, app, status, warning): From 9dd36d1e2f81b2b7f776f21f41410b833d418fc5 Mon Sep 17 00:00:00 2001 From: Adam Turner <9087854+aa-turner@users.noreply.github.com> Date: Mon, 22 Jul 2024 12:13:00 +0100 Subject: [PATCH 34/52] Formatting --- sphinx/ext/intersphinx/_load.py | 38 ++++++++++++++----- tests/test_extensions/test_ext_intersphinx.py | 8 ++-- 2 files changed, 33 insertions(+), 13 deletions(-) diff --git a/sphinx/ext/intersphinx/_load.py b/sphinx/ext/intersphinx/_load.py index 3d0a4722f5e..7e1c982cde9 100644 --- a/sphinx/ext/intersphinx/_load.py +++ b/sphinx/ext/intersphinx/_load.py @@ -43,13 +43,19 @@ def normalize_intersphinx_mapping(app: Sphinx, config: Config) -> None: # ensure that intersphinx projects are always named if not isinstance(name, str): errors += 1 - msg = __('Invalid intersphinx project identifier `%r` in intersphinx_mapping. Project identifiers must be non-empty strings.') + msg = __( + 'Invalid intersphinx project identifier `%r` in intersphinx_mapping. ' + 'Project identifiers must be non-empty strings.' + ) LOGGER.error(msg % name) del config.intersphinx_mapping[name] continue if not name: errors += 1 - msg = __('Invalid intersphinx project identifier `%r` in intersphinx_mapping. Project identifiers must be non-empty strings.') + msg = __( + 'Invalid intersphinx project identifier `%r` in intersphinx_mapping. ' + 'Project identifiers must be non-empty strings.' + ) LOGGER.error(msg % name) del config.intersphinx_mapping[name] continue @@ -57,7 +63,10 @@ def normalize_intersphinx_mapping(app: Sphinx, config: Config) -> None: # ensure values are properly formatted if not isinstance(value, (tuple, list)): errors += 1 - msg = __('Invalid value `%r` in intersphinx_mapping[%r]. Expected a two-element tuple or list.') + msg = __( + 'Invalid value `%r` in intersphinx_mapping[%r]. ' + 'Expected a two-element tuple or list.' + ) LOGGER.error(msg % (value, name)) del config.intersphinx_mapping[name] continue @@ -65,7 +74,10 @@ def normalize_intersphinx_mapping(app: Sphinx, config: Config) -> None: uri, inv = value except (TypeError, ValueError, Exception): errors += 1 - msg = __('Invalid value `%r` in intersphinx_mapping[%r]. Values must be a (target URI, inventory locations) pair.') + msg = __( + 'Invalid value `%r` in intersphinx_mapping[%r]. ' + 'Values must be a (target URI, inventory locations) pair.' + ) LOGGER.error(msg % (value, name)) del config.intersphinx_mapping[name] continue @@ -73,16 +85,21 @@ def normalize_intersphinx_mapping(app: Sphinx, config: Config) -> None: # ensure target URIs are non-empty and unique if not uri or not isinstance(uri, str): errors += 1 - msg = __('Invalid target URI value `%r` in intersphinx_mapping[%r][0]. Target URIs must be unique non-empty strings.') + msg = __('Invalid target URI value `%r` in intersphinx_mapping[%r][0]. ' + 'Target URIs must be unique non-empty strings.') LOGGER.error(msg % (uri, name)) del config.intersphinx_mapping[name] continue - if (name_for_uri := seen.setdefault(uri, name)) != name: + if uri in seen: errors += 1 - msg = __('Invalid target URI value `%r` in intersphinx_mapping[%r][0]. Target URIs must be unique (other instance in intersphinx_mapping[%r]).') - LOGGER.error(msg % (uri, name, name_for_uri)) + msg = __( + 'Invalid target URI value `%r` in intersphinx_mapping[%r][0]. ' + 'Target URIs must be unique (other instance in intersphinx_mapping[%r]).' + ) + LOGGER.error(msg % (uri, name, seen[uri])) del config.intersphinx_mapping[name] continue + seen[uri] = name # ensure inventory locations are None or non-empty targets: list[InventoryLocation] = [] @@ -91,7 +108,10 @@ def normalize_intersphinx_mapping(app: Sphinx, config: Config) -> None: targets.append(target) else: errors += 1 - msg = __('Invalid inventory location value `%r` in intersphinx_mapping[%r][1]. Inventory locations must be non-empty strings or None.') + msg = __( + 'Invalid inventory location value `%r` in intersphinx_mapping[%r][1]. ' + 'Inventory locations must be non-empty strings or None.' + ) LOGGER.error(msg % (target, name)) del config.intersphinx_mapping[name] continue diff --git a/tests/test_extensions/test_ext_intersphinx.py b/tests/test_extensions/test_ext_intersphinx.py index 21500a31fb1..dc644eb97e3 100644 --- a/tests/test_extensions/test_ext_intersphinx.py +++ b/tests/test_extensions/test_ext_intersphinx.py @@ -420,8 +420,8 @@ def test_normalize_intersphinx_mapping_warnings(app, warning): '': ('789.example', None), # invalid project name (value) 12345: ('456.example', None), # invalid project name (type) None: ('123.example', None), # invalid project name (type) - 'https://example/': 'inventory', # Sphinx 0.x style value (str) - 'https://server/': None, # Sphinx 0.x style value (None) + 'https://example/': None, # Sphinx 0.x style value (None) + 'https://server/': 'inventory', # Sphinx 0.x style value (str) 'bad-dict-item': 0, # invalid dict item type 'unpack-except-1': [0], # invalid dict item size (native ValueError) 'unpack-except-2': FakeList(), # invalid dict item size (custom exception) @@ -452,8 +452,8 @@ def test_normalize_intersphinx_mapping_warnings(app, warning): "ERROR: Invalid intersphinx project identifier `''` in intersphinx_mapping. Project identifiers must be non-empty strings.", "ERROR: Invalid intersphinx project identifier `12345` in intersphinx_mapping. Project identifiers must be non-empty strings.", "ERROR: Invalid intersphinx project identifier `None` in intersphinx_mapping. Project identifiers must be non-empty strings.", - "ERROR: Invalid value `'inventory'` in intersphinx_mapping['https://example/']. Expected a two-element tuple or list.", - "ERROR: Invalid value `None` in intersphinx_mapping['https://server/']. Expected a two-element tuple or list.", + "ERROR: Invalid value `None` in intersphinx_mapping['https://example/']. Expected a two-element tuple or list.", + "ERROR: Invalid value `'inventory'` in intersphinx_mapping['https://server/']. Expected a two-element tuple or list.", "ERROR: Invalid value `0` in intersphinx_mapping['bad-dict-item']. Expected a two-element tuple or list.", "ERROR: Invalid value `[0]` in intersphinx_mapping['unpack-except-1']. Values must be a (target URI, inventory locations) pair.", "ERROR: Invalid value `[]` in intersphinx_mapping['unpack-except-2']. Values must be a (target URI, inventory locations) pair.", From c33db5c40d63030cd0538900e649d189dbc1259d Mon Sep 17 00:00:00 2001 From: Adam Turner <9087854+aa-turner@users.noreply.github.com> Date: Mon, 22 Jul 2024 12:16:10 +0100 Subject: [PATCH 35/52] Explicit copy --- tests/test_extensions/test_ext_intersphinx.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/test_extensions/test_ext_intersphinx.py b/tests/test_extensions/test_ext_intersphinx.py index dc644eb97e3..571e0bf563c 100644 --- a/tests/test_extensions/test_ext_intersphinx.py +++ b/tests/test_extensions/test_ext_intersphinx.py @@ -57,7 +57,7 @@ def reference_check(app, *args, **kwds): def set_config(app, mapping): # copy *mapping* so that normalization does not alter it - app.config.intersphinx_mapping = dict(mapping) + app.config.intersphinx_mapping = mapping.copy() app.config.intersphinx_cache_limit = 0 app.config.intersphinx_disabled_reftypes = [] @@ -443,7 +443,7 @@ def test_normalize_intersphinx_mapping_warnings(app, warning): # normalise the inventory and check if it's done correctly with pytest.raises( ConfigError, - match=r'Invalid `intersphinx_mapping` configuration \(16 errors\).', + match=r'^Invalid `intersphinx_mapping` configuration \(16 errors\).$', ): normalize_intersphinx_mapping(app, app.config) warnings = strip_colors(warning.getvalue()).splitlines() From 35144c8416d235669c387355d986f2746f5782ac Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?B=C3=A9n=C3=A9dikt=20Tran?= <10796600+picnixz@users.noreply.github.com> Date: Mon, 22 Jul 2024 13:17:31 +0200 Subject: [PATCH 36/52] update ENV_VERSION --- 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 deb6af7975a..4539c1eab6a 100644 --- a/sphinx/environment/__init__.py +++ b/sphinx/environment/__init__.py @@ -59,7 +59,7 @@ # This is increased every time an environment attribute is added # or changed to properly invalidate pickle files. -ENV_VERSION = 62 +ENV_VERSION = 63 # config status CONFIG_UNSET = -1 From 487099c6782c13d8a63580a1c731b0fd01ef88a4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?B=C3=A9n=C3=A9dikt=20Tran?= <10796600+picnixz@users.noreply.github.com> Date: Mon, 22 Jul 2024 13:24:24 +0200 Subject: [PATCH 37/52] update ENV_VERSION --- tests/js/fixtures/cpp/searchindex.js | 2 +- tests/js/fixtures/multiterm/searchindex.js | 2 +- tests/js/fixtures/partial/searchindex.js | 2 +- tests/js/fixtures/titles/searchindex.js | 2 +- 4 files changed, 4 insertions(+), 4 deletions(-) diff --git a/tests/js/fixtures/cpp/searchindex.js b/tests/js/fixtures/cpp/searchindex.js index 46f48244741..78e5f761485 100644 --- a/tests/js/fixtures/cpp/searchindex.js +++ b/tests/js/fixtures/cpp/searchindex.js @@ -1 +1 @@ -Search.setIndex({"alltitles": {}, "docnames": ["index"], "envversion": {"sphinx": 62, "sphinx.domains.c": 3, "sphinx.domains.changeset": 1, "sphinx.domains.citation": 1, "sphinx.domains.cpp": 9, "sphinx.domains.index": 1, "sphinx.domains.javascript": 3, "sphinx.domains.math": 2, "sphinx.domains.python": 4, "sphinx.domains.rst": 2, "sphinx.domains.std": 2}, "filenames": ["index.rst"], "indexentries": {"sphinx (c++ class)": [[0, "_CPPv46Sphinx", false]]}, "objects": {"": [[0, 0, 1, "_CPPv46Sphinx", "Sphinx"]]}, "objnames": {"0": ["cpp", "class", "C++ class"]}, "objtypes": {"0": "cpp:class"}, "terms": {"The": 0, "becaus": 0, "c": 0, "can": 0, "cardin": 0, "challeng": 0, "charact": 0, "class": 0, "descript": 0, "drop": 0, "engin": 0, "fixtur": 0, "frequent": 0, "gener": 0, "i": 0, "index": 0, "inflat": 0, "mathemat": 0, "occur": 0, "often": 0, "project": 0, "punctuat": 0, "queri": 0, "relat": 0, "sampl": 0, "search": 0, "size": 0, "sphinx": 0, "term": 0, "thei": 0, "thi": 0, "token": 0, "us": 0, "web": 0, "would": 0}, "titles": ["<no title>"], "titleterms": {}}) \ No newline at end of file +Search.setIndex({"alltitles": {}, "docnames": ["index"], "envversion": {"sphinx": 63, "sphinx.domains.c": 3, "sphinx.domains.changeset": 1, "sphinx.domains.citation": 1, "sphinx.domains.cpp": 9, "sphinx.domains.index": 1, "sphinx.domains.javascript": 3, "sphinx.domains.math": 2, "sphinx.domains.python": 4, "sphinx.domains.rst": 2, "sphinx.domains.std": 2}, "filenames": ["index.rst"], "indexentries": {"sphinx (c++ class)": [[0, "_CPPv46Sphinx", false]]}, "objects": {"": [[0, 0, 1, "_CPPv46Sphinx", "Sphinx"]]}, "objnames": {"0": ["cpp", "class", "C++ class"]}, "objtypes": {"0": "cpp:class"}, "terms": {"The": 0, "becaus": 0, "c": 0, "can": 0, "cardin": 0, "challeng": 0, "charact": 0, "class": 0, "descript": 0, "drop": 0, "engin": 0, "fixtur": 0, "frequent": 0, "gener": 0, "i": 0, "index": 0, "inflat": 0, "mathemat": 0, "occur": 0, "often": 0, "project": 0, "punctuat": 0, "queri": 0, "relat": 0, "sampl": 0, "search": 0, "size": 0, "sphinx": 0, "term": 0, "thei": 0, "thi": 0, "token": 0, "us": 0, "web": 0, "would": 0}, "titles": ["<no title>"], "titleterms": {}}) \ No newline at end of file diff --git a/tests/js/fixtures/multiterm/searchindex.js b/tests/js/fixtures/multiterm/searchindex.js index a868eb6bdcb..96b093c5fda 100644 --- a/tests/js/fixtures/multiterm/searchindex.js +++ b/tests/js/fixtures/multiterm/searchindex.js @@ -1 +1 @@ -Search.setIndex({"alltitles": {"Main Page": [[0, null]]}, "docnames": ["index"], "envversion": {"sphinx": 62, "sphinx.domains.c": 3, "sphinx.domains.changeset": 1, "sphinx.domains.citation": 1, "sphinx.domains.cpp": 9, "sphinx.domains.index": 1, "sphinx.domains.javascript": 3, "sphinx.domains.math": 2, "sphinx.domains.python": 4, "sphinx.domains.rst": 2, "sphinx.domains.std": 2}, "filenames": ["index.rst"], "indexentries": {}, "objects": {}, "objnames": {}, "objtypes": {}, "terms": {"At": 0, "adjac": 0, "all": 0, "an": 0, "appear": 0, "applic": 0, "ar": 0, "built": 0, "can": 0, "check": 0, "contain": 0, "do": 0, "document": 0, "doesn": 0, "each": 0, "fixtur": 0, "format": 0, "function": 0, "futur": 0, "html": 0, "i": 0, "includ": 0, "match": 0, "messag": 0, "multipl": 0, "multiterm": 0, "order": 0, "other": 0, "output": 0, "perform": 0, "perhap": 0, "phrase": 0, "project": 0, "queri": 0, "requir": 0, "same": 0, "search": 0, "successfulli": 0, "support": 0, "t": 0, "term": 0, "test": 0, "thi": 0, "time": 0, "us": 0, "when": 0, "write": 0}, "titles": ["Main Page"], "titleterms": {"main": 0, "page": 0}}) \ No newline at end of file +Search.setIndex({"alltitles": {"Main Page": [[0, null]]}, "docnames": ["index"], "envversion": {"sphinx": 63, "sphinx.domains.c": 3, "sphinx.domains.changeset": 1, "sphinx.domains.citation": 1, "sphinx.domains.cpp": 9, "sphinx.domains.index": 1, "sphinx.domains.javascript": 3, "sphinx.domains.math": 2, "sphinx.domains.python": 4, "sphinx.domains.rst": 2, "sphinx.domains.std": 2}, "filenames": ["index.rst"], "indexentries": {}, "objects": {}, "objnames": {}, "objtypes": {}, "terms": {"At": 0, "adjac": 0, "all": 0, "an": 0, "appear": 0, "applic": 0, "ar": 0, "built": 0, "can": 0, "check": 0, "contain": 0, "do": 0, "document": 0, "doesn": 0, "each": 0, "fixtur": 0, "format": 0, "function": 0, "futur": 0, "html": 0, "i": 0, "includ": 0, "match": 0, "messag": 0, "multipl": 0, "multiterm": 0, "order": 0, "other": 0, "output": 0, "perform": 0, "perhap": 0, "phrase": 0, "project": 0, "queri": 0, "requir": 0, "same": 0, "search": 0, "successfulli": 0, "support": 0, "t": 0, "term": 0, "test": 0, "thi": 0, "time": 0, "us": 0, "when": 0, "write": 0}, "titles": ["Main Page"], "titleterms": {"main": 0, "page": 0}}) \ No newline at end of file diff --git a/tests/js/fixtures/partial/searchindex.js b/tests/js/fixtures/partial/searchindex.js index 356386af8dd..dc6bc1cddbb 100644 --- a/tests/js/fixtures/partial/searchindex.js +++ b/tests/js/fixtures/partial/searchindex.js @@ -1 +1 @@ -Search.setIndex({"alltitles": {"sphinx_utils module": [[0, null]]}, "docnames": ["index"], "envversion": {"sphinx": 62, "sphinx.domains.c": 3, "sphinx.domains.changeset": 1, "sphinx.domains.citation": 1, "sphinx.domains.cpp": 9, "sphinx.domains.index": 1, "sphinx.domains.javascript": 3, "sphinx.domains.math": 2, "sphinx.domains.python": 4, "sphinx.domains.rst": 2, "sphinx.domains.std": 2}, "filenames": ["index.rst"], "indexentries": {}, "objects": {}, "objnames": {}, "objtypes": {}, "terms": {"also": 0, "ar": 0, "built": 0, "confirm": 0, "document": 0, "function": 0, "html": 0, "i": 0, "includ": 0, "input": 0, "javascript": 0, "known": 0, "match": 0, "partial": 0, "possibl": 0, "prefix": 0, "project": 0, "provid": 0, "restructuredtext": 0, "sampl": 0, "search": 0, "should": 0, "thi": 0, "titl": 0, "us": 0, "when": 0}, "titles": ["sphinx_utils module"], "titleterms": {"modul": 0, "sphinx_util": 0}}) \ No newline at end of file +Search.setIndex({"alltitles": {"sphinx_utils module": [[0, null]]}, "docnames": ["index"], "envversion": {"sphinx": 63, "sphinx.domains.c": 3, "sphinx.domains.changeset": 1, "sphinx.domains.citation": 1, "sphinx.domains.cpp": 9, "sphinx.domains.index": 1, "sphinx.domains.javascript": 3, "sphinx.domains.math": 2, "sphinx.domains.python": 4, "sphinx.domains.rst": 2, "sphinx.domains.std": 2}, "filenames": ["index.rst"], "indexentries": {}, "objects": {}, "objnames": {}, "objtypes": {}, "terms": {"also": 0, "ar": 0, "built": 0, "confirm": 0, "document": 0, "function": 0, "html": 0, "i": 0, "includ": 0, "input": 0, "javascript": 0, "known": 0, "match": 0, "partial": 0, "possibl": 0, "prefix": 0, "project": 0, "provid": 0, "restructuredtext": 0, "sampl": 0, "search": 0, "should": 0, "thi": 0, "titl": 0, "us": 0, "when": 0}, "titles": ["sphinx_utils module"], "titleterms": {"modul": 0, "sphinx_util": 0}}) \ No newline at end of file diff --git a/tests/js/fixtures/titles/searchindex.js b/tests/js/fixtures/titles/searchindex.js index 9a229d060bf..f503bf35df9 100644 --- a/tests/js/fixtures/titles/searchindex.js +++ b/tests/js/fixtures/titles/searchindex.js @@ -1 +1 @@ -Search.setIndex({"alltitles": {"Main Page": [[0, null]], "Relevance": [[0, "relevance"], [1, null]]}, "docnames": ["index", "relevance"], "envversion": {"sphinx": 62, "sphinx.domains.c": 3, "sphinx.domains.changeset": 1, "sphinx.domains.citation": 1, "sphinx.domains.cpp": 9, "sphinx.domains.index": 1, "sphinx.domains.javascript": 3, "sphinx.domains.math": 2, "sphinx.domains.python": 4, "sphinx.domains.rst": 2, "sphinx.domains.std": 2}, "filenames": ["index.rst", "relevance.rst"], "indexentries": {"example (class in relevance)": [[0, "relevance.Example", false]], "module": [[0, "module-relevance", false]], "relevance": [[0, "module-relevance", false]], "relevance (relevance.example attribute)": [[0, "relevance.Example.relevance", false]]}, "objects": {"": [[0, 0, 0, "-", "relevance"]], "relevance": [[0, 1, 1, "", "Example"]], "relevance.Example": [[0, 2, 1, "", "relevance"]]}, "objnames": {"0": ["py", "module", "Python module"], "1": ["py", "class", "Python class"], "2": ["py", "attribute", "Python attribute"]}, "objtypes": {"0": "py:module", "1": "py:class", "2": "py:attribute"}, "terms": {"": [0, 1], "A": 1, "For": 1, "In": [0, 1], "against": 0, "also": 1, "an": 0, "answer": 0, "appear": 1, "ar": 1, "area": 0, "ask": 0, "attribut": 0, "built": 1, "can": [0, 1], "class": 0, "code": [0, 1], "consid": 1, "contain": 0, "context": 0, "corpu": 1, "could": 1, "demonstr": 0, "describ": 1, "detail": 1, "determin": 1, "docstr": 0, "document": [0, 1], "domain": 1, "engin": 0, "exampl": [0, 1], "extract": 0, "find": 0, "found": 0, "from": 0, "function": 1, "ha": 1, "handl": 0, "happen": 1, "head": 0, "help": 0, "highli": 1, "how": 0, "i": [0, 1], "improv": 0, "inform": 0, "intend": 0, "issu": 1, "itself": 1, "knowledg": 0, "languag": 1, "less": 1, "like": [0, 1], "match": 0, "mention": 1, "name": [0, 1], "object": 0, "one": 1, "onli": 1, "other": 0, "page": 1, "part": 1, "particular": 0, "printf": 1, "program": 1, "project": 0, "queri": [0, 1], "question": 0, "re": 0, "rel": 0, "research": 0, "result": 1, "sai": 0, "same": 1, "score": 0, "search": [0, 1], "seem": 0, "softwar": 1, "some": 1, "sphinx": 0, "straightforward": 1, "subject": 0, "subsect": 0, "term": [0, 1], "test": 0, "text": 0, "than": 1, "thei": 0, "them": 0, "thi": 0, "titl": 0, "user": [0, 1], "we": [0, 1], "when": 0, "whether": 1, "within": 0, "would": 1}, "titles": ["Main Page", "Relevance"], "titleterms": {"main": 0, "page": 0, "relev": [0, 1]}}) \ No newline at end of file +Search.setIndex({"alltitles": {"Main Page": [[0, null]], "Relevance": [[0, "relevance"], [1, null]]}, "docnames": ["index", "relevance"], "envversion": {"sphinx": 63, "sphinx.domains.c": 3, "sphinx.domains.changeset": 1, "sphinx.domains.citation": 1, "sphinx.domains.cpp": 9, "sphinx.domains.index": 1, "sphinx.domains.javascript": 3, "sphinx.domains.math": 2, "sphinx.domains.python": 4, "sphinx.domains.rst": 2, "sphinx.domains.std": 2}, "filenames": ["index.rst", "relevance.rst"], "indexentries": {"example (class in relevance)": [[0, "relevance.Example", false]], "module": [[0, "module-relevance", false]], "relevance": [[0, "module-relevance", false]], "relevance (relevance.example attribute)": [[0, "relevance.Example.relevance", false]]}, "objects": {"": [[0, 0, 0, "-", "relevance"]], "relevance": [[0, 1, 1, "", "Example"]], "relevance.Example": [[0, 2, 1, "", "relevance"]]}, "objnames": {"0": ["py", "module", "Python module"], "1": ["py", "class", "Python class"], "2": ["py", "attribute", "Python attribute"]}, "objtypes": {"0": "py:module", "1": "py:class", "2": "py:attribute"}, "terms": {"": [0, 1], "A": 1, "For": 1, "In": [0, 1], "against": 0, "also": 1, "an": 0, "answer": 0, "appear": 1, "ar": 1, "area": 0, "ask": 0, "attribut": 0, "built": 1, "can": [0, 1], "class": 0, "code": [0, 1], "consid": 1, "contain": 0, "context": 0, "corpu": 1, "could": 1, "demonstr": 0, "describ": 1, "detail": 1, "determin": 1, "docstr": 0, "document": [0, 1], "domain": 1, "engin": 0, "exampl": [0, 1], "extract": 0, "find": 0, "found": 0, "from": 0, "function": 1, "ha": 1, "handl": 0, "happen": 1, "head": 0, "help": 0, "highli": 1, "how": 0, "i": [0, 1], "improv": 0, "inform": 0, "intend": 0, "issu": 1, "itself": 1, "knowledg": 0, "languag": 1, "less": 1, "like": [0, 1], "match": 0, "mention": 1, "name": [0, 1], "object": 0, "one": 1, "onli": 1, "other": 0, "page": 1, "part": 1, "particular": 0, "printf": 1, "program": 1, "project": 0, "queri": [0, 1], "question": 0, "re": 0, "rel": 0, "research": 0, "result": 1, "sai": 0, "same": 1, "score": 0, "search": [0, 1], "seem": 0, "softwar": 1, "some": 1, "sphinx": 0, "straightforward": 1, "subject": 0, "subsect": 0, "term": [0, 1], "test": 0, "text": 0, "than": 1, "thei": 0, "them": 0, "thi": 0, "titl": 0, "user": [0, 1], "we": [0, 1], "when": 0, "whether": 1, "within": 0, "would": 1}, "titles": ["Main Page", "Relevance"], "titleterms": {"main": 0, "page": 0, "relev": [0, 1]}}) \ No newline at end of file From b0b25de43bc0b98432b9e0c9e521faa2427b6eac Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?B=C3=A9n=C3=A9dikt=20Tran?= <10796600+picnixz@users.noreply.github.com> Date: Mon, 22 Jul 2024 14:16:15 +0200 Subject: [PATCH 38/52] fixup --- sphinx/ext/intersphinx/_resolve.py | 1 - 1 file changed, 1 deletion(-) diff --git a/sphinx/ext/intersphinx/_resolve.py b/sphinx/ext/intersphinx/_resolve.py index 3fc39f3f6a7..d6d350670db 100644 --- a/sphinx/ext/intersphinx/_resolve.py +++ b/sphinx/ext/intersphinx/_resolve.py @@ -143,7 +143,6 @@ def _resolve_reference_in_domain(env: BuildEnvironment, full_qualified_name, node, contnode) - def _resolve_reference(env: BuildEnvironment, inv_name: InventoryName | None, inventory: Inventory, honor_disabled_refs: bool, From 3b1126d120b83cc9747242d1d295956a74aca144 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?B=C3=A9n=C3=A9dikt=20Tran?= <10796600+picnixz@users.noreply.github.com> Date: Mon, 22 Jul 2024 14:21:53 +0200 Subject: [PATCH 39/52] remove duplicated test --- tests/test_extensions/test_ext_intersphinx.py | 27 ------------------- 1 file changed, 27 deletions(-) diff --git a/tests/test_extensions/test_ext_intersphinx.py b/tests/test_extensions/test_ext_intersphinx.py index 9af917e63dc..edc300a3821 100644 --- a/tests/test_extensions/test_ext_intersphinx.py +++ b/tests/test_extensions/test_ext_intersphinx.py @@ -184,33 +184,6 @@ def set_config(app, mapping): app.config.intersphinx_disabled_reftypes = [] -# TODO(picnixz): investigate why 'caplog' fixture does not work -@mock.patch("sphinx.ext.intersphinx._load.LOGGER") -def test_normalize_intersphinx_mapping(logger): - app = mock.Mock() - app.config.intersphinx_mapping = { - 'uri': None, # invalid format - 'foo': ('foo.com', None), # valid format - 'dup': ('foo.com', None), # duplicated URI - 'bar': ['bar.com', None], # uses [...] instead of (...) (OK) - } - - normalize_intersphinx_mapping(app, app.config) - - assert app.config.intersphinx_mapping == { - 'foo': ('foo', ('foo.com', (None,))), - 'bar': ('bar', ('bar.com', (None,))), - } - - assert len(logger.method_calls) == 2 - args = logger.method_calls[0].args - assert args[0] % args[1:] == ("intersphinx_mapping['uri']: expecting a tuple or a list, " - "got: None; ignoring.") - args = logger.method_calls[1].args - assert args[0] % args[1:] == ("intersphinx_mapping['dup']: URI 'foo.com' shadows URI " - "from intersphinx_mapping['foo']; ignoring.") - - @mock.patch('sphinx.ext.intersphinx._load.InventoryFile') @mock.patch('sphinx.ext.intersphinx._load._read_from_url') def test_fetch_inventory_redirection(_read_from_url, InventoryFile, app, status, warning): # NoQA: PT019 From e1e6a112ca17919f730ef626d9e21f69c26b698d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?B=C3=A9n=C3=A9dikt=20Tran?= <10796600+picnixz@users.noreply.github.com> Date: Mon, 22 Jul 2024 14:25:33 +0200 Subject: [PATCH 40/52] update tests --- tests/test_extensions/test_ext_intersphinx.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/test_extensions/test_ext_intersphinx.py b/tests/test_extensions/test_ext_intersphinx.py index edc300a3821..6eaabedaa60 100644 --- a/tests/test_extensions/test_ext_intersphinx.py +++ b/tests/test_extensions/test_ext_intersphinx.py @@ -102,7 +102,7 @@ def __init__( #: The escaped project version. self.safe_version = re.sub(r'\\s+', ' ', version) - #: The project base URL (e.g., http://localhost:8080). + #: The project base URL (e.g., http://localhost:7777). self.baseurl = baseurl #: The project base URI, relative to *baseurl* (e.g., 'foo'). self.uri = baseuri @@ -630,7 +630,7 @@ def test_load_mappings_cache_update(make_app, app_params): DOMAIN_NAME = 'py' OBJECT_TYPE = 'module' REFTYPE = f'{DOMAIN_NAME}:{OBJECT_TYPE}' - PORT = 7777 + PORT = 7777 # needd since otherwise it's an automatic port PROJECT_NAME, PROJECT_BASEURL = 'foo', f'http://localhost:{PORT}' old_project = IntersphinxProject(PROJECT_NAME, 1337, PROJECT_BASEURL, 'old') From 5b9693e6f4e29649bca09315164fd1d248fb233f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?B=C3=A9n=C3=A9dikt=20Tran?= <10796600+picnixz@users.noreply.github.com> Date: Mon, 22 Jul 2024 14:39:58 +0200 Subject: [PATCH 41/52] update typing --- sphinx/ext/intersphinx/_shared.py | 1 - 1 file changed, 1 deletion(-) diff --git a/sphinx/ext/intersphinx/_shared.py b/sphinx/ext/intersphinx/_shared.py index 98f9b13c410..2f34763e0b3 100644 --- a/sphinx/ext/intersphinx/_shared.py +++ b/sphinx/ext/intersphinx/_shared.py @@ -37,7 +37,6 @@ tuple[InventoryName, tuple[InventoryURI, tuple[InventoryLocation, ...]]], ] - LOGGER: Final[logging.SphinxLoggerAdapter] = logging.getLogger('sphinx.ext.intersphinx') From bd423ff4e5f1bf121d50692689b99fda1f10e343 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?B=C3=A9n=C3=A9dikt=20Tran?= <10796600+picnixz@users.noreply.github.com> Date: Mon, 22 Jul 2024 14:40:07 +0200 Subject: [PATCH 42/52] fix some error messages --- sphinx/ext/intersphinx/_load.py | 32 ++++++++------------------------ 1 file changed, 8 insertions(+), 24 deletions(-) diff --git a/sphinx/ext/intersphinx/_load.py b/sphinx/ext/intersphinx/_load.py index 5aa5750f3db..acda00a56eb 100644 --- a/sphinx/ext/intersphinx/_load.py +++ b/sphinx/ext/intersphinx/_load.py @@ -53,22 +53,13 @@ def validate_intersphinx_mapping(app: Sphinx, config: Config) -> None: errors = 0 for name, value in config.intersphinx_mapping.copy().items(): # ensure that intersphinx projects are always named - if not isinstance(name, str): + if not isinstance(name, str) or not name: errors += 1 msg = __( 'Invalid intersphinx project identifier `%r` in intersphinx_mapping. ' 'Project identifiers must be non-empty strings.' ) - LOGGER.error(msg % name) - del config.intersphinx_mapping[name] - continue - if not name: - errors += 1 - msg = __( - 'Invalid intersphinx project identifier `%r` in intersphinx_mapping. ' - 'Project identifiers must be non-empty strings.' - ) - LOGGER.error(msg % name) + LOGGER.error(msg, name) del config.intersphinx_mapping[name] continue @@ -79,18 +70,18 @@ def validate_intersphinx_mapping(app: Sphinx, config: Config) -> None: 'Invalid value `%r` in intersphinx_mapping[%r]. ' 'Expected a two-element tuple or list.' ) - LOGGER.error(msg % (value, name)) + LOGGER.error(msg, value, name) del config.intersphinx_mapping[name] continue try: uri, inv = value - except (TypeError, ValueError, Exception): + except Exception: errors += 1 msg = __( 'Invalid value `%r` in intersphinx_mapping[%r]. ' 'Values must be a (target URI, inventory locations) pair.' ) - LOGGER.error(msg % (value, name)) + LOGGER.error(msg, value, name) del config.intersphinx_mapping[name] continue @@ -99,7 +90,7 @@ def validate_intersphinx_mapping(app: Sphinx, config: Config) -> None: errors += 1 msg = __('Invalid target URI value `%r` in intersphinx_mapping[%r][0]. ' 'Target URIs must be unique non-empty strings.') - LOGGER.error(msg % (uri, name)) + LOGGER.error(msg, uri, name) del config.intersphinx_mapping[name] continue if uri in seen: @@ -108,7 +99,7 @@ def validate_intersphinx_mapping(app: Sphinx, config: Config) -> None: 'Invalid target URI value `%r` in intersphinx_mapping[%r][0]. ' 'Target URIs must be unique (other instance in intersphinx_mapping[%r]).' ) - LOGGER.error(msg % (uri, name, seen[uri])) + LOGGER.error(msg, uri, name, seen[uri]) del config.intersphinx_mapping[name] continue seen[uri] = name @@ -124,7 +115,7 @@ def validate_intersphinx_mapping(app: Sphinx, config: Config) -> None: 'Invalid inventory location value `%r` in intersphinx_mapping[%r][1]. ' 'Inventory locations must be non-empty strings or None.' ) - LOGGER.error(msg % (target, name)) + LOGGER.error(msg, target, name) del config.intersphinx_mapping[name] continue @@ -176,13 +167,6 @@ def load_mappings(app: Sphinx) -> None: # We can however order the cache by URIs for reproducibility. intersphinx_cache_values = sorted(intersphinx_cache.values(), key=itemgetter(0, 1)) for name, _timeout, invdata in intersphinx_cache_values: - if not name: - LOGGER.warning( - __('intersphinx cache seems corrupted, please rebuild ' - 'the project with the "-E" option (see sphinx --help)'), - ) - continue - inventories.named_inventory[name] = invdata for objtype, objects in invdata.items(): inventories.main_inventory.setdefault(objtype, {}).update(objects) From 9d1f3312d73b8ce1a56613e84e685d9ba4fd20e4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?B=C3=A9n=C3=A9dikt=20Tran?= <10796600+picnixz@users.noreply.github.com> Date: Mon, 22 Jul 2024 14:43:39 +0200 Subject: [PATCH 43/52] address Jay's review --- sphinx/ext/intersphinx/_load.py | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/sphinx/ext/intersphinx/_load.py b/sphinx/ext/intersphinx/_load.py index acda00a56eb..1e96050f9ed 100644 --- a/sphinx/ext/intersphinx/_load.py +++ b/sphinx/ext/intersphinx/_load.py @@ -164,9 +164,10 @@ def load_mappings(app: Sphinx) -> None: # Duplicate values in different inventories will shadow each # other and which one will override which varies between builds. # - # We can however order the cache by URIs for reproducibility. - intersphinx_cache_values = sorted(intersphinx_cache.values(), key=itemgetter(0, 1)) - for name, _timeout, invdata in intersphinx_cache_values: + # We can however order the cache by (NAME, EXPIRY) for reproducibility. + by_name_and_time = itemgetter(0, 1) # 0: name, 1: expiry + cache_values = sorted(intersphinx_cache.values(), key=by_name_and_time) + for name, _expiry, invdata in cache_values: inventories.named_inventory[name] = invdata for objtype, objects in invdata.items(): inventories.main_inventory.setdefault(objtype, {}).update(objects) From ea911739c41fce559d96c8c019fa27b1efbf0924 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?B=C3=A9n=C3=A9dikt=20Tran?= <10796600+picnixz@users.noreply.github.com> Date: Mon, 22 Jul 2024 15:44:15 +0200 Subject: [PATCH 44/52] . --- sphinx/ext/intersphinx/_load.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/sphinx/ext/intersphinx/_load.py b/sphinx/ext/intersphinx/_load.py index 1e96050f9ed..1c43605e3c8 100644 --- a/sphinx/ext/intersphinx/_load.py +++ b/sphinx/ext/intersphinx/_load.py @@ -75,7 +75,7 @@ def validate_intersphinx_mapping(app: Sphinx, config: Config) -> None: continue try: uri, inv = value - except Exception: + except (TypeError, ValueError, Exception): errors += 1 msg = __( 'Invalid value `%r` in intersphinx_mapping[%r]. ' From f5c075b587ce69ba27f25a2511bd620535ce7223 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?B=C3=A9n=C3=A9dikt=20Tran?= <10796600+picnixz@users.noreply.github.com> Date: Mon, 22 Jul 2024 17:01:38 +0200 Subject: [PATCH 45/52] revert style changes --- sphinx/ext/intersphinx/_resolve.py | 20 +++++++++----------- 1 file changed, 9 insertions(+), 11 deletions(-) diff --git a/sphinx/ext/intersphinx/_resolve.py b/sphinx/ext/intersphinx/_resolve.py index d6d350670db..730385a7359 100644 --- a/sphinx/ext/intersphinx/_resolve.py +++ b/sphinx/ext/intersphinx/_resolve.py @@ -203,11 +203,10 @@ def resolve_reference_in_inventory(env: BuildEnvironment, False, node, contnode) -def resolve_reference_any_inventory( - env: BuildEnvironment, - honor_disabled_refs: bool, - node: pending_xref, contnode: TextElement, -) -> nodes.reference | None: +def resolve_reference_any_inventory(env: BuildEnvironment, + honor_disabled_refs: bool, + node: pending_xref, contnode: TextElement, + ) -> nodes.reference | None: """Attempt to resolve a missing reference via intersphinx references. Resolution is tried with the target as is in any inventory. @@ -217,9 +216,9 @@ def resolve_reference_any_inventory( node, contnode) -def resolve_reference_detect_inventory( - env: BuildEnvironment, node: pending_xref, contnode: TextElement, -) -> nodes.reference | None: +def resolve_reference_detect_inventory(env: BuildEnvironment, + node: pending_xref, contnode: TextElement, + ) -> nodes.reference | None: """Attempt to resolve a missing reference via intersphinx references. Resolution is tried first with the target as is in any inventory. @@ -245,9 +244,8 @@ def resolve_reference_detect_inventory( return res_inv -def missing_reference( - app: Sphinx, env: BuildEnvironment, node: pending_xref, contnode: TextElement, -) -> nodes.reference | None: +def missing_reference(app: Sphinx, env: BuildEnvironment, node: pending_xref, + contnode: TextElement) -> nodes.reference | None: """Attempt to resolve a missing reference via intersphinx references.""" return resolve_reference_detect_inventory(env, node, contnode) From bfe45f58600643cf9781ba1bb0a1910b617c07f0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?B=C3=A9n=C3=A9dikt=20Tran?= <10796600+picnixz@users.noreply.github.com> Date: Mon, 22 Jul 2024 17:03:22 +0200 Subject: [PATCH 46/52] revert style changes --- sphinx/ext/intersphinx/_shared.py | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/sphinx/ext/intersphinx/_shared.py b/sphinx/ext/intersphinx/_shared.py index fdd1d20e1b4..c8c0689c145 100644 --- a/sphinx/ext/intersphinx/_shared.py +++ b/sphinx/ext/intersphinx/_shared.py @@ -2,13 +2,11 @@ from __future__ import annotations -from typing import TYPE_CHECKING +from typing import TYPE_CHECKING, Final from sphinx.util import logging if TYPE_CHECKING: - from typing import Final - from sphinx.environment import BuildEnvironment from sphinx.util.typing import Inventory From a90be51c891737c49f77d8d7333d6f609d01a6e8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?B=C3=A9n=C3=A9dikt=20Tran?= <10796600+picnixz@users.noreply.github.com> Date: Mon, 22 Jul 2024 17:04:09 +0200 Subject: [PATCH 47/52] address review --- sphinx/ext/intersphinx/_load.py | 39 ++++++++++++++------------------- 1 file changed, 17 insertions(+), 22 deletions(-) diff --git a/sphinx/ext/intersphinx/_load.py b/sphinx/ext/intersphinx/_load.py index edb11eda1cb..2f4341d9650 100644 --- a/sphinx/ext/intersphinx/_load.py +++ b/sphinx/ext/intersphinx/_load.py @@ -139,7 +139,7 @@ def load_mappings(app: Sphinx) -> None: intersphinx_cache: dict[InventoryURI, InventoryCacheEntry] = inventories.cache intersphinx_mapping: IntersphinxMapping = app.config.intersphinx_mapping - expected_uris = {uri for uri, _invs in app.config.intersphinx_mapping.values()} + expected_uris = {uri for _name, (uri, _invs) in intersphinx_mapping.values()} # If the current cache contains some (project, uri) pair # say ("foo", "foo.com") and if the new intersphinx dict @@ -183,34 +183,29 @@ def fetch_inventory_group( ) -> bool: cache_time = now - app.config.intersphinx_cache_limit * 86400 - def should_store(uri: str, inv: str) -> bool: - # decide whether the inventory must be read: always read local - # files; remote ones only if the cache time is expired - return '://' not in inv or uri not in cache or cache[uri][1] < cache_time - updated = False failures = [] for location in invs: inv: str = location or posixpath.join(uri, INVENTORY_FILENAME) - if not should_store(uri, inv): - continue - - safe_inv_url = _get_safe_url(inv) - inv_descriptor = name or 'main_inventory' - LOGGER.info(__("loading intersphinx inventory '%s' from %s..."), - inv_descriptor, safe_inv_url) + # decide whether the inventory must be read: always read local + # files; remote ones only if the cache time is expired + if '://' not in inv or uri not in cache or cache[uri][1] < cache_time: + safe_inv_url = _get_safe_url(inv) + inv_descriptor = name or 'main_inventory' + LOGGER.info(__("loading intersphinx inventory '%s' from %s..."), + inv_descriptor, safe_inv_url) - try: - invdata = fetch_inventory(app, uri, inv) - except Exception as err: - failures.append(err.args) - continue + try: + invdata = fetch_inventory(app, uri, inv) + except Exception as err: + failures.append(err.args) + continue - if invdata: - cache[uri] = name, now, invdata - updated = True - break + if invdata: + cache[uri] = name, now, invdata + updated = True + break if not failures: pass From 124f1c104ee5ea6962fe8ab3d3b5e503154e8e55 Mon Sep 17 00:00:00 2001 From: Adam Turner <9087854+aa-turner@users.noreply.github.com> Date: Mon, 22 Jul 2024 17:02:16 +0100 Subject: [PATCH 48/52] Updates --- sphinx/ext/intersphinx/_load.py | 39 ++++++++++++++++++--------------- 1 file changed, 21 insertions(+), 18 deletions(-) diff --git a/sphinx/ext/intersphinx/_load.py b/sphinx/ext/intersphinx/_load.py index 2f4341d9650..7b8a9ad0aa9 100644 --- a/sphinx/ext/intersphinx/_load.py +++ b/sphinx/ext/intersphinx/_load.py @@ -141,13 +141,14 @@ def load_mappings(app: Sphinx) -> None: expected_uris = {uri for _name, (uri, _invs) in intersphinx_mapping.values()} - # If the current cache contains some (project, uri) pair - # say ("foo", "foo.com") and if the new intersphinx dict - # contains the pair ("foo", "bar.com"), we need to remove - # the ("foo", "foo.com") entry and use ("foo", "bar.com"). for uri in frozenset(intersphinx_cache): - if intersphinx_cache[uri][0] not in intersphinx_mapping or uri not in expected_uris: - # remove a cached inventory if the latter is no more used by intersphinx + if intersphinx_cache[uri][0] not in intersphinx_mapping: + # Remove all cached entries that are no longer in `intersphinx_mapping`. + del intersphinx_cache[uri] + if uri not in expected_uris: + # Remove cached entries with a different target URI + # than the one in `intersphinx_mapping`. + # This happens when the URI in `intersphinx_mapping` is changed. del intersphinx_cache[uri] with concurrent.futures.ThreadPoolExecutor() as pool: @@ -162,9 +163,11 @@ def load_mappings(app: Sphinx) -> None: inventories.clear() # Duplicate values in different inventories will shadow each - # other and which one will override which varies between builds. + # other; which one will override which can vary between builds. # - # We can however order the cache by (NAME, EXPIRY) for reproducibility. + # In an attempt to make this more consistent, + # we sort the named inventories in the cache + # by their name and expiry time ``(NAME, EXPIRY)``. by_name_and_time = itemgetter(0, 1) # 0: name, 1: expiry cache_values = sorted(intersphinx_cache.values(), key=by_name_and_time) for name, _expiry, invdata in cache_values: @@ -187,14 +190,14 @@ def fetch_inventory_group( failures = [] for location in invs: - inv: str = location or posixpath.join(uri, INVENTORY_FILENAME) - # decide whether the inventory must be read: always read local - # files; remote ones only if the cache time is expired + # location is either None or a non-empty string + inv = f'{uri}/{INVENTORY_FILENAME}' if location is None else location + + # decide whether the inventory must be read: + # always read local files; remote ones only if the cache time is expired if '://' not in inv or uri not in cache or cache[uri][1] < cache_time: - safe_inv_url = _get_safe_url(inv) - inv_descriptor = name or 'main_inventory' LOGGER.info(__("loading intersphinx inventory '%s' from %s..."), - inv_descriptor, safe_inv_url) + name, _get_safe_url(inv)) try: invdata = fetch_inventory(app, uri, inv) @@ -210,14 +213,14 @@ def fetch_inventory_group( if not failures: pass elif len(failures) < len(invs): - LOGGER.info(__("encountered some issues with some of the inventories," - " but they had working alternatives:")) + LOGGER.info(__('encountered some issues with some of the inventories,' + ' but they had working alternatives:')) for fail in failures: LOGGER.info(*fail) else: issues = '\n'.join(f[0] % f[1:] for f in failures) - LOGGER.warning(__("failed to reach any of the inventories " - "with the following issues:") + "\n" + issues) + LOGGER.warning(__('failed to reach any of the inventories ' + 'with the following issues:') + '\n' + issues) return updated From 7f9b7b0fa4101b7fdc68f82d5edfe5b39cbf7cc7 Mon Sep 17 00:00:00 2001 From: Adam Turner <9087854+AA-Turner@users.noreply.github.com> Date: Mon, 22 Jul 2024 17:04:47 +0100 Subject: [PATCH 49/52] Update sphinx/ext/intersphinx/_load.py Co-authored-by: James Addison <55152140+jayaddison@users.noreply.github.com> --- sphinx/ext/intersphinx/_load.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/sphinx/ext/intersphinx/_load.py b/sphinx/ext/intersphinx/_load.py index 7b8a9ad0aa9..807c1db4d0b 100644 --- a/sphinx/ext/intersphinx/_load.py +++ b/sphinx/ext/intersphinx/_load.py @@ -196,7 +196,7 @@ def fetch_inventory_group( # decide whether the inventory must be read: # always read local files; remote ones only if the cache time is expired if '://' not in inv or uri not in cache or cache[uri][1] < cache_time: - LOGGER.info(__("loading intersphinx inventory '%s' from %s..."), + LOGGER.info(__("loading intersphinx inventory '%s' from %s ..."), name, _get_safe_url(inv)) try: From d77eaade8bb0b3bc3a01c831e48d4c245fd9b893 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?B=C3=A9n=C3=A9dikt=20Tran?= <10796600+picnixz@users.noreply.github.com> Date: Mon, 22 Jul 2024 18:14:14 +0200 Subject: [PATCH 50/52] fix CI/CD --- sphinx/ext/intersphinx/_load.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/sphinx/ext/intersphinx/_load.py b/sphinx/ext/intersphinx/_load.py index 807c1db4d0b..26816a08f13 100644 --- a/sphinx/ext/intersphinx/_load.py +++ b/sphinx/ext/intersphinx/_load.py @@ -145,7 +145,7 @@ def load_mappings(app: Sphinx) -> None: if intersphinx_cache[uri][0] not in intersphinx_mapping: # Remove all cached entries that are no longer in `intersphinx_mapping`. del intersphinx_cache[uri] - if uri not in expected_uris: + elif uri not in expected_uris: # Remove cached entries with a different target URI # than the one in `intersphinx_mapping`. # This happens when the URI in `intersphinx_mapping` is changed. From 2ed2497381e67abb844dd8a3808a1c9e60a43d51 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?B=C3=A9n=C3=A9dikt=20Tran?= <10796600+picnixz@users.noreply.github.com> Date: Mon, 22 Jul 2024 18:59:58 +0200 Subject: [PATCH 51/52] split tests --- tests/test_extensions/test_ext_intersphinx.py | 210 +------------ .../test_ext_intersphinx_cache.py | 275 ++++++++++++++++++ 2 files changed, 276 insertions(+), 209 deletions(-) create mode 100644 tests/test_extensions/test_ext_intersphinx_cache.py diff --git a/tests/test_extensions/test_ext_intersphinx.py b/tests/test_extensions/test_ext_intersphinx.py index 895496cea1b..f1c84035d14 100644 --- a/tests/test_extensions/test_ext_intersphinx.py +++ b/tests/test_extensions/test_ext_intersphinx.py @@ -3,10 +3,6 @@ from __future__ import annotations import http.server -import posixpath -import re -import zlib -from io import BytesIO from typing import TYPE_CHECKING from unittest import mock @@ -35,125 +31,7 @@ from tests.utils import http_server if TYPE_CHECKING: - from collections.abc import Iterable - from typing import BinaryIO, NoReturn - - from sphinx.util.typing import InventoryItem - - -class InventoryEntry: - __slots__ = ( - 'name', 'display_name', 'domain_name', 'object_type', 'uri', 'anchor', 'priority', - ) - - def __init__( - self, - name: str = 'this', - *, - display_name: str | None = None, - domain_name: str = 'py', - object_type: str = 'obj', - uri: str = 'index.html', - anchor: str = '', - priority: int = 0, - ): - if anchor.endswith(name): - anchor = anchor[:-len(name)] + '$' - - if anchor: - uri += '#' + anchor - - if display_name is None or display_name == name: - display_name = '-' - - self.name = name - self.display_name = display_name - self.domain_name = domain_name - self.object_type = object_type - self.uri = uri - self.anchor = anchor - self.priority = priority - - def format(self) -> str: - return (f'{self.name} {self.domain_name}:{self.object_type} ' - f'{self.priority} {self.uri} {self.display_name}\n') - - def __repr__(self) -> str: - fields = (f'{attr}={getattr(self, attr)!r}' for attr in self.__slots__) - return f"{self.__class__.__name__}({', '.join(fields)})" - - -class IntersphinxProject: - def __init__( - self, - name: str = 'foo', - version: str | int = 1, - baseurl: str = '', - baseuri: str = '', - file: str | None = None, - ) -> None: - #: The project name. - self.name = name - #: The escaped project name. - self.safe_name = re.sub(r'\\s+', ' ', name) - - #: The project version as a string. - self.version = version = str(version) - #: The escaped project version. - self.safe_version = re.sub(r'\\s+', ' ', version) - - #: The project base URL (e.g., http://localhost:7777). - self.baseurl = baseurl - #: The project base URI, relative to *baseurl* (e.g., 'foo'). - self.uri = baseuri - #: The project URL, as specified in :confval:`intersphinx_mapping`. - self.url = posixpath.join(baseurl, baseuri) - #: The project local file, if any. - self.file = file - - @property - def record(self) -> dict[str, tuple[str | None, str | None]]: - """The :confval:`intersphinx_mapping` record for this project.""" - return {self.name: (self.url, self.file)} - - def normalize(self, entry: InventoryEntry) -> tuple[str, InventoryItem]: - url = posixpath.join(self.url, entry.uri) - return entry.name, (self.safe_name, self.safe_version, url, entry.display_name) - - -class FakeInventory: - protocol_version: int - - def __init__(self, project: IntersphinxProject | None = None) -> None: - self.project = project or IntersphinxProject() - - def serialize(self, entries: Iterable[InventoryEntry] | None = None) -> bytes: - buffer = BytesIO() - self._write_headers(buffer) - entries = entries or [InventoryEntry()] - self._write_body(buffer, (item.format().encode() for item in entries)) - return buffer.getvalue() - - def _write_headers(self, buffer: BinaryIO) -> None: - buffer.write((f'# Sphinx inventory version {self.protocol_version}\n' - f'# Project: {self.project.safe_name}\n' - f'# Version: {self.project.safe_version}\n').encode()) - - def _write_body(self, buffer: BinaryIO, lines: Iterable[bytes]) -> None: - raise NotImplementedError - - -class FakeInventoryV2(FakeInventory): - protocol_version = 2 - - def _write_headers(self, buffer: BinaryIO) -> None: - super()._write_headers(buffer) - buffer.write(b'# The remainder of this file is compressed using zlib.\n') - - def _write_body(self, buffer: BinaryIO, lines: Iterable[bytes]) -> None: - compressor = zlib.compressobj(9) - buffer.writelines(map(compressor.compress, lines)) - buffer.write(compressor.flush()) + from typing import NoReturn class FakeList(list): @@ -624,92 +502,6 @@ def test_load_mappings_fallback(tmp_path, app, status, warning): assert isinstance(rn, nodes.reference) -@pytest.mark.sphinx('dummy', testroot='basic') -def test_load_mappings_cache_update(make_app, app_params): - ITEM_NAME = 'bar' - DOMAIN_NAME = 'py' - OBJECT_TYPE = 'module' - REFTYPE = f'{DOMAIN_NAME}:{OBJECT_TYPE}' - PORT = 7777 # needd since otherwise it's an automatic port - - PROJECT_NAME, PROJECT_BASEURL = 'foo', f'http://localhost:{PORT}' - old_project = IntersphinxProject(PROJECT_NAME, 1337, PROJECT_BASEURL, 'old') - assert old_project.name == PROJECT_NAME - assert old_project.url == f'http://localhost:{PORT}/old' - - new_project = IntersphinxProject(PROJECT_NAME, 1701, PROJECT_BASEURL, 'new') - assert new_project.name == PROJECT_NAME - assert new_project.url == f'http://localhost:{PORT}/new' - - def make_entry(project: IntersphinxProject) -> InventoryEntry: - name = f'{ITEM_NAME}_{project.version}' - return InventoryEntry(name, domain_name=DOMAIN_NAME, object_type=OBJECT_TYPE) - - def make_invdata(project: IntersphinxProject) -> bytes: - return FakeInventoryV2(project).serialize([make_entry(project)]) - - class InventoryHandler(http.server.BaseHTTPRequestHandler): - def do_GET(self): - self.send_response(200, 'OK') - - data = b'' - for project in (old_project, new_project): - if self.path.startswith(f'/{project.uri}/'): - data = make_invdata(project) - break - - self.send_header('Content-Length', str(len(data))) - self.end_headers() - self.wfile.write(data) - - def log_message(*args, **kwargs): - pass - - # clean build - args, kwargs = app_params - _ = make_app(*args, freshenv=True, **kwargs) - _.build() - - baseconfig = {'extensions': ['sphinx.ext.intersphinx']} - - with http_server(InventoryHandler, port=PORT): - confoverrides1 = baseconfig | {'intersphinx_mapping': old_project.record} - app1 = make_app(*args, confoverrides=confoverrides1, **kwargs) - app1.build() - - # the inventory when querying the 'old' URL - old_entry = make_entry(old_project) - old_item = dict([old_project.normalize(old_entry)]) - - assert list(app1.env.intersphinx_cache) == [old_project.url] - assert app1.env.intersphinx_cache[old_project.url][0] == old_project.name - assert app1.env.intersphinx_cache[old_project.url][2] == {REFTYPE: old_item} - assert app1.env.intersphinx_named_inventory == {old_project.name: {REFTYPE: old_item}} - - # switch to new url and assert that the old URL is no more stored - confoverrides2 = baseconfig | {'intersphinx_mapping': new_project.record} - app2 = make_app(*args, confoverrides=confoverrides2, **kwargs) - app2.build() - - new_entry = make_entry(new_project) - new_item = dict([new_project.normalize(new_entry)]) - - assert list(app2.env.intersphinx_cache) == [new_project.url] - assert app2.env.intersphinx_cache[new_project.url][0] == PROJECT_NAME - assert app2.env.intersphinx_cache[new_project.url][2] == {REFTYPE: new_item} - assert app2.env.intersphinx_named_inventory == {PROJECT_NAME: {REFTYPE: new_item}} - - # switch back to old url (re-use 'old_item') - confoverrides3 = baseconfig | {'intersphinx_mapping': old_project.record} - app3 = make_app(*args, confoverrides=confoverrides3, **kwargs) - app3.build() - - assert list(app3.env.intersphinx_cache) == [old_project.url] - assert app3.env.intersphinx_cache[old_project.url][0] == PROJECT_NAME - assert app3.env.intersphinx_cache[old_project.url][2] == {REFTYPE: old_item} - assert app3.env.intersphinx_named_inventory == {PROJECT_NAME: {REFTYPE: old_item}} - - class TestStripBasicAuth: """Tests for sphinx.ext.intersphinx._strip_basic_auth()""" diff --git a/tests/test_extensions/test_ext_intersphinx_cache.py b/tests/test_extensions/test_ext_intersphinx_cache.py new file mode 100644 index 00000000000..f95349bd160 --- /dev/null +++ b/tests/test_extensions/test_ext_intersphinx_cache.py @@ -0,0 +1,275 @@ +"""Test the intersphinx extension.""" + +from __future__ import annotations + +import posixpath +import re +import zlib +from http.server import BaseHTTPRequestHandler +from io import BytesIO +from typing import TYPE_CHECKING + +import pytest + +from tests.utils import http_server + +if TYPE_CHECKING: + from collections.abc import Iterable + from typing import BinaryIO + + from sphinx.ext.intersphinx._shared import InventoryCacheEntry + from sphinx.util.typing import InventoryItem + + +class InventoryEntry: + """Entry in the Intersphinx inventory.""" + + __slots__ = ( + 'name', 'display_name', 'domain_name', + 'object_type', 'uri', 'anchor', 'priority', + ) + + def __init__( + self, + name: str = 'this', + *, + display_name: str | None = None, + domain_name: str = 'py', + object_type: str = 'obj', + uri: str = 'index.html', + anchor: str = '', + priority: int = 0, + ): + if anchor.endswith(name): + anchor = anchor[:-len(name)] + '$' + + if anchor: + uri += '#' + anchor + + if display_name is None or display_name == name: + display_name = '-' + + self.name = name + self.display_name = display_name + self.domain_name = domain_name + self.object_type = object_type + self.uri = uri + self.anchor = anchor + self.priority = priority + + def format(self) -> str: + """Format the entry as it appears in the inventory file.""" + return (f'{self.name} {self.domain_name}:{self.object_type} ' + f'{self.priority} {self.uri} {self.display_name}\n') + + +class IntersphinxProject: + def __init__( + self, + name: str = 'foo', + version: str | int = 1, + baseurl: str = '', + baseuri: str = '', + file: str | None = None, + ) -> None: + #: The project name. + self.name = name + #: The escaped project name. + self.safe_name = re.sub(r'\\s+', ' ', name) + + #: The project version as a string. + self.version = version = str(version) + #: The escaped project version. + self.safe_version = re.sub(r'\\s+', ' ', version) + + #: The project base URL (e.g., http://localhost:7777). + self.baseurl = baseurl + #: The project base URI, relative to *baseurl* (e.g., 'foo'). + self.uri = baseuri + #: The project URL, as specified in :confval:`intersphinx_mapping`. + self.url = posixpath.join(baseurl, baseuri) + #: The project local file, if any. + self.file = file + + @property + def record(self) -> dict[str, tuple[str | None, str | None]]: + """The :confval:`intersphinx_mapping` record for this project.""" + return {self.name: (self.url, self.file)} + + def normalize(self, entry: InventoryEntry) -> tuple[str, InventoryItem]: + """Format an inventory entry as if it were part of this project.""" + url = posixpath.join(self.url, entry.uri) + return entry.name, (self.safe_name, self.safe_version, url, entry.display_name) + + +class FakeInventory: + protocol_version: int + + def __init__(self, project: IntersphinxProject | None = None) -> None: + self.project = project or IntersphinxProject() + + def serialize(self, entries: Iterable[InventoryEntry] | None = None) -> bytes: + buffer = BytesIO() + self._write_headers(buffer) + entries = entries or [InventoryEntry()] + self._write_body(buffer, (item.format().encode() for item in entries)) + return buffer.getvalue() + + def _write_headers(self, buffer: BinaryIO) -> None: + buffer.write((f'# Sphinx inventory version {self.protocol_version}\n' + f'# Project: {self.project.safe_name}\n' + f'# Version: {self.project.safe_version}\n').encode()) + + def _write_body(self, buffer: BinaryIO, lines: Iterable[bytes]) -> None: + raise NotImplementedError + + +class FakeInventoryV2(FakeInventory): + protocol_version = 2 + + def _write_headers(self, buffer: BinaryIO) -> None: + super()._write_headers(buffer) + buffer.write(b'# The remainder of this file is compressed using zlib.\n') + + def _write_body(self, buffer: BinaryIO, lines: Iterable[bytes]) -> None: + compressor = zlib.compressobj(9) + buffer.writelines(map(compressor.compress, lines)) + buffer.write(compressor.flush()) + + +class SingleEntryProject(IntersphinxProject): + name = 'foo' + port = 7777 # needd since otherwise it's an automatic port + + def __init__( + self, + version: int, + route: str, + *, + item_name: str = 'bar', + domain_name: str = 'py', + object_type: str = 'module' + ) -> None: + self.item_name = item_name + self.domain_name = domain_name + self.object_type = object_type + self.reftype = f'{domain_name}:{object_type}' + super().__init__(self.name, version, f'http://localhost:{self.port}', route) + + def make_entry(self) -> InventoryEntry: + """Get an inventory entry for this project.""" + name = f'{self.item_name}_{self.version}' + return InventoryEntry(name, domain_name=self.domain_name, object_type=self.object_type) + + +def make_inventory_handler(*projects: SingleEntryProject) -> type[BaseHTTPRequestHandler]: + name, port = projects[0].name, projects[0].port + assert all(p.name == name for p in projects) + assert all(p.port == port for p in projects) + + class InventoryHandler(BaseHTTPRequestHandler): + def do_GET(self): + self.send_response(200, 'OK') + + data = b'' + for project in projects: + # create the data to return depending on the endpoint + if self.path.startswith(f'/{project.uri}/'): + entry = project.make_entry() + data = FakeInventoryV2(project).serialize([entry]) + break + + self.send_header('Content-Length', str(len(data))) + self.end_headers() + self.wfile.write(data) + + def log_message(*args, **kwargs): + pass + + return InventoryHandler + + +def test_intersphinx_project_fixture(): + # check that our fixture class is correct + project = SingleEntryProject(1, 'route') + assert project.url == 'http://localhost:7777/route' + + +@pytest.mark.sphinx('dummy', testroot='basic', freshenv=True) +def test_load_mappings_cache(make_app, app_params): + project = SingleEntryProject(1, 'a') + # clean build + args, kwargs = app_params + confoverrides = {'extensions': ['sphinx.ext.intersphinx'], + 'intersphinx_mapping': project.record} + + InventoryHandler = make_inventory_handler(project) + with http_server(InventoryHandler, port=project.port): + app = make_app(*args, confoverrides=confoverrides, **kwargs) + app.build() + + # the inventory when querying the 'old' URL + entry = project.make_entry() + item = dict([project.normalize(entry)]) + assert list(app.env.intersphinx_cache) == ['http://localhost:7777/a'] + e: InventoryCacheEntry = app.env.intersphinx_cache['http://localhost:7777/a'] + assert (e[0], e[2]) == ('foo', {'py:module': item}) + assert app.env.intersphinx_named_inventory == {'foo': {'py:module': item}} + + +@pytest.mark.sphinx('dummy', testroot='basic', freshenv=True) +def test_load_mappings_cache_update(make_app, app_params): + old_project = SingleEntryProject(1337, 'old') + new_project = SingleEntryProject(1701, 'new') + + args, kwargs = app_params + baseconfig = {'extensions': ['sphinx.ext.intersphinx']} + InventoryHandler = make_inventory_handler(old_project, new_project) + with http_server(InventoryHandler, port=SingleEntryProject.port): + # build normally to create an initial cache + confoverrides1 = baseconfig | {'intersphinx_mapping': old_project.record} + _ = make_app(*args, confoverrides=confoverrides1, **kwargs) + _.build() + # switch to new url and assert that the old URL is no more stored + confoverrides2 = baseconfig | {'intersphinx_mapping': new_project.record} + app = make_app(*args, confoverrides=confoverrides2, **kwargs) + app.build() + + entry = new_project.make_entry() + item = dict([new_project.normalize(entry)]) + # check that the URLs were changed accordingly + assert list(app.env.intersphinx_cache) == ['http://localhost:7777/new'] + e: InventoryCacheEntry = app.env.intersphinx_cache['http://localhost:7777/new'] + assert (e[0], e[2]) == ('foo', {'py:module': item}) + assert app.env.intersphinx_named_inventory == {'foo': {'py:module': item}} + + +@pytest.mark.sphinx('dummy', testroot='basic', freshenv=True) +def test_load_mappings_cache_revert_update(make_app, app_params): + old_project = SingleEntryProject(1337, 'old') + new_project = SingleEntryProject(1701, 'new') + + args, kwargs = app_params + baseconfig = {'extensions': ['sphinx.ext.intersphinx']} + InventoryHandler = make_inventory_handler(old_project, new_project) + with http_server(InventoryHandler, port=SingleEntryProject.port): + # build normally to create an initial cache + confoverrides1 = baseconfig | {'intersphinx_mapping': old_project.record} + _ = make_app(*args, confoverrides=confoverrides1, **kwargs) + _.build() + # switch to new url and build + confoverrides2 = baseconfig | {'intersphinx_mapping': new_project.record} + _ = make_app(*args, confoverrides=confoverrides2, **kwargs) + _.build() + # switch back to old url (re-use 'old_item') + confoverrides3 = baseconfig | {'intersphinx_mapping': old_project.record} + app = make_app(*args, confoverrides=confoverrides3, **kwargs) + app.build() + + entry = old_project.make_entry() + item = dict([old_project.normalize(entry)]) + # check that the URLs were changed accordingly + assert list(app.env.intersphinx_cache) == ['http://localhost:7777/old'] + e: InventoryCacheEntry = app.env.intersphinx_cache['http://localhost:7777/old'] + assert (e[0], e[2]) == ('foo', {'py:module': item}) + assert app.env.intersphinx_named_inventory == {'foo': {'py:module': item}} From fab0e896c67c870b470f59ae3853a23fd3f81ae8 Mon Sep 17 00:00:00 2001 From: Adam Turner <9087854+aa-turner@users.noreply.github.com> Date: Mon, 22 Jul 2024 22:50:21 +0100 Subject: [PATCH 52/52] Update tests --- sphinx/ext/intersphinx/_load.py | 4 +- .../test_ext_intersphinx_cache.py | 143 ++++++++++-------- 2 files changed, 85 insertions(+), 62 deletions(-) diff --git a/sphinx/ext/intersphinx/_load.py b/sphinx/ext/intersphinx/_load.py index 26816a08f13..4b53cca155a 100644 --- a/sphinx/ext/intersphinx/_load.py +++ b/sphinx/ext/intersphinx/_load.py @@ -193,8 +193,8 @@ def fetch_inventory_group( # location is either None or a non-empty string inv = f'{uri}/{INVENTORY_FILENAME}' if location is None else location - # decide whether the inventory must be read: - # always read local files; remote ones only if the cache time is expired + # decide whether the inventory must be read: always read local + # files; remote ones only if the cache time is expired if '://' not in inv or uri not in cache or cache[uri][1] < cache_time: LOGGER.info(__("loading intersphinx inventory '%s' from %s ..."), name, _get_safe_url(inv)) diff --git a/tests/test_extensions/test_ext_intersphinx_cache.py b/tests/test_extensions/test_ext_intersphinx_cache.py index f95349bd160..e34fab8b2d6 100644 --- a/tests/test_extensions/test_ext_intersphinx_cache.py +++ b/tests/test_extensions/test_ext_intersphinx_cache.py @@ -9,7 +9,8 @@ from io import BytesIO from typing import TYPE_CHECKING -import pytest +from sphinx.ext.intersphinx import InventoryAdapter +from sphinx.testing.util import SphinxTestApp from tests.utils import http_server @@ -17,9 +18,13 @@ from collections.abc import Iterable from typing import BinaryIO - from sphinx.ext.intersphinx._shared import InventoryCacheEntry from sphinx.util.typing import InventoryItem +BASE_CONFIG = { + 'extensions': ['sphinx.ext.intersphinx'], + 'intersphinx_timeout': 0.1, +} + class InventoryEntry: """Entry in the Intersphinx inventory.""" @@ -66,7 +71,8 @@ def format(self) -> str: class IntersphinxProject: def __init__( self, - name: str = 'foo', + *, + name: str = 'spam', version: str | int = 1, baseurl: str = '', baseuri: str = '', @@ -82,9 +88,9 @@ def __init__( #: The escaped project version. self.safe_version = re.sub(r'\\s+', ' ', version) - #: The project base URL (e.g., http://localhost:7777). + #: The project base URL (e.g., http://localhost:9341). self.baseurl = baseurl - #: The project base URI, relative to *baseurl* (e.g., 'foo'). + #: The project base URI, relative to *baseurl* (e.g., 'spam'). self.uri = baseuri #: The project URL, as specified in :confval:`intersphinx_mapping`. self.url = posixpath.join(baseurl, baseuri) @@ -96,7 +102,7 @@ def record(self) -> dict[str, tuple[str | None, str | None]]: """The :confval:`intersphinx_mapping` record for this project.""" return {self.name: (self.url, self.file)} - def normalize(self, entry: InventoryEntry) -> tuple[str, InventoryItem]: + def normalise(self, entry: InventoryEntry) -> tuple[str, InventoryItem]: """Format an inventory entry as if it were part of this project.""" url = posixpath.join(self.url, entry.uri) return entry.name, (self.safe_name, self.safe_version, url, entry.display_name) @@ -108,7 +114,7 @@ class FakeInventory: def __init__(self, project: IntersphinxProject | None = None) -> None: self.project = project or IntersphinxProject() - def serialize(self, entries: Iterable[InventoryEntry] | None = None) -> bytes: + def serialise(self, entries: Iterable[InventoryEntry] | None = None) -> bytes: buffer = BytesIO() self._write_headers(buffer) entries = entries or [InventoryEntry()] @@ -138,23 +144,28 @@ def _write_body(self, buffer: BinaryIO, lines: Iterable[bytes]) -> None: class SingleEntryProject(IntersphinxProject): - name = 'foo' - port = 7777 # needd since otherwise it's an automatic port + name = 'spam' + port = 9341 # needed since otherwise it's an automatic port def __init__( self, version: int, route: str, *, - item_name: str = 'bar', + item_name: str = 'ham', domain_name: str = 'py', object_type: str = 'module' ) -> None: + super().__init__( + name=self.name, + version=version, + baseurl=f'http://localhost:{self.port}', + baseuri=route, + ) self.item_name = item_name self.domain_name = domain_name self.object_type = object_type self.reftype = f'{domain_name}:{object_type}' - super().__init__(self.name, version, f'http://localhost:{self.port}', route) def make_entry(self) -> InventoryEntry: """Get an inventory entry for this project.""" @@ -176,7 +187,7 @@ def do_GET(self): # create the data to return depending on the endpoint if self.path.startswith(f'/{project.uri}/'): entry = project.make_entry() - data = FakeInventoryV2(project).serialize([entry]) + data = FakeInventoryV2(project).serialise([entry]) break self.send_header('Content-Length', str(len(data))) @@ -192,84 +203,96 @@ def log_message(*args, **kwargs): def test_intersphinx_project_fixture(): # check that our fixture class is correct project = SingleEntryProject(1, 'route') - assert project.url == 'http://localhost:7777/route' + assert project.url == 'http://localhost:9341/route' -@pytest.mark.sphinx('dummy', testroot='basic', freshenv=True) -def test_load_mappings_cache(make_app, app_params): +def test_load_mappings_cache(tmp_path): + tmp_path.joinpath('conf.py').touch() + tmp_path.joinpath('index.rst').touch() project = SingleEntryProject(1, 'a') - # clean build - args, kwargs = app_params - confoverrides = {'extensions': ['sphinx.ext.intersphinx'], - 'intersphinx_mapping': project.record} InventoryHandler = make_inventory_handler(project) with http_server(InventoryHandler, port=project.port): - app = make_app(*args, confoverrides=confoverrides, **kwargs) + # clean build + confoverrides = BASE_CONFIG | {'intersphinx_mapping': project.record} + app = SphinxTestApp('dummy', srcdir=tmp_path, confoverrides=confoverrides) app.build() + app.cleanup() # the inventory when querying the 'old' URL entry = project.make_entry() - item = dict([project.normalize(entry)]) - assert list(app.env.intersphinx_cache) == ['http://localhost:7777/a'] - e: InventoryCacheEntry = app.env.intersphinx_cache['http://localhost:7777/a'] - assert (e[0], e[2]) == ('foo', {'py:module': item}) - assert app.env.intersphinx_named_inventory == {'foo': {'py:module': item}} - - -@pytest.mark.sphinx('dummy', testroot='basic', freshenv=True) -def test_load_mappings_cache_update(make_app, app_params): + item = dict((project.normalise(entry),)) + inventories = InventoryAdapter(app.env) + assert list(inventories.cache) == ['http://localhost:9341/a'] + e_name, e_time, e_inv = inventories.cache['http://localhost:9341/a'] + assert e_name == 'spam' + assert e_inv == {'py:module': item} + assert inventories.named_inventory == {'spam': {'py:module': item}} + + +def test_load_mappings_cache_update(tmp_path): + tmp_path.joinpath('conf.py').touch() + tmp_path.joinpath('index.rst').touch() old_project = SingleEntryProject(1337, 'old') new_project = SingleEntryProject(1701, 'new') - args, kwargs = app_params - baseconfig = {'extensions': ['sphinx.ext.intersphinx']} InventoryHandler = make_inventory_handler(old_project, new_project) with http_server(InventoryHandler, port=SingleEntryProject.port): # build normally to create an initial cache - confoverrides1 = baseconfig | {'intersphinx_mapping': old_project.record} - _ = make_app(*args, confoverrides=confoverrides1, **kwargs) - _.build() + confoverrides1 = BASE_CONFIG | {'intersphinx_mapping': old_project.record} + app1 = SphinxTestApp('dummy', srcdir=tmp_path, confoverrides=confoverrides1) + app1.build() + app1.cleanup() + # switch to new url and assert that the old URL is no more stored - confoverrides2 = baseconfig | {'intersphinx_mapping': new_project.record} - app = make_app(*args, confoverrides=confoverrides2, **kwargs) - app.build() + confoverrides2 = BASE_CONFIG | {'intersphinx_mapping': new_project.record} + app2 = SphinxTestApp('dummy', srcdir=tmp_path, confoverrides=confoverrides2) + app2.build() + app2.cleanup() entry = new_project.make_entry() - item = dict([new_project.normalize(entry)]) + item = dict((new_project.normalise(entry),)) + inventories = InventoryAdapter(app2.env) # check that the URLs were changed accordingly - assert list(app.env.intersphinx_cache) == ['http://localhost:7777/new'] - e: InventoryCacheEntry = app.env.intersphinx_cache['http://localhost:7777/new'] - assert (e[0], e[2]) == ('foo', {'py:module': item}) - assert app.env.intersphinx_named_inventory == {'foo': {'py:module': item}} + assert list(inventories.cache) == ['http://localhost:9341/new'] + e_name, e_time, e_inv = inventories.cache['http://localhost:9341/new'] + assert e_name == 'spam' + assert e_inv == {'py:module': item} + assert inventories.named_inventory == {'spam': {'py:module': item}} -@pytest.mark.sphinx('dummy', testroot='basic', freshenv=True) -def test_load_mappings_cache_revert_update(make_app, app_params): +def test_load_mappings_cache_revert_update(tmp_path): + tmp_path.joinpath('conf.py').touch() + tmp_path.joinpath('index.rst').touch() old_project = SingleEntryProject(1337, 'old') new_project = SingleEntryProject(1701, 'new') - args, kwargs = app_params - baseconfig = {'extensions': ['sphinx.ext.intersphinx']} InventoryHandler = make_inventory_handler(old_project, new_project) with http_server(InventoryHandler, port=SingleEntryProject.port): # build normally to create an initial cache - confoverrides1 = baseconfig | {'intersphinx_mapping': old_project.record} - _ = make_app(*args, confoverrides=confoverrides1, **kwargs) - _.build() + confoverrides1 = BASE_CONFIG | {'intersphinx_mapping': old_project.record} + app1 = SphinxTestApp('dummy', srcdir=tmp_path, confoverrides=confoverrides1) + app1.build() + app1.cleanup() + # switch to new url and build - confoverrides2 = baseconfig | {'intersphinx_mapping': new_project.record} - _ = make_app(*args, confoverrides=confoverrides2, **kwargs) - _.build() + confoverrides2 = BASE_CONFIG | {'intersphinx_mapping': new_project.record} + app2 = SphinxTestApp('dummy', srcdir=tmp_path, confoverrides=confoverrides2) + app2.build() + app2.cleanup() + # switch back to old url (re-use 'old_item') - confoverrides3 = baseconfig | {'intersphinx_mapping': old_project.record} - app = make_app(*args, confoverrides=confoverrides3, **kwargs) - app.build() + confoverrides3 = BASE_CONFIG | {'intersphinx_mapping': old_project.record} + app3 = SphinxTestApp('dummy', srcdir=tmp_path, confoverrides=confoverrides3) + app3.build() + app3.cleanup() entry = old_project.make_entry() - item = dict([old_project.normalize(entry)]) + item = dict((old_project.normalise(entry),)) + inventories = InventoryAdapter(app3.env) # check that the URLs were changed accordingly - assert list(app.env.intersphinx_cache) == ['http://localhost:7777/old'] - e: InventoryCacheEntry = app.env.intersphinx_cache['http://localhost:7777/old'] - assert (e[0], e[2]) == ('foo', {'py:module': item}) - assert app.env.intersphinx_named_inventory == {'foo': {'py:module': item}} + assert list(inventories.cache) == ['http://localhost:9341/old'] + e_name, e_time, e_inv = inventories.cache['http://localhost:9341/old'] + assert e_name == 'spam' + assert e_inv == {'py:module': item} + assert inventories.named_inventory == {'spam': {'py:module': item}}