Add String() to BinaryFragment

Implement base85 encoding and improve how binary patches are displayed.
This fails the tests due to differences between the Git and Go
implementation of zlib. I need to decide how to resolve this.

Author: bluekeyes

gitdiff/base85.go

@@ -19,8 +19,8 @@ func init() {
 }
 
 // base85Decode decodes Base85-encoded data from src into dst. It uses the
-// alphabet defined by base85.c in the Git source tree, which appears to be
-// unique. src must contain at least len(dst) bytes of encoded data.
+// alphabet defined by base85.c in the Git source tree. src must contain at
+// least len(dst) bytes of encoded data.
 func base85Decode(dst, src []byte) error {
 	var v uint32
 	var n, ndst int
@@ -50,3 +50,42 @@ func base85Decode(dst, src []byte) error {
 	}
 	return nil
 }
+
+// base85Encode encodes src in Base85, writing the result to dst. It uses the
+// alphabet defined by base85.c in the Git source tree.
+func base85Encode(dst, src []byte) {
+	var di, si int
+
+	encode := func(v uint32) {
+		dst[di+0] = b85Alpha[(v/(85*85*85*85))%85]
+		dst[di+1] = b85Alpha[(v/(85*85*85))%85]
+		dst[di+2] = b85Alpha[(v/(85*85))%85]
+		dst[di+3] = b85Alpha[(v/85)%85]
+		dst[di+4] = b85Alpha[v%85]
+	}
+
+	n := (len(src) / 4) * 4
+	for si < n {
+		encode(uint32(src[si+0])<<24 | uint32(src[si+1])<<16 | uint32(src[si+2])<<8 | uint32(src[si+3]))
+		si += 4
+		di += 5
+	}
+
+	var v uint32
+	switch len(src) - si {
+	case 3:
+		v |= uint32(src[si+2]) << 8
+		fallthrough
+	case 2:
+		v |= uint32(src[si+1]) << 16
+		fallthrough
+	case 1:
+		v |= uint32(src[si+0]) << 24
+		encode(v)
+	}
+}
+
+// base85Len returns the length of n bytes of Base85 encoded data.
+func base85Len(n int) int {
+	return (n + 3) / 4 * 5
+}

gitdiff/base85_test.go

@@ -1,6 +1,9 @@
 package gitdiff
 
 import (
+	"bytes"
+	"fmt"
+	"math/rand"
 	"testing"
 )
 
@@ -58,3 +61,60 @@ func TestBase85Decode(t *testing.T) {
 		})
 	}
 }
+
+func TestBase85Encode(t *testing.T) {
+	tests := map[string]struct {
+		Input  []byte
+		Output string
+	}{
+		"zeroBytes": {
+			Input:  []byte{},
+			Output: "",
+		},
+		"twoBytes": {
+			Input:  []byte{0xCA, 0xFE},
+			Output: "%KiWV",
+		},
+		"fourBytes": {
+			Input:  []byte{0x0, 0x0, 0xCA, 0xFE},
+			Output: "007GV",
+		},
+		"sixBytes": {
+			Input:  []byte{0x0, 0x0, 0xCA, 0xFE, 0xCA, 0xFE},
+			Output: "007GV%KiWV",
+		},
+	}
+
+	for name, test := range tests {
+		t.Run(name, func(t *testing.T) {
+			dst := make([]byte, len(test.Output))
+			base85Encode(dst, test.Input)
+			for i, b := range test.Output {
+				if dst[i] != byte(b) {
+					t.Errorf("incorrect character at index %d: expected '%c', actual '%c'", i, b, dst[i])
+				}
+			}
+		})
+	}
+}
+
+func TestBase85Roundtrip(t *testing.T) {
+	r := rand.New(rand.NewSource(72)) // chosen by fair dice roll
+
+	for _, size := range []int{64, 85, 1025} {
+		t.Run(fmt.Sprintf("size%d", size), func(t *testing.T) {
+			in := make([]byte, size)
+			r.Read(in)
+
+			dst := make([]byte, base85Len(size))
+			out := make([]byte, size)
+
+			base85Encode(dst, in)
+			base85Decode(out, dst)
+
+			if !bytes.Equal(in, out) {
+				t.Errorf("decoded data differed from input data:\n   input: %x\n  output: %x\nencoding: %s\n", in, out, string(dst))
+			}
+		})
+	}
+}

gitdiff/gitdiff.go

@@ -1,9 +1,12 @@
 package gitdiff
 
 import (
+	"bytes"
+	"compress/zlib"
 	"errors"
 	"fmt"
 	"os"
+	"strconv"
 	"strings"
 )
 
@@ -128,8 +131,21 @@ func (f *File) String() string {
 		diff.WriteByte('\n')
 	}
 
