refactor(nix-compat/store_path): use AsRef<str>

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 <edef@edef.eu>
Autosubmit: flokli <flokli@flokli.de>

tvix/nix-compat/src/derivation/parser.rs

@@ -203,11 +203,7 @@ fn string_to_store_path<'a, 'i, S>(
     path_str: &'a str,
 ) -> Result<StorePath<S>, nom::Err<NomError<&'i [u8]>>>
 where
-    S: std::cmp::Eq
-        + std::fmt::Display
-        + std::clone::Clone
-        + std::ops::Deref<Target = str>
-        + std::convert::From<&'a str>,
+    S: std::clone::Clone + AsRef<str> + std::convert::From<&'a str>,
 {
     let path =
         StorePath::from_absolute_path(path_str.as_bytes()).map_err(|e: store_path::Error| {

tvix/nix-compat/src/derivation/write.rs

@@ -36,14 +36,14 @@ pub(crate) trait AtermWriteable {
 
 impl<S> AtermWriteable for StorePath<S>
 where
-    S: std::cmp::Eq + std::ops::Deref<Target = str>,
+    S: AsRef<str>,
 {
     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(())
     }

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<String> {
 // Custom implementation since Display does not use absolute paths.
 impl<S> NixSerialize for StorePath<S>
 where
-    S: std::cmp::Eq + Deref<Target = str> + Display + Sync,
+    S: AsRef<str> + Sync,
 {
     async fn serialize<W>(&self, writer: &mut W) -> Result<(), W::Error>
     where

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<S>
-where
-    S: std::cmp::Eq + std::cmp::PartialEq,
-{
+#[derive(Clone, Debug)]
+pub struct StorePath<S> {
     digest: [u8; DIGEST_SIZE],
     name: S,
 }
+
+impl<S> PartialEq for StorePath<S>
+where
+    S: AsRef<str>,
+{
+    fn eq(&self, other: &Self) -> bool {
+        self.digest() == other.digest() && self.name().as_ref() == other.name().as_ref()
+    }
+}
+
+impl<S> Eq for StorePath<S> where S: AsRef<str> {}
+
+impl<S> std::hash::Hash for StorePath<S>
+where
+    S: AsRef<str>,
+{
+    fn hash<H: std::hash::Hasher>(&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<S> StorePath<S>
 where
-    S: std::cmp::Eq + Deref<Target = str>,
+    S: AsRef<str>,
 {
     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<String> {
         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<S> PartialOrd for StorePath<S>
 where
-    S: std::cmp::PartialEq + std::cmp::Eq,
+    S: AsRef<str>,
 {
     fn partial_cmp(&self, other: &Self) -> Option<std::cmp::Ordering> {
         Some(self.cmp(other))
@@ -204,7 +219,7 @@ where
 /// of the nixbase32-encoded string.
 impl<S> Ord for StorePath<S>
 where
-    S: std::cmp::PartialEq + std::cmp::Eq,
+    S: AsRef<str>,
 {
     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<String> {
 
 impl<'a, 'de: 'a, S> Deserialize<'de> for StorePath<S>
 where
-    S: std::cmp::Eq + Deref<Target = str> + From<&'a str>,
+    S: AsRef<str> + From<&'a str>,
 {
     fn deserialize<D>(deserializer: D) -> Result<Self, D::Error>
     where
@@ -245,7 +260,7 @@ where
 
 impl<S> Serialize for StorePath<S>
 where
-    S: std::cmp::Eq + Deref<Target = str> + Display,
+    S: AsRef<str>,
 {
     fn serialize<SR>(&self, serializer: SR) -> Result<SR::Ok, SR::Error>
     where
@@ -305,13 +320,18 @@ pub(crate) fn validate_name(s: &(impl AsRef<[u8]> + ?Sized)) -> Result<&str, Err
 
 impl<S> fmt::Display for StorePath<S>
 where
-    S: fmt::Display + std::cmp::Eq,
+    S: AsRef<str>,
 {
     /// 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()
+        )
     }
 }
 

tvix/nix-compat/src/store_path/utils.rs

@@ -50,7 +50,7 @@ pub fn build_text_path<'a, S, SP, I, C>(
 ) -> Result<StorePath<SP>, BuildStorePathError>
 where
     S: AsRef<str>,
-    SP: std::cmp::Eq + std::ops::Deref<Target = str> + std::convert::From<&'a str>,
+    SP: AsRef<str> + std::convert::From<&'a str>,
     I: IntoIterator<Item = S>,
     C: AsRef<[u8]>,
 {
@@ -69,7 +69,7 @@ pub fn build_ca_path<'a, S, SP, I>(
 ) -> Result<StorePath<SP>, BuildStorePathError>
 where
     S: AsRef<str>,
-    SP: std::cmp::Eq + std::ops::Deref<Target = str> + std::convert::From<&'a str>,
+    SP: AsRef<str> + std::convert::From<&'a str>,
     I: IntoIterator<Item = S>,
 {
     // 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<StorePath<SP>, Error>
 where
-    SP: std::cmp::Eq + std::ops::Deref<Target = str> + std::convert::From<&'a str>,
+    SP: AsRef<str> + 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<StorePath<SP>, Error>
 where
-    SP: std::cmp::Eq + std::ops::Deref<Target = str> + std::convert::From<&'a str>,
+    SP: AsRef<str> + std::convert::From<&'a str>,
 {
     let fingerprint = format!(
         "{ty}:sha256:{}:{STORE_DIR}:{name}",