Support comparison of Oid with text string

Author: jdavid

CHANGELOG.md

@@ -5,6 +5,8 @@
 - New `push_options` optional argument in `Repository.push(...)`
   [#1282](https://github.com/libgit2/pygit2/pull/1282)
 
+- New support comparison of `Oid` with text string
+
 - Remove setuptools runtime dependency
   [#1281](https://github.com/libgit2/pygit2/pull/1281)
 

src/oid.c

@@ -198,19 +198,29 @@ Oid_hash(PyObject *oid)
 
 
 PyObject *
-Oid_richcompare(PyObject *o1, PyObject *o2, int op)
+Oid_richcompare(PyObject *self, PyObject *other, int op)
 {
-    PyObject *res;
+    git_oid *oid = &((Oid*)self)->oid;
     int cmp;
 
-    /* Comparing to something else than an Oid is not supported. */
-    if (!PyObject_TypeCheck(o2, &OidType)) {
+    // Can compare an oid against another oid or a unicode string
+    if (PyObject_TypeCheck(other, &OidType)) {
+        cmp = git_oid_cmp(oid, &((Oid*)other)->oid);
+    }
+    else if (PyObject_TypeCheck(other, &PyUnicode_Type)) {
+        const char * str = PyUnicode_AsUTF8(other);
+        if (str == NULL) {
+            return NULL;
+        }
+        cmp = git_oid_strcmp(oid, str);
+    }
+    else {
         Py_INCREF(Py_NotImplemented);
         return Py_NotImplemented;
     }
 
-    /* Ok go. */
-    cmp = git_oid_cmp(&((Oid*)o1)->oid, &((Oid*)o2)->oid);
+    // Return boolean
+    PyObject *res;
     switch (op) {
         case Py_LT:
             res = (cmp <= 0) ? Py_True: Py_False;

test/test_blob.py

@@ -82,8 +82,8 @@
 
 def test_read_blob(testrepo):
     blob = testrepo[BLOB_SHA]
-    assert str(blob.id) == BLOB_SHA
-    assert str(blob.id) == BLOB_SHA
+    assert blob.id == BLOB_SHA
+    assert blob.id == BLOB_SHA
     assert isinstance(blob, pygit2.Blob)
     assert not blob.is_binary
     assert ObjectType.BLOB == blob.type
@@ -100,7 +100,7 @@ def test_create_blob(testrepo):
     assert ObjectType.BLOB == blob.type
 
     assert blob_oid == blob.id
-    assert utils.gen_blob_sha1(BLOB_NEW_CONTENT) == str(blob_oid)
+    assert utils.gen_blob_sha1(BLOB_NEW_CONTENT) == blob_oid
 
     assert BLOB_NEW_CONTENT == blob.data
     assert len(BLOB_NEW_CONTENT) == blob.size
@@ -124,7 +124,7 @@ def test_create_blob_fromworkdir(testrepo):
     assert ObjectType.BLOB == blob.type
 
     assert blob_oid == blob.id
-    assert utils.gen_blob_sha1(BLOB_FILE_CONTENT) == str(blob_oid)
+    assert utils.gen_blob_sha1(BLOB_FILE_CONTENT) == blob_oid
 
     assert BLOB_FILE_CONTENT == blob.data
     assert len(BLOB_FILE_CONTENT) == blob.size
@@ -163,7 +163,7 @@ def test_create_blob_fromiobase(testrepo):
     assert ObjectType.BLOB == blob.type
 
     assert blob_oid == blob.id
-    assert BLOB_SHA == str(blob_oid)
+    assert BLOB_SHA == blob_oid
 
 
 def test_diff_blob(testrepo):

test/test_branch.py

@@ -40,10 +40,10 @@
 
 def test_branches_getitem(testrepo):
     branch = testrepo.branches['master']
-    assert str(branch.target) == LAST_COMMIT
+    assert branch.target == LAST_COMMIT
 
     branch = testrepo.branches.local['i18n']
-    assert str(branch.target) == I18N_LAST_COMMIT
+    assert branch.target == I18N_LAST_COMMIT
     assert testrepo.branches.get('not-exists') is None
     with pytest.raises(KeyError):
         testrepo.branches['not-exists']
@@ -59,15 +59,15 @@ def test_branches_create(testrepo):
     reference = testrepo.branches.create('version1', commit)
     assert 'version1' in testrepo.branches
     reference = testrepo.branches['version1']
-    assert str(reference.target) == LAST_COMMIT
+    assert reference.target == LAST_COMMIT
 
     # try to create existing reference
     with pytest.raises(ValueError):
         testrepo.branches.create('version1', commit)
 
     # try to create existing reference with force
     reference = testrepo.branches.create('version1', commit, True)
-    assert str(reference.target) == LAST_COMMIT
+    assert reference.target == LAST_COMMIT
 
 
 def test_branches_delete(testrepo):
@@ -92,10 +92,10 @@ def test_branches_is_not_head(testrepo):
 
 def test_branches_rename(testrepo):
     new_branch = testrepo.branches['i18n'].rename('new-branch')
-    assert str(new_branch.target) == I18N_LAST_COMMIT
+    assert new_branch.target == I18N_LAST_COMMIT
 
     new_branch_2 = testrepo.branches.get('new-branch')
-    assert str(new_branch_2.target) == I18N_LAST_COMMIT
+    assert new_branch_2.target == I18N_LAST_COMMIT
 
 
 def test_branches_rename_error(testrepo):
@@ -107,7 +107,7 @@ def test_branches_rename_error(testrepo):
 def test_branches_rename_force(testrepo):
     original_branch = testrepo.branches.get('master')
     new_branch = original_branch.rename('i18n', True)
-    assert str(new_branch.target) == LAST_COMMIT
+    assert new_branch.target == LAST_COMMIT
 
 
 def test_branches_rename_invalid(testrepo):
@@ -153,15 +153,11 @@ def test_branches_with_commit(testrepo):
 
 
 def test_lookup_branch_local(testrepo):
-    branch = testrepo.lookup_branch('master')
-    assert str(branch.target) == LAST_COMMIT
-    branch = testrepo.lookup_branch(b'master')
-    assert str(branch.target) == LAST_COMMIT
+    assert testrepo.lookup_branch('master').target == LAST_COMMIT
+    assert testrepo.lookup_branch(b'master').target == LAST_COMMIT
 
-    branch = testrepo.lookup_branch('i18n', BranchType.LOCAL)
-    assert str(branch.target) == I18N_LAST_COMMIT
-    branch = testrepo.lookup_branch(b'i18n', BranchType.LOCAL)
-    assert str(branch.target) == I18N_LAST_COMMIT
+    assert testrepo.lookup_branch('i18n', BranchType.LOCAL).target == I18N_LAST_COMMIT
+    assert testrepo.lookup_branch(b'i18n', BranchType.LOCAL).target == I18N_LAST_COMMIT
 
     assert testrepo.lookup_branch('not-exists') is None
     assert testrepo.lookup_branch(b'not-exists') is None
@@ -179,21 +175,18 @@ def test_listall_branches(testrepo):
 
 def test_create_branch(testrepo):
     commit = testrepo[LAST_COMMIT]
-    reference = testrepo.create_branch('version1', commit)
+    testrepo.create_branch('version1', commit)
     refs = testrepo.listall_branches()
     assert 'version1' in refs
-    reference = testrepo.lookup_branch('version1')
-    assert str(reference.target) == LAST_COMMIT
-    reference = testrepo.lookup_branch(b'version1')
-    assert str(reference.target) == LAST_COMMIT
+    assert testrepo.lookup_branch('version1').target == LAST_COMMIT
+    assert testrepo.lookup_branch(b'version1').target == LAST_COMMIT
 
     # try to create existing reference
     with pytest.raises(ValueError):
         testrepo.create_branch('version1', commit)
 
     # try to create existing reference with force
-    reference = testrepo.create_branch('version1', commit, True)
-    assert str(reference.target) == LAST_COMMIT
+    assert testrepo.create_branch('version1', commit, True).target == LAST_COMMIT
 
 
 def test_delete(testrepo):
@@ -231,12 +224,9 @@ def test_branch_is_checked_out_returns_false_if_branch_is_not_checked_out(testre
 
 
 def test_branch_rename_succeeds(testrepo):
-    original_branch = testrepo.lookup_branch('i18n')
-    new_branch = original_branch.rename('new-branch')
-    assert str(new_branch.target) == I18N_LAST_COMMIT
-
-    new_branch_2 = testrepo.lookup_branch('new-branch')
-    assert str(new_branch_2.target) == I18N_LAST_COMMIT
+    branch = testrepo.lookup_branch('i18n')
+    assert branch.rename('new-branch').target == I18N_LAST_COMMIT
+    assert testrepo.lookup_branch('new-branch').target == I18N_LAST_COMMIT
 
 
 def test_branch_rename_fails_if_destination_already_exists(testrepo):
@@ -246,9 +236,8 @@ def test_branch_rename_fails_if_destination_already_exists(testrepo):
 
 
 def test_branch_rename_not_fails_if_force_is_true(testrepo):
-    original_branch = testrepo.lookup_branch('master')
-    new_branch = original_branch.rename('i18n', True)
-    assert str(new_branch.target) == LAST_COMMIT
+    branch = testrepo.lookup_branch('master')
+    assert branch.rename('i18n', True).target == LAST_COMMIT
 
 
 def test_branch_rename_fails_with_invalid_names(testrepo):

test/test_branch_empty.py

@@ -39,8 +39,7 @@ def repo(emptyrepo):
 
 def test_branches_remote_get(repo):
     branch = repo.branches.remote.get('origin/master')
-    assert str(branch.target) == ORIGIN_MASTER_COMMIT
-
+    assert branch.target == ORIGIN_MASTER_COMMIT
     assert repo.branches.remote.get('origin/not-exists') is None
 
 
@@ -87,8 +86,7 @@ def test_branches_upstream_name(repo):
 
 def test_lookup_branch_remote(repo):
     branch = repo.lookup_branch('origin/master', BranchType.REMOTE)
-    assert str(branch.target) == ORIGIN_MASTER_COMMIT
-
+    assert branch.target == ORIGIN_MASTER_COMMIT
     assert repo.lookup_branch('origin/not-exists', BranchType.REMOTE) is None
 
 

test/test_commit.py

@@ -52,10 +52,10 @@ def test_commit_refcount(barerepo):
 
 def test_read_commit(barerepo):
     commit = barerepo[COMMIT_SHA]
-    assert COMMIT_SHA == str(commit.id)
+    assert COMMIT_SHA == commit.id
     parents = commit.parents
     assert 1 == len(parents)
-    assert 'c2792cfa289ae6321ecf2cd5806c2194b0fd070c' == str(parents[0].id)
+    assert 'c2792cfa289ae6321ecf2cd5806c2194b0fd070c' == parents[0].id
     assert commit.message_encoding is None
     assert commit.message == (
         'Second test data commit.\n\n' 'This commit has some additional text.\n'
@@ -68,7 +68,7 @@ def test_read_commit(barerepo):
     assert commit.author == Signature(
         'Dave Borowitz', '[email protected]', 1288477363, -420
     )
-    assert '967fce8df97cc71722d3c2a5930ef3e6f1d27b12' == str(commit.tree.id)
+    assert '967fce8df97cc71722d3c2a5930ef3e6f1d27b12' == commit.tree.id
 
 
 def test_new_commit(barerepo):
@@ -90,16 +90,16 @@ def test_new_commit(barerepo):
     commit = repo[sha]
 
     assert ObjectType.COMMIT == commit.type
-    assert '98286caaab3f1fde5bf52c8369b2b0423bad743b' == str(commit.id)
+    assert '98286caaab3f1fde5bf52c8369b2b0423bad743b' == commit.id
     assert commit.message_encoding is None
     assert message == commit.message
     assert 12346 == commit.commit_time
     assert committer == commit.committer
     assert author == commit.author
-    assert tree == str(commit.tree.id)
+    assert tree == commit.tree.id
     assert Oid(hex=tree) == commit.tree_id
     assert 1 == len(commit.parents)
-    assert COMMIT_SHA == str(commit.parents[0].id)
+    assert COMMIT_SHA == commit.parents[0].id
     assert Oid(hex=COMMIT_SHA) == commit.parent_ids[0]
 
 
@@ -124,10 +124,10 @@ def test_new_commit_encoding(barerepo):
     assert 12346 == commit.commit_time
     assert committer == commit.committer
     assert author == commit.author
-    assert tree == str(commit.tree.id)
+    assert tree == commit.tree.id
     assert Oid(hex=tree) == commit.tree_id
     assert 1 == len(commit.parents)
-    assert COMMIT_SHA == str(commit.parents[0].id)
+    assert COMMIT_SHA == commit.parents[0].id
     assert Oid(hex=COMMIT_SHA) == commit.parent_ids[0]
 
 
@@ -268,13 +268,13 @@ def test_amend_commit_argument_types(barerepo):
     amended_oid = repo.amend_commit(alt_commit1, None, message='Hello')
     amended_commit = repo[amended_oid]
     assert ObjectType.COMMIT == amended_commit.type
-    assert str(amended_oid) != COMMIT_SHA_TO_AMEND
+    assert amended_oid != COMMIT_SHA_TO_AMEND
 
     # Pass a str for the commit
     amended_oid = repo.amend_commit(alt_commit2, None, message='Hello', tree=alt_tree)
     amended_commit = repo[amended_oid]
     assert ObjectType.COMMIT == amended_commit.type
-    assert str(amended_oid) != COMMIT_SHA_TO_AMEND
+    assert amended_oid != COMMIT_SHA_TO_AMEND
     assert repo[COMMIT_SHA_TO_AMEND].tree != amended_commit.tree
     assert alt_tree.id == amended_commit.tree_id
 
@@ -283,5 +283,5 @@ def test_amend_commit_argument_types(barerepo):
     amended_oid = repo.amend_commit(alt_commit2, alt_refname, message='Hello')
     amended_commit = repo[amended_oid]
     assert ObjectType.COMMIT == amended_commit.type
-    assert str(amended_oid) != COMMIT_SHA_TO_AMEND
+    assert amended_oid != COMMIT_SHA_TO_AMEND
     assert repo.head.target == amended_oid

test/test_commit_gpg.py

@@ -120,16 +120,16 @@ def test_commit_signing(gpgsigned):
 
     # perform sanity checks
     assert ObjectType.COMMIT == commit.type
-    assert '6569fdf71dbd99081891154641869c537784a3ba' == str(commit.id)
+    assert '6569fdf71dbd99081891154641869c537784a3ba' == commit.id
     assert commit.message_encoding is None
     assert message == commit.message
     assert 1358451456 == commit.commit_time
     assert committer == commit.committer
     assert author == commit.author
-    assert tree == str(commit.tree.id)
+    assert tree == commit.tree.id
     assert Oid(hex=tree) == commit.tree_id
     assert 1 == len(commit.parents)
-    assert parent == str(commit.parents[0].id)
+    assert parent == commit.parents[0].id
     assert Oid(hex=parent) == commit.parent_ids[0]
 
 

test/test_diff.py

@@ -278,16 +278,16 @@ def test_diff_ids(barerepo):
     commit_b = barerepo[COMMIT_SHA1_2]
     patch = commit_a.tree.diff_to_tree(commit_b.tree)[0]
     delta = patch.delta
-    assert str(delta.old_file.id) == '7f129fd57e31e935c6d60a0c794efe4e6927664b'
-    assert str(delta.new_file.id) == 'af431f20fc541ed6d5afede3e2dc7160f6f01f16'
+    assert delta.old_file.id == '7f129fd57e31e935c6d60a0c794efe4e6927664b'
+    assert delta.new_file.id == 'af431f20fc541ed6d5afede3e2dc7160f6f01f16'
 
 
 def test_diff_patchid(barerepo):
     commit_a = barerepo[COMMIT_SHA1_1]
     commit_b = barerepo[COMMIT_SHA1_2]
     diff = commit_a.tree.diff_to_tree(commit_b.tree)
     assert diff.patch == PATCH
-    assert str(diff.patchid) == PATCHID
+    assert diff.patchid == PATCHID
 
 
 def test_hunk_content(barerepo):

test/test_index.py

@@ -147,16 +147,14 @@ def test_read_tree(testrepo):
     index.read_tree(tree_oid)
     assert len(index) == 1
     # Test read-write returns the same oid
-    oid = index.write_tree()
-    assert str(oid) == tree_oid
+    assert index.write_tree() == tree_oid
     # Test the index is only modified in memory
     index.read()
     assert len(index) == 2
 
 
 def test_write_tree(testrepo):
-    oid = testrepo.index.write_tree()
-    assert str(oid) == 'fd937514cb799514d4b81bb24c5fcfeb6472b245'
+    assert testrepo.index.write_tree() == 'fd937514cb799514d4b81bb24c5fcfeb6472b245'
 
 
 def test_iter(testrepo):
@@ -244,8 +242,7 @@ def test_create_entry(testrepo):
     hello_entry = index['hello.txt']
     entry = pygit2.IndexEntry('README.md', hello_entry.id, hello_entry.mode)
     index.add(entry)
-    tree_id = index.write_tree()
-    assert '60e769e57ae1d6a2ab75d8d253139e6260e1f912' == str(tree_id)
+    assert '60e769e57ae1d6a2ab75d8d253139e6260e1f912' == index.write_tree()
 
 
 def test_create_entry_aspath(testrepo):