Internal: Refactoring callbacks code

The purpose is to increase readibility.

Author: jdavid

pygit2/__init__.py

@@ -31,6 +31,7 @@
 
 # High level API
 from .blame import Blame, BlameHunk
+from .callbacks import git_fetch_options
 from .config import Config
 from .credentials import *
 from .errors import check_error, Passthrough
@@ -246,18 +247,12 @@ def clone_repository(
 
     opts.bare = bare
 
-    if callbacks is None:
-        callbacks = RemoteCallbacks()
-    callbacks._fill_fetch_options(opts.fetch_opts)
-
-    err = C.git_clone(crepo, to_bytes(url), to_bytes(path), opts)
-
-    # Error handling
-    exc = d.get('exception', callbacks._stored_exception)
-    if exc:
-        raise exc
-
-    check_error(err)
+    with git_fetch_options(callbacks, opts=opts.fetch_opts) as (_, cb):
+        err = C.git_clone(crepo, to_bytes(url), to_bytes(path), opts)
+        exc = d.get('exception')
+        if exc:
+            raise exc
+        check_error(err, cb)
 
     # Ok
     return Repository._from_c(crepo[0], owned=True)

pygit2/callbacks.py

@@ -0,0 +1,82 @@
+"""
+This is a utility module, it's here to support calls from libgit2 to written in
+Python.
+"""
+
+# Standard Library
+from contextlib import contextmanager
+
+# pygit2
+from .ffi import ffi, C
+
+
+@contextmanager
+def git_fetch_options(callbacks, opts=None):
+    from .remote import RemoteCallbacks
+    if callbacks is None:
+        callbacks = RemoteCallbacks()
+
+    if opts is None:
+        opts = ffi.new('git_fetch_options *')
+        C.git_fetch_init_options(opts, C.GIT_FETCH_OPTIONS_VERSION)
+
+    # Plug callbacks
+    opts.callbacks.sideband_progress = C._sideband_progress_cb
+    opts.callbacks.transfer_progress = C._transfer_progress_cb
+    opts.callbacks.update_tips = C._update_tips_cb
+    opts.callbacks.credentials = C._credentials_cb
+    opts.callbacks.certificate_check = C._certificate_cb
+    # Payload
+    callbacks._stored_exception = None
+    handle = ffi.new_handle(callbacks)
+    opts.callbacks.payload = handle
+
+    # Give back control
+    yield opts, callbacks
+
+
+@contextmanager
+def git_push_options(callbacks, opts=None):
+    from .remote import RemoteCallbacks
+    if callbacks is None:
+        callbacks = RemoteCallbacks()
+
+    if opts is None:
+        opts = ffi.new('git_push_options *')
+        C.git_push_init_options(opts, C.GIT_PUSH_OPTIONS_VERSION)
+
+    # Plug callbacks
+    opts.callbacks.sideband_progress = C._sideband_progress_cb
+    opts.callbacks.transfer_progress = C._transfer_progress_cb
+    opts.callbacks.update_tips = C._update_tips_cb
+    opts.callbacks.credentials = C._credentials_cb
+    opts.callbacks.certificate_check = C._certificate_cb
+    opts.callbacks.push_update_reference = C._push_update_reference_cb
+    # Payload
+    callbacks._stored_exception = None
+    handle = ffi.new_handle(callbacks)
+    opts.callbacks.payload = handle
+
+    # Give back control
+    yield opts, callbacks
+
+
+@contextmanager
+def git_remote_callbacks(callbacks):
+    from .remote import RemoteCallbacks
+    if callbacks is None:
+        callbacks = RemoteCallbacks()
+
+    cdata = ffi.new('git_remote_callbacks *')
+    C.git_remote_init_callbacks(cdata, C.GIT_REMOTE_CALLBACKS_VERSION)
+
+    # Plug callbacks
+    cdata.credentials = C._credentials_cb
+    cdata.update_tips = C._update_tips_cb
+    # Payload
+    callbacks._stored_exception = None
+    handle = ffi.new_handle(callbacks)
+    cdata.payload = handle
+
+    # Give back control
+    yield cdata, callbacks

pygit2/errors.py

@@ -30,7 +30,12 @@
 
 value_errors = set([C.GIT_EEXISTS, C.GIT_EINVALIDSPEC, C.GIT_EAMBIGUOUS])
 
-def check_error(err, io=False):
+def check_error(err, cb=None, io=False):
+    if err == C.GIT_EUSER:
+        assert cb is not None
+        assert cb._stored_exception is not None
+        raise cb._stored_exception
+
     if err >= 0:
         return
 

pygit2/remote.py

@@ -25,6 +25,7 @@
 
 # Import from pygit2
 from ._pygit2 import Oid
+from .callbacks import git_fetch_options, git_push_options, git_remote_callbacks
 from .errors import check_error, Passthrough
 from .ffi import ffi, C
 from .refspec import Refspec
@@ -182,41 +183,6 @@ def push_update_reference(self, refname, message):
             Rejection message from the remote. If None, the update was accepted.
         """
 
-    def _fill_fetch_options(self, fetch_opts):
-        fetch_opts.callbacks.sideband_progress = C._sideband_progress_cb
-        fetch_opts.callbacks.transfer_progress = C._transfer_progress_cb
-        fetch_opts.callbacks.update_tips = C._update_tips_cb
-        fetch_opts.callbacks.credentials = C._credentials_cb
-        fetch_opts.callbacks.certificate_check = C._certificate_cb
-        # We need to make sure that this handle stays alive
-        self._self_handle = ffi.new_handle(self)
-        fetch_opts.callbacks.payload = self._self_handle
-
-        self._stored_exception = None
-
-    def _fill_push_options(self, push_opts):
-        push_opts.callbacks.sideband_progress = C._sideband_progress_cb
-        push_opts.callbacks.transfer_progress = C._transfer_progress_cb
-        push_opts.callbacks.update_tips = C._update_tips_cb
-        push_opts.callbacks.credentials = C._credentials_cb
-        push_opts.callbacks.certificate_check = C._certificate_cb
-        push_opts.callbacks.push_update_reference = C._push_update_reference_cb
-        # We need to make sure that this handle stays alive
-        self._self_handle = ffi.new_handle(self)
-        push_opts.callbacks.payload = self._self_handle
-
-    def _fill_prune_callbacks(self, prune_callbacks):
-        prune_callbacks.update_tips = C._update_tips_cb
-        # We need to make sure that this handle stays alive
-        self._self_handle = ffi.new_handle(self)
-        prune_callbacks.payload = self._self_handle
-
-    def _fill_connect_callbacks(self, connect_callbacks):
-        connect_callbacks.credentials = C._credentials_cb
-        # We need to make sure that this handle stays alive
-        self._self_handle = ffi.new_handle(self)
-        connect_callbacks.payload = self._self_handle
-
 
 # These functions exist to be called by the git_remote as callbacks. They proxy
 # the call to whatever the user set
@@ -374,21 +340,15 @@ def push_url(self):
         return maybe_string(C.git_remote_pushurl(self._remote))
 
     def connect(self, callbacks=None, direction=C.GIT_DIRECTION_FETCH):
-        """Connect to the remote."""
-
-        remote_callbacks = ffi.new('git_remote_callbacks *')
-        C.git_remote_init_callbacks(remote_callbacks, C.GIT_REMOTE_CALLBACKS_VERSION)
-
-        if callbacks is None:
-            callbacks = RemoteCallbacks()
-        callbacks._fill_connect_callbacks(remote_callbacks)
-
-        err = C.git_remote_connect(self._remote, direction, remote_callbacks, ffi.NULL, ffi.NULL);
-        check_error(err)
+        """Connect to the remote.
+        """
+        with git_remote_callbacks(callbacks) as (remote_callbacks, cb):
+            err = C.git_remote_connect(self._remote, direction, remote_callbacks, ffi.NULL, ffi.NULL)
+            check_error(err, cb)
 
     def save(self):
-        """Save a remote to its repository's configuration."""
-
+        """Save a remote to its repository's configuration.
+        """
         err = C.git_remote_save(self._remote)
         check_error(err)
 
@@ -405,26 +365,12 @@ def fetch(self, refspecs=None, message=None, callbacks=None, prune=C.GIT_FETCH_P
             repository that does not exist in the remote and the last will
             always keep the remote branches
         """
-
         message = to_bytes(message)
-
-        opts = ffi.new('git_fetch_options *')
-        err = C.git_fetch_init_options(opts, C.GIT_FETCH_OPTIONS_VERSION)
-
-        if callbacks is None:
-            callbacks = RemoteCallbacks()
-        callbacks._fill_fetch_options(opts)
-
-        opts.prune = prune
-
-        try:
+        with git_fetch_options(callbacks) as (opts, cb):
+            opts.prune = prune
             with StrArray(refspecs) as arr:
-                err = C.git_remote_fetch(self._remote, arr, opts, message)
-                if callbacks._stored_exception:
-                    raise callbacks._stored_exception
-                check_error(err)
-        finally:
-            callbacks._self_handle = None
+                err = C.git_remote_fetch(self._remote, arr, opts, to_bytes(message))
+                check_error(err, cb)
 
         return TransferProgress(C.git_remote_stats(self._remote))
 
@@ -443,11 +389,8 @@ def ls_remotes(self, callbacks=None):
         check_error(err)
 
         results = []
-
         for i in range(int(refs_len[0])):
-
             local = bool(refs[0][i].local)
-
             if local:
                 loid = Oid(raw=bytes(ffi.buffer(refs[0][i].loid.id)[:]))
             else:
@@ -466,17 +409,11 @@ def ls_remotes(self, callbacks=None):
         return results
 
     def prune(self, callbacks=None):
-        """Perform a prune against this remote."""
-
-        remote_callbacks = ffi.new('git_remote_callbacks *')
-        C.git_remote_init_callbacks(remote_callbacks, C.GIT_REMOTE_CALLBACKS_VERSION)
-
-        if callbacks is None:
-            callbacks = RemoteCallbacks()
-        callbacks._fill_prune_callbacks(remote_callbacks)
-
-        err = C.git_remote_prune(self._remote, remote_callbacks)
-        check_error(err)
+        """Perform a prune against this remote.
+        """
+        with git_remote_callbacks(callbacks) as (remote_callbacks, cb):
+            err = C.git_remote_prune(self._remote, remote_callbacks)
+            check_error(err, cb)
 
     @property
     def refspec_count(self):
@@ -525,20 +462,11 @@ def push(self, specs, callbacks=None):
         specs : [str]
             Push refspecs to use.
         """
-        push_opts = ffi.new('git_push_options *')
-        err = C.git_push_init_options(push_opts, C.GIT_PUSH_OPTIONS_VERSION)
-
-        if callbacks is None:
-            callbacks = RemoteCallbacks()
-        callbacks._fill_push_options(push_opts)
-
-        # Build custom callback structure
-        try:
+        with git_push_options(callbacks) as (opts, cb):
             with StrArray(specs) as refspecs:
-                err = C.git_remote_push(self._remote, refspecs, push_opts)
-                check_error(err)
-        finally:
-            callbacks._self_handle = None
+                err = C.git_remote_push(self._remote, refspecs, opts)
+                check_error(err, cb)
+
 
 def get_credentials(fn, url, username, allowed):
     """Call fn and return the credentials object"""

pygit2/repository.py

@@ -40,11 +40,12 @@
 from ._pygit2 import Reference, Tree, Commit, Blob
 from ._pygit2 import InvalidSpecError
 
+from .callbacks import git_fetch_options
 from .config import Config
 from .errors import check_error
 from .ffi import ffi, C
 from .index import Index
-from .remote import RemoteCollection, RemoteCallbacks
+from .remote import RemoteCollection
 from .blame import Blame
 from .utils import to_bytes, StrArray
 from .submodule import Submodule
@@ -101,15 +102,12 @@ def update_submodules(self, submodules=None, init=False, callbacks=None):
         opts = ffi.new('git_submodule_update_options *')
         C.git_submodule_update_init_options(opts, C.GIT_SUBMODULE_UPDATE_OPTIONS_VERSION)
 
-        if callbacks is None:
-            callbacks = RemoteCallbacks()
-        callbacks._fill_fetch_options(opts.fetch_opts)
-
-        i = 1 if init else 0
-        for submodule in submodules:
-            submodule_instance = self.lookup_submodule(submodule)
-            err = C.git_submodule_update(submodule_instance._subm, i, opts)
-            check_error(err)
+        with git_fetch_options(callbacks, opts=opts.fetch_opts) as (_, cb):
+            i = 1 if init else 0
+            for submodule in submodules:
+                submodule_instance = self.lookup_submodule(submodule)
+                err = C.git_submodule_update(submodule_instance._subm, i, opts)
+                check_error(err, cb)
 
         return None