feat(tvix/eval/io): allow &mut self in EvalIO

It's okay if these calls mutate some internal state inside an
implementation.

Change-Id: I12bb11bde0310778c3da1275696bf7de058863a3
Reviewed-on: https://cl.tvl.fyi/c/depot/+/8571
Tested-by: BuildkiteCI
Reviewed-by: tazjin <tazjin@tvl.su>

tvix/Cargo.lock

@@ -2708,6 +2708,7 @@ dependencies = [
  "prost",
  "prost-build",
  "rayon",
+ "serde_json",
  "sha2 0.10.6",
  "sled",
  "tempfile",

tvix/cli/src/nix_compat.rs

@@ -40,7 +40,7 @@ impl EvalIO for NixCompatIO {
     }
 
     // Pass path imports through to `nix-store --add`
-    fn import_path(&self, path: &Path) -> Result<PathBuf, ErrorKind> {
+    fn import_path(&mut self, path: &Path) -> Result<PathBuf, ErrorKind> {
         let path = path.to_owned();
         if let Some(path) = self.import_cache.borrow().get(&path) {
             return Ok(path.to_path_buf());
@@ -58,7 +58,7 @@ impl EvalIO for NixCompatIO {
     }
 
     // Pass the rest of the functions through to `Self::underlying`
-    fn path_exists(&self, path: PathBuf) -> Result<bool, ErrorKind> {
+    fn path_exists(&mut self, path: PathBuf) -> Result<bool, ErrorKind> {
         if path.starts_with("/__corepkgs__") {
             return Ok(true);
         }
@@ -66,7 +66,7 @@ impl EvalIO for NixCompatIO {
         self.underlying.path_exists(path)
     }
 
-    fn read_to_string(&self, path: PathBuf) -> Result<String, ErrorKind> {
+    fn read_to_string(&mut self, path: PathBuf) -> Result<String, ErrorKind> {
         // Bundled version of corepkgs/fetchurl.nix. This workaround
         // is similar to what cppnix does for passing the path
         // through.
@@ -80,7 +80,7 @@ impl EvalIO for NixCompatIO {
         self.underlying.read_to_string(path)
     }
 
-    fn read_dir(&self, path: PathBuf) -> Result<Vec<(SmolStr, FileType)>, ErrorKind> {
+    fn read_dir(&mut self, path: PathBuf) -> Result<Vec<(SmolStr, FileType)>, ErrorKind> {
         self.underlying.read_dir(path)
     }
 }

tvix/eval/src/io.rs

@@ -34,14 +34,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(&self, path: PathBuf) -> Result<bool, ErrorKind>;
+    fn path_exists(&mut self, path: PathBuf) -> Result<bool, ErrorKind>;
 
     /// Read the file at the specified path to a string.
-    fn read_to_string(&self, path: PathBuf) -> Result<String, ErrorKind>;
+    fn read_to_string(&mut self, path: PathBuf) -> Result<String, ErrorKind>;
 
     /// Read the directory at the specified path and return the names
     /// of its entries associated with their [`FileType`].
-    fn read_dir(&self, path: PathBuf) -> Result<Vec<(SmolStr, FileType)>, ErrorKind>;
+    fn read_dir(&mut self, path: PathBuf) -> Result<Vec<(SmolStr, FileType)>, ErrorKind>;
 
     /// Import the given path. What this means depends on the
     /// implementation, for example for a `std::io`-based
@@ -50,7 +50,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(&self, path: &Path) -> Result<PathBuf, ErrorKind>;
+    fn import_path(&mut self, path: &Path) -> Result<PathBuf, ErrorKind>;
 
     /// Returns the root of the store directory, if such a thing
     /// exists in the evaluation context.
@@ -66,21 +66,21 @@ pub struct StdIO;
 
 #[cfg(feature = "impure")]
 impl EvalIO for StdIO {
-    fn path_exists(&self, path: PathBuf) -> Result<bool, ErrorKind> {
+    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 read_to_string(&self, path: PathBuf) -> Result<String, ErrorKind> {
+    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_dir(&self, path: PathBuf) -> Result<Vec<(SmolStr, FileType)>, ErrorKind> {
+    fn read_dir(&mut self, path: PathBuf) -> Result<Vec<(SmolStr, FileType)>, ErrorKind> {
         let mut result = vec![];
 
         let mk_err = |err| ErrorKind::IO {
@@ -116,7 +116,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(&self, path: &Path) -> Result<PathBuf, ErrorKind> {
+    fn import_path(&mut self, path: &Path) -> Result<PathBuf, ErrorKind> {
         Ok(path.to_path_buf())
     }
 }
@@ -126,25 +126,25 @@ impl EvalIO for StdIO {
 pub struct DummyIO;
 
 impl EvalIO for DummyIO {
-    fn path_exists(&self, _: PathBuf) -> Result<bool, ErrorKind> {
+    fn path_exists(&mut self, _: PathBuf) -> Result<bool, ErrorKind> {
         Err(ErrorKind::NotImplemented(
             "I/O methods are not implemented in DummyIO",
         ))
     }
 
-    fn read_to_string(&self, _: PathBuf) -> Result<String, ErrorKind> {
+    fn read_to_string(&mut self, _: PathBuf) -> Result<String, ErrorKind> {
         Err(ErrorKind::NotImplemented(
             "I/O methods are not implemented in DummyIO",
         ))
     }
 
-    fn read_dir(&self, _: PathBuf) -> Result<Vec<(SmolStr, FileType)>, ErrorKind> {
+    fn read_dir(&mut self, _: PathBuf) -> Result<Vec<(SmolStr, FileType)>, ErrorKind> {
         Err(ErrorKind::NotImplemented(
             "I/O methods are not implemented in DummyIO",
         ))
     }
 
-    fn import_path(&self, _: &Path) -> Result<PathBuf, ErrorKind> {
+    fn import_path(&mut self, _: &Path) -> Result<PathBuf, ErrorKind> {
         Err(ErrorKind::NotImplemented(
             "I/O methods are not implemented in DummyIO",
         ))

tvix/eval/src/nix_search_path.rs

@@ -63,7 +63,11 @@ impl NixSearchPathEntry {
     ///
     /// For prefixed path, an entry matches if the prefix does.
     // TODO(tazjin): verify these rules in the C++ impl, seems fishy.
-    fn resolve(&self, io: &dyn EvalIO, lookup_path: &Path) -> Result<Option<PathBuf>, ErrorKind> {
+    fn resolve(
+        &self,
+        io: &mut dyn EvalIO,
+        lookup_path: &Path,
+    ) -> Result<Option<PathBuf>, ErrorKind> {
         let path = match self {
             NixSearchPathEntry::Path(parent) => canonicalise(parent.join(lookup_path))?,
 
@@ -112,7 +116,7 @@ pub struct NixSearchPath {
 impl NixSearchPath {
     /// Attempt to resolve the given `path` within this [`NixSearchPath`] using the
     /// path resolution rules for `<...>`-style paths
-    pub fn resolve<P>(&self, io: &dyn EvalIO, path: P) -> Result<PathBuf, ErrorKind>
+    pub fn resolve<P>(&self, io: &mut dyn EvalIO, path: P) -> Result<PathBuf, ErrorKind>
     where
         P: AsRef<Path>,
     {
@@ -188,16 +192,16 @@ mod tests {
         #[test]
         fn simple_dir() {
             let nix_search_path = NixSearchPath::from_str("./.").unwrap();
-            let io = StdIO {};
-            let res = nix_search_path.resolve(&io, "src").unwrap();
+            let mut io = StdIO {};
+            let res = nix_search_path.resolve(&mut io, "src").unwrap();
             assert_eq!(res, current_dir().unwrap().join("src").clean());
         }
 
         #[test]
         fn failed_resolution() {
             let nix_search_path = NixSearchPath::from_str("./.").unwrap();
-            let io = StdIO {};
-            let err = nix_search_path.resolve(&io, "nope").unwrap_err();
+            let mut io = StdIO {};
+            let err = nix_search_path.resolve(&mut io, "nope").unwrap_err();
             assert!(
                 matches!(err, ErrorKind::NixPathResolution(..)),
                 "err = {err:?}"
@@ -207,24 +211,24 @@ mod tests {
         #[test]
         fn second_in_path() {
             let nix_search_path = NixSearchPath::from_str("./.:/").unwrap();
-            let io = StdIO {};
-            let res = nix_search_path.resolve(&io, "etc").unwrap();
+            let mut io = StdIO {};
+            let res = nix_search_path.resolve(&mut io, "etc").unwrap();
             assert_eq!(res, Path::new("/etc"));
         }
 
         #[test]
         fn prefix() {
             let nix_search_path = NixSearchPath::from_str("/:tvix=.").unwrap();
-            let io = StdIO {};
-            let res = nix_search_path.resolve(&io, "tvix/src").unwrap();
+            let mut io = StdIO {};
+            let res = nix_search_path.resolve(&mut io, "tvix/src").unwrap();
             assert_eq!(res, current_dir().unwrap().join("src").clean());
         }
 
         #[test]
         fn matching_prefix() {
             let nix_search_path = NixSearchPath::from_str("/:tvix=.").unwrap();
-            let io = StdIO {};
-            let res = nix_search_path.resolve(&io, "tvix").unwrap();
+            let mut io = StdIO {};
+            let res = nix_search_path.resolve(&mut io, "tvix").unwrap();
             assert_eq!(res, current_dir().unwrap().clean());
         }
     }

tvix/eval/src/vm/mod.rs

@@ -834,7 +834,7 @@ impl<'o> VM<'o> {
                     Value::UnresolvedPath(path) => {
                         let resolved = self
                             .nix_search_path
-                            .resolve(&*self.io_handle, *path)
+                            .resolve(&mut *self.io_handle, *path)
                             .with_span(&frame, self)?;
                         self.stack.push(resolved.into());
                     }