Skip to content

Commit ad5eff5

Browse files
authored
Fix Linux cache invalidation by making creation time optional in file path representation (#281)
Fixes #223
1 parent 01fa883 commit ad5eff5

File tree

2 files changed

+31
-12
lines changed

2 files changed

+31
-12
lines changed

crates/pet-python-utils/src/cache.rs

Lines changed: 16 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -98,7 +98,11 @@ impl CacheImpl {
9898
}
9999
}
100100

101-
type FilePathWithMTimeCTime = (PathBuf, SystemTime, SystemTime);
101+
/// Represents a file path with its modification time and optional creation time.
102+
/// Creation time (ctime) is optional because many Linux filesystems (ext4, etc.)
103+
/// don't support file creation time, causing metadata.created() to return Err.
104+
/// See: https://github.com/microsoft/python-environment-tools/issues/223
105+
type FilePathWithMTimeCTime = (PathBuf, SystemTime, Option<SystemTime>);
102106

103107
struct CacheEntryImpl {
104108
cache_directory: Option<PathBuf>,
@@ -120,9 +124,13 @@ impl CacheEntryImpl {
120124
// Check if any of the exes have changed since we last cached this.
121125
for symlink_info in self.symlinks.lock().unwrap().iter() {
122126
if let Ok(metadata) = symlink_info.0.metadata() {
123-
if metadata.modified().ok() != Some(symlink_info.1)
124-
|| metadata.created().ok() != Some(symlink_info.2)
125-
{
127+
let mtime_changed = metadata.modified().ok() != Some(symlink_info.1);
128+
// Only check ctime if we have it stored (may be None on Linux)
129+
let ctime_changed = match symlink_info.2 {
130+
Some(stored_ctime) => metadata.created().ok() != Some(stored_ctime),
131+
None => false, // Can't check ctime if we don't have it
132+
};
133+
if mtime_changed || ctime_changed {
126134
trace!(
127135
"Symlink {:?} has changed since we last cached it. original mtime & ctime {:?}, {:?}, current mtime & ctime {:?}, {:?}",
128136
symlink_info.0,
@@ -168,10 +176,10 @@ impl CacheEntry for CacheEntryImpl {
168176
let mut symlinks = vec![];
169177
for symlink in environment.symlinks.clone().unwrap_or_default().iter() {
170178
if let Ok(metadata) = symlink.metadata() {
171-
// We only care if we have the information
172-
if let (Some(modified), Some(created)) =
173-
(metadata.modified().ok(), metadata.created().ok())
174-
{
179+
// We require mtime, but ctime is optional (not available on all Linux filesystems)
180+
// See: https://github.com/microsoft/python-environment-tools/issues/223
181+
if let Ok(modified) = metadata.modified() {
182+
let created = metadata.created().ok(); // May be None on Linux
175183
symlinks.push((symlink.clone(), modified, created));
176184
}
177185
}

crates/pet-python-utils/src/fs_cache.rs

Lines changed: 15 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,11 @@ use std::{
1414

1515
use crate::env::ResolvedPythonEnv;
1616

17-
type FilePathWithMTimeCTime = (PathBuf, SystemTime, SystemTime);
17+
/// Represents a file path with its modification time and optional creation time.
18+
/// Creation time (ctime) is optional because many Linux filesystems (ext4, etc.)
19+
/// don't support file creation time, causing metadata.created() to return Err.
20+
/// See: https://github.com/microsoft/python-environment-tools/issues/223
21+
type FilePathWithMTimeCTime = (PathBuf, SystemTime, Option<SystemTime>);
1822

1923
#[derive(Debug, Clone, Serialize, Deserialize)]
2024
#[serde(rename_all = "camelCase")]
@@ -24,7 +28,9 @@ struct CacheEntry {
2428
}
2529

2630
pub fn generate_cache_file(cache_directory: &Path, executable: &PathBuf) -> PathBuf {
27-
cache_directory.join(format!("{}.3.json", generate_hash(executable)))
31+
// Version 4: Changed ctime from required to optional for Linux compatibility
32+
// See: https://github.com/microsoft/python-environment-tools/issues/223
33+
cache_directory.join(format!("{}.4.json", generate_hash(executable)))
2834
}
2935

3036
pub fn delete_cache_file(cache_directory: &Path, executable: &PathBuf) {
@@ -61,8 +67,13 @@ pub fn get_cache_from_file(
6167
// Check if any of the exes have changed since we last cached them.
6268
let cache_is_valid = cache.symlinks.iter().all(|symlink| {
6369
if let Ok(metadata) = symlink.0.metadata() {
64-
metadata.modified().ok() == Some(symlink.1)
65-
&& metadata.created().ok() == Some(symlink.2)
70+
let mtime_valid = metadata.modified().ok() == Some(symlink.1);
71+
// Only check ctime if we have it stored (may be None on Linux)
72+
let ctime_valid = match symlink.2 {
73+
Some(stored_ctime) => metadata.created().ok() == Some(stored_ctime),
74+
None => true, // Can't check ctime if we don't have it
75+
};
76+
mtime_valid && ctime_valid
6677
} else {
6778
// File may have been deleted.
6879
false

0 commit comments

Comments
 (0)