-	// The "---" and "+++" lines only appear for patches with fragments
-	if len(f.TextFragments) > 0 || f.BinaryFragment != nil {
+	if f.IsBinary {
+		if f.BinaryFragment == nil {
+			diff.WriteString("Binary files differ\n")
+		} else {
+			diff.WriteString("GIT binary patch\n")
+			diff.WriteString(f.BinaryFragment.String())
+			if f.ReverseBinaryFragment != nil {
+				diff.WriteByte('\n')
+				diff.WriteString(f.ReverseBinaryFragment.String())
+			}
+		}
+	}
+
+	// The "---" and "+++" lines only appear for text patches with fragments
+	if len(f.TextFragments) > 0 {
 		diff.WriteString("--- ")
 		if f.OldName == "" {
 			diff.WriteString("/dev/null")
@@ -145,11 +161,7 @@ func (f *File) String() string {
 			writeQuotedName(&diff, "b/"+f.NewName)
 		}
 		diff.WriteByte('\n')
-	}
 
-	if f.IsBinary {
-		// TODO(bkeyes): add string method for BinaryFragments
-	} else {
 		for _, frag := range f.TextFragments {
 			diff.WriteString(frag.String())
 		}
@@ -344,3 +356,83 @@ const (
 	// BinaryPatchLiteral indicates the data is the exact file content
 	BinaryPatchLiteral
 )
+
+func (f *BinaryFragment) String() string {
+	const (
+		maxBytesPerLine = 52
+	)
+
+	var diff strings.Builder
+
+	switch f.Method {
+	case BinaryPatchDelta:
+		diff.WriteString("delta ")
+	case BinaryPatchLiteral:
+		diff.WriteString("literal ")
+	}
+	diff.Write(strconv.AppendInt(nil, f.Size, 10))
+	diff.WriteByte('\n')
+
+	data := deflateBinaryChunk(f.Data)
+	n := (len(data) / maxBytesPerLine) * maxBytesPerLine
+
+	buf := make([]byte, base85Len(maxBytesPerLine))
+	for i := 0; i < n; i += maxBytesPerLine {
+		base85Encode(buf, data[i:i+maxBytesPerLine])
+		diff.WriteByte('z')
+		diff.Write(buf)
+		diff.WriteByte('\n')
+	}
+	if remainder := len(data) - n; remainder > 0 {
+		buf = buf[0:base85Len(remainder)]
+
+		sizeChar := byte(remainder)
+		if remainder <= 26 {
+			sizeChar = 'A' + sizeChar - 1
+		} else {
+			sizeChar = 'a' + sizeChar - 27
+		}
+
+		base85Encode(buf, data[n:])
+		diff.WriteByte(sizeChar)
+		diff.Write(buf)
+		diff.WriteByte('\n')
+	}
+
+	return diff.String()
+}
+
+// TODO(bkeyes): The 'compress/flate' package does not produce minimal output
+// streams. Instead of flagging that the last block of data represents the end
+// of the stream, it always writes a final empty block to mark the end. Git's
+// implementation using the 'zlib' C library does not do this, which means that
+// what we produce for binary patches does not match the input, even though it
+// is valid.
+//
+// This is mostly a problem for my tests, where I compare the input and output
+// bytes. This comparison isn't required, but is helpful to catch invalid
+// output that might otherwise still parse.
+//
+// Options for fixing this:
+//
+//  1. Fix the tests to compare parsed objects instead of raw patches, at least
+//     for binary patches. This means writing something to do reasonable
+//     comparisons of File structs.
+//
+//  2. Add my own deflate function. By default, Git appears to use no
+//     compression on binary patch data, which means "delfate" is just adding
+//     the appropriate headers and checksums around the data. This would fix my
+//     tests but means we could never emit compressed data, so we'd differ from
+//     Git in other situations.
+//
+// Either way, there will be situations in which re-formatted binary patches
+// differ from the original inputs.
+func deflateBinaryChunk(data []byte) []byte {
+	var b bytes.Buffer
+
+	zw, _ := zlib.NewWriterLevel(&b, zlib.NoCompression)
+	_, _ = zw.Write(data)
+	_ = zw.Close()
+
+	return b.Bytes()
+}

gitdiff/gitdiff_string_test.go

@@ -8,7 +8,10 @@ import (
 
 func TestFile_String(t *testing.T) {
 	sources := []string{
+		"testdata/string/binary_modify.patch",
+		"testdata/string/binary_new.patch",
 		"testdata/string/copy.patch",
+		"testdata/string/copy_modify.patch",
 		"testdata/string/delete.patch",
 		"testdata/string/mode.patch",
 		"testdata/string/mode_modify.patch",
@@ -18,7 +21,6 @@ func TestFile_String(t *testing.T) {
 		"testdata/string/new_mode.patch",
 		"testdata/string/rename.patch",
 		"testdata/string/rename_modify.patch",
-		"testdata/string/copy_modify.patch",
 	}
 
 	for _, src := range sources {
@@ -46,3 +48,20 @@ func assertParseSingleFile(t *testing.T, src string, b []byte) *File {
 	}
 	return files[0]
 }
+
+/*
+func TestDecode(t *testing.T) {
+	actual := []byte("cmV-O")
+	mine := []byte("cmV)N")
+
+	dst := make([]byte, 4)
+
+	base85Decode(dst, actual)
+	t.Logf("actual: %x / %b", dst, dst)
+
+	base85Decode(dst, mine)
+	t.Logf("  mine: %x / %b", dst, dst)
+
+	t.FailNow()
+}
+*/

gitdiff/testdata/string/binary_modify.patch

@@ -0,0 +1,9 @@
+diff --git a/file.bin b/file.bin
+index a7f4d5d6975ec021016c02b6d58345ebf434f38c..bdc9a70f055892146612dcdb413f0e339faaa0df 100644
+GIT binary patch
+delta 66
+QcmeZhVVvM$!$1K50C&Ox;s5{u
+
+delta 5
+McmZo+^qAlQ00i9urT_o{
+

gitdiff/testdata/string/binary_new.patch

@@ -0,0 +1,11 @@
+diff --git a/file.bin b/file.bin
+new file mode 100644
+index 0000000000000000000000000000000000000000..a7f4d5d6975ec021016c02b6d58345ebf434f38c
+GIT binary patch
+literal 72
+zcmV-O0Jr~td-`u6JcK&{KDK=<a#;v1^LR5&K)zQ0=Goz82(?nJ6_nD`f#8O9p}}{P
+eiXim+rDI+BDadMQmMsO5Sw@;DbrCA+PamP;Ng_@F
+
+literal 0
+HcmV?d00001
+