Return Oid wherever we returned raw oid (bytes) before

jdavid · jdavid · commit 406c3175728c · 2013-04-14T12:26:22.000+02:00
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
diff --git a/src/index.c b/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);
 }
 
 
diff --git a/src/object.c b/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);
 }
 
 
diff --git a/src/oid.c b/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    */
diff --git a/src/oid.h b/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
diff --git a/src/pygit2.c b/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);
 }
 
 
diff --git a/src/reference.c b/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
diff --git a/src/repository.c b/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);
 }
 
 
diff --git a/src/tag.c b/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);
 }
 
 
diff --git a/src/tree.c b/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);
 }
 
 
diff --git a/test/test_blob.py b/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 @@ def test_create_blob(self):
 
         self.assertEqual(blob_oid, blob.oid)
         self.assertEqual(
-          utils.gen_blob_sha1(BLOB_NEW_CONTENT),
-          utils.oid_to_hex(blob_oid)
-        )
+            utils.gen_blob_sha1(BLOB_NEW_CONTENT),
+            blob_oid.hex)
 
         self.assertEqual(BLOB_NEW_CONTENT, blob.data)
         self.assertEqual(len(BLOB_NEW_CONTENT), blob.size)
@@ -84,9 +83,8 @@ def test_create_blob_fromfile(self):
 
         self.assertEqual(blob_oid, blob.oid)
         self.assertEqual(
-          utils.gen_blob_sha1(BLOB_FILE_CONTENT),
-          utils.oid_to_hex(blob_oid)
-        )
+            utils.gen_blob_sha1(BLOB_FILE_CONTENT),
+            blob_oid.hex)
 
         self.assertEqual(BLOB_FILE_CONTENT, blob.data)
         self.assertEqual(len(BLOB_FILE_CONTENT), blob.size)
diff --git a/test/test_index.py b/test/test_index.py
@@ -98,17 +98,15 @@ def test_read_tree(self):
         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, tree_oid)
+        self.assertEqual(oid.hex, tree_oid)
         # Test the index is only modified in memory
         index.read()
         self.assertEqual(len(index), 2)
 
 
     def test_write_tree(self):
         oid = self.repo.index.write_tree()
-        sha = utils.oid_to_hex(oid)
-        self.assertEqual(sha, 'fd937514cb799514d4b81bb24c5fcfeb6472b245')
+        self.assertEqual(oid.hex, 'fd937514cb799514d4b81bb24c5fcfeb6472b245')
 
     def test_iter(self):
         index = self.repo.index
diff --git a/test/test_note.py b/test/test_note.py
@@ -38,9 +38,9 @@
 
 NOTES = [
   ('ab533997b80705767be3dae8cbb06a0740809f79', 'First Note - HEAD\n',
-    '784855caf26449a1914d2cf62d12b9374d76ae78'),
+   '784855caf26449a1914d2cf62d12b9374d76ae78'),
   ('d879714d880671ed84f8aaed8b27fca23ba01f27', 'Second Note - HEAD~1\n',
-    'f5e5aa4e36ab0fe62ee1ccc6eb8f79b866863b87')
+   'f5e5aa4e36ab0fe62ee1ccc6eb8f79b866863b87')
 ]
 
 class NotesTest(utils.BareRepoTestCase):
@@ -49,7 +49,7 @@ def test_create_note(self):
         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)
diff --git a/test/test_oid.py b/test/test_oid.py
@@ -65,6 +65,14 @@ def test_long(self):
         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()
diff --git a/test/test_repository.py b/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 @@ def test_write(self):
         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(len(oid), 20)
+        self.assertEqual(type(oid), Oid)
 
     def test_contains(self):
         self.assertRaises(TypeError, lambda: 123 in self.repo)
@@ -222,16 +225,18 @@ def test_checkout_head(self):
         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(len(oid), 20)
+        self.assertEqual(type(oid), Oid)
 
         assert os.path.exists(os.path.join(self._temp_dir, '.git'))
 
+
 class InitRepositoryTest(utils.NoRepoTestCase):
     # under the assumption that repo.is_bare works
 
diff --git a/test/test_tag.py b/test/test_tag.py
@@ -72,7 +72,7 @@ def test_new_tag(self):
 
         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)
diff --git a/test/utils.py b/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.

Original file line number	Diff line number	Diff line change
`@@ -423,7 +423,7 @@ Index_write_tree(Index *self)`
`423`	`423`	`if (err < 0)`
`424`	`424`	`return Error_set(err);`
`425`	`425`
`426`		`- return git_oid_to_python(oid.id);`
	`426`	`+ return git_oid_to_python(&oid);`
`427`	`427`	`}`
`428`	`428`
`429`	`429`	`PyMethodDef Index_methods[] = {`
`@@ -585,7 +585,7 @@ PyDoc_STRVAR(IndexEntry_oid__doc__, "Object id.");`
`585`	`585`	`PyObject *`
`586`	`586`	`IndexEntry_oid__get__(IndexEntry *self)`
`587`	`587`	`{`
`588`		`- return git_oid_to_python(self->entry->oid.id);`
	`588`	`+ return git_oid_to_python(&self->entry->oid);`
