Merge pull request #90 from libgit2/unchanify

Remove usage of channels for `ForEach`

odb.go

@@ -84,30 +84,40 @@ func (v *Odb) Read(oid *Oid) (obj *OdbObject, err error) {
 	return obj, nil
 }
 
+type OdbForEachCallback func(id *Oid) error
+
+type foreachData struct {
+	callback OdbForEachCallback
+	err error
+}
+
 //export odbForEachCb
 func odbForEachCb(id *C.git_oid, payload unsafe.Pointer) int {
-	ch := *(*chan *Oid)(payload)
-	oid := newOidFromC(id)
-	// Because the channel is unbuffered, we never read our own data. If ch is
-	// readable, the user has sent something on it, which means we should
-	// abort.
-	select {
-	case ch <- oid:
-	case <-ch:
-		return -1
+	data := (*foreachData)(payload)
+
+	err := data.callback(newOidFromC(id))
+	if err != nil {
+		data.err = err
+		return C.GIT_EUSER
 	}
+
 	return 0
 }
 
-func (v *Odb) forEachWrap(ch chan *Oid) {
-	C._go_git_odb_foreach(v.ptr, unsafe.Pointer(&ch))
-	close(ch)
-}
+func (v *Odb) ForEach(callback OdbForEachCallback) error {
+	data := foreachData {
+		callback: callback,
+		err: nil,
+	}
 
-func (v *Odb) ForEach() chan *Oid {
-	ch := make(chan *Oid, 0)
-	go v.forEachWrap(ch)
-	return ch
+	ret := C._go_git_odb_foreach(v.ptr, unsafe.Pointer(&data))
+	if ret == C.GIT_EUSER {
+		return data.err
+	} else if ret < 0 {
+		return MakeGitError(ret)
+	}
+
+	return nil
 }
 
 // Hash determines the object-ID (sha1) of a data buffer.

odb_test.go

@@ -3,6 +3,7 @@ package git
 import (
 	"io"
 	"os"
+	"errors"
 	"testing"
 )
 
@@ -48,7 +49,7 @@ parent 66e1c476199ebcd3e304659992233132c5a52c6c
 author John Doe <john@doe.com> 1390682018 +0000
 committer John Doe <john@doe.com> 1390682018 +0000
 
-Initial commit.`;
+Initial commit.`
 
 	oid, error := odb.Hash([]byte(str), ObjectCommit)
 	checkFatal(t, error)
@@ -60,3 +61,36 @@ Initial commit.`;
 		t.Fatal("Hash and write Oids are different")
 	}
 }
+
+func TestOdbForeach(t *testing.T) {
+	repo := createTestRepo(t)
+	defer os.RemoveAll(repo.Workdir())
+	_, _ = seedTestRepo(t, repo)
+
+	odb, err := repo.Odb()
+	checkFatal(t, err)
+
+	expect := 3
+	count := 0
+	err = odb.ForEach(func(id *Oid) error {
+		count++
+		return nil
+	})
+
+	checkFatal(t, err)
+	if count != expect {
+		t.Fatalf("Expected %v objects, got %v")
+	}
+
+	expect = 1
+	count = 0
+	to_return := errors.New("not really an error")
+	err = odb.ForEach(func(id *Oid) error {
+		count++
+		return to_return
+	})
+
+	if err != to_return {
+		t.Fatalf("Odb.ForEach() did not return the expected error, got %v", err)
+	}
+}

packbuilder.go

@@ -94,55 +94,51 @@ func (pb *Packbuilder) WriteToFile(name string, mode os.FileMode) error {
 }
 
 func (pb *Packbuilder) Write(w io.Writer) error {
-	ch, stop := pb.ForEach()
-	for slice := range ch {
+	return pb.ForEach(func(slice []byte) error {
 		_, err := w.Write(slice)
-		if err != nil {
-			close(stop)
-			return err
-		}
-	}
-	return nil
+		return err
+	})
 }
 
 func (pb *Packbuilder) Written() uint32 {
 	return uint32(C.git_packbuilder_written(pb.ptr))
 }
 
+type PackbuilderForeachCallback func([]byte) error
 type packbuilderCbData struct {
-	ch chan<- []byte
-	stop <-chan bool
+	callback PackbuilderForeachCallback
+	err      error
 }
 
 //export packbuilderForEachCb
 func packbuilderForEachCb(buf unsafe.Pointer, size C.size_t, payload unsafe.Pointer) int {
 	data := (*packbuilderCbData)(payload)
-	ch := data.ch
-	stop := data.stop
-
 	slice := C.GoBytes(buf, C.int(size))
-	select {
-	case <- stop:
-		return -1
-	case ch <- slice:
+
+	err := data.callback(slice)
+	if err != nil {
+		data.err = err
+		return C.GIT_EUSER
 	}
 
 	return 0
 }
 
-func (pb *Packbuilder) forEachWrap(data *packbuilderCbData) {
-	C._go_git_packbuilder_foreach(pb.ptr, unsafe.Pointer(data))
-	close(data.ch)
-}
+// ForEach repeatedly calls the callback with new packfile data until
+// there is no more data or the callback returns an error
+func (pb *Packbuilder) ForEach(callback PackbuilderForeachCallback) error {
+	data := packbuilderCbData{
+		callback: callback,
+		err: nil,
+	}
 
-// Foreach sends the packfile as slices through the "data" channel. If
-// you want to stop the pack-building process (e.g. there's an error
-// writing to the output), close or write a value into the "stop"
-// channel.
-func (pb *Packbuilder) ForEach() (<-chan []byte, chan<- bool) {
-	ch := make(chan []byte)
-	stop := make(chan bool)
-	data := packbuilderCbData{ch, stop}
-	go pb.forEachWrap(&data)
-	return ch, stop
+	err := C._go_git_packbuilder_foreach(pb.ptr, unsafe.Pointer(&data))
+	if err == C.GIT_EUSER {
+		return data.err
+	}
+	if err < 0 {
+		return MakeGitError(err)
+	}
+
+	return nil
 }