about summary refs log tree commit diff
path: root/tvix/cli
diff options
context:
space:
mode:
authorVincent Ambo <mail@tazj.in>2023-02-14T12·02+0300
committertazjin <tazjin@tvl.su>2023-03-13T20·30+0000
commit025c67bf4d5666411b4d6cdc929e1a677ebc0439 (patch)
treee6e683d5c194686c8112518965f7e11458b18f27 /tvix/cli
parentcbb4137dc08620af9d6360057c75891bf4d03b5f (diff)
refactor(tvix/eval): flatten call stack of VM using generators r/5964
Warning: This is probably the biggest refactor in tvix-eval history,
so far.

This replaces all instances of trampolines and recursion during
evaluation of the VM loop with generators. A generator is an
asynchronous function that can be suspended to yield a message (in our
case, vm::generators::GeneratorRequest) and receive a
response (vm::generators::GeneratorResponsee).

The `genawaiter` crate provides an interpreter for generators that can
drive their execution and lets us move control flow between the VM and
suspended generators.

To do this, massive changes have occured basically everywhere in the
code. On a high-level:

1. The VM is now organised around a frame stack. A frame is either a
   call frame (execution of Tvix bytecode) or a generator frame (a
   running or suspended generator).

   The VM has an outer loop that pops a frame off the frame stack, and
   then enters an inner loop either driving the execution of the
   bytecode or the execution of a generator.

   Both types of frames have several branches that can result in the
   frame re-enqueuing itself, and enqueuing some other work (in the
   form of a different frame) on top of itself. The VM will eventually
   resume the frame when everything "above" it has been suspended.

   In this way, the VM's new frame stack takes over much of the work
   that was previously achieved by recursion.

2. All methods previously taking a VM have been refactored into async
   functions that instead emit/receive generator messages for
   communication with the VM.

   Notably, this includes *all* builtins.

This has had some other effects:

- Some test have been removed or commented out, either because they
  tested code that was mostly already dead (nix_eq) or because they
  now require generator scaffolding which we do not have in place for
  tests (yet).

- Because generator functions are technically async (though no async
  IO is involved), we lose the ability to use much of the Rust
  standard library e.g. in builtins. This has led to many algorithms
  being unrolled into iterative versions instead of iterator
  combinations, and things like sorting had to be implemented from scratch.

- Many call sites that previously saw a `Result<..., ErrorKind>`
  bubble up now only see the result value, as the error handling is
  encapsulated within the generator loop.

  This reduces number of places inside of builtin implementations
  where error context can be attached to calls that can fail.
  Currently what we gain in this tradeoff is significantly more
  detailed span information (which we still need to bubble up, this
  commit does not change the error display).

  We'll need to do some analysis later of how useful the errors turn
  out to be and potentially introduce some methods for attaching
  context to a generator frame again.

This change is very difficult to do in stages, as it is very much an
"all or nothing" change that affects huge parts of the codebase. I've
tried to isolate changes that can be isolated into the parent CLs of
this one, but this change is still quite difficult to wrap one's mind
and I'm available to discuss it and explain things to any reviewer.

Fixes: b/238, b/237, b/251 and potentially others.
Change-Id: I39244163ff5bbecd169fe7b274df19262b515699
Reviewed-on: https://cl.tvl.fyi/c/depot/+/8104
Reviewed-by: raitobezarius <tvl@lahfa.xyz>
Reviewed-by: Adam Joseph <adam@westernsemico.com>
Tested-by: BuildkiteCI
Diffstat (limited to 'tvix/cli')
-rw-r--r--tvix/cli/src/derivation.rs590
-rw-r--r--tvix/cli/src/main.rs2
2 files changed, 301 insertions, 291 deletions
diff --git a/tvix/cli/src/derivation.rs b/tvix/cli/src/derivation.rs
index 88c5e52296..15c4c6f858 100644
--- a/tvix/cli/src/derivation.rs
+++ b/tvix/cli/src/derivation.rs
@@ -5,7 +5,8 @@ use std::cell::RefCell;
 use std::collections::{btree_map, BTreeSet};
 use std::rc::Rc;
 use tvix_eval::builtin_macros::builtins;
-use tvix_eval::{AddContext, CoercionKind, ErrorKind, NixAttrs, NixList, Value, VM};
+use tvix_eval::generators::{self, GenCo};
+use tvix_eval::{AddContext, CoercionKind, ErrorKind, NixAttrs, NixList, Value};
 
 use crate::errors::Error;
 use crate::known_paths::{KnownPaths, PathKind, PathName};
@@ -17,13 +18,17 @@ const IGNORE_NULLS: &str = "__ignoreNulls";
 /// Helper function for populating the `drv.outputs` field from a
 /// manually specified set of outputs, instead of the default
 /// `outputs`.
-fn populate_outputs(vm: &mut VM, drv: &mut Derivation, outputs: NixList) -> Result<(), ErrorKind> {
+async fn populate_outputs(
+    co: &GenCo,
+    drv: &mut Derivation,
+    outputs: NixList,
+) -> Result<(), ErrorKind> {
     // Remove the original default `out` output.
     drv.outputs.clear();
 
     for output in outputs {
-        let output_name = output
-            .force(vm)?
+        let output_name = generators::request_force(co, output)
+            .await
             .to_str()
             .context("determining output name")?;
 
@@ -144,9 +149,9 @@ fn populate_output_configuration(
 /// Handles derivation parameters which are not just forwarded to
 /// the environment. The return value indicates whether the
 /// parameter should be included in the environment.
-fn handle_derivation_parameters(
+async fn handle_derivation_parameters(
     drv: &mut Derivation,
-    vm: &mut VM,
+    co: &GenCo,
     name: &str,
     value: &Value,
     val_str: &str,
@@ -158,11 +163,7 @@ fn handle_derivation_parameters(
         "args" => {
             let args = value.to_list()?;
             for arg in args {
-                drv.arguments.push(strong_coerce_to_string(
-                    vm,
-                    &arg,
-                    "handling command-line builder arguments",
-                )?);
+                drv.arguments.push(strong_coerce_to_string(co, arg).await?);
             }
 
             // The arguments do not appear in the environment.
@@ -176,7 +177,7 @@ fn handle_derivation_parameters(
                 .context("looking at the `outputs` parameter of the derivation")?;
 
             drv.outputs.clear();
-            populate_outputs(vm, drv, outputs)?;
+            populate_outputs(co, drv, outputs).await?;
         }
 
         "builder" => {
@@ -193,22 +194,20 @@ fn handle_derivation_parameters(
     Ok(true)
 }
 
-fn strong_coerce_to_string(vm: &mut VM, val: &Value, ctx: &str) -> Result<String, ErrorKind> {
-    Ok(val
-        .force(vm)
-        .context(ctx)?
-        .coerce_to_string(CoercionKind::Strong, vm)
-        .context(ctx)?
-        .as_str()
-        .to_string())
+async fn strong_coerce_to_string(co: &GenCo, val: Value) -> Result<String, 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())
 }
 
 #[builtins(state = "Rc<RefCell<KnownPaths>>")]
 mod derivation_builtins {
     use super::*;
+    use tvix_eval::generators::Gen;
 
     #[builtin("placeholder")]
-    fn builtin_placeholder(_: &mut VM, input: Value) -> Result<Value, ErrorKind> {
+    async fn builtin_placeholder(co: GenCo, input: Value) -> Result<Value, ErrorKind> {
         let placeholder = hash_placeholder(
             input
                 .to_str()
@@ -224,29 +223,28 @@ mod derivation_builtins {
     /// This is considered an internal function, users usually want to
     /// use the higher-level `builtins.derivation` instead.
     #[builtin("derivationStrict")]
-    fn builtin_derivation_strict(
+    async fn builtin_derivation_strict(
         state: Rc<RefCell<KnownPaths>>,
-        vm: &mut VM,
+        co: GenCo,
         input: Value,
     ) -> Result<Value, ErrorKind> {
         let input = input.to_attrs()?;
-        let name = input
-            .select_required("name")?
-            .force(vm)?
+        let name = generators::request_force(&co, input.select_required("name")?.clone())
+            .await
             .to_str()
             .context("determining derivation name")?;
 
         // Check whether attributes should be passed as a JSON file.
         // TODO: the JSON serialisation has to happen here.
         if let Some(sa) = input.select(STRUCTURED_ATTRS) {
-            if sa.force(vm)?.as_bool()? {
+            if generators::request_force(&co, sa.clone()).await.as_bool()? {
                 return Err(ErrorKind::NotImplemented(STRUCTURED_ATTRS));
             }
         }
 
         // Check whether null attributes should be ignored or passed through.
         let ignore_nulls = match input.select(IGNORE_NULLS) {
-            Some(b) => b.force(vm)?.as_bool()?,
+            Some(b) => generators::request_force(&co, b.clone()).await.as_bool()?,
             None => false,
         };
 
@@ -254,37 +252,45 @@ mod derivation_builtins {
         drv.outputs.insert("out".to_string(), Default::default());
 
         // Configure fixed-output derivations if required.
+
+        async fn select_string(
+            co: &GenCo,
+            attrs: &NixAttrs,
+            key: &str,
+        ) -> Result<Option<String>, ErrorKind> {
+            if let Some(attr) = attrs.select(key) {
+                return Ok(Some(strong_coerce_to_string(co, attr.clone()).await?));
+            }
+
+            Ok(None)
+        }
+
         populate_output_configuration(
             &mut drv,
-            input
-                .select("outputHash")
-                .map(|v| strong_coerce_to_string(vm, v, "evaluating the `outputHash` parameter"))
-                .transpose()?,
-            input
-                .select("outputHashAlgo")
-                .map(|v| {
-                    strong_coerce_to_string(vm, v, "evaluating the `outputHashAlgo` parameter")
-                })
-                .transpose()?,
-            input
-                .select("outputHashMode")
-                .map(|v| {
-                    strong_coerce_to_string(vm, v, "evaluating the `outputHashMode` parameter")
-                })
-                .transpose()?,
+            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")?,
         )?;
 
         for (name, value) in input.into_iter_sorted() {
-            if ignore_nulls && matches!(*value.force(vm)?, Value::Null) {
+            let value = generators::request_force(&co, value).await;
+            if ignore_nulls && matches!(value, Value::Null) {
                 continue;
             }
 
-            let val_str = strong_coerce_to_string(vm, &value, "evaluating derivation attributes")?;
+            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, vm, name.as_str(), &value, &val_str)? {
+            if !handle_derivation_parameters(&mut drv, &co, name.as_str(), &value, &val_str).await?
+            {
                 continue;
             }
 
@@ -375,9 +381,9 @@ mod derivation_builtins {
     }
 
     #[builtin("toFile")]
-    fn builtin_to_file(
+    async fn builtin_to_file(
         state: Rc<RefCell<KnownPaths>>,
-        _: &mut VM,
+        co: GenCo,
         name: Value,
         content: Value,
     ) -> Result<Value, ErrorKind> {
@@ -421,247 +427,251 @@ mod tests {
     use super::*;
     use tvix_eval::observer::NoOpObserver;
 
-    static mut OBSERVER: NoOpObserver = NoOpObserver {};
-
-    // Creates a fake VM for tests, which can *not* actually be
-    // used to force (most) values but can satisfy the type
-    // parameter.
-    fn fake_vm() -> VM<'static> {
-        // safe because accessing the observer doesn't actually do anything
-        unsafe {
-            VM::new(
-                Default::default(),
-                Box::new(tvix_eval::DummyIO),
-                &mut OBSERVER,
-                Default::default(),
-            )
-        }
-    }
-
-    #[test]
-    fn populate_outputs_ok() {
-        let mut vm = fake_vm();
-        let mut drv = Derivation::default();
-        drv.outputs.insert("out".to_string(), Default::default());
-
-        let outputs = NixList::construct(
-            2,
-            vec![Value::String("foo".into()), Value::String("bar".into())],
-        );
-
-        populate_outputs(&mut vm, &mut drv, outputs).expect("populate_outputs should succeed");
-
-        assert_eq!(drv.outputs.len(), 2);
-        assert!(drv.outputs.contains_key("bar"));
-        assert!(drv.outputs.contains_key("foo"));
-    }
-
-    #[test]
-    fn populate_outputs_duplicate() {
-        let mut vm = fake_vm();
-        let mut drv = Derivation::default();
-        drv.outputs.insert("out".to_string(), Default::default());
-
-        let outputs = NixList::construct(
-            2,
-            vec![Value::String("foo".into()), Value::String("foo".into())],
-        );
-
-        populate_outputs(&mut vm, &mut drv, outputs)
-            .expect_err("supplying duplicate outputs should fail");
-    }
-
-    #[test]
-    fn populate_inputs_empty() {
-        let mut drv = Derivation::default();
-        let paths = KnownPaths::default();
-        let inputs = vec![];
-
-        populate_inputs(&mut drv, &paths, inputs);
-
-        assert!(drv.input_sources.is_empty());
-        assert!(drv.input_derivations.is_empty());
-    }
-
-    #[test]
-    fn populate_inputs_all() {
-        let mut drv = Derivation::default();
-
-        let mut paths = KnownPaths::default();
-        paths.plain("/nix/store/fn7zvafq26f0c8b17brs7s95s10ibfzs-foo");
-        paths.drv(
-            "/nix/store/aqffiyqx602lbam7n1zsaz3yrh6v08pc-bar.drv",
-            &["out"],
-        );
-        paths.output(
-            "/nix/store/zvpskvjwi72fjxg0vzq822sfvq20mq4l-bar",
-            "out",
-            "/nix/store/aqffiyqx602lbam7n1zsaz3yrh6v08pc-bar.drv",
-        );
-
-        let inputs = vec![
-            "/nix/store/fn7zvafq26f0c8b17brs7s95s10ibfzs-foo".into(),
-            "/nix/store/aqffiyqx602lbam7n1zsaz3yrh6v08pc-bar.drv".into(),
-            "/nix/store/zvpskvjwi72fjxg0vzq822sfvq20mq4l-bar".into(),
-        ];
-
-        populate_inputs(&mut drv, &paths, inputs);
-
-        assert_eq!(drv.input_sources.len(), 1);
-        assert!(drv
-            .input_sources
-            .contains("/nix/store/fn7zvafq26f0c8b17brs7s95s10ibfzs-foo"));
-
-        assert_eq!(drv.input_derivations.len(), 1);
-        assert!(drv
-            .input_derivations
-            .contains_key("/nix/store/aqffiyqx602lbam7n1zsaz3yrh6v08pc-bar.drv"));
-    }
-
-    #[test]
-    fn populate_output_config_std() {
-        let mut drv = Derivation::default();
-
-        populate_output_configuration(&mut drv, None, None, None)
-            .expect("populate_output_configuration() should succeed");
-
-        assert_eq!(drv, Derivation::default(), "derivation should be unchanged");
-    }
-
-    #[test]
-    fn populate_output_config_fod() {
-        let mut drv = Derivation::default();
-        drv.outputs.insert("out".to_string(), Default::default());
-
-        populate_output_configuration(
-            &mut drv,
-            Some("0000000000000000000000000000000000000000000000000000000000000000".into()),
-            Some("sha256".into()),
-            None,
-        )
-        .expect("populate_output_configuration() should succeed");
-
-        let expected = Hash {
-            algo: "sha256".into(),
-            digest: "0000000000000000000000000000000000000000000000000000000000000000".into(),
-        };
-
-        assert_eq!(drv.outputs["out"].hash, Some(expected));
-    }
-
-    #[test]
-    fn populate_output_config_fod_recursive() {
-        let mut drv = Derivation::default();
-        drv.outputs.insert("out".to_string(), Default::default());
-
-        populate_output_configuration(
-            &mut drv,
-            Some("0000000000000000000000000000000000000000000000000000000000000000".into()),
-            Some("sha256".into()),
-            Some("recursive".into()),
-        )
-        .expect("populate_output_configuration() should succeed");
-
-        let expected = Hash {
-            algo: "r:sha256".into(),
-            digest: "0000000000000000000000000000000000000000000000000000000000000000".into(),
-        };
-
-        assert_eq!(drv.outputs["out"].hash, Some(expected));
-    }
-
-    #[test]
-    /// hash_algo set to sha256, but SRI hash passed
-    fn populate_output_config_flat_sri_sha256() {
-        let mut drv = Derivation::default();
-        drv.outputs.insert("out".to_string(), Default::default());
-
-        populate_output_configuration(
-            &mut drv,
-            Some("sha256-swapHA/ZO8QoDPwumMt6s5gf91oYe+oyk4EfRSyJqMg=".into()),
-            Some("sha256".into()),
-            Some("flat".into()),
-        )
-        .expect("populate_output_configuration() should succeed");
-
-        let expected = Hash {
-            algo: "sha256".into(),
-            digest: "b306a91c0fd93bc4280cfc2e98cb7ab3981ff75a187bea3293811f452c89a8c8".into(), // lower hex
-        };
-
-        assert_eq!(drv.outputs["out"].hash, Some(expected));
-    }
-
-    #[test]
-    /// hash_algo set to empty string, SRI hash passed
-    fn populate_output_config_flat_sri() {
-        let mut drv = Derivation::default();
-        drv.outputs.insert("out".to_string(), Default::default());
-
-        populate_output_configuration(
-            &mut drv,
-            Some("sha256-s6JN6XqP28g1uYMxaVAQMLiXcDG8tUs7OsE3QPhGqzA=".into()),
-            Some("".into()),
-            Some("flat".into()),
-        )
-        .expect("populate_output_configuration() should succeed");
-
-        let expected = Hash {
-            algo: "sha256".into(),
-            digest: "b3a24de97a8fdbc835b9833169501030b8977031bcb54b3b3ac13740f846ab30".into(), // lower hex
-        };
-
-        assert_eq!(drv.outputs["out"].hash, Some(expected));
-    }
-
-    #[test]
-    fn handle_outputs_parameter() {
-        let mut vm = fake_vm();
-        let mut drv = Derivation::default();
-        drv.outputs.insert("out".to_string(), Default::default());
-
-        let outputs = Value::List(NixList::construct(
-            2,
-            vec![Value::String("foo".into()), Value::String("bar".into())],
-        ));
-        let outputs_str = outputs
-            .coerce_to_string(CoercionKind::Strong, &mut vm)
-            .unwrap();
-
-        handle_derivation_parameters(&mut drv, &mut vm, "outputs", &outputs, outputs_str.as_str())
-            .expect("handling 'outputs' parameter should succeed");
-
-        assert_eq!(drv.outputs.len(), 2);
-        assert!(drv.outputs.contains_key("bar"));
-        assert!(drv.outputs.contains_key("foo"));
-    }
-
-    #[test]
-    fn handle_args_parameter() {
-        let mut vm = fake_vm();
-        let mut drv = Derivation::default();
-
-        let args = Value::List(NixList::construct(
-            3,
-            vec![
-                Value::String("--foo".into()),
-                Value::String("42".into()),
-                Value::String("--bar".into()),
-            ],
-        ));
-
-        let args_str = args
-            .coerce_to_string(CoercionKind::Strong, &mut vm)
-            .unwrap();
-
-        handle_derivation_parameters(&mut drv, &mut vm, "args", &args, args_str.as_str())
-            .expect("handling 'args' parameter should succeed");
-
-        assert_eq!(
-            drv.arguments,
-            vec!["--foo".to_string(), "42".to_string(), "--bar".to_string()]
-        );
-    }
+    // TODO: These tests are commented out because we do not have
+    // scaffolding to drive generators during testing at the moment.
+
+    // static mut OBSERVER: NoOpObserver = NoOpObserver {};
+
+    // // Creates a fake VM for tests, which can *not* actually be
+    // // used to force (most) values but can satisfy the type
+    // // parameter.
+    // fn fake_vm() -> VM<'static> {
+    //     // safe because accessing the observer doesn't actually do anything
+    //     unsafe {
+    //         VM::new(
+    //             Default::default(),
+    //             Box::new(tvix_eval::DummyIO),
+    //             &mut OBSERVER,
+    //             Default::default(),
+    //             todo!(),
+    //         )
+    //     }
+    // }
+
+    // #[test]
+    // fn populate_outputs_ok() {
+    //     let mut vm = fake_vm();
+    //     let mut drv = Derivation::default();
+    //     drv.outputs.insert("out".to_string(), Default::default());
+
+    //     let outputs = NixList::construct(
+    //         2,
+    //         vec![Value::String("foo".into()), Value::String("bar".into())],
+    //     );
+
+    //     populate_outputs(&mut vm, &mut drv, outputs).expect("populate_outputs should succeed");
+
+    //     assert_eq!(drv.outputs.len(), 2);
+    //     assert!(drv.outputs.contains_key("bar"));
+    //     assert!(drv.outputs.contains_key("foo"));
+    // }
+
+    // #[test]
+    // fn populate_outputs_duplicate() {
+    //     let mut vm = fake_vm();
+    //     let mut drv = Derivation::default();
+    //     drv.outputs.insert("out".to_string(), Default::default());
+
+    //     let outputs = NixList::construct(
+    //         2,
+    //         vec![Value::String("foo".into()), Value::String("foo".into())],
+    //     );
+
+    //     populate_outputs(&mut vm, &mut drv, outputs)
+    //         .expect_err("supplying duplicate outputs should fail");
+    // }
+
+    // #[test]
+    // fn populate_inputs_empty() {
+    //     let mut drv = Derivation::default();
+    //     let paths = KnownPaths::default();
+    //     let inputs = vec![];
+
+    //     populate_inputs(&mut drv, &paths, inputs);
+
+    //     assert!(drv.input_sources.is_empty());
+    //     assert!(drv.input_derivations.is_empty());
+    // }
+
+    // #[test]
+    // fn populate_inputs_all() {
+    //     let mut drv = Derivation::default();
+
+    //     let mut paths = KnownPaths::default();
+    //     paths.plain("/nix/store/fn7zvafq26f0c8b17brs7s95s10ibfzs-foo");
+    //     paths.drv(
+    //         "/nix/store/aqffiyqx602lbam7n1zsaz3yrh6v08pc-bar.drv",
+    //         &["out"],
+    //     );
+    //     paths.output(
+    //         "/nix/store/zvpskvjwi72fjxg0vzq822sfvq20mq4l-bar",
+    //         "out",
+    //         "/nix/store/aqffiyqx602lbam7n1zsaz3yrh6v08pc-bar.drv",
+    //     );
+
+    //     let inputs = vec![
+    //         "/nix/store/fn7zvafq26f0c8b17brs7s95s10ibfzs-foo".into(),
+    //         "/nix/store/aqffiyqx602lbam7n1zsaz3yrh6v08pc-bar.drv".into(),
+    //         "/nix/store/zvpskvjwi72fjxg0vzq822sfvq20mq4l-bar".into(),
+    //     ];
+
+    //     populate_inputs(&mut drv, &paths, inputs);
+
+    //     assert_eq!(drv.input_sources.len(), 1);
+    //     assert!(drv
+    //         .input_sources
+    //         .contains("/nix/store/fn7zvafq26f0c8b17brs7s95s10ibfzs-foo"));
+
+    //     assert_eq!(drv.input_derivations.len(), 1);
+    //     assert!(drv
+    //         .input_derivations
+    //         .contains_key("/nix/store/aqffiyqx602lbam7n1zsaz3yrh6v08pc-bar.drv"));
+    // }
+
+    // #[test]
+    // fn populate_output_config_std() {
+    //     let mut drv = Derivation::default();
+
+    //     populate_output_configuration(&mut drv, None, None, None)
+    //         .expect("populate_output_configuration() should succeed");
+
+    //     assert_eq!(drv, Derivation::default(), "derivation should be unchanged");
+    // }
+
+    // #[test]
+    // fn populate_output_config_fod() {
+    //     let mut drv = Derivation::default();
+    //     drv.outputs.insert("out".to_string(), Default::default());
+
+    //     populate_output_configuration(
+    //         &mut drv,
+    //         Some("0000000000000000000000000000000000000000000000000000000000000000".into()),
+    //         Some("sha256".into()),
+    //         None,
+    //     )
+    //     .expect("populate_output_configuration() should succeed");
+
+    //     let expected = Hash {
+    //         algo: "sha256".into(),
+    //         digest: "0000000000000000000000000000000000000000000000000000000000000000".into(),
+    //     };
+
+    //     assert_eq!(drv.outputs["out"].hash, Some(expected));
+    // }
+
+    // #[test]
+    // fn populate_output_config_fod_recursive() {
+    //     let mut drv = Derivation::default();
+    //     drv.outputs.insert("out".to_string(), Default::default());
+
+    //     populate_output_configuration(
+    //         &mut drv,
+    //         Some("0000000000000000000000000000000000000000000000000000000000000000".into()),
+    //         Some("sha256".into()),
+    //         Some("recursive".into()),
+    //     )
+    //     .expect("populate_output_configuration() should succeed");
+
+    //     let expected = Hash {
+    //         algo: "r:sha256".into(),
+    //         digest: "0000000000000000000000000000000000000000000000000000000000000000".into(),
+    //     };
+
+    //     assert_eq!(drv.outputs["out"].hash, Some(expected));
+    // }
+
+    // #[test]
+    // /// hash_algo set to sha256, but SRI hash passed
+    // fn populate_output_config_flat_sri_sha256() {
+    //     let mut drv = Derivation::default();
+    //     drv.outputs.insert("out".to_string(), Default::default());
+
+    //     populate_output_configuration(
+    //         &mut drv,
+    //         Some("sha256-swapHA/ZO8QoDPwumMt6s5gf91oYe+oyk4EfRSyJqMg=".into()),
+    //         Some("sha256".into()),
+    //         Some("flat".into()),
+    //     )
+    //     .expect("populate_output_configuration() should succeed");
+
+    //     let expected = Hash {
+    //         algo: "sha256".into(),
+    //         digest: "b306a91c0fd93bc4280cfc2e98cb7ab3981ff75a187bea3293811f452c89a8c8".into(), // lower hex
+    //     };
+
+    //     assert_eq!(drv.outputs["out"].hash, Some(expected));
+    // }
+
+    // #[test]
+    // /// hash_algo set to empty string, SRI hash passed
+    // fn populate_output_config_flat_sri() {
+    //     let mut drv = Derivation::default();
+    //     drv.outputs.insert("out".to_string(), Default::default());
+
+    //     populate_output_configuration(
+    //         &mut drv,
+    //         Some("sha256-s6JN6XqP28g1uYMxaVAQMLiXcDG8tUs7OsE3QPhGqzA=".into()),
+    //         Some("".into()),
+    //         Some("flat".into()),
+    //     )
+    //     .expect("populate_output_configuration() should succeed");
+
+    //     let expected = Hash {
+    //         algo: "sha256".into(),
+    //         digest: "b3a24de97a8fdbc835b9833169501030b8977031bcb54b3b3ac13740f846ab30".into(), // lower hex
+    //     };
+
+    //     assert_eq!(drv.outputs["out"].hash, Some(expected));
+    // }
+
+    // #[test]
+    // fn handle_outputs_parameter() {
+    //     let mut vm = fake_vm();
+    //     let mut drv = Derivation::default();
+    //     drv.outputs.insert("out".to_string(), Default::default());
+
+    //     let outputs = Value::List(NixList::construct(
+    //         2,
+    //         vec![Value::String("foo".into()), Value::String("bar".into())],
+    //     ));
+    //     let outputs_str = outputs
+    //         .coerce_to_string(CoercionKind::Strong, &mut vm)
+    //         .unwrap();
+
+    //     handle_derivation_parameters(&mut drv, &mut vm, "outputs", &outputs, outputs_str.as_str())
+    //         .expect("handling 'outputs' parameter should succeed");
+
+    //     assert_eq!(drv.outputs.len(), 2);
+    //     assert!(drv.outputs.contains_key("bar"));
+    //     assert!(drv.outputs.contains_key("foo"));
+    // }
+
+    // #[test]
+    // fn handle_args_parameter() {
+    //     let mut vm = fake_vm();
+    //     let mut drv = Derivation::default();
+
+    //     let args = Value::List(NixList::construct(
+    //         3,
+    //         vec![
+    //             Value::String("--foo".into()),
+    //             Value::String("42".into()),
+    //             Value::String("--bar".into()),
+    //         ],
+    //     ));
+
+    //     let args_str = args
+    //         .coerce_to_string(CoercionKind::Strong, &mut vm)
+    //         .unwrap();
+
+    //     handle_derivation_parameters(&mut drv, &mut vm, "args", &args, args_str.as_str())
+    //         .expect("handling 'args' parameter should succeed");
+
+    //     assert_eq!(
+    //         drv.arguments,
+    //         vec!["--foo".to_string(), "42".to_string(), "--bar".to_string()]
+    //     );
+    // }
 
     #[test]
     fn builtins_placeholder_hashes() {
diff --git a/tvix/cli/src/main.rs b/tvix/cli/src/main.rs
index 447bb13c71..0942c128ec 100644
--- a/tvix/cli/src/main.rs
+++ b/tvix/cli/src/main.rs
@@ -12,7 +12,7 @@ use clap::Parser;
 use known_paths::KnownPaths;
 use rustyline::{error::ReadlineError, Editor};
 use tvix_eval::observer::{DisassemblingObserver, TracingObserver};
-use tvix_eval::{Builtin, BuiltinArgument, Value, VM};
+use tvix_eval::{Builtin, Value};
 
 #[derive(Parser)]
 struct Args {