diff --git a/scripts/generate_db.py b/scripts/generate_db.py index 828c85ca..398a3b71 100644 --- a/scripts/generate_db.py +++ b/scripts/generate_db.py @@ -71,6 +71,34 @@ def _canonical_name(filepath: Path) -> str: return filepath.name +def _settled_hashes( + filepath: Path, cached: dict | None, mtime: float, size: int +) -> dict: + """The file's digests, computed again when they contradict the cache. + + A forced run rehashes everything, and this host has given a wrong SHA-1 + under load (DSi_Nand_USA.bin read as 834b140f, not ef08ae02). An entry + whose mtime and size did not move but whose digest did is hashed a + second time, and the two computations that agree win. The previous run's + value is the independent check the forced run used to throw away. + """ + hashes = compute_hashes(filepath) + if not cached or cached.get("mtime") != mtime or cached.get("size") != size: + return hashes + if cached.get("sha1") in (None, hashes["sha1"]): + return hashes + again = compute_hashes(filepath, cache=False) + if again["sha1"] == hashes["sha1"]: + # Two fresh readings agree: the bytes changed under an unchanged mtime. + return hashes + print( + f" WARNING: {filepath}: sha1 read as {hashes['sha1'][:8]} then " + f"{again['sha1'][:8]}, cached {cached['sha1'][:8]}", + file=sys.stderr, + ) + return again if again["sha1"] == cached["sha1"] else hashes + + def scan_bios_dir(bios_dir: Path, cache: dict, force: bool) -> tuple[dict, dict, dict]: """Scan bios directory and compute hashes, using cache when possible.""" files = {} @@ -132,7 +160,7 @@ def scan_bios_dir(bios_dir: Path, cache: dict, force: bool) -> tuple[dict, dict, new_cache[cache_key] = {**hashes, "mtime": mtime, "size": size} continue - hashes = compute_hashes(filepath) + hashes = _settled_hashes(filepath, cache.get(cache_key), mtime, size) sha1 = hashes["sha1"] is_variant = "/.variants/" in rel_path or "\\.variants\\" in rel_path if sha1 in files: @@ -362,7 +390,9 @@ def main() -> int: print(f"Error: BIOS directory '{bios_dir}' not found", file=sys.stderr) sys.exit(1) - cache = {} if args.force else load_cache(CACHE_FILE) + # A forced run rehashes every file but keeps the previous digests to + # check the new ones against. + cache = load_cache(CACHE_FILE) print(f"Scanning {bios_dir}/ ...") files, aliases, new_cache = scan_bios_dir(bios_dir, cache, args.force) diff --git a/scripts/hashing.py b/scripts/hashing.py index c6d89736..2dee6246 100644 --- a/scripts/hashing.py +++ b/scripts/hashing.py @@ -24,9 +24,15 @@ _hash_cache: dict[tuple, dict[str, str]] = {} def compute_hashes( filepath: str | Path, algorithms: frozenset[str] | None = None, + cache: bool = True, ) -> dict[str, str]: - """Compute file hashes. Pass *algorithms* to limit which are computed.""" + """Compute file hashes. Pass *algorithms* to limit which are computed. + + cache=False reads the file again, for a second opinion on a digest. + """ algos = frozenset(algorithms) if algorithms else _ALL_ALGORITHMS + if not cache: + return _compute_hashes(filepath, algos) try: stat = os.stat(filepath) key: tuple | None = ( diff --git a/tests/test_audit_regressions.py b/tests/test_audit_regressions.py index 7485084b..ac385869 100644 --- a/tests/test_audit_regressions.py +++ b/tests/test_audit_regressions.py @@ -5409,5 +5409,34 @@ class AnUnansweredSearchIsNotANotFound(unittest.TestCase): self.assertEqual(result["stats"]["errors"], 0) +class AForcedRehashIsCheckedAgainstTheLastOne(unittest.TestCase): + """This host has computed a wrong SHA-1 under load. The pipeline's forced + rebuild threw the previous digests away and wrote whatever it read.""" + + def test_a_reading_the_cache_and_a_second_reading_contradict_is_outvoted(self): + from unittest import mock + + from scripts import generate_db + + cached = {"sha1": "a" * 40, "mtime": 1.0, "size": 4} + readings = iter([{"sha1": "b" * 40}, {"sha1": "a" * 40}]) + with mock.patch.object(generate_db, "compute_hashes", lambda *a, **k: next(readings)), \ + contextlib.redirect_stderr(io.StringIO()) as err: + settled = generate_db._settled_hashes(Path("x.bin"), cached, 1.0, 4) + self.assertEqual(settled["sha1"], "a" * 40) + self.assertIn("WARNING", err.getvalue()) + + def test_a_changed_file_takes_its_new_digest(self): + from unittest import mock + + from scripts import generate_db + + cached = {"sha1": "a" * 40, "mtime": 1.0, "size": 4} + with mock.patch.object(generate_db, "compute_hashes", lambda *a, **k: {"sha1": "c" * 40}): + self.assertEqual( + generate_db._settled_hashes(Path("x.bin"), cached, 2.0, 4)["sha1"], "c" * 40 + ) + + if __name__ == "__main__": unittest.main()