From 5f2c2e79e1778cda877d6122dd24be08740c8720 Mon Sep 17 00:00:00 2001 From: Florian Klink Date: Thu, 30 Mar 2023 14:01:38 +0200 Subject: [PATCH] refactor(tvix/nix-compat): move build_store_path out of derivation This doesn't have anything to do with ATerms, we just happen to be using the aterm representation of a Derivation as contents. Moving this into store_path/utils.rs makes these things much cleaner - Have a build_store_path_from_references function, and a build_store_path_from_fingerprint helper function that makes use of it. build_store_path_from_references is invoked from the derivation module which can be used to calculate the derivation path. In the derivation module, we also invoke build_store_path_from_fingerprint during the output path calculation. Change-Id: Ia8d61a5e8e5d3f396f93593676ed3f5d1a3f1d66 Reviewed-on: https://cl.tvl.fyi/c/depot/+/8367 Autosubmit: flokli Reviewed-by: tazjin Tested-by: BuildkiteCI --- tvix/cli/src/derivation.rs | 19 ++- tvix/nix-compat/src/derivation/errors.rs | 10 +- tvix/nix-compat/src/derivation/mod.rs | 28 ++--- tvix/nix-compat/src/derivation/tests/mod.rs | 39 +----- tvix/nix-compat/src/derivation/utils.rs | 54 -------- tvix/nix-compat/src/lib.rs | 18 --- .../src/{store_path.rs => store_path/mod.rs} | 31 +++-- tvix/nix-compat/src/store_path/utils.rs | 118 ++++++++++++++++++ tvix/store/src/proto/mod.rs | 6 +- tvix/store/src/proto/tests/pathinfo.rs | 8 +- 10 files changed, 176 insertions(+), 155 deletions(-) delete mode 100644 tvix/nix-compat/src/derivation/utils.rs rename tvix/nix-compat/src/{store_path.rs => store_path/mod.rs} (88%) create mode 100644 tvix/nix-compat/src/store_path/utils.rs diff --git a/tvix/cli/src/derivation.rs b/tvix/cli/src/derivation.rs index 5a04c2a11..23c49d3d0 100644 --- a/tvix/cli/src/derivation.rs +++ b/tvix/cli/src/derivation.rs @@ -1,6 +1,6 @@ //! Implements `builtins.derivation`, the core of what makes Nix build packages. use nix_compat::derivation::Derivation; -use nix_compat::{hash_placeholder, nixhash}; +use nix_compat::nixhash; use std::cell::RefCell; use std::collections::{btree_map, BTreeSet}; use std::rc::Rc; @@ -202,6 +202,7 @@ async fn strong_coerce_to_string(co: &GenCo, val: Value) -> Result Result { // append .drv to the name let name = &format!("{}.drv", name); @@ -97,9 +96,8 @@ impl Derivation { inputs }; - let text_hash_str = &text_hash_string(name, self.to_aterm_string(), references); - - utils::build_store_path(text_hash_str, name) + build_store_path_from_references(name, self.to_aterm_string(), references) + .map_err(|_e| DerivationError::InvalidOutputName(name.to_string())) } /// Returns the FOD digest, if the derivation is fixed-output, or None if @@ -249,8 +247,10 @@ impl Derivation { store_path::STORE_DIR, output_path_name, )); - let abs_store_path = - utils::build_store_path(&fp, &output_path_name)?.to_absolute_path(); + + let abs_store_path = build_store_path_from_fingerprint(&output_path_name, &fp) + .map_err(|_e| DerivationError::InvalidOutputName(output_path_name.to_string()))? + .to_absolute_path(); output.path = abs_store_path.clone(); self.environment diff --git a/tvix/nix-compat/src/derivation/tests/mod.rs b/tvix/nix-compat/src/derivation/tests/mod.rs index d7b63a45a..18aa23534 100644 --- a/tvix/nix-compat/src/derivation/tests/mod.rs +++ b/tvix/nix-compat/src/derivation/tests/mod.rs @@ -1,7 +1,7 @@ use crate::derivation::output::Output; use crate::derivation::Derivation; use crate::nixhash::NixHash; -use crate::store_path::StorePath; +use crate::store_path::{build_store_path_from_references, StorePath}; use std::collections::BTreeSet; use std::fs::File; use std::io::Read; @@ -313,40 +313,3 @@ fn output_path_construction() { .expect("must succeed") ); } - -#[test] -fn path_with_zero_references() { - // This hash should match `builtins.toFile`, e.g.: - // - // nix-repl> builtins.toFile "foo" "bar" - // "/nix/store/vxjiwkjkn7x4079qvh1jkl5pn05j2aw0-foo" - - let store_path = crate::derivation::path_with_references("foo", "bar", Vec::::new()) - .expect("path_with_references() should succeed"); - - assert_eq!( - store_path.to_absolute_path().as_str(), - "/nix/store/vxjiwkjkn7x4079qvh1jkl5pn05j2aw0-foo" - ); -} - -#[test] -fn path_with_non_zero_references() { - // This hash should match: - // - // nix-repl> builtins.toFile "baz" "${builtins.toFile "foo" "bar"}" - // "/nix/store/5xd714cbfnkz02h2vbsj4fm03x3f15nf-baz" - - let inner = crate::derivation::path_with_references("foo", "bar", Vec::::new()) - .expect("path_with_references() should succeed"); - let inner_path = inner.to_absolute_path(); - - let outer = - crate::derivation::path_with_references("baz", &inner_path, vec![inner_path.as_str()]) - .expect("path_with_references() should succeed"); - - assert_eq!( - outer.to_absolute_path().as_str(), - "/nix/store/5xd714cbfnkz02h2vbsj4fm03x3f15nf-baz" - ); -} diff --git a/tvix/nix-compat/src/derivation/utils.rs b/tvix/nix-compat/src/derivation/utils.rs deleted file mode 100644 index 5c41fa6e5..000000000 --- a/tvix/nix-compat/src/derivation/utils.rs +++ /dev/null @@ -1,54 +0,0 @@ -use crate::derivation::DerivationError; -use crate::nixbase32; -use crate::store_path::StorePath; -use crate::texthash::text_hash_string; -use sha2::{Digest, Sha256}; - -/// compress_hash takes an arbitrarily long sequence of bytes (usually -/// a hash digest), and returns a sequence of bytes of length -/// output_size. -/// -/// It's calculated by rotating through the bytes in the output buffer -/// (zero- initialized), and XOR'ing with each byte of the passed -/// input. It consumes 1 byte at a time, and XOR's it with the current -/// value in the output buffer. -/// -/// This mimics equivalent functionality in C++ Nix. -fn compress_hash(input: &[u8], output_size: usize) -> Vec { - let mut output: Vec = vec![0; output_size]; - - for (ii, ch) in input.iter().enumerate() { - output[ii % output_size] ^= ch; - } - - output -} - -/// This returns a store path, either of a derivation or a regular output. -/// The string is hashed with sha256, its digest is compressed to 20 bytes, and -/// nixbase32-encoded (32 characters) -pub(super) fn build_store_path( - fingerprint: &str, - name: &str, -) -> Result { - let digest = { - let hasher = Sha256::new_with_prefix(fingerprint); - hasher.finalize() - }; - let compressed = compress_hash(&digest, 20); - StorePath::from_string(format!("{}-{}", nixbase32::encode(&compressed), name,).as_str()) - .map_err(|_e| DerivationError::InvalidOutputName(name.to_string())) - // Constructing the StorePath can only fail if the passed output name was - // invalid, so map errors to a [DerivationError::InvalidOutputName]. -} - -/// Build a store path for a literal text file in the store that may -/// contain references. -pub fn path_with_references, I: IntoIterator, C: AsRef<[u8]>>( - name: &str, - content: C, - references: I, -) -> Result { - let text_hash_str = text_hash_string(name, content, references); - build_store_path(&text_hash_str, name) -} diff --git a/tvix/nix-compat/src/lib.rs b/tvix/nix-compat/src/lib.rs index ec9ddd1ff..2aac179bc 100644 --- a/tvix/nix-compat/src/lib.rs +++ b/tvix/nix-compat/src/lib.rs @@ -1,5 +1,3 @@ -use sha2::{Digest, Sha256}; - pub mod derivation; pub mod nar; pub mod nixbase32; @@ -8,19 +6,3 @@ mod nixhash_algos; mod nixhash_with_mode; pub mod store_path; mod texthash; - -/// Nix placeholders (i.e. values returned by `builtins.placeholder`) -/// are used to populate outputs with paths that must be -/// string-replaced with the actual placeholders later, at runtime. -/// -/// The actual placeholder is basically just a SHA256 hash encoded in -/// cppnix format. -pub fn hash_placeholder(name: &str) -> String { - let digest = { - let mut hasher = Sha256::new(); - hasher.update(format!("nix-output:{}", name)); - hasher.finalize() - }; - - format!("/{}", nixbase32::encode(&digest)) -} diff --git a/tvix/nix-compat/src/store_path.rs b/tvix/nix-compat/src/store_path/mod.rs similarity index 88% rename from tvix/nix-compat/src/store_path.rs rename to tvix/nix-compat/src/store_path/mod.rs index 980312bff..0e004ccd7 100644 --- a/tvix/nix-compat/src/store_path.rs +++ b/tvix/nix-compat/src/store_path/mod.rs @@ -2,6 +2,13 @@ use crate::nixbase32::{self, Nixbase32DecodeError}; use std::fmt; use thiserror::Error; +mod utils; + +pub use utils::{ + build_store_path_from_fingerprint, build_store_path_from_references, compress_hash, + hash_placeholder, +}; + pub const DIGEST_SIZE: usize = 20; // lazy_static doesn't allow us to call NIXBASE32.encode_len(), so we ran it // manually and have an assert in the tests. @@ -14,7 +21,7 @@ pub const STORE_DIR_WITH_SLASH: &str = "/nix/store/"; /// Errors that can occur during the validation of name characters. #[derive(Debug, PartialEq, Eq, Error)] -pub enum ParseStorePathError { +pub enum Error { #[error("Dash is missing between hash and name")] MissingDash(), #[error("Hash encoding is invalid: {0}")] @@ -42,25 +49,23 @@ pub struct StorePath { } impl StorePath { - pub fn from_string(s: &str) -> Result { + pub fn from_string(s: &str) -> Result { // the whole string needs to be at least: // // - 32 characters (encoded hash) // - 1 dash // - 1 character for the name if s.len() < ENCODED_DIGEST_SIZE + 2 { - return Err(ParseStorePathError::InvalidName("".to_string())); + return Err(Error::InvalidName("".to_string())); } let digest = match nixbase32::decode(s[..ENCODED_DIGEST_SIZE].as_bytes()) { Ok(decoded) => decoded, - Err(decoder_error) => { - return Err(ParseStorePathError::InvalidHashEncoding(decoder_error)) - } + Err(decoder_error) => return Err(Error::InvalidHashEncoding(decoder_error)), }; if s.as_bytes()[ENCODED_DIGEST_SIZE] != b'-' { - return Err(ParseStorePathError::MissingDash()); + return Err(Error::MissingDash()); } StorePath::validate_name(&s[ENCODED_DIGEST_SIZE + 2..])?; @@ -73,10 +78,10 @@ impl StorePath { /// Construct a [StorePath] from an absolute store path string. /// That is a string starting with the store prefix (/nix/store) - pub fn from_absolute_path(s: &str) -> Result { + pub fn from_absolute_path(s: &str) -> Result { match s.strip_prefix(STORE_DIR_WITH_SLASH) { Some(s_stripped) => Self::from_string(s_stripped), - None => Err(ParseStorePathError::MissingStoreDir()), + None => Err(Error::MissingStoreDir()), } } @@ -87,7 +92,7 @@ impl StorePath { } /// Checks a given &str to match the restrictions for store path names. - pub fn validate_name(s: &str) -> Result<(), ParseStorePathError> { + pub fn validate_name(s: &str) -> Result<(), Error> { for c in s.chars() { if c.is_ascii_alphanumeric() || c == '-' @@ -100,7 +105,7 @@ impl StorePath { continue; } - return Err(ParseStorePathError::InvalidName(s.to_string())); + return Err(Error::InvalidName(s.to_string())); } Ok(()) @@ -118,7 +123,7 @@ mod tests { use crate::nixbase32; use crate::store_path::{DIGEST_SIZE, ENCODED_DIGEST_SIZE}; - use super::{ParseStorePathError, StorePath}; + use super::{Error, StorePath}; #[test] fn encoded_digest_size() { @@ -191,7 +196,7 @@ mod tests { #[test] fn absolute_path_missing_prefix() { assert_eq!( - ParseStorePathError::MissingStoreDir(), + Error::MissingStoreDir(), StorePath::from_absolute_path("foobar-123").expect_err("must fail") ); } diff --git a/tvix/nix-compat/src/store_path/utils.rs b/tvix/nix-compat/src/store_path/utils.rs new file mode 100644 index 000000000..0d96414a2 --- /dev/null +++ b/tvix/nix-compat/src/store_path/utils.rs @@ -0,0 +1,118 @@ +use crate::nixbase32; +use crate::store_path::StorePath; +use crate::texthash::text_hash_string; +use sha2::{Digest, Sha256}; + +use super::Error; + +/// compress_hash takes an arbitrarily long sequence of bytes (usually +/// a hash digest), and returns a sequence of bytes of length +/// output_size. +/// +/// It's calculated by rotating through the bytes in the output buffer +/// (zero- initialized), and XOR'ing with each byte of the passed +/// input. It consumes 1 byte at a time, and XOR's it with the current +/// value in the output buffer. +/// +/// This mimics equivalent functionality in C++ Nix. +pub fn compress_hash(input: &[u8], output_size: usize) -> Vec { + let mut output: Vec = vec![0; output_size]; + + for (ii, ch) in input.iter().enumerate() { + output[ii % output_size] ^= ch; + } + + output +} + +/// This builds a store path, by calculating the text_hash_string of either a +/// derivation or a literal text file that may contain references. +pub fn build_store_path_from_references< + S: AsRef, + I: IntoIterator, + C: AsRef<[u8]>, +>( + name: &str, + content: C, + references: I, +) -> Result { + let text_hash_str = text_hash_string(name, content, references); + build_store_path_from_fingerprint(name, &text_hash_str) +} + +/// This builds a store path from a fingerprint. +/// Usually, that function is used from [build_store_path_from_references] and +/// passed a "text hash string" (starting with "text:" as fingerprint), +/// but other fingerprints starting with "output:" are also used in Derivation +/// output path calculation. +/// +/// The fingerprint is hashed with sha256, its digest is compressed to 20 bytes, +/// and nixbase32-encoded (32 characters). +pub fn build_store_path_from_fingerprint( + name: &str, + fingerprint: &str, +) -> Result { + let digest = { + let hasher = Sha256::new_with_prefix(fingerprint); + hasher.finalize() + }; + let compressed = compress_hash(&digest, 20); + StorePath::from_string(format!("{}-{}", nixbase32::encode(&compressed), name).as_str()) +} + +/// Nix placeholders (i.e. values returned by `builtins.placeholder`) +/// are used to populate outputs with paths that must be +/// string-replaced with the actual placeholders later, at runtime. +/// +/// The actual placeholder is basically just a SHA256 hash encoded in +/// cppnix format. +pub fn hash_placeholder(name: &str) -> String { + let digest = { + let mut hasher = Sha256::new(); + hasher.update(format!("nix-output:{}", name)); + hasher.finalize() + }; + + format!("/{}", nixbase32::encode(&digest)) +} + +#[cfg(test)] +mod test { + use crate::store_path::build_store_path_from_references; + + #[test] + fn build_store_path_with_zero_references() { + // This hash should match `builtins.toFile`, e.g.: + // + // nix-repl> builtins.toFile "foo" "bar" + // "/nix/store/vxjiwkjkn7x4079qvh1jkl5pn05j2aw0-foo" + + let store_path = build_store_path_from_references("foo", "bar", Vec::::new()) + .expect("build_store_path() should succeed"); + + assert_eq!( + store_path.to_absolute_path().as_str(), + "/nix/store/vxjiwkjkn7x4079qvh1jkl5pn05j2aw0-foo" + ); + } + + #[test] + fn build_store_path_with_non_zero_references() { + // This hash should match: + // + // nix-repl> builtins.toFile "baz" "${builtins.toFile "foo" "bar"}" + // "/nix/store/5xd714cbfnkz02h2vbsj4fm03x3f15nf-baz" + + let inner = build_store_path_from_references("foo", "bar", Vec::::new()) + .expect("path_with_references() should succeed"); + let inner_path = inner.to_absolute_path(); + + let outer = build_store_path_from_references("baz", &inner_path, vec![inner_path.as_str()]) + .expect("path_with_references() should succeed"); + + assert_eq!( + outer.to_absolute_path().as_str(), + "/nix/store/5xd714cbfnkz02h2vbsj4fm03x3f15nf-baz" + ); + } +} diff --git a/tvix/store/src/proto/mod.rs b/tvix/store/src/proto/mod.rs index f5945d650..5002d7e77 100644 --- a/tvix/store/src/proto/mod.rs +++ b/tvix/store/src/proto/mod.rs @@ -5,7 +5,7 @@ use thiserror::Error; use prost::Message; -use nix_compat::store_path::{ParseStorePathError, StorePath}; +use nix_compat::store_path::{self, StorePath}; mod grpc_blobservice_wrapper; mod grpc_directoryservice_wrapper; @@ -52,7 +52,7 @@ pub enum ValidatePathInfoError { /// Invalid node name encountered. #[error("Failed to parse {0} as NixPath: {1}")] - InvalidNodeName(String, ParseStorePathError), + InvalidNodeName(String, store_path::Error), /// The digest the (root) node refers to has invalid length. #[error("Invalid Digest length: {0}")] @@ -91,7 +91,7 @@ fn validate_digest(digest: &Vec, err: fn(usize) -> E) -> Result<(), E> { /// On error, it returns an error generated from the supplied constructor. fn parse_node_name_root( name: &str, - err: fn(String, ParseStorePathError) -> E, + err: fn(String, store_path::Error) -> E, ) -> Result { match StorePath::from_string(name) { Ok(np) => Ok(np), diff --git a/tvix/store/src/proto/tests/pathinfo.rs b/tvix/store/src/proto/tests/pathinfo.rs index 584bc5920..0cc8f8758 100644 --- a/tvix/store/src/proto/tests/pathinfo.rs +++ b/tvix/store/src/proto/tests/pathinfo.rs @@ -1,6 +1,6 @@ use crate::proto::{self, Node, PathInfo, ValidatePathInfoError}; use lazy_static::lazy_static; -use nix_compat::store_path::{ParseStorePathError, StorePath}; +use nix_compat::store_path::{self, StorePath}; use test_case::test_case; lazy_static! { @@ -66,7 +66,7 @@ fn validate_no_node( }, Err(ValidatePathInfoError::InvalidNodeName( "invalid".to_string(), - ParseStorePathError::InvalidName("".to_string()) + store_path::Error::InvalidName("".to_string()) )); "invalid node name" )] @@ -111,7 +111,7 @@ fn validate_directory( }, Err(ValidatePathInfoError::InvalidNodeName( "invalid".to_string(), - ParseStorePathError::InvalidName("".to_string()) + store_path::Error::InvalidName("".to_string()) )); "invalid node name" )] @@ -141,7 +141,7 @@ fn validate_file(t_file_node: proto::FileNode, t_result: Result