Remove support for the Sphinx 0.5 intersphinx_mapping format (#12083

)

Co-authored-by: Adam Turner <9087854+aa-turner@users.noreply.github.com>

CHANGES.rst

@@ -60,6 +60,9 @@ Incompatible changes
 * #12597: Change the default of :confval:`show_warning_types`
   from ``False`` to ``True``.
   Patch by Chris Sewell.
+* #12083: Remove support for the old (2008--2010) Sphinx 0.5 and Sphinx 0.6
+  :confval:`intersphinx_mapping` format.
+  Patch by Bénédikt Tran and Adam Turner.
 
 Deprecated
 ----------

doc/usage/extensions/intersphinx.rst

@@ -128,28 +128,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-block:: python
-
-      intersphinx_mapping = {'https://docs.python.org/': None}
-
-
 .. confval:: intersphinx_cache_limit
 
    The maximum number of days to cache remote inventories.  The default is

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

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
@@ -21,55 +22,123 @@
 
     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] = {}
+
+    errors = 0
+    for name, value in config.intersphinx_mapping.copy().items():
+        # 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.'
+            )
+            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)
+            del config.intersphinx_mapping[name]
+            continue
+
+        # 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.'
+            )
+            LOGGER.error(msg % (value, name))
+            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 (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):
+            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 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, seen[uri]))
+            del config.intersphinx_mapping[name]
+            continue
+        seen[uri] = name
+
+        # 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:
-                # 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 = {{'<name>': {(uri, inv)!r}}}`."
-                    'https://www.sphinx-doc.org/en/master/usage/extensions/intersphinx.html#confval-intersphinx_mapping'  # NoQA: E501
+                errors += 1
+                msg = __(
+                    'Invalid inventory location value `%r` in intersphinx_mapping[%r][1]. '
+                    'Inventory locations must be non-empty strings or None.'
                 )
-                LOGGER.warning(msg)
+                LOGGER.error(msg % (target, 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))
-        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)))
+
+    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 % errors)
 
 
 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 +169,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:
@@ -130,7 +199,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,'
@@ -143,7 +212,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

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:
@@ -100,7 +101,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,
@@ -142,20 +143,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,
@@ -170,8 +172,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 ()
@@ -183,12 +184,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.

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: