Skip to content

Commit 0fd43c2

Browse files
Copilotbranchseer
Copilot
and
branchseer
committed
Refactor Windows filesystem fingerprinting to reuse Unix implementation and add clippy rule for to_string_lossy
Co-authored-by: branchseer <3612422+branchseer@users.noreply.github.com>
1 parent 20040fd commit 0fd43c2

2 files changed

Lines changed: 111 additions & 116 deletions

File tree

‎.clippy.toml‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,8 @@ disallowed-methods = [
88
{ path = "str::replace", reason = "To avoid memory allocation, use `cow_utils::CowUtils::cow_replace` instead." },
99
{ path = "str::replacen", reason = "To avoid memory allocation, use `cow_utils::CowUtils::cow_replacen` instead." },
1010
{ path = "std::env::current_dir", reason = "To get an `AbsolutePathBuf`, Use `vite_path::current_dir` instead." },
11+
{ path = "std::ffi::OsStr::to_string_lossy", reason = "DO NOT ever use to_string_lossy on paths. Use proper UTF-8 handling instead." },
12+
{ path = "std::path::Path::to_string_lossy", reason = "DO NOT ever use to_string_lossy on paths. Use proper UTF-8 handling instead." },
1113
]
1214

1315
disallowed-types = [

‎crates/vite_task/src/fs.rs‎

Lines changed: 109 additions & 116 deletions
Original file line numberDiff line numberDiff line change
@@ -39,17 +39,26 @@ fn hash_content(mut stream: impl Read) -> io::Result<u64> {
3939
}
4040

4141
impl FileSystem for RealFileSystem {
42-
#[cfg(unix)]
4342
fn fingerprint_path(
4443
&self,
4544
path: &Arc<AbsolutePath>,
4645
path_read: PathRead,
4746
) -> Result<PathFingerprint, Error> {
48-
use nix::dir::{Dir, Type};
49-
5047
let file = match File::open(path.as_ref()) {
5148
Ok(file) => file,
5249
Err(err) => {
50+
// On Windows, File::open fails for directories, so we need to check
51+
// if this might be a directory before giving up
52+
#[cfg(windows)]
53+
{
54+
// On Windows, opening a directory can fail with various error codes
55+
// Try to check if it's a directory first
56+
let path_ref: &std::path::Path = path.as_ref();
57+
if let Ok(dir_iter) = std::fs::read_dir(path_ref) {
58+
return RealFileSystem::process_directory(dir_iter, path_read);
59+
}
60+
}
61+
5362
return if matches!(
5463
err.kind(),
5564
io::ErrorKind::NotFound |
@@ -69,133 +78,117 @@ impl FileSystem for RealFileSystem {
6978
if io_err.kind() != io::ErrorKind::IsADirectory {
7079
return Err(io_err.into());
7180
}
72-
// Is a directory
73-
let dir_entries: Option<std::collections::HashMap<Str, DirEntryKind>> =
74-
if path_read.read_dir_entries {
75-
let mut dir_entries = HashMap::<Str, DirEntryKind>::new();
76-
let dir = Dir::from_fd(reader.into_inner().into())?;
77-
for entry in dir {
78-
use bstr::ByteSlice;
79-
80-
let entry = entry?;
81-
82-
let entry_kind = match entry.file_type() {
83-
None => todo!("handle DT_UNKNOWN (see readdir(3))"),
84-
Some(Type::File) => DirEntryKind::File,
85-
Some(Type::Directory) => DirEntryKind::Dir,
86-
Some(Type::Symlink) => DirEntryKind::Symlink,
87-
Some(other_type) => {
88-
return Err(Error::UnsupportedFileType(other_type));
89-
}
90-
};
91-
let filename: &[u8] = entry.file_name().to_bytes();
92-
if matches!(filename, b"." | b".." | b".DS_Store") {
93-
continue;
94-
}
95-
dir_entries.insert(filename.to_str()?.into(), entry_kind);
96-
}
97-
Some(dir_entries)
98-
} else {
99-
None
100-
};
101-
return Ok(PathFingerprint::Folder(dir_entries));
81+
// Is a directory on Unix - use the optimized nix implementation first
82+
#[cfg(unix)]
83+
{
84+
return RealFileSystem::process_directory_unix(reader, path_read);
85+
}
86+
#[cfg(windows)]
87+
{
88+
// This shouldn't happen on Windows since File::open should have failed
89+
// But if it does, fallback to std::fs::read_dir
90+
let path_ref: &std::path::Path = path.as_ref();
91+
let dir_iter = std::fs::read_dir(path_ref)?;
92+
return RealFileSystem::process_directory(dir_iter, path_read);
93+
}
10294
}
10395
Ok(PathFingerprint::FileContentHash(hash_content(reader)?))
10496
}
97+
}
10598

106-
#[cfg(windows)]
107-
fn fingerprint_path(
108-
&self,
109-
path: &Arc<AbsolutePath>,
99+
impl RealFileSystem {
100+
#[cfg(unix)]
101+
fn process_directory_unix(
102+
reader: io::BufReader<File>,
110103
path_read: PathRead,
111104
) -> Result<PathFingerprint, Error> {
112-
use std::fs;
105+
use bstr::ByteSlice;
106+
use nix::dir::{Dir, Type};
113107

114-
// Try to open the path as a file first (single syscall)
115-
let file = match File::open(path.as_ref()) {
116-
Ok(file) => file,
117-
Err(err) => {
118-
return if matches!(
119-
err.kind(),
120-
io::ErrorKind::NotFound |
121-
// A component used as a directory in path is not a directory,
122-
// e.g. "/foo.txt/bar" where "/foo.txt" is a file
123-
io::ErrorKind::NotADirectory
124-
) {
125-
Ok(PathFingerprint::NotFound)
126-
} else {
127-
Err(Error::IoWithPath { err, path: path.clone() })
108+
let dir_entries: Option<HashMap<Str, DirEntryKind>> = if path_read.read_dir_entries {
109+
let mut dir_entries = HashMap::<Str, DirEntryKind>::new();
110+
let dir = Dir::from_fd(reader.into_inner().into())?;
111+
for entry in dir {
112+
let entry = entry?;
113+
114+
let entry_kind = match entry.file_type() {
115+
None => todo!("handle DT_UNKNOWN (see readdir(3))"),
116+
Some(Type::File) => DirEntryKind::File,
117+
Some(Type::Directory) => DirEntryKind::Dir,
118+
Some(Type::Symlink) => DirEntryKind::Symlink,
119+
Some(other_type) => {
120+
return Err(Error::UnsupportedFileType(other_type));
121+
}
128122
};
123+
let filename: &[u8] = entry.file_name().to_bytes();
124+
if matches!(filename, b"." | b".." | b".DS_Store") {
125+
continue;
126+
}
127+
dir_entries.insert(filename.to_str()?.into(), entry_kind);
129128
}
129+
Some(dir_entries)
130+
} else {
131+
None
130132
};
133+
Ok(PathFingerprint::Folder(dir_entries))
134+
}
131135

132-
// Try to read from the file to check if it's actually a directory
133-
let mut reader = io::BufReader::new(file);
134-
if let Err(io_err) = reader.fill_buf() {
135-
// On Windows, trying to read from a directory handle typically fails
136-
// with a different error than IsADirectory, but let's handle it generically
137-
// by checking if we can read the directory instead
138-
139-
// Check if this is a directory by trying to read it as such
140-
match fs::read_dir(path.as_ref()) {
141-
Ok(dir_iter) => {
142-
// It's a directory, process entries if requested
143-
let dir_entries: Option<std::collections::HashMap<Str, DirEntryKind>> =
144-
if path_read.read_dir_entries {
145-
let mut dir_entries = HashMap::<Str, DirEntryKind>::new();
146-
147-
for entry in dir_iter {
148-
let entry = entry?;
149-
let file_name = entry.file_name();
150-
151-
// Skip special entries (same as Unix version)
152-
if matches!(
153-
file_name.to_string_lossy().as_ref(),
154-
"." | ".." | ".DS_Store"
155-
) {
156-
continue;
157-
}
158-
159-
// Get file type with minimal additional syscalls
160-
let entry_kind = match entry.file_type() {
161-
Ok(file_type) => {
162-
if file_type.is_file() {
163-
DirEntryKind::File
164-
} else if file_type.is_dir() {
165-
DirEntryKind::Dir
166-
} else if file_type.is_symlink() {
167-
DirEntryKind::Symlink
168-
} else {
169-
// For any other file type, we'll treat it as a file
170-
// This is a conservative approach for Windows
171-
DirEntryKind::File
172-
}
173-
}
174-
Err(_) => {
175-
// If we can't determine the file type, treat as file
176-
DirEntryKind::File
177-
}
178-
};
179-
180-
// Convert filename to Str (using OsStr -> String conversion)
181-
let filename_str = file_name.to_string_lossy();
182-
dir_entries.insert(filename_str.as_ref().into(), entry_kind);
183-
}
184-
Some(dir_entries)
185-
} else {
186-
None
187-
};
188-
return Ok(PathFingerprint::Folder(dir_entries));
136+
#[cfg(any(unix, windows))]
137+
fn process_directory(
138+
dir_iter: std::fs::ReadDir,
139+
path_read: PathRead,
140+
) -> Result<PathFingerprint, Error> {
141+
let dir_entries: Option<HashMap<Str, DirEntryKind>> = if path_read.read_dir_entries {
142+
let mut dir_entries = HashMap::<Str, DirEntryKind>::new();
143+
144+
for entry in dir_iter {
145+
let entry = entry?;
146+
let file_name = entry.file_name();
147+
148+
// Skip special entries (same as Unix version)
149+
// Convert OsStr to bytes for comparison to avoid to_string_lossy
150+
let file_name_bytes = file_name.as_encoded_bytes();
151+
if matches!(file_name_bytes, b"." | b".." | b".DS_Store") {
152+
continue;
189153
}
190-
Err(_) => {
191-
// Not a directory, so the original error was legitimate
192-
return Err(io_err.into());
154+
155+
// Get file type with minimal additional syscalls
156+
let entry_kind = match entry.file_type() {
157+
Ok(file_type) => {
158+
if file_type.is_file() {
159+
DirEntryKind::File
160+
} else if file_type.is_dir() {
161+
DirEntryKind::Dir
162+
} else if file_type.is_symlink() {
163+
DirEntryKind::Symlink
164+
} else {
165+
// For any other file type, we'll treat it as a file
166+
// This is a conservative approach
167+
DirEntryKind::File
168+
}
169+
}
170+
Err(_) => {
171+
// If we can't determine the file type, treat as file
172+
DirEntryKind::File
173+
}
174+
};
175+
176+
// Convert filename to Str - need to ensure it's valid UTF-8
177+
match file_name.to_str() {
178+
Some(filename_str) => {
179+
dir_entries.insert(filename_str.into(), entry_kind);
180+
}
181+
None => {
182+
// Skip files with invalid UTF-8 names
183+
continue;
184+
}
193185
}
194186
}
195-
}
196-
197-
// If we got here, it's a regular file and we can read it
198-
Ok(PathFingerprint::FileContentHash(hash_content(reader)?))
187+
Some(dir_entries)
188+
} else {
189+
None
190+
};
191+
Ok(PathFingerprint::Folder(dir_entries))
199192
}
200193
}
201194

0 commit comments

Comments
 (0)