From e7af8e0d62603f5fdf85307dc57b2c9599aa1936 Mon Sep 17 00:00:00 2001 From: Ryan Lahfa Date: Mon, 25 Mar 2024 01:25:19 +0100 Subject: [PATCH] feat(tvix/eval): implement `appendContext` `appendContext s ctx` will just append a user-crafted context attrs to `s`. The most important part of this builtin is to perform all the relevant invariant validations to avoid letting the user craft invalid contexts which can never be built, e.g. invalid store paths, inexistent derivations, etc. This version is incomplete and full of TODOs, but passes all the Nix's context strings tests, so we turn them on. Change-Id: I625dc5e7c4f5b784f078b390f04b0ee5a8d65a7c Signed-off-by: Ryan Lahfa Reviewed-on: https://cl.tvl.fyi/c/depot/+/11263 Reviewed-by: flokli Tested-by: BuildkiteCI --- tvix/eval/docs/builtins.md | 2 +- tvix/eval/src/builtins/mod.rs | 80 +++++++++++++++++++ tvix/glue/src/tests/mod.rs | 16 ++-- .../eval-okay-context-introspection.exp | 0 .../eval-okay-context-introspection.nix | 0 5 files changed, 91 insertions(+), 7 deletions(-) rename tvix/glue/src/tests/nix_tests/{notyetpassing => }/eval-okay-context-introspection.exp (100%) rename tvix/glue/src/tests/nix_tests/{notyetpassing => }/eval-okay-context-introspection.nix (100%) diff --git a/tvix/eval/docs/builtins.md b/tvix/eval/docs/builtins.md index 4e09e0f9e..dba4c48c6 100644 --- a/tvix/eval/docs/builtins.md +++ b/tvix/eval/docs/builtins.md @@ -24,7 +24,7 @@ The `impl` column indicates implementation status in tvix: | addErrorContext | false | ? | | context | | all | false | 2 | true | | | any | false | 2 | true | | -| appendContext | false | ? | | context | +| appendContext | false | ? | | | | attrNames | false | 1 | true | | | attrValues | false | | true | | | baseNameOf | true | | | | diff --git a/tvix/eval/src/builtins/mod.rs b/tvix/eval/src/builtins/mod.rs index 048c8867a..8973a2592 100644 --- a/tvix/eval/src/builtins/mod.rs +++ b/tvix/eval/src/builtins/mod.rs @@ -692,6 +692,86 @@ mod pure_builtins { Ok(Value::attrs(NixAttrs::from_iter(elements))) } + #[builtin("appendContext")] + #[allow(non_snake_case)] + async fn builtin_appendContext( + co: GenCo, + origin: Value, + added_context: Value, + ) -> Result { + // `appendContext` is a "grow" context function. + // It cannot remove a context element, neither replace a piece of its contents. + // + // Growing context is always a safe operation, there's no loss of dependency tracking + // information. + // + // This is why this operation is not prefixed by `unsafe` and is deemed *safe*. + // Nonetheless, it is possible to craft nonsensical context elements referring + // to inexistent derivations, output paths or output names. + // + // In Nix, those nonsensical context elements are partially mitigated by checking + // that various parameters are indeed syntatically valid store paths in the context, i.e. + // starting with the same prefix as `builtins.storeDir`, or ending with `.drv`. + // In addition, if writing to the store is possible (evaluator not in read-only mode), Nix + // will realize some paths and ensures they are present in the store. + // + // In this implementation, we do none of that, no syntax checks, no realization. + // The next `TODO` are the checks that Nix implements. + let mut ctx_elements: HashSet = HashSet::new(); + let span = generators::request_span(&co).await; + let origin = origin + .coerce_to_string( + co, + CoercionKind { + strong: true, + import_paths: true, + }, + span, + ) + .await?; + let mut origin = origin.to_contextful_str()?; + + let added_context = added_context.to_attrs()?; + for (context_key, context_element) in added_context.into_iter() { + // Invariant checks: + // - TODO: context_key must be a syntactically valid store path. + // - Perform a deep force `context_element`. + let context_element = context_element.to_attrs()?; + if let Some(path) = context_element.select("path") { + if path.as_bool()? { + ctx_elements.insert(NixContextElement::Plain(context_key.to_string())); + } + } + if let Some(all_outputs) = context_element.select("allOutputs") { + if all_outputs.as_bool()? { + // TODO: check if `context_key` is a derivation path. + // This may require realization. + ctx_elements.insert(NixContextElement::Derivation(context_key.to_string())); + } + } + if let Some(some_outputs) = context_element.select("outputs") { + let some_outputs = some_outputs.to_list()?; + // TODO: check if `context_key` is a derivation path. + // This may require realization. + for output in some_outputs.into_iter() { + let output = output.to_str()?; + ctx_elements.insert(NixContextElement::Single { + derivation: context_key.to_string(), + name: output.to_string(), + }); + } + } + } + + if let Some(origin_ctx) = origin.context_mut() { + // FUTUREWORK(performance): avoid this clone + // and extend in-place. + *origin_ctx = origin_ctx.clone().join(&mut ctx_elements.into()); + } + + Ok(origin.into()) + } + #[builtin("hashString")] #[allow(non_snake_case)] async fn builtin_hashString(co: GenCo, algo: Value, s: Value) -> Result { diff --git a/tvix/glue/src/tests/mod.rs b/tvix/glue/src/tests/mod.rs index 469ed17a5..e66f484e3 100644 --- a/tvix/glue/src/tests/mod.rs +++ b/tvix/glue/src/tests/mod.rs @@ -134,9 +134,13 @@ fn nix_eval_okay(#[files("src/tests/nix_tests/eval-okay-*.nix")] code_path: Path // notyetpassing; this makes the test suite much more useful for // regression testing, since there should always be zero non-ignored // failing tests. -#[rstest] -fn nix_eval_okay_currently_failing( - #[files("src/tests/nix_tests/notyetpassing/eval-okay-*.nix")] code_path: PathBuf, -) { - eval_test(code_path, false) -} +// +// NOTE: There's no such test anymore. `rstest` does not handle empty directories, so, we +// just comment it for now. +// +// #[rstest] +// fn nix_eval_okay_currently_failing( +// #[files("src/tests/nix_tests/notyetpassing/eval-okay-*.nix")] code_path: PathBuf, +// ) { +// eval_test(code_path, false) +// } diff --git a/tvix/glue/src/tests/nix_tests/notyetpassing/eval-okay-context-introspection.exp b/tvix/glue/src/tests/nix_tests/eval-okay-context-introspection.exp similarity index 100% rename from tvix/glue/src/tests/nix_tests/notyetpassing/eval-okay-context-introspection.exp rename to tvix/glue/src/tests/nix_tests/eval-okay-context-introspection.exp diff --git a/tvix/glue/src/tests/nix_tests/notyetpassing/eval-okay-context-introspection.nix b/tvix/glue/src/tests/nix_tests/eval-okay-context-introspection.nix similarity index 100% rename from tvix/glue/src/tests/nix_tests/notyetpassing/eval-okay-context-introspection.nix rename to tvix/glue/src/tests/nix_tests/eval-okay-context-introspection.nix