Repository navigation
fix(libkernel): preserve Windows file timestamp precision - #1518
Conversation
klosixgaming-arch
left a comment
There was a problem hiding this comment.
This is a well-implemented fix for Windows file timestamp precision. The approach of using GetFileInformationByHandleEx with FileBasicInfo and direct SetFileTime calls is the right way to preserve microsecond/nanosecond precision that _wstat64/_utime lose.
Strengths:
- Clean encapsulation in a dedicated
WindowsFileTimehelper class - Thorough input validation (microsecond ranges, epoch boundaries, overflow checks)
- Comprehensive test suite covering edge cases: pre-epoch times, invalid microseconds, boundary conditions, directory timestamps, pipe handling
- Proper error mapping from Windows errors to POSIX errno values
- Good documentation of remaining limitations in TechnicalDebt.md
Minor notes:
- The
ticks == 0check inEncode()correctly handles the "leave unchanged" semantics ofSetFileTime, which is a subtle but important detail - Test coverage for both file and directory descriptors is excellent
Ready to merge. This will improve compatibility with games that rely on precise file timestamps.
|
Verified the same way — merges clean onto current
The approach reads well: One question on the non-disk path: for a pipe or a device, Written with AI assistance. |
|
Rebased onto current |
acea8db to
705e49d
Compare
…mestamps fix(libkernel): preserve Windows file timestamp precision
What
On Windows,
utimesandfutimesdiscarded the microsecond fields when passing timestamps to_wutime. The stat bridge also reported zero fractional seconds, even when the file contained a precise native timestamp. The existing filesystem test requested a half-second timestamp but checked only whole seconds.This change:
SetFileTime, retaining guest microseconds. Null time arrays use the current Windows clock.FileBasicInfo, retaining 100-nanosecond precision.st_ctimuses metadata change time andst_birthtimuses creation time.ReOpenFilefor regular-file descriptor updates, including read-only descriptors, so the operation targets the open file rather than reopening its pathname.guest_file_timestampsregression test and documents the remaining host limits.Native API behavior follows Microsoft's SetFileTime, FILE_BASIC_INFO, and ReOpenFile documentation.
Tested
Windows validation of PR head
acea8db3a4cd4db9dfdfbe1f3dcaeceff939c9d6, rebased onto upstream main4b2413fb5716bb56101e777536fe374e49ba90a2. This branch is independent of #1507 and contains no wcsrtombs changes.Windows 11 Enterprise 10.0.26200; WinLibs MinGW-w64 GCC 15.2.0 r2 UCRT/posix/SEH, Ninja Release,
BUILD_TESTING=ON, SPIRV-Tools disabled. The installed toolchain is r2, not CI's r7 package.The five selected test targets and their libc/libkernel dependencies rebuilt successfully after the rebase:
guest_file_timestampsguest_filesystemguest_directory_bufferguest_directory_closeguest_file_vector5/5 passed, 0 failed, 0 skipped. The new test covers distinct access/modification fractions, read-only descriptors, all three guest stat entry points, native 100-nanosecond timestamps, creation versus metadata-change time, dates around the Unix epoch, invalid microseconds in either field, unrepresentable Windows dates, unchanged timestamps after rejected updates, null time arrays, directories, missing paths and pipe stat behavior.
Focused reproduction, using the original timestamp implementation from
e28fa01ca60c1db29df747b16ce5595c68d7cc27and then rerun against the published head:The new regression test was also compiled against the pre-fix runtime and failed at
access time preserves microseconds. Native Windows reads independently check the stored timestamps, so matching guest read/write bugs cannot make the precision checks pass. These are focused reproductions, not a full-suite baseline comparison.The committed-head conventions check, diff whitespace check and test-registration check passed. The registration check found no newly unregistered test executables.
These results cover the pinned PR head. Linux, the full CTest suite, the complete default/libs build, games and the eventual merged tree were not tested. No performance or game-compatibility conclusion is claimed.
Remaining limits are recorded in TechnicalDebt: timestamps follow the host filesystem's range and resolution; the exact Windows epoch is rejected because zero means leave unchanged to SetFileTime; guest directory descriptors still use their recorded paths.
Checklist
main; no other open PR implements the same functionsNotImplemented_nid_no_patch); silent stubs are listed in TechnicalDebt.mdfiles or images added to the repository (attach them to this PR)