From d5078a64d7263e00507725645d41b04485d91ad4 Mon Sep 17 00:00:00 2001 From: Takeshi KOMIYA Date: Thu, 11 Aug 2016 11:19:42 +0900 Subject: [PATCH] Fix #2789: `sphinx.ext.intersphinx` generates wrong hyperlinks if the inventory is given --- CHANGES | 1 + sphinx/ext/intersphinx.py | 12 +++++----- tests/test_ext_intersphinx.py | 45 ++++++++++++++++++++++++++++++++++- 3 files changed, 51 insertions(+), 7 deletions(-) diff --git a/CHANGES b/CHANGES index 00729366f..eee1df01a 100644 --- a/CHANGES +++ b/CHANGES @@ -17,6 +17,7 @@ Bugs fixed * #2775: Fix failing linkcheck with servers not supporting identidy encoding * #2833: Fix formatting instance annotations in ext.autodoc. * #1911: ``-D`` option of ``sphinx-build`` does not override the ``extensions`` variable +* #2789: `sphinx.ext.intersphinx` generates wrong hyperlinks if the inventory is given Release 1.4.5 (released Jul 13, 2016) ===================================== diff --git a/sphinx/ext/intersphinx.py b/sphinx/ext/intersphinx.py index c43b8ae6a..16a487ef8 100644 --- a/sphinx/ext/intersphinx.py +++ b/sphinx/ext/intersphinx.py @@ -239,12 +239,12 @@ def fetch_inventory(app, uri, inv): return try: if hasattr(f, 'geturl'): - newuri = f.geturl() - if newuri.endswith("/" + INVENTORY_FILENAME): - newuri = newuri[:-len(INVENTORY_FILENAME) - 1] - if uri != newuri and uri != newuri + "/": - app.info('intersphinx inventory has moved: %s -> %s' % (uri, newuri)) - uri = newuri + newinv = f.geturl() + if inv != newinv: + app.info('intersphinx inventory has moved: %s -> %s' % (inv, newinv)) + + if uri in (inv, path.dirname(inv), path.dirname(inv) + '/'): + uri = path.dirname(newinv) line = f.readline().rstrip().decode('utf-8') try: if line == '# Sphinx inventory version 1': diff --git a/tests/test_ext_intersphinx.py b/tests/test_ext_intersphinx.py index 90987a3ef..a258b306f 100644 --- a/tests/test_ext_intersphinx.py +++ b/tests/test_ext_intersphinx.py @@ -19,7 +19,7 @@ from docutils import nodes from sphinx import addnodes from sphinx.ext.intersphinx import read_inventory_v1, read_inventory_v2, \ load_mappings, missing_reference, _strip_basic_auth, _read_from_url, \ - _get_safe_url + _get_safe_url, fetch_inventory, INVENTORY_FILENAME from util import with_app, with_tempdir, mock @@ -83,6 +83,49 @@ def test_read_inventory_v2(): '/util/glossary.html#term-a-term-including-colon' +@with_app() +@mock.patch('sphinx.ext.intersphinx.read_inventory_v2') +@mock.patch('sphinx.ext.intersphinx._read_from_url') +def test_fetch_inventory_redirection(app, status, warning, _read_from_url, read_inventory_v2): + _read_from_url().readline.return_value = '# Sphinx inventory version 2' + + # same uri and inv, not redirected + _read_from_url().geturl.return_value = 'http://hostname/' + INVENTORY_FILENAME + fetch_inventory(app, 'http://hostname/', 'http://hostname/' + INVENTORY_FILENAME) + assert 'intersphinx inventory has moved' not in status.getvalue() + assert read_inventory_v2.call_args[0][1] == 'http://hostname/' + status.truncate(0) + + # same uri and inv, redirected + status.truncate(0) + _read_from_url().geturl.return_value = 'http://hostname/new/' + INVENTORY_FILENAME + + fetch_inventory(app, 'http://hostname/', 'http://hostname/' + INVENTORY_FILENAME) + assert status.getvalue() == ('intersphinx inventory has moved: ' + 'http://hostname/%s -> http://hostname/new/%s\n' % + (INVENTORY_FILENAME, INVENTORY_FILENAME)) + print read_inventory_v2.call_args + assert read_inventory_v2.call_args[0][1] == 'http://hostname/new' + + # different uri and inv, not redirected + status.truncate(0) + _read_from_url().geturl.return_value = 'http://hostname/new/' + INVENTORY_FILENAME + + fetch_inventory(app, 'http://hostname/', 'http://hostname/new/' + INVENTORY_FILENAME) + assert 'intersphinx inventory has moved' not in status.getvalue() + assert read_inventory_v2.call_args[0][1] == 'http://hostname/' + + # different uri and inv, redirected + status.truncate(0) + _read_from_url().geturl.return_value = 'http://hostname/other/' + INVENTORY_FILENAME + + fetch_inventory(app, 'http://hostname/', 'http://hostname/new/' + INVENTORY_FILENAME) + assert status.getvalue() == ('intersphinx inventory has moved: ' + 'http://hostname/new/%s -> http://hostname/other/%s\n' % + (INVENTORY_FILENAME, INVENTORY_FILENAME)) + assert read_inventory_v2.call_args[0][1] == 'http://hostname/' + + @with_app() @with_tempdir def test_missing_reference(tempdir, app, status, warning):