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/cli/src/derivation.rs | 116 +++++++++++++++++++++++++++++---------------- 1 file changed, 75 insertions(+), 41 deletions(-) (limited to 'tvix/cli/src') diff --git a/tvix/cli/src/derivation.rs b/tvix/cli/src/derivation.rs index de6d58a13db2..76cc3f60f77e 100644 --- a/tvix/cli/src/derivation.rs +++ b/tvix/cli/src/derivation.rs @@ -6,7 +6,9 @@ use std::collections::{btree_map, BTreeSet}; use std::rc::Rc; use tvix_eval::builtin_macros::builtins; use tvix_eval::generators::{self, emit_warning_kind, GenCo}; -use tvix_eval::{AddContext, CoercionKind, ErrorKind, NixAttrs, NixList, Value, WarningKind}; +use tvix_eval::{ + AddContext, CatchableErrorKind, CoercionKind, ErrorKind, NixAttrs, NixList, Value, WarningKind, +}; use crate::errors::Error; use crate::known_paths::{KnownPaths, PathKind, PathName}; @@ -150,19 +152,22 @@ async fn handle_derivation_parameters( name: &str, value: &Value, val_str: &str, -) -> Result { +) -> Result, ErrorKind> { match name { - IGNORE_NULLS => return Ok(false), + IGNORE_NULLS => return Ok(Ok(false)), // Command line arguments to the builder. "args" => { let args = value.to_list()?; for arg in args { - drv.arguments.push(strong_coerce_to_string(co, arg).await?); + match strong_coerce_to_string(co, arg).await? { + Err(cek) => return Ok(Err(cek)), + Ok(s) => drv.arguments.push(s), + } } // The arguments do not appear in the environment. - return Ok(false); + return Ok(Ok(false)); } // Explicitly specified drv outputs (instead of default [ "out" ]) @@ -185,14 +190,18 @@ async fn handle_derivation_parameters( _ => {} } - Ok(true) + Ok(Ok(true)) } -async fn strong_coerce_to_string(co: &GenCo, val: Value) -> Result { +async fn strong_coerce_to_string( + co: &GenCo, + val: Value, +) -> Result, ErrorKind> { let val = generators::request_force(co, val).await; - let val_str = generators::request_string_coerce(co, val, CoercionKind::Strong).await; - - Ok(val_str.as_str().to_string()) + match generators::request_string_coerce(co, val, CoercionKind::Strong).await { + Err(cek) => Ok(Err(cek)), + Ok(val_str) => Ok(Ok(val_str.as_str().to_string())), + } } #[builtins(state = "Rc>")] @@ -256,12 +265,15 @@ mod derivation_builtins { co: &GenCo, attrs: &NixAttrs, key: &str, - ) -> Result, ErrorKind> { + ) -> Result, CatchableErrorKind>, ErrorKind> { if let Some(attr) = attrs.select(key) { - return Ok(Some(strong_coerce_to_string(co, attr.clone()).await?)); + match strong_coerce_to_string(co, attr.clone()).await? { + Err(cek) => return Ok(Err(cek)), + Ok(str) => return Ok(Ok(Some(str))), + } } - Ok(None) + Ok(Ok(None)) } for (name, value) in input.clone().into_iter_sorted() { @@ -270,38 +282,60 @@ mod derivation_builtins { continue; } - let val_str = strong_coerce_to_string(&co, value.clone()).await?; - - // handle_derivation_parameters tells us whether the - // argument should be added to the environment; continue - // to the next one otherwise - if !handle_derivation_parameters(&mut drv, &co, name.as_str(), &value, &val_str).await? - { - continue; - } + match strong_coerce_to_string(&co, value.clone()).await? { + Err(cek) => return Ok(Value::Catchable(cek)), + Ok(val_str) => { + // handle_derivation_parameters tells us whether the + // argument should be added to the environment; continue + // to the next one otherwise + match handle_derivation_parameters( + &mut drv, + &co, + name.as_str(), + &value, + &val_str, + ) + .await? + { + Err(cek) => return Ok(Value::Catchable(cek)), + Ok(false) => continue, + _ => (), + } - // Most of these are also added to the builder's environment in "raw" form. - if drv - .environment - .insert(name.as_str().to_string(), val_str.into()) - .is_some() - { - return Err(Error::DuplicateEnvVar(name.as_str().to_string()).into()); + // Most of these are also added to the builder's environment in "raw" form. + if drv + .environment + .insert(name.as_str().to_string(), val_str.into()) + .is_some() + { + return Err(Error::DuplicateEnvVar(name.as_str().to_string()).into()); + } + } } } - populate_output_configuration( - &mut drv, - select_string(&co, &input, "outputHash") - .await - .context("evaluating the `outputHash` parameter")?, - select_string(&co, &input, "outputHashAlgo") - .await - .context("evaluating the `outputHashAlgo` parameter")?, - select_string(&co, &input, "outputHashMode") - .await - .context("evaluating the `outputHashMode` parameter")?, - )?; + let output_hash = match select_string(&co, &input, "outputHash") + .await + .context("evaluating the `outputHash` parameter")? + { + Err(cek) => return Ok(Value::Catchable(cek)), + Ok(s) => s, + }; + let output_hash_algo = match select_string(&co, &input, "outputHashAlgo") + .await + .context("evaluating the `outputHashAlgo` parameter")? + { + Err(cek) => return Ok(Value::Catchable(cek)), + Ok(s) => s, + }; + let output_hash_mode = match select_string(&co, &input, "outputHashMode") + .await + .context("evaluating the `outputHashMode` parameter")? + { + Err(cek) => return Ok(Value::Catchable(cek)), + Ok(s) => s, + }; + populate_output_configuration(&mut drv, output_hash, output_hash_algo, output_hash_mode)?; // Scan references in relevant attributes to detect any build-references. let references = { -- cgit 1.4.1