ODB/RefDB interface needs refactoring #314

New Issue

2016-05-21T17:01:22-05:00

milgner commented

2016-05-21 17:01:22 -05:00

(Migrated from github.com)

After some experimentation, I found that the interface to the ODB and RefDB parts of libgit2 do not work as expected due to the way cgo handles scoping.

Since it is not possible for other packages to pass in C types, functions like NewOdbBackendFromC can be removed and the related functionality refactored. It seems to me like the only way to properly do this would be to enhance the OdbBackend type by all the functions included in its C counterpart, git_odb_backend.
Let me know if you think this is a desirable feature, I'll gladly try to implement it.

After some experimentation, I found that the interface to the ODB and RefDB parts of libgit2 [do not work as expected](http://stackoverflow.com/questions/36670761/go-conflicting-types-between-import-and-c-library) due to the way cgo handles scoping. Since it is not possible for other packages to pass in C types, functions like `NewOdbBackendFromC` can be removed and the related functionality refactored. It seems to me like the only way to properly do this would be to enhance the `OdbBackend` type by all the functions included in its C counterpart, `git_odb_backend`. Let me know if you think this is a desirable feature, I'll gladly try to implement it.

MagicalTux commented

2016-09-30 08:57:52 -05:00

(Migrated from github.com)

Actually this can be solved easily by having both functions accept a unsafe.Pointer type rather than the actual classes, then cast internally. This way NewOdbBackendFromC and NewRefdbBackendFromC both works and the difference in scope of C (which is weird, by the way) is not a problem.

carlosmn commented

2016-10-31 15:26:01 -05:00

(Migrated from github.com)

I've merged the PR due to the scoping issues, but it doesn't have to be either-or. As @MagicalTux mentions, passing the pointer around is still possibly, and for the use-case you mention in SO, it's still the right thing to do, a the backend is implemented in C (although that's just a detail) and what you have is a pointer to a backend.

Being able to implement a backend in Go is valuable regardless of wishing to pass in pointers from third-party backends.

I've merged the PR due to the scoping issues, but it doesn't have to be either-or. As @MagicalTux mentions, passing the pointer around is still possibly, and for the use-case you mention in SO, it's still the right thing to do, a the backend is implemented in C (although that's just a detail) and what you have is a pointer to a backend. Being able to implement a backend in Go is valuable regardless of wishing to pass in pointers from third-party backends.

milgner commented

2016-11-02 11:22:37 -05:00

(Migrated from github.com)

Sounds good, thanks for the enhancement!

Sign in to join this conversation.