Skip to content

Commit

Permalink
Make sure packages from "--uses" are loaded into "/bin"
Browse files Browse the repository at this point in the history
  • Loading branch information
Michael-F-Bryan committed May 22, 2023
1 parent fff6df5 commit 549df13
Show file tree
Hide file tree
Showing 3 changed files with 98 additions and 47 deletions.
11 changes: 8 additions & 3 deletions lib/wasi/src/state/builder.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -105,7 +105,7 @@ pub enum WasiStateCreationError {
#[error("wasi filesystem setup error: `{0}`")]
WasiFsSetupError(String),
#[error(transparent)]
FileSystemError(FsError),
FileSystemError(#[from] FsError),
#[error("wasi inherit error: `{0}`")]
WasiInheritError(String),
#[error("wasi include package: `{0}`")]
Expand Down Expand Up @@ -788,7 +788,7 @@ impl WasiEnvBuilder {
let start = instance.exports.get_function("_start")?;

env.data(store).thread.set_status_running();
let res = crate::run_wasi_func_start(start, store);
let mut res = crate::run_wasi_func_start(start, store);

tracing::trace!(
"wasi[{}:{}]::main exit (code = {:?})",
Expand All @@ -800,7 +800,12 @@ impl WasiEnvBuilder {
let exit_code = match &res {
Ok(_) => Errno::Success.into(),
Err(err) => match err.as_exit_code() {
Some(code) if code.is_success() => Errno::Success.into(),
Some(code) if code.is_success() => {
// This is actually not an error, so we need to fix up the
// result
res = Ok(());
Errno::Success.into()
}
Some(other) => other,
None => Errno::Noexec.into(),
},
Expand Down
124 changes: 83 additions & 41 deletions lib/wasi/src/state/env.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ use std::{
use derivative::Derivative;
use rand::Rng;
use tracing::{trace, warn};
use virtual_fs::{FileSystem, FsError, VirtualFile};
use virtual_fs::{AsyncWriteExt, FileSystem, FsError, VirtualFile};
use virtual_net::DynVirtualNetworking;
use wasmer::{
AsStoreMut, AsStoreRef, FunctionEnvMut, Global, Instance, Memory, MemoryType, MemoryView,
Expand Down Expand Up @@ -855,52 +855,94 @@ impl WasiEnv {

/// Make all the commands in a [`BinaryPackage`] available to the WASI
/// instance.
///
/// The [`BinaryPackageCommand::atom()`] will be saved to `/bin/command`.
///
/// This will also merge the command's filesystem
/// ([`BinaryPackage::webc_fs`]) into the current filesystem.
pub fn use_package(&self, pkg: &BinaryPackage) -> Result<(), WasiStateCreationError> {
if let WasiFsRoot::Sandbox(root_fs) = &self.state.fs.root_fs {
// We first need to copy any files in the package over to the temporary file system
root_fs.union(&pkg.webc_fs);
// PERF: We should avoid all these copies in the WasiFsRoot::Backing case.

let root_fs = &self.state.fs.root_fs;
// We first need to copy any files in the package over to the
// temporary file system
match root_fs {
WasiFsRoot::Sandbox(root_fs) => {
root_fs.union(&pkg.webc_fs);
}
WasiFsRoot::Backing(_fs) => {
// TODO: Manually copy each file across one-by-one
}
}

// Next, make sure all commands will be available
// Next, make sure all commands will be available

if !pkg.commands.is_empty() {
let _ = root_fs.create_dir(Path::new("/bin"));
if !pkg.commands.is_empty() {
let _ = root_fs.create_dir(Path::new("/bin"));

for command in &pkg.commands {
let path = format!("/bin/{}", command.name());
let path = Path::new(path.as_str());

// FIXME(Michael-F-Bryan): This is pretty sketchy.
// We should be using some sort of reference-counted
// pointer to some bytes that are either on the heap
// or from a memory-mapped file. However, that's not
// possible here because things like memfs and
// WasiEnv are expecting a Cow<'static, [u8]>. It's
// too hard to refactor those at the moment, and we
// were pulling the same trick before by storing an
// "ownership" object in the BinaryPackageCommand,
// so as long as packages aren't removed from the
// module cache it should be fine.
// See https://github.com/wasmerio/wasmer/issues/3875
let atom: &'static [u8] = unsafe { std::mem::transmute(command.atom()) };

if let Err(err) = root_fs
.new_open_options_ext()
.insert_ro_file(path, atom.into())
{
tracing::debug!(
"failed to add package [{}] command [{}] - {}",
pkg.package_name,
command.name(),
err
);
continue;
}
for command in &pkg.commands {
let path = format!("/bin/{}", command.name());
let path = Path::new(path.as_str());

let mut package = pkg.clone();
package.entrypoint_cmd = Some(command.name().to_string());
self.bin_factory
.set_binary(path.as_os_str().to_string_lossy().as_ref(), package);
match root_fs {
WasiFsRoot::Sandbox(root_fs) => {
// As a short-cut, when we are using a TmpFileSystem
// we can (unsafely) add the file to the filesystem
// without any copying.

// FIXME(Michael-F-Bryan): This is pretty sketchy.
// We should be using some sort of reference-counted
// pointer to some bytes that are either on the heap
// or from a memory-mapped file. However, that's not
// possible here because things like memfs and
// WasiEnv are expecting a Cow<'static, [u8]>. It's
// too hard to refactor those at the moment, and we
// were pulling the same trick before by storing an
// "ownership" object in the BinaryPackageCommand,
// so as long as packages aren't removed from the
// module cache it should be fine.
// See https://github.com/wasmerio/wasmer/issues/3875
let atom: &'static [u8] = unsafe { std::mem::transmute(command.atom()) };

if let Err(err) = root_fs
.new_open_options_ext()
.insert_ro_file(path, atom.into())
{
tracing::debug!(
"failed to add package [{}] command [{}] - {}",
pkg.package_name,
command.name(),
err
);
continue;
}
}
WasiFsRoot::Backing(fs) => {
// Looks like we need to make the copy
let mut f = fs.new_open_options().create(true).write(true).open(path)?;
self.tasks()
.block_on(f.write_all(command.atom()))
.map_err(|e| {
WasiStateCreationError::WasiIncludePackageError(format!(
"Unable to save \"{}\" to \"{}\": {e}",
command.name(),
path.display()
))
})?;
}
}

let mut package = pkg.clone();
package.entrypoint_cmd = Some(command.name().to_string());
self.bin_factory
.set_binary(path.as_os_str().to_string_lossy().as_ref(), package);

tracing::debug!(
package=%pkg.package_name,
command_name=command.name(),
path=%path.display(),
"Injected a command into the filesystem",
);
}
}

Expand Down
10 changes: 7 additions & 3 deletions tests/integration/cli/tests/run_unstable.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -21,8 +21,8 @@ const HTTP_GET_TIMEOUT: Duration = Duration::from_secs(5);
static RUST_LOG: Lazy<String> = Lazy::new(|| {
[
"info",
"wasmer_wasi::resolve=debug",
"wasmer_wasi::runners=debug",
"wasmer_wasix::resolve=debug",
"wasmer_wasix::runners=debug",
"virtual_fs::trace_fs=trace",
]
.join(",")
Expand Down Expand Up @@ -375,7 +375,11 @@ mod remote_webc {
.arg("ls /bin")
.assert();

assert.success().stdout(contains("Hello, World!"));
let some_expected_binaries = [
"arch", "base32", "base64", "baseenc", "basename", "bash", "cat",
]
.join("\n");
assert.success().stdout(contains(some_expected_binaries));
}
}

Expand Down

0 comments on commit 549df13

Please sign in to comment.