`589`	`589`	`}`
`590`	`590`
`591`	`591`
Original file line number	Diff line number	Diff line change
`@@ -60,7 +60,7 @@ Object_oid__get__(Object *self)`
`60`	`60`	`oid = git_object_id(self->obj);`
`61`	`61`	`assert(oid);`
`62`	`62`
`63`		`- return git_oid_to_python(oid->id);`
	`63`	`+ return git_oid_to_python(oid);`
`64`	`64`	`}`
`65`	`65`
`66`	`66`
Original file line number	Diff line number	Diff line change
`@@ -142,7 +142,7 @@ hashfile(PyObject self, PyObject args)`
`142`	`142`	`if (err < 0)`
`143`	`143`	`return Error_set(err);`
`144`	`144`
`145`		`- return git_oid_to_python(oid.id);`
	`145`	`+ return git_oid_to_python(&oid);`
`146`	`146`	`}`
`147`	`147`
`148`	`148`	`PyDoc_STRVAR(hash__doc__,`
`@@ -166,7 +166,7 @@ hash(PyObject self, PyObject args)`
`166`	`166`	`return Error_set(err);`
`167`	`167`	`}`
`168`	`168`
`169`		`- return git_oid_to_python(oid.id);`
	`169`	`+ return git_oid_to_python(&oid);`
`170`	`170`	`}`
`171`	`171`
`172`	`172`
Original file line number	Diff line number	Diff line change
`@@ -276,7 +276,7 @@ Reference_oid__get__(Reference *self)`
`276`	`276`	`}`
`277`	`277`
`278`	`278`	`/* Convert and return it */`
`279`		`- return git_oid_to_python(oid->id);`
	`279`	`+ return git_oid_to_python(oid);`
`280`	`280`	`}`
`281`	`281`
`282`	`282`	`int`
Original file line number	Diff line number	Diff line change
`@@ -403,7 +403,7 @@ Repository_write(Repository self, PyObject args)`
`403`	`403`	`stream->write(stream, buffer, buflen);`
`404`	`404`	`err = stream->finalize_write(&oid, stream);`
`405`	`405`	`stream->free(stream);`
`406`		`- return git_oid_to_python(oid.id);`
	`406`	`+ return git_oid_to_python(&oid);`
`407`	`407`	`}`
`408`	`408`
`409`	`409`
`@@ -578,7 +578,7 @@ Repository_create_blob(Repository self, PyObject args)`
`578`	`578`	`if (err < 0)`
`579`	`579`	`return Error_set(err);`
`580`	`580`
`581`		`- return git_oid_to_python(oid.id);`
	`581`	`+ return git_oid_to_python(&oid);`
`582`	`582`	`}`
`583`	`583`
`584`	`584`
`@@ -601,7 +601,7 @@ Repository_create_blob_fromfile(Repository self, PyObject args)`
`601`	`601`	`if (err < 0)`
`602`	`602`	`return Error_set(err);`
`603`	`603`
`604`		`- return git_oid_to_python(oid.id);`
	`604`	`+ return git_oid_to_python(&oid);`
`605`	`605`	`}`
`606`	`606`
`607`	`607`
`@@ -674,7 +674,7 @@ Repository_create_commit(Repository self, PyObject args)`
`674`	`674`	`goto out;`
`675`	`675`	`}`
`676`	`676`
`677`		`- py_result = git_oid_to_python(oid.id);`
	`677`	`+ py_result = git_oid_to_python(&oid);`
`678`	`678`
`679`	`679`	`out:`
`680`	`680`	`free(message);`
`@@ -722,7 +722,7 @@ Repository_create_tag(Repository self, PyObject args)`
`722`	`722`	`git_object_free(target);`
`723`	`723`	`if (err < 0)`
`724`	`724`	`return Error_set_oid(err, &oid, len);`
`725`		`- return git_oid_to_python(oid.id);`
	`725`	`+ return git_oid_to_python(&oid);`
`726`	`726`	`}`
`727`	`727`
`728`	`728`
`@@ -1171,7 +1171,7 @@ Repository_create_note(Repository self, PyObject args)`
`1171`	`1171`	`if (err < 0)`
`1172`	`1172`	`return Error_set(err);`
`1173`	`1173`
`1174`		`- return git_oid_to_python(note_id.id);`
	`1174`	`+ return git_oid_to_python(&note_id);`
`1175`	`1175`	`}`
`1176`	`1176`
`1177`	`1177`
Original file line number	Diff line number	Diff line change
`@@ -43,7 +43,7 @@ Tag_target__get__(Tag *self)`
`43`	`43`	`const git_oid *oid;`
`44`	`44`
`45`	`45`	`oid = git_tag_target_id(self->tag);`
`46`		`- return git_oid_to_python(oid->id);`
	`46`	`+ return git_oid_to_python(oid);`
`47`	`47`	`}`
`48`	`48`
`49`	`49`
Original file line number	Diff line number	Diff line change
`@@ -74,7 +74,7 @@ TreeEntry_oid__get__(TreeEntry *self)`
`74`	`74`	`const git_oid *oid;`
`75`	`75`
`76`	`76`	`oid = git_tree_entry_id(self->entry);`
`77`		`- return git_oid_to_python(oid->id);`
	`77`	`+ return git_oid_to_python(oid);`
`78`	`78`	`}`
`79`	`79`
`80`	`80`