From 225c3dea874e9e94e24abfa176c00cf65fa2e80a Mon Sep 17 00:00:00 2001 From: David Sherret Date: Thu, 2 Jan 2025 10:06:12 -0500 Subject: [PATCH] refactor: update some fs_util functions to use sys_traits (#27515) This is in preparation for extracting out these functions from the CLI crate. A side benefit is these functions will now work in Wasm. --- Cargo.lock | 10 +- Cargo.toml | 4 +- cli/npm/managed/resolvers/local.rs | 18 ++- cli/standalone/file_system.rs | 80 +++++++++++--- cli/standalone/virtual_fs.rs | 1 + cli/sys.rs | 70 +++++++----- cli/util/fs.rs | 170 +++++++++++++---------------- 7 files changed, 205 insertions(+), 148 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index bad4a8ea2c..30f4f0d394 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -4519,12 +4519,12 @@ dependencies = [ [[package]] name = "junction" -version = "0.2.0" +version = "1.2.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "be39922b087cecaba4e2d5592dedfc8bda5d4a5a1231f143337cca207950b61d" +checksum = "72bbdfd737a243da3dfc1f99ee8d6e166480f17ab4ac84d7c34aacd73fc7bd16" dependencies = [ "scopeguard", - "winapi", + "windows-sys 0.52.0", ] [[package]] @@ -7680,9 +7680,9 @@ dependencies = [ [[package]] name = "sys_traits" -version = "0.1.4" +version = "0.1.6" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "6683465f4e1d8fd75069cbc36c646258c05b7d8d6676bcb5d71968b99b7d5ae2" +checksum = "b1c12873696bde6de3aea3cd27de8e52897177c5b368a6a30987fd4926e30f85" dependencies = [ "filetime", "getrandom", diff --git a/Cargo.toml b/Cargo.toml index e4b9e0f496..117a1bf6a8 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -193,7 +193,7 @@ slab = "0.4" smallvec = "1.8" socket2 = { version = "0.5.3", features = ["all"] } spki = "0.7.2" -sys_traits = "=0.1.4" +sys_traits = "=0.1.6" tar = "=0.4.40" tempfile = "3.4.0" termcolor = "1.1.3" @@ -240,7 +240,7 @@ syn = { version = "2", features = ["full", "extra-traits"] } nix = "=0.27.1" # windows deps -junction = "=0.2.0" +junction = "=1.2.0" winapi = "=0.3.9" windows-sys = { version = "0.59.0", features = ["Win32_Foundation", "Win32_Media", "Win32_Storage_FileSystem", "Win32_System_IO", "Win32_System_WindowsProgramming", "Wdk", "Wdk_System", "Wdk_System_SystemInformation", "Win32_Security", "Win32_System_Pipes", "Wdk_Storage_FileSystem", "Win32_System_Registry", "Win32_System_Kernel", "Win32_System_Threading", "Win32_UI", "Win32_UI_Shell"] } winres = "=0.1.12" diff --git a/cli/npm/managed/resolvers/local.rs b/cli/npm/managed/resolvers/local.rs index 63f0e4f36c..1a4ec57a69 100644 --- a/cli/npm/managed/resolvers/local.rs +++ b/cli/npm/managed/resolvers/local.rs @@ -433,7 +433,11 @@ async fn sync_resolution_with_fs( deno_core::unsync::spawn_blocking({ let package_path = package_path.clone(); move || { - clone_dir_recursive(&cache_folder, &package_path)?; + clone_dir_recursive( + &crate::sys::CliSys::default(), + &cache_folder, + &package_path, + )?; // write out a file that indicates this folder has been initialized fs::write(initialized_file, tags)?; @@ -490,7 +494,11 @@ async fn sync_resolution_with_fs( &package.id.nv.name, ); - clone_dir_recursive(&source_path, &package_path)?; + clone_dir_recursive( + &crate::sys::CliSys::default(), + &source_path, + &package_path, + )?; // write out a file that indicates this folder has been initialized fs::write(initialized_file, "")?; } @@ -1057,7 +1065,8 @@ fn symlink_package_dir( } #[cfg(not(windows))] { - symlink_dir(&old_path_relative, new_path).map_err(Into::into) + symlink_dir(&crate::sys::CliSys::default(), &old_path_relative, new_path) + .map_err(Into::into) } } @@ -1079,7 +1088,8 @@ fn junction_or_symlink_dir( .context("Failed creating junction in node_modules folder"); } - match symlink_dir(old_path_relative, new_path) { + match symlink_dir(&crate::sys::CliSys::default(), old_path_relative, new_path) + { Ok(()) => Ok(()), Err(symlink_err) if symlink_err.kind() == std::io::ErrorKind::PermissionDenied => diff --git a/cli/standalone/file_system.rs b/cli/standalone/file_system.rs index f218277bef..b04db88c90 100644 --- a/cli/standalone/file_system.rs +++ b/cli/standalone/file_system.rs @@ -23,6 +23,7 @@ use sys_traits::boxed::BoxedFsDirEntry; use sys_traits::boxed::BoxedFsMetadataValue; use sys_traits::boxed::FsMetadataBoxed; use sys_traits::boxed::FsReadDirBoxed; +use sys_traits::FsCopy; use sys_traits::FsMetadata; use super::virtual_fs::FileBackedVfs; @@ -47,24 +48,32 @@ impl DenoCompileFileSystem { } } - fn copy_to_real_path(&self, oldpath: &Path, newpath: &Path) -> FsResult<()> { + fn copy_to_real_path( + &self, + oldpath: &Path, + newpath: &Path, + ) -> std::io::Result { let old_file = self.0.file_entry(oldpath)?; let old_file_bytes = self.0.read_file_all(old_file, VfsFileSubDataKind::Raw)?; - RealFs.write_file_sync( - newpath, - OpenOptions { - read: false, - write: true, - create: true, - truncate: true, - append: false, - create_new: false, - mode: None, - }, - None, - &old_file_bytes, - ) + let len = old_file_bytes.len() as u64; + RealFs + .write_file_sync( + newpath, + OpenOptions { + read: false, + write: true, + create: true, + truncate: true, + append: false, + create_new: false, + mode: None, + }, + None, + &old_file_bytes, + ) + .map_err(|err| err.into_io_error())?; + Ok(len) } } @@ -191,7 +200,10 @@ impl FileSystem for DenoCompileFileSystem { fn copy_file_sync(&self, oldpath: &Path, newpath: &Path) -> FsResult<()> { self.error_if_in_vfs(newpath)?; if self.0.is_path_within(oldpath) { - self.copy_to_real_path(oldpath, newpath) + self + .copy_to_real_path(oldpath, newpath) + .map(|_| ()) + .map_err(FsError::Io) } else { RealFs.copy_file_sync(oldpath, newpath) } @@ -206,6 +218,8 @@ impl FileSystem for DenoCompileFileSystem { let fs = self.clone(); tokio::task::spawn_blocking(move || { fs.copy_to_real_path(&oldpath, &newpath) + .map(|_| ()) + .map_err(FsError::Io) }) .await? } else { @@ -593,6 +607,32 @@ impl sys_traits::BaseFsMetadata for DenoCompileFileSystem { } } +impl sys_traits::BaseFsCopy for DenoCompileFileSystem { + #[inline] + fn base_fs_copy(&self, from: &Path, to: &Path) -> std::io::Result { + self + .error_if_in_vfs(to) + .map_err(|err| err.into_io_error())?; + if self.0.is_path_within(from) { + self.copy_to_real_path(from, to) + } else { + #[allow(clippy::disallowed_types)] // ok because we're implementing the fs + sys_traits::impls::RealSys.fs_copy(from, to) + } + } +} + +impl sys_traits::BaseFsCloneFile for DenoCompileFileSystem { + fn base_fs_clone_file( + &self, + _from: &Path, + _to: &Path, + ) -> std::io::Result<()> { + // will cause a fallback in the code that uses this + Err(not_supported("cloning files")) + } +} + impl sys_traits::BaseFsCreateDir for DenoCompileFileSystem { #[inline] fn base_fs_create_dir( @@ -794,6 +834,14 @@ impl sys_traits::BaseFsOpen for DenoCompileFileSystem { } } +impl sys_traits::BaseFsSymlinkDir for DenoCompileFileSystem { + fn base_fs_symlink_dir(&self, src: &Path, dst: &Path) -> std::io::Result<()> { + self + .symlink_sync(src, dst, Some(FsFileType::Directory)) + .map_err(|err| err.into_io_error()) + } +} + impl sys_traits::SystemRandom for DenoCompileFileSystem { #[inline] fn sys_random(&self, buf: &mut [u8]) -> std::io::Result<()> { diff --git a/cli/standalone/virtual_fs.rs b/cli/standalone/virtual_fs.rs index fab4fad83c..e167b153a7 100644 --- a/cli/standalone/virtual_fs.rs +++ b/cli/standalone/virtual_fs.rs @@ -1685,6 +1685,7 @@ mod test { temp_dir.write("src/a.txt", "data"); temp_dir.write("src/b.txt", "data"); util::fs::symlink_dir( + &crate::sys::CliSys::default(), temp_dir_path.join("src/nested/sub_dir").as_path(), temp_dir_path.join("src/sub_dir_link").as_path(), ) diff --git a/cli/sys.rs b/cli/sys.rs index f23bbc9dc8..718e9981e2 100644 --- a/cli/sys.rs +++ b/cli/sys.rs @@ -7,6 +7,8 @@ // denort or the deno binary. We should extract out denort to a separate binary. use std::borrow::Cow; +use std::path::Path; +use std::path::PathBuf; use sys_traits::boxed::BoxedFsDirEntry; use sys_traits::boxed::BoxedFsFile; @@ -35,12 +37,35 @@ impl Default for CliSys { impl deno_runtime::deno_node::ExtNodeSys for CliSys {} +impl sys_traits::BaseFsCloneFile for CliSys { + fn base_fs_clone_file(&self, src: &Path, dst: &Path) -> std::io::Result<()> { + match self { + Self::Real(sys) => sys.base_fs_clone_file(src, dst), + Self::DenoCompile(sys) => sys.base_fs_clone_file(src, dst), + } + } +} + +impl sys_traits::BaseFsSymlinkDir for CliSys { + fn base_fs_symlink_dir(&self, src: &Path, dst: &Path) -> std::io::Result<()> { + match self { + Self::Real(sys) => sys.base_fs_symlink_dir(src, dst), + Self::DenoCompile(sys) => sys.base_fs_symlink_dir(src, dst), + } + } +} + +impl sys_traits::BaseFsCopy for CliSys { + fn base_fs_copy(&self, src: &Path, dst: &Path) -> std::io::Result { + match self { + Self::Real(sys) => sys.base_fs_copy(src, dst), + Self::DenoCompile(sys) => sys.base_fs_copy(src, dst), + } + } +} + impl sys_traits::BaseFsHardLink for CliSys { - fn base_fs_hard_link( - &self, - src: &std::path::Path, - dst: &std::path::Path, - ) -> std::io::Result<()> { + fn base_fs_hard_link(&self, src: &Path, dst: &Path) -> std::io::Result<()> { match self { Self::Real(sys) => sys.base_fs_hard_link(src, dst), Self::DenoCompile(sys) => sys.base_fs_hard_link(src, dst), @@ -49,10 +74,7 @@ impl sys_traits::BaseFsHardLink for CliSys { } impl sys_traits::BaseFsRead for CliSys { - fn base_fs_read( - &self, - p: &std::path::Path, - ) -> std::io::Result> { + fn base_fs_read(&self, p: &Path) -> std::io::Result> { match self { Self::Real(sys) => sys.base_fs_read(p), Self::DenoCompile(sys) => sys.base_fs_read(p), @@ -65,7 +87,7 @@ impl sys_traits::BaseFsReadDir for CliSys { fn base_fs_read_dir( &self, - p: &std::path::Path, + p: &Path, ) -> std::io::Result< Box> + '_>, > { @@ -77,10 +99,7 @@ impl sys_traits::BaseFsReadDir for CliSys { } impl sys_traits::BaseFsCanonicalize for CliSys { - fn base_fs_canonicalize( - &self, - p: &std::path::Path, - ) -> std::io::Result { + fn base_fs_canonicalize(&self, p: &Path) -> std::io::Result { match self { Self::Real(sys) => sys.base_fs_canonicalize(p), Self::DenoCompile(sys) => sys.base_fs_canonicalize(p), @@ -91,10 +110,7 @@ impl sys_traits::BaseFsCanonicalize for CliSys { impl sys_traits::BaseFsMetadata for CliSys { type Metadata = BoxedFsMetadataValue; - fn base_fs_metadata( - &self, - path: &std::path::Path, - ) -> std::io::Result { + fn base_fs_metadata(&self, path: &Path) -> std::io::Result { match self { Self::Real(sys) => sys.fs_metadata_boxed(path), Self::DenoCompile(sys) => sys.fs_metadata_boxed(path), @@ -103,7 +119,7 @@ impl sys_traits::BaseFsMetadata for CliSys { fn base_fs_symlink_metadata( &self, - path: &std::path::Path, + path: &Path, ) -> std::io::Result { match self { Self::Real(sys) => sys.fs_symlink_metadata_boxed(path), @@ -115,7 +131,7 @@ impl sys_traits::BaseFsMetadata for CliSys { impl sys_traits::BaseFsCreateDir for CliSys { fn base_fs_create_dir( &self, - p: &std::path::Path, + p: &Path, options: &CreateDirOptions, ) -> std::io::Result<()> { match self { @@ -130,7 +146,7 @@ impl sys_traits::BaseFsOpen for CliSys { fn base_fs_open( &self, - path: &std::path::Path, + path: &Path, options: &sys_traits::OpenOptions, ) -> std::io::Result { match self { @@ -141,7 +157,7 @@ impl sys_traits::BaseFsOpen for CliSys { } impl sys_traits::BaseFsRemoveFile for CliSys { - fn base_fs_remove_file(&self, p: &std::path::Path) -> std::io::Result<()> { + fn base_fs_remove_file(&self, p: &Path) -> std::io::Result<()> { match self { Self::Real(sys) => sys.base_fs_remove_file(p), Self::DenoCompile(sys) => sys.base_fs_remove_file(p), @@ -150,11 +166,7 @@ impl sys_traits::BaseFsRemoveFile for CliSys { } impl sys_traits::BaseFsRename for CliSys { - fn base_fs_rename( - &self, - old: &std::path::Path, - new: &std::path::Path, - ) -> std::io::Result<()> { + fn base_fs_rename(&self, old: &Path, new: &Path) -> std::io::Result<()> { match self { Self::Real(sys) => sys.base_fs_rename(old, new), Self::DenoCompile(sys) => sys.base_fs_rename(old, new), @@ -190,7 +202,7 @@ impl sys_traits::ThreadSleep for CliSys { } impl sys_traits::EnvCurrentDir for CliSys { - fn env_current_dir(&self) -> std::io::Result { + fn env_current_dir(&self) -> std::io::Result { match self { Self::Real(sys) => sys.env_current_dir(), Self::DenoCompile(sys) => sys.env_current_dir(), @@ -211,7 +223,7 @@ impl sys_traits::BaseEnvVar for CliSys { } impl sys_traits::EnvHomeDir for CliSys { - fn env_home_dir(&self) -> Option { + fn env_home_dir(&self) -> Option { #[allow(clippy::disallowed_types)] // ok because sys impl sys_traits::impls::RealSys.env_home_dir() } diff --git a/cli/util/fs.rs b/cli/util/fs.rs index 396e5dc146..61f1786ee9 100644 --- a/cli/util/fs.rs +++ b/cli/util/fs.rs @@ -17,6 +17,9 @@ use deno_core::anyhow::Context; use deno_core::error::AnyError; use deno_core::unsync::spawn_blocking; use deno_core::ModuleSpecifier; +use sys_traits::FsCreateDirAll; +use sys_traits::FsDirEntry; +use sys_traits::FsSymlinkDir; use crate::sys::CliSys; use crate::util::progress_bar::ProgressBar; @@ -148,87 +151,74 @@ pub async fn remove_dir_all_if_exists(path: &Path) -> std::io::Result<()> { } } -mod clone_dir_imp { - - #[cfg(target_vendor = "apple")] - mod apple { - use std::os::unix::ffi::OsStrExt; - use std::path::Path; - - use deno_core::error::AnyError; - - use super::super::copy_dir_recursive; - fn clonefile(from: &Path, to: &Path) -> std::io::Result<()> { - let from = std::ffi::CString::new(from.as_os_str().as_bytes())?; - let to = std::ffi::CString::new(to.as_os_str().as_bytes())?; - // SAFETY: `from` and `to` are valid C strings. - let ret = unsafe { libc::clonefile(from.as_ptr(), to.as_ptr(), 0) }; - if ret != 0 { - return Err(std::io::Error::last_os_error()); - } - Ok(()) - } - - pub fn clone_dir_recursive(from: &Path, to: &Path) -> Result<(), AnyError> { - if let Some(parent) = to.parent() { - std::fs::create_dir_all(parent)?; - } - // Try to clone the whole directory - if let Err(err) = clonefile(from, to) { - if err.kind() != std::io::ErrorKind::AlreadyExists { - log::warn!( - "Failed to clone dir {:?} to {:?} via clonefile: {}", - from, - to, - err - ); - } - // clonefile won't overwrite existing files, so if the dir exists - // we need to handle it recursively. - copy_dir_recursive(from, to)?; - } - - Ok(()) - } - } - - #[cfg(target_vendor = "apple")] - pub(super) use apple::clone_dir_recursive; - - #[cfg(not(target_vendor = "apple"))] - pub(super) fn clone_dir_recursive( - from: &std::path::Path, - to: &std::path::Path, - ) -> Result<(), deno_core::error::AnyError> { - use crate::sys::CliSys; - - if let Err(e) = - deno_npm_cache::hard_link_dir_recursive(&CliSys::default(), from, to) - { - log::debug!("Failed to hard link dir {:?} to {:?}: {}", from, to, e); - super::copy_dir_recursive(from, to)?; - } - - Ok(()) - } -} - /// Clones a directory to another directory. The exact method /// is not guaranteed - it may be a hardlink, copy, or other platform-specific /// operation. /// /// Note: Does not handle symlinks. -pub fn clone_dir_recursive(from: &Path, to: &Path) -> Result<(), AnyError> { - clone_dir_imp::clone_dir_recursive(from, to) +pub fn clone_dir_recursive< + TSys: sys_traits::FsCopy + + sys_traits::FsCloneFile + + sys_traits::FsCloneFile + + sys_traits::FsCreateDir + + sys_traits::FsHardLink + + sys_traits::FsReadDir + + sys_traits::FsRemoveFile + + sys_traits::ThreadSleep, +>( + sys: &TSys, + from: &Path, + to: &Path, +) -> Result<(), AnyError> { + if cfg!(target_vendor = "apple") { + if let Some(parent) = to.parent() { + sys.fs_create_dir_all(parent)?; + } + // Try to clone the whole directory + if let Err(err) = sys.fs_clone_file(from, to) { + if !matches!( + err.kind(), + std::io::ErrorKind::AlreadyExists | std::io::ErrorKind::Unsupported + ) { + log::warn!( + "Failed to clone dir {:?} to {:?} via clonefile: {}", + from, + to, + err + ); + } + // clonefile won't overwrite existing files, so if the dir exists + // we need to handle it recursively. + copy_dir_recursive(sys, from, to)?; + } + } else if let Err(e) = deno_npm_cache::hard_link_dir_recursive(sys, from, to) + { + log::debug!("Failed to hard link dir {:?} to {:?}: {}", from, to, e); + copy_dir_recursive(sys, from, to)?; + } + + Ok(()) } /// Copies a directory to another directory. /// /// Note: Does not handle symlinks. -pub fn copy_dir_recursive(from: &Path, to: &Path) -> Result<(), AnyError> { - std::fs::create_dir_all(to) +pub fn copy_dir_recursive< + TSys: sys_traits::FsCopy + + sys_traits::FsCloneFile + + sys_traits::FsCreateDir + + sys_traits::FsHardLink + + sys_traits::FsReadDir, +>( + sys: &TSys, + from: &Path, + to: &Path, +) -> Result<(), AnyError> { + sys + .fs_create_dir_all(to) .with_context(|| format!("Creating {}", to.display()))?; - let read_dir = std::fs::read_dir(from) + let read_dir = sys + .fs_read_dir(from) .with_context(|| format!("Reading {}", from.display()))?; for entry in read_dir { @@ -238,11 +228,11 @@ pub fn copy_dir_recursive(from: &Path, to: &Path) -> Result<(), AnyError> { let new_to = to.join(entry.file_name()); if file_type.is_dir() { - copy_dir_recursive(&new_from, &new_to).with_context(|| { + copy_dir_recursive(sys, &new_from, &new_to).with_context(|| { format!("Dir {} to {}", new_from.display(), new_to.display()) })?; } else if file_type.is_file() { - std::fs::copy(&new_from, &new_to).with_context(|| { + sys.fs_copy(&new_from, &new_to).with_context(|| { format!("Copying {} to {}", new_from.display(), new_to.display()) })?; } @@ -251,7 +241,11 @@ pub fn copy_dir_recursive(from: &Path, to: &Path) -> Result<(), AnyError> { Ok(()) } -pub fn symlink_dir(oldpath: &Path, newpath: &Path) -> Result<(), Error> { +pub fn symlink_dir( + sys: &TSys, + oldpath: &Path, + newpath: &Path, +) -> Result<(), Error> { let err_mapper = |err: Error, kind: Option| { Error::new( kind.unwrap_or_else(|| err.kind()), @@ -263,26 +257,18 @@ pub fn symlink_dir(oldpath: &Path, newpath: &Path) -> Result<(), Error> { ), ) }; - #[cfg(unix)] - { - use std::os::unix::fs::symlink; - symlink(oldpath, newpath).map_err(|e| err_mapper(e, None))?; - } - #[cfg(not(unix))] - { - use std::os::windows::fs::symlink_dir; - symlink_dir(oldpath, newpath).map_err(|err| { - if let Some(code) = err.raw_os_error() { - if code as u32 == winapi::shared::winerror::ERROR_PRIVILEGE_NOT_HELD - || code as u32 == winapi::shared::winerror::ERROR_INVALID_FUNCTION - { - return err_mapper(err, Some(ErrorKind::PermissionDenied)); - } + + sys.fs_symlink_dir(oldpath, newpath).map_err(|err| { + #[cfg(windows)] + if let Some(code) = err.raw_os_error() { + if code as u32 == winapi::shared::winerror::ERROR_PRIVILEGE_NOT_HELD + || code as u32 == winapi::shared::winerror::ERROR_INVALID_FUNCTION + { + return err_mapper(err, Some(ErrorKind::PermissionDenied)); } - err_mapper(err, None) - })?; - } - Ok(()) + } + err_mapper(err, None) + }) } /// Gets the total size (in bytes) of a directory.