From 7165ebc43bfb1b53929fb31673c512bbbdbe4096 Mon Sep 17 00:00:00 2001
From: sterni <sternenseemann@systemli.org>
Date: Wed, 13 Dec 2023 14:52:07 +0100
Subject: fix(tvix/eval): remove incorrect imports when coercing

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>
---
 tvix/glue/src/builtins/derivation.rs | 19 ++++++++++++++-----
 1 file changed, 14 insertions(+), 5 deletions(-)

(limited to 'tvix/glue/src')

diff --git a/tvix/glue/src/builtins/derivation.rs b/tvix/glue/src/builtins/derivation.rs
index 6eedbc13e7e3..993e7612aed4 100644
--- a/tvix/glue/src/builtins/derivation.rs
+++ b/tvix/glue/src/builtins/derivation.rs
@@ -161,7 +161,7 @@ async fn handle_derivation_parameters(
         "args" => {
             let args = value.to_list()?;
             for arg in args {
-                match strong_coerce_to_string(co, arg).await? {
+                match strong_importing_coerce_to_string(co, arg).await? {
                     Err(cek) => return Ok(Err(cek)),
                     Ok(s) => drv.arguments.push(s),
                 }
@@ -194,12 +194,21 @@ async fn handle_derivation_parameters(
     Ok(Ok(true))
 }
 
-async fn strong_coerce_to_string(
+async fn strong_importing_coerce_to_string(
     co: &GenCo,
     val: Value,
 ) -> Result<Result<String, CatchableErrorKind>, ErrorKind> {
     let val = generators::request_force(co, val).await;
-    match generators::request_string_coerce(co, val, CoercionKind::Strong).await {
+    match generators::request_string_coerce(
+        co,
+        val,
+        CoercionKind {
+            strong: true,
+            import_paths: true,
+        },
+    )
+    .await
+    {
         Err(cek) => Ok(Err(cek)),
         Ok(val_str) => Ok(Ok(val_str.as_str().to_string())),
     }
@@ -268,7 +277,7 @@ pub(crate) mod derivation_builtins {
             key: &str,
         ) -> Result<Result<Option<String>, CatchableErrorKind>, ErrorKind> {
             if let Some(attr) = attrs.select(key) {
-                match strong_coerce_to_string(co, attr.clone()).await? {
+                match strong_importing_coerce_to_string(co, attr.clone()).await? {
                     Err(cek) => return Ok(Err(cek)),
                     Ok(str) => return Ok(Ok(Some(str))),
                 }
@@ -283,7 +292,7 @@ pub(crate) mod derivation_builtins {
                 continue;
             }
 
-            match strong_coerce_to_string(&co, value.clone()).await? {
+            match strong_importing_coerce_to_string(&co, value.clone()).await? {
                 Err(cek) => return Ok(Value::Catchable(cek)),
                 Ok(val_str) => {
                     // handle_derivation_parameters tells us whether the
-- 
cgit 1.4.1