Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix File System Merging Problems #4205

Merged
merged 2 commits into from
Sep 6, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
24 changes: 22 additions & 2 deletions lib/virtual-fs/src/empty_fs.rs
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,14 @@ pub struct EmptyFileSystem {}
#[allow(unused_variables)]
impl FileSystem for EmptyFileSystem {
fn read_dir(&self, path: &Path) -> Result<ReadDir> {
Err(FsError::EntryNotFound)
// Special-case the root path by returning an empty iterator.
// An empty file system should still be readable, just not contain
// any entries.
if path == Path::new("/") {
Ok(ReadDir::new(Vec::new()))
} else {
Err(FsError::EntryNotFound)
}
}

fn create_dir(&self, path: &Path) -> Result<()> {
Expand All @@ -29,7 +36,20 @@ impl FileSystem for EmptyFileSystem {
}

fn metadata(&self, path: &Path) -> Result<Metadata> {
Err(FsError::EntryNotFound)
// Special-case the root path by returning an stub value.
// An empty file system should still be readable, just not contain
// any entries.
if path == Path::new("/") {
Ok(Metadata {
ft: FileType::new_dir(),
accessed: 0,
created: 0,
modified: 0,
len: 0,
})
} else {
Err(FsError::EntryNotFound)
}
}

fn symlink_metadata(&self, path: &Path) -> Result<Metadata> {
Expand Down
7 changes: 7 additions & 0 deletions lib/virtual-fs/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -665,6 +665,13 @@ pub struct FileType {
}

impl FileType {
pub fn new_dir() -> Self {
Self {
dir: true,
..Default::default()
}
}

pub fn is_dir(&self) -> bool {
self.dir
}
Expand Down
8 changes: 7 additions & 1 deletion lib/wasix/src/fs/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -363,7 +363,13 @@ async fn merge_filesystems(
to_check.push_back(PathBuf::from("/"));

while let Some(path) = to_check.pop_front() {
let metadata = source.metadata(&path)?;
let metadata = match source.metadata(&path) {
Ok(m) => m,
Err(err) => {
tracing::debug!(path=%path.display(), source_fs=?source, ?err, "failed to get metadata for path while merging file systems");
return Err(err);
}
};

if metadata.is_dir() {
create_dir_all(destination, &path)?;
Expand Down
37 changes: 32 additions & 5 deletions lib/wasix/src/os/console/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -222,15 +222,14 @@ impl Console {
.with_envs(self.env.clone().into_iter())
.with_args(args)
.with_capabilities(self.capabilities.clone())
.with_stdin(Box::new(self.stdin.clone()))
.with_stdout(Box::new(self.stdout.clone()))
.with_stderr(Box::new(self.stderr.clone()))
.prepare_webc_env(prog, &wasi_opts, &pkg, self.runtime.clone(), Some(root_fs))
// TODO: better error conversion
.map_err(|err| SpawnError::Other(err.into()))?;

let env = builder
.stdin(Box::new(self.stdin.clone()))
.stdout(Box::new(self.stdout.clone()))
.stderr(Box::new(self.stderr.clone()))
.build()?;
let env = builder.build()?;

// Display the welcome message
if !self.whitelabel && !self.no_welcome {
Expand Down Expand Up @@ -365,4 +364,32 @@ mod tests {

assert_eq!(out, "hello VAL1\n");
}

/// Regression test to ensure merging of multiple packages works correctly.
#[test]
fn test_console_python_merge() {
std::env::set_var("RUST_LOG", "wasmer_wasix=trace");
tracing_subscriber::fmt::init();
let tokio_rt = tokio::runtime::Runtime::new().unwrap();
let rt_handle = tokio_rt.handle().clone();
let _guard = rt_handle.enter();

let tm = TokioTaskManager::new(tokio_rt);
let mut rt = PluggableRuntime::new(Arc::new(tm));
rt.set_engine(Some(wasmer::Engine::default()))
.set_package_loader(BuiltinPackageLoader::from_env().unwrap());

let cmd = "wasmer-tests/python-env-dump --help";

let (mut handle, _proc) = Console::new(cmd, Arc::new(rt)).run().unwrap();

let code = rt_handle
.block_on(async move {
let res = handle.wait_finished().await?;
Ok::<_, anyhow::Error>(res)
})
.unwrap();

assert_eq!(code.raw(), 0);
}
}
1 change: 1 addition & 0 deletions lib/wasix/src/state/env.rs
Original file line number Diff line number Diff line change
Expand Up @@ -869,6 +869,7 @@ impl WasiEnv {
/// [cmd-atom]: crate::bin_factory::BinaryPackageCommand::atom()
/// [pkg-fs]: crate::bin_factory::BinaryPackage::webc_fs
pub fn use_package(&self, pkg: &BinaryPackage) -> Result<(), WasiStateCreationError> {
tracing::trace!(packagae=%pkg.package_name, "merging package dependency into wasi environment");
let root_fs = &self.state.fs.root_fs;

// We first need to copy any files in the package over to the
Expand Down