From f4a35f543c90104207acbc9327c2efacfa7c521c Mon Sep 17 00:00:00 2001 From: lhchavez Date: Mon, 7 Dec 2020 19:02:35 -0800 Subject: [PATCH] More callbacks refactoring This change: * Gets rid of the `.toC()` functions for Options objects, since they were redundant with the `populateXxxOptions()`. * Adds support for `errorTarget` to the `RemoteOptions`, since they are used in the same stack for some functions (like `Fetch()`). Now for those cases, the error returned by the callback will be preserved as-is. --- checkout.go | 67 +++++------ cherrypick.go | 25 ++-- clone.go | 30 ++--- diff.go | 82 ++++++------- indexer.go | 25 ++-- merge.go | 63 +++++----- odb.go | 19 ++- patch.go | 2 +- rebase.go | 72 +++++++----- remote.go | 317 ++++++++++++++++++++++++++++++++------------------ reset.go | 2 +- revert.go | 41 ++++--- stash.go | 30 +++-- submodule.go | 18 +-- wrapper.c | 22 ++-- 15 files changed, 453 insertions(+), 362 deletions(-) diff --git a/checkout.go b/checkout.go index 3a60eb8..ebf7c31 100644 --- a/checkout.go +++ b/checkout.go @@ -87,13 +87,6 @@ func checkoutOptionsFromC(c *C.git_checkout_options) CheckoutOptions { return opts } -func (opts *CheckoutOptions) toC(errorTarget *error) *C.git_checkout_options { - if opts == nil { - return nil - } - return populateCheckoutOptions(&C.git_checkout_options{}, opts, errorTarget) -} - type checkoutCallbackData struct { options *CheckoutOptions errorTarget *error @@ -144,61 +137,61 @@ func checkoutProgressCallback( 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, errorTarget *error) *C.git_checkout_options { +// populateCheckoutOptions populates the provided C-struct with the contents of +// the provided CheckoutOptions struct. Returns copts, or nil if opts is nil, +// in order to help with what to pass. +func populateCheckoutOptions(copts *C.git_checkout_options, opts *CheckoutOptions, errorTarget *error) *C.git_checkout_options { + C.git_checkout_options_init(copts, C.GIT_CHECKOUT_OPTIONS_VERSION) if opts == nil { return nil } - C.git_checkout_options_init(ptr, 1) - ptr.checkout_strategy = C.uint(opts.Strategy) - ptr.disable_filters = cbool(opts.DisableFilters) - ptr.dir_mode = C.uint(opts.DirMode.Perm()) - ptr.file_mode = C.uint(opts.FileMode.Perm()) - ptr.notify_flags = C.uint(opts.NotifyFlags) + copts.checkout_strategy = C.uint(opts.Strategy) + copts.disable_filters = cbool(opts.DisableFilters) + copts.dir_mode = C.uint(opts.DirMode.Perm()) + copts.file_mode = C.uint(opts.FileMode.Perm()) + copts.notify_flags = C.uint(opts.NotifyFlags) if opts.NotifyCallback != nil || opts.ProgressCallback != nil { - C._go_git_populate_checkout_callbacks(ptr) + C._go_git_populate_checkout_callbacks(copts) data := &checkoutCallbackData{ options: opts, errorTarget: errorTarget, } payload := pointerHandles.Track(data) if opts.NotifyCallback != nil { - ptr.notify_payload = payload + copts.notify_payload = payload } if opts.ProgressCallback != nil { - ptr.progress_payload = payload + copts.progress_payload = payload } } if opts.TargetDirectory != "" { - ptr.target_directory = C.CString(opts.TargetDirectory) + copts.target_directory = C.CString(opts.TargetDirectory) } if len(opts.Paths) > 0 { - ptr.paths.strings = makeCStringsFromStrings(opts.Paths) - ptr.paths.count = C.size_t(len(opts.Paths)) + copts.paths.strings = makeCStringsFromStrings(opts.Paths) + copts.paths.count = C.size_t(len(opts.Paths)) } if opts.Baseline != nil { - ptr.baseline = opts.Baseline.cast_ptr + copts.baseline = opts.Baseline.cast_ptr } - return ptr + return copts } -func freeCheckoutOptions(ptr *C.git_checkout_options) { - if ptr == nil { +func freeCheckoutOptions(copts *C.git_checkout_options) { + if copts == nil { return } - C.free(unsafe.Pointer(ptr.target_directory)) - if ptr.paths.count > 0 { - freeStrarray(&ptr.paths) + C.free(unsafe.Pointer(copts.target_directory)) + if copts.paths.count > 0 { + freeStrarray(&copts.paths) } - if ptr.notify_payload != nil { - pointerHandles.Untrack(ptr.notify_payload) - } else if ptr.progress_payload != nil { - pointerHandles.Untrack(ptr.progress_payload) + if copts.notify_payload != nil { + pointerHandles.Untrack(copts.notify_payload) + } else if copts.progress_payload != nil { + pointerHandles.Untrack(copts.progress_payload) } } @@ -209,7 +202,7 @@ func (v *Repository) CheckoutHead(opts *CheckoutOptions) error { defer runtime.UnlockOSThread() var err error - cOpts := opts.toC(&err) + cOpts := populateCheckoutOptions(&C.git_checkout_options{}, opts, &err) defer freeCheckoutOptions(cOpts) ret := C.git_checkout_head(v.ptr, cOpts) @@ -238,7 +231,7 @@ func (v *Repository) CheckoutIndex(index *Index, opts *CheckoutOptions) error { defer runtime.UnlockOSThread() var err error - cOpts := opts.toC(&err) + cOpts := populateCheckoutOptions(&C.git_checkout_options{}, opts, &err) defer freeCheckoutOptions(cOpts) ret := C.git_checkout_index(v.ptr, iptr, cOpts) @@ -258,7 +251,7 @@ func (v *Repository) CheckoutTree(tree *Tree, opts *CheckoutOptions) error { defer runtime.UnlockOSThread() var err error - cOpts := opts.toC(&err) + cOpts := populateCheckoutOptions(&C.git_checkout_options{}, opts, &err) defer freeCheckoutOptions(cOpts) ret := C.git_checkout_tree(v.ptr, tree.ptr, cOpts) diff --git a/cherrypick.go b/cherrypick.go index b5d6c70..5ba57a5 100644 --- a/cherrypick.go +++ b/cherrypick.go @@ -25,24 +25,23 @@ func cherrypickOptionsFromC(c *C.git_cherrypick_options) CherrypickOptions { return opts } -func (opts *CherrypickOptions) toC(errorTarget *error) *C.git_cherrypick_options { +func populateCherrypickOptions(copts *C.git_cherrypick_options, opts *CherrypickOptions, errorTarget *error) *C.git_cherrypick_options { + C.git_cherrypick_options_init(copts, C.GIT_CHERRYPICK_OPTIONS_VERSION) 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(errorTarget) - return &c + copts.mainline = C.uint(opts.Mainline) + populateMergeOptions(&copts.merge_opts, &opts.MergeOpts) + populateCheckoutOptions(&copts.checkout_opts, &opts.CheckoutOpts, errorTarget) + return copts } -func freeCherrypickOpts(ptr *C.git_cherrypick_options) { - if ptr == nil { +func freeCherrypickOpts(copts *C.git_cherrypick_options) { + if copts == nil { return } - freeMergeOptions(&ptr.merge_opts) - freeCheckoutOptions(&ptr.checkout_opts) + freeMergeOptions(&copts.merge_opts) + freeCheckoutOptions(&copts.checkout_opts) } func DefaultCherrypickOptions() (CherrypickOptions, error) { @@ -64,7 +63,7 @@ func (v *Repository) Cherrypick(commit *Commit, opts CherrypickOptions) error { defer runtime.UnlockOSThread() var err error - cOpts := opts.toC(&err) + cOpts := populateCherrypickOptions(&C.git_cherrypick_options{}, &opts, &err) defer freeCherrypickOpts(cOpts) ret := C.git_cherrypick(v.ptr, commit.cast_ptr, cOpts) @@ -83,7 +82,7 @@ func (r *Repository) CherrypickCommit(pick, our *Commit, opts CherrypickOptions) runtime.LockOSThread() defer runtime.UnlockOSThread() - cOpts := opts.MergeOpts.toC() + cOpts := populateMergeOptions(&C.git_merge_options{}, &opts.MergeOpts) defer freeMergeOptions(cOpts) var ptr *C.git_index diff --git a/clone.go b/clone.go index 0f37a86..4921e12 100644 --- a/clone.go +++ b/clone.go @@ -94,15 +94,14 @@ type cloneCallbackData struct { errorTarget *error } -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) - +func populateCloneOptions(copts *C.git_clone_options, opts *CloneOptions, errorTarget *error) *C.git_clone_options { + C.git_clone_options_init(copts, C.GIT_CLONE_OPTIONS_VERSION) if opts == nil { return nil } - populateCheckoutOptions(&ptr.checkout_opts, opts.CheckoutOpts, errorTarget) - populateFetchOptions(&ptr.fetch_opts, opts.FetchOptions) - ptr.bare = cbool(opts.Bare) + populateCheckoutOptions(&copts.checkout_opts, opts.CheckoutOpts, errorTarget) + populateFetchOptions(&copts.fetch_opts, opts.FetchOptions, errorTarget) + copts.bare = cbool(opts.Bare) if opts.RemoteCreateCallback != nil { data := &cloneCallbackData{ @@ -110,23 +109,24 @@ func populateCloneOptions(ptr *C.git_clone_options, opts *CloneOptions, errorTar errorTarget: errorTarget, } // Go v1.1 does not allow to assign a C function pointer - C._go_git_populate_clone_callbacks(ptr) - ptr.remote_cb_payload = pointerHandles.Track(data) + C._go_git_populate_clone_callbacks(copts) + copts.remote_cb_payload = pointerHandles.Track(data) } - return ptr + return copts } -func freeCloneOptions(ptr *C.git_clone_options) { - if ptr == nil { +func freeCloneOptions(copts *C.git_clone_options) { + if copts == nil { return } - freeCheckoutOptions(&ptr.checkout_opts) + freeCheckoutOptions(&copts.checkout_opts) + freeFetchOptions(&copts.fetch_opts) - if ptr.remote_cb_payload != nil { - pointerHandles.Untrack(ptr.remote_cb_payload) + if copts.remote_cb_payload != nil { + pointerHandles.Untrack(copts.remote_cb_payload) } - C.free(unsafe.Pointer(ptr.checkout_branch)) + C.free(unsafe.Pointer(copts.checkout_branch)) } diff --git a/diff.go b/diff.go index dd793e4..e6ed284 100644 --- a/diff.go +++ b/diff.go @@ -641,29 +641,24 @@ func diffNotifyCallback(_diff_so_far unsafe.Pointer, delta_to_add *C.git_diff_de return C.int(ErrorCodeOK) } -func (opts *DiffOptions) toC(repo *Repository, errorTarget *error) *C.git_diff_options { +func populateDiffOptions(copts *C.git_diff_options, opts *DiffOptions, repo *Repository, errorTarget *error) *C.git_diff_options { + C.git_diff_options_init(copts, C.GIT_DIFF_OPTIONS_VERSION) if opts == nil { return nil } - cpathspec := C.git_strarray{} - if opts.Pathspec != nil { - cpathspec.count = C.size_t(len(opts.Pathspec)) - cpathspec.strings = makeCStringsFromStrings(opts.Pathspec) - } - - copts := &C.git_diff_options{ - version: C.GIT_DIFF_OPTIONS_VERSION, - flags: C.uint32_t(opts.Flags), - ignore_submodules: C.git_submodule_ignore_t(opts.IgnoreSubmodules), - pathspec: cpathspec, - context_lines: C.uint32_t(opts.ContextLines), - interhunk_lines: C.uint32_t(opts.InterhunkLines), - id_abbrev: C.uint16_t(opts.IdAbbrev), - max_size: C.git_off_t(opts.MaxSize), - old_prefix: C.CString(opts.OldPrefix), - new_prefix: C.CString(opts.NewPrefix), + copts.flags = C.uint32_t(opts.Flags) + copts.ignore_submodules = C.git_submodule_ignore_t(opts.IgnoreSubmodules) + if len(opts.Pathspec) > 0 { + copts.pathspec.count = C.size_t(len(opts.Pathspec)) + copts.pathspec.strings = makeCStringsFromStrings(opts.Pathspec) } + copts.context_lines = C.uint32_t(opts.ContextLines) + copts.interhunk_lines = C.uint32_t(opts.InterhunkLines) + copts.id_abbrev = C.uint16_t(opts.IdAbbrev) + copts.max_size = C.git_off_t(opts.MaxSize) + copts.old_prefix = C.CString(opts.OldPrefix) + copts.new_prefix = C.CString(opts.NewPrefix) if opts.NotifyCallback != nil { notifyData := &diffNotifyCallbackData{ @@ -681,8 +676,7 @@ func freeDiffOptions(copts *C.git_diff_options) { if copts == nil { return } - cpathspec := copts.pathspec - freeStrarray(&cpathspec) + freeStrarray(&copts.pathspec) C.free(unsafe.Pointer(copts.old_prefix)) C.free(unsafe.Pointer(copts.new_prefix)) if copts.payload != nil { @@ -703,7 +697,7 @@ func (v *Repository) DiffTreeToTree(oldTree, newTree *Tree, opts *DiffOptions) ( } var err error - copts := opts.toC(v, &err) + copts := populateDiffOptions(&C.git_diff_options{}, opts, v, &err) defer freeDiffOptions(copts) runtime.LockOSThread() @@ -730,7 +724,7 @@ func (v *Repository) DiffTreeToWorkdir(oldTree *Tree, opts *DiffOptions) (*Diff, } var err error - copts := opts.toC(v, &err) + copts := populateDiffOptions(&C.git_diff_options{}, opts, v, &err) defer freeDiffOptions(copts) runtime.LockOSThread() @@ -762,7 +756,7 @@ func (v *Repository) DiffTreeToIndex(oldTree *Tree, index *Index, opts *DiffOpti } var err error - copts := opts.toC(v, &err) + copts := populateDiffOptions(&C.git_diff_options{}, opts, v, &err) defer freeDiffOptions(copts) runtime.LockOSThread() @@ -790,7 +784,7 @@ func (v *Repository) DiffTreeToWorkdirWithIndex(oldTree *Tree, opts *DiffOptions } var err error - copts := opts.toC(v, &err) + copts := populateDiffOptions(&C.git_diff_options{}, opts, v, &err) defer freeDiffOptions(copts) runtime.LockOSThread() @@ -817,7 +811,7 @@ func (v *Repository) DiffIndexToWorkdir(index *Index, opts *DiffOptions) (*Diff, } var err error - copts := opts.toC(v, &err) + copts := populateDiffOptions(&C.git_diff_options{}, opts, v, &err) defer freeDiffOptions(copts) runtime.LockOSThread() @@ -873,7 +867,7 @@ func DiffBlobs(oldBlob *Blob, oldAsPath string, newBlob *Blob, newAsPath string, newBlobPath := C.CString(newAsPath) defer C.free(unsafe.Pointer(newBlobPath)) - copts := opts.toC(repo, &err) + copts := populateDiffOptions(&C.git_diff_options{}, opts, repo, &err) defer freeDiffOptions(copts) runtime.LockOSThread() @@ -977,40 +971,38 @@ func DefaultApplyOptions() (*ApplyOptions, error) { return applyOptionsFromC(&opts), nil } -func (a *ApplyOptions) toC(errorTarget *error) *C.git_apply_options { - if a == nil { +func populateApplyOptions(copts *C.git_apply_options, opts *ApplyOptions, errorTarget *error) *C.git_apply_options { + C.git_apply_options_init(copts, C.GIT_APPLY_OPTIONS_VERSION) + if opts == nil { return nil } - opts := &C.git_apply_options{ - version: C.GIT_APPLY_OPTIONS_VERSION, - flags: C.uint(a.Flags), - } + copts.flags = C.uint(opts.Flags) - if a.ApplyDeltaCallback != nil || a.ApplyHunkCallback != nil { + if opts.ApplyDeltaCallback != nil || opts.ApplyHunkCallback != nil { data := &applyCallbackData{ - options: a, + options: opts, errorTarget: errorTarget, } - C._go_git_populate_apply_callbacks(opts) - opts.payload = pointerHandles.Track(data) + C._go_git_populate_apply_callbacks(copts) + copts.payload = pointerHandles.Track(data) } - return opts + return copts } -func freeApplyOptions(opts *C.git_apply_options) { - if opts == nil { +func freeApplyOptions(copts *C.git_apply_options) { + if copts == nil { return } - if opts.payload != nil { - pointerHandles.Untrack(opts.payload) + if copts.payload != nil { + pointerHandles.Untrack(copts.payload) } } -func applyOptionsFromC(opts *C.git_apply_options) *ApplyOptions { +func applyOptionsFromC(copts *C.git_apply_options) *ApplyOptions { return &ApplyOptions{ - Flags: uint(opts.flags), + Flags: uint(copts.flags), } } @@ -1038,7 +1030,7 @@ func (v *Repository) ApplyDiff(diff *Diff, location ApplyLocation, opts *ApplyOp defer runtime.UnlockOSThread() var err error - cOpts := opts.toC(&err) + cOpts := populateApplyOptions(&C.git_apply_options{}, opts, &err) defer freeApplyOptions(cOpts) ret := C.git_apply(v.ptr, diff.ptr, C.git_apply_location_t(location), cOpts) @@ -1061,7 +1053,7 @@ func (v *Repository) ApplyToTree(diff *Diff, tree *Tree, opts *ApplyOptions) (*I defer runtime.UnlockOSThread() var err error - cOpts := opts.toC(&err) + cOpts := populateApplyOptions(&C.git_apply_options{}, opts, &err) defer freeApplyOptions(cOpts) var indexPtr *C.git_index diff --git a/indexer.go b/indexer.go index d1b9372..601ac9c 100644 --- a/indexer.go +++ b/indexer.go @@ -19,34 +19,31 @@ import ( // Indexer can post-process packfiles and create an .idx file for efficient // lookup. type Indexer struct { - ptr *C.git_indexer - stats C.git_transfer_progress - callbacks RemoteCallbacks - callbacksHandle unsafe.Pointer + ptr *C.git_indexer + stats C.git_transfer_progress + ccallbacks C.git_remote_callbacks } // NewIndexer creates a new indexer instance. func NewIndexer(packfilePath string, odb *Odb, callback TransferProgressCallback) (indexer *Indexer, err error) { - indexer = new(Indexer) - - runtime.LockOSThread() - defer runtime.UnlockOSThread() - var odbPtr *C.git_odb = nil if odb != nil { odbPtr = odb.ptr } - indexer.callbacks.TransferProgressCallback = callback - indexer.callbacksHandle = pointerHandles.Track(&indexer.callbacks) + indexer = new(Indexer) + populateRemoteCallbacks(&indexer.ccallbacks, &RemoteCallbacks{TransferProgressCallback: callback}, nil) + + runtime.LockOSThread() + defer runtime.UnlockOSThread() cstr := C.CString(packfilePath) defer C.free(unsafe.Pointer(cstr)) - ret := C._go_git_indexer_new(&indexer.ptr, cstr, 0, odbPtr, indexer.callbacksHandle) + ret := C._go_git_indexer_new(&indexer.ptr, cstr, 0, odbPtr, indexer.ccallbacks.payload) runtime.KeepAlive(odb) if ret < 0 { - pointerHandles.Untrack(indexer.callbacksHandle) + untrackCallbacksPayload(&indexer.ccallbacks) return nil, MakeGitError(ret) } @@ -93,7 +90,7 @@ func (indexer *Indexer) Commit() (*Oid, error) { // Free frees the indexer and its resources. func (indexer *Indexer) Free() { - pointerHandles.Untrack(indexer.callbacksHandle) + untrackCallbacksPayload(&indexer.ccallbacks) runtime.SetFinalizer(indexer, nil) C.git_indexer_free(indexer.ptr) } diff --git a/merge.go b/merge.go index 3fe76bb..f44a965 100644 --- a/merge.go +++ b/merge.go @@ -172,21 +172,20 @@ func DefaultMergeOptions() (MergeOptions, error) { return mergeOptionsFromC(&opts), nil } -func (mo *MergeOptions) toC() *C.git_merge_options { - if mo == nil { +func populateMergeOptions(copts *C.git_merge_options, opts *MergeOptions) *C.git_merge_options { + C.git_merge_options_init(copts, C.GIT_MERGE_OPTIONS_VERSION) + if opts == nil { return nil } - return &C.git_merge_options{ - version: C.uint(mo.Version), - flags: C.uint32_t(mo.TreeFlags), - rename_threshold: C.uint(mo.RenameThreshold), - target_limit: C.uint(mo.TargetLimit), - recursion_limit: C.uint(mo.RecursionLimit), - file_favor: C.git_merge_file_favor_t(mo.FileFavor), - } + copts.flags = C.uint32_t(opts.TreeFlags) + copts.rename_threshold = C.uint(opts.RenameThreshold) + copts.target_limit = C.uint(opts.TargetLimit) + copts.recursion_limit = C.uint(opts.RecursionLimit) + copts.file_favor = C.git_merge_file_favor_t(opts.FileFavor) + return copts } -func freeMergeOptions(opts *C.git_merge_options) { +func freeMergeOptions(copts *C.git_merge_options) { } type MergeFileFavor int @@ -203,9 +202,9 @@ func (r *Repository) Merge(theirHeads []*AnnotatedCommit, mergeOptions *MergeOpt defer runtime.UnlockOSThread() var err error - cMergeOpts := mergeOptions.toC() + cMergeOpts := populateMergeOptions(&C.git_merge_options{}, mergeOptions) defer freeMergeOptions(cMergeOpts) - cCheckoutOptions := checkoutOptions.toC(&err) + cCheckoutOptions := populateCheckoutOptions(&C.git_checkout_options{}, checkoutOptions, &err) defer freeCheckoutOptions(cCheckoutOptions) gmerge_head_array := make([]*C.git_annotated_commit, len(theirHeads)) @@ -269,7 +268,7 @@ func (r *Repository) MergeCommits(ours *Commit, theirs *Commit, options *MergeOp runtime.LockOSThread() defer runtime.UnlockOSThread() - copts := options.toC() + copts := populateMergeOptions(&C.git_merge_options{}, options) defer freeMergeOptions(copts) var ptr *C.git_index @@ -287,7 +286,7 @@ func (r *Repository) MergeTrees(ancestor *Tree, ours *Tree, theirs *Tree, option runtime.LockOSThread() defer runtime.UnlockOSThread() - copts := options.toC() + copts := populateMergeOptions(&C.git_merge_options{}, options) defer freeMergeOptions(copts) var ancestor_ptr *C.git_tree @@ -446,22 +445,28 @@ func mergeFileOptionsFromC(c C.git_merge_file_options) MergeFileOptions { } } -func populateCMergeFileOptions(c *C.git_merge_file_options, options MergeFileOptions) { - c.ancestor_label = C.CString(options.AncestorLabel) - c.our_label = C.CString(options.OurLabel) - c.their_label = C.CString(options.TheirLabel) - c.favor = C.git_merge_file_favor_t(options.Favor) - c.flags = C.uint32_t(options.Flags) - c.marker_size = C.ushort(options.MarkerSize) +func populateMergeFileOptions(copts *C.git_merge_file_options, opts *MergeFileOptions) *C.git_merge_file_options { + C.git_merge_file_options_init(copts, C.GIT_MERGE_FILE_OPTIONS_VERSION) + if opts == nil { + return nil + } + + copts.ancestor_label = C.CString(opts.AncestorLabel) + copts.our_label = C.CString(opts.OurLabel) + copts.their_label = C.CString(opts.TheirLabel) + copts.favor = C.git_merge_file_favor_t(opts.Favor) + copts.flags = C.uint32_t(opts.Flags) + copts.marker_size = C.ushort(opts.MarkerSize) + return copts } -func freeCMergeFileOptions(c *C.git_merge_file_options) { - if c == nil { +func freeMergeFileOptions(copts *C.git_merge_file_options) { + if copts == nil { return } - C.free(unsafe.Pointer(c.ancestor_label)) - C.free(unsafe.Pointer(c.our_label)) - C.free(unsafe.Pointer(c.their_label)) + C.free(unsafe.Pointer(copts.ancestor_label)) + C.free(unsafe.Pointer(copts.our_label)) + C.free(unsafe.Pointer(copts.their_label)) } func MergeFile(ancestor MergeFileInput, ours MergeFileInput, theirs MergeFileInput, options *MergeFileOptions) (*MergeFileResult, error) { @@ -494,8 +499,8 @@ func MergeFile(ancestor MergeFileInput, ours MergeFileInput, theirs MergeFileInp if ecode < 0 { return nil, MakeGitError(ecode) } - populateCMergeFileOptions(copts, *options) - defer freeCMergeFileOptions(copts) + populateMergeFileOptions(copts, options) + defer freeMergeFileOptions(copts) } runtime.LockOSThread() diff --git a/odb.go b/odb.go index 2d052ac..cef13bb 100644 --- a/odb.go +++ b/odb.go @@ -291,18 +291,16 @@ func (v *Odb) NewWriteStream(size int64, otype ObjectType) (*OdbWriteStream, err // layer does not understand pack files, the objects will be stored in whatever // format the ODB layer uses. func (v *Odb) NewWritePack(callback TransferProgressCallback) (*OdbWritepack, error) { - writepack := new(OdbWritepack) - runtime.LockOSThread() defer runtime.UnlockOSThread() - writepack.callbacks.TransferProgressCallback = callback - writepack.callbacksHandle = pointerHandles.Track(&writepack.callbacks) + writepack := new(OdbWritepack) + populateRemoteCallbacks(&writepack.ccallbacks, &RemoteCallbacks{TransferProgressCallback: callback}, nil) - ret := C._go_git_odb_write_pack(&writepack.ptr, v.ptr, writepack.callbacksHandle) + ret := C._go_git_odb_write_pack(&writepack.ptr, v.ptr, writepack.ccallbacks.payload) runtime.KeepAlive(v) if ret < 0 { - pointerHandles.Untrack(writepack.callbacksHandle) + untrackCallbacksPayload(&writepack.ccallbacks) return nil, MakeGitError(ret) } @@ -442,10 +440,9 @@ func (stream *OdbWriteStream) Free() { // OdbWritepack is a stream to write a packfile to the ODB. type OdbWritepack struct { - ptr *C.git_odb_writepack - stats C.git_transfer_progress - callbacks RemoteCallbacks - callbacksHandle unsafe.Pointer + ptr *C.git_odb_writepack + stats C.git_transfer_progress + ccallbacks C.git_remote_callbacks } func (writepack *OdbWritepack) Write(data []byte) (int, error) { @@ -479,7 +476,7 @@ func (writepack *OdbWritepack) Commit() error { } func (writepack *OdbWritepack) Free() { - pointerHandles.Untrack(writepack.callbacksHandle) + untrackCallbacksPayload(&writepack.ccallbacks) runtime.SetFinalizer(writepack, nil) C._go_git_odb_writepack_free(writepack.ptr) } diff --git a/patch.go b/patch.go index 6441751..b2595f1 100644 --- a/patch.go +++ b/patch.go @@ -78,7 +78,7 @@ func (v *Repository) PatchFromBuffers(oldPath, newPath string, oldBuf, newBuf [] defer C.free(unsafe.Pointer(cNewPath)) var err error - copts := opts.toC(v, &err) + copts := populateDiffOptions(&C.git_diff_options{}, opts, v, &err) defer freeDiffOptions(copts) runtime.LockOSThread() diff --git a/rebase.go b/rebase.go index 4c621ab..119d4f5 100644 --- a/rebase.go +++ b/rebase.go @@ -72,20 +72,23 @@ func newRebaseOperationFromC(c *C.git_rebase_operation) *RebaseOperation { } //export commitSigningCallback -func commitSigningCallback(errorMessage **C.char, _signature *C.git_buf, _signature_field *C.git_buf, _commit_content *C.char, _payload unsafe.Pointer) C.int { - opts, ok := pointerHandles.Get(_payload).(*RebaseOptions) +func commitSigningCallback(errorMessage **C.char, _signature *C.git_buf, _signature_field *C.git_buf, _commit_content *C.char, handle unsafe.Pointer) C.int { + data, ok := pointerHandles.Get(handle).(*rebaseOptionsData) if !ok { panic("invalid sign payload") } - if opts.CommitSigningCallback == nil { + if data.options.CommitSigningCallback == nil { return C.int(ErrorCodePassthrough) } commitContent := C.GoString(_commit_content) - signature, signatureField, err := opts.CommitSigningCallback(commitContent) + signature, signatureField, err := data.options.CommitSigningCallback(commitContent) if err != nil { + if data.errorTarget != nil { + *data.errorTarget = err + } return setCallbackError(errorMessage, err) } @@ -107,12 +110,18 @@ func commitSigningCallback(errorMessage **C.char, _signature *C.git_buf, _signat if signatureField != "" { err := fillBuf(signatureField, _signature_field) if err != nil { + if data.errorTarget != nil { + *data.errorTarget = err + } return setCallbackError(errorMessage, err) } } err = fillBuf(signature, _signature) if err != nil { + if data.errorTarget != nil { + *data.errorTarget = err + } return setCallbackError(errorMessage, err) } @@ -130,6 +139,11 @@ type RebaseOptions struct { CommitSigningCallback CommitSigningCallback } +type rebaseOptionsData struct { + options *RebaseOptions + errorTarget *error +} + // DefaultRebaseOptions returns a RebaseOptions with default values. func DefaultRebaseOptions() (RebaseOptions, error) { opts := C.git_rebase_options{} @@ -155,37 +169,39 @@ func rebaseOptionsFromC(opts *C.git_rebase_options) RebaseOptions { } } -func (ro *RebaseOptions) toC(errorTarget *error) *C.git_rebase_options { - if ro == nil { +func populateRebaseOptions(copts *C.git_rebase_options, opts *RebaseOptions, errorTarget *error) *C.git_rebase_options { + C.git_rebase_options_init(copts, C.GIT_REBASE_OPTIONS_VERSION) + if opts == nil { return nil } - cOptions := &C.git_rebase_options{ - version: C.uint(ro.Version), - quiet: C.int(ro.Quiet), - inmemory: C.int(ro.InMemory), - rewrite_notes_ref: mapEmptyStringToNull(ro.RewriteNotesRef), - merge_options: *ro.MergeOptions.toC(), - checkout_options: *ro.CheckoutOptions.toC(errorTarget), + copts.quiet = C.int(opts.Quiet) + copts.inmemory = C.int(opts.InMemory) + copts.rewrite_notes_ref = mapEmptyStringToNull(opts.RewriteNotesRef) + populateMergeOptions(&copts.merge_options, &opts.MergeOptions) + populateCheckoutOptions(&copts.checkout_options, &opts.CheckoutOptions, errorTarget) + + if opts.CommitSigningCallback != nil { + data := &rebaseOptionsData{ + options: opts, + errorTarget: errorTarget, + } + C._go_git_populate_rebase_callbacks(copts) + copts.payload = pointerHandles.Track(data) } - if ro.CommitSigningCallback != nil { - C._go_git_populate_rebase_callbacks(cOptions) - cOptions.payload = pointerHandles.Track(ro) - } - - return cOptions + return copts } -func freeRebaseOptions(opts *C.git_rebase_options) { - if opts == nil { +func freeRebaseOptions(copts *C.git_rebase_options) { + if copts == nil { return } - C.free(unsafe.Pointer(opts.rewrite_notes_ref)) - freeMergeOptions(&opts.merge_options) - freeCheckoutOptions(&opts.checkout_options) - if opts.payload != nil { - pointerHandles.Untrack(opts.payload) + C.free(unsafe.Pointer(copts.rewrite_notes_ref)) + freeMergeOptions(&copts.merge_options) + freeCheckoutOptions(&copts.checkout_options) + if copts.payload != nil { + pointerHandles.Untrack(copts.payload) } } @@ -222,7 +238,7 @@ func (r *Repository) InitRebase(branch *AnnotatedCommit, upstream *AnnotatedComm var ptr *C.git_rebase var err error - cOpts := opts.toC(&err) + cOpts := populateRebaseOptions(&C.git_rebase_options{}, opts, &err) ret := C.git_rebase_init(&ptr, r.ptr, branch.ptr, upstream.ptr, onto.ptr, cOpts) runtime.KeepAlive(branch) runtime.KeepAlive(upstream) @@ -246,7 +262,7 @@ func (r *Repository) OpenRebase(opts *RebaseOptions) (*Rebase, error) { var ptr *C.git_rebase var err error - cOpts := opts.toC(&err) + cOpts := populateRebaseOptions(&C.git_rebase_options{}, opts, &err) ret := C.git_rebase_open(&ptr, r.ptr, cOpts) runtime.KeepAlive(r) if ret == C.int(ErrorCodeUser) && err != nil { diff --git a/remote.go b/remote.go index b1c8532..1887f79 100644 --- a/remote.go +++ b/remote.go @@ -7,7 +7,6 @@ package git #include extern void _go_git_populate_remote_callbacks(git_remote_callbacks *callbacks); - */ import "C" import ( @@ -72,6 +71,11 @@ type RemoteCallbacks struct { PushUpdateReferenceCallback } +type remoteCallbacksData struct { + callbacks *RemoteCallbacks + errorTarget *error +} + type FetchPrune uint const ( @@ -86,7 +90,6 @@ const ( type DownloadTags uint const ( - // Use the setting from the configuration. DownloadTagsUnspecified DownloadTags = C.GIT_REMOTE_DOWNLOAD_TAGS_UNSPECIFIED // Ask the server for tags pointing to objects we're already @@ -209,44 +212,58 @@ func newRemoteHeadFromC(ptr *C.git_remote_head) RemoteHead { } } -func untrackCalbacksPayload(callbacks *C.git_remote_callbacks) { - if callbacks != nil && callbacks.payload != nil { - pointerHandles.Untrack(callbacks.payload) - } -} - -func populateRemoteCallbacks(ptr *C.git_remote_callbacks, callbacks *RemoteCallbacks) { - C.git_remote_init_callbacks(ptr, C.GIT_REMOTE_CALLBACKS_VERSION) - if callbacks == nil { +func untrackCallbacksPayload(callbacks *C.git_remote_callbacks) { + if callbacks == nil || callbacks.payload == nil { return } + pointerHandles.Untrack(callbacks.payload) +} + +func populateRemoteCallbacks(ptr *C.git_remote_callbacks, callbacks *RemoteCallbacks, errorTarget *error) *C.git_remote_callbacks { + C.git_remote_init_callbacks(ptr, C.GIT_REMOTE_CALLBACKS_VERSION) + if callbacks == nil { + return ptr + } C._go_git_populate_remote_callbacks(ptr) - ptr.payload = pointerHandles.Track(callbacks) + data := &remoteCallbacksData{ + callbacks: callbacks, + errorTarget: errorTarget, + } + ptr.payload = pointerHandles.Track(data) + return ptr } //export sidebandProgressCallback -func sidebandProgressCallback(errorMessage **C.char, _str *C.char, _len C.int, data unsafe.Pointer) C.int { - callbacks := pointerHandles.Get(data).(*RemoteCallbacks) - if callbacks.SidebandProgressCallback == nil { +func sidebandProgressCallback(errorMessage **C.char, _str *C.char, _len C.int, handle unsafe.Pointer) C.int { + data := pointerHandles.Get(handle).(*remoteCallbacksData) + if data.callbacks.SidebandProgressCallback == nil { return C.int(ErrorCodeOK) } str := C.GoStringN(_str, _len) - ret := callbacks.SidebandProgressCallback(str) + ret := data.callbacks.SidebandProgressCallback(str) if ret < 0 { - return setCallbackError(errorMessage, errors.New(ErrorCode(ret).String())) + err := errors.New(ErrorCode(ret).String()) + if data.errorTarget != nil { + *data.errorTarget = err + } + return setCallbackError(errorMessage, err) } return C.int(ErrorCodeOK) } //export completionCallback -func completionCallback(errorMessage **C.char, completion_type C.git_remote_completion_type, data unsafe.Pointer) C.int { - callbacks := pointerHandles.Get(data).(*RemoteCallbacks) - if callbacks.CompletionCallback == nil { +func completionCallback(errorMessage **C.char, completion_type C.git_remote_completion_type, handle unsafe.Pointer) C.int { + data := pointerHandles.Get(handle).(*remoteCallbacksData) + if data.callbacks.CompletionCallback == nil { return C.int(ErrorCodeOK) } - ret := callbacks.CompletionCallback(RemoteCompletion(completion_type)) + ret := data.callbacks.CompletionCallback(RemoteCompletion(completion_type)) if ret < 0 { - return setCallbackError(errorMessage, errors.New(ErrorCode(ret).String())) + err := errors.New(ErrorCode(ret).String()) + if data.errorTarget != nil { + *data.errorTarget = err + } + return setCallbackError(errorMessage, err) } return C.int(ErrorCodeOK) } @@ -258,16 +275,19 @@ func credentialsCallback( _url *C.char, _username_from_url *C.char, allowed_types uint, - data unsafe.Pointer, + handle unsafe.Pointer, ) C.int { - callbacks, _ := pointerHandles.Get(data).(*RemoteCallbacks) - if callbacks.CredentialsCallback == nil { + data := pointerHandles.Get(handle).(*remoteCallbacksData) + if data.callbacks.CredentialsCallback == nil { return C.int(ErrorCodePassthrough) } url := C.GoString(_url) username_from_url := C.GoString(_username_from_url) - cred, err := callbacks.CredentialsCallback(url, username_from_url, (CredentialType)(allowed_types)) + cred, err := data.callbacks.CredentialsCallback(url, username_from_url, (CredentialType)(allowed_types)) if err != nil { + if data.errorTarget != nil { + *data.errorTarget = err + } return setCallbackError(errorMessage, err) } if cred != nil { @@ -281,14 +301,18 @@ func credentialsCallback( } //export transferProgressCallback -func transferProgressCallback(errorMessage **C.char, stats *C.git_transfer_progress, data unsafe.Pointer) C.int { - callbacks, _ := pointerHandles.Get(data).(*RemoteCallbacks) - if callbacks.TransferProgressCallback == nil { +func transferProgressCallback(errorMessage **C.char, stats *C.git_transfer_progress, handle unsafe.Pointer) C.int { + data := pointerHandles.Get(handle).(*remoteCallbacksData) + if data.callbacks.TransferProgressCallback == nil { return C.int(ErrorCodeOK) } - ret := callbacks.TransferProgressCallback(newTransferProgressFromC(stats)) + ret := data.callbacks.TransferProgressCallback(newTransferProgressFromC(stats)) if ret < 0 { - return setCallbackError(errorMessage, errors.New(ErrorCode(ret).String())) + err := errors.New(ErrorCode(ret).String()) + if data.errorTarget != nil { + *data.errorTarget = err + } + return setCallbackError(errorMessage, err) } return C.int(ErrorCodeOK) } @@ -299,18 +323,22 @@ func updateTipsCallback( _refname *C.char, _a *C.git_oid, _b *C.git_oid, - data unsafe.Pointer, + handle unsafe.Pointer, ) C.int { - callbacks, _ := pointerHandles.Get(data).(*RemoteCallbacks) - if callbacks.UpdateTipsCallback == nil { + data := pointerHandles.Get(handle).(*remoteCallbacksData) + if data.callbacks.UpdateTipsCallback == nil { return C.int(ErrorCodeOK) } refname := C.GoString(_refname) a := newOidFromC(_a) b := newOidFromC(_b) - ret := callbacks.UpdateTipsCallback(refname, a, b) + ret := data.callbacks.UpdateTipsCallback(refname, a, b) if ret < 0 { - return setCallbackError(errorMessage, errors.New(ErrorCode(ret).String())) + err := errors.New(ErrorCode(ret).String()) + if data.errorTarget != nil { + *data.errorTarget = err + } + return setCallbackError(errorMessage, err) } return C.int(ErrorCodeOK) } @@ -321,11 +349,11 @@ func certificateCheckCallback( _cert *C.git_cert, _valid C.int, _host *C.char, - data unsafe.Pointer, + handle unsafe.Pointer, ) C.int { - callbacks, _ := pointerHandles.Get(data).(*RemoteCallbacks) + data := pointerHandles.Get(handle).(*remoteCallbacksData) // if there's no callback set, we need to make sure we fail if the library didn't consider this cert valid - if callbacks.CertificateCheckCallback == nil { + if data.callbacks.CertificateCheckCallback == nil { if _valid == 0 { return C.int(ErrorCodeCertificate) } @@ -341,10 +369,17 @@ func certificateCheckCallback( ccert := (*C.git_cert_x509)(unsafe.Pointer(_cert)) x509_certs, err := x509.ParseCertificates(C.GoBytes(ccert.data, C.int(ccert.len))) if err != nil { + if data.errorTarget != nil { + *data.errorTarget = err + } return setCallbackError(errorMessage, err) } if len(x509_certs) < 1 { - return setCallbackError(errorMessage, errors.New("empty certificate list")) + err := errors.New("empty certificate list") + if data.errorTarget != nil { + *data.errorTarget = err + } + return setCallbackError(errorMessage, err) } // we assume there's only one, which should hold true for any web server we want to talk to @@ -357,74 +392,95 @@ func certificateCheckCallback( C.memcpy(unsafe.Pointer(&cert.Hostkey.HashSHA1[0]), unsafe.Pointer(&ccert.hash_sha1[0]), C.size_t(len(cert.Hostkey.HashSHA1))) C.memcpy(unsafe.Pointer(&cert.Hostkey.HashSHA256[0]), unsafe.Pointer(&ccert.hash_sha256[0]), C.size_t(len(cert.Hostkey.HashSHA256))) } else { - return setCallbackError(errorMessage, errors.New("unsupported certificate type")) + err := errors.New("unsupported certificate type") + if data.errorTarget != nil { + *data.errorTarget = err + } + return setCallbackError(errorMessage, err) } - ret := callbacks.CertificateCheckCallback(&cert, valid, host) + ret := data.callbacks.CertificateCheckCallback(&cert, valid, host) if ret < 0 { - return setCallbackError(errorMessage, errors.New(ErrorCode(ret).String())) + err := errors.New(ErrorCode(ret).String()) + if data.errorTarget != nil { + *data.errorTarget = err + } + return setCallbackError(errorMessage, err) } return C.int(ErrorCodeOK) } //export packProgressCallback -func packProgressCallback(errorMessage **C.char, stage C.int, current, total C.uint, data unsafe.Pointer) C.int { - callbacks, _ := pointerHandles.Get(data).(*RemoteCallbacks) - if callbacks.PackProgressCallback == nil { +func packProgressCallback(errorMessage **C.char, stage C.int, current, total C.uint, handle unsafe.Pointer) C.int { + data := pointerHandles.Get(handle).(*remoteCallbacksData) + if data.callbacks.PackProgressCallback == nil { return C.int(ErrorCodeOK) } - ret := callbacks.PackProgressCallback(int32(stage), uint32(current), uint32(total)) + ret := data.callbacks.PackProgressCallback(int32(stage), uint32(current), uint32(total)) if ret < 0 { - return setCallbackError(errorMessage, errors.New(ErrorCode(ret).String())) + err := errors.New(ErrorCode(ret).String()) + if data.errorTarget != nil { + *data.errorTarget = err + } + return setCallbackError(errorMessage, err) } return C.int(ErrorCodeOK) } //export pushTransferProgressCallback -func pushTransferProgressCallback(errorMessage **C.char, current, total C.uint, bytes C.size_t, data unsafe.Pointer) C.int { - callbacks, _ := pointerHandles.Get(data).(*RemoteCallbacks) - if callbacks.PushTransferProgressCallback == nil { +func pushTransferProgressCallback(errorMessage **C.char, current, total C.uint, bytes C.size_t, handle unsafe.Pointer) C.int { + data := pointerHandles.Get(handle).(*remoteCallbacksData) + if data.callbacks.PushTransferProgressCallback == nil { return C.int(ErrorCodeOK) } - ret := callbacks.PushTransferProgressCallback(uint32(current), uint32(total), uint(bytes)) + ret := data.callbacks.PushTransferProgressCallback(uint32(current), uint32(total), uint(bytes)) if ret < 0 { - return setCallbackError(errorMessage, errors.New(ErrorCode(ret).String())) + err := errors.New(ErrorCode(ret).String()) + if data.errorTarget != nil { + *data.errorTarget = err + } + return setCallbackError(errorMessage, err) } return C.int(ErrorCodeOK) } //export pushUpdateReferenceCallback -func pushUpdateReferenceCallback(errorMessage **C.char, refname, status *C.char, data unsafe.Pointer) C.int { - callbacks, _ := pointerHandles.Get(data).(*RemoteCallbacks) - if callbacks.PushUpdateReferenceCallback == nil { +func pushUpdateReferenceCallback(errorMessage **C.char, refname, status *C.char, handle unsafe.Pointer) C.int { + data := pointerHandles.Get(handle).(*remoteCallbacksData) + if data.callbacks.PushUpdateReferenceCallback == nil { return C.int(ErrorCodeOK) } - ret := callbacks.PushUpdateReferenceCallback(C.GoString(refname), C.GoString(status)) + ret := data.callbacks.PushUpdateReferenceCallback(C.GoString(refname), C.GoString(status)) if ret < 0 { - return setCallbackError(errorMessage, errors.New(ErrorCode(ret).String())) + err := errors.New(ErrorCode(ret).String()) + if data.errorTarget != nil { + *data.errorTarget = err + } + return setCallbackError(errorMessage, err) } return C.int(ErrorCodeOK) } -func populateProxyOptions(ptr *C.git_proxy_options, opts *ProxyOptions) { - C.git_proxy_options_init(ptr, C.GIT_PROXY_OPTIONS_VERSION) +func populateProxyOptions(copts *C.git_proxy_options, opts *ProxyOptions) *C.git_proxy_options { + C.git_proxy_options_init(copts, C.GIT_PROXY_OPTIONS_VERSION) if opts == nil { - return + return nil } - ptr._type = C.git_proxy_t(opts.Type) - ptr.url = C.CString(opts.Url) + copts._type = C.git_proxy_t(opts.Type) + copts.url = C.CString(opts.Url) + return copts } -func freeProxyOptions(ptr *C.git_proxy_options) { - if ptr == nil { +func freeProxyOptions(copts *C.git_proxy_options) { + if copts == nil { return } - C.free(unsafe.Pointer(ptr.url)) + C.free(unsafe.Pointer(copts.url)) } // RemoteIsValidName returns whether the remote name is well-formed. @@ -738,35 +794,54 @@ func (o *Remote) RefspecCount() uint { return uint(count) } -func populateFetchOptions(options *C.git_fetch_options, opts *FetchOptions) { - C.git_fetch_options_init(options, C.GIT_FETCH_OPTIONS_VERSION) +func populateFetchOptions(copts *C.git_fetch_options, opts *FetchOptions, errorTarget *error) *C.git_fetch_options { + C.git_fetch_options_init(copts, C.GIT_FETCH_OPTIONS_VERSION) if opts == nil { - return + return nil } - populateRemoteCallbacks(&options.callbacks, &opts.RemoteCallbacks) - options.prune = C.git_fetch_prune_t(opts.Prune) - options.update_fetchhead = cbool(opts.UpdateFetchhead) - options.download_tags = C.git_remote_autotag_option_t(opts.DownloadTags) + populateRemoteCallbacks(&copts.callbacks, &opts.RemoteCallbacks, errorTarget) + copts.prune = C.git_fetch_prune_t(opts.Prune) + copts.update_fetchhead = cbool(opts.UpdateFetchhead) + copts.download_tags = C.git_remote_autotag_option_t(opts.DownloadTags) - options.custom_headers = C.git_strarray{} - options.custom_headers.count = C.size_t(len(opts.Headers)) - options.custom_headers.strings = makeCStringsFromStrings(opts.Headers) - populateProxyOptions(&options.proxy_opts, &opts.ProxyOptions) + copts.custom_headers = C.git_strarray{ + count: C.size_t(len(opts.Headers)), + strings: makeCStringsFromStrings(opts.Headers), + } + populateProxyOptions(&copts.proxy_opts, &opts.ProxyOptions) + return copts } -func populatePushOptions(options *C.git_push_options, opts *PushOptions) { - C.git_push_options_init(options, C.GIT_PUSH_OPTIONS_VERSION) - if opts == nil { +func freeFetchOptions(copts *C.git_fetch_options) { + if copts == nil { return } + freeStrarray(&copts.custom_headers) + untrackCallbacksPayload(&copts.callbacks) + freeProxyOptions(&copts.proxy_opts) +} - options.pb_parallelism = C.uint(opts.PbParallelism) +func populatePushOptions(copts *C.git_push_options, opts *PushOptions, errorTarget *error) *C.git_push_options { + C.git_push_options_init(copts, C.GIT_PUSH_OPTIONS_VERSION) + if opts == nil { + return nil + } - options.custom_headers = C.git_strarray{} - options.custom_headers.count = C.size_t(len(opts.Headers)) - options.custom_headers.strings = makeCStringsFromStrings(opts.Headers) + copts.pb_parallelism = C.uint(opts.PbParallelism) + copts.custom_headers = C.git_strarray{ + count: C.size_t(len(opts.Headers)), + strings: makeCStringsFromStrings(opts.Headers), + } + populateRemoteCallbacks(&copts.callbacks, &opts.RemoteCallbacks, errorTarget) + return copts +} - populateRemoteCallbacks(&options.callbacks, &opts.RemoteCallbacks) +func freePushOptions(copts *C.git_push_options) { + if copts == nil { + return + } + untrackCallbacksPayload(&copts.callbacks) + freeStrarray(&copts.custom_headers) } // Fetch performs a fetch operation. refspecs specifies which refspecs @@ -780,26 +855,29 @@ func (o *Remote) Fetch(refspecs []string, opts *FetchOptions, msg string) error defer C.free(unsafe.Pointer(cmsg)) } - crefspecs := C.git_strarray{} - crefspecs.count = C.size_t(len(refspecs)) - crefspecs.strings = makeCStringsFromStrings(refspecs) + var err error + crefspecs := C.git_strarray{ + count: C.size_t(len(refspecs)), + strings: makeCStringsFromStrings(refspecs), + } defer freeStrarray(&crefspecs) - coptions := (*C.git_fetch_options)(C.calloc(1, C.size_t(unsafe.Sizeof(C.git_fetch_options{})))) - defer C.free(unsafe.Pointer(coptions)) - - populateFetchOptions(coptions, opts) - defer untrackCalbacksPayload(&coptions.callbacks) - defer freeStrarray(&coptions.custom_headers) + coptions := populateFetchOptions(&C.git_fetch_options{}, opts, &err) + defer freeFetchOptions(coptions) runtime.LockOSThread() defer runtime.UnlockOSThread() ret := C.git_remote_fetch(o.ptr, &crefspecs, coptions, cmsg) runtime.KeepAlive(o) + + if ret == C.int(ErrorCodeUser) && err != nil { + return err + } if ret < 0 { return MakeGitError(ret) } + return nil } @@ -819,23 +897,27 @@ func (o *Remote) ConnectPush(callbacks *RemoteCallbacks, proxyOpts *ProxyOptions // // 'headers' are extra HTTP headers to use in this connection. func (o *Remote) Connect(direction ConnectDirection, callbacks *RemoteCallbacks, proxyOpts *ProxyOptions, headers []string) error { - var ccallbacks C.git_remote_callbacks - populateRemoteCallbacks(&ccallbacks, callbacks) + var err error + ccallbacks := populateRemoteCallbacks(&C.git_remote_callbacks{}, callbacks, &err) + defer untrackCallbacksPayload(ccallbacks) - var cproxy C.git_proxy_options - populateProxyOptions(&cproxy, proxyOpts) - defer freeProxyOptions(&cproxy) + cproxy := populateProxyOptions(&C.git_proxy_options{}, proxyOpts) + defer freeProxyOptions(cproxy) - cheaders := C.git_strarray{} - cheaders.count = C.size_t(len(headers)) - cheaders.strings = makeCStringsFromStrings(headers) + cheaders := C.git_strarray{ + count: C.size_t(len(headers)), + strings: makeCStringsFromStrings(headers), + } defer freeStrarray(&cheaders) runtime.LockOSThread() defer runtime.UnlockOSThread() - ret := C.git_remote_connect(o.ptr, C.git_direction(direction), &ccallbacks, &cproxy, &cheaders) + ret := C.git_remote_connect(o.ptr, C.git_direction(direction), ccallbacks, cproxy, &cheaders) runtime.KeepAlive(o) + if ret == C.int(ErrorCodeUser) && err != nil { + return err + } if ret != 0 { return MakeGitError(ret) } @@ -899,23 +981,24 @@ func (o *Remote) Ls(filterRefs ...string) ([]RemoteHead, error) { } func (o *Remote) Push(refspecs []string, opts *PushOptions) error { - crefspecs := C.git_strarray{} - crefspecs.count = C.size_t(len(refspecs)) - crefspecs.strings = makeCStringsFromStrings(refspecs) + crefspecs := C.git_strarray{ + count: C.size_t(len(refspecs)), + strings: makeCStringsFromStrings(refspecs), + } defer freeStrarray(&crefspecs) - coptions := (*C.git_push_options)(C.calloc(1, C.size_t(unsafe.Sizeof(C.git_push_options{})))) - defer C.free(unsafe.Pointer(coptions)) - - populatePushOptions(coptions, opts) - defer untrackCalbacksPayload(&coptions.callbacks) - defer freeStrarray(&coptions.custom_headers) + var err error + coptions := populatePushOptions(&C.git_push_options{}, opts, &err) + defer freePushOptions(coptions) runtime.LockOSThread() defer runtime.UnlockOSThread() ret := C.git_remote_push(o.ptr, &crefspecs, coptions) runtime.KeepAlive(o) + if ret == C.int(ErrorCodeUser) && err != nil { + return err + } if ret < 0 { return MakeGitError(ret) } @@ -927,14 +1010,18 @@ func (o *Remote) PruneRefs() bool { } func (o *Remote) Prune(callbacks *RemoteCallbacks) error { - var ccallbacks C.git_remote_callbacks - populateRemoteCallbacks(&ccallbacks, callbacks) + var err error + ccallbacks := populateRemoteCallbacks(&C.git_remote_callbacks{}, callbacks, &err) + defer untrackCallbacksPayload(ccallbacks) runtime.LockOSThread() defer runtime.UnlockOSThread() - ret := C.git_remote_prune(o.ptr, &ccallbacks) + ret := C.git_remote_prune(o.ptr, ccallbacks) runtime.KeepAlive(o) + if ret == C.int(ErrorCodeUser) && err != nil { + return err + } if ret < 0 { return MakeGitError(ret) } diff --git a/reset.go b/reset.go index 155b58b..6a2ed9d 100644 --- a/reset.go +++ b/reset.go @@ -19,7 +19,7 @@ func (r *Repository) ResetToCommit(commit *Commit, resetType ResetType, opts *Ch defer runtime.UnlockOSThread() var err error - cOpts := opts.toC(&err) + cOpts := populateCheckoutOptions(&C.git_checkout_options{}, opts, &err) defer freeCheckoutOptions(cOpts) ret := C.git_reset(r.ptr, commit.ptr, C.git_reset_t(resetType), cOpts) diff --git a/revert.go b/revert.go index f318ade..085df16 100644 --- a/revert.go +++ b/revert.go @@ -15,48 +15,47 @@ type RevertOptions struct { CheckoutOpts CheckoutOptions } -func (opts *RevertOptions) toC(errorTarget *error) *C.git_revert_options { +func populateRevertOptions(copts *C.git_revert_options, opts *RevertOptions, errorTarget *error) *C.git_revert_options { + C.git_revert_options_init(copts, C.GIT_REVERT_OPTIONS_VERSION) 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(errorTarget), - } + copts.mainline = C.uint(opts.Mainline) + populateMergeOptions(&copts.merge_opts, &opts.MergeOpts) + populateCheckoutOptions(&copts.checkout_opts, &opts.CheckoutOpts, errorTarget) + return copts } -func revertOptionsFromC(opts *C.git_revert_options) RevertOptions { +func revertOptionsFromC(copts *C.git_revert_options) RevertOptions { return RevertOptions{ - Mainline: uint(opts.mainline), - MergeOpts: mergeOptionsFromC(&opts.merge_opts), - CheckoutOpts: checkoutOptionsFromC(&opts.checkout_opts), + Mainline: uint(copts.mainline), + MergeOpts: mergeOptionsFromC(&copts.merge_opts), + CheckoutOpts: checkoutOptionsFromC(&copts.checkout_opts), } } -func freeRevertOptions(opts *C.git_revert_options) { - if opts != nil { +func freeRevertOptions(copts *C.git_revert_options) { + if copts != nil { return } - freeMergeOptions(&opts.merge_opts) - freeCheckoutOptions(&opts.checkout_opts) + freeMergeOptions(&copts.merge_opts) + freeCheckoutOptions(&copts.checkout_opts) } // DefaultRevertOptions initialises a RevertOptions struct with default values func DefaultRevertOptions() (RevertOptions, error) { - opts := C.git_revert_options{} + copts := C.git_revert_options{} runtime.LockOSThread() defer runtime.UnlockOSThread() - ecode := C.git_revert_options_init(&opts, C.GIT_REVERT_OPTIONS_VERSION) + ecode := C.git_revert_options_init(&copts, C.GIT_REVERT_OPTIONS_VERSION) if ecode < 0 { return RevertOptions{}, MakeGitError(ecode) } - defer freeRevertOptions(&opts) - return revertOptionsFromC(&opts), nil + defer freeRevertOptions(&copts) + return revertOptionsFromC(&copts), nil } // Revert the provided commit leaving the index updated with the results of the revert @@ -65,7 +64,7 @@ func (r *Repository) Revert(commit *Commit, revertOptions *RevertOptions) error defer runtime.UnlockOSThread() var err error - cOpts := revertOptions.toC(&err) + cOpts := populateRevertOptions(&C.git_revert_options{}, revertOptions, &err) defer freeRevertOptions(cOpts) ret := C.git_revert(r.ptr, commit.cast_ptr, cOpts) @@ -88,7 +87,7 @@ func (r *Repository) RevertCommit(revertCommit *Commit, ourCommit *Commit, mainl runtime.LockOSThread() defer runtime.UnlockOSThread() - cOpts := mergeOptions.toC() + cOpts := populateMergeOptions(&C.git_merge_options{}, mergeOptions) defer freeMergeOptions(cOpts) var index *C.git_index diff --git a/stash.go b/stash.go index 620e70f..2086e2e 100644 --- a/stash.go +++ b/stash.go @@ -161,34 +161,32 @@ func DefaultStashApplyOptions() (StashApplyOptions, error) { }, nil } -func (opts *StashApplyOptions) toC(errorTarget *error) *C.git_stash_apply_options { +func populateStashApplyOptions(copts *C.git_stash_apply_options, opts *StashApplyOptions, errorTarget *error) *C.git_stash_apply_options { + C.git_stash_apply_options_init(copts, C.GIT_STASH_APPLY_OPTIONS_VERSION) if opts == nil { return nil } - optsC := &C.git_stash_apply_options{ - version: C.GIT_STASH_APPLY_OPTIONS_VERSION, - flags: C.uint32_t(opts.Flags), - } - populateCheckoutOptions(&optsC.checkout_options, &opts.CheckoutOptions, errorTarget) + copts.flags = C.uint32_t(opts.Flags) + populateCheckoutOptions(&copts.checkout_options, &opts.CheckoutOptions, errorTarget) if opts.ProgressCallback != nil { progressData := &stashApplyProgressCallbackData{ callback: opts.ProgressCallback, errorTarget: errorTarget, } - C._go_git_populate_stash_apply_callbacks(optsC) - optsC.progress_payload = pointerHandles.Track(progressData) + C._go_git_populate_stash_apply_callbacks(copts) + copts.progress_payload = pointerHandles.Track(progressData) } - return optsC + return copts } -func freeStashApplyOptions(optsC *C.git_stash_apply_options) { - if optsC == nil { +func freeStashApplyOptions(copts *C.git_stash_apply_options) { + if copts == nil { return } - if optsC.progress_payload != nil { - pointerHandles.Untrack(optsC.progress_payload) + if copts.progress_payload != nil { + pointerHandles.Untrack(copts.progress_payload) } - freeCheckoutOptions(&optsC.checkout_options) + freeCheckoutOptions(&copts.checkout_options) } // Apply applies a single stashed state from the stash list. @@ -217,7 +215,7 @@ func freeStashApplyOptions(optsC *C.git_stash_apply_options) { // Error codes can be interogated with IsErrorCode(err, ErrorCodeNotFound). func (c *StashCollection) Apply(index int, opts StashApplyOptions) error { var err error - optsC := opts.toC(&err) + optsC := populateStashApplyOptions(&C.git_stash_apply_options{}, &opts, &err) defer freeStashApplyOptions(optsC) runtime.LockOSThread() @@ -320,7 +318,7 @@ func (c *StashCollection) Drop(index int) error { // state for the given index. func (c *StashCollection) Pop(index int, opts StashApplyOptions) error { var err error - optsC := opts.toC(&err) + optsC := populateStashApplyOptions(&C.git_stash_apply_options{}, &opts, &err) defer freeStashApplyOptions(optsC) runtime.LockOSThread() diff --git a/submodule.go b/submodule.go index 01474f6..2042125 100644 --- a/submodule.go +++ b/submodule.go @@ -383,22 +383,22 @@ func (sub *Submodule) Update(init bool, opts *SubmoduleUpdateOptions) error { return nil } -func populateSubmoduleUpdateOptions(ptr *C.git_submodule_update_options, opts *SubmoduleUpdateOptions, errorTarget *error) *C.git_submodule_update_options { - C.git_submodule_update_options_init(ptr, C.GIT_SUBMODULE_UPDATE_OPTIONS_VERSION) - +func populateSubmoduleUpdateOptions(copts *C.git_submodule_update_options, opts *SubmoduleUpdateOptions, errorTarget *error) *C.git_submodule_update_options { + C.git_submodule_update_options_init(copts, C.GIT_SUBMODULE_UPDATE_OPTIONS_VERSION) if opts == nil { return nil } - populateCheckoutOptions(&ptr.checkout_opts, opts.CheckoutOpts, errorTarget) - populateFetchOptions(&ptr.fetch_opts, opts.FetchOptions) + populateCheckoutOptions(&copts.checkout_opts, opts.CheckoutOpts, errorTarget) + populateFetchOptions(&copts.fetch_opts, opts.FetchOptions, errorTarget) - return ptr + return copts } -func freeSubmoduleUpdateOptions(ptr *C.git_submodule_update_options) { - if ptr == nil { +func freeSubmoduleUpdateOptions(copts *C.git_submodule_update_options) { + if copts == nil { return } - freeCheckoutOptions(&ptr.checkout_opts) + freeCheckoutOptions(&copts.checkout_opts) + freeFetchOptions(&copts.fetch_opts) } diff --git a/wrapper.c b/wrapper.c index f168425..282d9a8 100644 --- a/wrapper.c +++ b/wrapper.c @@ -14,7 +14,7 @@ // // // myfile.go // type FooCallback func(...) (..., error) -// type FooCallbackData struct { +// type fooCallbackData struct { // callback FooCallback // errorTarget *error // } @@ -60,20 +60,28 @@ // return git_my_function(..., (git_foo_cb)&fooCallback, payload); // } // -// * Otherwise, if the same callback can be invoked from multiple functions or -// from different stacks (e.g. when passing the callback to an object), a -// different pattern should be used instead, which has the downside of losing -// the original error object and converting it to a GitError: +// * Additionally, if the same callback can be invoked from multiple functions or +// from different stacks (e.g. when passing the callback to an object), the +// following pattern should be used in tandem, which has the downside of +// losing the original error object and converting it to a GitError if the +// callback happens from a different stack: // // // myfile.go // type FooCallback func(...) (..., error) +// type fooCallbackData struct { +// callback FooCallback +// errorTarget *error +// } // // //export fooCallback // func fooCallback(errorMessage **C.char, ..., handle unsafe.Pointer) C.int { -// callback := pointerHandles.Get(data).(*FooCallback) +// data := pointerHandles.Get(data).(*fooCallbackData) // ... -// err := callback(...) +// err := data.callback(...) // if err != nil { +// if data.errorTarget != nil { +// *data.errorTarget = err +// } // return setCallbackError(errorMessage, err) // } // return C.int(ErrorCodeOK)