From 9118dc8a5056ad0cdb91a0688f50ac5d6e28a1ec Mon Sep 17 00:00:00 2001
From: Florian Klink <flokli@flokli.de>
Date: Tue, 17 Oct 2023 23:27:37 +0100
Subject: refactor(tvix/store): move deriver field into narinfo submessage

This information is present in the .narinfo files, it should have gone
there.

Change-Id: Ib43d0cf30c2795bf1fe77c46646174353ade0458
Reviewed-on: https://cl.tvl.fyi/c/depot/+/9794
Autosubmit: flokli <flokli@flokli.de>
Reviewed-by: Connor Brewster <cbrewster@hey.com>
Tested-by: BuildkiteCI
---
 tvix/store/protos/pathinfo.proto       |  7 +++----
 tvix/store/src/bin/tvix-store.rs       |  2 +-
 tvix/store/src/proto/mod.rs            | 23 +++++++++++++----------
 tvix/store/src/proto/tests/pathinfo.rs | 10 ++++++----
 tvix/store/src/tests/fixtures.rs       |  2 +-
 5 files changed, 24 insertions(+), 20 deletions(-)

(limited to 'tvix/store')

diff --git a/tvix/store/protos/pathinfo.proto b/tvix/store/protos/pathinfo.proto
index 316201226884..abddf31acd2b 100644
--- a/tvix/store/protos/pathinfo.proto
+++ b/tvix/store/protos/pathinfo.proto
@@ -21,10 +21,6 @@ message PathInfo {
 
     // see below.
     NARInfo narinfo = 3;
-
-    // The StorePath of the .drv file producing this output.
-    // The .drv suffix is omitted in its `name` field.
-    StorePath deriver = 4;
 }
 
 // Represents a path in the Nix store (a direct child of STORE_DIR).
@@ -70,4 +66,7 @@ message NARInfo {
     // so we need to keep them somewhere.
     repeated string reference_names = 4;
 
+    // The StorePath of the .drv file producing this output.
+    // The .drv suffix is omitted in its `name` field.
+    StorePath deriver = 5;
 }
diff --git a/tvix/store/src/bin/tvix-store.rs b/tvix/store/src/bin/tvix-store.rs
index db19c532f380..891b10da6955 100644
--- a/tvix/store/src/bin/tvix-store.rs
+++ b/tvix/store/src/bin/tvix-store.rs
@@ -302,8 +302,8 @@ async fn main() -> Result<(), Box<dyn std::error::Error>> {
                                 nar_sha256: nar_sha256.to_vec().into(),
                                 signatures: vec![],
                                 reference_names: vec![],
+                                deriver: None,
                             }),
-                            deriver: None,
                         };
 
                         // put into [PathInfoService], and return the PathInfo that we get back
diff --git a/tvix/store/src/proto/mod.rs b/tvix/store/src/proto/mod.rs
index a982d71f8886..4b5bf4f59479 100644
--- a/tvix/store/src/proto/mod.rs
+++ b/tvix/store/src/proto/mod.rs
@@ -140,6 +140,19 @@ impl PathInfo {
                         );
                     }
                 }
+
+                // If the Deriver field is populated, ensure it parses to a
+                // [store_path::StorePath].
+                // We can't check for it to *not* end with .drv, as the .drv files produced by
+                // recursive Nix end with multiple .drv suffixes, and only one is popped when
+                // converting to this field.
+                if let Some(deriver) = &narinfo.deriver {
+                    store_path::StorePath::from_name_and_digest(
+                        deriver.name.clone(),
+                        &deriver.digest,
+                    )
+                    .map_err(ValidatePathInfoError::InvalidDeriverField)?;
+                }
             }
         }
 
@@ -156,16 +169,6 @@ impl PathInfo {
             }
         };
 
-        // If the Deriver field is populated, ensure it parses to a
-        // [store_path::StorePath].
-        // We can't check for it to *not* end with .drv, as the .drv files produced by
-        // recursive Nix end with multiple .drv suffixes, and only one is popped when
-        // converting to this field.
-        if let Some(deriver) = &self.deriver {
-            store_path::StorePath::from_name_and_digest(deriver.name.clone(), &deriver.digest)
-                .map_err(ValidatePathInfoError::InvalidDeriverField)?;
-        }
-
         // return the root nix path
         Ok(root_nix_path)
     }
diff --git a/tvix/store/src/proto/tests/pathinfo.rs b/tvix/store/src/proto/tests/pathinfo.rs
index 03094ed734ce..392d5d8127b4 100644
--- a/tvix/store/src/proto/tests/pathinfo.rs
+++ b/tvix/store/src/proto/tests/pathinfo.rs
@@ -266,10 +266,11 @@ fn validate_symlink_target_null_byte_invalid() {
 /// Create a PathInfo with a correct deriver field and ensure it succeeds.
 #[test]
 fn validate_valid_deriver() {
-    let mut path_info = PATH_INFO_WITHOUT_NARINFO.clone();
+    let mut path_info = PATH_INFO_WITH_NARINFO.clone();
 
     // add a valid deriver
-    path_info.deriver = Some(crate::proto::StorePath {
+    let narinfo = path_info.narinfo.as_mut().unwrap();
+    narinfo.deriver = Some(crate::proto::StorePath {
         name: "foo".to_string(),
         digest: DUMMY_OUTPUT_HASH.clone(),
     });
@@ -280,10 +281,11 @@ fn validate_valid_deriver() {
 /// Create a PathInfo with a broken deriver field and ensure it fails.
 #[test]
 fn validate_invalid_deriver() {
-    let mut path_info = PATH_INFO_WITHOUT_NARINFO.clone();
+    let mut path_info = PATH_INFO_WITH_NARINFO.clone();
 
     // add a broken deriver (invalid digest)
-    path_info.deriver = Some(crate::proto::StorePath {
+    let narinfo = path_info.narinfo.as_mut().unwrap();
+    narinfo.deriver = Some(crate::proto::StorePath {
         name: "foo".to_string(),
         digest: vec![].into(),
     });
diff --git a/tvix/store/src/tests/fixtures.rs b/tvix/store/src/tests/fixtures.rs
index 5290581688e6..95e77e3ba7b2 100644
--- a/tvix/store/src/tests/fixtures.rs
+++ b/tvix/store/src/tests/fixtures.rs
@@ -109,7 +109,6 @@ lazy_static! {
         }),
         references: vec![DUMMY_OUTPUT_HASH.clone()],
         narinfo: None,
-        deriver: None,
     };
 
     /// A PathInfo message with .narinfo populated.
@@ -121,6 +120,7 @@ lazy_static! {
             nar_sha256: DUMMY_DIGEST.clone().into(),
             signatures: vec![],
             reference_names: vec![DUMMY_NAME.to_string()],
+            deriver: None,
         }),
       ..PATH_INFO_WITHOUT_NARINFO.clone()
     };
-- 
cgit 1.4.1