Some constraints in mercury::internal are incorrect

[el-ev]

For example, in mercury::internal::blob:

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:

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.

[el-ev]

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

[MrBeanCpp]

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.

[Hou-Xiaoxuan]

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.

[el-ev]

Thanks. I'll look into this.

[MrBeanCpp]

Yes, I agree. @Hou-Xiaoxuan

[Ivanbeethoven]

I was responsible for this part of the code before. Thank you to the issue proposer @el-ev and others discussion. I think your views are basically correct.
But further discussion may introduce other more complex issues.
The reason for providing them with PartialEq and Ord traits is because various git objects are sorted during the pack file encoding process, but the current version may not rely on these traits anymore (I did not find any calls that require equality judgment for different objects after preliminary inspection).
As for why sorting is necessary, here is a brief explanation: in order to make the encoded files smaller (without storing all the full information of objects in one pack), we need to decide that some files need to be easily added, deleted, and modified through other files, which is called delta object in pack file. This dependency relationship can lead to an NP-problem similar to DAG structure. The author of Git proposed a heuristic algorithm, which simply sorts objects by type, name, and length, selects the most similar ones through a sliding window, pairs them, calculates delta, and encodes them.

[Ivanbeethoven]

This is related to a depth sub command in the git clone command.For example,

git clone [url] depth=1

If you are interested, you may take a look here. My previous implementation has been temporarily put on hold due to poor performance，And mega may adopt a more aggressive mandatory cloning depth of 1 to avoid this issue.. https://github.com/git/git/blob/master/Documentation/technical/pack-heuristics.txt