Merge remote-tracking branch 'mduggan/remote-refcount-fix'

pygit2/decl.h

@@ -136,6 +136,7 @@ int git_remote_add_push(git_remote *remote, const char *refspec);
 int git_remote_add_fetch(git_remote *remote, const char *refspec);
 int git_remote_save(const git_remote *remote);
 int git_remote_set_callbacks(git_remote *remote, const git_remote_callbacks *callbacks);
+int git_remote_init_callbacks(git_remote_callbacks *opts, unsigned int version);
 size_t git_remote_refspec_count(git_remote *remote);
 const git_refspec * git_remote_get_refspec(git_remote *remote, size_t n);
 

pygit2/remote.py

@@ -127,20 +127,6 @@ class Remote(object):
         self._remote = ptr
         self._stored_exception = None
 
-        # Build the callback structure
-        callbacks = ffi.new('git_remote_callbacks *')
-        callbacks.version = 1
-        callbacks.sideband_progress = self._sideband_progress_cb
-        callbacks.transfer_progress = self._transfer_progress_cb
-        callbacks.update_tips = self._update_tips_cb
-        callbacks.credentials = self._credentials_cb
-        # We need to make sure that this handle stays alive
-        self._self_handle = ffi.new_handle(self)
-        callbacks.payload = self._self_handle
-
-        err = C.git_remote_set_callbacks(self._remote, callbacks)
-        check_error(err)
-
     def __del__(self):
         C.git_remote_free(self._remote)
 
@@ -205,17 +191,46 @@ class Remote(object):
         Perform a fetch against this remote.
         """
 
+        # Get the default callbacks first
+        defaultcallbacks = ffi.new('git_remote_callbacks *')
+        err = C.git_remote_init_callbacks(defaultcallbacks, 1)
+        check_error(err)
+
+        # Build custom callback structure
+        callbacks = ffi.new('git_remote_callbacks *')
+        callbacks.version = 1
+        callbacks.sideband_progress = self._sideband_progress_cb
+        callbacks.transfer_progress = self._transfer_progress_cb
+        callbacks.update_tips = self._update_tips_cb
+        callbacks.credentials = self._credentials_cb
+        # We need to make sure that this handle stays alive
+        self._self_handle = ffi.new_handle(self)
+        callbacks.payload = self._self_handle
+
+        err = C.git_remote_set_callbacks(self._remote, callbacks)
+        try:
+            check_error(err)
+        finally:
+            self._self_handle = None
+
         if signature:
             ptr = signature._pointer[:]
         else:
             ptr = ffi.NULL
 
         self._stored_exception = None
-        err = C.git_remote_fetch(self._remote, ptr, to_bytes(message))
-        if self._stored_exception:
-            raise self._stored_exception
 
-        check_error(err)
+        try:
+            err = C.git_remote_fetch(self._remote, ptr, to_bytes(message))
+            if self._stored_exception:
+                raise self._stored_exception
+
+            check_error(err)
+        finally:
+            # Even on error, clear stored callbacks and reset to default
+            self._self_handle = None
+            err = C.git_remote_set_callbacks(self._remote, defaultcallbacks)
+            check_error(err)
 
         return TransferProgress(C.git_remote_stats(self._remote))
 

test/test_commit.py

@@ -30,6 +30,7 @@
 from __future__ import absolute_import
 from __future__ import unicode_literals
 import unittest
+import sys
 
 from pygit2 import GIT_OBJ_COMMIT, Signature, Oid
 from . import utils
@@ -46,6 +47,16 @@ COMMIT_SHA = '5fe808e8953c12735680c257f56600cb0de44b10'
 
 class CommitTest(utils.BareRepoTestCase):
 
+    @unittest.skipIf(__pypy__ is not None, "skip refcounts checks in pypy")
+    def test_commit_refcount(self):
+        commit = self.repo[COMMIT_SHA]
+        start = sys.getrefcount(commit)
+        tree = commit.tree
+        del tree
+        end = sys.getrefcount(commit)
+        self.assertEqual(start, end)
+
+
     def test_read_commit(self):
         commit = self.repo[COMMIT_SHA]
         self.assertEqual(COMMIT_SHA, str(commit.id))

test/test_remote.py

@@ -30,9 +30,15 @@
 
 import unittest
 import pygit2
+import sys
 from pygit2 import Oid
 from . import utils
 
+try:
+    import __pypy__
+except ImportError:
+    __pypy__ = None
+
 REMOTE_NAME = 'origin'
 REMOTE_URL = 'git://github.com/libgit2/pygit2.git'
 REMOTE_FETCHSPEC_SRC = 'refs/heads/*'
@@ -163,6 +169,14 @@ class RepositoryTest(utils.RepoTestCase):
         self.assertEqual('http://example.com/test.git',
                          self.repo.remotes[0].url)
 
+    @unittest.skipIf(__pypy__ is not None, "skip refcounts checks in pypy")
+    def test_remote_refcount(self):
+        start = sys.getrefcount(self.repo)
+        remote = self.repo.remotes[0]
+        del remote
+        end = sys.getrefcount(self.repo)
+        self.assertEqual(start, end)
+
     def test_add_refspec(self):
         remote = self.repo.create_remote('test_add_refspec', REMOTE_URL)
         remote.add_push('refs/heads/*:refs/heads/test_refspec/*')

test/test_repository.py

@@ -37,6 +37,7 @@ import unittest
 import tempfile
 import os
 from os.path import join, realpath
+import sys
 
 # Import from pygit2
 from pygit2 import GIT_OBJ_ANY, GIT_OBJ_BLOB, GIT_OBJ_COMMIT
@@ -45,6 +46,11 @@ from pygit2 import Oid, Reference, hashfile
 import pygit2
 from . import utils
 
+try:
+    import __pypy__
+except ImportError:
+    __pypy__ = None
+
 
 HEAD_SHA = '784855caf26449a1914d2cf62d12b9374d76ae78'
 PARENT_SHA = 'f5e5aa4e36ab0fe62ee1ccc6eb8f79b866863b87'  # HEAD^
@@ -150,6 +156,15 @@ class RepositoryTest(utils.BareRepoTestCase):
             commit.message)
         self.assertRaises(ValueError, self.repo.__getitem__, too_short_prefix)
 
+    @unittest.skipIf(__pypy__ is not None, "skip refcounts checks in pypy")
+    def test_lookup_commit_refcount(self):
+        start = sys.getrefcount(self.repo)
+        commit_sha = '5fe808e8953c12735680c257f56600cb0de44b10'
+        commit = self.repo[commit_sha]
+        del commit
+        end = sys.getrefcount(self.repo)
+        self.assertEqual(start, end)
+
     def test_get_path(self):
         directory = realpath(self.repo.path)
         expected = realpath(self.repo_path)