From 1428ea4e191aa897a0a23e2c90f997f0b65aae6d Mon Sep 17 00:00:00 2001 From: Florian Klink Date: Wed, 16 Oct 2024 02:51:35 +0300 Subject: refactor(nix-compat/store_path): use AsRef Implement PartialEq/Eq ourselves instead of deriving, by proxying to name.as_ref() (and digest of course). Also implement Hash on our own, clippy doesn't like this to be derived, while Eq/PartialEq is not. Change-Id: Idbe289a23ba3bc8dabf893d4d8752792ae2778c3 Reviewed-on: https://cl.tvl.fyi/c/depot/+/12744 Tested-by: BuildkiteCI Reviewed-by: edef Autosubmit: flokli --- tvix/nix-compat/src/derivation/parser.rs | 6 +--- tvix/nix-compat/src/derivation/write.rs | 4 +-- tvix/nix-compat/src/nix_daemon/types.rs | 4 +-- tvix/nix-compat/src/store_path/mod.rs | 60 +++++++++++++++++++++----------- tvix/nix-compat/src/store_path/utils.rs | 8 ++--- 5 files changed, 48 insertions(+), 34 deletions(-) diff --git a/tvix/nix-compat/src/derivation/parser.rs b/tvix/nix-compat/src/derivation/parser.rs index 8e9804157da3..a94ed2281a86 100644 --- a/tvix/nix-compat/src/derivation/parser.rs +++ b/tvix/nix-compat/src/derivation/parser.rs @@ -203,11 +203,7 @@ fn string_to_store_path<'a, 'i, S>( path_str: &'a str, ) -> Result, nom::Err>> where - S: std::cmp::Eq - + std::fmt::Display - + std::clone::Clone - + std::ops::Deref - + std::convert::From<&'a str>, + S: std::clone::Clone + AsRef + std::convert::From<&'a str>, { let path = StorePath::from_absolute_path(path_str.as_bytes()).map_err(|e: store_path::Error| { diff --git a/tvix/nix-compat/src/derivation/write.rs b/tvix/nix-compat/src/derivation/write.rs index 42dadcd76064..a8b43fad4cc6 100644 --- a/tvix/nix-compat/src/derivation/write.rs +++ b/tvix/nix-compat/src/derivation/write.rs @@ -36,14 +36,14 @@ pub(crate) trait AtermWriteable { impl AtermWriteable for StorePath where - S: std::cmp::Eq + std::ops::Deref, + S: AsRef, { fn aterm_write(&self, writer: &mut impl Write) -> std::io::Result<()> { write_char(writer, QUOTE)?; writer.write_all(STORE_DIR_WITH_SLASH.as_bytes())?; writer.write_all(nixbase32::encode(self.digest()).as_bytes())?; write_char(writer, '-')?; - writer.write_all(self.name().as_bytes())?; + writer.write_all(self.name().as_ref().as_bytes())?; write_char(writer, QUOTE)?; Ok(()) } diff --git a/tvix/nix-compat/src/nix_daemon/types.rs b/tvix/nix-compat/src/nix_daemon/types.rs index 3f2490c62657..33b74f37574f 100644 --- a/tvix/nix-compat/src/nix_daemon/types.rs +++ b/tvix/nix-compat/src/nix_daemon/types.rs @@ -1,5 +1,3 @@ -use std::{fmt::Display, ops::Deref}; - use nix_compat_derive::{NixDeserialize, NixSerialize}; use crate::{ @@ -130,7 +128,7 @@ impl NixDeserialize for StorePath { // Custom implementation since Display does not use absolute paths. impl NixSerialize for StorePath where - S: std::cmp::Eq + Deref + Display + Sync, + S: AsRef + Sync, { async fn serialize(&self, writer: &mut W) -> Result<(), W::Error> where diff --git a/tvix/nix-compat/src/store_path/mod.rs b/tvix/nix-compat/src/store_path/mod.rs index 56c10dc41417..13265048641b 100644 --- a/tvix/nix-compat/src/store_path/mod.rs +++ b/tvix/nix-compat/src/store_path/mod.rs @@ -2,8 +2,7 @@ use crate::nixbase32; use data_encoding::{DecodeError, BASE64}; use serde::{Deserialize, Serialize}; use std::{ - fmt::{self, Display}, - ops::Deref, + fmt, path::Path, str::{self, FromStr}, }; @@ -51,21 +50,40 @@ pub enum Error { /// /// A [StorePath] does not encode any additional subpath "inside" the store /// path. -#[derive(Clone, Debug, PartialEq, Eq, Hash)] -pub struct StorePath -where - S: std::cmp::Eq + std::cmp::PartialEq, -{ +#[derive(Clone, Debug)] +pub struct StorePath { digest: [u8; DIGEST_SIZE], name: S, } + +impl PartialEq for StorePath +where + S: AsRef, +{ + fn eq(&self, other: &Self) -> bool { + self.digest() == other.digest() && self.name().as_ref() == other.name().as_ref() + } +} + +impl Eq for StorePath where S: AsRef {} + +impl std::hash::Hash for StorePath +where + S: AsRef, +{ + fn hash(&self, state: &mut H) { + state.write(&self.digest); + state.write(self.name.as_ref().as_bytes()); + } +} + /// Like [StorePath], but without a heap allocation for the name. /// Used by [StorePath] for parsing. pub type StorePathRef<'a> = StorePath<&'a str>; impl StorePath where - S: std::cmp::Eq + Deref, + S: AsRef, { pub fn digest(&self) -> &[u8; DIGEST_SIZE] { &self.digest @@ -78,14 +96,14 @@ where pub fn as_ref(&self) -> StorePathRef<'_> { StorePathRef { digest: self.digest, - name: &self.name, + name: self.name.as_ref(), } } pub fn to_owned(&self) -> StorePath { StorePath { digest: self.digest, - name: self.name.to_string(), + name: self.name.as_ref().to_string(), } } @@ -183,17 +201,14 @@ where /// Returns an absolute store path string. /// That is just the string representation, prefixed with the store prefix /// ([STORE_DIR_WITH_SLASH]), - pub fn to_absolute_path(&self) -> String - where - S: Display, - { + pub fn to_absolute_path(&self) -> String { format!("{}{}", STORE_DIR_WITH_SLASH, self) } } impl PartialOrd for StorePath where - S: std::cmp::PartialEq + std::cmp::Eq, + S: AsRef, { fn partial_cmp(&self, other: &Self) -> Option { Some(self.cmp(other)) @@ -204,7 +219,7 @@ where /// of the nixbase32-encoded string. impl Ord for StorePath where - S: std::cmp::PartialEq + std::cmp::Eq, + S: AsRef, { fn cmp(&self, other: &Self) -> std::cmp::Ordering { self.digest.iter().rev().cmp(other.digest.iter().rev()) @@ -223,7 +238,7 @@ impl<'a, 'b: 'a> FromStr for StorePath { impl<'a, 'de: 'a, S> Deserialize<'de> for StorePath where - S: std::cmp::Eq + Deref + From<&'a str>, + S: AsRef + From<&'a str>, { fn deserialize(deserializer: D) -> Result where @@ -245,7 +260,7 @@ where impl Serialize for StorePath where - S: std::cmp::Eq + Deref + Display, + S: AsRef, { fn serialize(&self, serializer: SR) -> Result where @@ -305,13 +320,18 @@ pub(crate) fn validate_name(s: &(impl AsRef<[u8]> + ?Sized)) -> Result<&str, Err impl fmt::Display for StorePath where - S: fmt::Display + std::cmp::Eq, + S: AsRef, { /// The string representation of a store path starts with a digest (20 /// bytes), [crate::nixbase32]-encoded, followed by a `-`, /// and ends with the name. fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { - write!(f, "{}-{}", nixbase32::encode(&self.digest), self.name) + write!( + f, + "{}-{}", + nixbase32::encode(&self.digest), + self.name.as_ref() + ) } } diff --git a/tvix/nix-compat/src/store_path/utils.rs b/tvix/nix-compat/src/store_path/utils.rs index a0dc21160e7a..63b6969464d8 100644 --- a/tvix/nix-compat/src/store_path/utils.rs +++ b/tvix/nix-compat/src/store_path/utils.rs @@ -50,7 +50,7 @@ pub fn build_text_path<'a, S, SP, I, C>( ) -> Result, BuildStorePathError> where S: AsRef, - SP: std::cmp::Eq + std::ops::Deref + std::convert::From<&'a str>, + SP: AsRef + std::convert::From<&'a str>, I: IntoIterator, C: AsRef<[u8]>, { @@ -69,7 +69,7 @@ pub fn build_ca_path<'a, S, SP, I>( ) -> Result, BuildStorePathError> where S: AsRef, - SP: std::cmp::Eq + std::ops::Deref + std::convert::From<&'a str>, + SP: AsRef + std::convert::From<&'a str>, I: IntoIterator, { // self references are only allowed for CAHash::Nar(NixHash::Sha256(_)). @@ -129,7 +129,7 @@ pub fn build_output_path<'a, SP>( output_path_name: &'a str, ) -> Result, Error> where - SP: std::cmp::Eq + std::ops::Deref + std::convert::From<&'a str>, + SP: AsRef + std::convert::From<&'a str>, { build_store_path_from_fingerprint_parts( &(String::from("output:") + output_name), @@ -154,7 +154,7 @@ fn build_store_path_from_fingerprint_parts<'a, SP>( name: &'a str, ) -> Result, Error> where - SP: std::cmp::Eq + std::ops::Deref + std::convert::From<&'a str>, + SP: AsRef + std::convert::From<&'a str>, { let fingerprint = format!( "{ty}:sha256:{}:{STORE_DIR}:{name}", -- cgit 1.4.1