From e86c5ebec311b92348d7901f82973aa52efb0099 Mon Sep 17 00:00:00 2001 From: James Addison Date: Mon, 15 Jul 2024 14:17:23 +0100 Subject: [PATCH 1/4] intersphinx ambiguities: add illustrative test coverage to handle bugreport/false-positive case #12585 --- tests/test_util/intersphinx_data.py | 2 ++ tests/test_util/test_util_inventory.py | 3 ++- 2 files changed, 4 insertions(+), 1 deletion(-) diff --git a/tests/test_util/intersphinx_data.py b/tests/test_util/intersphinx_data.py index 889645903dd..95cf80a9b39 100644 --- a/tests/test_util/intersphinx_data.py +++ b/tests/test_util/intersphinx_data.py @@ -59,4 +59,6 @@ ''' + zlib.compress(b'''\ a term std:term -1 glossary.html#term-a-term - A term std:term -1 glossary.html#term-a-term - +b term std:term -1 document.html#id5 - +B term std:term -1 document.html#B - ''') diff --git a/tests/test_util/test_util_inventory.py b/tests/test_util/test_util_inventory.py index 0bdef9f67d9..d01785fda24 100644 --- a/tests/test_util/test_util_inventory.py +++ b/tests/test_util/test_util_inventory.py @@ -53,7 +53,8 @@ def test_ambiguous_definition_warning(warning): f = BytesIO(INVENTORY_V2_AMBIGUOUS_TERMS) InventoryFile.load(f, '/util', posixpath.join) - assert 'contains multiple definitions for std:term:a' in warning.getvalue().lower() + assert 'contains multiple definitions for std:term:a' not in warning.getvalue().lower() + assert 'contains multiple definitions for std:term:b' in warning.getvalue().lower() def _write_appconfig(dir, language, prefix=None): From 96c57978fdcd2183abd26a519fbfb8fc6664c278 Mon Sep 17 00:00:00 2001 From: James Addison Date: Mon, 15 Jul 2024 14:25:18 +0100 Subject: [PATCH 2/4] intersphinx ambiguities: don't warn about pure-duplicate ambiguous definitions in `objects.inv` entries. --- sphinx/util/inventory.py | 15 ++++++++++----- 1 file changed, 10 insertions(+), 5 deletions(-) diff --git a/sphinx/util/inventory.py b/sphinx/util/inventory.py index 55d7efd8396..3e34bdda659 100644 --- a/sphinx/util/inventory.py +++ b/sphinx/util/inventory.py @@ -126,7 +126,7 @@ def load_v2( invdata: Inventory = {} projname = stream.readline().rstrip()[11:] version = stream.readline().rstrip()[11:] - potential_ambiguities = set() + potential_ambiguities = {} actual_ambiguities = set() line = stream.readline() if 'zlib' not in line: @@ -154,11 +154,16 @@ def load_v2( # Some types require case insensitive matches: # * 'term': https://github.com/sphinx-doc/sphinx/issues/9291 # * 'label': https://github.com/sphinx-doc/sphinx/issues/12008 - definition = f"{type}:{name}" - if definition.lower() in potential_ambiguities: - actual_ambiguities.add(definition) + definition, content = f"{type}:{name}", {prio, location, dispname} + lowercase_definition = definition.lower() + if lowercase_definition in potential_ambiguities: + if potential_ambiguities[lowercase_definition] != content: + actual_ambiguities.add(definition) + else: + logger.debug(__("inventory <%s> contains duplicate definitions of %s"), + uri, definition, type='intersphinx', subtype='external') else: - potential_ambiguities.add(definition.lower()) + potential_ambiguities[lowercase_definition] = content if location.endswith('$'): location = location[:-1] + name location = join(uri, location) From 82934dcbc5426056fabaddd678906cdf76c77f69 Mon Sep 17 00:00:00 2001 From: James Addison Date: Mon, 15 Jul 2024 15:11:11 +0100 Subject: [PATCH 3/4] Simplify: use direct tuple comparison of entries for duplicate-detection (and add corresponding type-hint). --- sphinx/util/inventory.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/sphinx/util/inventory.py b/sphinx/util/inventory.py index 3e34bdda659..9a4c45870ad 100644 --- a/sphinx/util/inventory.py +++ b/sphinx/util/inventory.py @@ -126,7 +126,7 @@ def load_v2( invdata: Inventory = {} projname = stream.readline().rstrip()[11:] version = stream.readline().rstrip()[11:] - potential_ambiguities = {} + potential_ambiguities: dict[str, tuple[str, str, str]] = {} actual_ambiguities = set() line = stream.readline() if 'zlib' not in line: @@ -154,7 +154,7 @@ def load_v2( # Some types require case insensitive matches: # * 'term': https://github.com/sphinx-doc/sphinx/issues/9291 # * 'label': https://github.com/sphinx-doc/sphinx/issues/12008 - definition, content = f"{type}:{name}", {prio, location, dispname} + definition, content = f"{type}:{name}", (prio, location, dispname) lowercase_definition = definition.lower() if lowercase_definition in potential_ambiguities: if potential_ambiguities[lowercase_definition] != content: From 87d5c7639db829379dcee056a9abb2e9ed44ac09 Mon Sep 17 00:00:00 2001 From: Adam Turner <9087854+AA-Turner@users.noreply.github.com> Date: Mon, 15 Jul 2024 18:32:39 +0100 Subject: [PATCH 4/4] Apply suggestions from code review --- sphinx/util/inventory.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/sphinx/util/inventory.py b/sphinx/util/inventory.py index 9a4c45870ad..9065d31c971 100644 --- a/sphinx/util/inventory.py +++ b/sphinx/util/inventory.py @@ -126,6 +126,7 @@ def load_v2( invdata: Inventory = {} projname = stream.readline().rstrip()[11:] version = stream.readline().rstrip()[11:] + # definition -> priority, location, display name potential_ambiguities: dict[str, tuple[str, str, str]] = {} actual_ambiguities = set() line = stream.readline() @@ -154,7 +155,8 @@ def load_v2( # Some types require case insensitive matches: # * 'term': https://github.com/sphinx-doc/sphinx/issues/9291 # * 'label': https://github.com/sphinx-doc/sphinx/issues/12008 - definition, content = f"{type}:{name}", (prio, location, dispname) + definition = f"{type}:{name}" + content = prio, location, dispname lowercase_definition = definition.lower() if lowercase_definition in potential_ambiguities: if potential_ambiguities[lowercase_definition] != content: