Skip to content

Commit

Permalink
Log differences in configuration from the pickled environment (#12949)
Browse files Browse the repository at this point in the history
  • Loading branch information
AA-Turner authored Oct 8, 2024
1 parent 1bfa4e6 commit 886bb94
Show file tree
Hide file tree
Showing 6 changed files with 87 additions and 7 deletions.
3 changes: 3 additions & 0 deletions CHANGES.rst
Original file line number Diff line number Diff line change
Expand Up @@ -73,6 +73,9 @@ Features added
for builders to enable use of :mod:`sphinx.ext.linkcode`-generated
references.
Patch by James Knight.
* #12949: Print configuration options that differ from the pickled environment.
This can be helpful in diagnosing the cause of a full rebuild.
Patch by Adam Turner.

Bugs fixed
----------
Expand Down
39 changes: 37 additions & 2 deletions sphinx/environment/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@
from sphinx.transforms import SphinxTransformer
from sphinx.util import logging
from sphinx.util._files import DownloadFiles, FilenameUniqDict
from sphinx.util._serialise import stable_str
from sphinx.util._timestamps import _format_rfc3339_microseconds
from sphinx.util.docutils import LoggingReporter
from sphinx.util.i18n import CatalogRepository, docname_to_domain
Expand Down Expand Up @@ -270,7 +271,7 @@ def setup(self, app: Sphinx) -> None:
# The old config is self.config, restored from the pickled environment.
# The new config is app.config, always recreated from ``conf.py``
self.config_status, self.config_status_extra = self._config_status(
old_config=self.config, new_config=app.config
old_config=self.config, new_config=app.config, verbosity=app.verbosity
)
self.config = app.config

Expand All @@ -279,7 +280,7 @@ def setup(self, app: Sphinx) -> None:

@staticmethod
def _config_status(
*, old_config: Config | None, new_config: Config
*, old_config: Config | None, new_config: Config, verbosity: int
) -> tuple[int, str]:
"""Report the differences between two Config objects.
Expand All @@ -302,6 +303,27 @@ def _config_status(
extension = f'{len(extensions)}'
return CONFIG_EXTENSIONS_CHANGED, f' ({extension!r})'

# Log any changes in configuration keys
if changed_keys := _differing_config_keys(old_config, new_config):
changed_num = len(changed_keys)
if changed_num == 1:
logger.info(
__('The configuration has changed (1 option: %r)'),
next(iter(changed_keys)),
)
elif changed_num <= 5 or verbosity >= 1:
logger.info(
__('The configuration has changed (%d options: %s)'),
changed_num,
', '.join(map(repr, sorted(changed_keys))),
)
else:
logger.info(
__('The configuration has changed (%d options: %s, ...)'),
changed_num,
', '.join(map(repr, sorted(changed_keys)[:5])),
)

# check if a config value was changed that affects how doctrees are read
for item in new_config.filter(frozenset({'env'})):
if old_config[item.name] != item.value:
Expand Down Expand Up @@ -756,6 +778,19 @@ def check_consistency(self) -> None:
self.events.emit('env-check-consistency', self)


def _differing_config_keys(old: Config, new: Config) -> frozenset[str]:
"""Return a set of keys that differ between two config objects."""
old_vals = {c.name: c.value for c in old}
new_vals = {c.name: c.value for c in new}
not_in_both = old_vals.keys() ^ new_vals.keys()
different_values = {
key
for key in old_vals.keys() & new_vals.keys()
if stable_str(old_vals[key]) != stable_str(new_vals[key])
}
return frozenset(not_in_both | different_values)


def _traverse_toctree(
traversed: set[str],
parent: str | None,
Expand Down
2 changes: 2 additions & 0 deletions sphinx/ext/autosummary/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -151,6 +151,8 @@ def autosummary_table_visit_html(self: HTML5Translator, node: autosummary_table)
# -- autodoc integration -------------------------------------------------------

class FakeApplication:
verbosity = 0

def __init__(self) -> None:
self.doctreedir = None
self.events = None
Expand Down
2 changes: 1 addition & 1 deletion sphinx/util/_serialise.py
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ def _stable_str_prep(obj: Any) -> dict[str, Any] | list[Any] | str:
return dict(obj)
if isinstance(obj, list | tuple | set | frozenset):
# Convert to a sorted list
return sorted(map(_stable_str_prep, obj))
return sorted(map(_stable_str_prep, obj), key=str)
if isinstance(obj, type | types.FunctionType):
# The default repr() of functions includes the ID, which is not ideal.
# We use the fully qualified name instead.
Expand Down
1 change: 1 addition & 0 deletions tests/roots/test-basic/conf.py
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
html_theme = 'basic'
latex_documents = [
('index', 'test.tex', 'The basic Sphinx documentation for testing', 'Sphinx', 'report')
]
47 changes: 43 additions & 4 deletions tests/test_environment/test_environment.py
Original file line number Diff line number Diff line change
Expand Up @@ -8,12 +8,15 @@

from sphinx.builders.html import StandaloneHTMLBuilder
from sphinx.builders.latex import LaTeXBuilder
from sphinx.config import Config
from sphinx.environment import (
CONFIG_CHANGED,
CONFIG_EXTENSIONS_CHANGED,
CONFIG_NEW,
CONFIG_OK,
_differing_config_keys,
)
from sphinx.util.console import strip_colors


@pytest.mark.sphinx('dummy', testroot='basic')
Expand All @@ -24,13 +27,17 @@ def test_config_status(make_app, app_params):
app1 = make_app(*args, freshenv=True, **kwargs)
assert app1.env.config_status == CONFIG_NEW
app1.build()
assert '[new config] 1 added' in app1._status.getvalue()
output = strip_colors(app1.status.getvalue())
# assert 'The configuration has changed' not in output
assert '[new config] 1 added' in output

# incremental build (no config changed)
app2 = make_app(*args, **kwargs)
assert app2.env.config_status == CONFIG_OK
app2.build()
assert '0 added, 0 changed, 0 removed' in app2._status.getvalue()
output = strip_colors(app2.status.getvalue())
assert 'The configuration has changed' not in output
assert '0 added, 0 changed, 0 removed' in output

# incremental build (config entry changed)
app3 = make_app(*args, confoverrides={'root_doc': 'indexx'}, **kwargs)
Expand All @@ -40,7 +47,9 @@ def test_config_status(make_app, app_params):
assert app3.env.config_status == CONFIG_CHANGED
app3.build()
shutil.move(fname[:-4] + 'x.rst', fname)
assert "[config changed ('master_doc')] 1 added" in app3._status.getvalue()
output = strip_colors(app3.status.getvalue())
assert 'The configuration has changed' in output
assert "[config changed ('master_doc')] 1 added," in output

# incremental build (extension changed)
app4 = make_app(
Expand All @@ -49,7 +58,9 @@ def test_config_status(make_app, app_params):
assert app4.env.config_status == CONFIG_EXTENSIONS_CHANGED
app4.build()
want_str = "[extensions changed ('sphinx.ext.autodoc')] 1 added"
assert want_str in app4._status.getvalue()
output = strip_colors(app4.status.getvalue())
assert 'The configuration has changed' not in output
assert want_str in output


@pytest.mark.sphinx('dummy', testroot='root')
Expand Down Expand Up @@ -181,3 +192,31 @@ def test_env_relfn2path(app):
app.env.temp_data.clear()
with pytest.raises(KeyError):
app.env.relfn2path('images/logo.jpg')


def test_differing_config_keys():
diff = _differing_config_keys

old = Config({'project': 'old'})
new = Config({'project': 'new'})
assert diff(old, new) == frozenset({'project'})

old = Config({'project': 'project', 'release': 'release'})
new = Config({'project': 'project', 'version': 'version'})
assert diff(old, new) == frozenset({'release', 'version'})

old = Config({'project': 'project', 'release': 'release'})
new = Config({'project': 'project'})
assert diff(old, new) == frozenset({'release'})

old = Config({'project': 'project'})
new = Config({'project': 'project', 'version': 'version'})
assert diff(old, new) == frozenset({'version'})

old = Config({'project': 'project', 'release': 'release', 'version': 'version'})
new = Config({'project': 'project', 'release': 'release', 'version': 'version'})
assert diff(old, new) == frozenset()

old = Config({'project': 'old', 'release': 'release'})
new = Config({'project': 'new', 'version': 'version'})
assert diff(old, new) == frozenset({'project', 'release', 'version'})

0 comments on commit 886bb94

Please sign in to comment.