-
-
Notifications
You must be signed in to change notification settings - Fork 8.2k
unix/extmod: Fix bugs with timestamps and Epoch #6390
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
It's my understanding that VFAT timestamps are in localtime (and that's what windows will store). So if you move an sdcard from a windows machine to a micropython machine then the timestamps will all be off if you're treating them as UTC. |
Ok. But in our VFS FAT layer the retrieval of timestamps does not take this into account, it just assumes UTC when converting the Y/M/D/H/M/S stored with the file to seconds-since-the-Epoch. So there may be a problem there as well.
The fix here targets only the unix port, when making timestamps for FAT files on a locally mounted filesystem using the VFS layer (not something that is done often, it has nothing to do with usual file access through the VfsPosix class). And the aim is to make it so that, after creating a file on a VfsFat filesystem, the timestamp of that file is equal to the value returned by |
Doing some tests with FAT on my Arch Linux machine: it seems that Linux stores FAT timestamps on disk in UTC format. So looking at the file on a Windows machine shows that the timestamp is out by the timezone offset. And vice versa: creating a file on Windows and then stat'ing it on Linux gives a timestamp which is out by the timezone in the opposite direction. Linux has a mount option for vfat - At this stage I'd prefer not to go down the path of trying to fix timezone behaviour/support, and just apply the patch here to make the unix port have the same behaviour as bare-metal for VfsFat filesystems. |
On ports like unix where the Epoch is 1970/1/1 and atime/mtime/ctime are in seconds since the Epoch, this value will overflow a small-int on 32-bit systems. So far this is only an issue on 32-bit unix builds that use the VFS layer (eg dev and coverage unix variants) but the fix (using mp_obj_new_int_from_uint instead of MP_OBJ_NEW_SMALL_INT) is there for all ports so as to not complicate the code, and because they will need the range one day. Also apply a similar fix to other fields in VfsPosix.stat because they may also be large. Signed-off-by: Damien George <damien@micropython.org>
On 32-bit builds these stat fields will overflow a small-int, so use mp_obj_new_int_from_uint to construct the int object. Signed-off-by: Damien George <damien@micropython.org>
By setting MICROPY_EPOCH_IS_1970 a port can opt to use 1970/1/1 as the Epoch for timestamps returned by stat(). And this setting is enabled on the unix and windows ports because that's what they use. Signed-off-by: Damien George <damien@micropython.org>
On second thoughts, it's probably best to follow how FAT was designed and how Windows does it and store localtime in the timestamps. Then at least that side of things is correct, it's creating a valid and correct FAT filesystem. Note: this only affects unix dev and coverage variants, when using |
ffe0b04
to
04c60d1
Compare
Signed-off-by: Damien George <damien@micropython.org>
Verifies mtime timestamps on files match the value returned by time.time(). Also update vfs_fat_ramdisk.py so it doesn't check FAT timestamp of the root, because that may change across runs/ports. Signed-off-by: Damien George <damien@micropython.org>
04c60d1
to
0e6ef40
Compare
This PR fixes the following bugs, all related to the unix port:
Also adds some tests to test the second and third points.