diff --git a/src/sqlbuild/cli/commands/_helpers/compile/sql_test_artifact_cache.py b/src/sqlbuild/cli/commands/_helpers/compile/sql_test_artifact_cache.py index c34c588311..64c5ccd69f 100644 --- a/src/sqlbuild/cli/commands/_helpers/compile/sql_test_artifact_cache.py +++ b/src/sqlbuild/cli/commands/_helpers/compile/sql_test_artifact_cache.py @@ -6,6 +6,7 @@ import json import os import platform +from dataclasses import replace from importlib.metadata import PackageNotFoundError, version from pathlib import Path from typing import cast @@ -26,7 +27,7 @@ CompileSqlTestCte, ) -_CACHE_VERSION: int = 3 +_CACHE_VERSION: int = 4 _ALGORITHM_FINGERPRINT: str = "sql-test-artifact-v4-orjson-identity" _CACHE_FILE_NAME: str = "sql-test-artifacts.json" _MAX_CACHE_BYTES: int = 10_000_000 @@ -142,6 +143,7 @@ def write_sql_test_artifact_cache( "relative_path": record.relative_path.as_posix(), "size": record.size, "mtime_ns": record.mtime_ns, + "content_sha256": record.content_sha256, } for key, record in records.items() }, @@ -161,23 +163,23 @@ def write_sql_test_artifact_cache( def artifact_matches_cache_record( *, tests_root: Path, record: SqlTestArtifactCacheRecord, identity: str -) -> Path | None: - """Return the safe existing artifact path when identity and stat metadata match.""" +) -> SqlTestArtifactCacheRecord | None: + """Return the reusable artifact's record, verifying content when only its mtime moved.""" if record.identity != identity or not _is_safe_relative_sql_path(record.relative_path): return None artifact_path: Path = tests_root / record.relative_path try: stat: os.stat_result = artifact_path.stat() + if not artifact_path.is_file() or stat.st_size != record.size: + return None + if stat.st_mtime_ns == record.mtime_ns: + return record + if _file_sha256(artifact_path) != record.content_sha256: + return None except OSError: return None - if ( - not artifact_path.is_file() - or stat.st_size != record.size - or stat.st_mtime_ns != record.mtime_ns - ): - return None - return artifact_path + return replace(record, mtime_ns=stat.st_mtime_ns) def build_sql_test_artifact_cache_record( @@ -188,6 +190,7 @@ def build_sql_test_artifact_cache_record( try: relative_path: Path = artifact_path.relative_to(tests_root) stat: os.stat_result = artifact_path.stat() + content_sha256: str = _file_sha256(artifact_path) except (OSError, ValueError): return None if not _is_safe_relative_sql_path(relative_path): @@ -197,6 +200,7 @@ def build_sql_test_artifact_cache_record( relative_path=relative_path, size=stat.st_size, mtime_ns=stat.st_mtime_ns, + content_sha256=content_sha256, ) @@ -252,6 +256,7 @@ def _record_from_payload(payload: object) -> SqlTestArtifactCacheRecord | None: relative_path: object = values.get("relative_path") size: object = values.get("size") mtime_ns: object = values.get("mtime_ns") + content_sha256: object = values.get("content_sha256") if ( not isinstance(identity, str) or not isinstance(relative_path, str) @@ -261,6 +266,7 @@ def _record_from_payload(payload: object) -> SqlTestArtifactCacheRecord | None: or not isinstance(mtime_ns, int) or isinstance(mtime_ns, bool) or mtime_ns < 0 + or not isinstance(content_sha256, str) ): return None path: Path = Path(relative_path) @@ -271,6 +277,7 @@ def _record_from_payload(payload: object) -> SqlTestArtifactCacheRecord | None: relative_path=path, size=size, mtime_ns=mtime_ns, + content_sha256=content_sha256, ) @@ -282,6 +289,10 @@ def _is_safe_relative_sql_path(path: Path) -> bool: ) +def _file_sha256(path: Path) -> str: + return hashlib.sha256(path.read_bytes()).hexdigest() + + def _payload_digest(payload: object) -> str: return hashlib.sha256(orjson.dumps(payload, option=orjson.OPT_SORT_KEYS)).hexdigest() diff --git a/src/sqlbuild/cli/commands/_helpers/compile/target_writer.py b/src/sqlbuild/cli/commands/_helpers/compile/target_writer.py index 6f04c17d01..b58ec2ecce 100644 --- a/src/sqlbuild/cli/commands/_helpers/compile/target_writer.py +++ b/src/sqlbuild/cli/commands/_helpers/compile/target_writer.py @@ -544,19 +544,20 @@ def _partition_cached_static_tests( ) cached_record: SqlTestArtifactCacheRecord | None = cached_records.get(record_key) if cached_record is not None: - cached_path: Path | None = artifact_matches_cache_record( + reusable_record: SqlTestArtifactCacheRecord | None = artifact_matches_cache_record( tests_root=tests_root, record=cached_record, identity=artifact_identity, ) - if cached_path is not None: + if reusable_record is not None: + cached_path: Path = tests_root / reusable_record.relative_path COMPILE_ARTIFACT_WRITES.kept( path=cached_path, - size=cached_record.size, - mtime_ns=cached_record.mtime_ns, + size=reusable_record.size, + mtime_ns=reusable_record.mtime_ns, ) managed_paths.add(cached_path) - current_records[record_key] = cached_record + current_records[record_key] = reusable_record continue pending.append( PendingStaticSqlTest( diff --git a/src/sqlbuild/cli/compile/models.py b/src/sqlbuild/cli/compile/models.py index a590ab705e..2ef89e4674 100644 --- a/src/sqlbuild/cli/compile/models.py +++ b/src/sqlbuild/cli/compile/models.py @@ -66,6 +66,7 @@ class SqlTestArtifactCacheRecord: relative_path: Path size: int mtime_ns: int + content_sha256: str @dataclass(frozen=True) diff --git a/tests/unit/src/sqlbuild/cli/commands/main/compile/test_compile_target_writer.py b/tests/unit/src/sqlbuild/cli/commands/main/compile/test_compile_target_writer.py index 5ce8ab56c7..c851c21ef7 100644 --- a/tests/unit/src/sqlbuild/cli/commands/main/compile/test_compile_target_writer.py +++ b/tests/unit/src/sqlbuild/cli/commands/main/compile/test_compile_target_writer.py @@ -341,6 +341,48 @@ def test_given_unchanged_test_artifact_when_writing_again_then_skips_test_plan_r assert planner_spy.call_count == test_case.expected_builder_calls +@pytest.mark.parametrize( + "test_case", + ( + TargetWriterCacheTestCase( + description="identical bytes with a moved mtime", expected_builder_calls=0 + ), + ), + ids=lambda case: case.description, +) +def test_given_artifact_with_moved_mtime_and_same_bytes_when_writing_then_reuses_it( + tmp_path: Path, + monkeypatch: pytest.MonkeyPatch, + test_case: TargetWriterCacheTestCase, +) -> None: + target_dir: Path = tmp_path / "target" + project: CompiledProject = build_cached_target_writer_project(target_dir=target_dir) + _ = write_static_compile_target( + target_dir=target_dir, + adapter=DuckDbAdapter(), + project=project, + ) + artifact_path: Path = next((target_dir / "compiled" / "tests").rglob("*.sql")) + moved_mtime_ns: int = artifact_path.stat().st_mtime_ns + 1_000_000_000 + os.utime(artifact_path, ns=(moved_mtime_ns, moved_mtime_ns)) + planner_spy: Mock = Mock(wraps=target_writer_module.plan_and_render_sql_test_artifacts) + monkeypatch.setattr(target_writer_module, "plan_and_render_sql_test_artifacts", planner_spy) + + _ = write_static_compile_target( + target_dir=target_dir, + adapter=DuckDbAdapter(), + project=project, + ) + _ = write_static_compile_target( + target_dir=target_dir, + adapter=DuckDbAdapter(), + project=project, + ) + + assert planner_spy.call_count == test_case.expected_builder_calls + assert artifact_path.stat().st_mtime_ns == moved_mtime_ns + + @pytest.mark.parametrize( "test_case", (