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, 68 insertions, 31 deletions

diff --git a/checkout.go b/checkout.go
index fa6609a..3a60eb8 100644
--- a/checkout.go
+++ b/checkout.go
@@ -3,10 +3,11 @@ package git
 /*
 #include <git2.h>
 
-extern void _go_git_populate_checkout_cb(git_checkout_options *opts);
+extern void _go_git_populate_checkout_callbacks(git_checkout_options *opts);
 */
 import "C"
 import (
+	"errors"
 	"os"
 	"runtime"
 	"unsafe"
@@ -75,10 +76,10 @@ func checkoutOptionsFromC(c *C.git_checkout_options) CheckoutOptions {
 		NotifyFlags:    CheckoutNotifyType(c.notify_flags),
 	}
 	if c.notify_payload != nil {
-		opts.NotifyCallback = pointerHandles.Get(c.notify_payload).(*CheckoutOptions).NotifyCallback
+		opts.NotifyCallback = pointerHandles.Get(c.notify_payload).(*checkoutCallbackData).options.NotifyCallback
 	}
 	if c.progress_payload != nil {
-		opts.ProgressCallback = pointerHandles.Get(c.progress_payload).(*CheckoutOptions).ProgressCallback
+		opts.ProgressCallback = pointerHandles.Get(c.progress_payload).(*checkoutCallbackData).options.ProgressCallback
 	}
 	if c.target_directory != nil {
 		opts.TargetDirectory = C.GoString(c.target_directory)
@@ -86,19 +87,26 @@ func checkoutOptionsFromC(c *C.git_checkout_options) CheckoutOptions {
 	return opts
 }
 
-func (opts *CheckoutOptions) toC() *C.git_checkout_options {
+func (opts *CheckoutOptions) toC(errorTarget *error) *C.git_checkout_options {
 	if opts == nil {
 		return nil
 	}
-	c := C.git_checkout_options{}
-	populateCheckoutOptions(&c, opts)
-	return &c
+	return populateCheckoutOptions(&C.git_checkout_options{}, opts, errorTarget)
+}
+
+type checkoutCallbackData struct {
+	options     *CheckoutOptions
+	errorTarget *error
 }
 
 //export checkoutNotifyCallback
-func checkoutNotifyCallback(why C.git_checkout_notify_t, cpath *C.char, cbaseline, ctarget, cworkdir, data unsafe.Pointer) int {
-	if data == nil {
-		return 0
+func checkoutNotifyCallback(
+	why C.git_checkout_notify_t,
+	cpath *C.char,
+	cbaseline, ctarget, cworkdir, handle unsafe.Pointer,
+) C.int {
+	if handle == nil {
+		return C.int(ErrorCodeOK)
 	}
 	path := C.GoString(cpath)
 	var baseline, target, workdir DiffFile
@@ -111,26 +119,35 @@ func checkoutNotifyCallback(why C.git_checkout_notify_t, cpath *C.char, cbaselin
 	if cworkdir != nil {
 		workdir = diffFileFromC((*C.git_diff_file)(cworkdir))
 	}
-	opts := pointerHandles.Get(data).(*CheckoutOptions)
-	if opts.NotifyCallback == nil {
-		return 0
+	data := pointerHandles.Get(handle).(*checkoutCallbackData)
+	if data.options.NotifyCallback == nil {
+		return C.int(ErrorCodeOK)
+	}
+	ret := data.options.NotifyCallback(CheckoutNotifyType(why), path, baseline, target, workdir)
+	if ret < 0 {
+		*data.errorTarget = errors.New(ErrorCode(ret).String())
+		return C.int(ErrorCodeUser)
 	}
-	return int(opts.NotifyCallback(CheckoutNotifyType(why), path, baseline, target, workdir))
+	return C.int(ErrorCodeOK)
 }
 
 //export checkoutProgressCallback
-func checkoutProgressCallback(path *C.char, completed_steps, total_steps C.size_t, data unsafe.Pointer) int {
-	opts := pointerHandles.Get(data).(*CheckoutOptions)
-	if opts.ProgressCallback == nil {
-		return 0
+func checkoutProgressCallback(
+	path *C.char,
+	completed_steps, total_steps C.size_t,
+	handle unsafe.Pointer,
+) {
+	data := pointerHandles.Get(handle).(*checkoutCallbackData)
+	if data.options.ProgressCallback == nil {
+		return
 	}
-	return int(opts.ProgressCallback(C.GoString(path), uint(completed_steps), uint(total_steps)))
+	data.options.ProgressCallback(C.GoString(path), uint(completed_steps), uint(total_steps))
 }
 
 // Convert the CheckoutOptions struct to the corresponding
 // C-struct. Returns a pointer to ptr, or nil if opts is nil, in order
 // to help with what to pass.
-func populateCheckoutOptions(ptr *C.git_checkout_options, opts *CheckoutOptions) *C.git_checkout_options {
+func populateCheckoutOptions(ptr *C.git_checkout_options, opts *CheckoutOptions, errorTarget *error) *C.git_checkout_options {
 	if opts == nil {
 		return nil
 	}
@@ -142,14 +159,18 @@ func populateCheckoutOptions(ptr *C.git_checkout_options, opts *CheckoutOptions)
 	ptr.file_mode = C.uint(opts.FileMode.Perm())
 	ptr.notify_flags = C.uint(opts.NotifyFlags)
 	if opts.NotifyCallback != nil || opts.ProgressCallback != nil {
-		C._go_git_populate_checkout_cb(ptr)
-	}
-	payload := pointerHandles.Track(opts)
-	if opts.NotifyCallback != nil {
-		ptr.notify_payload = payload
-	}
-	if opts.ProgressCallback != nil {
-		ptr.progress_payload = payload
+		C._go_git_populate_checkout_callbacks(ptr)
+		data := &checkoutCallbackData{
+			options:     opts,
+			errorTarget: errorTarget,
+		}
+		payload := pointerHandles.Track(data)
+		if opts.NotifyCallback != nil {
+			ptr.notify_payload = payload
+		}
+		if opts.ProgressCallback != nil {
+			ptr.progress_payload = payload
+		}
 	}
 	if opts.TargetDirectory != "" {
 		ptr.target_directory = C.CString(opts.TargetDirectory)
@@ -176,6 +197,8 @@ func freeCheckoutOptions(ptr *C.git_checkout_options) {
 	}
 	if ptr.notify_payload != nil {
 		pointerHandles.Untrack(ptr.notify_payload)
+	} else if ptr.progress_payload != nil {
+		pointerHandles.Untrack(ptr.progress_payload)
 	}
 }
 
@@ -185,11 +208,16 @@ func (v *Repository) CheckoutHead(opts *CheckoutOptions) error {
 	runtime.LockOSThread()
 	defer runtime.UnlockOSThread()
 
-	cOpts := opts.toC()
+	var err error
+	cOpts := opts.toC(&err)
 	defer freeCheckoutOptions(cOpts)
 
 	ret := C.git_checkout_head(v.ptr, cOpts)
 	runtime.KeepAlive(v)
+
+	if ret == C.int(ErrorCodeUser) && err != nil {
+		return err
+	}
 	if ret < 0 {
 		return MakeGitError(ret)
 	}
@@ -209,11 +237,15 @@ func (v *Repository) CheckoutIndex(index *Index, opts *CheckoutOptions) error {
 	runtime.LockOSThread()
 	defer runtime.UnlockOSThread()
 
-	cOpts := opts.toC()
+	var err error
+	cOpts := opts.toC(&err)
 	defer freeCheckoutOptions(cOpts)
 
 	ret := C.git_checkout_index(v.ptr, iptr, cOpts)
 	runtime.KeepAlive(v)
+	if ret == C.int(ErrorCodeUser) && err != nil {
+		return err
+	}
 	if ret < 0 {
 		return MakeGitError(ret)
 	}
@@ -225,11 +257,16 @@ func (v *Repository) CheckoutTree(tree *Tree, opts *CheckoutOptions) error {
 	runtime.LockOSThread()
 	defer runtime.UnlockOSThread()
 
-	cOpts := opts.toC()
+	var err error
+	cOpts := opts.toC(&err)
 	defer freeCheckoutOptions(cOpts)
 
 	ret := C.git_checkout_tree(v.ptr, tree.ptr, cOpts)
 	runtime.KeepAlive(v)
+	runtime.KeepAlive(tree)
+	if ret == C.int(ErrorCodeUser) && err != nil {
+		return err
+	}
 	if ret < 0 {
 		return MakeGitError(ret)
 	}