refactor(tvix/eval/io): use io::Error instead of tvix_eval errors

We didn't return anything useful other than ErrorKind::IO anyways.

We can use io::ErrorKind::Unsupported for DummyIO.

Fixes b/271.

Change-Id: Icb231e9b38168e8b6fa473bfa405d160357b317f
Reviewed-on: https://cl.tvl.fyi/c/depot/+/8602
Autosubmit: flokli <flokli@flokli.de>
Tested-by: BuildkiteCI
Reviewed-by: tazjin <tazjin@tvl.su>

tvix/cli/src/nix_compat.rs

@@ -11,7 +11,7 @@ use std::process::Command;
 use std::{io, path::PathBuf};
 
 use smol_str::SmolStr;
-use tvix_eval::{ErrorKind, EvalIO, FileType, StdIO};
+use tvix_eval::{EvalIO, FileType, StdIO};
 
 /// Compatibility implementation of [`EvalIO`] that uses C++ Nix to
 /// write files to the Nix store.
@@ -33,16 +33,13 @@ impl EvalIO for NixCompatIO {
     }
 
     // Pass path imports through to `nix-store --add`
-    fn import_path(&mut self, path: &Path) -> Result<PathBuf, ErrorKind> {
+    fn import_path(&mut self, path: &Path) -> Result<PathBuf, io::Error> {
         let path = path.to_owned();
         if let Some(path) = self.import_cache.get(&path) {
             return Ok(path.to_path_buf());
         }
 
-        let store_path = self.add_to_store(&path).map_err(|error| ErrorKind::IO {
-            error: std::rc::Rc::new(error),
-            path: Some(path.to_path_buf()),
-        })?;
+        let store_path = self.add_to_store(&path)?;
 
         self.import_cache.insert(path, store_path.clone());
 
@@ -50,7 +47,7 @@ impl EvalIO for NixCompatIO {
     }
 
     // Pass the rest of the functions through to `Self::underlying`
-    fn path_exists(&mut self, path: PathBuf) -> Result<bool, ErrorKind> {
+    fn path_exists(&mut self, path: PathBuf) -> Result<bool, io::Error> {
         if path.starts_with("/__corepkgs__") {
             return Ok(true);
         }
@@ -58,7 +55,7 @@ impl EvalIO for NixCompatIO {
         self.underlying.path_exists(path)
     }
 
-    fn read_to_string(&mut self, path: PathBuf) -> Result<String, ErrorKind> {
+    fn read_to_string(&mut self, path: PathBuf) -> Result<String, io::Error> {
         // Bundled version of corepkgs/fetchurl.nix. This workaround
         // is similar to what cppnix does for passing the path
         // through.
@@ -72,7 +69,7 @@ impl EvalIO for NixCompatIO {
         self.underlying.read_to_string(path)
     }
 
-    fn read_dir(&mut self, path: PathBuf) -> Result<Vec<(SmolStr, FileType)>, ErrorKind> {
+    fn read_dir(&mut self, path: PathBuf) -> Result<Vec<(SmolStr, FileType)>, io::Error> {
         self.underlying.read_dir(path)
     }
 }

tvix/cli/src/tvix_io.rs

@@ -11,9 +11,10 @@
 use crate::KnownPaths;
 use smol_str::SmolStr;
 use std::cell::RefCell;
+use std::io;
 use std::path::{Path, PathBuf};
 use std::rc::Rc;
-use tvix_eval::{ErrorKind, EvalIO, FileType};
+use tvix_eval::{EvalIO, FileType};
 
 pub(crate) struct TvixIO<T: EvalIO> {
     /// Ingested paths must be reported to this known paths tracker
@@ -38,7 +39,7 @@ impl<T: EvalIO> EvalIO for TvixIO<T> {
         self.actual.store_dir()
     }
 
-    fn import_path(&mut self, path: &Path) -> Result<PathBuf, ErrorKind> {
+    fn import_path(&mut self, path: &Path) -> Result<PathBuf, io::Error> {
         let imported_path = self.actual.import_path(path)?;
         self.known_paths
             .borrow_mut()
@@ -47,7 +48,7 @@ impl<T: EvalIO> EvalIO for TvixIO<T> {
         Ok(imported_path)
     }
 
-    fn path_exists(&mut self, path: PathBuf) -> Result<bool, ErrorKind> {
+    fn path_exists(&mut self, path: PathBuf) -> Result<bool, io::Error> {
         if path.starts_with("/__corepkgs__") {
             return Ok(true);
         }
@@ -55,7 +56,7 @@ impl<T: EvalIO> EvalIO for TvixIO<T> {
         self.actual.path_exists(path)
     }
 
-    fn read_to_string(&mut self, path: PathBuf) -> Result<String, ErrorKind> {
+    fn read_to_string(&mut self, path: PathBuf) -> Result<String, io::Error> {
         // Bundled version of corepkgs/fetchurl.nix. The counterpart
         // of this happens in `main`, where the `nix_path` of the
         // evaluation has `nix=/__corepkgs__` added to it.
@@ -72,7 +73,7 @@ impl<T: EvalIO> EvalIO for TvixIO<T> {
         self.actual.read_to_string(path)
     }
 
-    fn read_dir(&mut self, path: PathBuf) -> Result<Vec<(SmolStr, FileType)>, ErrorKind> {
+    fn read_dir(&mut self, path: PathBuf) -> Result<Vec<(SmolStr, FileType)>, io::Error> {
         self.actual.read_dir(path)
     }
 }

tvix/eval/src/io.rs

@@ -15,12 +15,11 @@
 //! In the context of Nix builds, callers also use this interface to determine
 //! how store paths are opened and so on.
 
-use crate::errors::ErrorKind;
 use smol_str::SmolStr;
-use std::path::{Path, PathBuf};
-
-#[cfg(feature = "impure")]
-use std::rc::Rc;
+use std::{
+    io,
+    path::{Path, PathBuf},
+};
 
 /// Types of files as represented by `builtins.readDir` in Nix.
 #[derive(Debug)]
@@ -34,14 +33,14 @@ pub enum FileType {
 /// Defines how filesystem interaction occurs inside of tvix-eval.
 pub trait EvalIO {
     /// Verify whether the file at the specified path exists.
-    fn path_exists(&mut self, path: PathBuf) -> Result<bool, ErrorKind>;
+    fn path_exists(&mut self, path: PathBuf) -> Result<bool, io::Error>;
 
     /// Read the file at the specified path to a string.
-    fn read_to_string(&mut self, path: PathBuf) -> Result<String, ErrorKind>;
+    fn read_to_string(&mut self, path: PathBuf) -> Result<String, io::Error>;
 
     /// Read the directory at the specified path and return the names
     /// of its entries associated with their [`FileType`].
-    fn read_dir(&mut self, path: PathBuf) -> Result<Vec<(SmolStr, FileType)>, ErrorKind>;
+    fn read_dir(&mut self, path: PathBuf) -> Result<Vec<(SmolStr, FileType)>, io::Error>;
 
     /// Import the given path. What this means depends on the
     /// implementation, for example for a `std::io`-based
@@ -50,7 +49,7 @@ pub trait EvalIO {
     ///
     /// This is primarily used in the context of things like coercing
     /// a local path to a string, or builtins like `path`.
-    fn import_path(&mut self, path: &Path) -> Result<PathBuf, ErrorKind>;
+    fn import_path(&mut self, path: &Path) -> Result<PathBuf, io::Error>;
 
     /// Returns the root of the store directory, if such a thing
     /// exists in the evaluation context.
@@ -66,37 +65,20 @@ pub struct StdIO;
 
 #[cfg(feature = "impure")]
 impl EvalIO for StdIO {
-    fn path_exists(&mut self, path: PathBuf) -> Result<bool, ErrorKind> {
-        path.try_exists().map_err(|e| ErrorKind::IO {
-            path: Some(path),
-            error: Rc::new(e),
-        })
+    fn path_exists(&mut self, path: PathBuf) -> Result<bool, io::Error> {
+        path.try_exists()
     }
 
-    fn read_to_string(&mut self, path: PathBuf) -> Result<String, ErrorKind> {
-        std::fs::read_to_string(&path).map_err(|e| ErrorKind::IO {
-            path: Some(path),
-            error: Rc::new(e),
-        })
+    fn read_to_string(&mut self, path: PathBuf) -> Result<String, io::Error> {
+        std::fs::read_to_string(&path)
     }
 
-    fn read_dir(&mut self, path: PathBuf) -> Result<Vec<(SmolStr, FileType)>, ErrorKind> {
+    fn read_dir(&mut self, path: PathBuf) -> Result<Vec<(SmolStr, FileType)>, io::Error> {
         let mut result = vec![];
 
-        let mk_err = |err| ErrorKind::IO {
-            path: Some(path.clone()),
-            error: Rc::new(err),
-        };
-
-        for entry in path.read_dir().map_err(mk_err)? {
-            let entry = entry.map_err(mk_err)?;
-            let file_type = entry
-                .metadata()
-                .map_err(|err| ErrorKind::IO {
-                    path: Some(entry.path()),
-                    error: Rc::new(err),
-                })?
-                .file_type();
+        for entry in path.read_dir()? {
+            let entry = entry?;
+            let file_type = entry.metadata()?.file_type();
 
             let val = if file_type.is_dir() {
                 FileType::Directory
@@ -116,7 +98,7 @@ impl EvalIO for StdIO {
 
     // this is a no-op for `std::io`, as the user can already refer to
     // the path directly
-    fn import_path(&mut self, path: &Path) -> Result<PathBuf, ErrorKind> {
+    fn import_path(&mut self, path: &Path) -> Result<PathBuf, io::Error> {
         Ok(path.to_path_buf())
     }
 }
@@ -126,26 +108,30 @@ impl EvalIO for StdIO {
 pub struct DummyIO;
 
 impl EvalIO for DummyIO {
-    fn path_exists(&mut self, _: PathBuf) -> Result<bool, ErrorKind> {
-        Err(ErrorKind::NotImplemented(
+    fn path_exists(&mut self, _: PathBuf) -> Result<bool, io::Error> {
+        Err(io::Error::new(
+            io::ErrorKind::Unsupported,
             "I/O methods are not implemented in DummyIO",
         ))
     }
 
-    fn read_to_string(&mut self, _: PathBuf) -> Result<String, ErrorKind> {
-        Err(ErrorKind::NotImplemented(
+    fn read_to_string(&mut self, _: PathBuf) -> Result<String, io::Error> {
+        Err(io::Error::new(
+            io::ErrorKind::Unsupported,
             "I/O methods are not implemented in DummyIO",
         ))
     }
 
-    fn read_dir(&mut self, _: PathBuf) -> Result<Vec<(SmolStr, FileType)>, ErrorKind> {
-        Err(ErrorKind::NotImplemented(
+    fn read_dir(&mut self, _: PathBuf) -> Result<Vec<(SmolStr, FileType)>, io::Error> {
+        Err(io::Error::new(
+            io::ErrorKind::Unsupported,
             "I/O methods are not implemented in DummyIO",
         ))
     }
 
-    fn import_path(&mut self, _: &Path) -> Result<PathBuf, ErrorKind> {
-        Err(ErrorKind::NotImplemented(
+    fn import_path(&mut self, _: &Path) -> Result<PathBuf, io::Error> {
+        Err(io::Error::new(
+            io::ErrorKind::Unsupported,
             "I/O methods are not implemented in DummyIO",
         ))
     }

tvix/eval/src/vm/generators.rs

@@ -406,15 +406,27 @@ impl<'o> VM<'o> {
                         }
 
                         VMRequest::PathImport(path) => {
-                            let imported =
-                                self.io_handle.import_path(&path).with_span(&span, self)?;
+                            let imported = self
+                                .io_handle
+                                .import_path(&path)
+                                .map_err(|e| ErrorKind::IO {
+                                    path: Some(path),
+                                    error: e.into(),
+                                })
+                                .with_span(&span, self)?;
 
                             message = VMResponse::Path(imported);
                         }
 
                         VMRequest::ReadToString(path) => {
-                            let content =
-                                self.io_handle.read_to_string(path).with_span(&span, self)?;
+                            let content = self
+                                .io_handle
+                                .read_to_string(path.clone())
+                                .map_err(|e| ErrorKind::IO {
+                                    path: Some(path),
+                                    error: e.into(),
+                                })
+                                .with_span(&span, self)?;
 
                             message = VMResponse::Value(Value::String(content.into()))
                         }
@@ -422,7 +434,11 @@ impl<'o> VM<'o> {
                         VMRequest::PathExists(path) => {
                             let exists = self
                                 .io_handle
-                                .path_exists(path)
+                                .path_exists(path.clone())
+                                .map_err(|e| ErrorKind::IO {
+                                    path: Some(path),
+                                    error: e.into(),
+                                })
                                 .map(Value::Bool)
                                 .with_span(&span, self)?;
 
@@ -430,7 +446,14 @@ impl<'o> VM<'o> {
                         }
 
                         VMRequest::ReadDir(path) => {
-                            let dir = self.io_handle.read_dir(path).with_span(&span, self)?;
+                            let dir = self
+                                .io_handle
+                                .read_dir(path.clone())
+                                .map_err(|e| ErrorKind::IO {
+                                    path: Some(path),
+                                    error: e.into(),
+                                })
+                                .with_span(&span, self)?;
                             message = VMResponse::Directory(dir);
                         }