Repository: web3infra-foundation/mega (language: rust)

Issue:
Some constraints in mercury::internal are incorrect
For example, in `mercury::internal::blob`:
``` Rust
pub struct Blob {
    pub id: SHA1,
    pub data: Vec<u8>,
}

impl PartialEq for Blob {
    /// The Blob object is equal to another Blob object if their IDs are equal.
    fn eq(&self, other: &Self) -> bool {
        self.data == other.data
    }
}
```
Technically it is correct but inefficient, and it disagrees with the doc.
`mercory::internal::commit`:
``` Rust
pub struct Commit {
    pub id: SHA1,
    pub tree_id: SHA1,
    pub parent_commit_ids: Vec<SHA1>,
    pub author: Signature,
    pub committer: Signature,
    pub message: String,
}
impl PartialEq for Commit {
    fn eq(&self, other: &Self) -> bool {
        self.tree_id == other.tree_id
    }
}
```
This is incorrect. `git commit --allow-empty` could create a new commit with the same tree id, while their commit id(hash) differ.



Discussion / hints:
Help needed here. I'm not sure if it's a problem with my understanding or if the relevant code needs to be changed. @genedna 
This appears to be quite old legacy code, so it’s hard to determine the original author’s exact intent. 

However, from the current perspective, using `id` (hash) for comparison would indeed be more efficient and logical.

To minimize the potential impact of this legacy code, it might be necessary to review and verify all the affected usages.
For the commit’s eq function, I believe this is indeed a bug. It is correct that a commit with the --allow-empty option creates a new commit with the same tree ID.

Regarding the blob, it appears to be legacy code. Since the Blob member is pub, I’m not entirely sure if there might be cases where a Blob hasn’t had its ID set yet but still attempts to use eq.
Thanks. I'll look into this.
Yes, I agree. @Hou-Xiaoxuan 

Fix patch:
diff --git a/mercury/delta/src/encode/mod.rs b/mercury/delta/src/encode/mod.rs
index 3f40fa22..545c1a0d 100644
--- a/mercury/delta/src/encode/mod.rs
+++ b/mercury/delta/src/encode/mod.rs
@@ -5,7 +5,7 @@ use diffs::Diff;
 const DATA_INS_LEN: usize = 0x7f;
 const VAR_INT_ENCODING_BITS: u8 = 7;
 
