Fix detection of files that were modified since last indexing

Xapian's `sortable_unserialise' returned a float, which caused
rounding errors that could force some files to be always treated as
modified.

* bdx/binary.py (BinaryDirectory): Change `last_mtime' datetime
attribute to integer `last_mtime_ns'.
(BinaryDirectory.changed_files): Compare mtimes as integers.
* bdx/index.py (SymbolIndex.mtime): Return nanosecond mtime, exactly
as stored in the database.
(index_binary_directory): Pass nanosecond mtime to `BinaryDirectory'.
* tests/test_binary.py (setup_tmp_dir): Return nanosecond mtime.
(test_find_changed_files):
(test_find_deleted_files): Pass nanosecond mtime.
* tests/test_index.py (_compile_file): New helper.
(test_indexing_triggered_after_file_was_updated):
(test_indexing_not_triggered_if_mtime_not_changed): New tests.

Author: mkcms

bdx/binary.py

@@ -521,7 +521,7 @@ class BinaryDirectory:
 
     path: Path
     exclusions: Collection[Exclusion] = field(default_factory=list)
-    last_mtime: datetime = datetime.fromtimestamp(0)
+    last_mtime_ns: int = 0
     previous_file_list: list[Path] = field(repr=False, default_factory=list)
     use_compilation_database: bool = False
 
@@ -551,16 +551,17 @@ def changed_files(self) -> Iterator[Path]:
             desc="Finding changed files",
             leave=False,
         ):
-            mtime = datetime.fromtimestamp(path.stat().st_mtime)
+            st = path.stat()
             is_new = path not in previous_state
-            is_changed = not is_new and self.last_mtime < mtime
+            is_changed = not is_new and self.last_mtime_ns < st.st_mtime_ns
 
             trace(
-                "{}: is_new={} is_changed={} mtime={}",
+                "{}: is_new={} is_changed={} mtime={} ({})",
                 path,
                 is_new,
                 is_changed,
-                mtime,
+                datetime.fromtimestamp(st.st_mtime),
+                st.st_mtime_ns,
             )
 
             if is_new or is_changed:

bdx/index.py

@@ -11,7 +11,6 @@
 from collections.abc import Mapping
 from contextlib import contextmanager
 from dataclasses import asdict, dataclass, field
-from datetime import datetime
 from enum import Enum
 from functools import cache
 from pathlib import Path
@@ -688,16 +687,26 @@ def set_metadata(self, key: str, metadata: bytes):
             raise ValueError(msg)
         self._live_writable_db().set_metadata(key, metadata)
 
-    def mtime(self) -> datetime:
-        """Return the max modification time of this index."""
+    def mtime(self) -> int:
+        """Return the max modification time of this index in nanoseconds."""
         db = self._live_db()
         field_data = self.schema["mtime"]
-        val = db.get_value_upper_bound(field_data.slot)  # pyright: ignore
+        slot = field_data.slot  # pyright: ignore
+        val = db.get_value_upper_bound(slot)
         if not val:
-            return datetime.fromtimestamp(0)
+            return 0
+
+        # Search for symbol which has the known highest mtime.  We
+        # can't just use Xapian.sortable_unserialise() on ``val``, as
+        # that converts the value to a float, and we lose precision,
+        # causing errors and unexpected results, e.g. files won't be
+        # treated as modified even though they were.
 
-        val_int = xapian.sortable_unserialise(val)
-        return datetime.fromtimestamp(val_int / 1e9)
+        query = xapian.Query(xapian.Query.OP_VALUE_RANGE, slot, val, val)
+        results = self.search(query, limit=1)
+        if results.count == 0:
+            return 0
+        return list(results)[0].mtime
 
     def binary_dir(self) -> Optional[Path]:
         """Get binary directory of this index, set by ``set_binary_dir``."""
@@ -1083,14 +1092,14 @@ def index_binary_directory(
                 exclusions.append(excl)
 
         if reindex:
-            mtime = datetime.fromtimestamp(0)
+            mtime_ns = 0
         else:
-            mtime = index.mtime()
+            mtime_ns = index.mtime()
         existing_files = set(index.all_files())
         bdir = BinaryDirectory(
             path=bindir_path,
             exclusions=exclusions,
-            last_mtime=mtime,
+            last_mtime_ns=mtime_ns,
             previous_file_list=list(existing_files),
             use_compilation_database=use_compilation_database,
         )

tests/test_binary.py

@@ -23,20 +23,22 @@ def setup_tmp_dir(
     file_list: list[Path],
     files_to_delete: Optional[list[Path]] = None,
     files_to_modify: Optional[list[Path]] = None,
-) -> tuple[list[Path], list[Path], list[Path], datetime]:
+) -> tuple[list[Path], list[Path], list[Path], int]:
     if files_to_delete is None:
         files_to_delete = []
     if files_to_modify is None:
         files_to_modify = []
 
-    max_mtime = datetime.now() - timedelta(seconds=1)
+    mtime = datetime.now() - timedelta(seconds=1)
+    mtime_ns = 0
     for f in file_list:
-        create_fake_elf_file(f, max_mtime)
+        create_fake_elf_file(f, mtime)
+        mtime_ns = f.stat().st_mtime_ns
     for f in files_to_delete:
         f.unlink()
     for f in files_to_modify:
         f.touch()
-    return file_list, files_to_delete, files_to_modify, max_mtime
+    return file_list, files_to_delete, files_to_modify, mtime_ns
 
 
 def shuffled_int_range(stop):
@@ -101,7 +103,7 @@ def test_find_changed_files(tmp_path):
 
     bdir = BinaryDirectory(
         tmp_path,
-        last_mtime=max_mtime,
+        last_mtime_ns=max_mtime,
         previous_file_list=file_list,
     )
     deleted_files = list(bdir.deleted_files())
@@ -139,7 +141,7 @@ def test_find_deleted_files(tmp_path):
 
     bdir = BinaryDirectory(
         tmp_path,
-        last_mtime=max_mtime,
+        last_mtime_ns=max_mtime,
         previous_file_list=file_list,
     )
     changed_files = list(bdir.changed_files())

tests/test_index.py

@@ -1,6 +1,8 @@
+import os
 import shutil
 from pathlib import Path
 from shutil import rmtree
+from subprocess import check_call
 
 import pytest
 
@@ -17,6 +19,12 @@
 # isort: on
 
 
+def _compile_file(output_file: Path, source: str, flags: list[str], ext="c"):
+    source_file = output_file.parent / f"{output_file.stem}.{ext}"
+    source_file.write_text(source)
+    check_call(["gcc", str(source_file), "-o", str(output_file), *flags])
+
+
 def test_indexing(fixture_path, tmp_path):
     index_path = tmp_path / "index"
     index_binary_directory(
@@ -400,6 +408,60 @@ def test_indexing_delete_saved_filters(fixture_path, tmp_path):
         assert any([p.name.endswith(".cpp.o") for p in paths])
 
 
+def test_indexing_triggered_after_file_was_updated(tmp_path):
+    dir = tmp_path / "build"
+    dir.mkdir()
+
+    file: Path = dir / "foo.o"
+    index_path = tmp_path / "index"
+
+    _compile_file(file, "void foo() {}", ["-c"])
+    index_binary_directory(dir, index_path, IndexingOptions())
+    with SymbolIndex.open(index_path, readonly=True) as index:
+        symbols = {s.name: s for s in index.search("*:*")}
+        assert "foo" in symbols
+
+    _compile_file(file, "void bar() {}", ["-c"])
+    index_binary_directory(dir, index_path, IndexingOptions())
+    with SymbolIndex.open(index_path, readonly=True) as index:
+        symbols = {s.name: s for s in index.search("*:*")}
+        assert "foo" not in symbols
+        assert "bar" in symbols
+
+    st = file.stat()
+    _compile_file(file, "void quux() {}", ["-c"])
+    os.utime(file, ns=(st.st_atime_ns + 1, st.st_mtime_ns + 1))
+    index_binary_directory(dir, index_path, IndexingOptions())
+    with SymbolIndex.open(index_path, readonly=True) as index:
+        symbols = {s.name: s for s in index.search("*:*")}
+        assert "foo" not in symbols
+        assert "bar" not in symbols
+        assert "quux" in symbols
+
+
+def test_indexing_not_triggered_if_mtime_not_changed(tmp_path):
+    dir = tmp_path / "build"
+    dir.mkdir()
+
+    file: Path = dir / "foo.o"
+    index_path = tmp_path / "index"
+
+    _compile_file(file, "void foo() {}", ["-c"])
+    index_binary_directory(dir, index_path, IndexingOptions())
+    with SymbolIndex.open(index_path, readonly=True) as index:
+        symbols = {s.name: s for s in index.search("*:*")}
+        assert "foo" in symbols
+
+    st = file.stat()
+    _compile_file(file, "void quux() {}", ["-c"])
+    os.utime(file, ns=(st.st_atime_ns, st.st_mtime_ns))
+    index_binary_directory(dir, index_path, IndexingOptions())
+    with SymbolIndex.open(index_path, readonly=True) as index:
+        symbols = {s.name: s for s in index.search("*:*")}
+        assert "foo" in symbols
+        assert "quux" not in symbols
+
+
 def test_searching_by_wildcard(readonly_index):
     symbols = set(readonly_index.search("name:a_*"))
     assert symbols