Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

refs: make git_reference_cmp consider the name #6346

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

ethomson
Copy link
Member

@ethomson ethomson commented Jul 8, 2022

git_reference_cmp only considers the target of a reference, and
ignores the name. Meaning that a reference foo and reference bar
pointing to the same commit will compare equal.

Correct this, comparing the name and target of a reference.

This appears to be an oversight, as git_reference_cmp was added as a helper method. Despite that, this is a breaking API change. We'll target libgit2 2.0 for this change.

Fixes #6337

`git_reference_cmp` only considers the target of a reference, and
ignores the name. Meaning that a reference `foo` and reference `bar`
pointing to the same commit will compare equal.

Correct this, comparing the name _and_ target of a reference.

GIT_ASSERT_ARG(ref1);
GIT_ASSERT_ARG(ref2);

if ((ret = strcmp(ref1->name, ref2->name)) != 0)
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why is it necessary to continue inspecting the references if they have identical names? Is it so that "outdated" git_reference values compare as different?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Right, that's the most likely scenario, I would expect. You load a reference, mutate it, and then check an existing reference to the new reference.

@libgit2 libgit2 deleted a comment Jul 17, 2022
@csware
Copy link
Contributor

csware commented Nov 23, 2022

Maybe add git_reference_cmpname or rename git_reference_cmp to git_reference_cmptarget to make this more clear?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

git_reference_cmp returns 0 for *distinct* symbolic references to the same target
3 participants