about summary refs log tree commit diff
path: root/tvix/glue
diff options
context:
space:
mode:
authorFlorian Klink <flokli@flokli.de>2024-08-20T13·52+0300
committerclbot <clbot@tvl.fyi>2024-08-20T15·14+0000
commit2beabe968ca70ce2aef8def08d7dab7340979ea6 (patch)
tree5d8521dc0228451960c6c77fd094e3a90a39fae6 /tvix/glue
parent413135b9252de65c61045ade984de7b4d3319c2d (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.rs2
-rw-r--r--tvix/glue/src/builtins/import.rs20
-rw-r--r--tvix/glue/src/known_paths.rs36
-rw-r--r--tvix/glue/src/tvix_store_io.rs30
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, .. } => {