refactor(nix-compat/store_path): take [u8;32] for outer fingerprint r/7691

The outer fingerprint used for store path calculation is always a sha256
digest. This includes both input and output-addressed store paths.

We used a NixHash here, which can also represent other hash types, and
that had a bunch of annoyances:

 - Whenever we had the bytes, we had to wrap them in a NixHash::Sha256().
 - Things like AtermWriteable had to be implemented on NixHash,
   even though we then had an assertion it was only called in the
   NixHash::Sha256 case.

Change-Id: Ic895503d9b071800d2e52ae057666f44bd0ab9d6
Reviewed-on: https://cl.tvl.fyi/c/depot/+/11142
Tested-by: BuildkiteCI
Autosubmit: flokli <flokli@flokli.de>
Reviewed-by: John Ericson <git@johnericson.me>
Reviewed-by: picnoir picnoir <picnoir@alternativebit.fr>

3 files changed, 18 insertions, 22 deletions

diff --git a/tvix/nix-compat/src/derivation/mod.rs b/tvix/nix-compat/src/derivation/mod.rs
index 848f1a31ff36..9a7dd04c11c4 100644
--- a/tvix/nix-compat/src/derivation/mod.rs
+++ b/tvix/nix-compat/src/derivation/mod.rs
@@ -178,8 +178,8 @@ impl Derivation {
     ///
     /// This is called `hashDerivationModulo` in nixcpp.
     ///
-    /// It returns a [NixHash], created by calculating the sha256 digest of
-    /// the derivation ATerm representation, except that:
+    /// It returns the sha256 digest of the derivation ATerm representation,
+    /// except that:
     ///  -  any input derivation paths have beed replaced "by the result of a
     ///     recursive call to this function" and that
     ///  - for fixed-output derivations the special
@@ -190,16 +190,16 @@ impl Derivation {
     /// this function to provide a lookup function to lookup these calculation
     /// results of parent derivations at `fn_get_derivation_or_fod_hash` (by
     /// drv path).
-    pub fn derivation_or_fod_hash<F>(&self, fn_get_derivation_or_fod_hash: F) -> NixHash
+    pub fn derivation_or_fod_hash<F>(&self, fn_get_derivation_or_fod_hash: F) -> [u8; 32]
     where
-        F: Fn(&StorePathRef) -> NixHash,
+        F: Fn(&StorePathRef) -> [u8; 32],
     {
         // Fixed-output derivations return a fixed hash.
         // Non-Fixed-output derivations return the sha256 digest of the ATerm
         // notation, but with all input_derivation paths replaced by a recursive
         // call to this function.
         // We use fn_get_derivation_or_fod_hash here, so callers can precompute this.
-        NixHash::Sha256(self.fod_digest().unwrap_or({
+        self.fod_digest().unwrap_or({
             // For each input_derivation, look up the
             // derivation_or_fod_hash, and replace the derivation path with
             // it's HEXLOWER digest.
@@ -216,7 +216,7 @@ impl Derivation {
             hasher.update(self.to_aterm_bytes_with_replacements(&input_derivations));
 
             hasher.finalize().into()
-        }))
+        })
     }
 
     /// This calculates all output paths of a Derivation and updates the struct.
@@ -238,7 +238,7 @@ impl Derivation {
     pub fn calculate_output_paths(
         &mut self,
         name: &str,
-        derivation_or_fod_hash: &NixHash,
+        derivation_or_fod_hash: &[u8; 32],
     ) -> Result<(), DerivationError> {
         // The fingerprint and hash differs per output
         for (output_name, output) in self.outputs.iter_mut() {
diff --git a/tvix/nix-compat/src/derivation/tests/mod.rs b/tvix/nix-compat/src/derivation/tests/mod.rs
index 2bf09265bcda..56bad7869dbb 100644
--- a/tvix/nix-compat/src/derivation/tests/mod.rs
+++ b/tvix/nix-compat/src/derivation/tests/mod.rs
@@ -5,6 +5,7 @@ use crate::derivation::parser::Error;
 use crate::derivation::Derivation;
 use crate::store_path::StorePath;
 use bstr::{BStr, BString};
+use hex_literal::hex;
 use std::collections::BTreeSet;
 use std::fs::File;
 use std::io::Read;
@@ -184,16 +185,15 @@ fn derivation_with_trimmed_output_paths(derivation: &Derivation) -> Derivation {
     }
 }
 
-#[test_case("0hm2f1psjpcwg8fijsmr4wwxrx59s092-bar.drv", "sha256:724f3e3634fce4cbbbd3483287b8798588e80280660b9a63fd13a1bc90485b33"; "fixed_sha256")]
-#[test_case("ss2p4wmxijn652haqyd7dckxwl4c7hxx-bar.drv", "sha256:c79aebd0ce3269393d4a1fde2cbd1d975d879b40f0bf40a48f550edc107fd5df";"fixed-sha1")]
-fn derivation_or_fod_hash(drv_path: &str, expected_nix_hash_string: &str) {
+#[test_case("0hm2f1psjpcwg8fijsmr4wwxrx59s092-bar.drv", hex!("724f3e3634fce4cbbbd3483287b8798588e80280660b9a63fd13a1bc90485b33"); "fixed_sha256")]
+#[test_case("ss2p4wmxijn652haqyd7dckxwl4c7hxx-bar.drv", hex!("c79aebd0ce3269393d4a1fde2cbd1d975d879b40f0bf40a48f550edc107fd5df");"fixed-sha1")]
+fn derivation_or_fod_hash(drv_path: &str, expected_digest: [u8; 32]) {
     // read in the fixture
     let json_bytes = read_file(&format!("{}/ok/{}.json", RESOURCES_PATHS, drv_path));
     let drv: Derivation = serde_json::from_slice(&json_bytes).expect("must deserialize");
 
     let actual = drv.derivation_or_fod_hash(|_| panic!("must not be called"));
-
-    assert_eq!(expected_nix_hash_string, actual.to_nix_hex_string());
+    assert_eq!(expected_digest, actual);
 }
 
 /// This reads a Derivation (in A-Term), trims out all fields containing
@@ -401,7 +401,7 @@ fn output_path_construction() {
             if drv_path.to_string() != "0hm2f1psjpcwg8fijsmr4wwxrx59s092-bar.drv" {
                 panic!("lookup called with unexpected drv_path: {}", drv_path);
             }
-            bar_drv_derivation_or_fod_hash.clone()
+            bar_drv_derivation_or_fod_hash
         }),
     );
     assert!(foo_calc_result.is_ok());
diff --git a/tvix/nix-compat/src/derivation/write.rs b/tvix/nix-compat/src/derivation/write.rs
index f20bf4e121d2..735b781574e1 100644
--- a/tvix/nix-compat/src/derivation/write.rs
+++ b/tvix/nix-compat/src/derivation/write.rs
@@ -8,7 +8,8 @@ use crate::derivation::{ca_kind_prefix, output::Output};
 use crate::nixbase32;
 use crate::store_path::{StorePath, StorePathRef, STORE_DIR_WITH_SLASH};
 use bstr::BString;
-use std::fmt::Display;
+use data_encoding::HEXLOWER;
+
 use std::{
     collections::{BTreeMap, BTreeSet},
     io,
@@ -16,8 +17,6 @@ use std::{
     io::Write,
 };
 
-use super::NixHash;
-
 pub const DERIVATION_PREFIX: &str = "Derive";
 pub const PAREN_OPEN: char = '(';
 pub const PAREN_CLOSE: char = ')';
@@ -31,7 +30,7 @@ pub const QUOTE: char = '"';
 /// Note that we mostly use explicit `write_*` calls
 /// instead since the serialization of the items depends on
 /// the context a lot.
-pub(crate) trait AtermWriteable: Display {
+pub(crate) trait AtermWriteable {
     fn aterm_write(&self, writer: &mut impl Write) -> std::io::Result<()>;
 
     fn aterm_bytes(&self) -> Vec<u8> {
@@ -67,12 +66,9 @@ impl AtermWriteable for String {
     }
 }
 
-impl AtermWriteable for NixHash {
+impl AtermWriteable for [u8; 32] {
     fn aterm_write(&self, writer: &mut impl Write) -> std::io::Result<()> {
-        // When we serialize the placeholder hashes,
-        // they need to be SHA256.
-        debug_assert!(matches!(self, NixHash::Sha256(_)));
-        write_field(writer, self.to_plain_hex_string(), false)
+        write_field(writer, HEXLOWER.encode(self), false)
     }
 }