From e2c79e39f00f1b882bde838ecc450d18a1807235 Mon Sep 17 00:00:00 2001 From: Florian Klink Date: Mon, 15 Jan 2024 20:32:55 +0200 Subject: refactor(nix-compat): use StorePathRef for hash derivation modulo 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 --- tvix/glue/src/builtins/derivation.rs | 23 +++++++++++------------ tvix/glue/src/known_paths.rs | 12 ++++++------ tvix/nix-compat/src/derivation/mod.rs | 13 +++++++++---- tvix/nix-compat/src/derivation/tests/mod.rs | 16 ++++++++-------- 4 files changed, 34 insertions(+), 30 deletions(-) diff --git a/tvix/glue/src/builtins/derivation.rs b/tvix/glue/src/builtins/derivation.rs index 728882af394d..c5ed8ec2fffe 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 b0a4c0ac7129..626b320ed515 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, + derivation_or_fod_hashes: HashMap, } 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( + 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 b3f3adc55ba2..2f90c54d8f76 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(&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 d6770bdbaca8..168e11d46f1e 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() -- cgit 1.4.1