Skip to content

Commit d8131d8

Browse files
authored
🐛 Don't mutate rebuild='env' src_trace_projects config during builds (#75)
## Summary Incremental Sphinx builds always re-read every document, logging `[config changed ('src_trace_projects')] N added, 0 changed, 0 removed`, even when nothing changed — incremental builds behave like full rebuilds. `src_trace_projects` is registered with `rebuild="env"`, so Sphinx pickles it into `environment.pickle` and invalidates the environment whenever the pickled ("old") value differs from the freshly computed ("new") one. `generate_project_configs()` stores runtime `analyse_config` / `source_discover_config` dataclass objects in that value; on their own they round-trip through pickle as `==`-equal. The bug: the `src-trace` directive mutated the stored `analyse_config` **in place** during the build (`src_dir`, `src_files`, `git_root` set to build-time, resolved values). Those build-time values were persisted into the pickle, while the next build's `config-inited` produced a fresh `analyse_config` (`src_files=[]`, `src_dir=Path(".")`), so the comparison always differed: | Side | `src_dir` | `src_files` | |------|-----------|-------------| | persisted (pickle, "old") | `/abs/.../dcdc` | `['demo_3.cpp', ...]` | | fresh config-inited ("new") | `Path('.')` | `[]` | ## Fix Operate on a per-directive `dataclasses.replace()` copy instead of mutating the `analyse_config` stored in the `rebuild="env"` config value. The stored value then stays equal to what `generate_project_configs()` yields, so incremental builds report `0 added, 0 changed, 0 removed`. ## Verification - New regression test `test_incremental_build_keeps_src_trace_projects_unchanged` builds the fixture twice and asserts the 2nd (incremental) build sees `CONFIG_OK` — fails before the fix (`CONFIG_CHANGED ('src_trace_projects')`), passes after. - Full suite: **207 passed**. - Manual triple build: builds 2 & 3 now report `0 added, 0 changed, 0 removed`. Fixes #74
1 parent c9e78da commit d8131d8

2 files changed

Lines changed: 66 additions & 5 deletions

File tree

src/sphinx_codelinks/sphinx_extension/directives/src_trace.py

Lines changed: 18 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
from collections.abc import Callable
2+
from dataclasses import replace
23
import os
34
from pathlib import Path
45
from typing import Any, ClassVar, cast
@@ -113,16 +114,28 @@ def run(self) -> list[nodes.Node]:
113114
for source_file in source_files:
114115
self.env.note_dependency(str(source_file.resolve()))
115116

116-
analyse_config = src_trace_conf["analyse_config"]
117-
analyse_config.src_dir = src_dir
118-
analyse_config.src_files = source_files
117+
# ``analyse_config`` is stored in the ``src_trace_projects`` config value,
118+
# which is registered with ``rebuild="env"`` and therefore persisted into
119+
# ``environment.pickle``. Mutating it in place would make Sphinx compare the
120+
# build-populated object against the freshly generated (empty) config on the
121+
# next build and report ``[config changed ('src_trace_projects')]`` every
122+
# time, forcing a full re-read. Build a per-directive copy instead so the
123+
# stored config value stays equal to what ``generate_project_configs`` yields.
124+
base_analyse_config = src_trace_conf["analyse_config"]
119125
# git_root shall be relative to the config file's location (if provided)
120-
if analyse_config.git_root:
126+
git_root = base_analyse_config.git_root
127+
if git_root:
121128
conf_dir = Path(self.env.app.confdir)
122129
if src_trace_sphinx_config.config_from_toml:
123130
src_trace_toml_path = Path(src_trace_sphinx_config.config_from_toml)
124131
conf_dir = conf_dir / src_trace_toml_path.parent
125-
analyse_config.git_root = (conf_dir / analyse_config.git_root).resolve()
132+
git_root = (conf_dir / git_root).resolve()
133+
analyse_config = replace(
134+
base_analyse_config,
135+
src_dir=src_dir,
136+
src_files=source_files,
137+
git_root=git_root,
138+
)
126139
src_analyse = SourceAnalyse(analyse_config, name=project)
127140
src_analyse.run()
128141

tests/test_src_trace.py

Lines changed: 48 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@
44
import shutil
55

66
import pytest
7+
from sphinx.environment import CONFIG_OK
78
from sphinx.testing.util import SphinxTestApp
89

910
from sphinx_codelinks.analyse.projects import AnalyseProjects
@@ -228,3 +229,50 @@ def test_build_html(
228229
assert not warnings
229230

230231
assert app.env.get_doctree("index") == snapshot_doctree
232+
233+
234+
def test_incremental_build_keeps_src_trace_projects_unchanged(
235+
tmpdir: Path,
236+
make_app: Callable[..., SphinxTestApp],
237+
) -> None:
238+
"""An incremental rebuild with no source changes must not invalidate the env.
239+
240+
Regression test for the ``src-trace`` directive mutating the ``analyse_config``
241+
object stored inside the ``rebuild="env"`` ``src_trace_projects`` config value.
242+
The mutated object (populated ``src_dir``/``src_files``) was persisted into
243+
``environment.pickle``, so every incremental build compared it against the
244+
freshly generated (empty) config and reported
245+
``[config changed ('src_trace_projects')]``, forcing a full re-read.
246+
"""
247+
this_file_dir = Path(__file__).parent
248+
sphinx_project = Path("data") / "sphinx"
249+
source_code = Path("data") / "dcdc"
250+
251+
sphinx_src_dir = Path(tmpdir) / sphinx_project
252+
shutil.copytree(this_file_dir / sphinx_project, sphinx_src_dir, dirs_exist_ok=True)
253+
shutil.copytree(
254+
this_file_dir / source_code, Path(tmpdir) / source_code, dirs_exist_ok=True
255+
)
256+
257+
# First build populates environment.pickle in the shared build dir.
258+
make_app(srcdir=sphinx_src_dir, freshenv=True).build()
259+
260+
# Second build reuses the same build dir and loads the pickled environment.
261+
app = make_app(srcdir=sphinx_src_dir, freshenv=False)
262+
263+
captured: dict[str, object] = {}
264+
265+
def capture_config_status(_app, env, _added, _changed, _removed): # type: ignore[no-untyped-def]
266+
# ``env-get-outdated`` fires during read() after the config comparison
267+
# but before config_status is reset to CONFIG_OK at the end of read().
268+
captured["status"] = env.config_status
269+
captured["extra"] = env.config_status_extra
270+
return []
271+
272+
app.connect("env-get-outdated", capture_config_status)
273+
app.build()
274+
275+
assert captured["status"] == CONFIG_OK, (
276+
f"incremental build wrongly invalidated the environment: "
277+
f"config changed{captured.get('extra')}"
278+
)

0 commit comments

Comments
 (0)