From 25336fc47b02fe90bf489402ed84f8259aa80ca8 Mon Sep 17 00:00:00 2001
From: Adam Joseph <adam@westernsemico.com>
Date: Wed, 12 Oct 2022 21:58:42 -0700
Subject: refactor(tvix/eval): factor out all calls to canon_path

Right now we're pretending that the Rust library path_clean does the
same thing that cppnix's canonPath() does.  This is not true.  It's
close enough for the test suite, but may come back to bite us.

Let's create our own canon_path() function and call that in all the
places where we intend to match the behavior of cppnix's
canonPath().  That way when we fix this we can fix it once, in one
place.

Signed-off-by: Adam Joseph <adam@westernsemico.com>
Change-Id: Ia6f9577f62f49ef352ff9cfa5efdf37c32d31b11
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6993
Tested-by: BuildkiteCI
Reviewed-by: grfn <grfn@gws.fyi>
---
 tvix/eval/src/compiler/mod.rs |  3 +--
 tvix/eval/src/value/mod.rs    |  2 ++
 tvix/eval/src/value/path.rs   | 14 ++++++++++++++
 tvix/eval/src/vm.rs           |  4 +---
 4 files changed, 18 insertions(+), 5 deletions(-)
 create mode 100644 tvix/eval/src/value/path.rs

(limited to 'tvix/eval')

diff --git a/tvix/eval/src/compiler/mod.rs b/tvix/eval/src/compiler/mod.rs
index 9bd37cef8578..e4c2c159da40 100644
--- a/tvix/eval/src/compiler/mod.rs
+++ b/tvix/eval/src/compiler/mod.rs
@@ -17,7 +17,6 @@ mod bindings;
 mod scope;
 
 use codemap::Span;
-use path_clean::PathClean;
 use rnix::ast::{self, AstToken};
 use smol_str::SmolStr;
 use std::cell::RefCell;
@@ -300,7 +299,7 @@ impl Compiler<'_> {
 
         // TODO: Use https://github.com/rust-lang/rfcs/issues/2208
         // once it is available
-        let value = Value::Path(path.clean());
+        let value = Value::Path(crate::value::canon_path(path));
         self.emit_constant(value, node);
     }
 
diff --git a/tvix/eval/src/value/mod.rs b/tvix/eval/src/value/mod.rs
index b8b46b444fe7..c8c8a54a40c3 100644
--- a/tvix/eval/src/value/mod.rs
+++ b/tvix/eval/src/value/mod.rs
@@ -11,6 +11,7 @@ mod attrs;
 mod builtin;
 mod function;
 mod list;
+mod path;
 mod string;
 mod thunk;
 
@@ -21,6 +22,7 @@ pub use attrs::NixAttrs;
 pub use builtin::Builtin;
 pub use function::{Closure, Lambda};
 pub use list::NixList;
+pub use path::canon_path;
 pub use string::NixString;
 pub use thunk::Thunk;
 
diff --git a/tvix/eval/src/value/path.rs b/tvix/eval/src/value/path.rs
new file mode 100644
index 000000000000..ad526a8746f8
--- /dev/null
+++ b/tvix/eval/src/value/path.rs
@@ -0,0 +1,14 @@
+use path_clean::PathClean;
+use std::path::PathBuf;
+
+/// This function should match the behavior of canonPath() in
+/// src/libutil/util.cc of cppnix.  Currently it does not match that
+/// behavior; it uses the `path_clean` library which is based on the
+/// Go standard library
+///
+/// TODO: make this match the behavior of cppnix
+/// TODO: write tests for this
+
+pub fn canon_path(path: PathBuf) -> PathBuf {
+    path.clean()
+}
diff --git a/tvix/eval/src/vm.rs b/tvix/eval/src/vm.rs
index c54c005b0041..4e92c152a8d5 100644
--- a/tvix/eval/src/vm.rs
+++ b/tvix/eval/src/vm.rs
@@ -3,8 +3,6 @@
 
 use std::{cell::RefMut, path::PathBuf, rc::Rc};
 
-use path_clean::PathClean;
-
 use crate::{
     chunk::Chunk,
     errors::{Error, ErrorKind, EvalResult},
@@ -396,7 +394,7 @@ impl<'o> VM<'o> {
                             &v.coerce_to_string(CoercionKind::Weak, self)
                                 .map_err(|ek| self.error(ek))?,
                         );
-                        PathBuf::from(path).clean().into()
+                        crate::value::canon_path(PathBuf::from(path)).into()
                     }
                     _ => fallible!(self, arithmetic_op!(&a, &b, +)),
                 };
-- 
cgit 1.4.1