modulecache: implement file metadata on Windows - #28926
quaesitor-scientiam wants to merge 4 commits into
Conversation
v3_modulecache_file_metadata had implementations for macOS and Linux only. On Windows it fell through to the stub that returns 0, so file_metadata_signature() returned an empty signature: file_metadata_test.v failed (`assert signature.len > 0`), and modulecache_test.v panicked in test_cached_source_signature_tracks_vml_inputs because the cache directory was never created. Add a Windows branch that opens the file for metadata only (no access bits, FILE_FLAG_BACKUP_SEMANTICS so directories work too) and reads GetFileInformationByHandle: the volume serial number and the 64-bit file index are the device/inode pair, plus the size and the last-write and creation times. The CRT's stat() reports st_ino == 0 on Windows, so it cannot be used here. File systems without a file index (FAT, exFAT, some network redirectors) are declined, so callers keep hashing the contents rather than treating distinct files as equal. Co-Authored-By: WOZCODE <contact@withwoz.com>
medvednikov
left a comment
There was a problem hiding this comment.
One correctness concern with the file_index == 0 fallback: returning 0 here does not actually make the cached source-signature path fall back to hashing the dependency contents. optional_file_metadata_signature() maps an empty metadata signature to "missing", and valid_cached_source_signature() validates vml= entries by comparing file_metadata_signature() only. So on FAT/exFAT or a redirector where this branch returns 0, an existing VML input can change while validation still sees the same empty metadata; similarly, a previously missing lookup candidate can appear and still compare as "missing". That preserves the same stale-cache failure mode this Windows implementation is intended to fix on those filesystems.
Could we make the metadata-unavailable case actually hash the relevant dependency contents during validation (or otherwise provide a non-empty change-sensitive signature) instead of treating it as missing?
…etadata On FAT, exFAT and some network redirectors Windows reports no file index, and file_metadata_signature returns nothing. Source files already fall back to hashing their contents in that case, but the memoized source signature recorded the vml, vml lookup, vml candidate and v.mod dependencies by metadata alone. An empty signature matched itself, and "exists without metadata" read as "missing", so an edited template, an edited v.mod, or a template that appeared on such a drive kept a stale cached signature. file_change_signature records such a dependency by a digest of its contents instead, reports "missing" only when nothing exists at the path, and returns nothing for a path that can be neither described nor read; the signature is then not cached, and validation never accepts it. A vml input's state is now recorded before the bytes the signature hashes are read, so an edit in between leaves an older record that the next validation rejects. The same fallback, through the existing v3_cache_file_identity, now covers the crun build identity's external inputs and the C and V compiler executable identities, which recorded metadata alone too. V3_TEST_NO_FILE_METADATA lists paths for which file_metadata_signature reports nothing, so tests can put a dependency on such a file system while the source stays on one with file identities. Co-Authored-By: WOZCODE <contact@withwoz.com>
Tested on real drives on Windows 11: FAT32 and exFAT volumes do report a file index, so the file_index == 0 branch is not what makes them stale, and the comments saying so were wrong. What does make them stale is timestamp granularity: they keep modification times in 2 second steps (HFS+ in 1 second steps), so a same-size edit inside one step leaves the file index, size and times identical, and the metadata still matches the cached entry. file_metadata_signature now reports nothing for a file whose modification time has no sub-second part and is less than 3 seconds old, or in the future. Every caller already treats that as "compare the contents": source files skip the signature memo, dependency signatures record a content digest, and object stamps compare file signatures. Once the step is over, any edit gets a strictly later timestamp, so the metadata can be trusted again. Fine grained timestamps (NTFS, ext4, APFS) are unaffected, except for the rare file whose time falls exactly on a whole second. On a FAT32 and an exFAT drive, an immediate same-size edit to a vml template used by a source on NTFS was missed before this change and is detected after it; an edit made once the timestamp step had passed is detected too. Co-Authored-By: WOZCODE <contact@withwoz.com>
|
Thanks, the stale-cache concern was right, though on real drives it comes from a different place than the What real FAT32 and exFAT drives do (Windows 11): both report a non-zero file index, so that branch is not taken there (my comment claiming otherwise was wrong and is fixed). They are still stale, because they keep modification times in 2 second steps: a same-size edit inside one step leaves the file index, size and times identical. With a vml template on the FAT32 or exFAT drive and the source on NTFS, a same-size template edit was missed by this PR as it stood. Changes:
Verified on the real drives (same-size edit of a vml template on the drive, source on NTFS): missed before, detected after, on both FAT32 and exFAT; an edit made once the step had passed is detected too. The new tests reproduce both cases without special hardware: Test runs: |
medvednikov
left a comment
There was a problem hiding this comment.
Re-reviewed at 264d40e, ignoring CI.
The original metadata-unavailable finding is addressed: VML dependencies, lookup candidates, and v.mod inputs now distinguish missing files from existing files without metadata and validate the latter through their contents. The compiler-identity fallback is covered too.
One remaining Windows cache-invalidation case is noted inline: same-size in-place edits that preserve an older modification time.
Local validation after rebuilding the compiler: file_metadata_test.v (3 tests), modulecache_test.v (42 tests), and cache_prune_test.v (36 tests) all passed on Linux. The Windows finding is based on the code path and documented Win32 API behavior; I did not run a Windows reproducer.
| // Windows has no inode change time. Creation time is the closest stable | ||
| // companion field; the write time and size above are what actually move when | ||
| // a source file is edited. | ||
| v3_modulecache_filetime_parts(info.ftCreationTime, ctime_seconds, ctime_nanoseconds); |
There was a problem hiding this comment.
[P2] Include Windows change time in the metadata signature
Using ftCreationTime here still permits a stale cache after a same-size in-place edit that preserves the old last-write time. For example: set a VML input's mtime to now - 600, cache its source signature, overwrite First with Other in place, and restore that same mtime with os.utime. Its volume, file index, size, creation time, and mtime are unchanged; the new three-second guard accepts this old timestamp, so file_change_signature_matches() accepts the previous signature without hashing the changed contents. Timestamp-preserving updates can therefore reuse stale generated code even on NTFS.
Windows does expose a distinct FILE_BASIC_INFO.ChangeTime, obtainable through GetFileInformationByHandleEx(FileBasicInfo). Could we use that for the change-time fields, with a conservative content fallback where it is unavailable, and add a Windows regression covering an in-place same-size edit with a restored old mtime?
There was a problem hiding this comment.
Thanks, reproduced and fixed in 1c4f002.
Reproduction (NTFS, Windows 11): your sequence (mtime set to now − 600, signature taken, same-size overwrite in place, mtime restored with os.utime) left the volume serial, file index, size, write time and creation time identical. The new regression test failed on 264d40e with equal signatures.
Change: the Windows ctime fields now come from FILE_BASIC_INFO.ChangeTime via GetFileInformationByHandleEx(FileBasicInfo). It moves on every write and also when the write time is set back. Two tcc details:
- the kernel32 import list bundled with tcc has no
GetFileInformationByHandleEx(the same kind of gap asGetThreadIdin workers: record Windows worker thread ids instead of calling GetThreadId #28964), so it is resolved withGetProcAddress; - tcc's headers do not declare
FILE_BASIC_INFO, so the layout is declared locally.
Where it is unavailable: on real drives, FAT32 and exFAT report ChangeTime = 0. The helper then declines and callers compare contents, as on the existing file-index-0 path. A failed query does the same.
Test: test_file_metadata_signature_sees_an_edit_behind_a_restored_mtime runs on every platform (on Linux and macOS it covers st_ctime). It fails at 264d40e on Windows and passes now on Windows (tcc and gcc) and Linux.
Validation:
- Windows,
vlib/v/modulecache+vlib/v/driver: 18 passed, 2 failed. The failures aretest_v3_windows_default_tcc_prod_buildandmodulecache_testtest_cached_file_line_uses_source_file_name(its expected string has single backslashes, the rewritten line has escaped ones); both failed the same way in this branch's earlier rounds. - Linux:
file_metadata_test.v,modulecache_test.vandcache_prune_test.vpass.
Not covered: 32-bit Windows, MSVC and clang builds, and SMB shares (a failed query falls back to comparing contents). An edit that lands within one timestamp clock tick of the file's previous metadata change can still leave identical metadata on any file system; the test sleeps 100 ms for that reason.
The Windows file signature paired the write time with the creation time. A same-size edit in place that restores the old write time left the volume, file index, size and both times unchanged, so a stale cached signature still matched (reproduced on NTFS with os.utime). Take the change time from GetFileInformationByHandleEx(FileBasicInfo) instead: it moves on every write and also when the write time is set back. The function is resolved with GetProcAddress because the kernel32 import list bundled with tcc does not have it, and FILE_BASIC_INFO is declared locally because tcc's headers do not. FAT32 and exFAT report a change time of 0 (checked on real drives), as can a failed query; the metadata then declines and callers compare the contents, as they already do for files without a file index. Co-Authored-By: WOZCODE <contact@withwoz.com>
v3_modulecache_file_metadata(vlib/v/modulecache/file_metadata.c) had implementations for macOS and Linux only. On Windows it fell through to the stub that returns 0, sofile_metadata_signature()returned an empty signature.Fix
A
_WIN32branch opens the file for metadata only (no access bits,FILE_FLAG_BACKUP_SEMANTICSso directories work too) and readsGetFileInformationByHandle:stat()reportsst_ino == 0on Windows, so it can't be used.File systems without a file index (FAT, exFAT, some network redirectors) are declined, so callers keep hashing the contents rather than treating distinct files as equal.
file_metadata_test.vexpects the extra implementation (the signature count goes from 2 to 3).Tests (Windows 11, master
b9a2d87bc6)file_metadata_test.vassert signature.len > 0)modulecache_test.vtest_cached_source_signature_tracks_vml_inputsls()on a cache dir that was never created)Both pass with tcc, gcc and clang. The new branch also compiles cleanly on its own with gcc and clang (
-Wall -Werror=incompatible-pointer-types, bothu64typedefs).modulecache_test.vstill fails at line 92 (test_cached_file_line_uses_source_file_name) on Windows with and without this change: the rewritten path has doubled backslashes. That is a separate issue.Not run on Linux or macOS: the change is a new
#elif defined(_WIN32)branch, so their code is unchanged.🧙 Built with WOZCODE