From 0ef3c2fc2beb65b594de2ebc2574776b20205494 Mon Sep 17 00:00:00 2001
From: Vincent Ambo <mail@tazj.in>
Date: Mon, 12 Dec 2022 17:57:03 +0300
Subject: refactor(tvix/eval): use EvalIO::read_to_string in impure builtins

With this change, the behaviour of reading a string from a file path
is controlled by the provided `EvalIO` structure.

This is a huge step towards abstracting away I/O behaviour correctly.

Change-Id: Ifde8e46cd863b16e0301dca45a434ad27560399f
Reviewed-on: https://cl.tvl.fyi/c/depot/+/7567
Tested-by: BuildkiteCI
Reviewed-by: grfn <grfn@gws.fyi>
---
 tvix/eval/src/builtins/impure.rs | 37 ++++++++++++++++---------------------
 tvix/eval/src/errors.rs          | 17 -----------------
 tvix/eval/src/vm.rs              |  5 +++++
 3 files changed, 21 insertions(+), 38 deletions(-)

(limited to 'tvix/eval')

diff --git a/tvix/eval/src/builtins/impure.rs b/tvix/eval/src/builtins/impure.rs
index b086df2f84..c2ba2f2a07 100644
--- a/tvix/eval/src/builtins/impure.rs
+++ b/tvix/eval/src/builtins/impure.rs
@@ -1,9 +1,7 @@
 use builtin_macros::builtins;
 use std::{
     collections::BTreeMap,
-    env,
-    fs::File,
-    io::{self, Read},
+    env, io,
     rc::{Rc, Weak},
     time::{SystemTime, UNIX_EPOCH},
 };
@@ -68,9 +66,10 @@ mod impure_builtins {
 
     #[builtin("readFile")]
     fn builtin_read_file(vm: &mut VM, path: Value) -> Result<Value, ErrorKind> {
-        let mut buf = String::new();
-        File::open(&coerce_value_to_path(&path, vm)?)?.read_to_string(&mut buf)?;
-        Ok(buf.into())
+        let path = coerce_value_to_path(&path, vm)?;
+        vm.io()
+            .read_to_string(path)
+            .map(|s| Value::String(s.into()))
     }
 }
 
@@ -122,16 +121,12 @@ pub fn builtins_import(globals: &Weak<GlobalsMap>, source: SourceCode) -> Builti
             }
 
             let current_span = vm.current_span();
-            let entry = match vm.import_cache.entry(path.clone()) {
-                std::collections::btree_map::Entry::Occupied(oe) => return Ok(oe.get().clone()),
-                std::collections::btree_map::Entry::Vacant(ve) => ve,
-            };
 
-            let contents =
-                std::fs::read_to_string(&path).map_err(|err| ErrorKind::ReadFileError {
-                    path: path.clone(),
-                    error: Rc::new(err),
-                })?;
+            if let Some(cached) = vm.import_cache.get(&path) {
+                return Ok(cached.clone());
+            }
+
+            let contents = vm.io().read_to_string(path.clone())?;
 
             let parsed = rnix::ast::Root::parse(&contents);
             let errors = parsed.errors();
@@ -174,12 +169,12 @@ pub fn builtins_import(globals: &Weak<GlobalsMap>, source: SourceCode) -> Builti
 
             // Compilation succeeded, we can construct a thunk from whatever it spat
             // out and return that.
-            let res = entry
-                .insert(Value::Thunk(Thunk::new_suspended(
-                    result.lambda,
-                    LightSpan::new_actual(current_span),
-                )))
-                .clone();
+            let res = Value::Thunk(Thunk::new_suspended(
+                result.lambda,
+                LightSpan::new_actual(current_span),
+            ));
+
+            vm.import_cache.insert(path, res.clone());
 
             for warning in result.warnings {
                 vm.push_warning(warning);
diff --git a/tvix/eval/src/errors.rs b/tvix/eval/src/errors.rs
index 107b8b154a..92ec7b0799 100644
--- a/tvix/eval/src/errors.rs
+++ b/tvix/eval/src/errors.rs
@@ -109,12 +109,6 @@ pub enum ErrorKind {
     /// literal attribute sets.
     UnmergeableValue,
 
-    /// Tvix failed to read a file from disk for some reason.
-    ReadFileError {
-        path: PathBuf,
-        error: Rc<std::io::Error>,
-    },
-
     /// Parse errors occured while importing a file.
     ImportParseError {
         path: PathBuf,
@@ -342,15 +336,6 @@ to a missing value in the attribute set(s) included via `with`."#,
                 )
             }
 
-            ErrorKind::ReadFileError { path, error } => {
-                write!(
-                    f,
-                    "failed to read file '{}': {}",
-                    path.to_string_lossy(),
-                    error
-                )
-            }
-
             // Errors themselves ignored here & handled in Self::spans instead
             ErrorKind::ImportParseError { path, .. } => {
                 write!(
@@ -676,7 +661,6 @@ impl Error {
             | ErrorKind::NegativeLength { .. }
             | ErrorKind::UnmergeableInherit { .. }
             | ErrorKind::UnmergeableValue
-            | ErrorKind::ReadFileError { .. }
             | ErrorKind::ImportParseError { .. }
             | ErrorKind::ImportCompilerError { .. }
             | ErrorKind::IO { .. }
@@ -716,7 +700,6 @@ impl Error {
             ErrorKind::TailEmptyList { .. } => "E023",
             ErrorKind::UnmergeableInherit { .. } => "E024",
             ErrorKind::UnmergeableValue => "E025",
-            ErrorKind::ReadFileError { .. } => "E026",
             ErrorKind::ImportParseError { .. } => "E027",
             ErrorKind::ImportCompilerError { .. } => "E028",
             ErrorKind::IO { .. } => "E029",
diff --git a/tvix/eval/src/vm.rs b/tvix/eval/src/vm.rs
index baf594f936..fcbbe61909 100644
--- a/tvix/eval/src/vm.rs
+++ b/tvix/eval/src/vm.rs
@@ -220,6 +220,11 @@ impl<'o> VM<'o> {
         self.chunk().get_span(self.frame().ip - 1)
     }
 
+    /// Access the I/O handle used for filesystem access in this VM.
+    pub(crate) fn io(&self) -> &Box<dyn EvalIO> {
+        &self.io_handle
+    }
+
     /// Returns the information needed to calculate the current span,
     /// but without performing that calculation.
     fn current_light_span(&self) -> LightSpan {
-- 
cgit 1.4.1