Internal: Refactoring callbacks code (cont.)

Now clone_repository callbacks are implemented the same way.

Author: jdavid

pygit2/__init__.py

@@ -31,7 +31,8 @@
 
 # High level API
 from .blame import Blame, BlameHunk
-from .callbacks import git_fetch_options, get_credentials, RemoteCallbacks
+from .callbacks import git_clone_options, git_fetch_options, get_credentials
+from .callbacks import Payload, RemoteCallbacks
 from .config import Config
 from .credentials import *
 from .errors import check_error, Passthrough
@@ -152,33 +153,6 @@ def init_repository(path, bare=False,
     # Ok
     return Repository(to_str(path))
 
-@ffi.def_extern()
-def _repository_create_cb(repo_out, path, bare, data):
-    d = ffi.from_handle(data)
-    try:
-        repository = d['repository_cb'](ffi.string(path), bare != 0)
-        # we no longer own the C object
-        repository._disown()
-        repo_out[0] = repository._repo
-    except Exception as e:
-        d['exception'] = e
-        return C.GIT_EUSER
-
-    return 0
-
-@ffi.def_extern()
-def _remote_create_cb(remote_out, repo, name, url, data):
-    d = ffi.from_handle(data)
-    try:
-        remote = d['remote_cb'](Repository._from_c(repo, False), ffi.string(name), ffi.string(url))
-        remote_out[0] = remote._remote
-        # we no longer own the C object
-        remote._remote = ffi.NULL
-    except Exception as e:
-        d['exception'] = e
-        return C.GIT_EUSER
-
-    return 0
 
 def clone_repository(
         url, path, bare=False, repository=None, remote=None,
@@ -218,41 +192,22 @@ def clone_repository(
         `pyclass:RemoteCallbacks`.
     """
 
-    opts = ffi.new('git_clone_options *')
-    crepo = ffi.new('git_repository **')
+    # Initialize payload
+    payload = Payload(repository=repository, remote=remote)
 
-    branch = checkout_branch or None
+    with git_clone_options(payload) as (opts, _):
+        opts.bare = bare
 
-    # Data, let's use a dict as we don't really want much more
-    d = {'repository_cb': repository,
-         'remote_cb': remote}
-    d_handle = ffi.new_handle(d)
+        #checkout_branch_ref = None
+        if checkout_branch:
+            checkout_branch_ref = ffi.new('char []', to_bytes(checkout_branch))
+            opts.checkout_branch = checkout_branch_ref
 
-    # Perform the initialization with the version we compiled
-    C.git_clone_options_init(opts, C.GIT_CLONE_OPTIONS_VERSION)
-
-    # We need to keep the ref alive ourselves
-    checkout_branch_ref = None
-    if branch:
-        checkout_branch_ref = ffi.new('char []', to_bytes(branch))
-        opts.checkout_branch = checkout_branch_ref
-
-    if repository:
-        opts.repository_cb = C._repository_create_cb
-        opts.repository_cb_payload = d_handle
-
-    if remote:
-        opts.remote_cb = C._remote_create_cb
-        opts.remote_cb_payload = d_handle
-
-    opts.bare = bare
-
-    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)
+        with git_fetch_options(callbacks, opts=opts.fetch_opts) as (_, cb):
+            crepo = ffi.new('git_repository **')
+            err = C.git_clone(crepo, to_bytes(url), to_bytes(path), opts)
+            payload.check_error()
+            check_error(err, cb)
 
     # Ok
     return Repository._from_c(crepo[0], owned=True)

pygit2/callbacks.py

@@ -55,6 +55,38 @@
 #    function, or raises exception from libgit2 error
 #
 
+class Payload:
+
+    def __init__(self, **kw):
+        for key, value in kw.items():
+            setattr(self, key, value)
+        self._stored_exception = None
+
+    def check_error(self):
+        if self._stored_exception is not None:
+            raise self._stored_exception
+
+
+@contextmanager
+def git_clone_options(payload, opts=None):
+    if opts is None:
+        opts = ffi.new('git_clone_options *')
+        C.git_clone_options_init(opts, C.GIT_CLONE_OPTIONS_VERSION)
+
+    handle = ffi.new_handle(payload)
+
+    # Plug callbacks
+    if payload.repository:
+        opts.repository_cb = C._repository_create_cb
+        opts.repository_cb_payload = handle
+    if payload.remote:
+        opts.remote_cb = C._remote_create_cb
+        opts.remote_cb_payload = handle
+
+    # Give back control
+    yield opts, payload
+
+
 @contextmanager
 def git_fetch_options(callbacks, opts=None):
     if callbacks is None:
@@ -153,16 +185,15 @@ def wrapper(*args):
             data._stored_exception = e
             return C.GIT_EUSER
 
-    return wrapper
+    return ffi.def_extern()(wrapper)
 
 
-@ffi.def_extern()
 @callback_proxy
 def _certificate_cb(cert_i, valid, host, data):
-    # We want to simulate what should happen if libgit2 supported pass-through for
-    # this callback. For SSH, 'valid' is always False, because it doesn't look
-    # at known_hosts, but we do want to let it through in order to do what libgit2 would
-    # if the callback were not set.
+    # We want to simulate what should happen if libgit2 supported pass-through
+    # for this callback. For SSH, 'valid' is always False, because it doesn't
+    # look at known_hosts, but we do want to let it through in order to do what
+    # libgit2 would if the callback were not set.
     try:
         is_ssh = cert_i.cert_type == C.GIT_CERT_HOSTKEY_LIBSSH2
 
@@ -185,7 +216,6 @@ def _certificate_cb(cert_i, valid, host, data):
     return 0
 
 
-@ffi.def_extern()
 @callback_proxy
 def _credentials_cb(cred_out, url, username, allowed, data):
     credentials = getattr(data, 'credentials', None)
@@ -197,7 +227,6 @@ def _credentials_cb(cred_out, url, username, allowed, data):
     return 0
 
 
-@ffi.def_extern()
 @callback_proxy
 def _push_update_reference_cb(ref, msg, data):
     push_update_reference = getattr(data, 'push_update_reference', None)
@@ -210,7 +239,28 @@ def _push_update_reference_cb(ref, msg, data):
     return 0
 
 
-@ffi.def_extern()
+@callback_proxy
+def _remote_create_cb(remote_out, repo, name, url, data):
+    from .repository import Repository
+
+    remote = data.remote(Repository._from_c(repo, False), ffi.string(name), ffi.string(url))
+    remote_out[0] = remote._remote
+    # we no longer own the C object
+    remote._remote = ffi.NULL
+
+    return 0
+
+
+@callback_proxy
+def _repository_create_cb(repo_out, path, bare, data):
+    repository = data.repository(ffi.string(path), bare != 0)
+    # we no longer own the C object
+    repository._disown()
+    repo_out[0] = repository._repo
+
+    return 0
+
+
 @callback_proxy
 def _sideband_progress_cb(string, length, data):
     progress = getattr(data, 'progress', None)
@@ -222,7 +272,6 @@ def _sideband_progress_cb(string, length, data):
     return 0
 
 
-@ffi.def_extern()
 @callback_proxy
 def _transfer_progress_cb(stats_ptr, data):
     from .remote import TransferProgress
@@ -235,7 +284,6 @@ def _transfer_progress_cb(stats_ptr, data):
     return 0
 
 
-@ffi.def_extern()
 @callback_proxy
 def _update_tips_cb(refname, a, b, data):
     update_tips = getattr(data, 'update_tips', None)