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, 47 insertions, 8 deletions

diff --git a/index.go b/index.go
index 61d5d82..f1f2e9d 100644
--- a/index.go
+++ b/index.go
@@ -10,12 +10,17 @@ extern int _go_git_index_remove_all(git_index*, const git_strarray*, void*);
 */
 import "C"
 import (
+	"errors"
 	"fmt"
 	"runtime"
 	"unsafe"
 )
 
 type IndexMatchedPathCallback func(string, string) int
+type indexMatchedPathCallbackData struct {
+	callback    IndexMatchedPathCallback
+	errorTarget *error
+}
 
 // IndexAddOption is a set of flags for APIs that add files matching pathspec.
 type IndexAddOption uint
@@ -227,12 +232,17 @@ func (v *Index) AddAll(pathspecs []string, flags IndexAddOption, callback IndexM
 	cpathspecs.strings = makeCStringsFromStrings(pathspecs)
 	defer freeStrarray(&cpathspecs)
 
+	var err error
+	data := indexMatchedPathCallbackData{
+		callback:    callback,
+		errorTarget: &err,
+	}
 	runtime.LockOSThread()
 	defer runtime.UnlockOSThread()
 
 	var handle unsafe.Pointer
 	if callback != nil {
-		handle = pointerHandles.Track(callback)
+		handle = pointerHandles.Track(&data)
 		defer pointerHandles.Untrack(handle)
 	}
 
@@ -243,9 +253,13 @@ func (v *Index) AddAll(pathspecs []string, flags IndexAddOption, callback IndexM
 		handle,
 	)
 	runtime.KeepAlive(v)
+	if ret == C.int(ErrorCodeUser) && err != nil {
+		return err
+	}
 	if ret < 0 {
 		return MakeGitError(ret)
 	}
+
 	return nil
 }
 
@@ -255,12 +269,17 @@ func (v *Index) UpdateAll(pathspecs []string, callback IndexMatchedPathCallback)
 	cpathspecs.strings = makeCStringsFromStrings(pathspecs)
 	defer freeStrarray(&cpathspecs)
 
+	var err error
+	data := indexMatchedPathCallbackData{
+		callback:    callback,
+		errorTarget: &err,
+	}
 	runtime.LockOSThread()
 	defer runtime.UnlockOSThread()
 
 	var handle unsafe.Pointer
 	if callback != nil {
-		handle = pointerHandles.Track(callback)
+		handle = pointerHandles.Track(&data)
 		defer pointerHandles.Untrack(handle)
 	}
 
@@ -270,9 +289,13 @@ func (v *Index) UpdateAll(pathspecs []string, callback IndexMatchedPathCallback)
 		handle,
 	)
 	runtime.KeepAlive(v)
+	if ret == C.int(ErrorCodeUser) && err != nil {
+		return err
+	}
 	if ret < 0 {
 		return MakeGitError(ret)
 	}
+
 	return nil
 }
 
@@ -282,12 +305,17 @@ func (v *Index) RemoveAll(pathspecs []string, callback IndexMatchedPathCallback)
 	cpathspecs.strings = makeCStringsFromStrings(pathspecs)
 	defer freeStrarray(&cpathspecs)
 
+	var err error
+	data := indexMatchedPathCallbackData{
+		callback:    callback,
+		errorTarget: &err,
+	}
 	runtime.LockOSThread()
 	defer runtime.UnlockOSThread()
 
 	var handle unsafe.Pointer
 	if callback != nil {
-		handle = pointerHandles.Track(callback)
+		handle = pointerHandles.Track(&data)
 		defer pointerHandles.Untrack(handle)
 	}
 
@@ -297,19 +325,30 @@ func (v *Index) RemoveAll(pathspecs []string, callback IndexMatchedPathCallback)
 		handle,
 	)
 	runtime.KeepAlive(v)
+	if ret == C.int(ErrorCodeUser) && err != nil {
+		return err
+	}
 	if ret < 0 {
 		return MakeGitError(ret)
 	}
+
 	return nil
 }
 
 //export indexMatchedPathCallback
-func indexMatchedPathCallback(cPath, cMatchedPathspec *C.char, payload unsafe.Pointer) int {
-	if callback, ok := pointerHandles.Get(payload).(IndexMatchedPathCallback); ok {
-		return callback(C.GoString(cPath), C.GoString(cMatchedPathspec))
-	} else {
+func indexMatchedPathCallback(cPath, cMatchedPathspec *C.char, payload unsafe.Pointer) C.int {
+	data, ok := pointerHandles.Get(payload).(*indexMatchedPathCallbackData)
+	if !ok {
 		panic("invalid matched path callback")
 	}
+
+	ret := data.callback(C.GoString(cPath), C.GoString(cMatchedPathspec))
+	if ret < 0 {
+		*data.errorTarget = errors.New(ErrorCode(ret).String())
+		return C.int(ErrorCodeUser)
+	}
+
+	return C.int(ErrorCodeOK)
 }
 
 func (v *Index) RemoveByPath(path string) error {
@@ -435,7 +474,7 @@ func (v *Index) EntryByPath(path string, stage int) (*IndexEntry, error) {
 
 	centry := C.git_index_get_bypath(v.ptr, cpath, C.int(stage))
 	if centry == nil {
-		return nil, MakeGitError(C.GIT_ENOTFOUND)
+		return nil, MakeGitError(C.int(ErrorCodeNotFound))
 	}
 	ret := newIndexEntryFromC(centry)
 	runtime.KeepAlive(v)