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, 20 insertions, 16 deletions

diff --git a/revert.go b/revert.go
index 2c18bc9..f318ade 100644
--- a/revert.go
+++ b/revert.go
@@ -15,12 +15,15 @@ type RevertOptions struct {
 	CheckoutOpts CheckoutOptions
 }
 
-func (opts *RevertOptions) toC() *C.git_revert_options {
+func (opts *RevertOptions) toC(errorTarget *error) *C.git_revert_options {
+	if opts == nil {
+		return nil
+	}
 	return &C.git_revert_options{
 		version:       C.GIT_REVERT_OPTIONS_VERSION,
 		mainline:      C.uint(opts.Mainline),
 		merge_opts:    *opts.MergeOpts.toC(),
-		checkout_opts: *opts.CheckoutOpts.toC(),
+		checkout_opts: *opts.CheckoutOpts.toC(errorTarget),
 	}
 }
 
@@ -33,6 +36,10 @@ func revertOptionsFromC(opts *C.git_revert_options) RevertOptions {
 }
 
 func freeRevertOptions(opts *C.git_revert_options) {
+	if opts != nil {
+		return
+	}
+	freeMergeOptions(&opts.merge_opts)
 	freeCheckoutOptions(&opts.checkout_opts)
 }
 
@@ -57,19 +64,19 @@ func (r *Repository) Revert(commit *Commit, revertOptions *RevertOptions) error
 	runtime.LockOSThread()
 	defer runtime.UnlockOSThread()
 
-	var cOpts *C.git_revert_options
-
-	if revertOptions != nil {
-		cOpts = revertOptions.toC()
-		defer freeRevertOptions(cOpts)
-	}
+	var err error
+	cOpts := revertOptions.toC(&err)
+	defer freeRevertOptions(cOpts)
 
-	ecode := C.git_revert(r.ptr, commit.cast_ptr, cOpts)
+	ret := C.git_revert(r.ptr, commit.cast_ptr, cOpts)
 	runtime.KeepAlive(r)
 	runtime.KeepAlive(commit)
 
-	if ecode < 0 {
-		return MakeGitError(ecode)
+	if ret == C.int(ErrorCodeUser) && err != nil {
+		return err
+	}
+	if ret < 0 {
+		return MakeGitError(ret)
 	}
 
 	return nil
@@ -81,11 +88,8 @@ func (r *Repository) RevertCommit(revertCommit *Commit, ourCommit *Commit, mainl
 	runtime.LockOSThread()
 	defer runtime.UnlockOSThread()
 
-	var cOpts *C.git_merge_options
-
-	if mergeOptions != nil {
-		cOpts = mergeOptions.toC()
-	}
+	cOpts := mergeOptions.toC()
+	defer freeMergeOptions(cOpts)
 
 	var index *C.git_index