Skip to content

Updated interop and new external API design - #4

Merged
nulltoken merged 45 commits into
libgit2:masterfrom
tclem:master
Apr 11, 2011
Merged

Updated interop and new external API design#4
nulltoken merged 45 commits into
libgit2:masterfrom
tclem:master

Conversation

@tclem

@tclem tclem commented Mar 31, 2011

Copy link
Copy Markdown
Member

This is a significant update to libgit2sharp that includes these features:

  • Updated external API (using this gist as a guideline). The design focus for the API has been discover-ability, making the repository as the root aggregate object and fluent interfaces where possible.
  • Updated interop with C code with a focus on using marshaling instead of unsafe code.
  • Repository is the only IDisposable object and a RepositorySafeHandle has been created to ensure proper clean up in any situation.
  • All GitObjects (Commit, Blob, Tag, Tree) should now eager load their data and should not hold onto a C pointer as git_object_close() is immediately called after construction. Nested properties like the parents of a commit are to be lazy loaded and doing so requires a new lookup of the object in the repo.
  • Only implementing high level external API. Advanced API usage like raw reads and writes will come later if at all.
  • Separate .net 3.5 and .net 4.0 projects and assemblies are now being generated. (Silverlight and WP7 assemblies are in the works).
  • Solid test suite and some new test helpers like the disposable TemporaryRepositoryPath object.

tclem added 30 commits March 30, 2011 09:16
This is being done to clean up commit history on master.
Did not port all the compare, copy methods as I don't see a new
for them right now.
New Lookup API includes:
  TryLookup
  TryLookup<T>
  Lookup
  Lookup<T>

Looks like VS2010 can deal with optional params for .net 3.5 projects,
so NET35 ifdefs not necessary yet.
libgit2/libgit2@6dcb09b

Also fixed list references issues and reeanbled unit test
Still unsure about the currently strategy of always peeling back refs.
This needs some more thought and discussion
This is really just a wrapper around refs, but you can refer to branches
in a much nicer manner ('master' or 'origin/master' for example)
tclem added 15 commits March 30, 2011 09:32
Minor test refactoring to support this including new support
for creating a TemporaryRepository
External users of the API should not be able to change the value
of GitOid. It is now wrapped in ObjectId which is the new defacto
class for defining unique objects in libgit2sharp.
This requires that all GitObjects eager load their properties.
Handles to the C git objects are used and immediately closed now.
@vmg

vmg commented Mar 31, 2011

Copy link
Copy Markdown
Member

This is bongos, Tim. <3

@nulltoken

Copy link
Copy Markdown
Member

Wow. Looks quite nice! I'm going to review this shortly.

Thanks a lot :)

@txdv

txdv commented Mar 31, 2011

Copy link
Copy Markdown
Contributor

Em, you should have taken a look at the devel branch of libgit2sharp, it was far more advanced.

@txdv

txdv commented Mar 31, 2011

Copy link
Copy Markdown
Contributor

Well, instead of using all the defined structs you just used IntPtr.
Unsafe code was earlier needed in order to access some of the private fields easier, since there were no getter methods for some of the struct fields. Basically we could use the devel branch, get rid of most of the struct definitions and use IntPtr now, because the libgit2 api basically has a getter and setter for everything.

Now you removed the generator code. I don't know bro, if you really wan't to maintain every single API change by hand, this might not bother you, but I would strongly suggest just adjust it. It is quite a hack, but it makes your life easier. I see you just used the earlier NativeMethods.cs and adjusted it. Well THAT one was generated by the generator. It really made the transition between big API changes fast and easy. After the "big" API changes I needed like 2 hours to adjust the C# code to work again.

And you basically got rid of the Core, the core was meant to provide an C# interface to the "naked" C api, and the main lib would use the core in order to create POCOs, so it would be save/safer. It was basically an OOP mirror of the base C API.

All in all, I like the idea of moving to more marshaled code, but I'm not so fond of the rest. Especially that this comes out of nowhere, even though I provided some contacts in the readme, shows that you just did what you thought was right without discussing with anyone else. I appreciate the work, but a little bit of collaboration wouldn't have hurt.

@nulltoken
nulltoken merged commit 435b39d into libgit2:master Apr 11, 2011
@nulltoken

Copy link
Copy Markdown
Member

@tclem : Applied! That was an awesome proposal. Keep them coming :)

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.

4 participants