refactor(nix-compat): use StorePathRef for hash derivation modulo r/7389

Rather than passing strings around, use a StorePathRef.

This makes things a bit more typesafe, and more aligned with what we
want to do in b/264.

Change-Id: Ib7080addf27e7f1a9c8da1d8aaa66744468e3b5a
Reviewed-on: https://cl.tvl.fyi/c/depot/+/10633
Tested-by: BuildkiteCI
Reviewed-by: raitobezarius <tvl@lahfa.xyz>

4 files changed, 34 insertions, 30 deletions

diff --git a/tvix/glue/src/builtins/derivation.rs b/tvix/glue/src/builtins/derivation.rs
index 728882af39..c5ed8ec2ff 100644
--- a/tvix/glue/src/builtins/derivation.rs
+++ b/tvix/glue/src/builtins/derivation.rs
@@ -434,25 +434,24 @@ pub(crate) mod derivation_builtins {
 
         // Calculate the derivation_or_fod_hash for the current derivation.
         // This one is still intermediate (so not added to known_paths)
-        let derivation_or_fod_hash_tmp =
-            drv.derivation_or_fod_hash(|drv| known_paths.get_hash_derivation_modulo(drv));
+        let derivation_or_fod_hash_tmp = drv.derivation_or_fod_hash(|drv_path| {
+            known_paths.get_hash_derivation_modulo(&drv_path.to_owned())
+        });
 
         // Mutate the Derivation struct and set output paths
         drv.calculate_output_paths(&name, &derivation_or_fod_hash_tmp)
             .map_err(DerivationError::InvalidDerivation)?;
 
-        let derivation_path = drv
+        let drv_path = drv
             .calculate_derivation_path(&name)
             .map_err(DerivationError::InvalidDerivation)?;
 
         // recompute the hash derivation modulo and add to known_paths
-        let derivation_or_fod_hash_final =
-            drv.derivation_or_fod_hash(|drv| known_paths.get_hash_derivation_modulo(drv));
+        let derivation_or_fod_hash_final = drv.derivation_or_fod_hash(|drv_path| {
+            known_paths.get_hash_derivation_modulo(&drv_path.to_owned())
+        });
 
-        known_paths.add_hash_derivation_modulo(
-            derivation_path.to_absolute_path(),
-            &derivation_or_fod_hash_final,
-        );
+        known_paths.add_hash_derivation_modulo(drv_path.clone(), &derivation_or_fod_hash_final);
 
         let mut new_attrs: Vec<(String, NixString)> = drv
             .outputs
@@ -465,7 +464,7 @@ pub(crate) mod derivation_builtins {
                         Some(
                             NixContextElement::Single {
                                 name,
-                                derivation: derivation_path.to_absolute_path(),
+                                derivation: drv_path.to_absolute_path(),
                             }
                             .into(),
                         ),
@@ -478,8 +477,8 @@ pub(crate) mod derivation_builtins {
         new_attrs.push((
             "drvPath".to_string(),
             (
-                derivation_path.to_absolute_path(),
-                Some(NixContextElement::Derivation(derivation_path.to_absolute_path()).into()),
+                drv_path.to_absolute_path(),
+                Some(NixContextElement::Derivation(drv_path.to_absolute_path()).into()),
             )
                 .into(),
         ));
diff --git a/tvix/glue/src/known_paths.rs b/tvix/glue/src/known_paths.rs
index b0a4c0ac71..626b320ed5 100644
--- a/tvix/glue/src/known_paths.rs
+++ b/tvix/glue/src/known_paths.rs
@@ -8,7 +8,7 @@
 //! This data is required to find the derivation needed to actually trigger the
 //! build, if necessary.
 
-use nix_compat::nixhash::NixHash;
+use nix_compat::{nixhash::NixHash, store_path::StorePath};
 use std::collections::HashMap;
 
 #[derive(Debug, Default)]
@@ -16,26 +16,26 @@ pub struct KnownPaths {
     /// All known derivation or FOD hashes.
     ///
     /// Keys are derivation paths, values is the NixHash.
-    derivation_or_fod_hashes: HashMap<String, NixHash>,
+    derivation_or_fod_hashes: HashMap<StorePath, NixHash>,
 }
 
 impl KnownPaths {
     /// Fetch the opaque "hash derivation modulo" for a given derivation path.
-    pub fn get_hash_derivation_modulo(&self, drv_path: &str) -> NixHash {
+    pub fn get_hash_derivation_modulo(&self, drv_path: &StorePath) -> NixHash {
         // TODO: we rely on an invariant that things *should* have
         // been calculated if we get this far.
         self.derivation_or_fod_hashes[drv_path].clone()
     }
 
-    pub fn add_hash_derivation_modulo<D: ToString>(
+    pub fn add_hash_derivation_modulo(
         &mut self,
-        drv: D,
+        drv_path: StorePath,
         hash_derivation_modulo: &NixHash,
     ) {
         #[allow(unused_variables)] // assertions on this only compiled in debug builds
         let old = self
             .derivation_or_fod_hashes
-            .insert(drv.to_string(), hash_derivation_modulo.to_owned());
+            .insert(drv_path, hash_derivation_modulo.to_owned());
 
         #[cfg(debug_assertions)]
         {
diff --git a/tvix/nix-compat/src/derivation/mod.rs b/tvix/nix-compat/src/derivation/mod.rs
index b3f3adc55b..2f90c54d8f 100644
--- a/tvix/nix-compat/src/derivation/mod.rs
+++ b/tvix/nix-compat/src/derivation/mod.rs
@@ -1,4 +1,6 @@
-use crate::store_path::{self, build_ca_path, build_output_path, build_text_path, StorePath};
+use crate::store_path::{
+    self, build_ca_path, build_output_path, build_text_path, StorePath, StorePathRef,
+};
 use bstr::BString;
 use serde::{Deserialize, Serialize};
 use sha2::{Digest, Sha256};
@@ -162,7 +164,7 @@ impl Derivation {
     /// drv path).
     pub fn derivation_or_fod_hash<F>(&self, fn_get_derivation_or_fod_hash: F) -> NixHash
     where
-        F: Fn(&str) -> NixHash,
+        F: Fn(&StorePathRef) -> NixHash,
     {
         // Fixed-output derivations return a fixed hash.
         // Non-Fixed-output derivations return a hash of the ATerm notation, but with all
@@ -178,10 +180,13 @@ impl Derivation {
             // derivation_or_fod_hash, and replace the derivation path with it's HEXLOWER
             // digest.
             // This is not the [NixHash::to_nix_hash_string], but without the sha256: prefix).
-            for (drv_path, output_names) in &self.input_derivations {
+            for (drv_path_str, output_names) in &self.input_derivations {
+                // parse drv_path to StorePathRef
+                let drv_path = StorePathRef::from_absolute_path(drv_path_str.as_bytes())
+                    .expect("invalid input derivation path");
                 replaced_input_derivations.insert(
                     data_encoding::HEXLOWER
-                        .encode(fn_get_derivation_or_fod_hash(drv_path).digest_as_bytes()),
+                        .encode(fn_get_derivation_or_fod_hash(&drv_path).digest_as_bytes()),
                     output_names.clone(),
                 );
             }
diff --git a/tvix/nix-compat/src/derivation/tests/mod.rs b/tvix/nix-compat/src/derivation/tests/mod.rs
index d6770bdbac..168e11d46f 100644
--- a/tvix/nix-compat/src/derivation/tests/mod.rs
+++ b/tvix/nix-compat/src/derivation/tests/mod.rs
@@ -208,10 +208,10 @@ fn derivation_or_fod_hash(drv_path: &str, expected_nix_hash_string: &str) {
 #[test_case("unicode", "52a9id8hx688hvlnz4d1n25ml1jdykz0-unicode.drv"; "unicode")]
 #[test_case("cp1252", "m1vfixn8iprlf0v9abmlrz7mjw1xj8kp-cp1252.drv"; "cp1252")]
 #[test_case("latin1", "x6p0hg79i3wg0kkv7699935f7rrj9jf3-latin1.drv"; "latin1")]
-fn output_paths(name: &str, drv_path: &str) {
+fn output_paths(name: &str, drv_path_str: &str) {
     // read in the derivation
     let expected_derivation = Derivation::from_aterm_bytes(
-        read_file(&format!("{}/ok/{}", RESOURCES_PATHS, drv_path)).as_ref(),
+        read_file(&format!("{}/ok/{}", RESOURCES_PATHS, drv_path_str)).as_ref(),
     )
     .expect("must succeed");
 
@@ -225,10 +225,10 @@ fn output_paths(name: &str, drv_path: &str) {
         // 4wvvbi4jwn0prsdxb7vs673qa5h9gr7x-foo.drv may lookup /nix/store/0hm2f1psjpcwg8fijsmr4wwxrx59s092-bar.drv
         // ch49594n9avinrf8ip0aslidkc4lxkqv-foo.drv may lookup /nix/store/ss2p4wmxijn652haqyd7dckxwl4c7hxx-bar.drv
         if name == "foo"
-            && ((drv_path == "4wvvbi4jwn0prsdxb7vs673qa5h9gr7x-foo.drv"
-                && parent_drv_path == "/nix/store/0hm2f1psjpcwg8fijsmr4wwxrx59s092-bar.drv")
-                || (drv_path == "ch49594n9avinrf8ip0aslidkc4lxkqv-foo.drv"
-                    && parent_drv_path == "/nix/store/ss2p4wmxijn652haqyd7dckxwl4c7hxx-bar.drv"))
+            && ((drv_path_str == "4wvvbi4jwn0prsdxb7vs673qa5h9gr7x-foo.drv"
+                && parent_drv_path.to_string() == "0hm2f1psjpcwg8fijsmr4wwxrx59s092-bar.drv")
+                || (drv_path_str == "ch49594n9avinrf8ip0aslidkc4lxkqv-foo.drv"
+                    && parent_drv_path.to_string() == "ss2p4wmxijn652haqyd7dckxwl4c7hxx-bar.drv"))
         {
             // do the lookup, by reading in the fixture of the requested
             // drv_name, and calculating its drv replacement (on the non-stripped version)
@@ -237,7 +237,7 @@ fn output_paths(name: &str, drv_path: &str) {
             let json_bytes = read_file(&format!(
                 "{}/ok/{}.json",
                 RESOURCES_PATHS,
-                Path::new(parent_drv_path)
+                Path::new(&parent_drv_path.to_string())
                     .file_name()
                     .unwrap()
                     .to_string_lossy()
@@ -390,7 +390,7 @@ fn output_path_construction() {
     let foo_calc_result = foo_drv.calculate_output_paths(
         "foo",
         &foo_drv.derivation_or_fod_hash(|drv_path| {
-            if drv_path != "/nix/store/0hm2f1psjpcwg8fijsmr4wwxrx59s092-bar.drv" {
+            if drv_path.to_string() != "0hm2f1psjpcwg8fijsmr4wwxrx59s092-bar.drv" {
                 panic!("lookup called with unexpected drv_path: {}", drv_path);
             }
             bar_drv_derivation_or_fod_hash.clone()