From d00c75ea9cd2d31d6e0f02fa678678519317182c Mon Sep 17 00:00:00 2001 From: Florian Klink Date: Sun, 7 Apr 2024 23:40:38 +0300 Subject: fix(nix-compat/store_path): fix Deserialize with borrow 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 Reviewed-by: picnoir picnoir --- tvix/nix-compat/src/store_path/mod.rs | 40 ++++++++++++++++++++++++++++++++--- 1 file changed, 37 insertions(+), 3 deletions(-) (limited to 'tvix/nix-compat') 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(deserializer: D) -> Result 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!( -- cgit 1.4.1