fix(tvix/eval): use realpaths for import cache r/6341

I've noticed this behavior when writing the admittedly cursed test case
included in this CL. Alternatively we could use some sort of machinery
using `builtins.trace`, but I don't think we capture stderr anywhere.

I've elected to put this into the eval cache itself while C++ Nix does
it in builtins.import already, namely via `realisePath`. We don't have
an equivalent for this yet, since we don't support any kind of IfD, but
we could revise that later. In any case, it seems good to encapsulate
`ImportCache` in this way, as it'll also allow using file hashes as
identifiers, for example.

C++ Nix also does our equivalent of canon_path in `builtins.import`
which we still don't, but I suspect it hardly makes a difference.

Change-Id: I05004737ca2458a4c67359d9e7d9a2f2154a0a0f
Reviewed-on: https://cl.tvl.fyi/c/depot/+/8839
Autosubmit: sterni <sternenseemann@systemli.org>
Reviewed-by: tazjin <tazjin@tvl.su>
Tested-by: BuildkiteCI

1 files changed, 38 insertions, 2 deletions

diff --git a/tvix/eval/src/vm/mod.rs b/tvix/eval/src/vm/mod.rs
index d004af3c6f..cc66ee4b6f 100644
--- a/tvix/eval/src/vm/mod.rs
+++ b/tvix/eval/src/vm/mod.rs
@@ -14,7 +14,7 @@ mod macros;
 
 use codemap::Span;
 use serde_json::json;
-use std::{cmp::Ordering, collections::HashMap, ops::DerefMut, path::PathBuf, rc::Rc};
+use std::{cmp::Ordering, collections::HashMap, ops::DerefMut, path::Path, path::PathBuf, rc::Rc};
 
 use crate::{
     arithmetic_op,
@@ -218,6 +218,42 @@ impl Frame {
     }
 }
 
+#[derive(Default)]
+struct ImportCache(Box<HashMap<PathBuf, Value>>);
+
+/// The `ImportCache` holds the `Value` resulting from `import`ing a certain
+/// file, so that the same file doesn't need to be re-evaluated multiple times.
+/// Currently the real path of the imported file (determined using
+/// [`std::fs::canonicalize()`], not to be confused with our
+/// [`value::canon_path()`]) is used to identify the file, just like C++ Nix
+/// does.
+///
+/// Errors while determining the real path are currently just ignored, since we
+/// pass around some fake paths like `/__corepkgs__/fetchurl.nix`.
+///
+/// In the future, we could use something more sophisticated, like file hashes.
+/// However, a consideration is that the eval cache is observable via impurities
+/// like pointer equality and `builtins.trace`.
+impl ImportCache {
+    fn get(&self, path: PathBuf) -> Option<&Value> {
+        let path = match std::fs::canonicalize(path.as_path()).map_err(ErrorKind::from) {
+            Ok(path) => path,
+            Err(_) => path,
+        };
+        self.0.get(&path)
+    }
+
+    fn insert(&mut self, path: PathBuf, value: Value) -> Option<Value> {
+        self.0.insert(
+            match std::fs::canonicalize(path.as_path()).map_err(ErrorKind::from) {
+                Ok(path) => path,
+                Err(_) => path,
+            },
+            value,
+        )
+    }
+}
+
 struct VM<'o> {
     /// VM's frame stack, representing the execution contexts the VM is working
     /// through. Elements are usually pushed when functions are called, or
@@ -241,7 +277,7 @@ struct VM<'o> {
     /// Import cache, mapping absolute file paths to the value that
     /// they compile to. Note that this reuses thunks, too!
     // TODO: should probably be based on a file hash
-    pub import_cache: Box<HashMap<PathBuf, Value>>,
+    pub import_cache: ImportCache,
 
     /// Parsed Nix search path, which is used to resolve `<...>`
     /// references.