From 430866e029c7fddc0da65613dc32b732424b6fb7 Mon Sep 17 00:00:00 2001
From: Chris Sewell <chrisj_sewell@hotmail.com>
Date: Wed, 27 Mar 2024 12:59:24 +0100
Subject: [PATCH] =?UTF-8?q?=F0=9F=91=8C=20Add=20warning=20log=20for=20conf?=
 =?UTF-8?q?ig=20values=20that=20cannot=20be=20cached=20(#12203)?=
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

As a user and/or extension developer, it can be a source of confusion when,
on performing a `sphinx-build` for a cached project,
an unchanged configuration variable is always marked as changed (triggering a full rebuild).

This is often due to the variable being an unpicklable value,
which is silently omitted from the cache (i.e. the `environment.pickle`).

This commit introduces a specific warning for when a configuration variable is omitted from the cache,
allowing users to understand the root of the problem.
---
 sphinx/config.py                           | 11 +++++++++++
 tests/test_builders/test_build_warnings.py | 16 ++++++++++++++++
 2 files changed, 27 insertions(+)

diff --git a/sphinx/config.py b/sphinx/config.py
index 9e49423d5cc..922fa710fbb 100644
--- a/sphinx/config.py
+++ b/sphinx/config.py
@@ -465,6 +465,17 @@ def __getstate__(self) -> dict:
         for name, opt in self._options.items():
             real_value = getattr(self, name)
             if not is_serializable(real_value):
+                if opt.rebuild:
+                    # if the value is not cached, then any build that utilises this cache
+                    # will always mark the config value as changed,
+                    # and thus always invalidate the cache and perform a rebuild.
+                    logger.warning(
+                        __('cannot cache unpickable configuration value: %r'),
+                        name,
+                        type='config',
+                        subtype='cache',
+                        once=True,
+                    )
                 # omit unserializable value
                 real_value = None
             # valid_types is also omitted
diff --git a/tests/test_builders/test_build_warnings.py b/tests/test_builders/test_build_warnings.py
index 1db4e051f47..8605d5d26ef 100644
--- a/tests/test_builders/test_build_warnings.py
+++ b/tests/test_builders/test_build_warnings.py
@@ -70,3 +70,19 @@ def test_texinfo_warnings(app, warning):
     app.build(force_all=True)
     warnings_exp = TEXINFO_WARNINGS.format(root=re.escape(app.srcdir.as_posix()))
     _check_warnings(warnings_exp, warning.getvalue())
+
+
+def test_uncacheable_config_warning(make_app, tmp_path):
+    """Test than an unpickleable config value raises a warning."""
+    tmp_path.joinpath('conf.py').write_text("""
+my_config = lambda: None
+show_warning_types = True
+def setup(app):
+    app.add_config_value('my_config', None, 'env')
+    """, encoding='utf-8')
+    tmp_path.joinpath('index.rst').write_text('Test\n====\n', encoding='utf-8')
+    app = make_app(srcdir=tmp_path)
+    app.build()
+    assert strip_colors(app.warning.getvalue()).strip() == (
+        "WARNING: cannot cache unpickable configuration value: 'my_config' [config.cache]"
+    )