Skip to content
Open
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
2 changes: 1 addition & 1 deletion crates/spfs-cli/cmd-clean/src/cmd_clean.rs
Original file line number Diff line number Diff line change
Expand Up @@ -167,7 +167,7 @@ impl CmdClean {
// durable upper path's workdir that have 'd---------' permissions
// from overlayfs.
std::fs::remove_dir_all(durable_path.clone())
.map_err(|err| spfs::Error::RuntimeWriteError(durable_path, err))?;
.map_err(|err| spfs::runtime::Error::RuntimeWriteError(durable_path, err))?;
}
return Ok(0);
}
Expand Down
7 changes: 4 additions & 3 deletions crates/spfs-cli/cmd-render/src/cmd_render.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ use clap::Parser;
use clap::builder::TypedValueParser;
use miette::{Context, Result};
use spfs::prelude::*;
use spfs::runtime::Error as RuntimeError;
use spfs::storage::fallback::FallbackProxy;
use spfs::{Error, RenderResult, graph};
use spfs_cli_common::{self as cli, CommandName, HasRepositoryArgs};
Expand Down Expand Up @@ -97,15 +98,15 @@ impl CmdRender {
) -> Result<RenderResult> {
tokio::fs::create_dir_all(&target)
.await
.map_err(|err| Error::RuntimeWriteError(target.to_owned(), err))?;
.map_err(|err| RuntimeError::RuntimeWriteError(target.to_owned(), err))?;
let target_dir = tokio::task::block_in_place(|| dunce::canonicalize(target))
.map_err(|err| Error::InvalidPath(target.to_owned(), err))?;
if tokio::fs::read_dir(&target_dir)
.await
.map_err(|err| Error::RuntimeReadError(target_dir.clone(), err))?
.map_err(|err| RuntimeError::RuntimeReadError(target_dir.clone(), err))?
.next_entry()
.await
.map_err(|err| Error::RuntimeReadError(target_dir.clone(), err))?
.map_err(|err| RuntimeError::RuntimeReadError(target_dir.clone(), err))?
Comment on lines +101 to +109
Copy link

Copilot AI Feb 24, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Using RuntimeError::RuntimeWriteError and RuntimeError::RuntimeReadError for general directory operations in the render command is semantically incorrect. These errors are intended for runtime-specific filesystem operations (e.g., operations on runtime directories like upper_dir, work_dir, etc.), not for general file I/O operations during rendering. Consider using a different error type or the existing Error::StorageWriteError/Error::StorageReadError, or create more appropriate error variants for render operations.

Copilot uses AI. Check for mistakes.
.is_some()
&& !self.allow_existing
{
Expand Down
9 changes: 6 additions & 3 deletions crates/spfs-cli/common/src/args.rs
Original file line number Diff line number Diff line change
Expand Up @@ -679,7 +679,10 @@ macro_rules! handle_result {
tracing::error!("Out of disk space: {msg}");
Ok(1)
}
Some(spfs::Error::RuntimeWriteError(path, io_err))
Some(spfs::Error::Runtime(spfs::runtime::Error::RuntimeWriteError(
path,
io_err,
)))
| Some(spfs::Error::StorageWriteError(_, path, io_err))
if std::matches!(io_err.os_error(), Some($crate::__private::libc::ENOSPC)) =>
{
Expand All @@ -706,11 +709,11 @@ macro_rules! handle_result {

pub fn capture_if_relevant(err: &Error) {
match err.root_cause().downcast_ref::<spfs::Error>() {
Some(spfs::Error::NoActiveRuntime) => (),
Some(spfs::Error::Runtime(spfs::runtime::Error::NoActiveRuntime)) => (),
Some(spfs::Error::UnknownObject(_)) => (),
Some(spfs::Error::UnknownReference(_)) => (),
Some(spfs::Error::AmbiguousReference(_)) => (),
Some(spfs::Error::NothingToCommit) => (),
Some(spfs::Error::Runtime(spfs::runtime::Error::NothingToCommit)) => (),
_ => {
#[cfg(feature = "sentry")]
sentry_miette::capture_miette(err);
Expand Down
2 changes: 1 addition & 1 deletion crates/spfs-cli/main/src/cmd_commit.rs
Original file line number Diff line number Diff line change
Expand Up @@ -134,7 +134,7 @@ impl CmdCommit {
if let Some(path) = &self.path {
let manifest = committer.commit_dir(path).await?;
if manifest.is_empty() && !self.allow_empty {
return Err(spfs::Error::NothingToCommit);
return Err(spfs::runtime::Error::NothingToCommit.into());
}
return Ok(repo
.create_layer(&manifest.to_graph_manifest())
Expand Down
4 changes: 2 additions & 2 deletions crates/spfs-cli/main/src/cmd_edit.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@

use clap::Args;
use miette::Result;
use spfs::Error;
use spfs::runtime::Error as RuntimeError;

/// Make the current runtime editable
#[derive(Debug, Args)]
Expand Down Expand Up @@ -38,7 +38,7 @@ impl CmdEdit {
if !self.off {
match spfs::make_active_runtime_editable().await {
Ok(_) => tracing::info!("edit mode enabled"),
Err(Error::RuntimeAlreadyEditable) => {}
Err(spfs::Error::Runtime(RuntimeError::RuntimeAlreadyEditable)) => {}
Err(err) => {
return Err(err.into());
}
Expand Down
2 changes: 1 addition & 1 deletion crates/spfs-cli/main/src/cmd_info.rs
Original file line number Diff line number Diff line change
Expand Up @@ -272,7 +272,7 @@ impl CmdInfo {
let found = match spfs::find_path::find_path_providers_in_spfs_runtime(filepath, repo).await
{
Ok(f) => f,
Err(spfs::Error::NoActiveRuntime) => {
Err(spfs::Error::Runtime(spfs::runtime::Error::NoActiveRuntime)) => {
in_a_runtime = false;
Vec::new()
}
Expand Down
6 changes: 3 additions & 3 deletions crates/spfs-cli/main/src/cmd_write.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,8 +8,8 @@ use std::path::PathBuf;

use clap::Args;
use miette::Result;
use spfs::Error;
use spfs::prelude::*;
use spfs::runtime::Error as RuntimeError;
use spfs::tracking::BlobReadExt;
use spfs_cli_common as cli;

Expand Down Expand Up @@ -40,12 +40,12 @@ impl CmdWrite {
Some(file) => {
let handle = tokio::fs::File::open(&file)
.await
.map_err(|err| Error::RuntimeWriteError(file.clone(), err))?;
.map_err(|err| RuntimeError::RuntimeWriteError(file.clone(), err))?;
#[cfg(unix)]
let mode = handle
.metadata()
.await
.map_err(|err| Error::RuntimeWriteError(file.clone(), err))?
.map_err(|err| RuntimeError::RuntimeWriteError(file.clone(), err))?
Comment on lines +43 to +48
Copy link

Copilot AI Feb 24, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Using RuntimeError::RuntimeWriteError for file open and metadata operations in the write command is semantically incorrect. The first error is actually a read error (opening a file for reading), not a write error. These operations are also not runtime-specific filesystem operations. Consider using appropriate error variants like Error::StorageReadError for file opening or a more general I/O error type.

Copilot uses AI. Check for mistakes.
.permissions()
.mode();
#[cfg(windows)]
Expand Down
2 changes: 1 addition & 1 deletion crates/spfs-vfs/src/winfsp/router.rs
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,7 @@ impl Router {
tracing::info!(%root_pid, env_spec=%env_spec.to_string(),"mounted");
let mut routes = self.routes.write().expect("lock is never poisoned");
if routes.contains_key(&root_pid) {
return Err(spfs::Error::RuntimeExists(root_pid.to_string()));
return Err(spfs::runtime::Error::RuntimeExists(root_pid.to_string()).into());
}
routes.insert(root_pid, Arc::new(mount));
Ok(())
Expand Down
9 changes: 5 additions & 4 deletions crates/spfs/src/commit.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ use spfs_encoding::prelude::*;

use super::status::remount_runtime;
use crate::prelude::*;
use crate::runtime::Error as RuntimeError;
use crate::tracking::{BlobHasher, BlobRead, ManifestBuilder, PathFilter};
use crate::{Error, Result, encoding, graph, runtime, storage, tracking};

Expand Down Expand Up @@ -198,7 +199,7 @@ where
runtime: &mut runtime::Runtime,
) -> Result<graph::Layer> {
if manifest.is_empty() && !self.allow_empty {
return Err(Error::NothingToCommit);
return Err(RuntimeError::NothingToCommit.into());
}
let layer = self
.repo
Expand All @@ -208,7 +209,7 @@ where
// Don't bother putting the empty layer on the stack, the goal
// with allow_empty is to create an empty manifest.
if !runtime.push_digest(layer.digest()?) {
return Err(Error::NothingToCommit);
return Err(RuntimeError::NothingToCommit.into());
}
runtime.status.editable = false;
runtime.save_state_to_storage().await?;
Expand All @@ -220,13 +221,13 @@ where
/// Commit the full layer stack and working files to a new platform.
pub async fn commit_platform(&self, runtime: &mut runtime::Runtime) -> Result<graph::Platform> {
match self.commit_layer(runtime).await {
Ok(_) | Err(Error::NothingToCommit) => (),
Ok(_) | Err(Error::Runtime(RuntimeError::NothingToCommit)) => (),
Err(err) => return Err(err),
}

runtime.reload_state_from_storage().await?;
if runtime.status.stack.is_empty() && !self.allow_empty {
Err(Error::NothingToCommit)
Err(RuntimeError::NothingToCommit.into())
} else {
self.repo
.create_platform(runtime.status.stack.clone())
Expand Down
5 changes: 3 additions & 2 deletions crates/spfs/src/commit_test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ use rstest::rstest;
use super::Committer;
use crate::Error;
use crate::fixtures::*;
use crate::runtime::Error as RuntimeError;

#[rstest]
#[tokio::test]
Expand All @@ -27,12 +28,12 @@ async fn test_commit_empty(tmpdir: tempfile::TempDir) {
rt.ensure_required_directories().await.unwrap();
let committer = Committer::new(&repo);
match committer.commit_layer(&mut rt).await {
Err(Error::NothingToCommit) => {}
Err(Error::Runtime(RuntimeError::NothingToCommit)) => {}
res => panic!("expected nothing to commit, got {res:?}"),
}

match committer.commit_platform(&mut rt).await {
Err(Error::NothingToCommit) => {}
Err(Error::Runtime(RuntimeError::NothingToCommit)) => {}
res => panic!("expected nothing to commit, got {res:?}"),
}
}
69 changes: 41 additions & 28 deletions crates/spfs/src/env.rs
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ use linux_syscall::{

use super::runtime;
use crate::config::OverlayFsOptions;
use crate::runtime::Error as RuntimeError;
use crate::{Error, Result, which};

pub const SPFS_DIR: &str = "/spfs";
Expand Down Expand Up @@ -248,7 +249,7 @@ impl<User> RuntimeConfigurator<User, NoMountNamespace> {
rt: &runtime::Runtime,
) -> Result<RuntimeConfigurator<User, ProcessIsInMountNamespace>> {
let Some(runtime_ns) = &rt.config.mount_namespace else {
return Err(Error::NoActiveRuntime);
return Err(RuntimeError::NoActiveRuntime.into());
};
// Safety: we are going to validate that this is the
// expected namespace for the provided runtime and so
Expand Down Expand Up @@ -283,7 +284,7 @@ impl<User> RuntimeConfigurator<User, NoMountNamespace> {
check_can_join()?;

let pid = match rt.status.owner {
None => return Err(Error::RuntimeNotInitialized(rt.name().into())),
None => return Err(RuntimeError::RuntimeNotInitialized(rt.name().into()).into()),
Some(pid) => pid,
};

Expand All @@ -296,11 +297,12 @@ impl<User> RuntimeConfigurator<User, NoMountNamespace> {
Ok(file) => file,
Err(err) => {
return match err.kind() {
std::io::ErrorKind::NotFound => Err(Error::UnknownRuntime {
std::io::ErrorKind::NotFound => Err(RuntimeError::UnknownRuntime {
runtime: rt.name().into(),
source: Box::new(err),
}),
_ => Err(Error::RuntimeReadError(ns_path, err)),
}
.into()),
_ => Err(RuntimeError::RuntimeReadError(ns_path, err).into()),
};
}
};
Expand Down Expand Up @@ -445,11 +447,11 @@ where
pub fn ensure_mount_targets_exist(&self, config: &runtime::Config) -> Result<()> {
tracing::debug!("ensuring mount targets exist...");
runtime::makedirs_with_perms(SPFS_DIR, 0o777)
.map_err(|source| Error::CouldNotCreateSpfsRoot { source })?;
.map_err(|source| RuntimeError::CouldNotCreateSpfsRoot { source })?;

if let Some(dir) = &config.runtime_dir {
runtime::makedirs_with_perms(dir, 0o777)
.map_err(|err| Error::RuntimeWriteError(dir.clone(), err))?
.map_err(|err| RuntimeError::RuntimeWriteError(dir.clone(), err))?
}
Ok(())
}
Expand Down Expand Up @@ -639,7 +641,7 @@ where
if let Some(parent) = fullpath.parent() {
tracing::trace!(?parent, "build parent dir for mask");
runtime::makedirs_with_perms(parent, 0o777)
.map_err(|err| Error::RuntimeWriteError(parent.to_owned(), err))?;
.map_err(|err| RuntimeError::RuntimeWriteError(parent.to_owned(), err))?;
}
tracing::trace!(?node.path, "Creating file mask");

Expand All @@ -649,11 +651,13 @@ where
continue;
}
if meta.is_file() {
std::fs::remove_file(&fullpath)
.map_err(|err| Error::RuntimeWriteError(fullpath.clone(), err))?;
std::fs::remove_file(&fullpath).map_err(|err| {
RuntimeError::RuntimeWriteError(fullpath.clone(), err)
})?;
} else {
std::fs::remove_dir_all(&fullpath)
.map_err(|err| Error::RuntimeWriteError(fullpath.clone(), err))?;
std::fs::remove_dir_all(&fullpath).map_err(|err| {
RuntimeError::RuntimeWriteError(fullpath.clone(), err)
})?;
}
}

Expand All @@ -677,7 +681,7 @@ where
continue;
}
let existing = std::fs::symlink_metadata(&fullpath)
.map_err(|err| Error::RuntimeReadError(fullpath.clone(), err))?;
.map_err(|err| RuntimeError::RuntimeReadError(fullpath.clone(), err))?;
if existing.permissions().mode() != node.entry.mode
&& let Err(err) = std::fs::set_permissions(
&fullpath,
Expand All @@ -687,7 +691,9 @@ where
match err.kind() {
std::io::ErrorKind::NotFound => continue,
_ => {
return Err(Error::RuntimeSetPermissionsError(fullpath, err));
return Err(
RuntimeError::RuntimeSetPermissionsError(fullpath, err).into()
);
}
}
}
Expand Down Expand Up @@ -728,10 +734,11 @@ where
match runtime.config.mount_backend {
runtime::MountBackend::FuseOnly | runtime::MountBackend::WinFsp => {
// a vfs-only runtime cannot be change to durable
return Err(Error::RuntimeChangeToDurableError(format!(
return Err(RuntimeError::RuntimeChangeToDurableError(format!(
"{} backend does not support durable runtimes",
runtime.config.mount_backend
)));
))
.into());
}
runtime::MountBackend::OverlayFsWithFuse
| runtime::MountBackend::OverlayFsWithRenders => {}
Expand All @@ -755,29 +762,32 @@ where
let src_dir = match old_upper_dir.to_str() {
Some(path) => path,
None => {
return Err(Error::RuntimeChangeToDurableError(format!(
return Err(RuntimeError::RuntimeChangeToDurableError(format!(
"current upper_dir '{}' has invalid characters",
old_upper_dir.display()
)));
))
.into());
}
};
let dest_dir = match new_path.to_str() {
Some(path) => path,
None => {
return Err(Error::RuntimeChangeToDurableError(format!(
return Err(RuntimeError::RuntimeChangeToDurableError(format!(
"new upper_dir '{}' has invalid characters",
new_path.display()
)));
))
.into());
}
};

let args = vec!["-aD", src_dir, dest_dir];
let cmd_path = match which("rsync") {
Some(cmd) => cmd,
None => {
return Err(Error::RuntimeChangeToDurableError(
return Err(RuntimeError::RuntimeChangeToDurableError(
"rsync is not available on this host".to_string(),
));
)
.into());
}
};

Expand All @@ -793,16 +803,19 @@ where
tracing::info!("runtime saved as durable");
Ok(0)
}
Some(code) => Err(Error::RuntimeChangeToDurableError(format!(
Some(code) => Err(RuntimeError::RuntimeChangeToDurableError(format!(
"rsync failed with exit code: {code}"
))),
None => Err(Error::RuntimeChangeToDurableError(
))
.into()),
None => Err(RuntimeError::RuntimeChangeToDurableError(
"rsync was terminated by an unexpected signal".to_string(),
)),
)
.into()),
},
Err(err) => Err(Error::RuntimeChangeToDurableError(format!(
Err(err) => Err(RuntimeError::RuntimeChangeToDurableError(format!(
"rsync failed to run: {err}"
))),
))
.into()),
}
}

Expand Down
6 changes: 4 additions & 2 deletions crates/spfs/src/env_win.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
// SPDX-License-Identifier: Apache-2.0
// https://github.com/spkenv/spk

use crate::runtime::Error as RuntimeError;
use crate::tracking::EnvSpec;
use crate::{Error, Result, runtime};

Expand Down Expand Up @@ -29,9 +30,10 @@ impl RuntimeConfigurator {
/// Mount the provided runtime via the winfsp backend
pub async fn mount_env_winfsp(&self, rt: &runtime::Runtime) -> Result<()> {
let Some(root_pid) = rt.status.owner else {
return Err(Error::RuntimeNotInitialized(
return Err(RuntimeError::RuntimeNotInitialized(
"Missing owner in runtime, cannot initialize".to_string(),
));
)
.into());
};

let env_spec = rt
Expand Down
Loading
Loading