[8.x] Remove Intersphinx Legacy Format and Fix intersphinx cache loading in incremental builds

CHANGES.rst

@@ -41,6 +41,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
 -------

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
@@ -237,7 +238,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 +274,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 | None, 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,

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]]
 
 

tests/test_ext_intersphinx.py

@@ -1,6 +1,9 @@
 """Test the intersphinx extension."""
 
+from __future__ import annotations
+
 import http.server
+import zlib
 from unittest import mock
 
 import pytest
@@ -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 = '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 = '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):