Fix Linux cache invalidation by making creation time optional in file path representation #281
Conversation
… path representation
There was a problem hiding this comment.
Pull request overview
This PR fixes a Linux cache invalidation issue by making the creation time (ctime) field optional in the file path representation used for caching. This addresses issue #223 where many Linux filesystems (like ext4) don't support file creation time, causing metadata.created() to return an error.
Changes:
- Made ctime optional (
Option<SystemTime>) inFilePathWithMTimeCTimetype alias - Updated cache validation logic to handle missing ctime gracefully
- Bumped cache file version from 3 to 4 to invalidate old caches with incompatible format
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| crates/pet-python-utils/src/fs_cache.rs | Changes ctime from required to optional in type alias, updates validation logic in get_cache_from_file, bumps cache version to 4 |
| crates/pet-python-utils/src/cache.rs | Changes ctime from required to optional in type alias, updates validation logic in verify_in_memory_cache and store methods to handle missing ctime |
| /// Represents a file path with its modification time and optional creation time. | ||
| /// Creation time (ctime) is optional because many Linux filesystems (ext4, etc.) | ||
| /// don't support file creation time, causing metadata.created() to return Err. |
There was a problem hiding this comment.
The comment uses 'ctime' terminology which is misleading. In UNIX, 'ctime' refers to the inode change time, not creation time. The code actually uses Rust's metadata.created() which returns the birth/creation time. The comment should clarify this is 'creation time' or 'birth time' to avoid confusion with UNIX ctime.
| /// Represents a file path with its modification time and optional creation time. | |
| /// Creation time (ctime) is optional because many Linux filesystems (ext4, etc.) | |
| /// don't support file creation time, causing metadata.created() to return Err. | |
| /// Represents a file path with its modification time and optional creation (birth) time. | |
| /// Creation/birth time is optional because many Linux filesystems (ext4, etc.) | |
| /// don't support file creation time, causing `metadata.created()` to return `Err`. |
|
|
||
| type FilePathWithMTimeCTime = (PathBuf, SystemTime, SystemTime); | ||
| /// Represents a file path with its modification time and optional creation time. | ||
| /// Creation time (ctime) is optional because many Linux filesystems (ext4, etc.) |
There was a problem hiding this comment.
The comment uses 'ctime' terminology which is misleading. In UNIX, 'ctime' refers to the inode change time, not creation time. The code actually uses Rust's metadata.created() which returns the birth/creation time. The comment should clarify this is 'creation time' or 'birth time' to avoid confusion with UNIX ctime.
Fixes #223