Return Oid wherever we returned raw oid (bytes) before

Changes:

- Return Oid wherever we returned raw oid (bytes) before
- Now py_str_to_git_oid accepts Oid objects
- Add ability to compare two Oid objects

src/index.c

@@ -423,7 +423,7 @@ Index_write_tree(Index *self)
     if (err < 0)
         return Error_set(err);
 
-    return git_oid_to_python(oid.id);
+    return git_oid_to_python(&oid);
 }
 
 PyMethodDef Index_methods[] = {
@@ -585,7 +585,7 @@ PyDoc_STRVAR(IndexEntry_oid__doc__, "Object id.");
 PyObject *
 IndexEntry_oid__get__(IndexEntry *self)
 {
-    return git_oid_to_python(self->entry->oid.id);
+    return git_oid_to_python(&self->entry->oid);
 }
 
 

src/object.c

@@ -60,7 +60,7 @@ Object_oid__get__(Object *self)
     oid = git_object_id(self->obj);
     assert(oid);
 
-    return git_oid_to_python(oid->id);
+    return git_oid_to_python(oid);
 }
 
 

src/oid.c

@@ -32,6 +32,20 @@
 #include "error.h"
 #include "oid.h"
 
+PyTypeObject OidType;
+
+
+PyObject *
+git_oid_to_python(const git_oid *oid)
+{
+    Oid *py_oid;
+
+    py_oid = PyObject_New(Oid, &OidType);
+    git_oid_cpy(&(py_oid->oid), oid);
+    return (PyObject*)py_oid;
+}
+
+
 int
 py_str_to_git_oid(PyObject *py_str, git_oid *oid)
 {
@@ -40,7 +54,13 @@ py_str_to_git_oid(PyObject *py_str, git_oid *oid)
     int err;
     Py_ssize_t len;
 
-    /* Case 1: raw sha */
+    /* Case 1: Git Oid */
+    if (PyObject_TypeCheck(py_str, (PyTypeObject*)&OidType)) {
+        git_oid_cpy(oid, &((Oid*)py_str)->oid);
+        return GIT_OID_RAWSZ;
+    }
+
+    /* Case 2: raw sha (bytes) */
     if (PyBytes_Check(py_str)) {
         err = PyBytes_AsStringAndSize(py_str, &hex_or_bin, &len);
         if (err)
@@ -53,7 +73,7 @@ py_str_to_git_oid(PyObject *py_str, git_oid *oid)
         return len * 2;
     }
 
-    /* Case 2: hex sha */
+    /* Case 3: hex sha (unicode) */
     if (PyUnicode_Check(py_str)) {
         py_hex = PyUnicode_AsASCIIString(py_str);
         if (py_hex == NULL)
@@ -159,12 +179,19 @@ Oid_init(Oid *self, PyObject *args, PyObject *kw)
 }
 
 
+int
+Oid_compare(PyObject *o1, PyObject *o2)
+{
+    return git_oid_cmp(&((Oid*)o1)->oid, &((Oid*)o2)->oid);
+}
+
+
 PyDoc_STRVAR(Oid_raw__doc__, "Raw oid.");
 
 PyObject *
 Oid_raw__get__(Oid *self)
 {
-    return git_oid_to_python(self->oid.id);
+    return PyBytes_FromStringAndSize((const char*)self->oid.id, GIT_OID_RAWSZ);
 }
 
 
@@ -193,7 +220,7 @@ PyTypeObject OidType = {
     0,                                         /* tp_print          */
     0,                                         /* tp_getattr        */
     0,                                         /* tp_setattr        */
-    0,                                         /* tp_compare        */
+    (cmpfunc)Oid_compare,                      /* tp_compare        */
     0,                                         /* tp_repr           */
     0,                                         /* tp_as_number      */
     0,                                         /* tp_as_sequence    */

src/oid.h

@@ -34,10 +34,8 @@
 
 int py_str_to_git_oid(PyObject *py_str, git_oid *oid);
 int py_str_to_git_oid_expand(git_repository *repo, PyObject *py_str,
-                              git_oid *oid);
+                             git_oid *oid);
+PyObject* git_oid_to_python(const git_oid *oid);
 PyObject* git_oid_to_py_str(const git_oid *oid);
 
-#define git_oid_to_python(id) \
-        PyBytes_FromStringAndSize((const char*)id, GIT_OID_RAWSZ)
-
 #endif

src/pygit2.c

@@ -142,7 +142,7 @@ hashfile(PyObject *self, PyObject *args)
     if (err < 0)
         return Error_set(err);
 
-    return git_oid_to_python(oid.id);
+    return git_oid_to_python(&oid);
 }
 
 PyDoc_STRVAR(hash__doc__,
@@ -166,7 +166,7 @@ hash(PyObject *self, PyObject *args)
         return Error_set(err);
     }
 
-    return git_oid_to_python(oid.id);
+    return git_oid_to_python(&oid);
 }
 
 

src/reference.c

@@ -276,7 +276,7 @@ Reference_oid__get__(Reference *self)
     }
 
     /* Convert and return it */
-    return git_oid_to_python(oid->id);
+    return git_oid_to_python(oid);
 }
 
 int

