Extract change detection into _has_doc_changed() function (#13681)

Author: AA-Turner

sphinx/environment/__init__.py

@@ -24,7 +24,7 @@
 from sphinx.transforms import SphinxTransformer
 from sphinx.util import logging
 from sphinx.util._files import DownloadFiles, FilenameUniqDict
-from sphinx.util._pathlib import _StrPath, _StrPathProperty
+from sphinx.util._pathlib import _StrPathProperty
 from sphinx.util._serialise import stable_str
 from sphinx.util._timestamps import _format_rfc3339_microseconds
 from sphinx.util.docutils import LoggingReporter
@@ -33,7 +33,7 @@
 from sphinx.util.osutil import _last_modified_time, _relative_path
 
 if TYPE_CHECKING:
-    from collections.abc import Callable, Iterable, Iterator, Mapping
+    from collections.abc import Callable, Iterable, Iterator, Mapping, Set
     from typing import Any, Final, Literal
 
     from docutils import nodes
@@ -50,6 +50,7 @@
     from sphinx.extension import Extension
     from sphinx.project import Project
     from sphinx.registry import SphinxComponentRegistry
+    from sphinx.util._pathlib import _StrPath
     from sphinx.util.tags import Tags
 
 logger = logging.getLogger(__name__)
@@ -74,7 +75,7 @@
 
 # This is increased every time an environment attribute is added
 # or changed to properly invalidate pickle files.
-ENV_VERSION = 65
+ENV_VERSION = 66
 
 # config status
 CONFIG_UNSET = -1
@@ -519,73 +520,33 @@ def get_outdated_files(
     ) -> tuple[set[str], set[str], set[str]]:
         """Return (added, changed, removed) sets."""
         # clear all files no longer present
-        removed = set(self.all_docs) - self.found_docs
+        removed = self.all_docs.keys() - self.found_docs
 
         added: set[str] = set()
         changed: set[str] = set()
 
         if config_changed:
             # config values affect e.g. substitutions
             added = self.found_docs
-        else:
-            for docname in self.found_docs:
-                if docname not in self.all_docs:
-                    logger.debug('[build target] added %r', docname)
-                    added.add(docname)
-                    continue
-                # if the doctree file is not there, rebuild
-                filename = self.doctreedir / f'{docname}.doctree'
-                if not filename.is_file():
-                    logger.debug('[build target] changed %r', docname)
-                    changed.add(docname)
-                    continue
-                # check the "reread always" list
-                if docname in self.reread_always:
-                    logger.debug('[build target] changed %r', docname)
-                    changed.add(docname)
-                    continue
-                # check the mtime of the document
-                mtime = self.all_docs[docname]
-                newmtime = _last_modified_time(self.doc2path(docname))
-                if newmtime > mtime:
-                    logger.debug(
-                        '[build target] outdated %r: %s -> %s',
-                        docname,
-                        _format_rfc3339_microseconds(mtime),
-                        _format_rfc3339_microseconds(newmtime),
-                    )
-                    changed.add(docname)
-                    continue
-                # finally, check the mtime of dependencies
-                if docname not in self.dependencies:
-                    continue
-                for dep in self.dependencies[docname]:
-                    try:
-                        # this will do the right thing when dep is absolute too
-                        dep_path = self.srcdir / dep
-                        if not dep_path.is_file():
-                            logger.debug(
-                                '[build target] changed %r missing dependency %r',
-                                docname,
-                                dep_path,
-                            )
-                            changed.add(docname)
-                            break
-                        depmtime = _last_modified_time(dep_path)
-                        if depmtime > mtime:
-                            logger.debug(
-                                '[build target] outdated %r from dependency %r: %s -> %s',
-                                docname,
-                                dep_path,
-                                _format_rfc3339_microseconds(mtime),
-                                _format_rfc3339_microseconds(depmtime),
-                            )
-                            changed.add(docname)
-                            break
-                    except OSError:
-                        # give it another chance
-                        changed.add(docname)
-                        break
+            return added, changed, removed
+
+        for docname in self.found_docs:
+            if docname not in self.all_docs:
+                logger.debug('[build target] added %r', docname)
+                added.add(docname)
+                continue
+
+            # if the document has changed, rebuild
+            if _has_doc_changed(
+                docname,
+                filename=self.doc2path(docname),
+                reread_always=self.reread_always,
+                doctreedir=self.doctreedir,
+                all_docs=self.all_docs,
+                dependencies=self.dependencies,
+            ):
+                changed.add(docname)
+                continue
 
         return added, changed, removed
 
@@ -649,7 +610,9 @@ def note_dependency(
         """
         if docname is None:
             docname = self.docname
-        self.dependencies.setdefault(docname, set()).add(_StrPath(filename))
+        # this will do the right thing when *filename* is absolute too
+        filename = self.srcdir / filename
+        self.dependencies.setdefault(docname, set()).add(filename)
 
     def note_included(self, filename: str | os.PathLike[str]) -> None:
         """Add *filename* as a included from other document.
@@ -872,6 +835,71 @@ def _differing_config_keys(old: Config, new: Config) -> frozenset[str]:
     return frozenset(not_in_both | different_values)
 
 
+def _has_doc_changed(
+    docname: str,
+    *,
+    filename: Path,
+    reread_always: Set[str],
+    doctreedir: Path,
+    all_docs: Mapping[str, int],
+    dependencies: Mapping[str, Set[Path]],
+) -> bool:
+    # check the "reread always" list
+    if docname in reread_always:
+        logger.debug('[build target] changed %r: re-read forced', docname)
+        return True
+
+    # if the doctree file is not there, rebuild
+    doctree_path = doctreedir / f'{docname}.doctree'
+    if not doctree_path.is_file():
+        logger.debug('[build target] changed %r: doctree file does not exist', docname)
+        return True
+
+    # check the mtime of the document
+    mtime = all_docs[docname]
+    new_mtime = _last_modified_time(filename)
+    if new_mtime > mtime:
+        logger.debug(
+            '[build target] changed: %r is outdated (%s -> %s)',
+            docname,
+            _format_rfc3339_microseconds(mtime),
+            _format_rfc3339_microseconds(new_mtime),
+        )
+        return True
+
+    # finally, check the mtime of dependencies
+    if docname not in dependencies:
+        return False
+    for dep_path in dependencies[docname]:
+        try:
+            dep_path_is_file = dep_path.is_file()
+        except OSError:
+            return True  # give it another chance
+        if not dep_path_is_file:
+            logger.debug(
+                '[build target] changed: %r is missing dependency %r',
+                docname,
+                dep_path,
+            )
+            return True
+
+        try:
+            dep_mtime = _last_modified_time(dep_path)
+        except OSError:
+            return True  # give it another chance
+        if dep_mtime > mtime:
+            logger.debug(
+                '[build target] changed: %r is outdated due to dependency %r (%s -> %s)',
+                docname,
+                dep_path,
+                _format_rfc3339_microseconds(mtime),
+                _format_rfc3339_microseconds(dep_mtime),
+            )
+            return True
+
+    return False
+
+
 def _traverse_toctree(
     traversed: set[str],
     parent: str | None,

tests/js/fixtures/cpp/searchindex.js

@@ -6,8 +6,6 @@
 
 import pytest
 
-from sphinx.util._pathlib import _StrPath
-
 if TYPE_CHECKING:
     from sphinx.testing.util import SphinxTestApp
 
@@ -16,4 +14,4 @@
 def test_record_dependencies_cleared(app: SphinxTestApp) -> None:
     app.builder.read()
     assert 'index' not in app.env.dependencies
-    assert app.env.dependencies['api'] == {_StrPath('example_module.py')}
+    assert app.env.dependencies['api'] == {app.srcdir / 'example_module.py'}