From 4dcb8f38c2e442b6433b21b8a7de303736a04568 Mon Sep 17 00:00:00 2001 From: sterni Date: Sat, 15 Oct 2022 16:42:27 +0200 Subject: fix(tvix/eval): resolve home relative paths at runtime Home relative paths depend on the environment to be resolved. We have elected to do everything that depends on the environment, e.g. resolving SPATH expressions using NIX_PATH, at runtime, so tvix evaluation would continue to behave correctly even if we separated the compilation and execution phases more, e.g. via serializing bytecode. Then the value of HOME, NIX_PATH etc. could reasonably change in the time until execution, yielding wrong results if the resolution results were cached in the bytecode. We also take the opportunity to fix the broken path concatenation previously found in the compiler, fixing b/205. Another thing we could consider is emitting a warning for home relative path literals, as they are by nature relatively fragile. One sideeffect of this change is that home path resolution errors become catchable which is not the case in C++ Nix. This will need to be fixed up in a subsequent change. Change-Id: I30bd69b575667c49170a9fdea23a020565d0f9ec Reviewed-on: https://cl.tvl.fyi/c/depot/+/7024 Autosubmit: sterni Reviewed-by: Adam Joseph Tested-by: BuildkiteCI --- tvix/eval/src/compiler/mod.rs | 20 ++++++++------------ tvix/eval/src/opcode.rs | 3 +++ tvix/eval/src/vm.rs | 20 ++++++++++++++++++++ 3 files changed, 31 insertions(+), 12 deletions(-) diff --git a/tvix/eval/src/compiler/mod.rs b/tvix/eval/src/compiler/mod.rs index 11537e76b5..95de694870 100644 --- a/tvix/eval/src/compiler/mod.rs +++ b/tvix/eval/src/compiler/mod.rs @@ -268,19 +268,15 @@ impl Compiler<'_> { let path = if raw_path.starts_with('/') { Path::new(&raw_path).to_owned() } else if raw_path.starts_with('~') { - let mut buf = match dirs::home_dir() { - Some(buf) => buf, - None => { - self.emit_error( - node, - ErrorKind::PathResolution("failed to determine home directory".into()), - ); - return; - } - }; + return self.thunk(slot, node, move |c, _| { + // We assume that paths that home paths start with ~/ or fail to parse + // TODO: this should be checked using a parse-fail test. + debug_assert!(raw_path.len() > 2 && raw_path.starts_with("~/")); - buf.push(&raw_path); - buf + let home_relative_path = &raw_path[2..(raw_path.len())]; + c.emit_constant(Value::UnresolvedPath(home_relative_path.into()), node); + c.push_op(OpCode::OpResolveHomePath, node); + }); } else if raw_path.starts_with('.') { let mut buf = self.root_dir.clone(); buf.push(&raw_path); diff --git a/tvix/eval/src/opcode.rs b/tvix/eval/src/opcode.rs index fdc916ca7f..70ef1a3b53 100644 --- a/tvix/eval/src/opcode.rs +++ b/tvix/eval/src/opcode.rs @@ -117,6 +117,9 @@ pub enum OpCode { /// [`NixSearchPath`]: crate::nix_search_path::NixSearchPath OpFindFile, + /// Attempt to resolve a path literal relative to the home dir + OpResolveHomePath, + // Type assertion operators OpAssertBool, diff --git a/tvix/eval/src/vm.rs b/tvix/eval/src/vm.rs index 05adec5f6e..7fcdb9ea73 100644 --- a/tvix/eval/src/vm.rs +++ b/tvix/eval/src/vm.rs @@ -536,6 +536,26 @@ impl<'o> VM<'o> { _ => panic!("tvix compiler bug: OpFindFile called on non-UnresolvedPath"), }, + OpCode::OpResolveHomePath => match self.pop() { + Value::UnresolvedPath(path) => { + match dirs::home_dir() { + None => { + return Err(self.error(ErrorKind::PathResolution( + "failed to determine home directory".into(), + ))); + } + Some(mut buf) => { + buf.push(path); + self.push(buf.into()); + } + }; + } + + _ => { + panic!("tvix compiler bug: OpResolveHomePath called on non-UnresolvedPath") + } + }, + OpCode::OpJump(JumpOffset(offset)) => { debug_assert!(offset != 0); self.frame_mut().ip += offset; -- cgit 1.4.1