Skip to content

Commit

Permalink
Refactor all callbacks (#700)
Browse files Browse the repository at this point in the history 
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`.

(cherry picked from commit 5d8eaf7)
  • Loading branch information
@lhchavez
lhchavez committed Dec 5, 2020
1 parent a54915d commit 3f9fb4f
Show file tree
Hide file tree
Showing 21 changed files with 1,153 additions and 578 deletions.
99 changes: 68 additions & 31 deletions checkout.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -75,30 +76,37 @@ 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)
}
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
Expand All @@ -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
}
Expand All @@ -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)
Expand All @@ -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)
}
}

Expand All @@ -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)
}
Expand All @@ -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)
}
Expand All @@ -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)
}
Expand Down
18 changes: 12 additions & 6 deletions cherrypick.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -25,22 +25,23 @@ func cherrypickOptionsFromC(c *C.git_cherrypick_options) CherrypickOptions {
return opts
}

func (opts *CherrypickOptions) toC() *C.git_cherrypick_options {
func (opts *CherrypickOptions) toC(errorTarget *error) *C.git_cherrypick_options {
if opts == nil {
return nil
}
c := C.git_cherrypick_options{}
c.version = C.uint(opts.Version)
c.mainline = C.uint(opts.Mainline)
c.merge_opts = *opts.MergeOpts.toC()
c.checkout_opts = *opts.CheckoutOpts.toC()
c.checkout_opts = *opts.CheckoutOpts.toC(errorTarget)
return &c
}

func freeCherrypickOpts(ptr *C.git_cherrypick_options) {
if ptr == nil {
return
}
freeMergeOptions(&ptr.merge_opts)
freeCheckoutOptions(&ptr.checkout_opts)
}

Expand All @@ -62,14 +63,18 @@ func (v *Repository) Cherrypick(commit *Commit, opts CherrypickOptions) error {
runtime.LockOSThread()
defer runtime.UnlockOSThread()

cOpts := opts.toC()
var err error
cOpts := opts.toC(&err)
defer freeCherrypickOpts(cOpts)

ecode := C.git_cherrypick(v.ptr, commit.cast_ptr, cOpts)
ret := C.git_cherrypick(v.ptr, commit.cast_ptr, cOpts)
runtime.KeepAlive(v)
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
}
Expand All @@ -79,6 +84,7 @@ func (r *Repository) CherrypickCommit(pick, our *Commit, opts CherrypickOptions)
defer runtime.UnlockOSThread()

cOpts := opts.MergeOpts.toC()
defer freeMergeOptions(cOpts)

var ptr *C.git_index
ret := C.git_cherrypick_commit(&ptr, r.ptr, pick.cast_ptr, our.cast_ptr, C.uint(opts.Mainline), cOpts)
Expand Down
81 changes: 53 additions & 28 deletions clone.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,10 +3,11 @@ package git
/*
#include <git2.h>
extern void _go_git_populate_remote_cb(git_clone_options *opts);
extern void _go_git_populate_clone_callbacks(git_clone_options *opts);
*/
import "C"
import (
"errors"
"runtime"
"unsafe"
)
Expand All @@ -28,20 +29,23 @@ func Clone(url string, path string, options *CloneOptions) (*Repository, error)
cpath := C.CString(path)
defer C.free(unsafe.Pointer(cpath))

copts := (*C.git_clone_options)(C.calloc(1, C.size_t(unsafe.Sizeof(C.git_clone_options{}))))
populateCloneOptions(copts, options)
defer freeCloneOptions(copts)
var err error
cOptions := populateCloneOptions(&C.git_clone_options{}, options, &err)
defer freeCloneOptions(cOptions)

if len(options.CheckoutBranch) != 0 {
copts.checkout_branch = C.CString(options.CheckoutBranch)
cOptions.checkout_branch = C.CString(options.CheckoutBranch)
}

runtime.LockOSThread()
defer runtime.UnlockOSThread()

var ptr *C.git_repository
ret := C.git_clone(&ptr, curl, cpath, copts)
ret := C.git_clone(&ptr, curl, cpath, cOptions)

if ret == C.int(ErrorCodeUser) && err != nil {
return nil, err
}
if ret < 0 {
return nil, MakeGitError(ret)
}
Expand All @@ -50,47 +54,69 @@ func Clone(url string, path string, options *CloneOptions) (*Repository, error)
}

//export remoteCreateCallback
func remoteCreateCallback(cremote unsafe.Pointer, crepo unsafe.Pointer, cname, curl *C.char, payload unsafe.Pointer) C.int {
func remoteCreateCallback(
cremote unsafe.Pointer,
crepo unsafe.Pointer,
cname, curl *C.char,
payload unsafe.Pointer,
) C.int {
name := C.GoString(cname)
url := C.GoString(curl)
repo := newRepositoryFromC((*C.git_repository)(crepo))
// We don't own this repository, so make sure we don't try to free it
runtime.SetFinalizer(repo, nil)

if opts, ok := pointerHandles.Get(payload).(CloneOptions); ok {
remote, errorCode := opts.RemoteCreateCallback(repo, name, url)
// clear finalizer as the calling C function will
// free the remote itself
runtime.SetFinalizer(remote, nil)

if errorCode == ErrorCodeOK && remote != nil {
cptr := (**C.git_remote)(cremote)
*cptr = remote.ptr
} else if errorCode == ErrorCodeOK && remote == nil {
panic("no remote created by callback")
}

return C.int(errorCode)
} else {
data, ok := pointerHandles.Get(payload).(*cloneCallbackData)
if !ok {
panic("invalid remote create callback")
}

remote, ret := data.options.RemoteCreateCallback(repo, name, url)
// clear finalizer as the calling C function will
// free the remote itself
runtime.SetFinalizer(remote, nil)

if ret < 0 {
*data.errorTarget = errors.New(ErrorCode(ret).String())
return C.int(ErrorCodeUser)
}

if remote == nil {
panic("no remote created by callback")
}

cptr := (**C.git_remote)(cremote)
*cptr = remote.ptr

return C.int(ErrorCodeOK)
}

type cloneCallbackData struct {
options *CloneOptions
errorTarget *error
}

func populateCloneOptions(ptr *C.git_clone_options, opts *CloneOptions) {
func populateCloneOptions(ptr *C.git_clone_options, opts *CloneOptions, errorTarget *error) *C.git_clone_options {
C.git_clone_options_init(ptr, C.GIT_CLONE_OPTIONS_VERSION)

if opts == nil {
return
return nil
}
populateCheckoutOptions(&ptr.checkout_opts, opts.CheckoutOpts)
populateCheckoutOptions(&ptr.checkout_opts, opts.CheckoutOpts, errorTarget)
populateFetchOptions(&ptr.fetch_opts, opts.FetchOptions)
ptr.bare = cbool(opts.Bare)

if opts.RemoteCreateCallback != nil {
data := &cloneCallbackData{
options: opts,
errorTarget: errorTarget,
}
// Go v1.1 does not allow to assign a C function pointer
C._go_git_populate_remote_cb(ptr)
ptr.remote_cb_payload = pointerHandles.Track(*opts)
C._go_git_populate_clone_callbacks(ptr)
ptr.remote_cb_payload = pointerHandles.Track(data)
}

return ptr
}

func freeCloneOptions(ptr *C.git_clone_options) {
Expand All @@ -105,5 +131,4 @@ func freeCloneOptions(ptr *C.git_clone_options) {
}

C.free(unsafe.Pointer(ptr.checkout_branch))
C.free(unsafe.Pointer(ptr))
}
Loading

0 comments on commit 3f9fb4f

Please sign in to comment.