refactor(tvix/nix-compat): move build_store_path out of derivation r/6060

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 <flokli@flokli.de>
Reviewed-by: tazjin <tazjin@tvl.su>
Tested-by: BuildkiteCI

7 files changed, 156 insertions, 142 deletions

diff --git a/tvix/nix-compat/src/derivation/errors.rs b/tvix/nix-compat/src/derivation/errors.rs
index 80d1e6c6a4a9..2ffd56cf64da 100644
--- a/tvix/nix-compat/src/derivation/errors.rs
+++ b/tvix/nix-compat/src/derivation/errors.rs
@@ -1,4 +1,4 @@
-use crate::{nixbase32::Nixbase32DecodeError, store_path::ParseStorePathError};
+use crate::{nixbase32::Nixbase32DecodeError, store_path};
 use thiserror::Error;
 
 /// Errors that can occur during the validation of Derivation structs.
@@ -17,7 +17,7 @@ pub enum DerivationError {
     InvalidOutput(String, OutputError),
     // input derivation
     #[error("unable to parse input derivation path {0}: {1}")]
-    InvalidInputDerivationPath(String, ParseStorePathError),
+    InvalidInputDerivationPath(String, store_path::Error),
     #[error("input derivation {0} doesn't end with .drv")]
     InvalidInputDerivationPrefix(String),
     #[error("input derivation {0} output names are empty")]
@@ -27,7 +27,7 @@ pub enum DerivationError {
 
     // input sources
     #[error("unable to parse input sources path {0}: {1}")]
-    InvalidInputSourcesPath(String, ParseStorePathError),
+    InvalidInputSourcesPath(String, store_path::Error),
 
     // platform
     #[error("invalid platform field: {0}")]
@@ -46,8 +46,8 @@ pub enum DerivationError {
 // [crate::derivation::Output] of a [crate::derivation::Derviation].
 #[derive(Debug, Error, PartialEq)]
 pub enum OutputError {
-    #[error("Invalid ouput path {0}: {1}")]
-    InvalidOutputPath(String, ParseStorePathError),
+    #[error("Invalid output path {0}: {1}")]
+    InvalidOutputPath(String, store_path::Error),
     #[error("Invalid hash encoding: {0}")]
     InvalidHashEncoding(String, Nixbase32DecodeError),
     #[error("Invalid hash algo: {0}")]
diff --git a/tvix/nix-compat/src/derivation/mod.rs b/tvix/nix-compat/src/derivation/mod.rs
index 6912b692b2ca..2fa77cd26f49 100644
--- a/tvix/nix-compat/src/derivation/mod.rs
+++ b/tvix/nix-compat/src/derivation/mod.rs
@@ -1,7 +1,8 @@
 use crate::{
     nixhash::HashAlgo,
-    store_path::{self, StorePath},
-    texthash::text_hash_string,
+    store_path::{
+        self, build_store_path_from_fingerprint, build_store_path_from_references, StorePath,
+    },
 };
 use serde::{Deserialize, Serialize};
 use sha2::{Digest, Sha256};
@@ -10,7 +11,6 @@ use std::collections::{BTreeMap, BTreeSet};
 mod errors;
 mod output;
 mod string_escape;
-mod utils;
 mod validate;
 mod write;
 
@@ -21,7 +21,6 @@ mod tests;
 pub use crate::nixhash::{NixHash, NixHashWithMode};
 pub use errors::{DerivationError, OutputError};
 pub use output::Output;
-pub use utils::path_with_references;
 
 #[derive(Clone, Debug, Default, Eq, PartialEq, Serialize, Deserialize)]
 pub struct Derivation {
@@ -77,12 +76,12 @@ impl Derivation {
         buffer
     }
 
-    /// Returns the drv path of a Derivation struct.
+    /// Returns the drv path of a [Derivation] struct.
     ///
-    /// The drv path is calculated by calculating the [text_hash_string], using
-    /// the `name` with a `.drv` suffix as name, all d.InputDerivations and d.InputSources as references,
-    /// and the ATerm representation of the Derivation as contents.
-    /// The text_hash_string is then passed to the build_store_path function.
+    /// The drv path is calculated by invoking [build_store_path_from_references], using
+    /// the `name` with a `.drv` suffix as name, all [Derivation::input_sources] and
+    /// keys of [Derivation::input_derivations] as references, and the ATerm string of
+    /// the [Derivation] as content.
     pub fn calculate_derivation_path(&self, name: &str) -> Result<StorePath, DerivationError> {
         // 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 d7b63a45ad9c..18aa23534c77 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::<String>::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::<String>::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 5c41fa6e55d2..000000000000
--- 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<u8> {
-    let mut output: Vec<u8> = 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<StorePath, DerivationError> {
-    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<S: AsRef<str>, I: IntoIterator<Item = S>, C: AsRef<[u8]>>(
-    name: &str,
-    content: C,
-    references: I,
-) -> Result<StorePath, DerivationError> {
-    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 ec9ddd1ffa7b..2aac179bcfce 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
index 980312bff1b1..0e004ccd7a10 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<StorePath, ParseStorePathError> {
+    pub fn from_string(s: &str) -> Result<StorePath, Error> {
         // 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<StorePath, ParseStorePathError> {
+    pub fn from_absolute_path(s: &str) -> Result<StorePath, Error> {
         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 000000000000..0d96414a212d
--- /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<u8> {
+    let mut output: Vec<u8> = 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<str>,
+    I: IntoIterator<Item = S>,
+    C: AsRef<[u8]>,
+>(
+    name: &str,
+    content: C,
+    references: I,
+) -> Result<StorePath, Error> {
+    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<StorePath, Error> {
+    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::<String>::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::<String>::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"
+        );
+    }
+}