Support OnProgress in Push / PushOptions

[sitereactor]

Feature Request:

When pushing to a remote repository its currently possible to handle callbacks for OnPushStatusError OnPushTransferProgress and OnPackBuilderProgress but there is currently no way to handle the OnProgress callback similar to the one that is available for Fetch (through FetchOptions).

This feature request comes out of this question posted on StackOverflow:
http://stackoverflow.com/questions/22149891/remote-response-when-pushing-with-libgit2sharp

I've been looking through the code to get an idea of how this should be implemented, and I'll post back with a pull request and/or additional questions once I get into it.

[sitereactor]

Okay, had a chance to look into the codebase.

I see both Push and Fetch on the Network class uses RemoteCallbacks but while Fetch passes the FetchOptions to the ctor only Credentials are passed to RemoteCallbacks in the Push method.
I can't figure out if Proxy.git_remote_set_callbacks(remoteHandle, ref gitCallbacks); is supposed to work the same way for both Push and Fetch. Looking at the similarities I would have assumed that I could simply pass an OnProgress handler to RemoteCallbacks and the approriate git callbacks would be created for Push. But it doesn't seem to have any effect. So maybe the OnProgress handler / callback needs to be handled differently in Push then its currently done in Fetch.
I tried looking through the C code in libgit2 for any clues, but can't find anything specific to a progress callback for Push.

For reference here are the methods I have been looking at:
DoFetch line 99-134 in the Network class.
Push line 236-322 also in the Network class.

Usage of RemoteCallbacks in Fetch and in Push

@nulltoken Would you be able to provide any pointers on this?

[nulltoken]

Hmmm. Your approach looks sane.

@carlosmn does the progress callback is also leveraged by push()? I can find traces in the smart protocol implementation, but I've never tested it.

[carlosmn]

TBH I forget which network implementations I've tested, so I might not have tested out the libgit2 one. We do have a commit which claims to add side-band-64k to push. I suppose it's possible it might not have wired up the progress reporting.

[sitereactor]

Are there any tests that cover the progress callback during Fetch in libgit2, which I can look at to investigate and test for possible differences?

Update: Found this example which shows that progress works with Fetch, but haven't found anything similar for Push.

[sitereactor]

This must be the pull request you referred to right @carlosmn libgit2/libgit2#1410

I'll try to digg through this.

[sitereactor]

@jamill I see you worked on Push progress reporting in 981f439 but as far as I can tell it only shows errors (OnPushStatusError) and not other responses from the remote like:

remote: Updating branch 'master'.
remote: Updating submodules.
remote: Preparing deployment for commit id '3fe0a458ac'.
remote: Generating deployment script.
remote: Running deployment command...
remote: Handling Basic Web Site deployment.

Is that correct? And if so have you looked at the other type of progress via git_remote_set_callbacks - in Push as mentioned above?

[jamill]

I did not try to expose these server messages. From what I remember of the push logic, I don't think libgit2 currently reports this progress.

[uluhonolulu]

Not sure if this is the one, but what about libgit2/libgit2#2284? Since it's been merged, are the progress events reported in Push now? The 0.21.0.176 doesn't seem to have them..

[sitereactor]

I just tested this and the OnProgress handler now works as expected when I add it to the PushOptions. Thanks for the heads up @uluhonolulu

I'd be happy to submit a PR for this, but not sure how you want it tested @nulltoken ? I see the OnProgress handler is covered in the Fetch tests, but would I need to do anything special to cover it in the PushFixture?

[sergeyzwezdin]

@sitereactor did you have a chance to PR this feature?

[pablonardone]

Is this actually merged with the current version? Because I don't see the OnProgress callback in the PushOptons, and this could be related to the issue #1838
Thanks

[sitereactor]

I wish I had submitted a pull request for this a long time ago as now I really need it 😅
Anyway, if anyone else is still looking for this here is a PR: #1876

I hope its something thats easy to get through as its a simply change using functionality already available.