Refactor all callbacks (#700)

This change is a preparation for another change that makes all callback
types return a Go error instead of an error code / an integer. That is
going to make make things a lot more idiomatic.

The reason this change is split is threefold:

a) This change is mostly mechanical and should contain no semantic
   changes.
b) This change is backwards-compatible (in the Go API compatibility
   sense of the word), and thus can be backported to all other releases.
c) It makes the other change a bit smaller and more focused on just one
   thing.

Concretely, this change makes all callbacks populate a Go error when
they fail. If the callback is invoked from the same stack as the
function to which it was passed (e.g. for `Tree.Walk`), it will preserve
the error object directly into a struct that also holds the callback
function. Otherwise if the callback is pased to one func and will be
invoked when run from another one (e.g. for `Repository.InitRebase`),
the error string is saved into the libgit2 thread-local storage and then
re-created as a `GitError`.

1 files changed, 18 insertions, 17 deletions

diff --git a/packbuilder.go b/packbuilder.go
index 576e5ca..5d3a933 100644
--- a/packbuilder.go
+++ b/packbuilder.go
@@ -133,15 +133,15 @@ func (pb *Packbuilder) Written() uint32 {
 }
 
 type PackbuilderForeachCallback func([]byte) error
-type packbuilderCbData struct {
-	callback PackbuilderForeachCallback
-	err      error
+type packbuilderCallbackData struct {
+	callback    PackbuilderForeachCallback
+	errorTarget *error
 }
 
-//export packbuilderForEachCb
-func packbuilderForEachCb(buf unsafe.Pointer, size C.size_t, handle unsafe.Pointer) int {
+//export packbuilderForEachCallback
+func packbuilderForEachCallback(buf unsafe.Pointer, size C.size_t, handle unsafe.Pointer) C.int {
 	payload := pointerHandles.Get(handle)
-	data, ok := payload.(*packbuilderCbData)
+	data, ok := payload.(*packbuilderCallbackData)
 	if !ok {
 		panic("could not get packbuilder CB data")
 	}
@@ -150,19 +150,20 @@ func packbuilderForEachCb(buf unsafe.Pointer, size C.size_t, handle unsafe.Point
 
 	err := data.callback(slice)
 	if err != nil {
-		data.err = err
-		return C.GIT_EUSER
+		*data.errorTarget = err
+		return C.int(ErrorCodeUser)
 	}
 
-	return 0
+	return C.int(ErrorCodeOK)
 }
 
 // ForEach repeatedly calls the callback with new packfile data until
 // there is no more data or the callback returns an error
 func (pb *Packbuilder) ForEach(callback PackbuilderForeachCallback) error {
-	data := packbuilderCbData{
-		callback: callback,
-		err:      nil,
+	var err error
+	data := packbuilderCallbackData{
+		callback:    callback,
+		errorTarget: &err,
 	}
 	handle := pointerHandles.Track(&data)
 	defer pointerHandles.Untrack(handle)
@@ -170,13 +171,13 @@ func (pb *Packbuilder) ForEach(callback PackbuilderForeachCallback) error {
 	runtime.LockOSThread()
 	defer runtime.UnlockOSThread()
 
-	err := C._go_git_packbuilder_foreach(pb.ptr, handle)
+	ret := C._go_git_packbuilder_foreach(pb.ptr, handle)
 	runtime.KeepAlive(pb)
-	if err == C.GIT_EUSER {
-		return data.err
+	if ret == C.int(ErrorCodeUser) && err != nil {
+		return err
 	}
-	if err < 0 {
-		return MakeGitError(err)
+	if ret < 0 {
+		return MakeGitError(ret)
 	}
 
 	return nil