From 05f42519b53575ad3235b5e0a0cd7d71f04076a5 Mon Sep 17 00:00:00 2001 From: Adam Joseph Date: Sat, 9 Sep 2023 22:02:56 -0700 Subject: fix(tvix/eval): fix b/281 by adding Value::Catchable This commit makes catchable errors a variant of Value. The main downside of this approach is that we lose the ability to use Rust's `?` syntax for propagating catchable errors. Change-Id: Ibe89438d8a70dcec29e016df692b5bf88a5cad13 Reviewed-on: https://cl.tvl.fyi/c/depot/+/9289 Reviewed-by: tazjin Autosubmit: Adam Joseph Tested-by: BuildkiteCI --- tvix/eval/src/builtins/impure.rs | 57 ++++++++++++++++++++++------------------ tvix/eval/src/builtins/mod.rs | 57 +++++++++++++++++++++++++++------------- tvix/eval/src/builtins/to_xml.rs | 4 +++ 3 files changed, 75 insertions(+), 43 deletions(-) (limited to 'tvix/eval/src/builtins') diff --git a/tvix/eval/src/builtins/impure.rs b/tvix/eval/src/builtins/impure.rs index 26d1ba4945..5324fd5653 100644 --- a/tvix/eval/src/builtins/impure.rs +++ b/tvix/eval/src/builtins/impure.rs @@ -27,40 +27,47 @@ mod impure_builtins { #[builtin("pathExists")] async fn builtin_path_exists(co: GenCo, path: Value) -> Result { - let path = coerce_value_to_path(&co, path).await?; - Ok(generators::request_path_exists(&co, path).await) + match coerce_value_to_path(&co, path).await? { + Err(cek) => Ok(Value::Catchable(cek)), + Ok(path) => Ok(generators::request_path_exists(&co, path).await), + } } #[builtin("readDir")] async fn builtin_read_dir(co: GenCo, path: Value) -> Result { - let path = coerce_value_to_path(&co, path).await?; + match coerce_value_to_path(&co, path).await? { + Err(cek) => Ok(Value::Catchable(cek)), + Ok(path) => { + let dir = generators::request_read_dir(&co, path).await; + let res = dir.into_iter().map(|(name, ftype)| { + ( + // TODO: propagate Vec or bytes::Bytes into NixString. + NixString::from( + String::from_utf8(name.to_vec()).expect("parsing file name as string"), + ), + Value::String( + match ftype { + FileType::Directory => "directory", + FileType::Regular => "regular", + FileType::Symlink => "symlink", + FileType::Unknown => "unknown", + } + .into(), + ), + ) + }); - let dir = generators::request_read_dir(&co, path).await; - let res = dir.into_iter().map(|(name, ftype)| { - ( - // TODO: propagate Vec or bytes::Bytes into NixString. - NixString::from( - String::from_utf8(name.to_vec()).expect("parsing file name as string"), - ), - Value::String( - match ftype { - FileType::Directory => "directory", - FileType::Regular => "regular", - FileType::Symlink => "symlink", - FileType::Unknown => "unknown", - } - .into(), - ), - ) - }); - - Ok(Value::attrs(NixAttrs::from_iter(res))) + Ok(Value::attrs(NixAttrs::from_iter(res))) + } + } } #[builtin("readFile")] async fn builtin_read_file(co: GenCo, path: Value) -> Result { - let path = coerce_value_to_path(&co, path).await?; - Ok(generators::request_read_to_string(&co, path).await) + match coerce_value_to_path(&co, path).await? { + Err(cek) => Ok(Value::Catchable(cek)), + Ok(path) => Ok(generators::request_read_to_string(&co, path).await), + } } } diff --git a/tvix/eval/src/builtins/mod.rs b/tvix/eval/src/builtins/mod.rs index 322f917108..13355fa1c5 100644 --- a/tvix/eval/src/builtins/mod.rs +++ b/tvix/eval/src/builtins/mod.rs @@ -40,18 +40,25 @@ pub const CURRENT_PLATFORM: &str = env!("TVIX_CURRENT_SYSTEM"); /// builtin. This coercion can _never_ be performed in a Nix program /// without using builtins (i.e. the trick `path: /. + path` to /// convert from a string to a path wouldn't hit this code). -pub async fn coerce_value_to_path(co: &GenCo, v: Value) -> Result { +pub async fn coerce_value_to_path( + co: &GenCo, + v: Value, +) -> Result, ErrorKind> { let value = generators::request_force(co, v).await; if let Value::Path(p) = value { - return Ok(*p); + return Ok(Ok(*p)); } - let vs = generators::request_string_coerce(co, value, CoercionKind::Weak).await; - let path = PathBuf::from(vs.as_str()); - if path.is_absolute() { - Ok(path) - } else { - Err(ErrorKind::NotAnAbsolutePath(path)) + match generators::request_string_coerce(co, value, CoercionKind::Weak).await { + Ok(vs) => { + let path = PathBuf::from(vs.as_str()); + if path.is_absolute() { + Ok(Ok(path)) + } else { + Err(ErrorKind::NotAnAbsolutePath(path)) + } + } + Err(cek) => Ok(Err(cek)), } } @@ -218,8 +225,10 @@ mod pure_builtins { if i != 0 { res.push_str(&separator); } - let s = generators::request_string_coerce(&co, val, CoercionKind::Weak).await; - res.push_str(s.as_str()); + match generators::request_string_coerce(&co, val, CoercionKind::Weak).await { + Ok(s) => res.push_str(s.as_str()), + Err(c) => return Ok(Value::Catchable(c)), + } } Ok(res.into()) } @@ -313,6 +322,9 @@ mod pure_builtins { // and our tests for foldl'. nul = generators::request_call_with(&co, op.clone(), [nul, val]).await; nul = generators::request_force(&co, nul).await; + if let c @ Value::Catchable(_) = nul { + return Ok(c); + } } Ok(nul) @@ -340,9 +352,13 @@ mod pure_builtins { #[builtin("toJSON")] async fn builtin_to_json(co: GenCo, val: Value) -> Result { - let json_value = val.to_json(&co).await?; - let json_str = serde_json::to_string(&json_value)?; - Ok(json_str.into()) + match val.to_json(&co).await? { + Err(cek) => Ok(Value::Catchable(cek)), + Ok(json_value) => { + let json_str = serde_json::to_string(&json_value)?; + Ok(json_str.into()) + } + } } #[builtin("fromTOML")] @@ -893,7 +909,7 @@ mod pure_builtins { #[builtin("throw")] async fn builtin_throw(co: GenCo, message: Value) -> Result { - Err(ErrorKind::CatchableErrorKind(CatchableErrorKind::Throw( + Ok(Value::Catchable(CatchableErrorKind::Throw( message.to_str()?.to_string(), ))) } @@ -929,15 +945,20 @@ mod pure_builtins { #[builtin("toPath")] async fn builtin_to_path(co: GenCo, s: Value) -> Result { - let path: Value = crate::value::canon_path(coerce_value_to_path(&co, s).await?).into(); - Ok(path.coerce_to_string(co, CoercionKind::Weak).await?) + match coerce_value_to_path(&co, s).await? { + Err(cek) => return Ok(Value::Catchable(cek)), + Ok(path) => { + let path: Value = crate::value::canon_path(path).into(); + Ok(path.coerce_to_string(co, CoercionKind::Weak).await?) + } + } } #[builtin("tryEval")] async fn builtin_try_eval(co: GenCo, #[lazy] e: Value) -> Result { let res = match generators::request_try_force(&co, e).await { - Some(value) => [("value", value), ("success", true.into())], - None => [("value", false.into()), ("success", false.into())], + Value::Catchable(_) => [("value", false.into()), ("success", false.into())], + value => [("value", value), ("success", true.into())], }; Ok(Value::attrs(NixAttrs::from_iter(res.into_iter()))) diff --git a/tvix/eval/src/builtins/to_xml.rs b/tvix/eval/src/builtins/to_xml.rs index 9fb6bb5f51..6d486d3565 100644 --- a/tvix/eval/src/builtins/to_xml.rs +++ b/tvix/eval/src/builtins/to_xml.rs @@ -134,6 +134,10 @@ fn value_variant_to_xml(w: &mut EventWriter, value: &Value) -> Resu metadata: Some(Rc::new(value.clone())), }) } + + Value::Catchable(_) => { + panic!("tvix bug: value_to_xml() called on a value which had not been deep-forced") + } }?; Ok(()) -- cgit 1.4.1