-#[derive(Debug, Clone, Copy, PartialEq)]
+#[derive(Debug, Clone, Copy, PartialEq, Eq)]
 enum Optype {
     Data,
     Copy,
@@ -17,6 +17,7 @@ struct DeltaOp {
     begin: usize,
     len: usize,
 }
+
 #[derive(Debug)]
 pub struct DeltaDiff<'a> {
     ops: Vec<DeltaOp>,
diff --git a/mercury/delta/src/errors.rs b/mercury/delta/src/errors.rs
index 4bb7b57d..0e7e8e27 100644
--- a/mercury/delta/src/errors.rs
+++ b/mercury/delta/src/errors.rs
@@ -1,7 +1,6 @@
 use thiserror::Error;
 
 #[derive(Error, Debug)]
-#[allow(unused)]
 pub enum GitDeltaError{
     #[error("The `{0}` is not a valid git object type.")]
     DeltaEncoderError(String),
diff --git a/mercury/src/errors.rs b/mercury/src/errors.rs
index cfb8d1c8..98ecb4cd 100644
--- a/mercury/src/errors.rs
+++ b/mercury/src/errors.rs
@@ -4,7 +4,6 @@ use std::string::FromUtf8Error;
 use thiserror::Error;
 
 #[derive(Error, Debug)]
-#[allow(unused)]
 pub enum GitError {
     #[error("The `{0}` is not a valid git object type.")]
     InvalidObjectType(String),
diff --git a/mercury/src/hash.rs b/mercury/src/hash.rs
index 402754ea..2c17ecf4 100644
--- a/mercury/src/hash.rs
+++ b/mercury/src/hash.rs
@@ -26,7 +26,6 @@ use crate::internal::object::types::ObjectType;
 /// allows for easier adaptation to different hash algorithms while keeping the underlying implementation consistent and
 /// understandable. - Nov 26, 2023 (by @genedna)
 ///
-#[allow(unused)]
 #[derive(
     Clone, Copy, Debug, PartialEq, Eq, Hash, PartialOrd, Ord, Default, Deserialize, Serialize,
 )]
diff --git a/mercury/src/internal/object/blob.rs b/mercury/src/internal/object/blob.rs
index 73f37d06..7aa6dd4a 100644
--- a/mercury/src/internal/object/blob.rs
+++ b/mercury/src/internal/object/blob.rs
@@ -35,8 +35,6 @@ use crate::internal::object::types::ObjectType;
 use crate::internal::object::ObjectTrait;
 
 /// **The Blob Object**
-///
-#[allow(unused)]
 #[derive(Eq, Debug, Clone)]
 pub struct Blob {
     pub id: SHA1,
@@ -46,7 +44,7 @@ pub struct Blob {
 impl PartialEq for Blob {
     /// The Blob object is equal to another Blob object if their IDs are equal.
     fn eq(&self, other: &Self) -> bool {
-        self.data == other.data
+        self.id == other.id
     }
 }
 
diff --git a/mercury/src/internal/object/commit.rs b/mercury/src/internal/object/commit.rs
index 3032b7b2..b299c6a8 100644
--- a/mercury/src/internal/object/commit.rs
+++ b/mercury/src/internal/object/commit.rs
@@ -15,8 +15,8 @@ use std::fmt::Display;
 use std::str::FromStr;
 
 use bstr::ByteSlice;
-use serde::{Deserialize,Serialize};
-
+use serde::Deserialize;
+use serde::Serialize;
 use crate::errors::GitError;
 use crate::hash::SHA1;
 use crate::internal::object::signature::Signature;
@@ -33,8 +33,7 @@ use crate::internal::object::ObjectType;
 ///   history of a repository with a single commit object at its root.
 /// - The author and committer fields contain the name, email address, timestamp and timezone.
 /// - The message field contains the commit message, which maybe include signed or DCO.
-#[allow(unused)]
-#[derive(Eq, Debug, Clone,Serialize,Deserialize)]
+#[derive(Eq, Debug, Clone, Serialize, Deserialize)]
 pub struct Commit {
     pub id: SHA1,
     pub tree_id: SHA1,
@@ -45,7 +44,7 @@ pub struct Commit {
 }
 impl PartialEq for Commit {
     fn eq(&self, other: &Self) -> bool {
-        self.tree_id == other.tree_id
+        self.id == other.id
     }
 }
 
diff --git a/mercury/src/internal/object/signature.rs b/mercury/src/internal/object/signature.rs
index 06cfb0d4..c7ba8651 100644
--- a/mercury/src/internal/object/signature.rs
+++ b/mercury/src/internal/object/signature.rs
@@ -30,7 +30,7 @@ use crate::errors::GitError;
 /// ```
 ///
 /// So, we design a `SignatureType` enum to indicate the signature type.
-#[derive(PartialEq, Eq, Debug, Hash, Ord, PartialOrd, Clone, Serialize,Deserialize)]
+#[derive(PartialEq, Eq, Debug, Clone, Serialize, Deserialize)]
 pub enum SignatureType {
     Author,
     Committer,
@@ -60,14 +60,12 @@ impl FromStr for SignatureType {
 }
 impl SignatureType {
     /// The `from_data` method is used to convert a `Vec<u8>` to a `SignatureType` enum.
-    #[allow(unused)]
     pub fn from_data(data: Vec<u8>) -> Result<Self, GitError> {
         let s = String::from_utf8(data.to_vec())?;
         SignatureType::from_str(s.as_str())
     }
 
     /// The `to_bytes` method is used to convert a `SignatureType` enum to a `Vec<u8>`.
-    #[allow(unused)]
     pub fn to_bytes(&self) -> Vec<u8> {
         match self {
             SignatureType::Author => "author".to_string().into_bytes(),
@@ -77,8 +75,7 @@ impl SignatureType {
     }
 }
 
-#[allow(unused)]
-#[derive(PartialEq, Eq, Debug, Hash, Ord, PartialOrd, Clone,Serialize,Deserialize)]
+#[derive(PartialEq, Eq, Debug, Clone, Serialize, Deserialize)]
 pub struct Signature {
     pub signature_type: SignatureType,
     pub name: String,
@@ -154,7 +151,6 @@ impl Signature {
         })
     }
 
-    #[allow(unused)]
     pub fn to_data(&self) -> Result<Vec<u8>, GitError> {
         // Create a new empty vector to store the encoded data.
         let mut sign = Vec::new();
diff --git a/mercury/src/internal/object/tag.rs b/mercury/src/internal/object/tag.rs
index 8eb8d633..12a80a57 100644
--- a/mercury/src/internal/object/tag.rs
+++ b/mercury/src/internal/object/tag.rs
@@ -48,7 +48,7 @@ use crate::internal::object::ObjectTrait;
 use crate::internal::object::ObjectType;
 
 /// The tag object is used to Annotated tag
-#[derive(PartialEq, Eq, Debug, Clone)]
+#[derive(Eq, Debug, Clone)]
 pub struct Tag {
     pub id: SHA1,
     pub object_hash: SHA1,
@@ -58,6 +58,12 @@ pub struct Tag {
     pub message: String,
 }
 
+impl PartialEq for Tag {
+    fn eq(&self, other: &Self) -> bool {
+        self.id == other.id
+    }
+}
+
 impl Display for Tag {
     fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
         write!(
diff --git a/mercury/src/internal/object/tree.rs b/mercury/src/internal/object/tree.rs
index d74c9cc4..3f947320 100644
--- a/mercury/src/internal/object/tree.rs
+++ b/mercury/src/internal/object/tree.rs
@@ -15,7 +15,6 @@
 //! operations like merging and rebasing more quickly and accurately.
 //!
 use std::fmt::Display;
-use std::hash::Hash;
 use colored::Colorize;
 use serde::Deserialize;
 use serde::Serialize;
@@ -29,8 +28,7 @@ use crate::internal::object::ObjectType;
 /// that entry. The mode is a three-digit octal number that encodes both the permissions and the
 /// type of the object. The first digit specifies the object type, and the remaining two digits
 /// specify the file mode or permissions.
-#[allow(unused)]
-#[derive(PartialEq, Eq, Hash, Ord, PartialOrd, Debug, Clone, Copy,Serialize,Deserialize)]
+#[derive(PartialEq, Eq, Debug, Clone, Copy, Serialize, Deserialize)]
 pub enum TreeItemMode {
     Blob,
     BlobExecutable,
@@ -81,7 +79,6 @@ impl TreeItemMode {
     /// Submodules can be a powerful tool for managing dependencies between different projects and
     /// components. However, they can also add complexity to your workflow, so it's important to
     /// understand how they work and when to use them.
-    #[allow(unused)]
     pub fn tree_item_type_from_bytes(mode: &[u8]) -> Result<TreeItemMode, GitError> {
         Ok(match mode {
             b"40000" => TreeItemMode::Tree,
@@ -103,7 +100,6 @@ impl TreeItemMode {
     /// - 4-bit object type: valid values in binary are 1000 (regular file), 1010 (symbolic link) and 1110 (gitlink)
     /// - 3-bit unused
     /// - 9-bit unix permission: Only 0755 and 0644 are valid for regular files. Symbolic links and gitlink have value 0 in this field.
-    #[allow(unused)]
     pub fn to_bytes(self) -> &'static [u8] {
         match self {
             TreeItemMode::Blob => b"100644",
@@ -133,8 +129,7 @@ impl TreeItemMode {
 /// 100644 hello-world\0<blob object ID>
 /// 040000 data\0<tree object ID>
 /// ```
-#[allow(unused)]
-#[derive(PartialEq, Eq, Debug, Hash, Ord, PartialOrd, Clone,Serialize,Deserialize)]
+#[derive(PartialEq, Eq, Debug, Clone, Serialize, Deserialize)]
 pub struct TreeItem {
     pub mode: TreeItemMode,
     pub id: SHA1,
@@ -170,7 +165,6 @@ impl TreeItem {
     /// // Create a tree TreeItem with a custom Hash, and directory name
     /// let dir_item = TreeItem::new(TreeItemMode::Tree, SHA1::new_from_str("1234567890abcdef1234567890abcdef12345678"), String::from("data"));
     /// ```
-    #[allow(unused)]
     pub fn new(mode: TreeItemMode, id: SHA1, name: String) -> Self {
         TreeItem { mode, id, name }
     }
@@ -210,7 +204,6 @@ impl TreeItem {
     ///
     //  let bytes = tree_item.to_bytes();
     /// ```
-    #[allow(unused)]
     pub fn to_data(&self) -> Vec<u8> {
         let mut bytes = Vec::new();
 
@@ -226,19 +219,24 @@ impl TreeItem {
 
 /// A tree object is a Git object that represents a directory. It contains a list of entries, one
 /// for each file or directory in the tree.
-#[derive(PartialEq, Eq, Debug, Hash, Ord, PartialOrd, Clone,Serialize,Deserialize)]
+#[derive(Eq, Debug, Clone, Serialize, Deserialize)]
 pub struct Tree {
     pub id: SHA1,
     pub tree_items: Vec<TreeItem>,
 }
 
+impl PartialEq for Tree {
+    fn eq(&self, other: &Self) -> bool {
+        self.id == other.id
+    }
+}
+
 impl Display for Tree {
     fn fmt(&self, f: &mut std::fmt::Formatter) -> std::fmt::Result {
         writeln!(f, "Tree: {}", self.id.to_string().blue())?;
         for item in &self.tree_items {
             writeln!(f, "{}", item)?;
         }
-
         Ok(())
     }
 }
@@ -272,6 +270,7 @@ impl Tree {
         self.id = SHA1::from_type_and_data(ObjectType::Tree, &data);
     }
 }
+
 impl TryFrom<&[u8]> for Tree{
     type Error = GitError;
     fn try_from(data: &[u8]) -> Result<Self, Self::Error>  {
diff --git a/mercury/src/internal/object/types.rs b/mercury/src/internal/object/types.rs
index 0d2b7327..0c22b770 100644
--- a/mercury/src/internal/object/types.rs
+++ b/mercury/src/internal/object/types.rs
@@ -22,7 +22,7 @@ use crate::errors::GitError;
 /// identify the type of an object and perform the appropriate operations on it. when parsing a Git
 /// repository, Git can use the integer value of an object's type to determine how to parse
 /// the object's content.
-#[derive(PartialEq, Eq, Hash, Ord, PartialOrd, Debug, Clone, Copy, Serialize, Deserialize)]
+#[derive(PartialEq, Eq, Hash, Debug, Clone, Copy, Serialize, Deserialize)]
 pub enum ObjectType {
     Commit,
     Tree,
diff --git a/mercury/src/internal/pack/entry.rs b/mercury/src/internal/pack/entry.rs
index 416ebaf6..19fb5b4f 100644
--- a/mercury/src/internal/pack/entry.rs
+++ b/mercury/src/internal/pack/entry.rs
@@ -13,7 +13,7 @@ use crate::internal::object::{GitObject, ObjectTrait};
 ///
 /// Git object data from pack file
 ///
-#[derive(Clone, Debug, Serialize, Deserialize)]
+#[derive(Eq, Clone, Debug, Serialize, Deserialize)]
 pub struct Entry {
     pub obj_type: ObjectType,
     pub data: Vec<u8>,
@@ -26,8 +26,6 @@ impl PartialEq for Entry {
     }
 }
 
-impl Eq for Entry {}
-
 impl Hash for Entry {
     fn hash<H: Hasher>(&self, state: &mut H) {
         self.obj_type.hash(state);
diff --git a/mercury/src/internal/pack/utils.rs b/mercury/src/internal/pack/utils.rs
index a0865303..04b586dc 100644
--- a/mercury/src/internal/pack/utils.rs
+++ b/mercury/src/internal/pack/utils.rs
@@ -24,7 +24,6 @@ use crate::internal::object::types::ObjectType;
 /// # Returns
 ///
 /// true if the reader reached EOF, false otherwise
-#[allow(unused)]
 pub fn is_eof(reader: &mut dyn Read) -> bool {
     let mut buf = [0; 1];
     matches!(reader.read(&mut buf), Ok(0))
@@ -42,7 +41,6 @@ pub fn is_eof(reader: &mut dyn Read) -> bool {
 /// Returns an `io::Result` containing a tuple. The first element is the value of the first 7 bits,
 /// and the second element is a boolean indicating whether more bytes need to be read.
 ///
-#[allow(unused)]
 pub fn read_byte_and_check_continuation<R: Read>(stream: &mut R) -> io::Result<(u8, bool)> {
     // Create a buffer for a single byte
     let mut bytes = [0; 1];
@@ -74,7 +72,6 @@ pub fn read_byte_and_check_continuation<R: Read>(stream: &mut R) -> io::Result<(
 /// # Returns
 /// Returns an `io::Result` containing a tuple of the type and the computed size.
 ///
-#[allow(unused)]
 pub fn read_type_and_varint_size<R: Read>(stream: &mut R, offset: &mut usize) -> io::Result<(u8, usize)> {
     let (first_byte, continuation) = read_byte_and_check_continuation(stream)?;
 
@@ -116,7 +113,6 @@ pub fn read_type_and_varint_size<R: Read>(stream: &mut R, offset: &mut usize) ->
 /// * A tuple of the decoded `u64` value and the number of bytes read (`offset`).
 /// * An `io::Error` in case of any reading error or if the VarInt is too long.
 ///
-#[allow(unused)]
 pub fn read_varint_le<R: Read>(reader: &mut R) -> io::Result<(u64, usize)> {
     // The decoded value
     let mut value: u64 = 0;


Test patch: