From ef25f45786d61d81632d49c08f6b0e6ad3e3e9e6 Mon Sep 17 00:00:00 2001 From: Marco Heinemann Date: Mon, 8 Jun 2026 10:45:21 +0200 Subject: [PATCH] =?UTF-8?q?=F0=9F=90=9B=20Don't=20mutate=20rebuild=3D'env'?= =?UTF-8?q?=20src=5Ftrace=5Fprojects=20config=20during=20builds?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The src-trace directive populated src_dir/src_files/git_root directly on the analyse_config object stored inside src_trace_projects. That value is registered with rebuild="env", so the build-mutated object was pickled into environment.pickle; on the next build Sphinx compared it against the freshly generated (empty) config and reported [config changed ('src_trace_projects')], forcing a full re-read of all docs on every incremental build. Work on a per-directive dataclasses.replace() copy so the stored config value stays equal to what generate_project_configs() yields. Add a regression test that builds the fixture twice and asserts the second build sees CONFIG_OK. Refs #74 --- .../sphinx_extension/directives/src_trace.py | 23 +++++++-- tests/test_src_trace.py | 48 +++++++++++++++++++ 2 files changed, 66 insertions(+), 5 deletions(-) diff --git a/src/sphinx_codelinks/sphinx_extension/directives/src_trace.py b/src/sphinx_codelinks/sphinx_extension/directives/src_trace.py index 0b522f7..df73b56 100644 --- a/src/sphinx_codelinks/sphinx_extension/directives/src_trace.py +++ b/src/sphinx_codelinks/sphinx_extension/directives/src_trace.py @@ -1,4 +1,5 @@ from collections.abc import Callable +from dataclasses import replace import os from pathlib import Path from typing import Any, ClassVar, cast @@ -113,16 +114,28 @@ def run(self) -> list[nodes.Node]: for source_file in source_files: self.env.note_dependency(str(source_file.resolve())) - analyse_config = src_trace_conf["analyse_config"] - analyse_config.src_dir = src_dir - analyse_config.src_files = source_files + # ``analyse_config`` is stored in the ``src_trace_projects`` config value, + # which is registered with ``rebuild="env"`` and therefore persisted into + # ``environment.pickle``. Mutating it in place would make Sphinx compare the + # build-populated object against the freshly generated (empty) config on the + # next build and report ``[config changed ('src_trace_projects')]`` every + # time, forcing a full re-read. Build a per-directive copy instead so the + # stored config value stays equal to what ``generate_project_configs`` yields. + base_analyse_config = src_trace_conf["analyse_config"] # git_root shall be relative to the config file's location (if provided) - if analyse_config.git_root: + git_root = base_analyse_config.git_root + if git_root: conf_dir = Path(self.env.app.confdir) if src_trace_sphinx_config.config_from_toml: src_trace_toml_path = Path(src_trace_sphinx_config.config_from_toml) conf_dir = conf_dir / src_trace_toml_path.parent - analyse_config.git_root = (conf_dir / analyse_config.git_root).resolve() + git_root = (conf_dir / git_root).resolve() + analyse_config = replace( + base_analyse_config, + src_dir=src_dir, + src_files=source_files, + git_root=git_root, + ) src_analyse = SourceAnalyse(analyse_config, name=project) src_analyse.run() diff --git a/tests/test_src_trace.py b/tests/test_src_trace.py index 8e87a71..2273c89 100644 --- a/tests/test_src_trace.py +++ b/tests/test_src_trace.py @@ -4,6 +4,7 @@ import shutil import pytest +from sphinx.environment import CONFIG_OK from sphinx.testing.util import SphinxTestApp from sphinx_codelinks.analyse.projects import AnalyseProjects @@ -228,3 +229,50 @@ def test_build_html( assert not warnings assert app.env.get_doctree("index") == snapshot_doctree + + +def test_incremental_build_keeps_src_trace_projects_unchanged( + tmpdir: Path, + make_app: Callable[..., SphinxTestApp], +) -> None: + """An incremental rebuild with no source changes must not invalidate the env. + + Regression test for the ``src-trace`` directive mutating the ``analyse_config`` + object stored inside the ``rebuild="env"`` ``src_trace_projects`` config value. + The mutated object (populated ``src_dir``/``src_files``) was persisted into + ``environment.pickle``, so every incremental build compared it against the + freshly generated (empty) config and reported + ``[config changed ('src_trace_projects')]``, forcing a full re-read. + """ + this_file_dir = Path(__file__).parent + sphinx_project = Path("data") / "sphinx" + source_code = Path("data") / "dcdc" + + sphinx_src_dir = Path(tmpdir) / sphinx_project + shutil.copytree(this_file_dir / sphinx_project, sphinx_src_dir, dirs_exist_ok=True) + shutil.copytree( + this_file_dir / source_code, Path(tmpdir) / source_code, dirs_exist_ok=True + ) + + # First build populates environment.pickle in the shared build dir. + make_app(srcdir=sphinx_src_dir, freshenv=True).build() + + # Second build reuses the same build dir and loads the pickled environment. + app = make_app(srcdir=sphinx_src_dir, freshenv=False) + + captured: dict[str, object] = {} + + def capture_config_status(_app, env, _added, _changed, _removed): # type: ignore[no-untyped-def] + # ``env-get-outdated`` fires during read() after the config comparison + # but before config_status is reset to CONFIG_OK at the end of read(). + captured["status"] = env.config_status + captured["extra"] = env.config_status_extra + return [] + + app.connect("env-get-outdated", capture_config_status) + app.build() + + assert captured["status"] == CONFIG_OK, ( + f"incremental build wrongly invalidated the environment: " + f"config changed{captured.get('extra')}" + )