diff --git a/crates/vite_workspace/src/lib.rs b/crates/vite_workspace/src/lib.rs index a9801d91..84a894ef 100644 --- a/crates/vite_workspace/src/lib.rs +++ b/crates/vite_workspace/src/lib.rs @@ -248,16 +248,16 @@ pub fn load_package_graph( let mut graph_builder = PackageGraphBuilder::default(); let workspaces = match &workspace_root.workspace_file { WorkspaceFile::PnpmWorkspaceYaml(file_with_path) => { - let workspace: PnpmWorkspace = - serde_yml::from_reader(file_with_path.file()).map_err(|e| Error::SerdeYml { + let workspace: PnpmWorkspace = serde_yml::from_slice(file_with_path.content()) + .map_err(|e| Error::SerdeYml { file_path: Arc::clone(file_with_path.path()), serde_yml_error: e, })?; workspace.packages } WorkspaceFile::NpmWorkspaceJson(file_with_path) => { - let workspace: NpmWorkspace = - serde_json::from_reader(file_with_path.file()).map_err(|e| Error::SerdeJson { + let workspace: NpmWorkspace = serde_json::from_slice(file_with_path.content()) + .map_err(|e| Error::SerdeJson { file_path: Arc::clone(file_with_path.path()), serde_json_error: e, })?; @@ -265,7 +265,7 @@ pub fn load_package_graph( } WorkspaceFile::NonWorkspacePackage(file_with_path) => { // For non-workspace packages, add the package.json to the graph as a root package - let package_json: PackageJson = serde_json::from_reader(file_with_path.file()) + let package_json: PackageJson = serde_json::from_slice(file_with_path.content()) .map_err(|e| Error::SerdeJson { file_path: Arc::clone(file_with_path.path()), serde_json_error: e, diff --git a/crates/vite_workspace/src/package_manager.rs b/crates/vite_workspace/src/package_manager.rs index c30bf9a7..995d3762 100644 --- a/crates/vite_workspace/src/package_manager.rs +++ b/crates/vite_workspace/src/package_manager.rs @@ -1,51 +1,49 @@ -use std::{ - fs::File, - io::{BufReader, Seek, SeekFrom}, - sync::Arc, -}; +use std::{fs, sync::Arc}; use vite_path::{AbsolutePath, RelativePathBuf}; use crate::Error; -/// A file handle bundled with its absolute path for error context. +/// The contents of a file bundled with its absolute path for error context. +/// +/// The file is read to memory on construction and its handle closed +/// immediately — the struct itself never holds a live OS file handle. This +/// keeps long-lived `WorkspaceRoot`s (held across an entire `vp run` session) +/// from pinning files like `pnpm-workspace.yaml`, which on Windows could +/// otherwise block pnpm's atomic write-and-rename and fail with EPERM +/// (). #[derive(Debug)] pub struct FileWithPath { - file: File, + content: Vec, path: Arc, } impl FileWithPath { - /// Open a file at the given path. + /// Open a file at the given path and read its contents into memory. /// /// # Errors - /// Returns an error if the file cannot be opened. + /// Returns an error if the file cannot be read. pub fn open(path: Arc) -> Result { - let file = File::open(&*path)?; - Ok(Self { file, path }) + let content = fs::read(&*path)?; + Ok(Self { content, path }) } - /// Try to open a file, returning None if it doesn't exist. + /// Try to read a file, returning None if it doesn't exist. /// /// # Errors - /// Returns an error if the file exists but cannot be opened. + /// Returns an error if the file exists but cannot be read. pub fn open_if_exists(path: Arc) -> Result, Error> { - match File::open(&*path) { - Ok(file) => Ok(Some(Self { file, path })), + match fs::read(&*path) { + Ok(content) => Ok(Some(Self { content, path })), Err(e) if e.kind() == std::io::ErrorKind::NotFound => Ok(None), Err(e) => Err(e.into()), } } - /// Get a reference to the file handle. + /// Get the file contents as a byte slice. #[must_use] - pub const fn file(&self) -> &File { - &self.file - } - - /// Get a mutable reference to the file handle. - pub const fn file_mut(&mut self) -> &mut File { - &mut self.file + pub fn content(&self) -> &[u8] { + &self.content } /// Get the file path. @@ -156,17 +154,13 @@ pub fn find_workspace_root( // Check for package.json with workspaces field for npm/yarn workspace let package_json_path: Arc = cwd.join("package.json").into(); - if let Some(mut file_with_path) = FileWithPath::open_if_exists(package_json_path)? { - let package_json: serde_json::Value = - serde_json::from_reader(BufReader::new(file_with_path.file())).map_err(|e| { - Error::SerdeJson { - file_path: Arc::clone(file_with_path.path()), - serde_json_error: e, - } + if let Some(file_with_path) = FileWithPath::open_if_exists(package_json_path)? { + let package_json: serde_json::Value = serde_json::from_slice(file_with_path.content()) + .map_err(|e| Error::SerdeJson { + file_path: Arc::clone(file_with_path.path()), + serde_json_error: e, })?; if package_json.get("workspaces").is_some() { - // Reset the file cursor since we consumed it reading - file_with_path.file_mut().seek(SeekFrom::Start(0))?; let relative_cwd = original_cwd.strip_prefix(cwd)?.expect("cwd must be within the workspace"); return Ok(( @@ -195,3 +189,80 @@ pub fn find_workspace_root( } } } + +#[cfg(test)] +mod tests { + use tempfile::TempDir; + + use super::*; + + /// Regression test for : + /// on Windows, an open handle to `pnpm-workspace.yaml` without + /// `FILE_SHARE_DELETE` blocks pnpm's atomic write-tmp-then-rename. + #[test] + fn find_workspace_root_does_not_lock_pnpm_workspace_yaml() { + let temp_dir = TempDir::new().unwrap(); + let temp_dir_path = AbsolutePath::new(temp_dir.path()).unwrap(); + let ws_yaml = temp_dir_path.join("pnpm-workspace.yaml"); + let ws_yaml_tmp = temp_dir_path.join("pnpm-workspace.yaml.tmp"); + + fs::write(&ws_yaml, b"packages:\n - apps/*\n").unwrap(); + + let (workspace_root, _) = find_workspace_root(temp_dir_path).unwrap(); + + fs::write(&ws_yaml_tmp, b"packages:\n - apps/*\n - packages/*\n").unwrap(); + fs::rename(&ws_yaml_tmp, &ws_yaml) + .expect("rename over pnpm-workspace.yaml must succeed while WorkspaceRoot is alive"); + + drop(workspace_root); + } + + /// Linux-only: `/proc/self/fd` lets us verify no descriptor remains + /// pointing at `pnpm-workspace.yaml` regardless of Rust's default + /// share mode on the platform. + #[cfg(target_os = "linux")] + #[test] + fn find_workspace_root_releases_pnpm_workspace_yaml_fd_on_linux() { + let temp_dir = TempDir::new().unwrap(); + let temp_dir_path = AbsolutePath::new(temp_dir.path()).unwrap(); + let ws_yaml = temp_dir_path.join("pnpm-workspace.yaml"); + fs::write(&ws_yaml, b"packages:\n - apps/*\n").unwrap(); + + let (workspace_root, _) = find_workspace_root(temp_dir_path).unwrap(); + + let ws_yaml_canonical = fs::canonicalize(&ws_yaml).unwrap(); + let mut open_to_target = 0; + for entry in fs::read_dir("/proc/self/fd").unwrap().flatten() { + if let Ok(link) = fs::read_link(entry.path()) + && link == ws_yaml_canonical + { + open_to_target += 1; + } + } + assert_eq!( + open_to_target, 0, + "expected no open file descriptor for pnpm-workspace.yaml, got {open_to_target}", + ); + drop(workspace_root); + } + + #[test] + fn file_with_path_content_matches_file_on_disk() { + let temp_dir = TempDir::new().unwrap(); + let temp_dir_path = AbsolutePath::new(temp_dir.path()).unwrap(); + let path: Arc = temp_dir_path.join("pnpm-workspace.yaml").into(); + fs::write(&*path, b"packages:\n - apps/*\n").unwrap(); + + let file_with_path = FileWithPath::open(Arc::clone(&path)).unwrap(); + assert_eq!(file_with_path.content(), b"packages:\n - apps/*\n"); + assert_eq!(&**file_with_path.path(), &*path); + } + + #[test] + fn file_with_path_open_if_exists_returns_none_when_missing() { + let temp_dir = TempDir::new().unwrap(); + let temp_dir_path = AbsolutePath::new(temp_dir.path()).unwrap(); + let path: Arc = temp_dir_path.join("missing.yaml").into(); + assert!(FileWithPath::open_if_exists(path).unwrap().is_none()); + } +}