Skip to content

Commit 5def02a

Browse files
lhchavezlhchavez
committed
The big Callback type adjustment of 2020
This change makes all callbacks that can fail return an `error`. This makes things a lot more idiomatic.
1 parent 70e5e41 commit 5def02a

13 files changed

+77
-89
lines changed

‎checkout.go

+5-6
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,6 @@ extern void _go_git_populate_checkout_callbacks(git_checkout_options *opts);
77
*/
88
import "C"
99
import (
10-
"errors"
1110
"os"
1211
"runtime"
1312
"unsafe"
@@ -49,8 +48,8 @@ const (
4948
CheckoutUpdateSubmodulesIfChanged CheckoutStrategy = C.GIT_CHECKOUT_UPDATE_SUBMODULES_IF_CHANGED // Recursively checkout submodules if HEAD moved in super repo (NOT IMPLEMENTED)
5049
)
5150

52-
type CheckoutNotifyCallback func(why CheckoutNotifyType, path string, baseline, target, workdir DiffFile) ErrorCode
53-
type CheckoutProgressCallback func(path string, completed, total uint) ErrorCode
51+
type CheckoutNotifyCallback func(why CheckoutNotifyType, path string, baseline, target, workdir DiffFile) error
52+
type CheckoutProgressCallback func(path string, completed, total uint)
5453

5554
type CheckoutOptions struct {
5655
Strategy CheckoutStrategy // Default will be a dry run
@@ -116,9 +115,9 @@ func checkoutNotifyCallback(
116115
if data.options.NotifyCallback == nil {
117116
return C.int(ErrorCodeOK)
118117
}
119-
ret := data.options.NotifyCallback(CheckoutNotifyType(why), path, baseline, target, workdir)
120-
if ret < 0 {
121-
*data.errorTarget = errors.New(ErrorCode(ret).String())
118+
err := data.options.NotifyCallback(CheckoutNotifyType(why), path, baseline, target, workdir)
119+
if err != nil {
120+
*data.errorTarget = err
122121
return C.int(ErrorCodeUser)
123122
}
124123
return C.int(ErrorCodeOK)

‎clone.go

+5-5
Original file line numberDiff line numberDiff line change
@@ -7,12 +7,11 @@ extern void _go_git_populate_clone_callbacks(git_clone_options *opts);
77
*/
88
import "C"
99
import (
10-
"errors"
1110
"runtime"
1211
"unsafe"
1312
)
1413

15-
type RemoteCreateCallback func(repo *Repository, name, url string) (*Remote, ErrorCode)
14+
type RemoteCreateCallback func(repo *Repository, name, url string) (*Remote, error)
1615

1716
type CloneOptions struct {
1817
*CheckoutOpts
@@ -71,9 +70,10 @@ func remoteCreateCallback(
7170
panic("invalid remote create callback")
7271
}
7372

74-
remote, ret := data.options.RemoteCreateCallback(repo, name, url)
75-
if ret < 0 {
76-
*data.errorTarget = errors.New(ErrorCode(ret).String())
73+
remote, err := data.options.RemoteCreateCallback(repo, name, url)
74+
75+
if err != nil {
76+
*data.errorTarget = err
7777
return C.int(ErrorCodeUser)
7878
}
7979
if remote == nil {

‎clone_test.go

+2-8
Original file line numberDiff line numberDiff line change
@@ -49,15 +49,9 @@ func TestCloneWithCallback(t *testing.T) {
4949

5050
opts := CloneOptions{
5151
Bare: true,
52-
RemoteCreateCallback: func(r *Repository, name, url string) (*Remote, ErrorCode) {
52+
RemoteCreateCallback: func(r *Repository, name, url string) (*Remote, error) {
5353
testPayload += 1
54-
55-
remote, err := r.Remotes.Create(REMOTENAME, url)
56-
if err != nil {
57-
return nil, ErrorCodeGeneric
58-
}
59-
60-
return remote, ErrorCodeOK
54+
return r.Remotes.Create(REMOTENAME, url)
6155
},
6256
}
6357

‎deprecated.go

+12-4
Original file line numberDiff line numberDiff line change
@@ -230,7 +230,11 @@ func SubmoduleVisitor(csub unsafe.Pointer, name *C.char, handle unsafe.Pointer)
230230
if !ok {
231231
panic("invalid submodule visitor callback")
232232
}
233-
return (C.int)(callback(sub, C.GoString(name)))
233+
err := callback(sub, C.GoString(name))
234+
if err != nil {
235+
return C.int(ErrorCodeUser)
236+
}
237+
return C.int(ErrorCodeOK)
234238
}
235239

236240
// tree.go
@@ -239,9 +243,13 @@ func SubmoduleVisitor(csub unsafe.Pointer, name *C.char, handle unsafe.Pointer)
239243
func CallbackGitTreeWalk(_root *C.char, entry *C.git_tree_entry, ptr unsafe.Pointer) C.int {
240244
root := C.GoString(_root)
241245

242-
if callback, ok := pointerHandles.Get(ptr).(TreeWalkCallback); ok {
243-
return C.int(callback(root, newTreeEntry(entry)))
244-
} else {
246+
callback, ok := pointerHandles.Get(ptr).(TreeWalkCallback)
247+
if !ok {
245248
panic("invalid treewalk callback")
246249
}
250+
err := callback(root, newTreeEntry(entry))
251+
if err != nil {
252+
return C.int(ErrorCodeUser)
253+
}
254+
return C.int(ErrorCodeOK)
247255
}

‎index.go

+4-5
Original file line numberDiff line numberDiff line change
@@ -10,13 +10,12 @@ extern int _go_git_index_remove_all(git_index*, const git_strarray*, void*);
1010
*/
1111
import "C"
1212
import (
13-
"errors"
1413
"fmt"
1514
"runtime"
1615
"unsafe"
1716
)
1817

19-
type IndexMatchedPathCallback func(string, string) int
18+
type IndexMatchedPathCallback func(string, string) error
2019
type indexMatchedPathCallbackData struct {
2120
callback IndexMatchedPathCallback
2221
errorTarget *error
@@ -343,9 +342,9 @@ func indexMatchedPathCallback(cPath, cMatchedPathspec *C.char, payload unsafe.Po
343342
panic("invalid matched path callback")
344343
}
345344

346-
ret := data.callback(C.GoString(cPath), C.GoString(cMatchedPathspec))
347-
if ret < 0 {
348-
*data.errorTarget = errors.New(ErrorCode(ret).String())
345+
err := data.callback(C.GoString(cPath), C.GoString(cMatchedPathspec))
346+
if err != nil {
347+
*data.errorTarget = err
349348
return C.int(ErrorCodeUser)
350349
}
351350

‎index_test.go

+2-2
Original file line numberDiff line numberDiff line change
@@ -223,9 +223,9 @@ func TestIndexAddAllCallback(t *testing.T) {
223223
checkFatal(t, err)
224224

225225
cbPath := ""
226-
err = idx.AddAll([]string{}, IndexAddDefault, func(p, mP string) int {
226+
err = idx.AddAll([]string{}, IndexAddDefault, func(p, mP string) error {
227227
cbPath = p
228-
return 0
228+
return nil
229229
})
230230
checkFatal(t, err)
231231
if cbPath != "README" {

‎indexer_test.go

+2-2
Original file line numberDiff line numberDiff line change
@@ -33,9 +33,9 @@ func TestIndexerOutOfOrder(t *testing.T) {
3333
defer os.RemoveAll(tmpPath)
3434

3535
var finalStats TransferProgress
36-
idx, err := NewIndexer(tmpPath, nil, func(stats TransferProgress) ErrorCode {
36+
idx, err := NewIndexer(tmpPath, nil, func(stats TransferProgress) error {
3737
finalStats = stats
38-
return ErrorCodeOK
38+
return nil
3939
})
4040
checkFatal(t, err)
4141
defer idx.Free()

‎odb_test.go

+2-2
Original file line numberDiff line numberDiff line change
@@ -167,9 +167,9 @@ func TestOdbWritepack(t *testing.T) {
167167
checkFatal(t, err)
168168

169169
var finalStats TransferProgress
170-
writepack, err := odb.NewWritePack(func(stats TransferProgress) ErrorCode {
170+
writepack, err := odb.NewWritePack(func(stats TransferProgress) error {
171171
finalStats = stats
172-
return ErrorCodeOK
172+
return nil
173173
})
174174
checkFatal(t, err)
175175
defer writepack.Free()

‎remote.go

+25-34
Original file line numberDiff line numberDiff line change
@@ -69,15 +69,15 @@ const (
6969
ConnectDirectionPush ConnectDirection = C.GIT_DIRECTION_PUSH
7070
)
7171

72-
type TransportMessageCallback func(str string) ErrorCode
73-
type CompletionCallback func(RemoteCompletion) ErrorCode
72+
type TransportMessageCallback func(str string) error
73+
type CompletionCallback func(RemoteCompletion) error
7474
type CredentialsCallback func(url string, username_from_url string, allowed_types CredentialType) (*Credential, error)
75-
type TransferProgressCallback func(stats TransferProgress) ErrorCode
76-
type UpdateTipsCallback func(refname string, a *Oid, b *Oid) ErrorCode
77-
type CertificateCheckCallback func(cert *Certificate, valid bool, hostname string) ErrorCode
78-
type PackbuilderProgressCallback func(stage int32, current, total uint32) ErrorCode
79-
type PushTransferProgressCallback func(current, total uint32, bytes uint) ErrorCode
80-
type PushUpdateReferenceCallback func(refname, status string) ErrorCode
75+
type TransferProgressCallback func(stats TransferProgress) error
76+
type UpdateTipsCallback func(refname string, a *Oid, b *Oid) error
77+
type CertificateCheckCallback func(cert *Certificate, valid bool, hostname string) error
78+
type PackbuilderProgressCallback func(stage int32, current, total uint32) error
79+
type PushTransferProgressCallback func(current, total uint32, bytes uint) error
80+
type PushUpdateReferenceCallback func(refname, status string) error
8181

8282
type RemoteCallbacks struct {
8383
SidebandProgressCallback TransportMessageCallback
@@ -329,10 +329,8 @@ func sidebandProgressCallback(errorMessage **C.char, _str *C.char, _len C.int, h
329329
if data.callbacks.SidebandProgressCallback == nil {
330330
return C.int(ErrorCodeOK)
331331
}
332-
str := C.GoStringN(_str, _len)
333-
ret := data.callbacks.SidebandProgressCallback(str)
334-
if ret < 0 {
335-
err := errors.New(ErrorCode(ret).String())
332+
err := data.callbacks.SidebandProgressCallback(C.GoStringN(_str, _len))
333+
if err != nil {
336334
if data.errorTarget != nil {
337335
*data.errorTarget = err
338336
}
@@ -342,14 +340,13 @@ func sidebandProgressCallback(errorMessage **C.char, _str *C.char, _len C.int, h
342340
}
343341

344342
//export completionCallback
345-
func completionCallback(errorMessage **C.char, completion_type C.git_remote_completion_type, handle unsafe.Pointer) C.int {
343+
func completionCallback(errorMessage **C.char, completionType C.git_remote_completion_type, handle unsafe.Pointer) C.int {
346344
data := pointerHandles.Get(handle).(*remoteCallbacksData)
347345
if data.callbacks.CompletionCallback == nil {
348346
return C.int(ErrorCodeOK)
349347
}
350-
ret := data.callbacks.CompletionCallback(RemoteCompletion(completion_type))
351-
if ret < 0 {
352-
err := errors.New(ErrorCode(ret).String())
348+
err := data.callbacks.CompletionCallback(RemoteCompletion(completionType))
349+
if err != nil {
353350
if data.errorTarget != nil {
354351
*data.errorTarget = err
355352
}
@@ -396,9 +393,8 @@ func transferProgressCallback(errorMessage **C.char, stats *C.git_transfer_progr
396393
if data.callbacks.TransferProgressCallback == nil {
397394
return C.int(ErrorCodeOK)
398395
}
399-
ret := data.callbacks.TransferProgressCallback(newTransferProgressFromC(stats))
400-
if ret < 0 {
401-
err := errors.New(ErrorCode(ret).String())
396+
err := data.callbacks.TransferProgressCallback(newTransferProgressFromC(stats))
397+
if err != nil {
402398
if data.errorTarget != nil {
403399
*data.errorTarget = err
404400
}
@@ -422,9 +418,8 @@ func updateTipsCallback(
422418
refname := C.GoString(_refname)
423419
a := newOidFromC(_a)
424420
b := newOidFromC(_b)
425-
ret := data.callbacks.UpdateTipsCallback(refname, a, b)
426-
if ret < 0 {
427-
err := errors.New(ErrorCode(ret).String())
421+
err := data.callbacks.UpdateTipsCallback(refname, a, b)
422+
if err != nil {
428423
if data.errorTarget != nil {
429424
*data.errorTarget = err
430425
}
@@ -489,9 +484,8 @@ func certificateCheckCallback(
489484
return setCallbackError(errorMessage, err)
490485
}
491486

492-
ret := data.callbacks.CertificateCheckCallback(&cert, valid, host)
493-
if ret < 0 {
494-
err := errors.New(ErrorCode(ret).String())
487+
err := data.callbacks.CertificateCheckCallback(&cert, valid, host)
488+
if err != nil {
495489
if data.errorTarget != nil {
496490
*data.errorTarget = err
497491
}
@@ -507,9 +501,8 @@ func packProgressCallback(errorMessage **C.char, stage C.int, current, total C.u
507501
return C.int(ErrorCodeOK)
508502
}
509503

510-
ret := data.callbacks.PackProgressCallback(int32(stage), uint32(current), uint32(total))
511-
if ret < 0 {
512-
err := errors.New(ErrorCode(ret).String())
504+
err := data.callbacks.PackProgressCallback(int32(stage), uint32(current), uint32(total))
505+
if err != nil {
513506
if data.errorTarget != nil {
514507
*data.errorTarget = err
515508
}
@@ -525,9 +518,8 @@ func pushTransferProgressCallback(errorMessage **C.char, current, total C.uint,
525518
return C.int(ErrorCodeOK)
526519
}
527520

528-
ret := data.callbacks.PushTransferProgressCallback(uint32(current), uint32(total), uint(bytes))
529-
if ret < 0 {
530-
err := errors.New(ErrorCode(ret).String())
521+
err := data.callbacks.PushTransferProgressCallback(uint32(current), uint32(total), uint(bytes))
522+
if err != nil {
531523
if data.errorTarget != nil {
532524
*data.errorTarget = err
533525
}
@@ -543,9 +535,8 @@ func pushUpdateReferenceCallback(errorMessage **C.char, refname, status *C.char,
543535
return C.int(ErrorCodeOK)
544536
}
545537

546-
ret := data.callbacks.PushUpdateReferenceCallback(C.GoString(refname), C.GoString(status))
547-
if ret < 0 {
548-
err := errors.New(ErrorCode(ret).String())
538+
err := data.callbacks.PushUpdateReferenceCallback(C.GoString(refname), C.GoString(status))
539+
if err != nil {
549540
if data.errorTarget != nil {
550541
*data.errorTarget = err
551542
}

‎remote_test.go

+8-9
Original file line numberDiff line numberDiff line change
@@ -38,13 +38,13 @@ func TestListRemotes(t *testing.T) {
3838
compareStringList(t, expected, actual)
3939
}
4040

41-
func assertHostname(cert *Certificate, valid bool, hostname string, t *testing.T) ErrorCode {
41+
func assertHostname(cert *Certificate, valid bool, hostname string, t *testing.T) error {
4242
if hostname != "github.com" {
43-
t.Fatal("Hostname does not match")
44-
return ErrorCodeUser
43+
t.Fatal("hostname does not match")
44+
return errors.New("hostname does not match")
4545
}
4646

47-
return ErrorCodeOK
47+
return nil
4848
}
4949

5050
func TestCertificateCheck(t *testing.T) {
@@ -58,7 +58,7 @@ func TestCertificateCheck(t *testing.T) {
5858

5959
options := FetchOptions{
6060
RemoteCallbacks: RemoteCallbacks{
61-
CertificateCheckCallback: func(cert *Certificate, valid bool, hostname string) ErrorCode {
61+
CertificateCheckCallback: func(cert *Certificate, valid bool, hostname string) error {
6262
return assertHostname(cert, valid, hostname, t)
6363
},
6464
},
@@ -479,14 +479,13 @@ func TestRemoteSSH(t *testing.T) {
479479
certificateCheckCallbackCalled := false
480480
fetchOpts := FetchOptions{
481481
RemoteCallbacks: RemoteCallbacks{
482-
CertificateCheckCallback: func(cert *Certificate, valid bool, hostname string) ErrorCode {
482+
CertificateCheckCallback: func(cert *Certificate, valid bool, hostname string) error {
483483
hostkeyFingerprint := fmt.Sprintf("%x", cert.Hostkey.HashMD5[:])
484484
if hostkeyFingerprint != publicKeyFingerprint {
485-
t.Logf("server hostkey %q, want %q", hostkeyFingerprint, publicKeyFingerprint)
486-
return ErrorCodeAuth
485+
return fmt.Errorf("server hostkey %q, want %q", hostkeyFingerprint, publicKeyFingerprint)
487486
}
488487
certificateCheckCallbackCalled = true
489-
return ErrorCodeOK
488+
return nil
490489
},
491490
CredentialsCallback: func(url, username string, allowedTypes CredentialType) (*Credential, error) {
492491
if allowedTypes&(CredentialTypeSSHKey|CredentialTypeSSHCustom|CredentialTypeSSHMemory) != 0 {

‎submodule.go

+4-5
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,6 @@ extern int _go_git_visit_submodule(git_repository *repo, void *fct);
88
import "C"
99

1010
import (
11-
"errors"
1211
"runtime"
1312
"unsafe"
1413
)
@@ -111,7 +110,7 @@ func (c *SubmoduleCollection) Lookup(name string) (*Submodule, error) {
111110
}
112111

113112
// SubmoduleCallback is a function that is called for every submodule found in SubmoduleCollection.Foreach.
114-
type SubmoduleCallback func(sub *Submodule, name string) int
113+
type SubmoduleCallback func(sub *Submodule, name string) error
115114
type submoduleCallbackData struct {
116115
callback SubmoduleCallback
117116
errorTarget *error
@@ -126,9 +125,9 @@ func submoduleCallback(csub unsafe.Pointer, name *C.char, handle unsafe.Pointer)
126125
panic("invalid submodule visitor callback")
127126
}
128127

129-
ret := data.callback(sub, C.GoString(name))
130-
if ret < 0 {
131-
*data.errorTarget = errors.New(ErrorCode(ret).String())
128+
err := data.callback(sub, C.GoString(name))
129+
if err != nil {
130+
*data.errorTarget = err
132131
return C.int(ErrorCodeUser)
133132
}
134133

‎submodule_test.go

+2-2
Original file line numberDiff line numberDiff line change
@@ -15,9 +15,9 @@ func TestSubmoduleForeach(t *testing.T) {
1515
checkFatal(t, err)
1616

1717
i := 0
18-
err = repo.Submodules.Foreach(func(sub *Submodule, name string) int {
18+
err = repo.Submodules.Foreach(func(sub *Submodule, name string) error {
1919
i++
20-
return 0
20+
return nil
2121
})
2222
checkFatal(t, err)
2323

0 commit comments

Comments
 (0)