diff options
author | sterni <sternenseemann@systemli.org> | 2023-12-13T13·52+0100 |
---|---|---|
committer | clbot <clbot@tvl.fyi> | 2023-12-14T13·15+0000 |
commit | 7165ebc43bfb1b53929fb31673c512bbbdbe4096 (patch) | |
tree | 99e818716faca25777ea15499ce670a506269bed /tvix/eval/src/vm | |
parent | a30dd0905a08a78bb0573136064dd334a0567f6a (diff) |
fix(tvix/eval): remove incorrect imports when coercing r/7218
The default behavior of string coercion in C++ Nix is to weakly coerce and import to store if necessary. There is a flag to make it strongly coerce (coerceMore) and a flag that controls whether path values have the corresponding file/directory imported into the store before returning the (store) path as a string (copyToStore). We need to implement our equivalent to the copyToStore (import_paths) flag for the benefit of weak coercions that don't import into the store (dirOf, baseNameOf, readFile, ...) and strong coercions that don't import into the store (toString). This makes coerce_to_string as well as CoercionKind weirder and more versatile, but prevents us from reimplementing parts of the coercion logic constantly as can be seen in the case of baseNameOf. Note that it is not possible to test this properly in //tvix/eval tests due to the lack of an appropriate EvalIO implementation being available. Tests should be added to //tvix/glue down the line. Change-Id: I8fb8ab99c7fe08e311d2ba1c36960746bf22f566 Reviewed-on: https://cl.tvl.fyi/c/depot/+/10361 Autosubmit: sterni <sternenseemann@systemli.org> Tested-by: BuildkiteCI Reviewed-by: Adam Joseph <adam@westernsemico.com>
Diffstat (limited to 'tvix/eval/src/vm')
-rw-r--r-- | tvix/eval/src/vm/generators.rs | 17 | ||||
-rw-r--r-- | tvix/eval/src/vm/mod.rs | 63 |
2 files changed, 68 insertions, 12 deletions
diff --git a/tvix/eval/src/vm/generators.rs b/tvix/eval/src/vm/generators.rs index 6563a82790a6..9686e6542f13 100644 --- a/tvix/eval/src/vm/generators.rs +++ b/tvix/eval/src/vm/generators.rs @@ -144,10 +144,19 @@ impl Display for VMRequest { } VMRequest::StackPush(v) => write!(f, "stack_push({})", v.type_of()), VMRequest::StackPop => write!(f, "stack_pop"), - VMRequest::StringCoerce(v, kind) => match kind { - CoercionKind::Weak => write!(f, "weak_string_coerce({})", v.type_of()), - CoercionKind::Strong => write!(f, "strong_string_coerce({})", v.type_of()), - }, + VMRequest::StringCoerce( + v, + CoercionKind { + strong, + import_paths, + }, + ) => write!( + f, + "{}_{}importing_string_coerce({})", + if *strong { "strong" } else { "weak" }, + if *import_paths { "" } else { "non_" }, + v.type_of() + ), VMRequest::Call(v) => write!(f, "call({})", v), VMRequest::EnterLambda { lambda, .. } => { write!(f, "enter_lambda({:p})", *lambda) diff --git a/tvix/eval/src/vm/mod.rs b/tvix/eval/src/vm/mod.rs index 902121ebc727..0ac5024d6b11 100644 --- a/tvix/eval/src/vm/mod.rs +++ b/tvix/eval/src/vm/mod.rs @@ -780,7 +780,14 @@ impl<'o> VM<'o> { self.push_call_frame(span, frame); self.enqueue_generator("coerce_to_string", gen_span.clone(), |co| { - value.coerce_to_string(co, CoercionKind::Weak, gen_span) + value.coerce_to_string( + co, + CoercionKind { + strong: false, + import_paths: true, + }, + gen_span, + ) }); return Ok(false); @@ -1206,7 +1213,23 @@ async fn add_values(co: GenCo, a: Value, b: Value) -> Result<Value, ErrorKind> { let result = match (a, b) { (Value::Path(p), v) => { let mut path = p.to_string_lossy().into_owned(); - match generators::request_string_coerce(&co, v, CoercionKind::Weak).await { + match generators::request_string_coerce( + &co, + v, + CoercionKind { + strong: false, + + // Concatenating a Path with something else results in a + // Path, so we don't need to import any paths (paths + // imported by Nix always exist as a string, unless + // converted by the user). In C++ Nix they even may not + // contain any string context, the resulting error of such a + // case can not be replicated by us. + import_paths: false, + }, + ) + .await + { Ok(vs) => { path.push_str(vs.as_str()); crate::value::canon_path(PathBuf::from(path)).into() @@ -1215,14 +1238,38 @@ async fn add_values(co: GenCo, a: Value, b: Value) -> Result<Value, ErrorKind> { } } (Value::String(s1), Value::String(s2)) => Value::String(s1.concat(&s2)), - (Value::String(s1), v) => generators::request_string_coerce(&co, v, CoercionKind::Weak) - .await - .map(|s2| Value::String(s1.concat(&s2))) - .into(), + (Value::String(s1), v) => generators::request_string_coerce( + &co, + v, + CoercionKind { + strong: false, + // Behaves the same as string interpolation + import_paths: true, + }, + ) + .await + .map(|s2| Value::String(s1.concat(&s2))) + .into(), (a @ Value::Integer(_), b) | (a @ Value::Float(_), b) => arithmetic_op!(&a, &b, +)?, (a, b) => { - let r1 = generators::request_string_coerce(&co, a, CoercionKind::Weak).await; - let r2 = generators::request_string_coerce(&co, b, CoercionKind::Weak).await; + let r1 = generators::request_string_coerce( + &co, + a, + CoercionKind { + strong: false, + import_paths: false, + }, + ) + .await; + let r2 = generators::request_string_coerce( + &co, + b, + CoercionKind { + strong: false, + import_paths: false, + }, + ) + .await; match (r1, r2) { (Ok(s1), Ok(s2)) => Value::String(s1.concat(&s2)), (Err(c), _) => return Ok(Value::Catchable(c)), |