Fix intersphinx cache handling

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
@@ -138,36 +139,41 @@ def load_mappings(app: Sphinx) -> None:
     intersphinx_cache: dict[InventoryURI, InventoryCacheEntry] = inventories.cache
     intersphinx_mapping: IntersphinxMapping = app.config.intersphinx_mapping
 
+    expected_uris = {uri for _name, (uri, _invs) in intersphinx_mapping.values()}
+
+    for uri in frozenset(intersphinx_cache):
+        if intersphinx_cache[uri][0] not in intersphinx_mapping:
+            # Remove all cached entries that are no longer in `intersphinx_mapping`.
+            del intersphinx_cache[uri]
+        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.
+            del intersphinx_cache[uri]
+
     with concurrent.futures.ThreadPoolExecutor() as pool:
-        futures = []
-        for name, (uri, invs) in 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; which one will override which can vary between builds.
+        #
+        # 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:
+            inventories.named_inventory[name] = invdata
+            for objtype, objects in invdata.items():
+                inventories.main_inventory.setdefault(objtype, {}).update(objects)
 
 
 def fetch_inventory_group(
@@ -179,39 +185,43 @@ def fetch_inventory_group(
     now: int,
 ) -> bool:
     cache_time = now - app.config.intersphinx_cache_limit * 86400
+
+    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 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 invs:
+        # 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:
+            LOGGER.info(__("loading intersphinx inventory '%s' from %s ..."),
+                        name, _get_safe_url(inv))
+
+            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: InventoryURI, inv: str) -> Inventory: