Skip to content
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
24 changes: 16 additions & 8 deletions crates/pet-python-utils/src/cache.rs
Original file line number Diff line number Diff line change
Expand Up @@ -98,7 +98,11 @@ impl CacheImpl {
}
}

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.)
/// don't support file creation time, causing metadata.created() to return Err.
Comment on lines +101 to +103
Copy link

Copilot AI Jan 16, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Suggested change
/// 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`.

Copilot uses AI. Check for mistakes.
/// See: https://github.com/microsoft/python-environment-tools/issues/223
type FilePathWithMTimeCTime = (PathBuf, SystemTime, Option<SystemTime>);

struct CacheEntryImpl {
cache_directory: Option<PathBuf>,
Expand All @@ -120,9 +124,13 @@ impl CacheEntryImpl {
// Check if any of the exes have changed since we last cached this.
for symlink_info in self.symlinks.lock().unwrap().iter() {
if let Ok(metadata) = symlink_info.0.metadata() {
if metadata.modified().ok() != Some(symlink_info.1)
|| metadata.created().ok() != Some(symlink_info.2)
{
let mtime_changed = metadata.modified().ok() != Some(symlink_info.1);
// Only check ctime if we have it stored (may be None on Linux)
let ctime_changed = match symlink_info.2 {
Some(stored_ctime) => metadata.created().ok() != Some(stored_ctime),
None => false, // Can't check ctime if we don't have it
};
if mtime_changed || ctime_changed {
trace!(
"Symlink {:?} has changed since we last cached it. original mtime & ctime {:?}, {:?}, current mtime & ctime {:?}, {:?}",
symlink_info.0,
Expand Down Expand Up @@ -168,10 +176,10 @@ impl CacheEntry for CacheEntryImpl {
let mut symlinks = vec![];
for symlink in environment.symlinks.clone().unwrap_or_default().iter() {
if let Ok(metadata) = symlink.metadata() {
// We only care if we have the information
if let (Some(modified), Some(created)) =
(metadata.modified().ok(), metadata.created().ok())
{
// We require mtime, but ctime is optional (not available on all Linux filesystems)
// See: https://github.com/microsoft/python-environment-tools/issues/223
if let Ok(modified) = metadata.modified() {
let created = metadata.created().ok(); // May be None on Linux
symlinks.push((symlink.clone(), modified, created));
}
}
Expand Down
19 changes: 15 additions & 4 deletions crates/pet-python-utils/src/fs_cache.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,11 @@ use std::{

use crate::env::ResolvedPythonEnv;

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.)
Copy link

Copilot AI Jan 16, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copilot uses AI. Check for mistakes.
/// don't support file creation time, causing metadata.created() to return Err.
/// See: https://github.com/microsoft/python-environment-tools/issues/223
type FilePathWithMTimeCTime = (PathBuf, SystemTime, Option<SystemTime>);

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

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

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