Merge pull request #1955 from git-lfs/zero-bytes-forever

progress: fix never reading bytes with sufficiently small files

progress/copycallback.go

@@ -31,12 +31,11 @@ func (r *bodyWithCallback) Read(p []byte) (int, error) {
 
 	if n > 0 {
 		r.readSize += int64(n)
-	}
 
-	if err == nil && r.c != nil {
-		err = r.c(r.totalSize, r.readSize, n)
+		if (err == nil || err == io.EOF) && r.c != nil {
+			err = r.c(r.totalSize, r.readSize, n)
+		}
 	}
-
 	return n, err
 }
 
@@ -52,12 +51,11 @@ func (w *CallbackReader) Read(p []byte) (int, error) {
 
 	if n > 0 {
 		w.ReadSize += int64(n)
-	}
 
-	if err == nil && w.C != nil {
-		err = w.C(w.TotalSize, w.ReadSize, n)
+		if (err == nil || err == io.EOF) && w.C != nil {
+			err = w.C(w.TotalSize, w.ReadSize, n)
+		}
 	}
-
 	return n, err
 }
 

progress/copycallback_test.go

@@ -0,0 +1,74 @@
+package progress
+
+import (
+	"io"
+	"sync/atomic"
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+)
+
+func TestCopyCallbackReaderCallsCallbackUnderfilledBuffer(t *testing.T) {
+	var (
+		calls               uint32
+		actualTotalSize     int64
+		actualReadSoFar     int64
+		actualReadSinceLast int
+	)
+
+	cb := func(totalSize int64, readSoFar int64, readSinceLast int) error {
+		atomic.AddUint32(&calls, 1)
+
+		actualTotalSize = totalSize
+		actualReadSoFar = readSoFar
+		actualReadSinceLast = readSinceLast
+
+		return nil
+	}
+
+	buf := []byte{0x1}
+	r := &CallbackReader{
+		C:         cb,
+		TotalSize: 3,
+		ReadSize:  2,
+		Reader:    &EOFReader{b: buf},
+	}
+
+	p := make([]byte, len(buf)+1)
+	n, err := r.Read(p)
+
+	assert.Equal(t, 1, n)
+	assert.Nil(t, err)
+
+	assert.EqualValues(t, 1, calls, "expected 1 call(s) to callback, got %d", calls)
+	assert.EqualValues(t, 3, actualTotalSize)
+	assert.EqualValues(t, 2+1, actualReadSoFar)
+	assert.EqualValues(t, 1, actualReadSinceLast)
+}
+
+type EOFReader struct {
+	b []byte
+	i int
+}
+
+var _ io.Reader = (*EOFReader)(nil)
+
+func (r *EOFReader) Read(p []byte) (n int, err error) {
+	n = copy(p, r.b[r.i:])
+	r.i += n
+
+	if r.i == len(r.b) {
+		err = io.EOF
+	}
+	return
+}
+
+func TestEOFReaderReturnsEOFs(t *testing.T) {
+	r := EOFReader{[]byte{0x1}, 0}
+
+	p := make([]byte, 2)
+	n, err := r.Read(p)
+
+	assert.Equal(t, 1, n)
+	assert.Equal(t, io.EOF, err)
+}

test/test-progress-meter.sh

@@ -0,0 +1,29 @@
+#!/usr/bin/env bash
+
+. "test/testlib.sh"
+
+begin_test "progress meter displays positive progress"
+(
+  set -e
+
+  reponame="progress-meter"
+  setup_remote_repo "$reponame"
+  clone_repo "$reponame" "$reponame"
+
+  git lfs track "*.dat"
+  git add .gitattributes
+  git commit -m "initial commit"
+
+  for i in `seq 1 128`; do
+    printf "$i" > "$i.dat"
+  done
+
+  git add *.dat
+  git commit -m "add many objects"
+
+  git push origin master 2>&1 | tee push.log
+  [ "0" -eq "${PIPESTATUS[0]}" ]
+
+  grep "Git LFS: (128 of 128 files) 276 B / 276 B" push.log
+)
+end_test