From 03a061ed53b5f8e240c71df65c3642a949aa1fcb Mon Sep 17 00:00:00 2001 From: Michael-F-Bryan Date: Mon, 13 Mar 2023 15:07:34 +0800 Subject: [PATCH 01/12] Introduce a FileSystemExt trait --- lib/vfs/src/builder.rs | 2 +- lib/vfs/src/filesystem_ext.rs | 177 ++++++++++++++++++++++++++++++++++ lib/vfs/src/lib.rs | 2 + lib/vfs/src/passthru_fs.rs | 2 +- 4 files changed, 181 insertions(+), 2 deletions(-) create mode 100644 lib/vfs/src/filesystem_ext.rs diff --git a/lib/vfs/src/builder.rs b/lib/vfs/src/builder.rs index a6aa9143d87..3c1351782f5 100644 --- a/lib/vfs/src/builder.rs +++ b/lib/vfs/src/builder.rs @@ -133,7 +133,7 @@ mod test_builder { .unwrap(); assert_eq!(dev_zero.write(b"hello").await.unwrap(), 5); let mut buf = vec![1; 10]; - dev_zero.read(&mut buf[..]).await.unwrap(); + dev_zero.read_exact(&mut buf[..]).await.unwrap(); assert_eq!(buf, vec![0; 10]); assert!(dev_zero.get_special_fd().is_none()); diff --git a/lib/vfs/src/filesystem_ext.rs b/lib/vfs/src/filesystem_ext.rs new file mode 100644 index 00000000000..093bcb5b026 --- /dev/null +++ b/lib/vfs/src/filesystem_ext.rs @@ -0,0 +1,177 @@ +use std::path::Path; + +use tokio::io::{AsyncReadExt, AsyncWriteExt}; + +use crate::{FileSystem, FsError}; + +/// Helper methods for working with [`FileSystem`]s. +#[async_trait::async_trait] +pub trait FileSystemExt { + fn exists(&self, path: impl AsRef) -> bool; + fn is_dir(&self, path: impl AsRef) -> bool; + fn is_file(&self, path: impl AsRef) -> bool; + fn create_dir_all(&self, path: impl AsRef) -> Result<(), FsError>; + async fn write( + &self, + path: impl AsRef + Send, + data: impl AsRef<[u8]> + Send, + ) -> Result<(), FsError>; + async fn read(&self, path: impl AsRef + Send) -> Result, FsError>; + async fn read_to_string(&self, path: impl AsRef + Send) -> Result; + fn touch(&self, path: impl AsRef + Send) -> Result<(), FsError>; +} + +#[async_trait::async_trait] +impl FileSystemExt for F { + fn exists(&self, path: impl AsRef) -> bool { + self.metadata(path.as_ref()).is_ok() + } + + fn is_dir(&self, path: impl AsRef) -> bool { + match self.metadata(path.as_ref()) { + Ok(meta) => meta.is_dir(), + Err(_) => false, + } + } + + fn is_file(&self, path: impl AsRef) -> bool { + match self.metadata(path.as_ref()) { + Ok(meta) => meta.is_file(), + Err(_) => false, + } + } + + fn create_dir_all(&self, path: impl AsRef) -> Result<(), FsError> { + create_dir_all(self, path.as_ref()) + } + + async fn write( + &self, + path: impl AsRef + Send, + data: impl AsRef<[u8]> + Send, + ) -> Result<(), FsError> { + let path = path.as_ref(); + let data = data.as_ref(); + + let mut f = self + .new_open_options() + .create(true) + .write(true) + .truncate(true) + .open(path)?; + + f.write_all(data).await?; + + Ok(()) + } + + async fn read(&self, path: impl AsRef + Send) -> Result, FsError> { + let mut f = self.new_open_options().read(true).open(path.as_ref())?; + let mut buffer = Vec::new(); + f.read_to_end(&mut buffer).await?; + + Ok(buffer) + } + + async fn read_to_string(&self, path: impl AsRef + Send) -> Result { + let mut f = self.new_open_options().read(true).open(path.as_ref())?; + let mut buffer = String::new(); + f.read_to_string(&mut buffer).await?; + + Ok(buffer) + } + + fn touch(&self, path: impl AsRef + Send) -> Result<(), FsError> { + let _ = self + .new_open_options() + .create(true) + .append(true) + .write(true) + .open(path)?; + + Ok(()) + } +} + +fn create_dir_all(fs: &impl FileSystem, path: &Path) -> Result<(), FsError> { + if let Some(parent) = path.parent() { + create_dir_all(fs, parent)?; + } + + if let Ok(metadata) = fs.metadata(path) { + if metadata.is_dir() { + return Ok(()); + } + if metadata.is_file() { + return Err(FsError::BaseNotDirectory); + } + } + + fs.create_dir(path) +} + +#[cfg(test)] +mod tests { + use tokio::io::AsyncReadExt; + + use super::*; + + #[tokio::test] + async fn write() { + let fs = crate::mem_fs::FileSystem::default(); + + fs.write("/file.txt", b"Hello, World!").await.unwrap(); + + let mut contents = String::new(); + fs.new_open_options() + .read(true) + .open("/file.txt") + .unwrap() + .read_to_string(&mut contents) + .await + .unwrap(); + assert_eq!(contents, "Hello, World!"); + } + + #[tokio::test] + async fn read() { + let fs = crate::mem_fs::FileSystem::default(); + fs.new_open_options() + .create(true) + .write(true) + .open("/file.txt") + .unwrap() + .write_all(b"Hello, World!") + .await + .unwrap(); + + let contents = fs.read_to_string("/file.txt").await.unwrap(); + assert_eq!(contents, "Hello, World!"); + + let contents = fs.read("/file.txt").await.unwrap(); + assert_eq!(contents, b"Hello, World!"); + } + + #[tokio::test] + async fn create_dir_all() { + let fs = crate::mem_fs::FileSystem::default(); + fs.write("/file.txt", b"").await.unwrap(); + + assert!(!fs.exists("/really/nested/directory")); + fs.create_dir_all("/really/nested/directory").unwrap(); + assert!(fs.exists("/really/nested/directory")); + + // It's okay to create the same directory multiple times + fs.create_dir_all("/really/nested/directory").unwrap(); + + // You can't create a directory on top of a file + assert_eq!( + fs.create_dir_all("/file.txt").unwrap_err(), + FsError::BaseNotDirectory + ); + assert_eq!( + fs.create_dir_all("/file.txt/invalid/path").unwrap_err(), + FsError::BaseNotDirectory + ); + } +} diff --git a/lib/vfs/src/lib.rs b/lib/vfs/src/lib.rs index 580905e5983..db9fe793e48 100644 --- a/lib/vfs/src/lib.rs +++ b/lib/vfs/src/lib.rs @@ -25,6 +25,7 @@ pub mod tmp_fs; pub mod union_fs; pub mod zero_file; // tty_file -> see wasmer_wasi::tty_file +mod filesystem_ext; pub mod pipe; #[cfg(feature = "static-fs")] pub mod static_fs; @@ -38,6 +39,7 @@ pub use builder::*; pub use combine_file::*; pub use dual_write_file::*; pub use empty_fs::*; +pub use filesystem_ext::FileSystemExt; pub use null_file::*; pub use passthru_fs::*; pub use pipe::*; diff --git a/lib/vfs/src/passthru_fs.rs b/lib/vfs/src/passthru_fs.rs index c75f3265b7c..7164bee7718 100644 --- a/lib/vfs/src/passthru_fs.rs +++ b/lib/vfs/src/passthru_fs.rs @@ -70,7 +70,7 @@ mod test_builder { .create(true) .open("/foo.txt") .unwrap() - .write(b"hello") + .write_all(b"hello") .await .unwrap(); From 82befd52069aefe479e175109209dd288729425e Mon Sep 17 00:00:00 2001 From: Michael-F-Bryan Date: Mon, 13 Mar 2023 17:59:36 +0800 Subject: [PATCH 02/12] Start implementing an overlayed filesystem --- Cargo.lock | 1 + lib/vfs/Cargo.toml | 1 + lib/vfs/src/filesystem_ext.rs | 72 +++++- lib/vfs/src/filesystems.rs | 96 ++++++++ lib/vfs/src/lib.rs | 82 +++++++ lib/vfs/src/overlay_fs.rs | 439 ++++++++++++++++++++++++++++++++++ 6 files changed, 683 insertions(+), 8 deletions(-) create mode 100644 lib/vfs/src/filesystems.rs create mode 100644 lib/vfs/src/overlay_fs.rs diff --git a/Cargo.lock b/Cargo.lock index 66d928d5164..4af84bf85fd 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -5216,6 +5216,7 @@ dependencies = [ "lazy_static", "libc", "pin-project-lite", + "pretty_assertions", "slab", "tempfile", "thiserror", diff --git a/lib/vfs/Cargo.toml b/lib/vfs/Cargo.toml index 03c70a78ead..5b11cabb8d6 100644 --- a/lib/vfs/Cargo.toml +++ b/lib/vfs/Cargo.toml @@ -25,6 +25,7 @@ pin-project-lite = "0.2.9" indexmap = "1.9.2" [dev-dependencies] +pretty_assertions = "1.3.0" tempfile = "3.4.0" tokio = { version = "1", features = [ "io-util", "rt" ], default_features = false } diff --git a/lib/vfs/src/filesystem_ext.rs b/lib/vfs/src/filesystem_ext.rs index 093bcb5b026..0aa28dd3093 100644 --- a/lib/vfs/src/filesystem_ext.rs +++ b/lib/vfs/src/filesystem_ext.rs @@ -1,24 +1,51 @@ -use std::path::Path; +use std::{collections::VecDeque, path::Path}; use tokio::io::{AsyncReadExt, AsyncWriteExt}; -use crate::{FileSystem, FsError}; +use crate::{DirEntry, FileSystem, FsError}; /// Helper methods for working with [`FileSystem`]s. #[async_trait::async_trait] pub trait FileSystemExt { + /// Does this item exists? fn exists(&self, path: impl AsRef) -> bool; + + /// Does this path refer to a directory? fn is_dir(&self, path: impl AsRef) -> bool; + + /// Does this path refer to a file? fn is_file(&self, path: impl AsRef) -> bool; + + /// Make sure a directory (and all its parents) exist. + /// + /// This is analogous to [`std::fs::create_dir_all()`]. fn create_dir_all(&self, path: impl AsRef) -> Result<(), FsError>; + + /// Asynchronously write some bytes to a file. + /// + /// This is analogous to [`std::fs::write()`]. async fn write( &self, path: impl AsRef + Send, data: impl AsRef<[u8]> + Send, ) -> Result<(), FsError>; + + /// Asynchronously read a file's contents into memory. + /// + /// This is analogous to [`std::fs::read()`]. async fn read(&self, path: impl AsRef + Send) -> Result, FsError>; + + /// Asynchronously read a file's contents into memory as a string. + /// + /// This is analogous to [`std::fs::read_to_string()`]. async fn read_to_string(&self, path: impl AsRef + Send) -> Result; + + /// Make sure a file exists, creating an empty file if it doesn't. fn touch(&self, path: impl AsRef + Send) -> Result<(), FsError>; + + /// Recursively iterate over all paths inside a directory, discarding any + /// errors that may occur along the way. + fn walk(&self, path: impl AsRef) -> Box + '_>; } #[async_trait::async_trait] @@ -86,11 +113,31 @@ impl FileSystemExt for F { .new_open_options() .create(true) .append(true) - .write(true) .open(path)?; Ok(()) } + + fn walk(&self, path: impl AsRef) -> Box + '_> { + let path = path.as_ref(); + let mut dirs_to_visit: VecDeque<_> = self + .read_dir(path) + .ok() + .into_iter() + .flatten() + .filter_map(|result| result.ok()) + .collect(); + + Box::new(std::iter::from_fn(move || { + let next = dirs_to_visit.pop_back()?; + + if let Ok(children) = self.read_dir(&next.path) { + dirs_to_visit.extend(children.flatten()); + } + + Some(next) + })) + } } fn create_dir_all(fs: &impl FileSystem, path: &Path) -> Result<(), FsError> { @@ -112,13 +159,13 @@ fn create_dir_all(fs: &impl FileSystem, path: &Path) -> Result<(), FsError> { #[cfg(test)] mod tests { - use tokio::io::AsyncReadExt; - use super::*; + use crate::mem_fs::FileSystem as MemFS; + use tokio::io::AsyncReadExt; #[tokio::test] async fn write() { - let fs = crate::mem_fs::FileSystem::default(); + let fs = MemFS::default(); fs.write("/file.txt", b"Hello, World!").await.unwrap(); @@ -135,7 +182,7 @@ mod tests { #[tokio::test] async fn read() { - let fs = crate::mem_fs::FileSystem::default(); + let fs = MemFS::default(); fs.new_open_options() .create(true) .write(true) @@ -154,7 +201,7 @@ mod tests { #[tokio::test] async fn create_dir_all() { - let fs = crate::mem_fs::FileSystem::default(); + let fs = MemFS::default(); fs.write("/file.txt", b"").await.unwrap(); assert!(!fs.exists("/really/nested/directory")); @@ -174,4 +221,13 @@ mod tests { FsError::BaseNotDirectory ); } + + #[tokio::test] + async fn touch() { + let fs = MemFS::default(); + + fs.touch("/file.txt").unwrap(); + + assert_eq!(fs.read("/file.txt").await.unwrap(), b""); + } } diff --git a/lib/vfs/src/filesystems.rs b/lib/vfs/src/filesystems.rs new file mode 100644 index 00000000000..ee60c82a0e0 --- /dev/null +++ b/lib/vfs/src/filesystems.rs @@ -0,0 +1,96 @@ +use crate::FileSystem; + +/// A chain of one or more [`FileSystem`]s. +// FIXME(Michael-F-Bryan): We could remove this trait's HRTBs and lifetimes if +// we had access to GATs, but our MSRV is currently 1.64 and GATs require 1.65. +pub trait FileSystems<'a>: 'a { + type Iter: IntoIterator + 'a; + + fn iter_filesystems(&'a self) -> Self::Iter; +} + +impl<'a, S> FileSystems<'a> for &'a S +where + S: FileSystems<'a> + 'a, +{ + type Iter = >::Iter; + + fn iter_filesystems(&'a self) -> Self::Iter { + (**self).iter_filesystems() + } +} + +impl<'a, F> FileSystems<'a> for Vec +where + F: FileSystem + 'a, +{ + type Iter = std::iter::Map, fn(&F) -> &dyn FileSystem>; + + fn iter_filesystems(&'a self) -> Self::Iter { + fn downcast(value: &T) -> &dyn FileSystem { + value + } + self.iter().map(downcast) + } +} + +impl<'a, F, const N: usize> FileSystems<'a> for [F; N] +where + F: FileSystem + 'a, +{ + type Iter = [&'a dyn FileSystem; N]; + + fn iter_filesystems(&'a self) -> Self::Iter { + // a poor man's version of the unstable array::each_ref() + let mut i = 0; + [(); N].map(|()| { + let f = &self[i] as &dyn FileSystem; + i += 1; + f + }) + } +} + +impl<'a> FileSystems<'a> for () { + type Iter = std::iter::Empty<&'a dyn FileSystem>; + + fn iter_filesystems(&'a self) -> Self::Iter { + std::iter::empty() + } +} + +macro_rules! count { + ($($t:ident),* $(,)?) => { + 0 $( + count!(@$t) )* + }; + (@$t:ident) => { 1 }; +} + +macro_rules! tuple_filesystems { + ($first:ident $(, $rest:ident)* $(,)?) => { + impl<'a, $first, $( $rest ),*> FileSystems<'a> for ($first, $($rest),*) + where + $first: FileSystem + 'a, + $($rest: FileSystem + 'a),* + { + type Iter = [ &'a dyn FileSystem; { count!($first, $($rest),*) }]; + + fn iter_filesystems(&'a self) -> Self::Iter { + #[allow(non_snake_case)] + let ($first, $($rest),*) = &self; + + [ + $first as &dyn FileSystem, + $( + $rest as &dyn FileSystem, + )* + ] + } + } + + tuple_filesystems!($($rest),*); + }; + () => {}; +} + +tuple_filesystems!(A, B, C, D, E, F, G, H, I, J, K); diff --git a/lib/vfs/src/lib.rs b/lib/vfs/src/lib.rs index db9fe793e48..7a72402fbb1 100644 --- a/lib/vfs/src/lib.rs +++ b/lib/vfs/src/lib.rs @@ -1,7 +1,12 @@ +#[cfg(test)] +#[macro_use] +extern crate pretty_assertions; + use std::any::Any; use std::ffi::OsString; use std::fmt; use std::io; +use std::ops::Deref; use std::path::{Path, PathBuf}; use std::pin::Pin; use std::task::Context; @@ -26,6 +31,8 @@ pub mod union_fs; pub mod zero_file; // tty_file -> see wasmer_wasi::tty_file mod filesystem_ext; +mod filesystems; +pub mod overlay_fs; pub mod pipe; #[cfg(feature = "static-fs")] pub mod static_fs; @@ -40,6 +47,7 @@ pub use combine_file::*; pub use dual_write_file::*; pub use empty_fs::*; pub use filesystem_ext::FileSystemExt; +pub use filesystems::FileSystems; pub use null_file::*; pub use passthru_fs::*; pub use pipe::*; @@ -86,6 +94,40 @@ impl dyn FileSystem + 'static { } } +impl FileSystem for P +where + P: Deref + std::fmt::Debug + Send + Sync + 'static, + F: FileSystem + ?Sized, +{ + fn read_dir(&self, path: &Path) -> Result { + (**self).read_dir(path) + } + + fn create_dir(&self, path: &Path) -> Result<()> { + (**self).create_dir(path) + } + + fn remove_dir(&self, path: &Path) -> Result<()> { + (**self).remove_dir(path) + } + + fn rename(&self, from: &Path, to: &Path) -> Result<()> { + (**self).rename(from, to) + } + + fn metadata(&self, path: &Path) -> Result { + (**self).metadata(path) + } + + fn remove_file(&self, path: &Path) -> Result<()> { + (**self).remove_file(path) + } + + fn new_open_options(&self) -> OpenOptions { + (**self).new_open_options() + } +} + pub trait FileOpener { fn open( &self, @@ -140,6 +182,20 @@ impl OpenOptionsConfig { pub const fn truncate(&self) -> bool { self.truncate } + + /// Would a file opened with this [`OpenOptionsConfig`] change files on the + /// filesystem. + pub const fn would_mutate(&self) -> bool { + let OpenOptionsConfig { + read: _, + write, + create_new, + create, + append, + truncate, + } = *self; + append || write || create || create_new || truncate + } } impl<'a> fmt::Debug for OpenOptions<'a> { @@ -172,36 +228,62 @@ impl<'a> OpenOptions<'a> { self.conf.clone() } + /// Use an existing [`OpenOptionsConfig`] to configure this [`OpenOptions`]. pub fn options(&mut self, options: OpenOptionsConfig) -> &mut Self { self.conf = options; self } + /// Sets the option for read access. + /// + /// This option, when true, will indicate that the file should be + /// `read`-able if opened. pub fn read(&mut self, read: bool) -> &mut Self { self.conf.read = read; self } + /// Sets the option for write access. + /// + /// This option, when true, will indicate that the file should be + /// `write`-able if opened. + /// + /// If the file already exists, any write calls on it will overwrite its + /// contents, without truncating it. pub fn write(&mut self, write: bool) -> &mut Self { self.conf.write = write; self } + /// Sets the option for the append mode. + /// + /// This option, when true, means that writes will append to a file instead + /// of overwriting previous contents. + /// Note that setting `.write(true).append(true)` has the same effect as + /// setting only `.append(true)`. pub fn append(&mut self, append: bool) -> &mut Self { self.conf.append = append; self } + /// Sets the option for truncating a previous file. + /// + /// If a file is successfully opened with this option set it will truncate + /// the file to 0 length if it already exists. + /// + /// The file must be opened with write access for truncate to work. pub fn truncate(&mut self, truncate: bool) -> &mut Self { self.conf.truncate = truncate; self } + /// Sets the option to create a new file, or open it if it already exists. pub fn create(&mut self, create: bool) -> &mut Self { self.conf.create = create; self } + /// Sets the option to create a new file, failing if it already exists. pub fn create_new(&mut self, create_new: bool) -> &mut Self { self.conf.create_new = create_new; self diff --git a/lib/vfs/src/overlay_fs.rs b/lib/vfs/src/overlay_fs.rs new file mode 100644 index 00000000000..1a6050a695a --- /dev/null +++ b/lib/vfs/src/overlay_fs.rs @@ -0,0 +1,439 @@ +use std::{fmt::Debug, path::Path}; + +use crate::{ + FileOpener, FileSystems, FsError, Metadata, OpenOptions, OpenOptionsConfig, ReadDir, + VirtualFile, +}; + +/// A primary read-write filesystem and chain of read-only secondary filesystems +/// that are overlayed on top of each other. +/// +/// # Precedence +/// +/// The general rule is that mutating operations (e.g. +/// [`crate::FileSystem::remove_dir()`] or [`FileOpener::open()`] with +/// [`OpenOptions::write()`] set) will only be executed against the "primary" +/// filesystem. +/// +/// For operations which don't make modifications (e.g. [`FileOpener::open()`] +/// in read-only mode), [`FileSystem`] will first check the primary filesystem, +/// and if that fails it will iterate through the secondary filesystems until +/// either one of them succeeds or there are no more filesystems. +/// +/// Most importantly, this means earlier filesystems can shadow files and +/// directories that have a lower precedence. +/// +///# Examples +/// +/// Something useful to know is that the [`FileSystems`] trait is implemented +/// for both arrays and tuples. +/// +/// For example, if you want to create a [`crate::FileSystem`] which will +/// create files in-memory while still being able to read from the host, you +/// might do something like this: +/// +/// ```rust +/// use wasmer_vfs::{ +/// mem_fs::FileSystem as MemFS, +/// host_fs::FileSystem as HostFS, +/// overlay_fs::FileSystem as OverlayFS, +/// }; +/// let fs = OverlayFS::new(MemFS::default(), [HostFS]); +/// +/// // This also has the benefit of storing the two values in-line with no extra +/// // overhead or indirection. +/// assert_eq!( +/// std::mem::size_of_val(&fs), +/// std::mem::size_of::<(MemFS, HostFS)>(), +/// ); +/// ``` +/// +/// A more complex example is +#[derive(Clone, PartialEq, Eq)] +pub struct FileSystem +where + P: crate::FileSystem, + S: for<'a> FileSystems<'a>, +{ + primary: P, + secondaries: S, +} + +impl FileSystem +where + P: crate::FileSystem, + S: for<'a> FileSystems<'a>, +{ + /// Create a new [`FileSystem`] using a primary [`crate::FileSystem`] and + /// a chain of read-only secondary [`FileSystems`]. + pub fn new(primary: P, secondaries: S) -> Self { + FileSystem { + primary, + secondaries, + } + } + + pub fn primary(&self) -> &P { + &self.primary + } + + pub fn primary_mut(&mut self) -> &mut P { + &mut self.primary + } + + pub fn secondaries(&self) -> &S { + &self.secondaries + } + + pub fn secondaries_mut(&mut self) -> &mut S { + &mut self.secondaries + } + + pub fn into_inner(self) -> (P, S) { + (self.primary, self.secondaries) + } + + /// Iterate over all filesystems in order of precedence. + pub fn iter(&self) -> impl Iterator + '_ { + std::iter::once(self.primary() as &dyn crate::FileSystem) + .chain(self.secondaries().iter_filesystems()) + } +} + +impl crate::FileSystem for FileSystem +where + P: crate::FileSystem, + S: for<'a> crate::FileSystems<'a> + Send + Sync, +{ + fn read_dir(&self, path: &Path) -> Result { + let mut entries = Vec::new(); + let mut had_at_least_one_success = false; + + for fs in self.iter() { + match fs.read_dir(path) { + Ok(r) => { + for entry in r { + entries.push(entry?); + } + had_at_least_one_success = true; + } + Err(e) if should_continue(e) => continue, + Err(e) => return Err(e), + } + } + + if had_at_least_one_success { + // Note: this sort is guaranteed to be stable, so it will prefer + // filesystems "higher up" the chain. + entries.sort_by(|a, b| a.path.cmp(&b.path)); + // Make sure earlier entries shadow layer ones. + entries.dedup_by(|a, b| a.path == b.path); + + Ok(ReadDir::new(entries)) + } else { + Err(FsError::BaseNotDirectory) + } + } + + fn create_dir(&self, _path: &Path) -> Result<(), FsError> { + todo!() + } + + fn remove_dir(&self, path: &Path) -> Result<(), FsError> { + match self.primary.remove_dir(path) { + Ok(_) => return Ok(()), + Err(e) if should_continue(e) => { + // It's not in the primary filesystem, so we'll check the + // secondaries to see whether we need to return a permission + // error or a not found error. + } + Err(other) => return Err(other), + } + + for fs in self.secondaries().iter_filesystems() { + match fs.metadata(path) { + Ok(m) if m.is_dir() => { + return Err(FsError::PermissionDenied); + } + Ok(_) => return Err(FsError::BaseNotDirectory), + Err(e) if should_continue(e) => continue, + Err(e) => return Err(e), + } + } + + Err(FsError::BaseNotDirectory) + } + + fn rename(&self, from: &Path, to: &Path) -> Result<(), FsError> { + match self.primary.rename(from, to) { + Ok(_) => return Ok(()), + Err(e) if should_continue(e) => {} + Err(e) => return Err(e), + } + + for fs in self.secondaries().iter_filesystems() { + if fs.metadata(from).is_ok() { + return Err(FsError::PermissionDenied); + } + } + + Err(FsError::EntryNotFound) + } + + fn metadata(&self, path: &Path) -> Result { + for fs in self.iter() { + match fs.metadata(path) { + Ok(meta) => return Ok(meta), + Err(e) if should_continue(e) => continue, + Err(e) => return Err(e), + } + } + + Err(FsError::EntryNotFound) + } + + fn remove_file(&self, path: &Path) -> Result<(), FsError> { + match self.primary.remove_file(path) { + Ok(_) => return Ok(()), + Err(e) if should_continue(e) => {} + Err(e) => return Err(e), + } + + for fs in self.secondaries.iter_filesystems() { + match fs.metadata(path) { + Ok(meta) if meta.is_file() => { + return Err(FsError::PermissionDenied); + } + Ok(_) => return Err(FsError::NotAFile), + Err(FsError::EntryNotFound) => {} + Err(e) => return Err(e), + } + } + + Err(FsError::EntryNotFound) + } + + fn new_open_options(&self) -> OpenOptions<'_> { + OpenOptions::new(self) + } +} + +impl FileOpener for FileSystem +where + P: crate::FileSystem, + S: for<'a> FileSystems<'a>, +{ + fn open( + &self, + path: &Path, + conf: &OpenOptionsConfig, + ) -> Result, FsError> { + // First, try the primary filesystem + match self + .primary + .new_open_options() + .options(conf.clone()) + .open(path) + { + Ok(f) => return Ok(f), + Err(e) if should_continue(e) => {} + Err(e) => return Err(e), + }; + + if conf.would_mutate() { + return Err(FsError::PermissionDenied); + } + + for fs in self.secondaries.iter_filesystems() { + match fs.new_open_options().options(conf.clone()).open(path) { + Ok(f) => return Ok(f), + Err(e) if should_continue(e) => continue, + Err(e) => return Err(e), + } + } + + Err(FsError::EntryNotFound) + } +} + +impl Debug for FileSystem +where + P: crate::FileSystem, + S: for<'a> FileSystems<'a>, +{ + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + struct IterFilesystems<'a, S>(&'a S); + impl<'a, S> Debug for IterFilesystems<'a, S> + where + S: for<'b> FileSystems<'b>, + { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + let mut f = f.debug_list(); + + for fs in self.0.iter_filesystems() { + f.entry(&fs); + } + + f.finish() + } + } + + f.debug_struct("FileSystem") + .field("primary", &self.primary) + .field("secondaries", &IterFilesystems(&self.secondaries)) + .finish() + } +} + +/// Is it okay to use a fallback filesystem to deal with this particular error? +fn should_continue(e: FsError) -> bool { + matches!(e, FsError::EntryNotFound) +} + +#[cfg(test)] +mod tests { + use std::{path::PathBuf, sync::Arc}; + + use tokio::io::AsyncWriteExt; + + use super::*; + use crate::{mem_fs::FileSystem as MemFS, FileSystem as _, FileSystemExt}; + + #[test] + fn can_be_used_as_an_object() { + fn _box_with_memfs( + fs: FileSystem>, + ) -> Box { + Box::new(fs) + } + + fn _arc( + fs: FileSystem, Vec>>, + ) -> Arc { + Arc::new(fs) + } + } + + #[tokio::test] + async fn remove_directory() { + let primary = MemFS::default(); + let secondary = MemFS::default(); + let first = Path::new("/first"); + let second = Path::new("/second"); + let file_txt = second.join("file.txt"); + let third = Path::new("/third"); + primary.create_dir(first).unwrap(); + primary.create_dir(second).unwrap(); + primary + .new_open_options() + .create(true) + .write(true) + .open(&file_txt) + .unwrap() + .write_all(b"Hello, World!") + .await + .unwrap(); + secondary.create_dir(third).unwrap(); + + let overlay = FileSystem::new(primary, [secondary]); + + // Delete a folder on the primary filesystem + overlay.remove_dir(first).unwrap(); + assert_eq!( + overlay.primary().metadata(first).unwrap_err(), + FsError::EntryNotFound, + "Deleted from primary" + ); + assert!(!overlay.secondaries[0].exists(&second)); + + // Directory on the primary fs isn't empty + assert_eq!( + overlay.remove_dir(second).unwrap_err(), + FsError::DirectoryNotEmpty, + ); + + // Try to remove something on one of the overlay filesystems + assert_eq!( + overlay.remove_dir(third).unwrap_err(), + FsError::PermissionDenied, + ); + assert!(overlay.secondaries[0].exists(third)); + } + + #[tokio::test] + async fn open_files() { + let primary = MemFS::default(); + let secondary = MemFS::default(); + primary.create_dir_all("/primary").unwrap(); + primary.touch("/primary/read.txt").unwrap(); + primary.touch("/primary/write.txt").unwrap(); + secondary.create_dir_all("/secondary").unwrap(); + secondary.touch("/secondary/read.txt").unwrap(); + secondary.touch("/secondary/write.txt").unwrap(); + secondary.create_dir_all("/primary").unwrap(); + secondary + .write("/primary/read.txt", "This is shadowed") + .await + .unwrap(); + + let fs = FileSystem::new(primary, [secondary]); + + // Any new files will be created on the primary fs + let _ = fs + .new_open_options() + .create(true) + .write(true) + .open("/new.txt") + .unwrap(); + assert!(fs.primary.exists("/new.txt")); + assert!(!fs.secondaries[0].exists("/new.txt")); + + // You can open a file for reading and writing on the primary fs + let _ = fs + .new_open_options() + .create(false) + .write(true) + .read(true) + .open("/primary/write.txt") + .unwrap(); + + // Files on the primary should always shadow the secondary + let content = fs.read_to_string("/primary/read.txt").await.unwrap(); + assert_ne!(content, "This is shadowed"); + } + + #[tokio::test] + async fn listed_files_appear_overlayed() { + let primary = MemFS::default(); + let secondary = MemFS::default(); + let secondary_overlayed = MemFS::default(); + primary.create_dir_all("/primary").unwrap(); + primary.touch("/primary/read.txt").unwrap(); + primary.touch("/primary/write.txt").unwrap(); + secondary.create_dir_all("/secondary").unwrap(); + secondary.touch("/secondary/read.txt").unwrap(); + secondary.touch("/secondary/write.txt").unwrap(); + // This second "secondary" filesystem should share the same folders as + // the first one. + secondary_overlayed.create_dir_all("/secondary").unwrap(); + secondary_overlayed + .touch("/secondary/overlayed.txt") + .unwrap(); + + let fs = FileSystem::new(primary, [secondary, secondary_overlayed]); + + let paths: Vec<_> = fs.walk("/").map(|entry| entry.path()).collect(); + assert_eq!( + paths, + vec![ + PathBuf::from("/secondary"), + PathBuf::from("/secondary/write.txt"), + PathBuf::from("/secondary/read.txt"), + PathBuf::from("/secondary/overlayed.txt"), + PathBuf::from("/primary"), + PathBuf::from("/primary/write.txt"), + PathBuf::from("/primary/read.txt"), + ] + ); + } +} From c83bf92ab797716d92025c8fa4c256bab16a000c Mon Sep 17 00:00:00 2001 From: Michael-F-Bryan Date: Mon, 13 Mar 2023 19:20:37 +0800 Subject: [PATCH 03/12] Reworked the file opening logic --- lib/vfs/src/overlay_fs.rs | 196 +++++++++++++++----------------------- 1 file changed, 76 insertions(+), 120 deletions(-) diff --git a/lib/vfs/src/overlay_fs.rs b/lib/vfs/src/overlay_fs.rs index 1a6050a695a..989d788896d 100644 --- a/lib/vfs/src/overlay_fs.rs +++ b/lib/vfs/src/overlay_fs.rs @@ -1,24 +1,17 @@ use std::{fmt::Debug, path::Path}; use crate::{ - FileOpener, FileSystems, FsError, Metadata, OpenOptions, OpenOptionsConfig, ReadDir, - VirtualFile, + FileOpener, FileSystem, FileSystemExt, FileSystems, FsError, Metadata, OpenOptions, + OpenOptionsConfig, ReadDir, VirtualFile, }; -/// A primary read-write filesystem and chain of read-only secondary filesystems -/// that are overlayed on top of each other. +/// A primary filesystem and chain of secondary filesystems that are overlayed +/// on top of each other. /// /// # Precedence /// -/// The general rule is that mutating operations (e.g. -/// [`crate::FileSystem::remove_dir()`] or [`FileOpener::open()`] with -/// [`OpenOptions::write()`] set) will only be executed against the "primary" -/// filesystem. +/// The [`OverlayFileSystem`] will execute operations based on precedence. /// -/// For operations which don't make modifications (e.g. [`FileOpener::open()`] -/// in read-only mode), [`FileSystem`] will first check the primary filesystem, -/// and if that fails it will iterate through the secondary filesystems until -/// either one of them succeeds or there are no more filesystems. /// /// Most importantly, this means earlier filesystems can shadow files and /// directories that have a lower precedence. @@ -36,7 +29,7 @@ use crate::{ /// use wasmer_vfs::{ /// mem_fs::FileSystem as MemFS, /// host_fs::FileSystem as HostFS, -/// overlay_fs::FileSystem as OverlayFS, +/// overlay_fs::FileSystem, /// }; /// let fs = OverlayFS::new(MemFS::default(), [HostFS]); /// @@ -50,24 +43,24 @@ use crate::{ /// /// A more complex example is #[derive(Clone, PartialEq, Eq)] -pub struct FileSystem +pub struct OverlayFileSystem where - P: crate::FileSystem, + P: FileSystem, S: for<'a> FileSystems<'a>, { primary: P, secondaries: S, } -impl FileSystem +impl OverlayFileSystem where - P: crate::FileSystem, + P: FileSystem, S: for<'a> FileSystems<'a>, { - /// Create a new [`FileSystem`] using a primary [`crate::FileSystem`] and - /// a chain of read-only secondary [`FileSystems`]. + /// Create a new [`FileSystem`] using a primary [`crate::FileSystem`] and a + /// chain of secondary [`FileSystems`]. pub fn new(primary: P, secondaries: S) -> Self { - FileSystem { + OverlayFileSystem { primary, secondaries, } @@ -94,15 +87,35 @@ where } /// Iterate over all filesystems in order of precedence. - pub fn iter(&self) -> impl Iterator + '_ { - std::iter::once(self.primary() as &dyn crate::FileSystem) + pub fn iter(&self) -> impl Iterator + '_ { + std::iter::once(self.primary() as &dyn FileSystem) .chain(self.secondaries().iter_filesystems()) } + + /// Try to apply an operation to each [`FileSystem`] in order of precedence. + /// + /// This uses [`should_continue()`] to determine whether an error is fatal + /// and needs to be returned to the caller, or whether we should try the + /// next [`FileSystem`] in the chain. + fn for_each(&self, mut func: F) -> Result + where + F: FnMut(&dyn FileSystem) -> Result, + { + for fs in self.iter() { + match func(fs) { + Ok(result) => return Ok(result), + Err(e) if should_continue(e) => continue, + Err(other) => return Err(other), + } + } + + Err(FsError::EntryNotFound) + } } -impl crate::FileSystem for FileSystem +impl FileSystem for OverlayFileSystem where - P: crate::FileSystem, + P: FileSystem, S: for<'a> crate::FileSystems<'a> + Send + Sync, { fn read_dir(&self, path: &Path) -> Result { @@ -135,82 +148,24 @@ where } } - fn create_dir(&self, _path: &Path) -> Result<(), FsError> { - todo!() + fn create_dir(&self, path: &Path) -> Result<(), FsError> { + self.for_each(|fs| fs.create_dir(path)) } fn remove_dir(&self, path: &Path) -> Result<(), FsError> { - match self.primary.remove_dir(path) { - Ok(_) => return Ok(()), - Err(e) if should_continue(e) => { - // It's not in the primary filesystem, so we'll check the - // secondaries to see whether we need to return a permission - // error or a not found error. - } - Err(other) => return Err(other), - } - - for fs in self.secondaries().iter_filesystems() { - match fs.metadata(path) { - Ok(m) if m.is_dir() => { - return Err(FsError::PermissionDenied); - } - Ok(_) => return Err(FsError::BaseNotDirectory), - Err(e) if should_continue(e) => continue, - Err(e) => return Err(e), - } - } - - Err(FsError::BaseNotDirectory) + self.for_each(|fs| fs.remove_dir(path)) } fn rename(&self, from: &Path, to: &Path) -> Result<(), FsError> { - match self.primary.rename(from, to) { - Ok(_) => return Ok(()), - Err(e) if should_continue(e) => {} - Err(e) => return Err(e), - } - - for fs in self.secondaries().iter_filesystems() { - if fs.metadata(from).is_ok() { - return Err(FsError::PermissionDenied); - } - } - - Err(FsError::EntryNotFound) + self.for_each(|fs| fs.rename(from, to)) } fn metadata(&self, path: &Path) -> Result { - for fs in self.iter() { - match fs.metadata(path) { - Ok(meta) => return Ok(meta), - Err(e) if should_continue(e) => continue, - Err(e) => return Err(e), - } - } - - Err(FsError::EntryNotFound) + self.for_each(|fs| fs.metadata(path)) } fn remove_file(&self, path: &Path) -> Result<(), FsError> { - match self.primary.remove_file(path) { - Ok(_) => return Ok(()), - Err(e) if should_continue(e) => {} - Err(e) => return Err(e), - } - - for fs in self.secondaries.iter_filesystems() { - match fs.metadata(path) { - Ok(meta) if meta.is_file() => { - return Err(FsError::PermissionDenied); - } - Ok(_) => return Err(FsError::NotAFile), - Err(FsError::EntryNotFound) => {} - Err(e) => return Err(e), - } - } - - Err(FsError::EntryNotFound) + self.for_each(|fs| fs.remove_file(path)) } fn new_open_options(&self) -> OpenOptions<'_> { @@ -218,47 +173,48 @@ where } } -impl FileOpener for FileSystem +impl FileOpener for OverlayFileSystem where - P: crate::FileSystem, - S: for<'a> FileSystems<'a>, + P: FileSystem, + S: for<'a> FileSystems<'a> + Send + Sync, { fn open( &self, path: &Path, conf: &OpenOptionsConfig, ) -> Result, FsError> { - // First, try the primary filesystem - match self - .primary - .new_open_options() - .options(conf.clone()) - .open(path) - { - Ok(f) => return Ok(f), - Err(e) if should_continue(e) => {} - Err(e) => return Err(e), - }; - - if conf.would_mutate() { - return Err(FsError::PermissionDenied); - } - - for fs in self.secondaries.iter_filesystems() { - match fs.new_open_options().options(conf.clone()).open(path) { - Ok(f) => return Ok(f), - Err(e) if should_continue(e) => continue, - Err(e) => return Err(e), + // FIXME: There is probably a smarter way to do this without the extra + // FileSystem::metadata() calls or the risk of TOCTOU issues. + + if (conf.create || conf.create_new) && !self.exists(path) { + if let Some(parent) = path.parent() { + // As a special case, we want to direct all newly created files + // to the primary filesystem so it just *looks* like they are + // created alongside secondary filesystems. + let would_normally_be_created_on_a_secondary_fs = self + .secondaries + .iter_filesystems() + .into_iter() + .any(|fs| fs.exists(parent)); + + if would_normally_be_created_on_a_secondary_fs { + self.primary.create_dir_all(parent)?; + return self + .primary + .new_open_options() + .options(conf.clone()) + .open(path); + } } } - Err(FsError::EntryNotFound) + self.for_each(|fs| fs.new_open_options().options(conf.clone()).open(path)) } } -impl Debug for FileSystem +impl Debug for OverlayFileSystem where - P: crate::FileSystem, + P: FileSystem, S: for<'a> FileSystems<'a>, { fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { @@ -302,13 +258,13 @@ mod tests { #[test] fn can_be_used_as_an_object() { fn _box_with_memfs( - fs: FileSystem>, + fs: OverlayFileSystem>, ) -> Box { Box::new(fs) } fn _arc( - fs: FileSystem, Vec>>, + fs: OverlayFileSystem, Vec>>, ) -> Arc { Arc::new(fs) } @@ -335,7 +291,7 @@ mod tests { .unwrap(); secondary.create_dir(third).unwrap(); - let overlay = FileSystem::new(primary, [secondary]); + let overlay = OverlayFileSystem::new(primary, [secondary]); // Delete a folder on the primary filesystem overlay.remove_dir(first).unwrap(); @@ -376,7 +332,7 @@ mod tests { .await .unwrap(); - let fs = FileSystem::new(primary, [secondary]); + let fs = OverlayFileSystem::new(primary, [secondary]); // Any new files will be created on the primary fs let _ = fs @@ -420,7 +376,7 @@ mod tests { .touch("/secondary/overlayed.txt") .unwrap(); - let fs = FileSystem::new(primary, [secondary, secondary_overlayed]); + let fs = OverlayFileSystem::new(primary, [secondary, secondary_overlayed]); let paths: Vec<_> = fs.walk("/").map(|entry| entry.path()).collect(); assert_eq!( From 3109e467ee86b80fc568b513966ba7ac4b1a5b73 Mon Sep 17 00:00:00 2001 From: Michael-F-Bryan Date: Tue, 14 Mar 2023 21:16:58 +0800 Subject: [PATCH 04/12] Reworking the read-only/read-write logic --- lib/vfs/src/host_fs.rs | 2 - lib/vfs/src/overlay_fs.rs | 111 +++++++++++++++++++++++++------------- 2 files changed, 74 insertions(+), 39 deletions(-) diff --git a/lib/vfs/src/host_fs.rs b/lib/vfs/src/host_fs.rs index 253f191b9e3..ef4f3fc4633 100644 --- a/lib/vfs/src/host_fs.rs +++ b/lib/vfs/src/host_fs.rs @@ -1365,7 +1365,5 @@ mod tests { .join("hello.txt")), "canonicalizing a crazily stupid path name", ); - - let _ = fs_extra::remove_items(&["./test_canonicalize"]); } } diff --git a/lib/vfs/src/overlay_fs.rs b/lib/vfs/src/overlay_fs.rs index 989d788896d..d27a99b4f95 100644 --- a/lib/vfs/src/overlay_fs.rs +++ b/lib/vfs/src/overlay_fs.rs @@ -43,11 +43,7 @@ use crate::{ /// /// A more complex example is #[derive(Clone, PartialEq, Eq)] -pub struct OverlayFileSystem -where - P: FileSystem, - S: for<'a> FileSystems<'a>, -{ +pub struct OverlayFileSystem { primary: P, secondaries: S, } @@ -136,10 +132,10 @@ where } if had_at_least_one_success { - // Note: this sort is guaranteed to be stable, so it will prefer - // filesystems "higher up" the chain. + // Note: this sort is guaranteed to be stable, so filesystems + // "higher up" the chain will be further towards the start. entries.sort_by(|a, b| a.path.cmp(&b.path)); - // Make sure earlier entries shadow layer ones. + // Make sure later entries are removed in favour of earlier ones. entries.dedup_by(|a, b| a.path == b.path); Ok(ReadDir::new(entries)) @@ -149,15 +145,51 @@ where } fn create_dir(&self, path: &Path) -> Result<(), FsError> { - self.for_each(|fs| fs.create_dir(path)) + match self.primary.create_dir(path) { + Ok(()) => return Ok(()), + Err(e) if should_continue(e) => {} + Err(e) => return Err(e), + } + + for fs in self.secondaries.iter_filesystems() { + if fs.is_dir(path) { + return Err(FsError::PermissionDenied); + } + } + + Err(FsError::EntryNotFound) } fn remove_dir(&self, path: &Path) -> Result<(), FsError> { - self.for_each(|fs| fs.remove_dir(path)) + match self.primary.remove_dir(path) { + Ok(()) => return Ok(()), + Err(e) if should_continue(e) => {} + Err(e) => return Err(e), + } + + for fs in self.secondaries.iter_filesystems() { + if fs.is_dir(path) { + return Err(FsError::PermissionDenied); + } + } + + Err(FsError::EntryNotFound) } fn rename(&self, from: &Path, to: &Path) -> Result<(), FsError> { - self.for_each(|fs| fs.rename(from, to)) + match self.primary.rename(from, to) { + Ok(()) => return Ok(()), + Err(e) if should_continue(e) => {} + Err(e) => return Err(e), + } + + for fs in self.secondaries.iter_filesystems() { + if fs.exists(from) { + return Err(FsError::PermissionDenied); + } + } + + Err(FsError::EntryNotFound) } fn metadata(&self, path: &Path) -> Result { @@ -165,7 +197,19 @@ where } fn remove_file(&self, path: &Path) -> Result<(), FsError> { - self.for_each(|fs| fs.remove_file(path)) + match self.primary.remove_file(path) { + Ok(()) => return Ok(()), + Err(e) if should_continue(e) => {} + Err(e) => return Err(e), + } + + for fs in self.secondaries.iter_filesystems() { + if fs.exists(path) { + return Err(FsError::PermissionDenied); + } + } + + Err(FsError::EntryNotFound) } fn new_open_options(&self) -> OpenOptions<'_> { @@ -183,31 +227,9 @@ where path: &Path, conf: &OpenOptionsConfig, ) -> Result, FsError> { - // FIXME: There is probably a smarter way to do this without the extra - // FileSystem::metadata() calls or the risk of TOCTOU issues. - - if (conf.create || conf.create_new) && !self.exists(path) { - if let Some(parent) = path.parent() { - // As a special case, we want to direct all newly created files - // to the primary filesystem so it just *looks* like they are - // created alongside secondary filesystems. - let would_normally_be_created_on_a_secondary_fs = self - .secondaries - .iter_filesystems() - .into_iter() - .any(|fs| fs.exists(parent)); - - if would_normally_be_created_on_a_secondary_fs { - self.primary.create_dir_all(parent)?; - return self - .primary - .new_open_options() - .options(conf.clone()) - .open(path); - } - } - } - + // TODO: Re-work this method so that trying to create a file inside a + // secondary filesystem will actually create the file on the primary + // filesystem, running create_dir_all() if necessary. self.for_each(|fs| fs.new_open_options().options(conf.clone()).open(path)) } } @@ -358,6 +380,21 @@ mod tests { assert_ne!(content, "This is shadowed"); } + #[test] + fn create_file_that_looks_like_it_is_in_a_secondary_filesystem_folder() { + let primary = MemFS::default(); + let secondary = MemFS::default(); + secondary.create_dir_all("/path/to/").unwrap(); + assert!(!primary.is_dir("/path/to/")); + let fs = OverlayFileSystem::new(primary, [secondary]); + + fs.touch("/path/to/file.txt").unwrap(); + + assert!(fs.primary.is_dir("/path/to/")); + assert!(fs.primary.is_file("/path/to/file.txt")); + assert!(!fs.secondaries[0].is_file("/path/to/file.txt")); + } + #[tokio::test] async fn listed_files_appear_overlayed() { let primary = MemFS::default(); From 20c352a5734012e27813fb3b6852e0999ba0a32f Mon Sep 17 00:00:00 2001 From: Michael-F-Bryan Date: Tue, 14 Mar 2023 23:57:21 +0800 Subject: [PATCH 05/12] Fleshed out the WASI runner use case --- lib/vfs/src/filesystem_ext.rs | 9 +- lib/vfs/src/filesystems.rs | 124 ++++++++------ lib/vfs/src/lib.rs | 35 ---- lib/vfs/src/overlay_fs.rs | 309 +++++++++++++++++++++++++--------- 4 files changed, 301 insertions(+), 176 deletions(-) diff --git a/lib/vfs/src/filesystem_ext.rs b/lib/vfs/src/filesystem_ext.rs index 0aa28dd3093..e416630a1ba 100644 --- a/lib/vfs/src/filesystem_ext.rs +++ b/lib/vfs/src/filesystem_ext.rs @@ -49,7 +49,7 @@ pub trait FileSystemExt { } #[async_trait::async_trait] -impl FileSystemExt for F { +impl FileSystemExt for F { fn exists(&self, path: impl AsRef) -> bool { self.metadata(path.as_ref()).is_ok() } @@ -112,7 +112,7 @@ impl FileSystemExt for F { let _ = self .new_open_options() .create(true) - .append(true) + .write(true) .open(path)?; Ok(()) @@ -140,7 +140,10 @@ impl FileSystemExt for F { } } -fn create_dir_all(fs: &impl FileSystem, path: &Path) -> Result<(), FsError> { +fn create_dir_all(fs: &F, path: &Path) -> Result<(), FsError> +where + F: FileSystem + ?Sized, +{ if let Some(parent) = path.parent() { create_dir_all(fs, parent)?; } diff --git a/lib/vfs/src/filesystems.rs b/lib/vfs/src/filesystems.rs index ee60c82a0e0..d6e37ed2233 100644 --- a/lib/vfs/src/filesystems.rs +++ b/lib/vfs/src/filesystems.rs @@ -1,90 +1,104 @@ use crate::FileSystem; +use std::ops::ControlFlow; /// A chain of one or more [`FileSystem`]s. -// FIXME(Michael-F-Bryan): We could remove this trait's HRTBs and lifetimes if -// we had access to GATs, but our MSRV is currently 1.64 and GATs require 1.65. -pub trait FileSystems<'a>: 'a { - type Iter: IntoIterator + 'a; - - fn iter_filesystems(&'a self) -> Self::Iter; +pub trait FileSystems { + // FIXME(Michael-F-Bryan): Rewrite this to use GATs and an external iterator + // when we bump the MSRV to 1.65 or higher. + fn for_each_filesystems(&self, func: F) -> Option + where + F: FnMut(&dyn FileSystem) -> ControlFlow; } -impl<'a, S> FileSystems<'a> for &'a S +impl<'b, S> FileSystems for &'b S where - S: FileSystems<'a> + 'a, + S: FileSystems + 'b, { - type Iter = >::Iter; - - fn iter_filesystems(&'a self) -> Self::Iter { - (**self).iter_filesystems() + fn for_each_filesystems(&self, func: F) -> Option + where + F: FnMut(&dyn FileSystem) -> ControlFlow, + { + (**self).for_each_filesystems(func) } } -impl<'a, F> FileSystems<'a> for Vec +impl FileSystems for Vec where - F: FileSystem + 'a, + T: FileSystem, { - type Iter = std::iter::Map, fn(&F) -> &dyn FileSystem>; - - fn iter_filesystems(&'a self) -> Self::Iter { - fn downcast(value: &T) -> &dyn FileSystem { - value - } - self.iter().map(downcast) + fn for_each_filesystems(&self, func: F) -> Option + where + F: FnMut(&dyn FileSystem) -> ControlFlow, + { + self[..].for_each_filesystems(func) } } -impl<'a, F, const N: usize> FileSystems<'a> for [F; N] +impl FileSystems for [T; N] where - F: FileSystem + 'a, + T: FileSystem, { - type Iter = [&'a dyn FileSystem; N]; - - fn iter_filesystems(&'a self) -> Self::Iter { - // a poor man's version of the unstable array::each_ref() - let mut i = 0; - [(); N].map(|()| { - let f = &self[i] as &dyn FileSystem; - i += 1; - f - }) + fn for_each_filesystems(&self, func: F) -> Option + where + F: FnMut(&dyn FileSystem) -> ControlFlow, + { + self[..].for_each_filesystems(func) } } -impl<'a> FileSystems<'a> for () { - type Iter = std::iter::Empty<&'a dyn FileSystem>; +impl FileSystems for [T] +where + T: FileSystem, +{ + fn for_each_filesystems(&self, mut func: F) -> Option + where + F: FnMut(&dyn FileSystem) -> ControlFlow, + { + for fs in self.iter() { + match func(fs) { + ControlFlow::Continue(_) => continue, + ControlFlow::Break(result) => return Some(result), + } + } - fn iter_filesystems(&'a self) -> Self::Iter { - std::iter::empty() + None } } -macro_rules! count { - ($($t:ident),* $(,)?) => { - 0 $( + count!(@$t) )* - }; - (@$t:ident) => { 1 }; +impl FileSystems for () { + fn for_each_filesystems(&self, _func: F) -> Option + where + F: FnMut(&dyn FileSystem) -> ControlFlow, + { + None + } } macro_rules! tuple_filesystems { ($first:ident $(, $rest:ident)* $(,)?) => { - impl<'a, $first, $( $rest ),*> FileSystems<'a> for ($first, $($rest),*) + impl<$first, $( $rest ),*> FileSystems for ($first, $($rest),*) where - $first: FileSystem + 'a, - $($rest: FileSystem + 'a),* + $first: FileSystem, + $($rest: FileSystem),* { - type Iter = [ &'a dyn FileSystem; { count!($first, $($rest),*) }]; - - fn iter_filesystems(&'a self) -> Self::Iter { + fn for_each_filesystems(&self, mut func: F) -> Option + where + F: FnMut(&dyn FileSystem) -> ControlFlow, + { #[allow(non_snake_case)] let ($first, $($rest),*) = &self; - [ - $first as &dyn FileSystem, - $( - $rest as &dyn FileSystem, - )* - ] + if let ControlFlow::Break(result) = func($first) { + return Some(result); + } + + $( + if let ControlFlow::Break(result) = func($rest) { + return Some(result); + } + )* + + None } } @@ -93,4 +107,4 @@ macro_rules! tuple_filesystems { () => {}; } -tuple_filesystems!(A, B, C, D, E, F, G, H, I, J, K); +tuple_filesystems!(F1, F2, F3, F4, F5, F6, F7, F8, F9, F10, F11, F12, F13, F14, F15, F16,); diff --git a/lib/vfs/src/lib.rs b/lib/vfs/src/lib.rs index 7a72402fbb1..fa6cbfb6bf0 100644 --- a/lib/vfs/src/lib.rs +++ b/lib/vfs/src/lib.rs @@ -6,7 +6,6 @@ use std::any::Any; use std::ffi::OsString; use std::fmt; use std::io; -use std::ops::Deref; use std::path::{Path, PathBuf}; use std::pin::Pin; use std::task::Context; @@ -94,40 +93,6 @@ impl dyn FileSystem + 'static { } } -impl FileSystem for P -where - P: Deref + std::fmt::Debug + Send + Sync + 'static, - F: FileSystem + ?Sized, -{ - fn read_dir(&self, path: &Path) -> Result { - (**self).read_dir(path) - } - - fn create_dir(&self, path: &Path) -> Result<()> { - (**self).create_dir(path) - } - - fn remove_dir(&self, path: &Path) -> Result<()> { - (**self).remove_dir(path) - } - - fn rename(&self, from: &Path, to: &Path) -> Result<()> { - (**self).rename(from, to) - } - - fn metadata(&self, path: &Path) -> Result { - (**self).metadata(path) - } - - fn remove_file(&self, path: &Path) -> Result<()> { - (**self).remove_file(path) - } - - fn new_open_options(&self) -> OpenOptions { - (**self).new_open_options() - } -} - pub trait FileOpener { fn open( &self, diff --git a/lib/vfs/src/overlay_fs.rs b/lib/vfs/src/overlay_fs.rs index d27a99b4f95..c3d8a7c8203 100644 --- a/lib/vfs/src/overlay_fs.rs +++ b/lib/vfs/src/overlay_fs.rs @@ -1,4 +1,4 @@ -use std::{fmt::Debug, path::Path}; +use std::{fmt::Debug, ops::ControlFlow, path::Path}; use crate::{ FileOpener, FileSystem, FileSystemExt, FileSystems, FsError, Metadata, OpenOptions, @@ -51,7 +51,7 @@ pub struct OverlayFileSystem { impl OverlayFileSystem where P: FileSystem, - S: for<'a> FileSystems<'a>, + S: FileSystems, { /// Create a new [`FileSystem`] using a primary [`crate::FileSystem`] and a /// chain of secondary [`FileSystems`]. @@ -82,53 +82,58 @@ where (self.primary, self.secondaries) } - /// Iterate over all filesystems in order of precedence. - pub fn iter(&self) -> impl Iterator + '_ { - std::iter::once(self.primary() as &dyn FileSystem) - .chain(self.secondaries().iter_filesystems()) - } - - /// Try to apply an operation to each [`FileSystem`] in order of precedence. - /// - /// This uses [`should_continue()`] to determine whether an error is fatal - /// and needs to be returned to the caller, or whether we should try the - /// next [`FileSystem`] in the chain. - fn for_each(&self, mut func: F) -> Result - where - F: FnMut(&dyn FileSystem) -> Result, - { - for fs in self.iter() { - match func(fs) { - Ok(result) => return Ok(result), - Err(e) if should_continue(e) => continue, - Err(other) => return Err(other), + fn permission_error_or_not_found(&self, path: &Path) -> Result<(), FsError> { + let result = self.secondaries.for_each_filesystems(|fs| { + if fs.exists(path) { + ControlFlow::Break(FsError::PermissionDenied) + } else { + ControlFlow::Continue(()) } - } + }); - Err(FsError::EntryNotFound) + Err(result.unwrap_or(FsError::EntryNotFound)) } } impl FileSystem for OverlayFileSystem where - P: FileSystem, - S: for<'a> crate::FileSystems<'a> + Send + Sync, + P: FileSystem + 'static, + S: crate::FileSystems + Send + Sync + 'static, { fn read_dir(&self, path: &Path) -> Result { let mut entries = Vec::new(); let mut had_at_least_one_success = false; - for fs in self.iter() { - match fs.read_dir(path) { + match self.primary.read_dir(path) { + Ok(r) => { + for entry in r { + entries.push(entry?); + } + had_at_least_one_success = true; + } + Err(e) if should_continue(e) => {} + Err(e) => return Err(e), + } + + let result = self + .secondaries + .for_each_filesystems(|fs| match fs.read_dir(path) { Ok(r) => { for entry in r { - entries.push(entry?); + match entry { + Ok(entry) => entries.push(entry), + Err(e) => return ControlFlow::Break(e), + } } had_at_least_one_success = true; + ControlFlow::Continue(()) } - Err(e) if should_continue(e) => continue, - Err(e) => return Err(e), - } + Err(e) if should_continue(e) => ControlFlow::Continue(()), + Err(e) => ControlFlow::Break(e), + }); + + if let Some(error) = result { + return Err(error); } if had_at_least_one_success { @@ -146,70 +151,55 @@ where fn create_dir(&self, path: &Path) -> Result<(), FsError> { match self.primary.create_dir(path) { - Ok(()) => return Ok(()), Err(e) if should_continue(e) => {} - Err(e) => return Err(e), + other => return other, } - for fs in self.secondaries.iter_filesystems() { - if fs.is_dir(path) { - return Err(FsError::PermissionDenied); - } - } - - Err(FsError::EntryNotFound) + self.permission_error_or_not_found(path) } fn remove_dir(&self, path: &Path) -> Result<(), FsError> { match self.primary.remove_dir(path) { - Ok(()) => return Ok(()), Err(e) if should_continue(e) => {} - Err(e) => return Err(e), + other => return other, } - for fs in self.secondaries.iter_filesystems() { - if fs.is_dir(path) { - return Err(FsError::PermissionDenied); - } - } - - Err(FsError::EntryNotFound) + self.permission_error_or_not_found(path) } fn rename(&self, from: &Path, to: &Path) -> Result<(), FsError> { match self.primary.rename(from, to) { - Ok(()) => return Ok(()), Err(e) if should_continue(e) => {} - Err(e) => return Err(e), + other => return other, } - for fs in self.secondaries.iter_filesystems() { - if fs.exists(from) { - return Err(FsError::PermissionDenied); - } - } - - Err(FsError::EntryNotFound) + self.permission_error_or_not_found(from) } fn metadata(&self, path: &Path) -> Result { - self.for_each(|fs| fs.metadata(path)) + match self.primary.metadata(path) { + Ok(meta) => return Ok(meta), + Err(e) if should_continue(e) => {} + Err(e) => return Err(e), + } + + let result = self + .secondaries + .for_each_filesystems(|fs| match fs.metadata(path) { + Err(e) if should_continue(e) => ControlFlow::Continue(()), + other => ControlFlow::Break(other), + }); + + result.unwrap_or(Err(FsError::EntryNotFound)) } fn remove_file(&self, path: &Path) -> Result<(), FsError> { match self.primary.remove_file(path) { - Ok(()) => return Ok(()), Err(e) if should_continue(e) => {} - Err(e) => return Err(e), - } - - for fs in self.secondaries.iter_filesystems() { - if fs.exists(path) { - return Err(FsError::PermissionDenied); - } + other => return other, } - Err(FsError::EntryNotFound) + self.permission_error_or_not_found(path) } fn new_open_options(&self) -> OpenOptions<'_> { @@ -220,37 +210,108 @@ where impl FileOpener for OverlayFileSystem where P: FileSystem, - S: for<'a> FileSystems<'a> + Send + Sync, + S: FileSystems + Send + Sync + 'static, { fn open( &self, path: &Path, conf: &OpenOptionsConfig, ) -> Result, FsError> { - // TODO: Re-work this method so that trying to create a file inside a - // secondary filesystem will actually create the file on the primary - // filesystem, running create_dir_all() if necessary. - self.for_each(|fs| fs.new_open_options().options(conf.clone()).open(path)) + match self + .primary + .new_open_options() + .options(conf.clone()) + .open(path) + { + Err(e) if should_continue(e) => {} + other => return other, + } + + if conf.create || conf.create_new && !self.exists(path) { + if let Some(parent) = path.parent() { + let is_secondary_dir = self + .secondaries + .for_each_filesystems(|fs| match fs.is_dir(parent) { + true => ControlFlow::Break(true), + false => ControlFlow::Continue(()), + }) + .unwrap_or(false); + if is_secondary_dir { + // We fall into the special case where you can create a file + // that looks like it is inside a secondary filesystem folder, + // but actually it gets created on the host + self.primary.create_dir_all(parent)?; + return self + .primary + .new_open_options() + .options(conf.clone()) + .open(path); + } + } + } + + if opening_would_require_mutations(&self.secondaries, path, conf) { + return Err(FsError::PermissionDenied); + } + + self.secondaries + .for_each_filesystems(|fs| { + match fs.new_open_options().options(conf.clone()).open(path) { + Err(e) if should_continue(e) => ControlFlow::Continue(()), + other => ControlFlow::Break(other), + } + }) + .unwrap_or(Err(FsError::EntryNotFound)) } } +fn opening_would_require_mutations( + secondaries: &S, + path: &Path, + conf: &OpenOptionsConfig, +) -> bool +where + S: FileSystems + Send + Sync, +{ + if conf.append || conf.write || conf.create_new { + return true; + } + + if conf.create { + // Would we create the file if it doesn't exist yet. + let already_exists = secondaries + .for_each_filesystems(|fs| match fs.is_file(path) { + true => ControlFlow::Break(()), + false => ControlFlow::Continue(()), + }) + .is_some(); + + if !already_exists { + return true; + } + } + + false +} + impl Debug for OverlayFileSystem where P: FileSystem, - S: for<'a> FileSystems<'a>, + S: FileSystems, { fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { struct IterFilesystems<'a, S>(&'a S); impl<'a, S> Debug for IterFilesystems<'a, S> where - S: for<'b> FileSystems<'b>, + S: FileSystems, { fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { let mut f = f.debug_list(); - for fs in self.0.iter_filesystems() { + let _: Option<()> = self.0.for_each_filesystems(|fs| { f.entry(&fs); - } + ControlFlow::Continue(()) + }); f.finish() } @@ -272,22 +333,28 @@ fn should_continue(e: FsError) -> bool { mod tests { use std::{path::PathBuf, sync::Arc}; + use tempfile::TempDir; use tokio::io::AsyncWriteExt; use super::*; - use crate::{mem_fs::FileSystem as MemFS, FileSystem as _, FileSystemExt}; + use crate::{ + mem_fs::FileSystem as MemFS, webc_fs::WebcFileSystem, FileSystemExt, RootFileSystemBuilder, + UnionFileSystem, + }; #[test] - fn can_be_used_as_an_object() { + fn object_safe() { fn _box_with_memfs( fs: OverlayFileSystem>, ) -> Box { Box::new(fs) } - fn _arc( - fs: OverlayFileSystem, Vec>>, - ) -> Arc { + fn _arc(fs: OverlayFileSystem) -> Arc + where + A: FileSystem + 'static, + S: FileSystems + Send + Sync + Debug + 'static, + { Arc::new(fs) } } @@ -429,4 +496,80 @@ mod tests { ] ); } + + #[tokio::test] + async fn wasi_runner_use_case() { + // Set up some dummy files on the host + let temp = TempDir::new().unwrap(); + let first = temp.path().join("first"); + let file_txt = first.join("file.txt"); + let second = temp.path().join("second"); + std::fs::create_dir_all(&first).unwrap(); + std::fs::write(&file_txt, b"First!").unwrap(); + std::fs::create_dir_all(&second).unwrap(); + // configure the union FS so things are saved in memory by default, but + // certain folders are mapped to the host. + let mut primary = UnionFileSystem::default(); + let root = RootFileSystemBuilder::new().build(); + primary.mount("in-memory", "/", false, Box::new(root), None); + let mapped_dirs = [&first, &second]; + for dir in mapped_dirs { + let dir = dir.display().to_string(); + let fs = Box::new(crate::host_fs::FileSystem); + primary.mount("in-memory", &dir, false, fs, Some(&dir)); + } + // Set up the secondary file systems + let webc = webc::v1::WebCOwned::parse( + include_bytes!("../../c-api/examples/assets/python-0.1.0.wasmer").to_vec(), + &webc::v1::ParseOptions::default(), + ) + .unwrap(); + let webc = WebcFileSystem::init_all(Arc::new(webc)); + + let fs = OverlayFileSystem::new(primary, [webc]); + + let paths: Vec<_> = fs.walk("/").map(|entry| entry.path()).collect(); + println!("{paths:#?}"); + + // We should get all the normal directories from rootfs (primary) + assert!(fs.is_dir("/lib")); + assert!(fs.is_dir("/bin")); + assert!(fs.is_file("/dev/stdin")); + assert!(fs.is_file("/dev/stdout")); + // We also want to see files from the WEBC volumes (secondary) + assert!(fs.is_dir("/lib/python3.6")); + assert!(fs.is_file("/lib/python3.6/collections/__init__.py")); + // files on a secondary fs aren't writable + assert_eq!( + fs.new_open_options() + .append(true) + .open("/lib/python3.6/collections/__init__.py") + .unwrap_err(), + FsError::PermissionDenied + ); + // you are allowed to create files that look like they are in a secondary + // folder, though + fs.touch("/lib/python3.6/collections/something-else.py") + .unwrap(); + // But it'll be on the primary filesystem, not the secondary one + assert!(fs + .primary + .is_file("/lib/python3.6/collections/something-else.py")); + assert!(!fs.secondaries[0].is_file("/lib/python3.6/collections/something-else.py")); + // You can do the same thing with folders + fs.create_dir("/lib/python3.6/something-else".as_ref()) + .unwrap(); + assert!(fs.primary.is_dir("/lib/python3.6/something-else")); + assert!(!fs.secondaries[0].is_dir("/lib/python3.6/something-else")); + // you should also be able to read files mounted from the host + assert_eq!(fs.read_to_string(&file_txt).await.unwrap(), "First!"); + // Overwriting them is fine and we'll see the changes on the host + fs.write(&file_txt, "Updated").await.unwrap(); + assert_eq!(std::fs::read_to_string(&file_txt).unwrap(), "Updated"); + // The filesystem will see changes on the host that happened after it was + // set up + let another = second.join("another.txt"); + std::fs::write(&another, "asdf").unwrap(); + assert_eq!(fs.read_to_string(&another).await.unwrap(), "asdf"); + } } From 10e0d600fc13ae1148c75489bb03eaf1555fa8ab Mon Sep 17 00:00:00 2001 From: Michael-F-Bryan Date: Wed, 15 Mar 2023 01:11:43 +0800 Subject: [PATCH 06/12] Rewrote the FileSystems trait to use external iterators --- lib/vfs/src/filesystem_ext.rs | 5 +- lib/vfs/src/filesystems.rs | 114 +++++++++++++++---------------- lib/vfs/src/overlay_fs.rs | 124 +++++++++++++++++----------------- 3 files changed, 119 insertions(+), 124 deletions(-) diff --git a/lib/vfs/src/filesystem_ext.rs b/lib/vfs/src/filesystem_ext.rs index e416630a1ba..bd4678258b2 100644 --- a/lib/vfs/src/filesystem_ext.rs +++ b/lib/vfs/src/filesystem_ext.rs @@ -40,10 +40,11 @@ pub trait FileSystemExt { /// This is analogous to [`std::fs::read_to_string()`]. async fn read_to_string(&self, path: impl AsRef + Send) -> Result; - /// Make sure a file exists, creating an empty file if it doesn't. + /// Update a file's modification and access times, creating the file if it + /// doesn't already exist. fn touch(&self, path: impl AsRef + Send) -> Result<(), FsError>; - /// Recursively iterate over all paths inside a directory, discarding any + /// Recursively iterate over all paths inside a directory, ignoring any /// errors that may occur along the way. fn walk(&self, path: impl AsRef) -> Box + '_>; } diff --git a/lib/vfs/src/filesystems.rs b/lib/vfs/src/filesystems.rs index d6e37ed2233..73a1b6faaef 100644 --- a/lib/vfs/src/filesystems.rs +++ b/lib/vfs/src/filesystems.rs @@ -1,105 +1,99 @@ use crate::FileSystem; -use std::ops::ControlFlow; /// A chain of one or more [`FileSystem`]s. -pub trait FileSystems { +pub trait FileSystems<'a>: 'a { // FIXME(Michael-F-Bryan): Rewrite this to use GATs and an external iterator - // when we bump the MSRV to 1.65 or higher. - fn for_each_filesystems(&self, func: F) -> Option - where - F: FnMut(&dyn FileSystem) -> ControlFlow; + // when we bump the MSRV to 1.65 or higher. That'll get rid of all the + // lifetimes and HRTBs. + type Iter: IntoIterator + 'a; + fn iter_filesystems(&'a self) -> Self::Iter; } -impl<'b, S> FileSystems for &'b S +impl<'a, 'b, S> FileSystems<'a> for &'b S where - S: FileSystems + 'b, + S: FileSystems<'a> + 'b, + 'b: 'a, { - fn for_each_filesystems(&self, func: F) -> Option - where - F: FnMut(&dyn FileSystem) -> ControlFlow, - { - (**self).for_each_filesystems(func) + type Iter = S::Iter; + + fn iter_filesystems(&'a self) -> Self::Iter { + (**self).iter_filesystems() } } -impl FileSystems for Vec +impl<'a, T> FileSystems<'a> for Vec where T: FileSystem, { - fn for_each_filesystems(&self, func: F) -> Option - where - F: FnMut(&dyn FileSystem) -> ControlFlow, - { - self[..].for_each_filesystems(func) + type Iter = <[T] as FileSystems<'a>>::Iter; + + fn iter_filesystems(&'a self) -> Self::Iter { + self[..].iter_filesystems() } } -impl FileSystems for [T; N] +impl<'a, T, const N: usize> FileSystems<'a> for [T; N] where T: FileSystem, { - fn for_each_filesystems(&self, func: F) -> Option - where - F: FnMut(&dyn FileSystem) -> ControlFlow, - { - self[..].for_each_filesystems(func) + type Iter = [&'a dyn FileSystem; N]; + + fn iter_filesystems(&'a self) -> Self::Iter { + // TODO: rewrite this when array::each_ref() is stable + let mut i = 0; + [(); N].map(|_| { + let f = &self[i] as &dyn FileSystem; + i += 1; + f + }) } } -impl FileSystems for [T] +impl<'a, T> FileSystems<'a> for [T] where T: FileSystem, { - fn for_each_filesystems(&self, mut func: F) -> Option - where - F: FnMut(&dyn FileSystem) -> ControlFlow, - { - for fs in self.iter() { - match func(fs) { - ControlFlow::Continue(_) => continue, - ControlFlow::Break(result) => return Some(result), - } - } + type Iter = std::iter::Map, fn(&T) -> &dyn FileSystem>; - None + fn iter_filesystems(&'a self) -> Self::Iter { + self.iter().map(|fs| fs as &dyn FileSystem) } } -impl FileSystems for () { - fn for_each_filesystems(&self, _func: F) -> Option - where - F: FnMut(&dyn FileSystem) -> ControlFlow, - { - None +impl<'a> FileSystems<'a> for () { + type Iter = std::iter::Empty<&'a dyn FileSystem>; + + fn iter_filesystems(&'a self) -> Self::Iter { + std::iter::empty() } } +macro_rules! count { + ($first:tt $($rest:tt)*) => { + 1 + count!($($rest)*) + }; + () => { 0 }; +} + macro_rules! tuple_filesystems { ($first:ident $(, $rest:ident)* $(,)?) => { - impl<$first, $( $rest ),*> FileSystems for ($first, $($rest),*) + impl<'a, $first, $( $rest ),*> FileSystems<'a> for ($first, $($rest),*) where $first: FileSystem, $($rest: FileSystem),* { - fn for_each_filesystems(&self, mut func: F) -> Option - where - F: FnMut(&dyn FileSystem) -> ControlFlow, - { - #[allow(non_snake_case)] - let ($first, $($rest),*) = &self; + type Iter = [&'a dyn FileSystem; count!($first $($rest)*)]; - if let ControlFlow::Break(result) = func($first) { - return Some(result); - } - - $( - if let ControlFlow::Break(result) = func($rest) { - return Some(result); - } - )* + fn iter_filesystems(&'a self) -> Self::Iter { + #[allow(non_snake_case)] + let ($first, $($rest),*) = self; - None + [ + $first as &dyn FileSystem, + $($rest),* + ] } + } tuple_filesystems!($($rest),*); diff --git a/lib/vfs/src/overlay_fs.rs b/lib/vfs/src/overlay_fs.rs index c3d8a7c8203..1a1dc91bc5f 100644 --- a/lib/vfs/src/overlay_fs.rs +++ b/lib/vfs/src/overlay_fs.rs @@ -1,4 +1,4 @@ -use std::{fmt::Debug, ops::ControlFlow, path::Path}; +use std::{fmt::Debug, path::Path}; use crate::{ FileOpener, FileSystem, FileSystemExt, FileSystems, FsError, Metadata, OpenOptions, @@ -50,8 +50,8 @@ pub struct OverlayFileSystem { impl OverlayFileSystem where - P: FileSystem, - S: FileSystems, + P: FileSystem + 'static, + S: for<'a> FileSystems<'a> + Send + Sync + 'static, { /// Create a new [`FileSystem`] using a primary [`crate::FileSystem`] and a /// chain of secondary [`FileSystems`]. @@ -62,43 +62,47 @@ where } } + /// Get a reference to the primary filesystem. pub fn primary(&self) -> &P { &self.primary } + /// Get a mutable reference to the primary filesystem. pub fn primary_mut(&mut self) -> &mut P { &mut self.primary } + /// Get a reference to the secondary filesystems. pub fn secondaries(&self) -> &S { &self.secondaries } + /// Get a mutable reference to the secondary filesystems. pub fn secondaries_mut(&mut self) -> &mut S { &mut self.secondaries } + /// Consume the [`OverlayFileSystem`], returning the underlying primary and + /// secondary filesystems. pub fn into_inner(self) -> (P, S) { (self.primary, self.secondaries) } fn permission_error_or_not_found(&self, path: &Path) -> Result<(), FsError> { - let result = self.secondaries.for_each_filesystems(|fs| { + for fs in self.secondaries.iter_filesystems() { if fs.exists(path) { - ControlFlow::Break(FsError::PermissionDenied) - } else { - ControlFlow::Continue(()) + return Err(FsError::PermissionDenied); } - }); + } - Err(result.unwrap_or(FsError::EntryNotFound)) + Err(FsError::EntryNotFound) } } impl FileSystem for OverlayFileSystem where P: FileSystem + 'static, - S: crate::FileSystems + Send + Sync + 'static, + S: for<'a> FileSystems<'a> + Send + Sync + 'static, { fn read_dir(&self, path: &Path) -> Result { let mut entries = Vec::new(); @@ -115,25 +119,17 @@ where Err(e) => return Err(e), } - let result = self - .secondaries - .for_each_filesystems(|fs| match fs.read_dir(path) { + for fs in self.secondaries.iter_filesystems() { + match fs.read_dir(path) { Ok(r) => { for entry in r { - match entry { - Ok(entry) => entries.push(entry), - Err(e) => return ControlFlow::Break(e), - } + entries.push(entry?); } had_at_least_one_success = true; - ControlFlow::Continue(()) } - Err(e) if should_continue(e) => ControlFlow::Continue(()), - Err(e) => ControlFlow::Break(e), - }); - - if let Some(error) = result { - return Err(error); + Err(e) if should_continue(e) => continue, + Err(e) => return Err(e), + } } if had_at_least_one_success { @@ -183,14 +179,14 @@ where Err(e) => return Err(e), } - let result = self - .secondaries - .for_each_filesystems(|fs| match fs.metadata(path) { - Err(e) if should_continue(e) => ControlFlow::Continue(()), - other => ControlFlow::Break(other), - }); + for fs in self.secondaries.iter_filesystems() { + match fs.metadata(path) { + Err(e) if should_continue(e) => continue, + other => return other, + } + } - result.unwrap_or(Err(FsError::EntryNotFound)) + Err(FsError::EntryNotFound) } fn remove_file(&self, path: &Path) -> Result<(), FsError> { @@ -210,7 +206,7 @@ where impl FileOpener for OverlayFileSystem where P: FileSystem, - S: FileSystems + Send + Sync + 'static, + S: for<'a> FileSystems<'a> + Send + Sync + 'static, { fn open( &self, @@ -227,16 +223,14 @@ where other => return other, } - if conf.create || conf.create_new && !self.exists(path) { + if (conf.create || conf.create_new) && !self.exists(path) { if let Some(parent) = path.parent() { - let is_secondary_dir = self + let parent_exists_on_secondary_fs = self .secondaries - .for_each_filesystems(|fs| match fs.is_dir(parent) { - true => ControlFlow::Break(true), - false => ControlFlow::Continue(()), - }) - .unwrap_or(false); - if is_secondary_dir { + .iter_filesystems() + .into_iter() + .any(|fs| fs.is_dir(parent)); + if parent_exists_on_secondary_fs { // We fall into the special case where you can create a file // that looks like it is inside a secondary filesystem folder, // but actually it gets created on the host @@ -246,6 +240,8 @@ where .new_open_options() .options(conf.clone()) .open(path); + } else { + return Err(FsError::EntryNotFound); } } } @@ -254,14 +250,14 @@ where return Err(FsError::PermissionDenied); } - self.secondaries - .for_each_filesystems(|fs| { - match fs.new_open_options().options(conf.clone()).open(path) { - Err(e) if should_continue(e) => ControlFlow::Continue(()), - other => ControlFlow::Break(other), - } - }) - .unwrap_or(Err(FsError::EntryNotFound)) + for fs in self.secondaries.iter_filesystems() { + match fs.new_open_options().options(conf.clone()).open(path) { + Err(e) if should_continue(e) => continue, + other => return other, + } + } + + Err(FsError::EntryNotFound) } } @@ -271,20 +267,18 @@ fn opening_would_require_mutations( conf: &OpenOptionsConfig, ) -> bool where - S: FileSystems + Send + Sync, + S: for<'a> FileSystems<'a> + Send + Sync, { - if conf.append || conf.write || conf.create_new { + if conf.append || conf.write || conf.create_new | conf.truncate { return true; } if conf.create { - // Would we create the file if it doesn't exist yet. + // Would we create the file if it doesn't exist yet? let already_exists = secondaries - .for_each_filesystems(|fs| match fs.is_file(path) { - true => ControlFlow::Break(()), - false => ControlFlow::Continue(()), - }) - .is_some(); + .iter_filesystems() + .into_iter() + .any(|fs| fs.is_file(path)); if !already_exists { return true; @@ -297,21 +291,20 @@ where impl Debug for OverlayFileSystem where P: FileSystem, - S: FileSystems, + S: for<'a> FileSystems<'a>, { fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { struct IterFilesystems<'a, S>(&'a S); impl<'a, S> Debug for IterFilesystems<'a, S> where - S: FileSystems, + S: for<'b> FileSystems<'b>, { fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { let mut f = f.debug_list(); - let _: Option<()> = self.0.for_each_filesystems(|fs| { + for fs in self.0.iter_filesystems() { f.entry(&fs); - ControlFlow::Continue(()) - }); + } f.finish() } @@ -353,7 +346,7 @@ mod tests { fn _arc(fs: OverlayFileSystem) -> Arc where A: FileSystem + 'static, - S: FileSystems + Send + Sync + Debug + 'static, + S: for<'a> FileSystems<'a> + Send + Sync + Debug + 'static, { Arc::new(fs) } @@ -561,6 +554,13 @@ mod tests { .unwrap(); assert!(fs.primary.is_dir("/lib/python3.6/something-else")); assert!(!fs.secondaries[0].is_dir("/lib/python3.6/something-else")); + // It only works when you are directly inside an existing directory + // on the secondary filesystem, though + assert_eq!( + fs.touch("/lib/python3.6/collections/this/doesnt/exist.txt") + .unwrap_err(), + FsError::EntryNotFound + ); // you should also be able to read files mounted from the host assert_eq!(fs.read_to_string(&file_txt).await.unwrap(), "First!"); // Overwriting them is fine and we'll see the changes on the host From a9d752d93cff94cde8d97b5b670b0333f22af446 Mon Sep 17 00:00:00 2001 From: Michael-F-Bryan Date: Wed, 15 Mar 2023 02:25:45 +0800 Subject: [PATCH 07/12] The WASI runner test now mounts to separate folders --- lib/vfs/src/filesystem_ext.rs | 3 ++- lib/vfs/src/lib.rs | 3 ++- lib/vfs/src/overlay_fs.rs | 44 +++++++++++++++++++---------------- 3 files changed, 28 insertions(+), 22 deletions(-) diff --git a/lib/vfs/src/filesystem_ext.rs b/lib/vfs/src/filesystem_ext.rs index bd4678258b2..a57c490f629 100644 --- a/lib/vfs/src/filesystem_ext.rs +++ b/lib/vfs/src/filesystem_ext.rs @@ -84,11 +84,12 @@ impl FileSystemExt for F { let mut f = self .new_open_options() .create(true) - .write(true) .truncate(true) + .write(true) .open(path)?; f.write_all(data).await?; + f.flush().await?; Ok(()) } diff --git a/lib/vfs/src/lib.rs b/lib/vfs/src/lib.rs index fa6cbfb6bf0..87545f76965 100644 --- a/lib/vfs/src/lib.rs +++ b/lib/vfs/src/lib.rs @@ -31,7 +31,7 @@ pub mod zero_file; // tty_file -> see wasmer_wasi::tty_file mod filesystem_ext; mod filesystems; -pub mod overlay_fs; +mod overlay_fs; pub mod pipe; #[cfg(feature = "static-fs")] pub mod static_fs; @@ -48,6 +48,7 @@ pub use empty_fs::*; pub use filesystem_ext::FileSystemExt; pub use filesystems::FileSystems; pub use null_file::*; +pub use overlay_fs::OverlayFileSystem; pub use passthru_fs::*; pub use pipe::*; pub use special_file::*; diff --git a/lib/vfs/src/overlay_fs.rs b/lib/vfs/src/overlay_fs.rs index 1a1dc91bc5f..ee172897bde 100644 --- a/lib/vfs/src/overlay_fs.rs +++ b/lib/vfs/src/overlay_fs.rs @@ -29,9 +29,9 @@ use crate::{ /// use wasmer_vfs::{ /// mem_fs::FileSystem as MemFS, /// host_fs::FileSystem as HostFS, -/// overlay_fs::FileSystem, +/// overlay_fs::OverlayFileSystem, /// }; -/// let fs = OverlayFS::new(MemFS::default(), [HostFS]); +/// let fs = OverlayFileSystem::new(MemFS::default(), [HostFS]); /// /// // This also has the benefit of storing the two values in-line with no extra /// // overhead or indirection. @@ -332,7 +332,6 @@ mod tests { use super::*; use crate::{ mem_fs::FileSystem as MemFS, webc_fs::WebcFileSystem, FileSystemExt, RootFileSystemBuilder, - UnionFileSystem, }; #[test] @@ -500,16 +499,16 @@ mod tests { std::fs::create_dir_all(&first).unwrap(); std::fs::write(&file_txt, b"First!").unwrap(); std::fs::create_dir_all(&second).unwrap(); - // configure the union FS so things are saved in memory by default, but - // certain folders are mapped to the host. - let mut primary = UnionFileSystem::default(); - let root = RootFileSystemBuilder::new().build(); - primary.mount("in-memory", "/", false, Box::new(root), None); - let mapped_dirs = [&first, &second]; - for dir in mapped_dirs { - let dir = dir.display().to_string(); - let fs = Box::new(crate::host_fs::FileSystem); - primary.mount("in-memory", &dir, false, fs, Some(&dir)); + // configure the union FS so things are saved in memory by default + // (initialized with a set of unix-like folders), but certain folders + // are first to the host. + let primary = RootFileSystemBuilder::new().build(); + let host_fs: Arc = Arc::new(crate::host_fs::FileSystem); + let first_dirs = [(&first, "/first"), (&second, "/second")]; + for (host, guest) in first_dirs { + primary + .mount(PathBuf::from(guest), &host_fs, host.clone()) + .unwrap(); } // Set up the secondary file systems let webc = webc::v1::WebCOwned::parse( @@ -521,9 +520,6 @@ mod tests { let fs = OverlayFileSystem::new(primary, [webc]); - let paths: Vec<_> = fs.walk("/").map(|entry| entry.path()).collect(); - println!("{paths:#?}"); - // We should get all the normal directories from rootfs (primary) assert!(fs.is_dir("/lib")); assert!(fs.is_dir("/bin")); @@ -538,7 +534,7 @@ mod tests { .append(true) .open("/lib/python3.6/collections/__init__.py") .unwrap_err(), - FsError::PermissionDenied + FsError::PermissionDenied, ); // you are allowed to create files that look like they are in a secondary // folder, though @@ -562,14 +558,22 @@ mod tests { FsError::EntryNotFound ); // you should also be able to read files mounted from the host - assert_eq!(fs.read_to_string(&file_txt).await.unwrap(), "First!"); + assert!(fs.is_dir("/first")); + assert!(fs.is_file("/first/file.txt")); + assert_eq!( + fs.read_to_string("/first/file.txt").await.unwrap(), + "First!" + ); // Overwriting them is fine and we'll see the changes on the host - fs.write(&file_txt, "Updated").await.unwrap(); + fs.write("/first/file.txt", "Updated").await.unwrap(); assert_eq!(std::fs::read_to_string(&file_txt).unwrap(), "Updated"); // The filesystem will see changes on the host that happened after it was // set up let another = second.join("another.txt"); std::fs::write(&another, "asdf").unwrap(); - assert_eq!(fs.read_to_string(&another).await.unwrap(), "asdf"); + assert_eq!( + fs.read_to_string("/second/another.txt").await.unwrap(), + "asdf" + ); } } From 0184baa7c9232d51cc59884c3de856b7e4256488 Mon Sep 17 00:00:00 2001 From: Michael-F-Bryan Date: Wed, 15 Mar 2023 02:26:03 +0800 Subject: [PATCH 08/12] Added tests for #3678 --- lib/vfs/src/mem_fs/filesystem.rs | 50 +++++++++++++++++++++++--------- lib/vfs/src/union_fs.rs | 42 +++++++++++++++++++++++---- 2 files changed, 72 insertions(+), 20 deletions(-) diff --git a/lib/vfs/src/mem_fs/filesystem.rs b/lib/vfs/src/mem_fs/filesystem.rs index e8a9dc178d9..50a947eb4f7 100644 --- a/lib/vfs/src/mem_fs/filesystem.rs +++ b/lib/vfs/src/mem_fs/filesystem.rs @@ -937,9 +937,25 @@ impl Default for FileSystemInner { } } +#[allow(dead_code)] // The `No` variant. +pub(super) enum DirectoryMustBeEmpty { + Yes, + No, +} + +impl DirectoryMustBeEmpty { + pub(super) fn yes(&self) -> bool { + matches!(self, Self::Yes) + } + + pub(super) fn no(&self) -> bool { + !self.yes() + } +} + #[cfg(test)] mod test_filesystem { - use crate::{mem_fs::*, DirEntry, FileSystem as FS, FileType, FsError}; + use crate::{mem_fs::*, DirEntry, FileSystem as FS, FileSystemExt, FileType, FsError}; macro_rules! path { ($path:expr) => { @@ -1686,20 +1702,26 @@ mod test_filesystem { "canonicalizing a crazily stupid path name", ); } -} - -#[allow(dead_code)] // The `No` variant. -pub(super) enum DirectoryMustBeEmpty { - Yes, - No, -} -impl DirectoryMustBeEmpty { - pub(super) fn yes(&self) -> bool { - matches!(self, Self::Yes) - } + #[test] + #[ignore = "Not yet supported. See https://github.com/wasmerio/wasmer/issues/3678"] + fn mount_to_overlapping_directories() { + let top_level = FileSystem::default(); + top_level.touch("/file.txt").unwrap(); + let nested = FileSystem::default(); + nested.touch("/another-file.txt").unwrap(); + let top_level: Arc = Arc::new(top_level); + let nested: Arc = Arc::new(nested); - pub(super) fn no(&self) -> bool { - !self.yes() + let fs = FileSystem::default(); + fs.mount("/top-level".into(), &top_level, "/".into()) + .unwrap(); + fs.mount("/top-level/nested".into(), &nested, "/".into()) + .unwrap(); + + assert!(fs.is_dir("/top-level")); + assert!(fs.is_file("/top-level/file.txt")); + assert!(fs.is_dir("/top-level/nested")); + assert!(fs.is_file("/top-level/nested/another-file.txt")); } } diff --git a/lib/vfs/src/union_fs.rs b/lib/vfs/src/union_fs.rs index 61df13517fc..5908558ac79 100644 --- a/lib/vfs/src/union_fs.rs +++ b/lib/vfs/src/union_fs.rs @@ -460,14 +460,14 @@ impl FileOpener for UnionFileSystem { #[cfg(test)] mod tests { + use std::{path::Path, sync::Arc}; + use tokio::io::AsyncWriteExt; - use crate::host_fs::FileSystem; - use crate::mem_fs; - use crate::FileSystem as FileSystemTrait; - use crate::FsError; - use crate::UnionFileSystem; - use std::path::Path; + use crate::{ + host_fs::FileSystem, mem_fs, FileSystem as FileSystemTrait, FileSystemExt, FsError, + UnionFileSystem, + }; fn gen_filesystem() -> UnionFileSystem { let mut union = UnionFileSystem::new(); @@ -1073,4 +1073,34 @@ mod tests { let _ = fs_extra::remove_items(&["./test_canonicalize"]); } */ + + #[test] + #[ignore = "Not yet supported. See https://github.com/wasmerio/wasmer/issues/3678"] + fn mount_to_overlapping_directories() { + let top_level = mem_fs::FileSystem::default(); + top_level.touch("/file.txt").unwrap(); + let nested = mem_fs::FileSystem::default(); + nested.touch("/another-file.txt").unwrap(); + + let mut fs = UnionFileSystem::default(); + fs.mount( + "top-level", + "/", + false, + Box::new(top_level), + Some("/top-level"), + ); + fs.mount( + "nested", + "/", + false, + Box::new(nested), + Some("/top-level/nested"), + ); + + assert!(fs.is_dir("/top-level")); + assert!(fs.is_file("/top-level/file.txt")); + assert!(fs.is_dir("/top-level/nested")); + assert!(fs.is_file("/top-level/nested/another-file.txt")); + } } From eda8807a3a35559361609bd0d541280df59e87c1 Mon Sep 17 00:00:00 2001 From: Michael-F-Bryan Date: Wed, 15 Mar 2023 02:32:28 +0800 Subject: [PATCH 09/12] Fixed a doctest --- lib/vfs/src/overlay_fs.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/vfs/src/overlay_fs.rs b/lib/vfs/src/overlay_fs.rs index ee172897bde..eaf349b399d 100644 --- a/lib/vfs/src/overlay_fs.rs +++ b/lib/vfs/src/overlay_fs.rs @@ -29,7 +29,7 @@ use crate::{ /// use wasmer_vfs::{ /// mem_fs::FileSystem as MemFS, /// host_fs::FileSystem as HostFS, -/// overlay_fs::OverlayFileSystem, +/// OverlayFileSystem, /// }; /// let fs = OverlayFileSystem::new(MemFS::default(), [HostFS]); /// From b30024fc7f1f077b2a132f6b76add0b2a071d08c Mon Sep 17 00:00:00 2001 From: Michael-F-Bryan Date: Wed, 15 Mar 2023 02:46:32 +0800 Subject: [PATCH 10/12] renamed iter_filesystems() to filesystems() --- lib/vfs/src/filesystems.rs | 26 ++++++++++++++------------ lib/vfs/src/overlay_fs.rs | 34 ++++++++++++++-------------------- 2 files changed, 28 insertions(+), 32 deletions(-) diff --git a/lib/vfs/src/filesystems.rs b/lib/vfs/src/filesystems.rs index 73a1b6faaef..bf47eb2df8e 100644 --- a/lib/vfs/src/filesystems.rs +++ b/lib/vfs/src/filesystems.rs @@ -2,11 +2,13 @@ use crate::FileSystem; /// A chain of one or more [`FileSystem`]s. pub trait FileSystems<'a>: 'a { - // FIXME(Michael-F-Bryan): Rewrite this to use GATs and an external iterator - // when we bump the MSRV to 1.65 or higher. That'll get rid of all the - // lifetimes and HRTBs. + // FIXME(Michael-F-Bryan): Rewrite this to use GATs when we bump the MSRV to + // 1.65 or higher. That'll get rid of all the lifetimes and HRTBs. type Iter: IntoIterator + 'a; - fn iter_filesystems(&'a self) -> Self::Iter; + + /// Get something that can be used to iterate over the underlying + /// filesystems. + fn filesystems(&'a self) -> Self::Iter; } impl<'a, 'b, S> FileSystems<'a> for &'b S @@ -16,8 +18,8 @@ where { type Iter = S::Iter; - fn iter_filesystems(&'a self) -> Self::Iter { - (**self).iter_filesystems() + fn filesystems(&'a self) -> Self::Iter { + (**self).filesystems() } } @@ -27,8 +29,8 @@ where { type Iter = <[T] as FileSystems<'a>>::Iter; - fn iter_filesystems(&'a self) -> Self::Iter { - self[..].iter_filesystems() + fn filesystems(&'a self) -> Self::Iter { + self[..].filesystems() } } @@ -38,7 +40,7 @@ where { type Iter = [&'a dyn FileSystem; N]; - fn iter_filesystems(&'a self) -> Self::Iter { + fn filesystems(&'a self) -> Self::Iter { // TODO: rewrite this when array::each_ref() is stable let mut i = 0; [(); N].map(|_| { @@ -55,7 +57,7 @@ where { type Iter = std::iter::Map, fn(&T) -> &dyn FileSystem>; - fn iter_filesystems(&'a self) -> Self::Iter { + fn filesystems(&'a self) -> Self::Iter { self.iter().map(|fs| fs as &dyn FileSystem) } } @@ -63,7 +65,7 @@ where impl<'a> FileSystems<'a> for () { type Iter = std::iter::Empty<&'a dyn FileSystem>; - fn iter_filesystems(&'a self) -> Self::Iter { + fn filesystems(&'a self) -> Self::Iter { std::iter::empty() } } @@ -84,7 +86,7 @@ macro_rules! tuple_filesystems { { type Iter = [&'a dyn FileSystem; count!($first $($rest)*)]; - fn iter_filesystems(&'a self) -> Self::Iter { + fn filesystems(&'a self) -> Self::Iter { #[allow(non_snake_case)] let ($first, $($rest),*) = self; diff --git a/lib/vfs/src/overlay_fs.rs b/lib/vfs/src/overlay_fs.rs index eaf349b399d..35627dc82d7 100644 --- a/lib/vfs/src/overlay_fs.rs +++ b/lib/vfs/src/overlay_fs.rs @@ -89,7 +89,7 @@ where } fn permission_error_or_not_found(&self, path: &Path) -> Result<(), FsError> { - for fs in self.secondaries.iter_filesystems() { + for fs in self.secondaries.filesystems() { if fs.exists(path) { return Err(FsError::PermissionDenied); } @@ -108,18 +108,11 @@ where let mut entries = Vec::new(); let mut had_at_least_one_success = false; - match self.primary.read_dir(path) { - Ok(r) => { - for entry in r { - entries.push(entry?); - } - had_at_least_one_success = true; - } - Err(e) if should_continue(e) => {} - Err(e) => return Err(e), - } + let filesystems = std::iter::once(&self.primary as &dyn FileSystem) + .into_iter() + .chain(self.secondaries().filesystems()); - for fs in self.secondaries.iter_filesystems() { + for fs in filesystems { match fs.read_dir(path) { Ok(r) => { for entry in r { @@ -133,10 +126,11 @@ where } if had_at_least_one_success { - // Note: this sort is guaranteed to be stable, so filesystems - // "higher up" the chain will be further towards the start. - entries.sort_by(|a, b| a.path.cmp(&b.path)); // Make sure later entries are removed in favour of earlier ones. + // Note: this sort is guaranteed to be stable, meaning filesystems + // "higher up" the chain will be further towards the start and kept + // when deduplicating. + entries.sort_by(|a, b| a.path.cmp(&b.path)); entries.dedup_by(|a, b| a.path == b.path); Ok(ReadDir::new(entries)) @@ -179,7 +173,7 @@ where Err(e) => return Err(e), } - for fs in self.secondaries.iter_filesystems() { + for fs in self.secondaries.filesystems() { match fs.metadata(path) { Err(e) if should_continue(e) => continue, other => return other, @@ -227,7 +221,7 @@ where if let Some(parent) = path.parent() { let parent_exists_on_secondary_fs = self .secondaries - .iter_filesystems() + .filesystems() .into_iter() .any(|fs| fs.is_dir(parent)); if parent_exists_on_secondary_fs { @@ -250,7 +244,7 @@ where return Err(FsError::PermissionDenied); } - for fs in self.secondaries.iter_filesystems() { + for fs in self.secondaries.filesystems() { match fs.new_open_options().options(conf.clone()).open(path) { Err(e) if should_continue(e) => continue, other => return other, @@ -276,7 +270,7 @@ where if conf.create { // Would we create the file if it doesn't exist yet? let already_exists = secondaries - .iter_filesystems() + .filesystems() .into_iter() .any(|fs| fs.is_file(path)); @@ -302,7 +296,7 @@ where fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { let mut f = f.debug_list(); - for fs in self.0.iter_filesystems() { + for fs in self.0.filesystems() { f.entry(&fs); } From 6cbaa0ab721892d5179aa973671b314b1d35cf6b Mon Sep 17 00:00:00 2001 From: Michael-F-Bryan Date: Thu, 16 Mar 2023 18:02:31 +0800 Subject: [PATCH 11/12] Moved the FileSystemExt trait into its own top-level functions --- lib/vfs/src/filesystem_ext.rs | 238 ---------------------------------- lib/vfs/src/lib.rs | 3 +- lib/vfs/src/ops.rs | 230 ++++++++++++++++++++++++++++++++ lib/vfs/src/overlay_fs.rs | 126 +++++++++--------- 4 files changed, 295 insertions(+), 302 deletions(-) delete mode 100644 lib/vfs/src/filesystem_ext.rs create mode 100644 lib/vfs/src/ops.rs diff --git a/lib/vfs/src/filesystem_ext.rs b/lib/vfs/src/filesystem_ext.rs deleted file mode 100644 index a57c490f629..00000000000 --- a/lib/vfs/src/filesystem_ext.rs +++ /dev/null @@ -1,238 +0,0 @@ -use std::{collections::VecDeque, path::Path}; - -use tokio::io::{AsyncReadExt, AsyncWriteExt}; - -use crate::{DirEntry, FileSystem, FsError}; - -/// Helper methods for working with [`FileSystem`]s. -#[async_trait::async_trait] -pub trait FileSystemExt { - /// Does this item exists? - fn exists(&self, path: impl AsRef) -> bool; - - /// Does this path refer to a directory? - fn is_dir(&self, path: impl AsRef) -> bool; - - /// Does this path refer to a file? - fn is_file(&self, path: impl AsRef) -> bool; - - /// Make sure a directory (and all its parents) exist. - /// - /// This is analogous to [`std::fs::create_dir_all()`]. - fn create_dir_all(&self, path: impl AsRef) -> Result<(), FsError>; - - /// Asynchronously write some bytes to a file. - /// - /// This is analogous to [`std::fs::write()`]. - async fn write( - &self, - path: impl AsRef + Send, - data: impl AsRef<[u8]> + Send, - ) -> Result<(), FsError>; - - /// Asynchronously read a file's contents into memory. - /// - /// This is analogous to [`std::fs::read()`]. - async fn read(&self, path: impl AsRef + Send) -> Result, FsError>; - - /// Asynchronously read a file's contents into memory as a string. - /// - /// This is analogous to [`std::fs::read_to_string()`]. - async fn read_to_string(&self, path: impl AsRef + Send) -> Result; - - /// Update a file's modification and access times, creating the file if it - /// doesn't already exist. - fn touch(&self, path: impl AsRef + Send) -> Result<(), FsError>; - - /// Recursively iterate over all paths inside a directory, ignoring any - /// errors that may occur along the way. - fn walk(&self, path: impl AsRef) -> Box + '_>; -} - -#[async_trait::async_trait] -impl FileSystemExt for F { - fn exists(&self, path: impl AsRef) -> bool { - self.metadata(path.as_ref()).is_ok() - } - - fn is_dir(&self, path: impl AsRef) -> bool { - match self.metadata(path.as_ref()) { - Ok(meta) => meta.is_dir(), - Err(_) => false, - } - } - - fn is_file(&self, path: impl AsRef) -> bool { - match self.metadata(path.as_ref()) { - Ok(meta) => meta.is_file(), - Err(_) => false, - } - } - - fn create_dir_all(&self, path: impl AsRef) -> Result<(), FsError> { - create_dir_all(self, path.as_ref()) - } - - async fn write( - &self, - path: impl AsRef + Send, - data: impl AsRef<[u8]> + Send, - ) -> Result<(), FsError> { - let path = path.as_ref(); - let data = data.as_ref(); - - let mut f = self - .new_open_options() - .create(true) - .truncate(true) - .write(true) - .open(path)?; - - f.write_all(data).await?; - f.flush().await?; - - Ok(()) - } - - async fn read(&self, path: impl AsRef + Send) -> Result, FsError> { - let mut f = self.new_open_options().read(true).open(path.as_ref())?; - let mut buffer = Vec::new(); - f.read_to_end(&mut buffer).await?; - - Ok(buffer) - } - - async fn read_to_string(&self, path: impl AsRef + Send) -> Result { - let mut f = self.new_open_options().read(true).open(path.as_ref())?; - let mut buffer = String::new(); - f.read_to_string(&mut buffer).await?; - - Ok(buffer) - } - - fn touch(&self, path: impl AsRef + Send) -> Result<(), FsError> { - let _ = self - .new_open_options() - .create(true) - .write(true) - .open(path)?; - - Ok(()) - } - - fn walk(&self, path: impl AsRef) -> Box + '_> { - let path = path.as_ref(); - let mut dirs_to_visit: VecDeque<_> = self - .read_dir(path) - .ok() - .into_iter() - .flatten() - .filter_map(|result| result.ok()) - .collect(); - - Box::new(std::iter::from_fn(move || { - let next = dirs_to_visit.pop_back()?; - - if let Ok(children) = self.read_dir(&next.path) { - dirs_to_visit.extend(children.flatten()); - } - - Some(next) - })) - } -} - -fn create_dir_all(fs: &F, path: &Path) -> Result<(), FsError> -where - F: FileSystem + ?Sized, -{ - if let Some(parent) = path.parent() { - create_dir_all(fs, parent)?; - } - - if let Ok(metadata) = fs.metadata(path) { - if metadata.is_dir() { - return Ok(()); - } - if metadata.is_file() { - return Err(FsError::BaseNotDirectory); - } - } - - fs.create_dir(path) -} - -#[cfg(test)] -mod tests { - use super::*; - use crate::mem_fs::FileSystem as MemFS; - use tokio::io::AsyncReadExt; - - #[tokio::test] - async fn write() { - let fs = MemFS::default(); - - fs.write("/file.txt", b"Hello, World!").await.unwrap(); - - let mut contents = String::new(); - fs.new_open_options() - .read(true) - .open("/file.txt") - .unwrap() - .read_to_string(&mut contents) - .await - .unwrap(); - assert_eq!(contents, "Hello, World!"); - } - - #[tokio::test] - async fn read() { - let fs = MemFS::default(); - fs.new_open_options() - .create(true) - .write(true) - .open("/file.txt") - .unwrap() - .write_all(b"Hello, World!") - .await - .unwrap(); - - let contents = fs.read_to_string("/file.txt").await.unwrap(); - assert_eq!(contents, "Hello, World!"); - - let contents = fs.read("/file.txt").await.unwrap(); - assert_eq!(contents, b"Hello, World!"); - } - - #[tokio::test] - async fn create_dir_all() { - let fs = MemFS::default(); - fs.write("/file.txt", b"").await.unwrap(); - - assert!(!fs.exists("/really/nested/directory")); - fs.create_dir_all("/really/nested/directory").unwrap(); - assert!(fs.exists("/really/nested/directory")); - - // It's okay to create the same directory multiple times - fs.create_dir_all("/really/nested/directory").unwrap(); - - // You can't create a directory on top of a file - assert_eq!( - fs.create_dir_all("/file.txt").unwrap_err(), - FsError::BaseNotDirectory - ); - assert_eq!( - fs.create_dir_all("/file.txt/invalid/path").unwrap_err(), - FsError::BaseNotDirectory - ); - } - - #[tokio::test] - async fn touch() { - let fs = MemFS::default(); - - fs.touch("/file.txt").unwrap(); - - assert_eq!(fs.read("/file.txt").await.unwrap(), b""); - } -} diff --git a/lib/vfs/src/lib.rs b/lib/vfs/src/lib.rs index 87545f76965..dba1fa3750f 100644 --- a/lib/vfs/src/lib.rs +++ b/lib/vfs/src/lib.rs @@ -29,8 +29,8 @@ pub mod tmp_fs; pub mod union_fs; pub mod zero_file; // tty_file -> see wasmer_wasi::tty_file -mod filesystem_ext; mod filesystems; +pub mod ops; mod overlay_fs; pub mod pipe; #[cfg(feature = "static-fs")] @@ -45,7 +45,6 @@ pub use builder::*; pub use combine_file::*; pub use dual_write_file::*; pub use empty_fs::*; -pub use filesystem_ext::FileSystemExt; pub use filesystems::FileSystems; pub use null_file::*; pub use overlay_fs::OverlayFileSystem; diff --git a/lib/vfs/src/ops.rs b/lib/vfs/src/ops.rs new file mode 100644 index 00000000000..db6d45616b9 --- /dev/null +++ b/lib/vfs/src/ops.rs @@ -0,0 +1,230 @@ +//! Common [`FileSystem`] operations. + +use std::{collections::VecDeque, path::Path}; + +use tokio::io::{AsyncReadExt, AsyncWriteExt}; + +use crate::{DirEntry, FileSystem, FsError}; + +/// Does this item exists? +pub fn exists(fs: &F, path: impl AsRef) -> bool +where + F: FileSystem + ?Sized, +{ + fs.metadata(path.as_ref()).is_ok() +} + +/// Does this path refer to a directory? +pub fn is_dir(fs: &F, path: impl AsRef) -> bool +where + F: FileSystem + ?Sized, +{ + match fs.metadata(path.as_ref()) { + Ok(meta) => meta.is_dir(), + Err(_) => false, + } +} + +/// Does this path refer to a file? +pub fn is_file(fs: &F, path: impl AsRef) -> bool +where + F: FileSystem + ?Sized, +{ + match fs.metadata(path.as_ref()) { + Ok(meta) => meta.is_file(), + Err(_) => false, + } +} + +/// Make sure a directory (and all its parents) exist. +/// +/// This is analogous to [`std::fs::create_dir_all()`]. +pub fn create_dir_all(fs: &F, path: impl AsRef) -> Result<(), FsError> +where + F: FileSystem + ?Sized, +{ + let path = path.as_ref(); + if let Some(parent) = path.parent() { + create_dir_all(fs, parent)?; + } + + if let Ok(metadata) = fs.metadata(path) { + if metadata.is_dir() { + return Ok(()); + } + if metadata.is_file() { + return Err(FsError::BaseNotDirectory); + } + } + + fs.create_dir(path) +} + +/// Asynchronously write some bytes to a file. +/// +/// This is analogous to [`std::fs::write()`]. +pub async fn write( + fs: &F, + path: impl AsRef + Send, + data: impl AsRef<[u8]> + Send, +) -> Result<(), FsError> +where + F: FileSystem + ?Sized, +{ + let path = path.as_ref(); + let data = data.as_ref(); + + let mut f = fs + .new_open_options() + .create(true) + .truncate(true) + .write(true) + .open(path)?; + + f.write_all(data).await?; + f.flush().await?; + + Ok(()) +} + +/// Asynchronously read a file's contents into memory. +/// +/// This is analogous to [`std::fs::read()`]. +pub async fn read(fs: &F, path: impl AsRef + Send) -> Result, FsError> +where + F: FileSystem + ?Sized, +{ + let mut f = fs.new_open_options().read(true).open(path.as_ref())?; + let mut buffer = Vec::new(); + f.read_to_end(&mut buffer).await?; + + Ok(buffer) +} + +/// Asynchronously read a file's contents into memory as a string. +/// +/// This is analogous to [`std::fs::read_to_string()`]. +pub async fn read_to_string(fs: &F, path: impl AsRef + Send) -> Result +where + F: FileSystem + ?Sized, +{ + let mut f = fs.new_open_options().read(true).open(path.as_ref())?; + let mut buffer = String::new(); + f.read_to_string(&mut buffer).await?; + + Ok(buffer) +} + +/// Update a file's modification and access times, creating the file if it +/// doesn't already exist. +pub fn touch(fs: &F, path: impl AsRef + Send) -> Result<(), FsError> +where + F: FileSystem + ?Sized, +{ + let _ = fs.new_open_options().create(true).write(true).open(path)?; + + Ok(()) +} + +/// Recursively iterate over all paths inside a directory, ignoring any +/// errors that may occur along the way. +pub fn walk(fs: &F, path: impl AsRef) -> Box + '_> +where + F: FileSystem + ?Sized, +{ + let path = path.as_ref(); + let mut dirs_to_visit: VecDeque<_> = fs + .read_dir(path) + .ok() + .into_iter() + .flatten() + .filter_map(|result| result.ok()) + .collect(); + + Box::new(std::iter::from_fn(move || { + let next = dirs_to_visit.pop_back()?; + + if let Ok(children) = fs.read_dir(&next.path) { + dirs_to_visit.extend(children.flatten()); + } + + Some(next) + })) +} + +#[cfg(test)] +mod tests { + use super::*; + use crate::mem_fs::FileSystem as MemFS; + use tokio::io::AsyncReadExt; + + #[tokio::test] + async fn write() { + let fs = MemFS::default(); + + super::write(&fs, "/file.txt", b"Hello, World!") + .await + .unwrap(); + + let mut contents = String::new(); + fs.new_open_options() + .read(true) + .open("/file.txt") + .unwrap() + .read_to_string(&mut contents) + .await + .unwrap(); + assert_eq!(contents, "Hello, World!"); + } + + #[tokio::test] + async fn read() { + let fs = MemFS::default(); + fs.new_open_options() + .create(true) + .write(true) + .open("/file.txt") + .unwrap() + .write_all(b"Hello, World!") + .await + .unwrap(); + + let contents = super::read_to_string(&fs, "/file.txt").await.unwrap(); + assert_eq!(contents, "Hello, World!"); + + let contents = super::read(&fs, "/file.txt").await.unwrap(); + assert_eq!(contents, b"Hello, World!"); + } + + #[tokio::test] + async fn create_dir_all() { + let fs = MemFS::default(); + super::write(&fs, "/file.txt", b"").await.unwrap(); + + assert!(!super::exists(&fs, "/really/nested/directory")); + super::create_dir_all(&fs, "/really/nested/directory").unwrap(); + assert!(super::exists(&fs, "/really/nested/directory")); + + // It's okay to create the same directory multiple times + super::create_dir_all(&fs, "/really/nested/directory").unwrap(); + + // You can't create a directory on top of a file + assert_eq!( + super::create_dir_all(&fs, "/file.txt").unwrap_err(), + FsError::BaseNotDirectory + ); + assert_eq!( + super::create_dir_all(&fs, "/file.txt/invalid/path").unwrap_err(), + FsError::BaseNotDirectory + ); + } + + #[tokio::test] + async fn touch() { + let fs = MemFS::default(); + + super::touch(&fs, "/file.txt").unwrap(); + + assert_eq!(super::read(&fs, "/file.txt").await.unwrap(), b""); + } +} diff --git a/lib/vfs/src/overlay_fs.rs b/lib/vfs/src/overlay_fs.rs index 35627dc82d7..e454d653268 100644 --- a/lib/vfs/src/overlay_fs.rs +++ b/lib/vfs/src/overlay_fs.rs @@ -1,8 +1,8 @@ use std::{fmt::Debug, path::Path}; use crate::{ - FileOpener, FileSystem, FileSystemExt, FileSystems, FsError, Metadata, OpenOptions, - OpenOptionsConfig, ReadDir, VirtualFile, + ops, FileOpener, FileSystem, FileSystems, FsError, Metadata, OpenOptions, OpenOptionsConfig, + ReadDir, VirtualFile, }; /// A primary filesystem and chain of secondary filesystems that are overlayed @@ -90,7 +90,7 @@ where fn permission_error_or_not_found(&self, path: &Path) -> Result<(), FsError> { for fs in self.secondaries.filesystems() { - if fs.exists(path) { + if ops::exists(fs, path) { return Err(FsError::PermissionDenied); } } @@ -217,18 +217,18 @@ where other => return other, } - if (conf.create || conf.create_new) && !self.exists(path) { + if (conf.create || conf.create_new) && !ops::exists(self, path) { if let Some(parent) = path.parent() { let parent_exists_on_secondary_fs = self .secondaries .filesystems() .into_iter() - .any(|fs| fs.is_dir(parent)); + .any(|fs| ops::is_dir(fs, parent)); if parent_exists_on_secondary_fs { // We fall into the special case where you can create a file // that looks like it is inside a secondary filesystem folder, // but actually it gets created on the host - self.primary.create_dir_all(parent)?; + ops::create_dir_all(&self.primary, parent)?; return self .primary .new_open_options() @@ -272,7 +272,7 @@ where let already_exists = secondaries .filesystems() .into_iter() - .any(|fs| fs.is_file(path)); + .any(|fs| ops::is_file(fs, path)); if !already_exists { return true; @@ -324,9 +324,7 @@ mod tests { use tokio::io::AsyncWriteExt; use super::*; - use crate::{ - mem_fs::FileSystem as MemFS, webc_fs::WebcFileSystem, FileSystemExt, RootFileSystemBuilder, - }; + use crate::{mem_fs::FileSystem as MemFS, webc_fs::WebcFileSystem, RootFileSystemBuilder}; #[test] fn object_safe() { @@ -375,7 +373,7 @@ mod tests { FsError::EntryNotFound, "Deleted from primary" ); - assert!(!overlay.secondaries[0].exists(&second)); + assert!(!ops::exists(&overlay.secondaries[0], &second)); // Directory on the primary fs isn't empty assert_eq!( @@ -388,22 +386,21 @@ mod tests { overlay.remove_dir(third).unwrap_err(), FsError::PermissionDenied, ); - assert!(overlay.secondaries[0].exists(third)); + assert!(ops::exists(&overlay.secondaries[0], third)); } #[tokio::test] async fn open_files() { let primary = MemFS::default(); let secondary = MemFS::default(); - primary.create_dir_all("/primary").unwrap(); - primary.touch("/primary/read.txt").unwrap(); - primary.touch("/primary/write.txt").unwrap(); - secondary.create_dir_all("/secondary").unwrap(); - secondary.touch("/secondary/read.txt").unwrap(); - secondary.touch("/secondary/write.txt").unwrap(); - secondary.create_dir_all("/primary").unwrap(); - secondary - .write("/primary/read.txt", "This is shadowed") + ops::create_dir_all(&primary, "/primary").unwrap(); + ops::touch(&primary, "/primary/read.txt").unwrap(); + ops::touch(&primary, "/primary/write.txt").unwrap(); + ops::create_dir_all(&secondary, "/secondary").unwrap(); + ops::touch(&secondary, "/secondary/read.txt").unwrap(); + ops::touch(&secondary, "/secondary/write.txt").unwrap(); + ops::create_dir_all(&secondary, "/primary").unwrap(); + ops::write(&secondary, "/primary/read.txt", "This is shadowed") .await .unwrap(); @@ -416,8 +413,8 @@ mod tests { .write(true) .open("/new.txt") .unwrap(); - assert!(fs.primary.exists("/new.txt")); - assert!(!fs.secondaries[0].exists("/new.txt")); + assert!(ops::exists(&fs.primary, "/new.txt")); + assert!(!ops::exists(&fs.secondaries[0], "/new.txt")); // You can open a file for reading and writing on the primary fs let _ = fs @@ -429,7 +426,7 @@ mod tests { .unwrap(); // Files on the primary should always shadow the secondary - let content = fs.read_to_string("/primary/read.txt").await.unwrap(); + let content = ops::read_to_string(&fs, "/primary/read.txt").await.unwrap(); assert_ne!(content, "This is shadowed"); } @@ -437,15 +434,15 @@ mod tests { fn create_file_that_looks_like_it_is_in_a_secondary_filesystem_folder() { let primary = MemFS::default(); let secondary = MemFS::default(); - secondary.create_dir_all("/path/to/").unwrap(); - assert!(!primary.is_dir("/path/to/")); + ops::create_dir_all(&secondary, "/path/to/").unwrap(); + assert!(!ops::is_dir(&primary, "/path/to/")); let fs = OverlayFileSystem::new(primary, [secondary]); - fs.touch("/path/to/file.txt").unwrap(); + ops::touch(&fs, "/path/to/file.txt").unwrap(); - assert!(fs.primary.is_dir("/path/to/")); - assert!(fs.primary.is_file("/path/to/file.txt")); - assert!(!fs.secondaries[0].is_file("/path/to/file.txt")); + assert!(ops::is_dir(&fs.primary, "/path/to/")); + assert!(ops::is_file(&fs.primary, "/path/to/file.txt")); + assert!(!ops::is_file(&fs.secondaries[0], "/path/to/file.txt")); } #[tokio::test] @@ -453,22 +450,20 @@ mod tests { let primary = MemFS::default(); let secondary = MemFS::default(); let secondary_overlayed = MemFS::default(); - primary.create_dir_all("/primary").unwrap(); - primary.touch("/primary/read.txt").unwrap(); - primary.touch("/primary/write.txt").unwrap(); - secondary.create_dir_all("/secondary").unwrap(); - secondary.touch("/secondary/read.txt").unwrap(); - secondary.touch("/secondary/write.txt").unwrap(); + ops::create_dir_all(&primary, "/primary").unwrap(); + ops::touch(&primary, "/primary/read.txt").unwrap(); + ops::touch(&primary, "/primary/write.txt").unwrap(); + ops::create_dir_all(&secondary, "/secondary").unwrap(); + ops::touch(&secondary, "/secondary/read.txt").unwrap(); + ops::touch(&secondary, "/secondary/write.txt").unwrap(); // This second "secondary" filesystem should share the same folders as // the first one. - secondary_overlayed.create_dir_all("/secondary").unwrap(); - secondary_overlayed - .touch("/secondary/overlayed.txt") - .unwrap(); + ops::create_dir_all(&secondary_overlayed, "/secondary").unwrap(); + ops::touch(&secondary_overlayed, "/secondary/overlayed.txt").unwrap(); let fs = OverlayFileSystem::new(primary, [secondary, secondary_overlayed]); - let paths: Vec<_> = fs.walk("/").map(|entry| entry.path()).collect(); + let paths: Vec<_> = ops::walk(&fs, "/").map(|entry| entry.path()).collect(); assert_eq!( paths, vec![ @@ -515,13 +510,13 @@ mod tests { let fs = OverlayFileSystem::new(primary, [webc]); // We should get all the normal directories from rootfs (primary) - assert!(fs.is_dir("/lib")); - assert!(fs.is_dir("/bin")); - assert!(fs.is_file("/dev/stdin")); - assert!(fs.is_file("/dev/stdout")); + assert!(ops::is_dir(&fs, "/lib")); + assert!(ops::is_dir(&fs, "/bin")); + assert!(ops::is_file(&fs, "/dev/stdin")); + assert!(ops::is_file(&fs, "/dev/stdout")); // We also want to see files from the WEBC volumes (secondary) - assert!(fs.is_dir("/lib/python3.6")); - assert!(fs.is_file("/lib/python3.6/collections/__init__.py")); + assert!(ops::is_dir(&fs, "/lib/python3.6")); + assert!(ops::is_file(&fs, "/lib/python3.6/collections/__init__.py")); // files on a secondary fs aren't writable assert_eq!( fs.new_open_options() @@ -532,41 +527,48 @@ mod tests { ); // you are allowed to create files that look like they are in a secondary // folder, though - fs.touch("/lib/python3.6/collections/something-else.py") - .unwrap(); + ops::touch(&fs, "/lib/python3.6/collections/something-else.py").unwrap(); // But it'll be on the primary filesystem, not the secondary one - assert!(fs - .primary - .is_file("/lib/python3.6/collections/something-else.py")); - assert!(!fs.secondaries[0].is_file("/lib/python3.6/collections/something-else.py")); + assert!(ops::is_file( + &fs.primary, + "/lib/python3.6/collections/something-else.py" + )); + assert!(!ops::is_file( + &fs.secondaries[0], + "/lib/python3.6/collections/something-else.py" + )); // You can do the same thing with folders fs.create_dir("/lib/python3.6/something-else".as_ref()) .unwrap(); - assert!(fs.primary.is_dir("/lib/python3.6/something-else")); - assert!(!fs.secondaries[0].is_dir("/lib/python3.6/something-else")); + assert!(ops::is_dir(&fs.primary, "/lib/python3.6/something-else")); + assert!(!ops::is_dir( + &fs.secondaries[0], + "/lib/python3.6/something-else" + )); // It only works when you are directly inside an existing directory // on the secondary filesystem, though assert_eq!( - fs.touch("/lib/python3.6/collections/this/doesnt/exist.txt") - .unwrap_err(), + ops::touch(&fs, "/lib/python3.6/collections/this/doesnt/exist.txt").unwrap_err(), FsError::EntryNotFound ); // you should also be able to read files mounted from the host - assert!(fs.is_dir("/first")); - assert!(fs.is_file("/first/file.txt")); + assert!(ops::is_dir(&fs, "/first")); + assert!(ops::is_file(&fs, "/first/file.txt")); assert_eq!( - fs.read_to_string("/first/file.txt").await.unwrap(), + ops::read_to_string(&fs, "/first/file.txt").await.unwrap(), "First!" ); // Overwriting them is fine and we'll see the changes on the host - fs.write("/first/file.txt", "Updated").await.unwrap(); + ops::write(&fs, "/first/file.txt", "Updated").await.unwrap(); assert_eq!(std::fs::read_to_string(&file_txt).unwrap(), "Updated"); // The filesystem will see changes on the host that happened after it was // set up let another = second.join("another.txt"); std::fs::write(&another, "asdf").unwrap(); assert_eq!( - fs.read_to_string("/second/another.txt").await.unwrap(), + ops::read_to_string(&fs, "/second/another.txt") + .await + .unwrap(), "asdf" ); } From 6fb20e4981b08e8db51dac9bb7c53c5b328173a4 Mon Sep 17 00:00:00 2001 From: Michael-F-Bryan Date: Thu, 16 Mar 2023 18:08:36 +0800 Subject: [PATCH 12/12] Applied suggestions from @theduke --- lib/vfs/src/lib.rs | 2 +- lib/vfs/src/mem_fs/filesystem.rs | 19 ++++----- lib/vfs/src/ops.rs | 1 + lib/vfs/src/overlay_fs.rs | 70 +++++++++++++++++++++++++------- lib/vfs/src/union_fs.rs | 15 ++++--- 5 files changed, 73 insertions(+), 34 deletions(-) diff --git a/lib/vfs/src/lib.rs b/lib/vfs/src/lib.rs index dba1fa3750f..1f0fd36f5d9 100644 --- a/lib/vfs/src/lib.rs +++ b/lib/vfs/src/lib.rs @@ -30,7 +30,7 @@ pub mod union_fs; pub mod zero_file; // tty_file -> see wasmer_wasi::tty_file mod filesystems; -pub mod ops; +pub(crate) mod ops; mod overlay_fs; pub mod pipe; #[cfg(feature = "static-fs")] diff --git a/lib/vfs/src/mem_fs/filesystem.rs b/lib/vfs/src/mem_fs/filesystem.rs index 50a947eb4f7..92a505ad447 100644 --- a/lib/vfs/src/mem_fs/filesystem.rs +++ b/lib/vfs/src/mem_fs/filesystem.rs @@ -12,9 +12,8 @@ use std::sync::{Arc, RwLock}; /// The in-memory file system! /// -/// It's a thin wrapper around [`FileSystemInner`]. This `FileSystem` -/// type can be cloned, it's a light copy of the `FileSystemInner` -/// (which is behind a `Arc` + `RwLock`. +/// This `FileSystem` type can be cloned, it's a light copy of the +/// `FileSystemInner` (which is behind a `Arc` + `RwLock`). #[derive(Clone, Default)] pub struct FileSystem { pub(super) inner: Arc>, @@ -955,7 +954,7 @@ impl DirectoryMustBeEmpty { #[cfg(test)] mod test_filesystem { - use crate::{mem_fs::*, DirEntry, FileSystem as FS, FileSystemExt, FileType, FsError}; + use crate::{mem_fs::*, ops, DirEntry, FileSystem as FS, FileType, FsError}; macro_rules! path { ($path:expr) => { @@ -1707,9 +1706,9 @@ mod test_filesystem { #[ignore = "Not yet supported. See https://github.com/wasmerio/wasmer/issues/3678"] fn mount_to_overlapping_directories() { let top_level = FileSystem::default(); - top_level.touch("/file.txt").unwrap(); + ops::touch(&top_level, "/file.txt").unwrap(); let nested = FileSystem::default(); - nested.touch("/another-file.txt").unwrap(); + ops::touch(&nested, "/another-file.txt").unwrap(); let top_level: Arc = Arc::new(top_level); let nested: Arc = Arc::new(nested); @@ -1719,9 +1718,9 @@ mod test_filesystem { fs.mount("/top-level/nested".into(), &nested, "/".into()) .unwrap(); - assert!(fs.is_dir("/top-level")); - assert!(fs.is_file("/top-level/file.txt")); - assert!(fs.is_dir("/top-level/nested")); - assert!(fs.is_file("/top-level/nested/another-file.txt")); + assert!(ops::is_dir(&fs, "/top-level")); + assert!(ops::is_file(&fs, "/top-level/file.txt")); + assert!(ops::is_dir(&fs, "/top-level/nested")); + assert!(ops::is_file(&fs, "/top-level/nested/another-file.txt")); } } diff --git a/lib/vfs/src/ops.rs b/lib/vfs/src/ops.rs index db6d45616b9..2c124d9b8e7 100644 --- a/lib/vfs/src/ops.rs +++ b/lib/vfs/src/ops.rs @@ -1,4 +1,5 @@ //! Common [`FileSystem`] operations. +#![allow(dead_code)] // Most of these helpers are used during testing use std::{collections::VecDeque, path::Path}; diff --git a/lib/vfs/src/overlay_fs.rs b/lib/vfs/src/overlay_fs.rs index e454d653268..b1e96a8bd18 100644 --- a/lib/vfs/src/overlay_fs.rs +++ b/lib/vfs/src/overlay_fs.rs @@ -120,7 +120,14 @@ where } had_at_least_one_success = true; } - Err(e) if should_continue(e) => continue, + Err(e) + if { + let e = e; + matches!(e, FsError::EntryNotFound) + } => + { + continue + } Err(e) => return Err(e), } } @@ -141,7 +148,11 @@ where fn create_dir(&self, path: &Path) -> Result<(), FsError> { match self.primary.create_dir(path) { - Err(e) if should_continue(e) => {} + Err(e) + if { + let e = e; + matches!(e, FsError::EntryNotFound) + } => {} other => return other, } @@ -150,7 +161,11 @@ where fn remove_dir(&self, path: &Path) -> Result<(), FsError> { match self.primary.remove_dir(path) { - Err(e) if should_continue(e) => {} + Err(e) + if { + let e = e; + matches!(e, FsError::EntryNotFound) + } => {} other => return other, } @@ -159,7 +174,11 @@ where fn rename(&self, from: &Path, to: &Path) -> Result<(), FsError> { match self.primary.rename(from, to) { - Err(e) if should_continue(e) => {} + Err(e) + if { + let e = e; + matches!(e, FsError::EntryNotFound) + } => {} other => return other, } @@ -169,13 +188,24 @@ where fn metadata(&self, path: &Path) -> Result { match self.primary.metadata(path) { Ok(meta) => return Ok(meta), - Err(e) if should_continue(e) => {} + Err(e) + if { + let e = e; + matches!(e, FsError::EntryNotFound) + } => {} Err(e) => return Err(e), } for fs in self.secondaries.filesystems() { match fs.metadata(path) { - Err(e) if should_continue(e) => continue, + Err(e) + if { + let e = e; + matches!(e, FsError::EntryNotFound) + } => + { + continue + } other => return other, } } @@ -185,7 +215,11 @@ where fn remove_file(&self, path: &Path) -> Result<(), FsError> { match self.primary.remove_file(path) { - Err(e) if should_continue(e) => {} + Err(e) + if { + let e = e; + matches!(e, FsError::EntryNotFound) + } => {} other => return other, } @@ -213,7 +247,11 @@ where .options(conf.clone()) .open(path) { - Err(e) if should_continue(e) => {} + Err(e) + if { + let e = e; + matches!(e, FsError::EntryNotFound) + } => {} other => return other, } @@ -246,7 +284,14 @@ where for fs in self.secondaries.filesystems() { match fs.new_open_options().options(conf.clone()).open(path) { - Err(e) if should_continue(e) => continue, + Err(e) + if { + let e = e; + matches!(e, FsError::EntryNotFound) + } => + { + continue + } other => return other, } } @@ -304,18 +349,13 @@ where } } - f.debug_struct("FileSystem") + f.debug_struct("OverlayFileSystem") .field("primary", &self.primary) .field("secondaries", &IterFilesystems(&self.secondaries)) .finish() } } -/// Is it okay to use a fallback filesystem to deal with this particular error? -fn should_continue(e: FsError) -> bool { - matches!(e, FsError::EntryNotFound) -} - #[cfg(test)] mod tests { use std::{path::PathBuf, sync::Arc}; diff --git a/lib/vfs/src/union_fs.rs b/lib/vfs/src/union_fs.rs index 5908558ac79..9685f588110 100644 --- a/lib/vfs/src/union_fs.rs +++ b/lib/vfs/src/union_fs.rs @@ -465,8 +465,7 @@ mod tests { use tokio::io::AsyncWriteExt; use crate::{ - host_fs::FileSystem, mem_fs, FileSystem as FileSystemTrait, FileSystemExt, FsError, - UnionFileSystem, + host_fs::FileSystem, mem_fs, ops, FileSystem as FileSystemTrait, FsError, UnionFileSystem, }; fn gen_filesystem() -> UnionFileSystem { @@ -1078,9 +1077,9 @@ mod tests { #[ignore = "Not yet supported. See https://github.com/wasmerio/wasmer/issues/3678"] fn mount_to_overlapping_directories() { let top_level = mem_fs::FileSystem::default(); - top_level.touch("/file.txt").unwrap(); + ops::touch(&top_level, "/file.txt").unwrap(); let nested = mem_fs::FileSystem::default(); - nested.touch("/another-file.txt").unwrap(); + ops::touch(&nested, "/another-file.txt").unwrap(); let mut fs = UnionFileSystem::default(); fs.mount( @@ -1098,9 +1097,9 @@ mod tests { Some("/top-level/nested"), ); - assert!(fs.is_dir("/top-level")); - assert!(fs.is_file("/top-level/file.txt")); - assert!(fs.is_dir("/top-level/nested")); - assert!(fs.is_file("/top-level/nested/another-file.txt")); + assert!(ops::is_dir(&fs, "/top-level")); + assert!(ops::is_file(&fs, "/top-level/file.txt")); + assert!(ops::is_dir(&fs, "/top-level/nested")); + assert!(ops::is_file(&fs, "/top-level/nested/another-file.txt")); } }