src/repository.c

@@ -403,7 +403,7 @@ Repository_write(Repository *self, PyObject *args)
     stream->write(stream, buffer, buflen);
     err = stream->finalize_write(&oid, stream);
     stream->free(stream);
-    return git_oid_to_python(oid.id);
+    return git_oid_to_python(&oid);
 }
 
 
@@ -578,7 +578,7 @@ Repository_create_blob(Repository *self, PyObject *args)
     if (err < 0)
         return Error_set(err);
 
-    return git_oid_to_python(oid.id);
+    return git_oid_to_python(&oid);
 }
 
 
@@ -601,7 +601,7 @@ Repository_create_blob_fromfile(Repository *self, PyObject *args)
     if (err < 0)
         return Error_set(err);
 
-    return git_oid_to_python(oid.id);
+    return git_oid_to_python(&oid);
 }
 
 
@@ -674,7 +674,7 @@ Repository_create_commit(Repository *self, PyObject *args)
         goto out;
     }
 
-    py_result = git_oid_to_python(oid.id);
+    py_result = git_oid_to_python(&oid);
 
 out:
     free(message);
@@ -722,7 +722,7 @@ Repository_create_tag(Repository *self, PyObject *args)
     git_object_free(target);
     if (err < 0)
         return Error_set_oid(err, &oid, len);
-    return git_oid_to_python(oid.id);
+    return git_oid_to_python(&oid);
 }
 
 
@@ -1171,7 +1171,7 @@ Repository_create_note(Repository *self, PyObject* args)
     if (err < 0)
         return Error_set(err);
 
-    return git_oid_to_python(note_id.id);
+    return git_oid_to_python(&note_id);
 }
 
 

src/tag.c

@@ -43,7 +43,7 @@ Tag_target__get__(Tag *self)
     const git_oid *oid;
 
     oid = git_tag_target_id(self->tag);
-    return git_oid_to_python(oid->id);
+    return git_oid_to_python(oid);
 }
 
 

src/tree.c

@@ -74,7 +74,7 @@ TreeEntry_oid__get__(TreeEntry *self)
     const git_oid *oid;
 
     oid = git_tree_entry_id(self->entry);
-    return git_oid_to_python(oid->id);
+    return git_oid_to_python(oid);
 }
 
 

test/test_blob.py

@@ -49,7 +49,7 @@ class BlobTest(utils.RepoTestCase):
     def test_read_blob(self):
         blob = self.repo[BLOB_SHA]
         self.assertEqual(blob.hex, BLOB_SHA)
-        sha = utils.oid_to_hex(blob.oid)
+        sha = blob.oid.hex
         self.assertEqual(sha, BLOB_SHA)
         self.assertTrue(isinstance(blob, pygit2.Blob))
         self.assertEqual(pygit2.GIT_OBJ_BLOB, blob.type)
@@ -66,9 +66,8 @@ class BlobTest(utils.RepoTestCase):
 
         self.assertEqual(blob_oid, blob.oid)
         self.assertEqual(
-          utils.gen_blob_sha1(BLOB_NEW_CONTENT),
+            utils.gen_blob_sha1(BLOB_NEW_CONTENT),
-          utils.oid_to_hex(blob_oid)
+            blob_oid.hex)
-        )
 
         self.assertEqual(BLOB_NEW_CONTENT, blob.data)
         self.assertEqual(len(BLOB_NEW_CONTENT), blob.size)
@@ -84,9 +83,8 @@ class BlobTest(utils.RepoTestCase):
 
         self.assertEqual(blob_oid, blob.oid)
         self.assertEqual(
-          utils.gen_blob_sha1(BLOB_FILE_CONTENT),
+            utils.gen_blob_sha1(BLOB_FILE_CONTENT),
-          utils.oid_to_hex(blob_oid)
+            blob_oid.hex)
-        )
 
         self.assertEqual(BLOB_FILE_CONTENT, blob.data)
         self.assertEqual(len(BLOB_FILE_CONTENT), blob.size)

test/test_index.py

@@ -98,8 +98,7 @@ class IndexTest(utils.RepoTestCase):
         self.assertEqual(len(index), 1)
         # Test read-write returns the same oid
         oid = index.write_tree()
-        oid = utils.oid_to_hex(oid)
+        self.assertEqual(oid.hex, tree_oid)
-        self.assertEqual(oid, tree_oid)
         # Test the index is only modified in memory
         index.read()
         self.assertEqual(len(index), 2)
@@ -107,8 +106,7 @@ class IndexTest(utils.RepoTestCase):
 
     def test_write_tree(self):
         oid = self.repo.index.write_tree()
-        sha = utils.oid_to_hex(oid)
+        self.assertEqual(oid.hex, 'fd937514cb799514d4b81bb24c5fcfeb6472b245')
-        self.assertEqual(sha, 'fd937514cb799514d4b81bb24c5fcfeb6472b245')
 
     def test_iter(self):
         index = self.repo.index

test/test_note.py

