about summary refs log tree commit diff
diff options
context:
space:
mode:
authorFlorian Klink <flokli@flokli.de>2024-04-07T20·40+0300
committerclbot <clbot@tvl.fyi>2024-04-07T21·21+0000
commitd00c75ea9cd2d31d6e0f02fa678678519317182c (patch)
treec5959f320619f2993d59d2eebe17310a2fc6dfac
parentefd8ab5e0b595648acad8a96d0b4f0d49fddee8f (diff)
fix(nix-compat/store_path): fix Deserialize with borrow r/7869
We were wrongly using `'de` as a lifetime for both `Deserializer` and
`StorePathRef`.

This prevented Deserializing into a struct containing `StorePathRef`.

See https://serde.rs/lifetimes.html#the-deserializede-lifetime, the last
part of the paragraph:

The 'de lifetime should not appear in the type to which the Deserialize
impl applies.

- // Do not do this. Sooner or later you will be sad.
- impl<'de> Deserialize<'de> for Q<'de> {
+ // Do this instead.
+ impl<'de: 'a, 'a> Deserialize<'de> for Q<'a> {

This fixes it, and adds a test, deserializing into a `Container` struct.

It also fixes the existing test cases, deserialize_ref was actually
deserialize_owned, and deserialize_owned didn't exist yet - but they
alone are not enough to provoke the lifetime issues.

Change-Id: Iaed2832998cae5f192eafe7fd5243e82ff6e051e
Reviewed-on: https://cl.tvl.fyi/c/depot/+/11372
Tested-by: BuildkiteCI
Autosubmit: flokli <flokli@flokli.de>
Reviewed-by: picnoir picnoir <picnoir@alternativebit.fr>
-rw-r--r--tvix/nix-compat/src/store_path/mod.rs40
1 files changed, 37 insertions, 3 deletions
diff --git a/tvix/nix-compat/src/store_path/mod.rs b/tvix/nix-compat/src/store_path/mod.rs
index 71b7bdcc740c..639bf4289ff6 100644
--- a/tvix/nix-compat/src/store_path/mod.rs
+++ b/tvix/nix-compat/src/store_path/mod.rs
@@ -275,7 +275,7 @@ impl<'a> StorePathRef<'a> {
     }
 }
 
-impl<'de> Deserialize<'de> for StorePathRef<'de> {
+impl<'de: 'a, 'a> Deserialize<'de> for StorePathRef<'a> {
     fn deserialize<D>(deserializer: D) -> Result<Self, D::Error>
     where
         D: serde::Deserializer<'de>,
@@ -373,15 +373,23 @@ impl fmt::Display for StorePathRef<'_> {
 
 #[cfg(test)]
 mod tests {
+    use super::Error;
     use std::cmp::Ordering;
     use std::path::PathBuf;
 
-    use crate::store_path::{StorePathRef, DIGEST_SIZE};
+    use crate::store_path::{StorePath, StorePathRef, DIGEST_SIZE};
     use hex_literal::hex;
     use pretty_assertions::assert_eq;
+    use serde::Deserialize;
     use test_case::test_case;
 
-    use super::{Error, StorePath};
+    #[derive(Deserialize)]
+    /// An example struct, holding a StorePathRef.
+    /// Used to test deserializing StorePathRef.
+    struct Container<'a> {
+        #[serde(borrow)]
+        store_path: StorePathRef<'a>,
+    }
 
     #[test]
     fn happy_path() {
@@ -550,6 +558,32 @@ mod tests {
         let store_path_str_json =
             "\"/nix/store/00bgd045z0d4icpbc2yyz4gx48ak44la-net-tools-1.60_p20170221182432\"";
 
+        let store_path: StorePathRef<'_> =
+            serde_json::from_str(store_path_str_json).expect("valid json");
+
+        assert_eq!(
+            "/nix/store/00bgd045z0d4icpbc2yyz4gx48ak44la-net-tools-1.60_p20170221182432",
+            store_path.to_absolute_path()
+        );
+    }
+
+    #[test]
+    fn deserialize_ref_container() {
+        let str_json = "{\"store_path\":\"/nix/store/00bgd045z0d4icpbc2yyz4gx48ak44la-net-tools-1.60_p20170221182432\"}";
+
+        let container: Container<'_> = serde_json::from_str(str_json).expect("must deserialize");
+
+        assert_eq!(
+            "/nix/store/00bgd045z0d4icpbc2yyz4gx48ak44la-net-tools-1.60_p20170221182432",
+            container.store_path.to_absolute_path()
+        );
+    }
+
+    #[test]
+    fn deserialize_owned() {
+        let store_path_str_json =
+            "\"/nix/store/00bgd045z0d4icpbc2yyz4gx48ak44la-net-tools-1.60_p20170221182432\"";
+
         let store_path: StorePath = serde_json::from_str(store_path_str_json).expect("valid json");
 
         assert_eq!(