From 34fc4637ebbb906d38647ca8a12fdb80cd2baf18 Mon Sep 17 00:00:00 2001 From: Florian Klink Date: Wed, 18 Oct 2023 11:39:36 +0100 Subject: refactor(tvix/nix-compat): rename NixHashWithMode -> CAHash This specific struct is only used to represent content-addressed paths (in case a Derivation has a fixed-output hash, for example). Rename `Output`'s `hash_with_mode` to `ca_hash`. We now also include `CAHash::Text`, and update the `validate` function of the `Output` struct to reject text hashes there. This allows cleaning up the various output path calculation functions inside nix-compat/src/store_path/utils.rs, as they can now match on the type. `make_type` is renamed to `make_references_string`, `build_regular_ca_path` is renamed to `build_ca_path`, and `build_text_path` has a disclaimer added, because you might not actually want to use it. Change-Id: I674d065f2ed5c804012ddfed56e161ac49d23931 Reviewed-on: https://cl.tvl.fyi/c/depot/+/9814 Tested-by: BuildkiteCI Reviewed-by: raitobezarius --- tvix/nix-compat/src/derivation/errors.rs | 12 ++--- tvix/nix-compat/src/derivation/mod.rs | 36 ++++++++++---- tvix/nix-compat/src/derivation/output.rs | 20 ++++---- tvix/nix-compat/src/derivation/parser.rs | 74 +++++++++++++++++++++-------- tvix/nix-compat/src/derivation/tests/mod.rs | 4 +- tvix/nix-compat/src/derivation/write.rs | 14 ++---- 6 files changed, 103 insertions(+), 57 deletions(-) (limited to 'tvix/nix-compat/src/derivation') diff --git a/tvix/nix-compat/src/derivation/errors.rs b/tvix/nix-compat/src/derivation/errors.rs index 8e9e6a121096..305957b10f6d 100644 --- a/tvix/nix-compat/src/derivation/errors.rs +++ b/tvix/nix-compat/src/derivation/errors.rs @@ -1,6 +1,8 @@ -use crate::{nixbase32::Nixbase32DecodeError, store_path}; +use crate::store_path; use thiserror::Error; +use super::CAHash; + /// Errors that can occur during the validation of Derivation structs. #[derive(Debug, Error, PartialEq)] pub enum DerivationError { @@ -50,10 +52,6 @@ pub enum DerivationError { pub enum OutputError { #[error("Invalid output path {0}: {1}")] InvalidOutputPath(String, store_path::Error), - #[error("Invalid hash encoding: {0}")] - InvalidHashEncoding(String, Nixbase32DecodeError), - #[error("Invalid hash algo: {0}")] - InvalidHashAlgo(String), - #[error("Invalid Digest size {0} for algo {1}")] - InvalidDigestSizeForAlgo(usize, String), + #[error("Invalid CAHash: {:?}", .0)] + InvalidCAHash(CAHash), } diff --git a/tvix/nix-compat/src/derivation/mod.rs b/tvix/nix-compat/src/derivation/mod.rs index d7f894c89a8f..a765e343096e 100644 --- a/tvix/nix-compat/src/derivation/mod.rs +++ b/tvix/nix-compat/src/derivation/mod.rs @@ -1,6 +1,4 @@ -use crate::store_path::{ - self, build_output_path, build_regular_ca_path, build_text_path, StorePath, -}; +use crate::store_path::{self, build_ca_path, build_output_path, build_text_path, StorePath}; use bstr::BString; use serde::{Deserialize, Serialize}; use sha2::{Digest, Sha256}; @@ -18,7 +16,7 @@ mod write; mod tests; // Public API of the crate. -pub use crate::nixhash::{NixHash, NixHashWithMode}; +pub use crate::nixhash::{CAHash, NixHash}; pub use errors::{DerivationError, OutputError}; pub use output::Output; @@ -122,16 +120,22 @@ impl Derivation { /// Returns the FOD digest, if the derivation is fixed-output, or None if /// it's not. + /// TODO: this is kinda the string from [build_ca_path] with a + /// [CAHash::Flat], what's fed to `build_store_path_from_fingerprint_parts` + /// (except the out_output.path being an empty string) fn fod_digest(&self) -> Option<[u8; 32]> { if self.outputs.len() != 1 { return None; } let out_output = self.outputs.get("out")?; + let ca_hash = &out_output.ca_hash.as_ref()?; + Some( Sha256::new_with_prefix(format!( - "fixed:out:{}:{}", - out_output.hash_with_mode.clone()?.to_nix_hash_string(), + "fixed:out:{}{}:{}", + ca_kind_prefix(ca_hash), + ca_hash.digest().to_nix_hash_string(), out_output.path )) .finalize() @@ -229,10 +233,10 @@ impl Derivation { // For fixed output derivation we use the per-output info, otherwise we use the // derivation hash. - let abs_store_path = if let Some(ref hwm) = output.hash_with_mode { - build_regular_ca_path(&path_name, hwm, Vec::::new(), false).map_err( - |e| DerivationError::InvalidOutputDerivationPath(output_name.to_string(), e), - )? + let abs_store_path = if let Some(ref hwm) = output.ca_hash { + build_ca_path(&path_name, hwm, Vec::::new(), false).map_err(|e| { + DerivationError::InvalidOutputDerivationPath(output_name.to_string(), e) + })? } else { build_output_path(derivation_or_fod_hash, output_name, &path_name).map_err(|e| { DerivationError::InvalidOutputDerivationPath( @@ -265,3 +269,15 @@ fn output_path_name(derivation_name: &str, output_name: &str) -> String { } output_path_name } + +/// For a [CAHash], return the "prefix" used for NAR purposes. +/// For [CAHash::Flat], this is an empty string, for [CAHash::Nar], it's "r:". +/// Panics for other [CAHash] kinds, as they're not valid in a derivation +/// context. +fn ca_kind_prefix(ca_hash: &CAHash) -> &'static str { + match ca_hash { + CAHash::Flat(_) => "", + CAHash::Nar(_) => "r:", + _ => panic!("invalid ca hash in derivation context: {:?}", ca_hash), + } +} diff --git a/tvix/nix-compat/src/derivation/output.rs b/tvix/nix-compat/src/derivation/output.rs index 78a83b03be45..c13f94859dc1 100644 --- a/tvix/nix-compat/src/derivation/output.rs +++ b/tvix/nix-compat/src/derivation/output.rs @@ -1,5 +1,5 @@ use crate::derivation::OutputError; -use crate::nixhash::{HashAlgo, NixHashWithMode}; +use crate::nixhash::CAHash; use crate::store_path::StorePath; use serde::{Deserialize, Serialize}; use serde_json::Map; @@ -9,7 +9,7 @@ pub struct Output { pub path: String, #[serde(flatten)] - pub hash_with_mode: Option, + pub ca_hash: Option, // we can only represent a subset here. } impl<'de> Deserialize<'de> for Output { @@ -30,26 +30,26 @@ impl<'de> Deserialize<'de> for Output { &"a string", ))? .to_owned(), - hash_with_mode: NixHashWithMode::from_map::(&fields)?, + ca_hash: CAHash::from_map::(&fields)?, }) } } impl Output { pub fn is_fixed(&self) -> bool { - self.hash_with_mode.is_some() + self.ca_hash.is_some() } pub fn validate(&self, validate_output_paths: bool) -> Result<(), OutputError> { - if let Some(hash) = &self.hash_with_mode { - match hash { - NixHashWithMode::Flat(h) | NixHashWithMode::Recursive(h) => { - if h.algo() != HashAlgo::Sha1 || h.algo() != HashAlgo::Sha256 { - return Err(OutputError::InvalidHashAlgo(h.algo().to_string())); - } + if let Some(fixed_output_hash) = &self.ca_hash { + match fixed_output_hash { + CAHash::Flat(_) | CAHash::Nar(_) => { + // all hashes allowed for Flat, and Nar. } + _ => return Err(OutputError::InvalidCAHash(fixed_output_hash.clone())), } } + if validate_output_paths { if let Err(e) = StorePath::from_absolute_path(self.path.as_bytes()) { return Err(OutputError::InvalidOutputPath(self.path.to_string(), e)); diff --git a/tvix/nix-compat/src/derivation/parser.rs b/tvix/nix-compat/src/derivation/parser.rs index 48f2b92d9168..b04187c433dc 100644 --- a/tvix/nix-compat/src/derivation/parser.rs +++ b/tvix/nix-compat/src/derivation/parser.rs @@ -12,8 +12,8 @@ use nom::sequence::{delimited, preceded, separated_pair, terminated, tuple}; use std::collections::{BTreeMap, BTreeSet}; use thiserror; -use super::parse_error::{into_nomerror, ErrorKind, NomError, NomResult}; -use super::{write, Derivation, NixHashWithMode, Output}; +use crate::derivation::parse_error::{into_nomerror, ErrorKind, NomError, NomResult}; +use crate::derivation::{write, CAHash, Derivation, Output}; use crate::{aterm, nixhash}; #[derive(Debug, thiserror::Error)] @@ -42,6 +42,24 @@ pub(crate) fn parse(i: &[u8]) -> Result> { } } +/// Consume a string containing the algo, and optionally a `r:` +/// prefix, and a digest (bytes), return a [CAHash::Nar] or [CAHash::Flat]. +fn from_algo_and_mode_and_digest>( + algo_and_mode: &str, + digest: B, +) -> crate::nixhash::Result { + Ok(match algo_and_mode.strip_prefix("r:") { + Some(algo) => nixhash::CAHash::Nar(nixhash::from_algo_and_digest( + algo.try_into()?, + digest.as_ref(), + )?), + None => nixhash::CAHash::Flat(nixhash::from_algo_and_digest( + algo_and_mode.try_into()?, + digest.as_ref(), + )?), + }) +} + /// Parse one output in ATerm. This is 4 string fields inside parans: /// output name, output path, algo (and mode), digest. /// Returns the output name and [Output] struct. @@ -60,27 +78,26 @@ fn parse_output(i: &[u8]) -> NomResult<&[u8], (String, Output)> { }, |(output_name, output_path, algo_and_mode, encoded_digest)| { // convert these 4 fields into an [Output]. - let hash_with_mode_res = { + let ca_hash_res = { if algo_and_mode.is_empty() && encoded_digest.is_empty() { None } else { match data_encoding::HEXLOWER.decode(&encoded_digest) { - Ok(digest) => Some(NixHashWithMode::from_algo_mode_hash( - &algo_and_mode, - &digest, - )), + Ok(digest) => { + Some(from_algo_and_mode_and_digest(&algo_and_mode, digest)) + } Err(e) => Some(Err(nixhash::Error::InvalidBase64Encoding(e))), } } } .transpose(); - match hash_with_mode_res { + match ca_hash_res { Ok(hash_with_mode) => Ok(( output_name, Output { path: output_path, - hash_with_mode, + ca_hash: hash_with_mode, }, )), Err(e) => Err(nom::Err::Failure(NomError { @@ -279,12 +296,20 @@ where mod tests { use std::collections::{BTreeMap, BTreeSet}; - use crate::derivation::{parse_error::ErrorKind, Output}; + use crate::derivation::{ + parse_error::ErrorKind, parser::from_algo_and_mode_and_digest, CAHash, NixHash, Output, + }; use bstr::{BString, ByteSlice}; use lazy_static::lazy_static; use test_case::test_case; + const DIGEST_SHA256: [u8; 32] = [ + 0xa5, 0xce, 0x9c, 0x15, 0x5e, 0xd0, 0x93, 0x97, 0x61, 0x46, 0x46, 0xc9, 0x71, 0x7f, 0xc7, + 0xcd, 0x94, 0xb1, 0x02, 0x3d, 0x7b, 0x76, 0xb6, 0x18, 0xd4, 0x09, 0xe4, 0xfe, 0xfd, 0x6e, + 0x9d, 0x39, + ]; lazy_static! { + pub static ref NIXHASH_SHA256: NixHash = NixHash::Sha256(DIGEST_SHA256); static ref EXP_MULTI_OUTPUTS: BTreeMap = { let mut b = BTreeMap::new(); b.insert( @@ -292,14 +317,14 @@ mod tests { Output { path: "/nix/store/2vixb94v0hy2xc6p7mbnxxcyc095yyia-has-multi-out-lib" .to_string(), - hash_with_mode: None, + ca_hash: None, }, ); b.insert( "out".to_string(), Output { path: "/nix/store/55lwldka5nyxa08wnvlizyqw02ihy8ic-has-multi-out".to_string(), - hash_with_mode: None, + ca_hash: None, }, ); b @@ -441,19 +466,15 @@ mod tests { br#"("out","/nix/store/5vyvcwah9l9kf07d52rcgdk70g2f4y13-foo","","")"#, ("out".to_string(), Output { path: "/nix/store/5vyvcwah9l9kf07d52rcgdk70g2f4y13-foo".to_string(), - hash_with_mode: None + ca_hash: None }); "simple" )] #[test_case( br#"("out","/nix/store/4q0pg5zpfmznxscq3avycvf9xdvx50n3-bar","r:sha256","08813cbee9903c62be4c5027726a418a300da4500b2d369d3af9286f4815ceba")"#, ("out".to_string(), Output { path: "/nix/store/4q0pg5zpfmznxscq3avycvf9xdvx50n3-bar".to_string(), - hash_with_mode: Some(crate::derivation::NixHashWithMode::Recursive( - crate::nixhash::from_algo_and_digest ( - crate::nixhash::HashAlgo::Sha256, - &data_encoding::HEXLOWER.decode(b"08813cbee9903c62be4c5027726a418a300da4500b2d369d3af9286f4815ceba").unwrap() - ).unwrap() - )), + ca_hash: Some(from_algo_and_mode_and_digest("r:sha256", + &data_encoding::HEXLOWER.decode(b"08813cbee9903c62be4c5027726a418a300da4500b2d369d3af9286f4815ceba").unwrap() ).unwrap()), }); "fod" )] fn parse_output(input: &[u8], expected: (String, Output)) { @@ -472,4 +493,19 @@ mod tests { assert!(rest.is_empty()); assert_eq!(*expected, parsed); } + + #[test_case("sha256", &DIGEST_SHA256, CAHash::Flat(NIXHASH_SHA256.clone()); "sha256 flat")] + #[test_case("r:sha256", &DIGEST_SHA256, CAHash::Nar(NIXHASH_SHA256.clone()); "sha256 recursive")] + fn test_from_algo_and_mode_and_digest(algo_and_mode: &str, digest: &[u8], expected: CAHash) { + assert_eq!( + expected, + from_algo_and_mode_and_digest(algo_and_mode, digest).unwrap() + ); + } + + #[test] + fn from_algo_and_mode_and_digest_failure() { + assert!(from_algo_and_mode_and_digest("r:sha256", &[]).is_err()); + assert!(from_algo_and_mode_and_digest("ha256", &DIGEST_SHA256).is_err()); + } } diff --git a/tvix/nix-compat/src/derivation/tests/mod.rs b/tvix/nix-compat/src/derivation/tests/mod.rs index 36b44e047f9a..b969625d9760 100644 --- a/tvix/nix-compat/src/derivation/tests/mod.rs +++ b/tvix/nix-compat/src/derivation/tests/mod.rs @@ -315,7 +315,7 @@ fn output_path_construction() { "out".to_string(), Output { path: "".to_string(), // will be calculated - hash_with_mode: Some(crate::nixhash::NixHashWithMode::Recursive( + ca_hash: Some(crate::nixhash::CAHash::Nar( crate::nixhash::from_algo_and_digest( crate::nixhash::HashAlgo::Sha256, &data_encoding::HEXLOWER @@ -376,7 +376,7 @@ fn output_path_construction() { "out".to_string(), Output { path: "".to_string(), // will be calculated - hash_with_mode: None, + ca_hash: None, }, ); diff --git a/tvix/nix-compat/src/derivation/write.rs b/tvix/nix-compat/src/derivation/write.rs index 7ebbbffa4b55..087227c99998 100644 --- a/tvix/nix-compat/src/derivation/write.rs +++ b/tvix/nix-compat/src/derivation/write.rs @@ -4,7 +4,7 @@ //! [ATerm]: http://program-transformation.org/Tools/ATermFormat.html use crate::aterm::escape_bytes; -use crate::derivation::output::Output; +use crate::derivation::{ca_kind_prefix, output::Output}; use bstr::BString; use std::{ collections::{BTreeMap, BTreeSet}, @@ -79,14 +79,10 @@ pub fn write_outputs( let mut elements: Vec<&str> = vec![output_name, &output.path]; - let (mode_and_algo, digest) = match &output.hash_with_mode { - Some(crate::nixhash::NixHashWithMode::Flat(h)) => ( - h.algo().to_string(), - data_encoding::HEXLOWER.encode(h.digest_as_bytes()), - ), - Some(crate::nixhash::NixHashWithMode::Recursive(h)) => ( - format!("r:{}", h.algo()), - data_encoding::HEXLOWER.encode(h.digest_as_bytes()), + let (mode_and_algo, digest) = match &output.ca_hash { + Some(ca_hash) => ( + format!("{}{}", ca_kind_prefix(ca_hash), ca_hash.digest().algo()), + data_encoding::HEXLOWER.encode(ca_hash.digest().digest_as_bytes()), ), None => ("".to_string(), "".to_string()), }; -- cgit 1.4.1