diff options
author | Florian Klink <flokli@flokli.de> | 2024-08-20T13·52+0300 |
---|---|---|
committer | clbot <clbot@tvl.fyi> | 2024-08-20T15·14+0000 |
commit | 2beabe968ca70ce2aef8def08d7dab7340979ea6 (patch) | |
tree | 5d8521dc0228451960c6c77fd094e3a90a39fae6 /tvix/glue | |
parent | 413135b9252de65c61045ade984de7b4d3319c2d (diff) |
refactor(nix-compat/store_path): make StorePath generic on S r/8545
Similar to how cl/12253 already did this for `Signature`, we apply the same logic to `StorePath`. `StorePathRef<'a>'` is now a `StorePath<&'a str>`, and there's less redundant code for the two different implementation. `.as_ref()` returns a `StorePathRef<'_>`, `.to_owned()` gives a `StorePath<String>` (for now). I briefly thought about only publicly exporting `StorePath<String>` as `StorePath`, but the diff is not too large and this will make it easier to gradually introduce more flexibility in which store paths to accept. Also, remove some silliness in `StorePath::from_absolute_path_full`, which now doesn't allocate anymore. Change-Id: Ife8843857a1a0a3a99177ca997649fd45b8198e6 Reviewed-on: https://cl.tvl.fyi/c/depot/+/12258 Autosubmit: flokli <flokli@flokli.de> Tested-by: BuildkiteCI Reviewed-by: Connor Brewster <cbrewster@hey.com>
Diffstat (limited to 'tvix/glue')
-rw-r--r-- | tvix/glue/src/builtins/derivation.rs | 2 | ||||
-rw-r--r-- | tvix/glue/src/builtins/import.rs | 20 | ||||
-rw-r--r-- | tvix/glue/src/known_paths.rs | 36 | ||||
-rw-r--r-- | tvix/glue/src/tvix_store_io.rs | 30 |
4 files changed, 45 insertions, 43 deletions
diff --git a/tvix/glue/src/builtins/derivation.rs b/tvix/glue/src/builtins/derivation.rs index 6a61b0dbaf36..5bc9dab71283 100644 --- a/tvix/glue/src/builtins/derivation.rs +++ b/tvix/glue/src/builtins/derivation.rs @@ -568,7 +568,7 @@ pub(crate) mod derivation_builtins { let blob_digest = blob_writer.close().await?; let ca_hash = CAHash::Text(Sha256::digest(&content).into()); - let store_path = + let store_path: StorePathRef = build_ca_path(name.to_str()?, &ca_hash, content.iter_ctx_plain(), false) .map_err(|_e| { nix_compat::derivation::DerivationError::InvalidOutputName( diff --git a/tvix/glue/src/builtins/import.rs b/tvix/glue/src/builtins/import.rs index 3660f575c843..34ae2778ecdd 100644 --- a/tvix/glue/src/builtins/import.rs +++ b/tvix/glue/src/builtins/import.rs @@ -112,7 +112,7 @@ mod import_builtins { use crate::tvix_store_io::TvixStoreIO; use nix_compat::nixhash::{CAHash, NixHash}; - use nix_compat::store_path::StorePath; + use nix_compat::store_path::StorePathRef; use sha2::Digest; use tokio::io::AsyncWriteExt; use tvix_eval::builtins::coerce_value_to_path; @@ -377,16 +377,16 @@ mod import_builtins { } })?; - let path_exists = if let Ok((store_path, sub_path)) = StorePath::from_absolute_path_full(p) - { - if !sub_path.as_os_str().is_empty() { - false + let path_exists = + if let Ok((store_path, sub_path)) = StorePathRef::from_absolute_path_full(p) { + if !sub_path.as_os_str().is_empty() { + false + } else { + state.store_path_exists(store_path.as_ref()).await? + } } else { - state.store_path_exists(store_path.as_ref()).await? - } - } else { - false - }; + false + }; if !path_exists { return Err(ImportError::PathNotInStore(p.into()).into()); diff --git a/tvix/glue/src/known_paths.rs b/tvix/glue/src/known_paths.rs index b303b8a13520..239acca9829a 100644 --- a/tvix/glue/src/known_paths.rs +++ b/tvix/glue/src/known_paths.rs @@ -25,27 +25,27 @@ pub struct KnownPaths { /// /// Keys are derivation paths, values are a tuple of the "hash derivation /// modulo" and the Derivation struct itself. - derivations: HashMap<StorePath, ([u8; 32], Derivation)>, + derivations: HashMap<StorePath<String>, ([u8; 32], Derivation)>, /// A map from output path to (one) drv path. /// Note that in the case of FODs, multiple drvs can produce the same output /// path. We use one of them. - outputs_to_drvpath: HashMap<StorePath, StorePath>, + outputs_to_drvpath: HashMap<StorePath<String>, StorePath<String>>, /// A map from output path to fetches (and their names). - outputs_to_fetches: HashMap<StorePath, (String, Fetch)>, + outputs_to_fetches: HashMap<StorePath<String>, (String, Fetch)>, } impl KnownPaths { /// Fetch the opaque "hash derivation modulo" for a given derivation path. - pub fn get_hash_derivation_modulo(&self, drv_path: &StorePath) -> Option<&[u8; 32]> { + pub fn get_hash_derivation_modulo(&self, drv_path: &StorePath<String>) -> Option<&[u8; 32]> { self.derivations .get(drv_path) .map(|(hash_derivation_modulo, _derivation)| hash_derivation_modulo) } /// Return a reference to the Derivation for a given drv path. - pub fn get_drv_by_drvpath(&self, drv_path: &StorePath) -> Option<&Derivation> { + pub fn get_drv_by_drvpath(&self, drv_path: &StorePath<String>) -> Option<&Derivation> { self.derivations .get(drv_path) .map(|(_hash_derivation_modulo, derivation)| derivation) @@ -54,7 +54,10 @@ impl KnownPaths { /// Return the drv path of the derivation producing the passed output path. /// Note there can be multiple Derivations producing the same output path in /// flight; this function will only return one of them. - pub fn get_drv_path_for_output_path(&self, output_path: &StorePath) -> Option<&StorePath> { + pub fn get_drv_path_for_output_path( + &self, + output_path: &StorePath<String>, + ) -> Option<&StorePath<String>> { self.outputs_to_drvpath.get(output_path) } @@ -63,7 +66,7 @@ impl KnownPaths { /// be fully calculated. /// All input derivations this refers to must also be inserted to this /// struct. - pub fn add_derivation(&mut self, drv_path: StorePath, drv: Derivation) { + pub fn add_derivation(&mut self, drv_path: StorePath<String>, drv: Derivation) { // check input derivations to have been inserted. #[cfg(debug_assertions)] { @@ -124,14 +127,17 @@ impl KnownPaths { /// Return the name and fetch producing the passed output path. /// Note there can also be (multiple) Derivations producing the same output path. - pub fn get_fetch_for_output_path(&self, output_path: &StorePath) -> Option<(String, Fetch)> { + pub fn get_fetch_for_output_path( + &self, + output_path: &StorePath<String>, + ) -> Option<(String, Fetch)> { self.outputs_to_fetches .get(output_path) .map(|(name, fetch)| (name.to_owned(), fetch.to_owned())) } /// Returns an iterator over all known derivations and their store path. - pub fn get_derivations(&self) -> impl Iterator<Item = (&StorePath, &Derivation)> { + pub fn get_derivations(&self) -> impl Iterator<Item = (&StorePath<String>, &Derivation)> { self.derivations.iter().map(|(k, v)| (k, &v.1)) } } @@ -156,26 +162,26 @@ mod tests { "tests/ch49594n9avinrf8ip0aslidkc4lxkqv-foo.drv" )) .expect("must parse"); - static ref BAR_DRV_PATH: StorePath = + static ref BAR_DRV_PATH: StorePath<String> = StorePath::from_bytes(b"ss2p4wmxijn652haqyd7dckxwl4c7hxx-bar.drv").expect("must parse"); - static ref FOO_DRV_PATH: StorePath = + static ref FOO_DRV_PATH: StorePath<String> = StorePath::from_bytes(b"ch49594n9avinrf8ip0aslidkc4lxkqv-foo.drv").expect("must parse"); - static ref BAR_OUT_PATH: StorePath = + static ref BAR_OUT_PATH: StorePath<String> = StorePath::from_bytes(b"mp57d33657rf34lzvlbpfa1gjfv5gmpg-bar").expect("must parse"); - static ref FOO_OUT_PATH: StorePath = + static ref FOO_OUT_PATH: StorePath<String> = StorePath::from_bytes(b"fhaj6gmwns62s6ypkcldbaj2ybvkhx3p-foo").expect("must parse"); static ref FETCH_URL : Fetch = Fetch::URL{ url: Url::parse("https://raw.githubusercontent.com/aaptel/notmuch-extract-patch/f732a53e12a7c91a06755ebfab2007adc9b3063b/notmuch-extract-patch").unwrap(), exp_hash: Some(nixhash::from_sri_str("sha256-Xa1Jbl2Eq5+L0ww+Ph1osA3Z/Dxe/RkN1/dITQCdXFk=").unwrap()) }; - static ref FETCH_URL_OUT_PATH: StorePath = StorePath::from_bytes(b"06qi00hylriyfm0nl827crgjvbax84mz-notmuch-extract-patch").unwrap(); + static ref FETCH_URL_OUT_PATH: StorePath<String> = StorePath::from_bytes(b"06qi00hylriyfm0nl827crgjvbax84mz-notmuch-extract-patch").unwrap(); static ref FETCH_TARBALL : Fetch = Fetch::Tarball{ url: Url::parse("https://github.com/NixOS/nixpkgs/archive/91050ea1e57e50388fa87a3302ba12d188ef723a.tar.gz").unwrap(), exp_nar_sha256: Some(nixbase32::decode_fixed("1hf6cgaci1n186kkkjq106ryf8mmlq9vnwgfwh625wa8hfgdn4dm").unwrap()) }; - static ref FETCH_TARBALL_OUT_PATH: StorePath = StorePath::from_bytes(b"7adgvk5zdfq4pwrhsm3n9lzypb12gw0g-source").unwrap(); + static ref FETCH_TARBALL_OUT_PATH: StorePath<String> = StorePath::from_bytes(b"7adgvk5zdfq4pwrhsm3n9lzypb12gw0g-source").unwrap(); } /// ensure we don't allow acdding a Derivation that depends on another, diff --git a/tvix/glue/src/tvix_store_io.rs b/tvix/glue/src/tvix_store_io.rs index 0e12c261f575..93675b438f25 100644 --- a/tvix/glue/src/tvix_store_io.rs +++ b/tvix/glue/src/tvix_store_io.rs @@ -105,7 +105,7 @@ impl TvixStoreIO { #[instrument(skip(self, store_path), fields(store_path=%store_path, indicatif.pb_show=1), ret(level = Level::TRACE), err)] async fn store_path_to_node( &self, - store_path: &StorePath, + store_path: &StorePath<String>, sub_path: &Path, ) -> io::Result<Option<Node>> { // Find the root node for the store_path. @@ -213,7 +213,7 @@ impl TvixStoreIO { }; // convert output names to actual paths - let output_paths: Vec<StorePath> = output_names + let output_paths: Vec<StorePath<String>> = output_names .iter() .map(|output_name| { input_drv @@ -372,13 +372,13 @@ impl TvixStoreIO { .map_err(|e| std::io::Error::new(io::ErrorKind::Other, e)) } - pub(crate) async fn node_to_path_info( + pub(crate) async fn node_to_path_info<'a>( &self, - name: &str, + name: &'a str, path: &Path, ca: &CAHash, root_node: Node, - ) -> io::Result<(PathInfo, NixHash, StorePath)> { + ) -> io::Result<(PathInfo, NixHash, StorePathRef<'a>)> { // Ask the PathInfoService for the NAR size and sha256 // We always need it no matter what is the actual hash mode // because the path info construct a narinfo which *always* @@ -411,20 +411,16 @@ impl TvixStoreIO { root_node, ); - Ok(( - path_info, - NixHash::Sha256(nar_sha256), - output_path.to_owned(), - )) + Ok((path_info, NixHash::Sha256(nar_sha256), output_path)) } - pub(crate) async fn register_node_in_path_info_service( + pub(crate) async fn register_node_in_path_info_service<'a>( &self, - name: &str, + name: &'a str, path: &Path, ca: &CAHash, root_node: Node, - ) -> io::Result<StorePath> { + ) -> io::Result<StorePathRef<'a>> { let (path_info, _, output_path) = self.node_to_path_info(name, path, ca, root_node).await?; let _path_info = self.path_info_service.as_ref().put(path_info).await?; @@ -449,7 +445,7 @@ impl EvalIO for TvixStoreIO { { if self .tokio_handle - .block_on(self.store_path_to_node(&store_path, &sub_path))? + .block_on(self.store_path_to_node(&store_path, sub_path))? .is_some() { Ok(true) @@ -471,7 +467,7 @@ impl EvalIO for TvixStoreIO { { if let Some(node) = self .tokio_handle - .block_on(async { self.store_path_to_node(&store_path, &sub_path).await })? + .block_on(async { self.store_path_to_node(&store_path, sub_path).await })? { // depending on the node type, treat open differently match node { @@ -527,7 +523,7 @@ impl EvalIO for TvixStoreIO { { if let Some(node) = self .tokio_handle - .block_on(async { self.store_path_to_node(&store_path, &sub_path).await })? + .block_on(async { self.store_path_to_node(&store_path, sub_path).await })? { match node { Node::Directory { .. } => Ok(FileType::Directory), @@ -549,7 +545,7 @@ impl EvalIO for TvixStoreIO { { if let Some(node) = self .tokio_handle - .block_on(async { self.store_path_to_node(&store_path, &sub_path).await })? + .block_on(async { self.store_path_to_node(&store_path, sub_path).await })? { match node { Node::Directory { digest, .. } => { |