Skip to content

Expose libgit2 fetch functionality. #206

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 1 commit into from
Closed

Conversation

jamill
Copy link
Member

@jamill jamill commented Aug 27, 2012

This pull request implements the basics needed to expose fetch through Libgit2Sharp.

I see there is issue #65 indicating that there might already be some other work done on fetch, and that the code is being polished, but this was over 4 months ago and I have not seen it in the main branch.

I did not implement the callback on git_remote_update_tips, as this function seems to be changing in the latest version of libgit2 (possibly why fetch has not made it into the main branch on LibGit2Sharp?).

Thanks,
Jameson

@travisbot
Copy link

This pull request passes (merged 91de647 into 58932d0).

@nulltoken
Copy link
Member

@jamill Wow! That's not a little PR, is it? ;-) Very nice job!

I see there is issue #65 indicating that there might already be some other work done on fetch, and that the code is being polished, but this was over 4 months ago and I have not seen it in the main branch.

Yes, it's true. In fact, my dear @carlosmn has kind of a habit of rewriting the libgit2 network stack once every two months, which makes it kind of a moving tagret... 😄

More seriously, the same awesome @carlosmn is once again showing that there are so many ways to transmit some bytes over a wire, by leveraging WinHTTP on Windows. You can follow his work on libgit2/libgit2#901.

I was thinking that, maybe, it would be a good idea to wait until he's done, then bind against this implementation.

As LibGit2Sharp consumers are mostly Windows based, this could provide libgit2 with a quick feedback loop on potential issues and dragons out there. Moreover, some network callbacks have been recently added which may be quite handy.

How do you feel about this?

@jamill
Copy link
Member Author

jamill commented Aug 29, 2012

@nulltoken Thanks!

I am fine with waiting until the WinHTTP work is complete. Do you have an idea when (roughly) the libgit2 library with the WinHTTP work and network callbacks will make it to LibGit2Sharp? I could build libgit2 locally, but would prefer to work against the version included with LibGit2Sharp :)

Alternatively, I would also be fine submitting this now and then following up once the libgit2 changes make it to LibGit2Sharp (if, for instance, it will be a while before LibGit2Sharp gets these new additions).

@nulltoken
Copy link
Member

I am fine with waiting until the WinHTTP work is complete. Do you have an idea when (roughly) the libgit2 library with the WinHTTP work and network callbacks will make it to LibGit2Sharp?

One option would be to wait for the branch to be merged.

However, @ben is very eager as well to start on binding git_clone() and make it available to LibGit2Sharp.

So, here's another option:

  • @ben will build a temporary libgit2 build (x86/amd64) based on the https://github.com/carlosmn/libgit2/tree/winhttp branch
  • He will make this build available on a dedicated branch in the LibGit2Sharp project
  • You would have you to rebase this PR on top this new branch
  • You may (will?) encounter some bugs as the branch hasn't been thoroughly tested yet. However, I'm pretty sure @carlosmn will be happy to benefit from some additional testing and will gladly work on any reproducible issue you'd come up with.

Note: as Fetch and Clone are pretty closely functions, you may have to team up with @ben on some aspects ;)
Note 2: Do you think it might be possible to benefit from some tests? I have a little repository (https://github.com/nulltoken/TestGitRepository) that comes from the GitSharp/JGit times. I could add some tags, branches to it in order to make it more interesting from a clone/fetch perspective... We could start with cloning it, then moving the HEAD and the remote/HEAD 3 commits backwards, and eventually fetching the missing commits...

@jamill @ben What's your opinion about this, guys?

@ben
Copy link
Member

ben commented Aug 29, 2012

I'm down for that. @jamill, take a look at https://github.com/libgit2/libgit2sharp/tree/benstraub/clone, which has a libgit2 built from @carlosmn's winhttp branch, and all the tests are green as of right now. Let me know if you need anything at all!

@carlosmn
Copy link
Member

Be advised that I'm probably going to rewrite my winhttp branch as I try to get to grips with the API, but yeah, if you find problems, so tell me, as I only run Windows to do or test Windows-specific things and otherwise never run the code.

@jamill
Copy link
Member Author

jamill commented Aug 30, 2012

@nulltoken @ben Sounds good! Thanks for creating this new branch - I will grab it and work from there. I will include some more testing by fetching from the repository you have suggested.

@nulltoken
Copy link
Member

@jamill @ben I've updated the test repository and added some references

$ git ls-remote
From git@github.com:nulltoken/TestGitRepository.git
49322bb17d3acc9146f98c97d078513228bbf3c0        HEAD
0966a434eb1a025db6b71485ab63a3bfbea520b6        refs/heads/first-merge
49322bb17d3acc9146f98c97d078513228bbf3c0        refs/heads/master
42e4e7c5e507e113ebbb7801b16b52cf867b7ce1        refs/heads/no-parent
d96c4e80345534eccee5ac7b07fc7603b56124cb        refs/tags/annotated_tag
c070ad8c08840c8116da865b2d65593a6bb9cd2a        refs/tags/annotated_tag^{}
55a1a760df4b86a02094a904dfa511deb5655905        refs/tags/blob
8f50ba15d49353813cc6e20298002c0d17b0a9ee        refs/tags/commit_tree
6e0c7bdb9b4ed93212491ee778ca1c65047cab4e        refs/tags/nearly-dangling

One thing to note: refs/tags/nearly-dangling points to a blob which is referenced by no other reference or tree in the repository.

@jamill jamill mentioned this pull request Sep 12, 2012
@jamill
Copy link
Member Author

jamill commented Sep 12, 2012

@nulltoken, @ben I have created another PR from the repository Ben created.
@carlosmn I did not hit any specific WinHttp issues :)

Should I close this PR and we can continue the discussion on the updated PR (#213)?

@nulltoken
Copy link
Member

Should I close this PR and we can continue the discussion on the updated PR (#213)?

Let's switch over to #213, then :)

@nulltoken nulltoken closed this Sep 13, 2012
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants