Git.check_version() can return incorrect results

[cjerdonek]

While working on PR #4674 for issue #4507, I noticed that the Git class's check_version() method can return incorrect results, leading to pip install -e git+git://... not updating its clone in certain (albeit rare) cases when it needs to.

Specifically, Git.check_version(dest, rev_options) isn't supposed to return True if given a branch or tag. For example, its docstring says:

Compare the current sha to the ref. ref may be a branch or tag name ... This means that a branch or tag will never compare as True. So this ultimately only matches against exact shas.

However, with the way it's currently written, the method can return True on a branch or tag since it does prefix matching instead of checking for an exact match. For example, given a repo with HEAD:

10d02d33e110f0b18e628ab8cab028a0a219773c

if you pass the tag name 10, then check_version('/path', '10') will return True even though 10 isn't a commit hash. This can prevent pip from checking the remote repo for new tags / commits / etc.

Related to this, I also noticed that test_git_check_version() has a bug because it is incorrectly passing a ref in each call to check_version() instead of a list containing the ref.

This means the following test case would incorrectly pass when it shouldn't (the other cases are "accidentally" working):

@pytest.mark.parametrize('ref,result', (
    ('5547fa909e83df8bd743d3978d6667497983a4b7', True),
    ('5547fa909', True),
    ('5678', True),  # This would incorrectly pass since it uses "5" as the ref.
    ('abc123', False),
    ('foo', False),
))
def test_git_check_version(git, ref, result):
    assert git.check_version('foo', ref) is result

I posted a fix for both of these issues in PR #4674, which I'm going to break into smaller PR's. In that PR, I also simplified the API for check_version() to make it harder to use incorrectly, like in both of the cases above.

[cjerdonek]

I posted PR #4676 to address this.

[cjerdonek]

PR #4676 is now ready for review.

[xavfernandez]

Closed by #4676