@@ -38,9 +38,9 @@ NOTE = ('6c8980ba963cad8b25a9bcaf68d4023ee57370d8', 'note message')
 
 NOTES = [
   ('ab533997b80705767be3dae8cbb06a0740809f79', 'First Note - HEAD\n',
-    '784855caf26449a1914d2cf62d12b9374d76ae78'),
+   '784855caf26449a1914d2cf62d12b9374d76ae78'),
   ('d879714d880671ed84f8aaed8b27fca23ba01f27', 'Second Note - HEAD~1\n',
-    'f5e5aa4e36ab0fe62ee1ccc6eb8f79b866863b87')
+   'f5e5aa4e36ab0fe62ee1ccc6eb8f79b866863b87')
 ]
 
 class NotesTest(utils.BareRepoTestCase):
@@ -49,7 +49,7 @@ class NotesTest(utils.BareRepoTestCase):
         annotated_id = self.repo.revparse_single('HEAD~3').hex
         author = committer = Signature('Foo bar', 'foo@bar.com', 12346, 0)
         note_id = self.repo.create_note(NOTE[1], author, committer, annotated_id)
-        self.assertEqual(NOTE[0], utils.oid_to_hex(note_id))
+        self.assertEqual(NOTE[0], note_id.hex)
 
         # check the note blob
         self.assertEqual(NOTE[1].encode(), self.repo[note_id].data)

test/test_oid.py

@@ -65,6 +65,14 @@ class OidTest(utils.BareRepoTestCase):
         self.assertRaises(ValueError, Oid, raw=RAW + b'a')
         self.assertRaises(ValueError, Oid, hex=HEX + 'a')
 
+    def test_cmp(self):
+        oid1 = Oid(raw=RAW)
+        oid2 = Oid(hex=HEX)
+        self.assertEqual(oid1, oid2)
+
+        oid2 = Oid(hex="15b648aec6ed045b5ca6f57f8b7831a8b4757299")
+        self.assertNotEqual(oid1, oid2)
+
 
 if __name__ == '__main__':
     unittest.main()

test/test_repository.py

@@ -27,18 +27,22 @@
 
 """Tests for Repository objects."""
 
+# Import from the future
 from __future__ import absolute_import
 from __future__ import unicode_literals
+
+# Import from the Standard Library
 import binascii
 import unittest
 import tempfile
 import os
 from os.path import join, realpath
 
+# Import from pygit2
 from pygit2 import GIT_OBJ_ANY, GIT_OBJ_BLOB, GIT_OBJ_COMMIT
 from pygit2 import init_repository, discover_repository, Commit, hashfile
+from pygit2 import Oid
 import pygit2
-
 from . import utils
 
 
@@ -85,8 +89,7 @@ class RepositoryTest(utils.BareRepoTestCase):
         self.assertRaises(ValueError, self.repo.write, GIT_OBJ_ANY, data)
 
         oid = self.repo.write(GIT_OBJ_BLOB, data)
-        self.assertEqual(type(oid), bytes)
+        self.assertEqual(type(oid), Oid)
-        self.assertEqual(len(oid), 20)
 
     def test_contains(self):
         self.assertRaises(TypeError, lambda: 123 in self.repo)
@@ -222,16 +225,18 @@ class RepositoryTest_II(utils.RepoTestCase):
         self.repo.checkout(pygit2.GIT_CHECKOUT_FORCE, head=True)
         self.assertTrue('bye.txt' not in self.repo.status())
 
+
 class NewRepositoryTest(utils.NoRepoTestCase):
+
     def test_new_repo(self):
         repo = init_repository(self._temp_dir, False)
 
         oid = repo.write(GIT_OBJ_BLOB, "Test")
-        self.assertEqual(type(oid), bytes)
+        self.assertEqual(type(oid), Oid)
-        self.assertEqual(len(oid), 20)
 
         assert os.path.exists(os.path.join(self._temp_dir, '.git'))
 
+
 class InitRepositoryTest(utils.NoRepoTestCase):
     # under the assumption that repo.is_bare works
 

test/test_tag.py

@@ -72,7 +72,7 @@ class TagTest(utils.BareRepoTestCase):
 
         self.assertEqual('3ee44658fd11660e828dfc96b9b5c5f38d5b49bb', tag.hex)
         self.assertEqual(name, tag.name)
-        self.assertEqual(target, utils.oid_to_hex(tag.target))
+        self.assertEqual(target, tag.target.hex)
         self.assertEqualSignature(tagger, tag.tagger)
         self.assertEqual(message, tag.message)
         self.assertEqual(name, self.repo[tag.hex].name)

test/utils.py

@@ -47,8 +47,6 @@ def force_rm_handle(remove_path, path, excinfo):
     )
     remove_path(path)
 
-def oid_to_hex(oid):
-    return b2a_hex(oid).decode('ascii')
 
 def gen_blob_sha1(data):
     # http://stackoverflow.com/questions/552659/assigning-git-sha1s-without-git
@@ -58,6 +56,7 @@ def gen_blob_sha1(data):
 
     return m.hexdigest()
 
+
 def rmtree(path):
     """In Windows a read-only file cannot be removed, and shutil.rmtree fails.
     So we implement our own version of rmtree to address this issue.