From bd736e2d130bd441fe7e767d9a7289d720bd0aea Mon Sep 17 00:00:00 2001 From: Jameson Miller Date: Fri, 18 Oct 2013 14:56:24 -0400 Subject: [PATCH] Clean up cancellation patterns in callbacks and other small cleanups. --- LibGit2Sharp.Tests/CloneFixture.cs | 2 +- LibGit2Sharp.Tests/PushFixture.cs | 2 +- .../TestHelpers/ExpectedFetchState.cs | 6 +-- LibGit2Sharp/CheckoutCallbacks.cs | 6 +-- LibGit2Sharp/Core/PackbuilderCallbacks.cs | 3 +- LibGit2Sharp/Core/Proxy.cs | 14 ++++++ .../Core/PushTransferProgressCallbacks.cs | 2 +- LibGit2Sharp/Handlers.cs | 17 +++---- LibGit2Sharp/Network.cs | 8 ++-- LibGit2Sharp/NetworkExtensions.cs | 2 - LibGit2Sharp/PushOptions.cs | 8 +++- LibGit2Sharp/RemoteCallbacks.cs | 45 +++---------------- LibGit2Sharp/Repository.cs | 2 +- LibGit2Sharp/RepositoryExtensions.cs | 4 +- 14 files changed, 47 insertions(+), 74 deletions(-) diff --git a/LibGit2Sharp.Tests/CloneFixture.cs b/LibGit2Sharp.Tests/CloneFixture.cs index 800ccc2f..10e01034 100644 --- a/LibGit2Sharp.Tests/CloneFixture.cs +++ b/LibGit2Sharp.Tests/CloneFixture.cs @@ -116,7 +116,7 @@ namespace LibGit2Sharp.Tests var scd = BuildSelfCleaningDirectory(); Repository.Clone(url, scd.DirectoryPath, - onTransferProgress: _ => { transferWasCalled = true; return 0; }, + onTransferProgress: _ => { transferWasCalled = true; return true; }, onCheckoutProgress: (a, b, c) => checkoutWasCalled = true); Assert.True(transferWasCalled); diff --git a/LibGit2Sharp.Tests/PushFixture.cs b/LibGit2Sharp.Tests/PushFixture.cs index 2b836e45..e4afc803 100644 --- a/LibGit2Sharp.Tests/PushFixture.cs +++ b/LibGit2Sharp.Tests/PushFixture.cs @@ -61,7 +61,7 @@ namespace LibGit2Sharp.Tests public void CanPushABranchTrackingAnUpstreamBranch() { bool packBuilderCalled = false; - Handlers.PackBuilderProgressHandler packBuilderCb = (x, y, z) => { packBuilderCalled = true; return false; }; + Handlers.PackBuilderProgressHandler packBuilderCb = (x, y, z) => { packBuilderCalled = true; return true; }; AssertPush(repo => repo.Network.Push(repo.Head)); AssertPush(repo => repo.Network.Push(repo.Branches["master"])); diff --git a/LibGit2Sharp.Tests/TestHelpers/ExpectedFetchState.cs b/LibGit2Sharp.Tests/TestHelpers/ExpectedFetchState.cs index 722b8873..78a3e682 100644 --- a/LibGit2Sharp.Tests/TestHelpers/ExpectedFetchState.cs +++ b/LibGit2Sharp.Tests/TestHelpers/ExpectedFetchState.cs @@ -78,8 +78,8 @@ namespace LibGit2Sharp.Tests.TestHelpers /// Name of reference being updated. /// Old ID of reference. /// New ID of reference. - /// 0 on success; a negative value to abort the process. - public int RemoteUpdateTipsHandler(string referenceName, ObjectId oldId, ObjectId newId) + /// True to continue, false to cancel. + public bool RemoteUpdateTipsHandler(string referenceName, ObjectId oldId, ObjectId newId) { // assert that we have not seen this reference before Assert.DoesNotContain(referenceName, ObservedReferenceUpdates.Keys); @@ -97,7 +97,7 @@ namespace LibGit2Sharp.Tests.TestHelpers Assert.Equal(referenceUpdate.NewId, newId); } - return 0; + return true; } /// diff --git a/LibGit2Sharp/CheckoutCallbacks.cs b/LibGit2Sharp/CheckoutCallbacks.cs index 93f2b3b7..fa8817b8 100644 --- a/LibGit2Sharp/CheckoutCallbacks.cs +++ b/LibGit2Sharp/CheckoutCallbacks.cs @@ -100,14 +100,14 @@ namespace LibGit2Sharp IntPtr workdirPtr, IntPtr payloadPtr) { - int result = 0; + bool result = true; if (this.onCheckoutNotify != null) { FilePath path = LaxFilePathMarshaler.FromNative(pathPtr) ?? FilePath.Empty; - result = onCheckoutNotify(path.Native, why) ? 0 : 1; + result = onCheckoutNotify(path.Native, why); } - return result; + return Proxy.ConvertResultToCancelFlag(result); } } } diff --git a/LibGit2Sharp/Core/PackbuilderCallbacks.cs b/LibGit2Sharp/Core/PackbuilderCallbacks.cs index 880748c3..7c496654 100644 --- a/LibGit2Sharp/Core/PackbuilderCallbacks.cs +++ b/LibGit2Sharp/Core/PackbuilderCallbacks.cs @@ -3,7 +3,6 @@ using LibGit2Sharp.Handlers; namespace LibGit2Sharp.Core { - // internal class PackbuilderCallbacks { private readonly PackBuilderProgressHandler onPackBuilderProgress; @@ -33,7 +32,7 @@ namespace LibGit2Sharp.Core private int OnGitPackBuilderProgress(int stage, uint current, uint total, IntPtr payload) { - return onPackBuilderProgress((PackBuilderStage) stage, (int)current, (int)total) ? -1 : 0; + return Proxy.ConvertResultToCancelFlag(onPackBuilderProgress((PackBuilderStage)stage, (int)current, (int)total)); } } } diff --git a/LibGit2Sharp/Core/Proxy.cs b/LibGit2Sharp/Core/Proxy.cs index 3cd16b8b..86a54228 100644 --- a/LibGit2Sharp/Core/Proxy.cs +++ b/LibGit2Sharp/Core/Proxy.cs @@ -1134,6 +1134,7 @@ namespace LibGit2Sharp.Core return handle; } } + public static void git_push_set_callbacks( PushSafeHandle push, NativeMethods.git_push_transfer_progress pushTransferProgress, @@ -2524,6 +2525,19 @@ namespace LibGit2Sharp.Core { typeof(bool), value => git_config_parse_bool(value) }, { typeof(string), value => value }, }; + + /// + /// Helper method for consistent conversion of return value on + /// Callbacks that support cancellation from bool to native type. + /// True indicates that function should continue, false indicates + /// user wants to cancel. + /// + /// + /// + internal static int ConvertResultToCancelFlag(bool result) + { + return result ? 0 : -1; + } } } // ReSharper restore InconsistentNaming diff --git a/LibGit2Sharp/Core/PushTransferProgressCallbacks.cs b/LibGit2Sharp/Core/PushTransferProgressCallbacks.cs index b5cee842..2d40bf68 100644 --- a/LibGit2Sharp/Core/PushTransferProgressCallbacks.cs +++ b/LibGit2Sharp/Core/PushTransferProgressCallbacks.cs @@ -32,7 +32,7 @@ namespace LibGit2Sharp.Core private int OnGitTransferProgress(uint current, uint total, UIntPtr bytes, IntPtr payload) { - return onPushTransferProgress((int)current, (int)total, (long)bytes) ? -1 : 0; + return Proxy.ConvertResultToCancelFlag(onPushTransferProgress((int)current, (int)total, (long)bytes)); } } } diff --git a/LibGit2Sharp/Handlers.cs b/LibGit2Sharp/Handlers.cs index 85e0dd9e..f6a798cb 100644 --- a/LibGit2Sharp/Handlers.cs +++ b/LibGit2Sharp/Handlers.cs @@ -18,20 +18,15 @@ /// Name of the updated reference. /// Old ID of the reference. /// New ID of the reference. - /// Return negative integer to cancel. - public delegate int UpdateTipsHandler(string referenceName, ObjectId oldId, ObjectId newId); - - /// - /// Delegate definition to handle Completion callback. - /// - public delegate int CompletionHandler(RemoteCompletionType remoteCompletionType); + /// True to continue, false to cancel. + public delegate bool UpdateTipsHandler(string referenceName, ObjectId oldId, ObjectId newId); /// /// Delegate definition for transfer progress callback. /// /// The object containing progress information. - /// Return negative integer to cancel. - public delegate int TransferProgressHandler(TransferProgress progress); + /// True to continue, false to cancel. + public delegate bool TransferProgressHandler(TransferProgress progress); /// /// Delegate definition for callback reporting push network progress. @@ -39,7 +34,7 @@ /// The current number of objects sent to server. /// The total number of objects to send to the server. /// The number of bytes sent to the server. - /// True to cancel. + /// True to continue, false to cancel. public delegate bool PushTransferProgressHandler(int current, int total, long bytes); /// @@ -48,7 +43,7 @@ /// The current stage progress is being reported for. /// The current number of objects processed in this this stage. /// The total number of objects to process for the current stage. - /// True to cancel. + /// True to continue, false to cancel. public delegate bool PackBuilderProgressHandler(PackBuilderStage stage, int current, int total); /// diff --git a/LibGit2Sharp/Network.cs b/LibGit2Sharp/Network.cs index 7b69a37b..d74fb8cb 100644 --- a/LibGit2Sharp/Network.cs +++ b/LibGit2Sharp/Network.cs @@ -95,7 +95,6 @@ namespace LibGit2Sharp /// The remote to fetch /// Optional parameter indicating what tags to download. /// Progress callback. Corresponds to libgit2 progress callback. - /// Completion callback. Corresponds to libgit2 completion callback. /// UpdateTips callback. Corresponds to libgit2 update_tips callback. /// Callback method that transfer progress will be reported through. /// Reports the client's state regarding the received and processed (bytes, objects) from the server. @@ -104,7 +103,6 @@ namespace LibGit2Sharp Remote remote, TagFetchMode? tagFetchMode = null, ProgressHandler onProgress = null, - CompletionHandler onCompletion = null, UpdateTipsHandler onUpdateTips = null, TransferProgressHandler onTransferProgress = null, Credentials credentials = null) @@ -113,7 +111,7 @@ namespace LibGit2Sharp using (RemoteSafeHandle remoteHandle = Proxy.git_remote_load(repository.Handle, remote.Name, true)) { - var callbacks = new RemoteCallbacks(onProgress, onTransferProgress, onCompletion, onUpdateTips, credentials); + var callbacks = new RemoteCallbacks(onProgress, onTransferProgress, onUpdateTips, credentials); GitRemoteCallbacks gitCallbacks = callbacks.GenerateCallbacks(); if (tagFetchMode.HasValue) @@ -212,7 +210,7 @@ namespace LibGit2Sharp // Load the remote. using (RemoteSafeHandle remoteHandle = Proxy.git_remote_load(repository.Handle, remote.Name, true)) { - var callbacks = new RemoteCallbacks(null, null, null, null, pushOptions.Credentials); + var callbacks = new RemoteCallbacks(null, null, null, pushOptions.Credentials); GitRemoteCallbacks gitCallbacks = callbacks.GenerateCallbacks(); Proxy.git_remote_set_callbacks(remoteHandle, ref gitCallbacks); @@ -228,7 +226,7 @@ namespace LibGit2Sharp NativeMethods.git_push_transfer_progress pushProgress = pushTransferCallbacks.GenerateCallback(); NativeMethods.git_packbuilder_progress packBuilderProgress = packBuilderCallbacks.GenerateCallback(); - + Proxy.git_push_set_callbacks(pushHandle, pushProgress, packBuilderProgress); // Set push options. diff --git a/LibGit2Sharp/NetworkExtensions.cs b/LibGit2Sharp/NetworkExtensions.cs index 943ecac8..6016aafa 100644 --- a/LibGit2Sharp/NetworkExtensions.cs +++ b/LibGit2Sharp/NetworkExtensions.cs @@ -16,13 +16,11 @@ namespace LibGit2Sharp /// /// The being worked with. /// The branch to push. - /// Handler for reporting failed push updates. /// controlling push behavior /// Throws if either the Remote or the UpstreamBranchCanonicalName is not set. public static void Push( this Network network, Branch branch, - PushStatusErrorHandler onPushStatusError = null, PushOptions pushOptions = null) { network.Push(new[] { branch }, pushOptions); diff --git a/LibGit2Sharp/PushOptions.cs b/LibGit2Sharp/PushOptions.cs index 5306f851..db998ff2 100644 --- a/LibGit2Sharp/PushOptions.cs +++ b/LibGit2Sharp/PushOptions.cs @@ -27,12 +27,16 @@ namespace LibGit2Sharp public PushStatusErrorHandler OnPushStatusError { get; set; } /// - /// Delegate to report push network transfer progress. + /// Delegate that progress updates of the network transfer portion of push + /// will be reported through. The frequency of progress updates will not + /// be more than once every 0.5 seconds (in general). /// public PushTransferProgressHandler OnPushTransferProgress { get; set; } /// - /// Delagate to report pack builder progress. + /// Delegate that progress updates of the pack building portion of push + /// will be reported through. The frequency of progress updates will not + /// be more than once every 0.5 seconds (in general). /// public PackBuilderProgressHandler OnPackBuilderProgress { get; set; } } diff --git a/LibGit2Sharp/RemoteCallbacks.cs b/LibGit2Sharp/RemoteCallbacks.cs index 5815ed02..962a5d3a 100644 --- a/LibGit2Sharp/RemoteCallbacks.cs +++ b/LibGit2Sharp/RemoteCallbacks.cs @@ -15,13 +15,11 @@ namespace LibGit2Sharp internal RemoteCallbacks( ProgressHandler onProgress = null, TransferProgressHandler onDownloadProgress = null, - CompletionHandler onCompletion = null, UpdateTipsHandler onUpdateTips = null, Credentials credentials = null) { Progress = onProgress; DownloadTransferProgress = onDownloadProgress; - Completion = onCompletion; UpdateTips = onUpdateTips; Credentials = credentials; } @@ -38,11 +36,6 @@ namespace LibGit2Sharp /// private readonly UpdateTipsHandler UpdateTips; - /// - /// Completion callback. Corresponds to libgit2 Completion callback. - /// - private readonly CompletionHandler Completion; - /// /// Managed delegate to be called in response to a git_transfer_progress_callback callback from libgit2. /// This will in turn call the user provided delegate. @@ -70,11 +63,6 @@ namespace LibGit2Sharp callbacks.update_tips = GitUpdateTipsHandler; } - if (Completion != null) - { - callbacks.completion = GitCompletionHandler; - } - if (Credentials != null) { callbacks.acquire_credentials = GitCredentialHandler; @@ -122,36 +110,15 @@ namespace LibGit2Sharp private int GitUpdateTipsHandler(IntPtr str, ref GitOid oldId, ref GitOid newId, IntPtr data) { UpdateTipsHandler onUpdateTips = UpdateTips; - int result = 0; + bool shouldContinue = true; if (onUpdateTips != null) { string refName = LaxUtf8Marshaler.FromNative(str); - result = onUpdateTips(refName, oldId, newId); + shouldContinue = onUpdateTips(refName, oldId, newId); } - return result; - } - - /// - /// Handler for libgit2 completion callback. Converts values - /// received from libgit2 callback to more suitable types - /// and calls delegate provided by LibGit2Sharp consumer. - /// - /// Which operation completed. - /// IntPtr to optional payload passed back to the callback. - /// 0 on success; a negative value to abort the process. - private int GitCompletionHandler(RemoteCompletionType remoteCompletionType, IntPtr data) - { - CompletionHandler completion = Completion; - int result = 0; - - if (completion != null) - { - result = completion(remoteCompletionType); - } - - return result; + return Proxy.ConvertResultToCancelFlag(shouldContinue); } /// @@ -162,14 +129,14 @@ namespace LibGit2Sharp /// the result of the wrapped private int GitDownloadTransferProgressHandler(ref GitTransferProgress progress, IntPtr payload) { - int result = 0; + bool shouldContinue = true; if (DownloadTransferProgress != null) { - result = DownloadTransferProgress(new TransferProgress(progress)); + shouldContinue = DownloadTransferProgress(new TransferProgress(progress)); } - return result; + return Proxy.ConvertResultToCancelFlag(shouldContinue); } private int GitCredentialHandler(out IntPtr cred, IntPtr url, IntPtr username_from_url, uint types, IntPtr payload) diff --git a/LibGit2Sharp/Repository.cs b/LibGit2Sharp/Repository.cs index 93b726f8..e9914562 100644 --- a/LibGit2Sharp/Repository.cs +++ b/LibGit2Sharp/Repository.cs @@ -560,7 +560,7 @@ namespace LibGit2Sharp { CheckoutCallbacks checkoutCallbacks = CheckoutCallbacks.GenerateCheckoutCallbacks(onCheckoutProgress, null); - var callbacks = new RemoteCallbacks(null, onTransferProgress, null, null, credentials); + var callbacks = new RemoteCallbacks(null, onTransferProgress, null, credentials); GitRemoteCallbacks gitCallbacks = callbacks.GenerateCallbacks(); var cloneOpts = new GitCloneOptions diff --git a/LibGit2Sharp/RepositoryExtensions.cs b/LibGit2Sharp/RepositoryExtensions.cs index f00e6971..30e8bb8f 100644 --- a/LibGit2Sharp/RepositoryExtensions.cs +++ b/LibGit2Sharp/RepositoryExtensions.cs @@ -227,7 +227,6 @@ namespace LibGit2Sharp /// The name of the to fetch from. /// Optional parameter indicating what tags to download. /// Progress callback. Corresponds to libgit2 progress callback. - /// Completion callback. Corresponds to libgit2 completion callback. /// UpdateTips callback. Corresponds to libgit2 update_tips callback. /// Callback method that transfer progress will be reported through. /// Reports the client's state regarding the received and processed (bytes, objects) from the server. @@ -235,7 +234,6 @@ namespace LibGit2Sharp public static void Fetch(this IRepository repository, string remoteName, TagFetchMode tagFetchMode = TagFetchMode.Auto, ProgressHandler onProgress = null, - CompletionHandler onCompletion = null, UpdateTipsHandler onUpdateTips = null, TransferProgressHandler onTransferProgress = null, Credentials credentials = null) @@ -244,7 +242,7 @@ namespace LibGit2Sharp Ensure.ArgumentNotNullOrEmptyString(remoteName, "remoteName"); Remote remote = repository.Network.Remotes.RemoteForName(remoteName, true); - repository.Network.Fetch(remote, tagFetchMode, onProgress, onCompletion, onUpdateTips, + repository.Network.Fetch(remote, tagFetchMode, onProgress, onUpdateTips, onTransferProgress, credentials); }