Should `Reference` implement `PartialEq`/`Eq`?
Nobody has claimed this yet.
- Dominant language
- Rust
- Stars
- 2.1k
- Forks
- 450
- Avg merge
- 11m
- Merged PRs (30d)
- 1
Description
Currently, Reference has an implementation for Eq (and PartialEq) that uses git_reference_cmp under the hood to determine if two references are "equal".
However, git_reference_cmp may return 0 ("yes they're equal") when comparing two symbolic references that have different names but the same target. See libgit2/libgit2#6337 for further details, and refs.c:1069 for the current implementation of git_reference_cmp, effectively comparing the targets of symbolic references instead of the references themselves.
This leads to the following behaviour:
# create the refs (they can also be created using git2, but using the git CLI here for simplicity)
git symbolic-ref refs/foo refs/heads/master
git symbolic-ref refs/bar refs/heads/master
let foo = repo.find_reference("refs/foo")?;
let bar = repo.find_reference("refs/bar")?;
assert_ne!(foo.name(), bar.name()); // ok, "refs/foo" != "refs/bar" (disregarding the `Option` here)
assert_ne!(foo, bar); // assertion fails! despite being *different* references, they share the same target
While it can make sense in some cases to have git_reference_cmp behave the way it does, I'm not sure if this is expected for an implementation of PartialEq, and especially so for an implementation of Eq. It's easy to overlook, especially since I couldn't find any mention of this in the documentation (of both git2-rs or libgit2).
Maybe git2-rs shouldn't translate git_reference_cmp to a PartialEq/Eq implementation? Maybe libgit2 should change the behaviour of git_reference_cmp? Maybe nothing should change and the documentation should just make this more clear? I'm not too sure what's best here, but this has definitely confused me more than once.
Contributor guide
First steps
- Read the whole issue, then the project's contributing guide.
- Comment on the issue to say you are picking it up — it saves two people doing the same work.
- Fork the repository and make your change on a branch.
- Open a pull request that references the issue number.
Research direction
Start by reviewing the Reference PartialEq/Eq implementation and libgit2's refs.c:1069 behavior, then compare the available documentation for reference equality. A complete resolution needs a decided equality contract, corresponding tests for differently named symbolic references, and documentation if the existing behavior remains.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- rust
- Domain
- backend-api-design
- Issue type
- Bug
- Difficulty
- 5/5
- Estimated time
- Over a week
- Activity status
- Stale
- Clarity
- Needs clarification
- Newbie friendliness
- 25/100