From 9065089b0c4b16237cacf7c0878779cffb7f520f Mon Sep 17 00:00:00 2001 From: Florian Klink Date: Thu, 21 Dec 2023 16:29:38 +0200 Subject: [PATCH] refactor(tvix/nix-compat): have helpers interact with StorePathRef In most case, we don't actually need an owned `StorePath` struct, but a `StorePathRef<'_>` is sufficient. The lifetime is only due to it holding onto the name, but that one is mostly left untouched. `Derivation::calculate_derivation_path` still needs to return `StorePath`, as its name has a `.drv` appended. Change-Id: Ie0d52f369d785711bb0658ea2b0bd2617fd9f45e Reviewed-on: https://cl.tvl.fyi/c/depot/+/10389 Autosubmit: flokli Tested-by: BuildkiteCI Reviewed-by: edef --- tvix/nix-compat/src/derivation/mod.rs | 1 + tvix/nix-compat/src/store_path/utils.rs | 44 ++++++++++++------------- 2 files changed, 23 insertions(+), 22 deletions(-) diff --git a/tvix/nix-compat/src/derivation/mod.rs b/tvix/nix-compat/src/derivation/mod.rs index d62a339b7..b3f3adc55 100644 --- a/tvix/nix-compat/src/derivation/mod.rs +++ b/tvix/nix-compat/src/derivation/mod.rs @@ -115,6 +115,7 @@ impl Derivation { }; build_text_path(name, self.to_aterm_bytes(), references) + .map(|s| s.to_owned()) .map_err(|_e| DerivationError::InvalidOutputName(name.to_string())) } diff --git a/tvix/nix-compat/src/store_path/utils.rs b/tvix/nix-compat/src/store_path/utils.rs index dd3213513..50a1d9824 100644 --- a/tvix/nix-compat/src/store_path/utils.rs +++ b/tvix/nix-compat/src/store_path/utils.rs @@ -1,6 +1,6 @@ use crate::nixbase32; use crate::nixhash::{CAHash, NixHash}; -use crate::store_path::{Error, StorePath, STORE_DIR}; +use crate::store_path::{Error, StorePathRef, DIGEST_SIZE, STORE_DIR}; use sha2::{Digest, Sha256}; use std::io::Write; use thiserror; @@ -47,7 +47,7 @@ pub fn build_text_path, I: IntoIterator, C: AsRef<[u8]>> name: &str, content: C, references: I, -) -> Result { +) -> Result, BuildStorePathError> { // produce the sha256 digest of the contents let content_digest = Sha256::new_with_prefix(content).finalize().into(); @@ -55,12 +55,12 @@ pub fn build_text_path, I: IntoIterator, C: AsRef<[u8]>> } /// This builds a store path from a [CAHash] and a list of references. -pub fn build_ca_path, S: AsRef, I: IntoIterator>( - name: B, +pub fn build_ca_path<'a, S: AsRef, I: IntoIterator>( + name: &'a str, ca_hash: &CAHash, references: I, self_reference: bool, -) -> Result { +) -> Result, BuildStorePathError> { match &ca_hash { CAHash::Text(ref digest) => { if self_reference { @@ -71,14 +71,12 @@ pub fn build_ca_path, S: AsRef, I: IntoIterator>( &NixHash::Sha256(*digest), name, ) - .map_err(BuildStorePathError::InvalidStorePath) } CAHash::Nar(ref hash @ NixHash::Sha256(_)) => build_store_path_from_fingerprint_parts( &make_references_string("source", references, self_reference), hash, name, - ) - .map_err(BuildStorePathError::InvalidStorePath), + ), // for all other CAHash::Nar, another custom scheme is used. CAHash::Nar(ref hash) => { if references.into_iter().next().is_some() { @@ -101,7 +99,6 @@ pub fn build_ca_path, S: AsRef, I: IntoIterator>( }, name, ) - .map_err(BuildStorePathError::InvalidStorePath) } // CaHash::Flat is using something very similar, except the `r:` prefix. CAHash::Flat(ref hash) => { @@ -122,13 +119,17 @@ pub fn build_ca_path, S: AsRef, I: IntoIterator>( }, name, ) - .map_err(BuildStorePathError::InvalidStorePath) } } + .map_err(BuildStorePathError::InvalidStorePath) } -/// For given NAR sha256 digest and name, return the new [StorePath] this would have. -pub fn build_nar_based_store_path(nar_sha256_digest: &[u8; 32], name: &str) -> StorePath { +/// For given NAR sha256 digest and name, return the new [StorePathRef] this +/// would have. +pub fn build_nar_based_store_path<'a>( + nar_sha256_digest: &[u8; 32], + name: &'a str, +) -> StorePathRef<'a> { let nar_hash_with_mode = CAHash::Nar(NixHash::Sha256(nar_sha256_digest.to_owned())); build_ca_path(name, &nar_hash_with_mode, Vec::::new(), false).unwrap() @@ -138,11 +139,11 @@ pub fn build_nar_based_store_path(nar_sha256_digest: &[u8; 32], name: &str) -> S /// /// Input-addresed store paths are always derivation outputs, the "input" in question is the /// derivation and its closure. -pub fn build_output_path( +pub fn build_output_path<'a>( drv_hash: &NixHash, output_name: &str, - output_path_name: &str, -) -> Result { + output_path_name: &'a str, +) -> Result, Error> { build_store_path_from_fingerprint_parts( &(String::from("output:") + output_name), drv_hash, @@ -158,20 +159,19 @@ pub fn build_output_path( /// /// The fingerprint is hashed with sha256, its digest is compressed to 20 bytes, /// and nixbase32-encoded (32 characters). -fn build_store_path_from_fingerprint_parts>( +fn build_store_path_from_fingerprint_parts<'a>( ty: &str, hash: &NixHash, - name: B, -) -> Result { - let name = super::validate_name(&name)?.to_owned(); - - let digest = compress_hash(&{ + name: &'a str, +) -> Result, Error> { + let digest: [u8; DIGEST_SIZE] = compress_hash(&{ let mut h = Sha256::new(); write!(h, "{ty}:{}:{STORE_DIR}:{name}", hash.to_nix_hex_string()).unwrap(); h.finalize() }); - Ok(StorePath { digest, name }) + // name validation happens in here. + StorePathRef::from_name_and_digest(name, &digest) } /// This contains the Nix logic to create "text hash strings", which are used