From b7ab6c08561143df9ab6eb2255a74c37719f48f2 Mon Sep 17 00:00:00 2001 From: Florian Klink Date: Sun, 21 May 2023 11:00:49 +0300 Subject: 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 Tested-by: BuildkiteCI Reviewed-by: tazjin --- tvix/cli/src/nix_compat.rs | 15 ++++----- tvix/cli/src/tvix_io.rs | 11 ++++--- tvix/eval/src/io.rs | 72 +++++++++++++++++------------------------- tvix/eval/src/vm/generators.rs | 35 ++++++++++++++++---- 4 files changed, 70 insertions(+), 63 deletions(-) diff --git a/tvix/cli/src/nix_compat.rs b/tvix/cli/src/nix_compat.rs index 4141e0a217..ce352f0a96 100644 --- a/tvix/cli/src/nix_compat.rs +++ b/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 { + fn import_path(&mut self, path: &Path) -> Result { 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 { + fn path_exists(&mut self, path: PathBuf) -> Result { 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 { + fn read_to_string(&mut self, path: PathBuf) -> Result { // 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, ErrorKind> { + fn read_dir(&mut self, path: PathBuf) -> Result, io::Error> { self.underlying.read_dir(path) } } diff --git a/tvix/cli/src/tvix_io.rs b/tvix/cli/src/tvix_io.rs index 401b8a5f48..5a3f5b0e7a 100644 --- a/tvix/cli/src/tvix_io.rs +++ b/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 { /// Ingested paths must be reported to this known paths tracker @@ -38,7 +39,7 @@ impl EvalIO for TvixIO { self.actual.store_dir() } - fn import_path(&mut self, path: &Path) -> Result { + fn import_path(&mut self, path: &Path) -> Result { let imported_path = self.actual.import_path(path)?; self.known_paths .borrow_mut() @@ -47,7 +48,7 @@ impl EvalIO for TvixIO { Ok(imported_path) } - fn path_exists(&mut self, path: PathBuf) -> Result { + fn path_exists(&mut self, path: PathBuf) -> Result { if path.starts_with("/__corepkgs__") { return Ok(true); } @@ -55,7 +56,7 @@ impl EvalIO for TvixIO { self.actual.path_exists(path) } - fn read_to_string(&mut self, path: PathBuf) -> Result { + fn read_to_string(&mut self, path: PathBuf) -> Result { // 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 EvalIO for TvixIO { self.actual.read_to_string(path) } - fn read_dir(&mut self, path: PathBuf) -> Result, ErrorKind> { + fn read_dir(&mut self, path: PathBuf) -> Result, io::Error> { self.actual.read_dir(path) } } diff --git a/tvix/eval/src/io.rs b/tvix/eval/src/io.rs index 789f83b2e8..3299c92d1f 100644 --- a/tvix/eval/src/io.rs +++ b/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; + fn path_exists(&mut self, path: PathBuf) -> Result; /// Read the file at the specified path to a string. - fn read_to_string(&mut self, path: PathBuf) -> Result; + fn read_to_string(&mut self, path: PathBuf) -> Result; /// 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, ErrorKind>; + fn read_dir(&mut self, path: PathBuf) -> Result, 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; + fn import_path(&mut self, path: &Path) -> Result; /// 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 { - path.try_exists().map_err(|e| ErrorKind::IO { - path: Some(path), - error: Rc::new(e), - }) + fn path_exists(&mut self, path: PathBuf) -> Result { + path.try_exists() } - fn read_to_string(&mut self, path: PathBuf) -> Result { - 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 { + std::fs::read_to_string(&path) } - fn read_dir(&mut self, path: PathBuf) -> Result, ErrorKind> { + fn read_dir(&mut self, path: PathBuf) -> Result, 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 { + fn import_path(&mut self, path: &Path) -> Result { 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 { - Err(ErrorKind::NotImplemented( + fn path_exists(&mut self, _: PathBuf) -> Result { + Err(io::Error::new( + io::ErrorKind::Unsupported, "I/O methods are not implemented in DummyIO", )) } - fn read_to_string(&mut self, _: PathBuf) -> Result { - Err(ErrorKind::NotImplemented( + fn read_to_string(&mut self, _: PathBuf) -> Result { + Err(io::Error::new( + io::ErrorKind::Unsupported, "I/O methods are not implemented in DummyIO", )) } - fn read_dir(&mut self, _: PathBuf) -> Result, ErrorKind> { - Err(ErrorKind::NotImplemented( + fn read_dir(&mut self, _: PathBuf) -> Result, io::Error> { + Err(io::Error::new( + io::ErrorKind::Unsupported, "I/O methods are not implemented in DummyIO", )) } - fn import_path(&mut self, _: &Path) -> Result { - Err(ErrorKind::NotImplemented( + fn import_path(&mut self, _: &Path) -> Result { + Err(io::Error::new( + io::ErrorKind::Unsupported, "I/O methods are not implemented in DummyIO", )) } diff --git a/tvix/eval/src/vm/generators.rs b/tvix/eval/src/vm/generators.rs index b7d8f7aeb5..0e3123ae37 100644 --- a/tvix/eval/src/vm/generators.rs +++ b/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); } -- cgit 1.4.1