From 77cc6a1f785caad0c5fb84457013177a99aaf158 Mon Sep 17 00:00:00 2001
From: Florian Klink <flokli@flokli.de>
Date: Wed, 4 Jan 2023 13:36:27 +0100
Subject: [PATCH] refactor(tvix/derivation): make output hashes an Option<Hash>

This conveys better if an output of a Derivation is fixed-output or not,
and provides a Hash struct that can be used to store the algo and
digest.

In case it's not, this can simply be None. The serde field attributes
have been updated to still accept the same JSON.

We currently still store the hash algo and digest as strings, mostly
because the only thing populating it so far is the example JSONs.
We might want to update this, once actual Nix code populates this.

While updating write.rs, I pushed some of the Vec<String> to [&str]
conversions inline, and made it a Vec<&str>, because it was annoying to
juggle with.

Change-Id: Ia9cd0568fe179ac22a4a636237f22ab4ad92b95b
Reviewed-on: https://cl.tvl.fyi/c/depot/+/7746
Tested-by: BuildkiteCI
Reviewed-by: jrhahn <mail.jhahn@gmail.com>
Reviewed-by: tazjin <tazjin@tvl.su>
---
 tvix/derivation/src/output.rs | 27 +++++++++++++++----------
 tvix/derivation/src/write.rs  | 37 +++++++++++++++++------------------
 2 files changed, 35 insertions(+), 29 deletions(-)

diff --git a/tvix/derivation/src/output.rs b/tvix/derivation/src/output.rs
index 0d764011f..69ae8167e 100644
--- a/tvix/derivation/src/output.rs
+++ b/tvix/derivation/src/output.rs
@@ -1,16 +1,23 @@
 use serde::{Deserialize, Serialize};
 
-// This function is required by serde to deserialize files
-// with missing keys.
-fn default_resource() -> String {
-    "".to_string()
-}
-
 #[derive(Serialize, Deserialize)]
 pub struct Output {
     pub path: String,
-    #[serde(default = "default_resource")]
-    pub hash_algorithm: String,
-    #[serde(default = "default_resource")]
-    pub hash: String,
+
+    #[serde(flatten)]
+    pub hash: Option<Hash>,
+}
+
+#[derive(Serialize, Deserialize)]
+pub struct Hash {
+    #[serde(rename = "hash")]
+    pub digest: String,
+    #[serde(rename = "hash_algorithm")]
+    pub algo: String,
+}
+
+impl Output {
+    pub fn is_fixed(&self) -> bool {
+        self.hash.is_some()
+    }
 }
diff --git a/tvix/derivation/src/write.rs b/tvix/derivation/src/write.rs
index 3fd8db792..0cbde3c0f 100644
--- a/tvix/derivation/src/write.rs
+++ b/tvix/derivation/src/write.rs
@@ -20,7 +20,7 @@ fn write_array_elements(
     quote: bool,
     open: &str,
     closing: &str,
-    elements: &[&str],
+    elements: Vec<&str>,
 ) -> Result<(), fmt::Error> {
     writer.write_str(open)?;
 
@@ -56,19 +56,25 @@ pub fn write_outputs(
         }
 
         // TODO(jrhahn) option to strip output
-        let elements: [&str; 4] = [
-            &output_name,
-            &output.path,
-            &output.hash_algorithm,
-            &output.hash,
-        ];
+        let mut elements: Vec<&str> = vec![output_name, &output.path];
+
+        match &output.hash {
+            Some(hash) => {
+                elements.push(&hash.algo);
+                elements.push(&hash.digest);
+            }
+            None => {
+                elements.push("");
+                elements.push("");
+            }
+        }
 
         write_array_elements(
             writer,
             true,
             &PAREN_OPEN.to_string(),
             &PAREN_CLOSE.to_string(),
-            &elements,
+            elements,
         )?
     }
     writer.write_char(BRACKET_CLOSE)?;
@@ -94,15 +100,12 @@ pub fn write_input_derivations(
         writer.write_char(QUOTE)?;
         writer.write_char(COMMA)?;
 
-        // convert Vec<String> to [&str]
-        let v: Vec<&str> = input_derivation.iter().map(|x| &**x).collect();
-
         write_array_elements(
             writer,
             true,
             &BRACKET_OPEN.to_string(),
             &BRACKET_CLOSE.to_string(),
-            &v,
+            input_derivation.iter().map(|s| &**s).collect(),
         )?;
 
         writer.write_char(PAREN_CLOSE)?;
@@ -119,14 +122,12 @@ pub fn write_input_sources(
 ) -> Result<(), fmt::Error> {
     writer.write_char(COMMA)?;
 
-    // convert Vec<String> to [&str]
-    let v: Vec<&str> = input_sources.iter().map(|x| &**x).collect();
     write_array_elements(
         writer,
         true,
         &BRACKET_OPEN.to_string(),
         &BRACKET_CLOSE.to_string(),
-        &v,
+        input_sources.iter().map(|s| &**s).collect(),
     )?;
 
     Ok(())
@@ -145,14 +146,12 @@ pub fn write_builder(writer: &mut impl Write, builder: &str) -> Result<(), fmt::
 }
 pub fn write_arguments(writer: &mut impl Write, arguments: &[String]) -> Result<(), fmt::Error> {
     writer.write_char(COMMA)?;
-    // convert Vec<String> to [&str]
-    let v: Vec<&str> = arguments.iter().map(|x| &**x).collect();
     write_array_elements(
         writer,
         true,
         &BRACKET_OPEN.to_string(),
         &BRACKET_CLOSE.to_string(),
-        &v,
+        arguments.iter().map(|s| &**s).collect(),
     )?;
 
     Ok(())
@@ -176,7 +175,7 @@ pub fn write_enviroment(
             false,
             &PAREN_OPEN.to_string(),
             &PAREN_CLOSE.to_string(),
-            &[&escape_string(key), &escape_string(environment)],
+            vec![&escape_string(key), &escape_string(environment)],
         )?;
     }