Merge remote-tracking branch 'jorio/push_transfer_progress'

Author: jdavid

pygit2/callbacks.py

@@ -183,15 +183,30 @@ def certificate_check(self, certificate, valid, host):
 
     def transfer_progress(self, stats):
         """
-        Transfer progress callback. Override with your own function to report
-        transfer progress.
+        During the download of new data, this will be regularly called with
+        the indexer's progress.
+
+        Override with your own function to report transfer progress.
 
         Parameters:
 
         stats : TransferProgress
             The progress up to now.
         """
 
+    def push_transfer_progress(
+        self, objects_pushed: int, total_objects: int, bytes_pushed: int
+    ):
+        """
+        During the upload portion of a push, this will be regularly called
+        with progress information.
+
+        Be aware that this is called inline with pack building operations,
+        so performance may be affected.
+
+        Override with your own function to report push transfer progress.
+        """
+
     def update_tips(self, refname, old, new):
         """
         Update tips callback. Override with your own function to report
@@ -370,6 +385,13 @@ def git_push_options(payload, opts=None):
     opts.callbacks.credentials = C._credentials_cb
     opts.callbacks.certificate_check = C._certificate_check_cb
     opts.callbacks.push_update_reference = C._push_update_reference_cb
+    # Per libgit2 sources, push_transfer_progress may incur a performance hit.
+    # So, set it only if the user has overridden the no-op stub.
+    if (
+        type(payload).push_transfer_progress
+        is not RemoteCallbacks.push_transfer_progress
+    ):
+        opts.callbacks.push_transfer_progress = C._push_transfer_progress_cb
     # Payload
     handle = ffi.new_handle(payload)
     opts.callbacks.payload = handle
@@ -554,6 +576,16 @@ def _transfer_progress_cb(stats_ptr, data):
     return 0
 
 
+@libgit2_callback
+def _push_transfer_progress_cb(current, total, bytes_pushed, payload):
+    push_transfer_progress = getattr(payload, 'push_transfer_progress', None)
+    if not push_transfer_progress:
+        return 0
+
+    push_transfer_progress(current, total, bytes_pushed)
+    return 0
+
+
 @libgit2_callback
 def _update_tips_cb(refname, a, b, data):
     update_tips = getattr(data, 'update_tips', None)

pygit2/decl/callbacks.h

@@ -38,6 +38,12 @@ extern "Python" int _transfer_progress_cb(
     const git_indexer_progress *stats,
     void *payload);
 
+extern "Python" int _push_transfer_progress_cb(
+    unsigned int objects_pushed,
+    unsigned int total_objects,
+    size_t bytes_pushed,
+    void *payload);
+
 extern "Python" int _update_tips_cb(
 	const char *refname,
 	const git_oid *a,

test/test_remote.py

@@ -252,8 +252,8 @@ def test_fetch_depth_one(testrepo):
 
 def test_transfer_progress(emptyrepo):
     class MyCallbacks(pygit2.RemoteCallbacks):
-        def transfer_progress(emptyrepo, stats):
-            emptyrepo.tp = stats
+        def transfer_progress(self, stats):
+            self.tp = stats
 
     callbacks = MyCallbacks()
     remote = emptyrepo.remotes[0]
@@ -362,6 +362,59 @@ def test_push_when_up_to_date_succeeds(origin, clone, remote):
     assert origin_tip == clone_tip
 
 
+def test_push_transfer_progress(origin, clone, remote):
+    tip = clone[clone.head.target]
+    new_tip_id = clone.create_commit(
+        'refs/heads/master',
+        tip.author,
+        tip.author,
+        'empty commit',
+        tip.tree.id,
+        [tip.id],
+    )
+
+    # NOTE: We're currently not testing bytes_pushed due to a bug in libgit2
+    # 1.9.0: it passes a junk value for bytes_pushed when pushing to a remote
+    # on the local filesystem, as is the case in this unit test. (When pushing
+    # to a remote over the network, the value is correct.)
+    class MyCallbacks(pygit2.RemoteCallbacks):
+        def push_transfer_progress(self, objects_pushed, total_objects, bytes_pushed):
+            self.objects_pushed = objects_pushed
+            self.total_objects = total_objects
+
+    assert origin.branches['master'].target == tip.id
+
+    callbacks = MyCallbacks()
+    remote.push(['refs/heads/master'], callbacks=callbacks)
+    assert callbacks.objects_pushed == 1
+    assert callbacks.total_objects == 1
+    assert origin.branches['master'].target == new_tip_id
+
+
+def test_push_interrupted_from_callbacks(origin, clone, remote):
+    tip = clone[clone.head.target]
+    clone.create_commit(
+        'refs/heads/master',
+        tip.author,
+        tip.author,
+        'empty commit',
+        tip.tree.id,
+        [tip.id],
+    )
+
+    class MyCallbacks(pygit2.RemoteCallbacks):
+        def push_transfer_progress(self, objects_pushed, total_objects, bytes_pushed):
+            raise InterruptedError('retreat! retreat!')
+
+    assert origin.branches['master'].target == tip.id
+
+    callbacks = MyCallbacks()
+    with pytest.raises(InterruptedError, match='retreat! retreat!'):
+        remote.push(['refs/heads/master'], callbacks=callbacks)
+
+    assert origin.branches['master'].target == tip.id
+
+
 def test_push_non_fast_forward_commits_to_remote_fails(origin, clone, remote):
     tip = origin[origin.head.target]
     origin.create_commit(
@@ -386,22 +439,31 @@ def test_push_non_fast_forward_commits_to_remote_fails(origin, clone, remote):
         remote.push(['refs/heads/master'])
 
 
-@patch.object(pygit2.callbacks, 'RemoteCallbacks')
-def test_push_options(mock_callbacks, origin, clone, remote):
-    remote.push(['refs/heads/master'])
-    remote_push_options = mock_callbacks.return_value.push_options.remote_push_options
+def test_push_options(origin, clone, remote):
+    from pygit2 import RemoteCallbacks
+
+    callbacks = RemoteCallbacks()
+    remote.push(['refs/heads/master'], callbacks)
+    remote_push_options = callbacks.push_options.remote_push_options
     assert remote_push_options.count == 0
 
-    remote.push(['refs/heads/master'], push_options=[])
-    remote_push_options = mock_callbacks.return_value.push_options.remote_push_options
+    callbacks = RemoteCallbacks()
+    remote.push(['refs/heads/master'], callbacks, push_options=[])
+    remote_push_options = callbacks.push_options.remote_push_options
     assert remote_push_options.count == 0
 
-    remote.push(['refs/heads/master'], push_options=['foo'])
-    remote_push_options = mock_callbacks.return_value.push_options.remote_push_options
+    callbacks = RemoteCallbacks()
+    # Local remotes don't support push_options, so pushing will raise an error.
+    # However, push_options should still be set in RemoteCallbacks.
+    with pytest.raises(pygit2.GitError, match='push-options not supported by remote'):
+        remote.push(['refs/heads/master'], callbacks, push_options=['foo'])
+    remote_push_options = callbacks.push_options.remote_push_options
     assert remote_push_options.count == 1
     # strings pointed to by remote_push_options.strings[] are already freed
 
-    remote.push(['refs/heads/master'], push_options=['Option A', 'Option B'])
-    remote_push_options = mock_callbacks.return_value.push_options.remote_push_options
+    callbacks = RemoteCallbacks()
+    with pytest.raises(pygit2.GitError, match='push-options not supported by remote'):
+        remote.push(['refs/heads/master'], callbacks, push_options=['Opt A', 'Opt B'])
+    remote_push_options = callbacks.push_options.remote_push_options
     assert remote_push_options.count == 2
     # strings pointed to by remote_push_options.strings[] are already freed

test/test_repository.py

@@ -99,6 +99,8 @@ def __init__(self):
             super().__init__()
             self.conflicting_paths = set()
             self.updated_paths = set()
+            self.completed_steps = -1
+            self.total_steps = -1
 
         def checkout_notify_flags(self) -> CheckoutNotify:
             return CheckoutNotify.CONFLICT | CheckoutNotify.UPDATED
@@ -109,12 +111,17 @@ def checkout_notify(self, why, path, baseline, target, workdir):
             elif why == CheckoutNotify.UPDATED:
                 self.updated_paths.add(path)
 
+        def checkout_progress(self, path: str, completed_steps: int, total_steps: int):
+            self.completed_steps = completed_steps
+            self.total_steps = total_steps
+
     # checkout i18n with conflicts and default strategy should not be possible
     callbacks = MyCheckoutCallbacks()
     with pytest.raises(pygit2.GitError):
         testrepo.checkout(ref_i18n, callbacks=callbacks)
     # make sure the callbacks caught that
     assert {'bye.txt'} == callbacks.conflicting_paths
+    assert -1 == callbacks.completed_steps  # shouldn't have done anything
 
     # checkout i18n with GIT_CHECKOUT_FORCE
     head = testrepo.head
@@ -125,6 +132,8 @@ def checkout_notify(self, why, path, baseline, target, workdir):
     # make sure the callbacks caught the files affected by the checkout
     assert set() == callbacks.conflicting_paths
     assert {'bye.txt', 'new'} == callbacks.updated_paths
+    assert callbacks.completed_steps > 0
+    assert callbacks.completed_steps == callbacks.total_steps
 
 
 def test_checkout_aborted_from_callbacks(testrepo):