From 91efd23b95a750ba0b2f84baa991a59ae334977d Mon Sep 17 00:00:00 2001 From: Iliyas Jorio Date: Sat, 1 Feb 2025 12:39:24 +0100 Subject: [PATCH 1/3] Test CheckoutCallbacks.checkout_progress --- test/test_repository.py | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/test/test_repository.py b/test/test_repository.py index ddfc1186..e7cd9313 100644 --- a/test/test_repository.py +++ b/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): From 3d02993604f68f9906255a3ff71d8f8a69d26107 Mon Sep 17 00:00:00 2001 From: Iliyas Jorio Date: Sat, 1 Feb 2025 12:41:15 +0100 Subject: [PATCH 2/3] Add RemoteCallbacks.push_transfer_progress --- pygit2/callbacks.py | 36 ++++++++++++++++++++++++-- pygit2/decl/callbacks.h | 6 +++++ test/test_remote.py | 57 +++++++++++++++++++++++++++++++++++++++-- 3 files changed, 95 insertions(+), 4 deletions(-) diff --git a/pygit2/callbacks.py b/pygit2/callbacks.py index 855ef12c..25514764 100644 --- a/pygit2/callbacks.py +++ b/pygit2/callbacks.py @@ -183,8 +183,10 @@ 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: @@ -192,6 +194,19 @@ def transfer_progress(self, stats): 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) diff --git a/pygit2/decl/callbacks.h b/pygit2/decl/callbacks.h index f6991a5b..9d5409de 100644 --- a/pygit2/decl/callbacks.h +++ b/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, diff --git a/test/test_remote.py b/test/test_remote.py index 0de2dc0e..4fdb3a3b 100644 --- a/test/test_remote.py +++ b/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( From 7bccdffe6c360f3b5327f58df63f407aa8514595 Mon Sep 17 00:00:00 2001 From: Iliyas Jorio Date: Sat, 1 Feb 2025 13:36:29 +0100 Subject: [PATCH 3/3] Rewrite test_push_options without mocking RemoteCallbacks This has been spurred by the introduction of RemoteCallbacks.push_transfer_progress. --- test/test_remote.py | 29 +++++++++++++++++++---------- 1 file changed, 19 insertions(+), 10 deletions(-) diff --git a/test/test_remote.py b/test/test_remote.py index 4fdb3a3b..8b4fab4d 100644 --- a/test/test_remote.py +++ b/test/test_remote.py @@ -439,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