check: improved reporting of differences in sizes and contents

fixes rclone check --download not showing differing files

cmd/bisync/checkfn.go

@@ -125,12 +125,12 @@ func (b *bisyncRun) ReverseCryptCheckFn(ctx context.Context, dst, src fs.Object)
 }
 
 // DownloadCheckFn is a slightly modified version of Check with --download
-func DownloadCheckFn(ctx context.Context, a, b fs.Object) (differ bool, noHash bool, err error) {
+func DownloadCheckFn(ctx context.Context, dst, src fs.Object) (equal bool, noHash bool, err error) {
-	differ, err = operations.CheckIdenticalDownload(ctx, a, b)
+	equal, err = operations.CheckIdenticalDownload(ctx, src, dst)
 	if err != nil {
 		return true, true, fmt.Errorf("failed to download: %w", err)
 	}
-	return differ, false, nil
+	return equal, false, nil
 }
 
 // check potential conflicts (to avoid renaming if already identical)

fs/operations/check.go

@@ -296,8 +296,8 @@ func Check(ctx context.Context, opt *CheckOpt) error {
 // CheckEqualReaders checks to see if in1 and in2 have the same
 // content when read.
 //
-// it returns true if differences were found
+// it returns true if no differences were found
-func CheckEqualReaders(in1, in2 io.Reader) (differ bool, err error) {
+func CheckEqualReaders(in1, in2 io.Reader) (equal bool, err error) {
 	const bufSize = 64 * 1024
 	buf1 := make([]byte, bufSize)
 	buf2 := make([]byte, bufSize)
@@ -306,42 +306,42 @@ func CheckEqualReaders(in1, in2 io.Reader) (differ bool, err error) {
 		n2, err2 := readers.ReadFill(in2, buf2)
 		// check errors
 		if err1 != nil && err1 != io.EOF {
-			return true, err1
+			return false, err1
 		} else if err2 != nil && err2 != io.EOF {
-			return true, err2
+			return false, err2
 		}
 		// err1 && err2 are nil or io.EOF here
 		// process the data
 		if n1 != n2 || !bytes.Equal(buf1[:n1], buf2[:n2]) {
-			return true, nil
+			return false, nil
 		}
 		// if both streams finished the we have finished
 		if err1 == io.EOF && err2 == io.EOF {
 			break
 		}
 	}
-	return false, nil
+	return true, nil
 }
 
 // CheckIdenticalDownload checks to see if dst and src are identical
 // by reading all their bytes if necessary.
 //
-// it returns true if differences were found
+// it returns true if no differences were found
-func CheckIdenticalDownload(ctx context.Context, dst, src fs.Object) (differ bool, err error) {
+func CheckIdenticalDownload(ctx context.Context, src, dst fs.Object) (equal bool, err error) {
 	ci := fs.GetConfig(ctx)
 	err = Retry(ctx, src, ci.LowLevelRetries, func() error {
-		differ, err = checkIdenticalDownload(ctx, dst, src)
+		equal, err = checkIdenticalDownload(ctx, src, dst)
 		return err
 	})
-	return differ, err
+	return equal, err
 }
 
 // Does the work for CheckIdenticalDownload
-func checkIdenticalDownload(ctx context.Context, dst, src fs.Object) (differ bool, err error) {
+func checkIdenticalDownload(ctx context.Context, src, dst fs.Object) (equal bool, err error) {
 	var in1, in2 io.ReadCloser
 	in1, err = Open(ctx, dst)
 	if err != nil {
-		return true, fmt.Errorf("failed to open %q: %w", dst, err)
+		return false, fmt.Errorf("failed to open %q: %w", dst, err)
 	}
 	tr1 := accounting.Stats(ctx).NewTransfer(dst, nil)
 	defer func() {
@@ -351,7 +351,7 @@ func checkIdenticalDownload(ctx context.Context, dst, src fs.Object) (differ boo
 
 	in2, err = Open(ctx, src)
 	if err != nil {
-		return true, fmt.Errorf("failed to open %q: %w", src, err)
+		return false, fmt.Errorf("failed to open %q: %w", src, err)
 	}
 	tr2 := accounting.Stats(ctx).NewTransfer(dst, nil)
 	defer func() {
@@ -360,7 +360,7 @@ func checkIdenticalDownload(ctx context.Context, dst, src fs.Object) (differ boo
 	in2 = tr2.Account(ctx, in2).WithBuffer() // account and buffer the transfer
 
 	// To assign err variable before defer.
-	differ, err = CheckEqualReaders(in1, in2)
+	equal, err = CheckEqualReaders(in1, in2)
 	return
 }
 
@@ -368,12 +368,17 @@ func checkIdenticalDownload(ctx context.Context, dst, src fs.Object) (differ boo
 // and the actual contents of the files.
 func CheckDownload(ctx context.Context, opt *CheckOpt) error {
 	optCopy := *opt
-	optCopy.Check = func(ctx context.Context, a, b fs.Object) (differ bool, noHash bool, err error) {
+	optCopy.Check = func(ctx context.Context, dst, src fs.Object) (differ bool, noHash bool, err error) {
-		differ, err = CheckIdenticalDownload(ctx, a, b)
+		same, err := CheckIdenticalDownload(ctx, src, dst)
 		if err != nil {
 			return true, true, fmt.Errorf("failed to download: %w", err)
 		}
-		return differ, false, nil
+		if !same {
+			err = errors.New("contents differ")
+			fs.Errorf(src, "%v", err)
+			return true, false, nil
+		}
+		return false, false, nil
 	}
 	return CheckFn(ctx, &optCopy)
 }

fs/operations/check_test.go

@@ -218,21 +218,21 @@ func TestCheckEqualReaders(t *testing.T) {
 	b65b[len(b65b)-1] = 1
 	b66 := make([]byte, 66*1024)
 
-	differ, err := operations.CheckEqualReaders(bytes.NewBuffer(b65a), bytes.NewBuffer(b65a))
+	equal, err := operations.CheckEqualReaders(bytes.NewBuffer(b65a), bytes.NewBuffer(b65a))
 	assert.NoError(t, err)
-	assert.Equal(t, differ, false)
+	assert.Equal(t, equal, true)
 
-	differ, err = operations.CheckEqualReaders(bytes.NewBuffer(b65a), bytes.NewBuffer(b65b))
+	equal, err = operations.CheckEqualReaders(bytes.NewBuffer(b65a), bytes.NewBuffer(b65b))
 	assert.NoError(t, err)
-	assert.Equal(t, differ, true)
+	assert.Equal(t, equal, false)
 
-	differ, err = operations.CheckEqualReaders(bytes.NewBuffer(b65a), bytes.NewBuffer(b66))
+	equal, err = operations.CheckEqualReaders(bytes.NewBuffer(b65a), bytes.NewBuffer(b66))
 	assert.NoError(t, err)
-	assert.Equal(t, differ, true)
+	assert.Equal(t, equal, false)
 
-	differ, err = operations.CheckEqualReaders(bytes.NewBuffer(b66), bytes.NewBuffer(b65a))
+	equal, err = operations.CheckEqualReaders(bytes.NewBuffer(b66), bytes.NewBuffer(b65a))
 	assert.NoError(t, err)
-	assert.Equal(t, differ, true)
+	assert.Equal(t, equal, false)
 
 	myErr := errors.New("sentinel")
 	wrap := func(b []byte) io.Reader {
@@ -241,37 +241,37 @@ func TestCheckEqualReaders(t *testing.T) {
 		return io.MultiReader(r, e)
 	}
 
-	differ, err = operations.CheckEqualReaders(wrap(b65a), bytes.NewBuffer(b65a))
+	equal, err = operations.CheckEqualReaders(wrap(b65a), bytes.NewBuffer(b65a))
 	assert.Equal(t, myErr, err)
-	assert.Equal(t, differ, true)
+	assert.Equal(t, equal, false)
 
-	differ, err = operations.CheckEqualReaders(wrap(b65a), bytes.NewBuffer(b65b))
+	equal, err = operations.CheckEqualReaders(wrap(b65a), bytes.NewBuffer(b65b))
 	assert.Equal(t, myErr, err)
-	assert.Equal(t, differ, true)
+	assert.Equal(t, equal, false)
 
-	differ, err = operations.CheckEqualReaders(wrap(b65a), bytes.NewBuffer(b66))
+	equal, err = operations.CheckEqualReaders(wrap(b65a), bytes.NewBuffer(b66))
 	assert.Equal(t, myErr, err)
-	assert.Equal(t, differ, true)
+	assert.Equal(t, equal, false)
 
-	differ, err = operations.CheckEqualReaders(wrap(b66), bytes.NewBuffer(b65a))
+	equal, err = operations.CheckEqualReaders(wrap(b66), bytes.NewBuffer(b65a))
 	assert.Equal(t, myErr, err)
-	assert.Equal(t, differ, true)
+	assert.Equal(t, equal, false)
 
-	differ, err = operations.CheckEqualReaders(bytes.NewBuffer(b65a), wrap(b65a))
+	equal, err = operations.CheckEqualReaders(bytes.NewBuffer(b65a), wrap(b65a))
 	assert.Equal(t, myErr, err)
-	assert.Equal(t, differ, true)
+	assert.Equal(t, equal, false)
 
-	differ, err = operations.CheckEqualReaders(bytes.NewBuffer(b65a), wrap(b65b))
+	equal, err = operations.CheckEqualReaders(bytes.NewBuffer(b65a), wrap(b65b))
 	assert.Equal(t, myErr, err)
-	assert.Equal(t, differ, true)
+	assert.Equal(t, equal, false)
 
-	differ, err = operations.CheckEqualReaders(bytes.NewBuffer(b65a), wrap(b66))
+	equal, err = operations.CheckEqualReaders(bytes.NewBuffer(b65a), wrap(b66))
 	assert.Equal(t, myErr, err)
-	assert.Equal(t, differ, true)
+	assert.Equal(t, equal, false)
 
-	differ, err = operations.CheckEqualReaders(bytes.NewBuffer(b66), wrap(b65a))
+	equal, err = operations.CheckEqualReaders(bytes.NewBuffer(b66), wrap(b65a))
 	assert.Equal(t, myErr, err)
-	assert.Equal(t, differ, true)
+	assert.Equal(t, equal, false)
 }
 
 func TestParseSumFile(t *testing.T) {

fs/operations/operations.go

@@ -115,10 +115,10 @@ func checkHashes(ctx context.Context, src fs.ObjectInfo, dst fs.Object, ht hash.
 	if srcHash != dstHash {
 		fs.Debugf(src, "%v = %s (%v)", ht, srcHash, src.Fs())
 		fs.Debugf(dst, "%v = %s (%v)", ht, dstHash, dst.Fs())
-	} else {
+		return false, ht, srcHash, dstHash, nil
-		fs.Debugf(src, "%v = %s OK", ht, srcHash)
 	}
-	return srcHash == dstHash, ht, srcHash, dstHash, nil
+	fs.Debugf(src, "%v = %s OK", ht, srcHash)
+	return true, ht, srcHash, dstHash, nil
 }
 
 // Equal checks to see if the src and dst objects are equal by looking at
@@ -184,7 +184,13 @@ func sizeDiffers(ctx context.Context, src, dst fs.ObjectInfo) bool {
 	if ci.IgnoreSize || src.Size() < 0 || dst.Size() < 0 {
 		return false
 	}
-	return src.Size() != dst.Size()
+	if src.Size() == dst.Size() {
+		fs.Debugf(dst, "size = %d OK", dst.Size())
+		return false
+	}
+	fs.Debugf(src, "size = %d (%v)", src.Size(), src.Fs())
+	fs.Debugf(dst, "size = %d (%v)", dst.Size(), dst.Fs())
+	return true
 }
 
 var checksumWarning sync.Once
@@ -241,7 +247,7 @@ func equal(ctx context.Context, src fs.ObjectInfo, dst fs.Object, opt equalOpt)
 	ci := fs.GetConfig(ctx)
 	logger, _ := GetLogger(ctx)
 	if sizeDiffers(ctx, src, dst) {
-		fs.Debugf(src, "Sizes differ (src %d vs dst %d)", src.Size(), dst.Size())
+		fs.Debug(src, "Sizes differ")
 		logger(ctx, Differ, src, dst, nil)
 		return false
 	}