Substitute old pktline encoder/decoder with new pktline scanner (#84)

* replace old pktline package with new pktline scanner

* remove error checks on pktline.NewFromString

* fix deppend bug

* reduce memory garbage when pktline.NewFromStrings

* improve int to hex conversion to help gc

* make intToHex func private

* clean function names

4 files changed, 69 insertions, 48 deletions

diff --git a/clients/common/common.go b/clients/common/common.go
index 1518081..df1c233 100644
--- a/clients/common/common.go
+++ b/clients/common/common.go
@@ -11,7 +11,7 @@ import (
 	"strings"
 
 	"gopkg.in/src-d/go-git.v4/core"
-	"gopkg.in/src-d/go-git.v4/formats/pktline"
+	"gopkg.in/src-d/go-git.v4/formats/packp/pktline"
 	"gopkg.in/src-d/go-git.v4/storage/memory"
 )
 
@@ -204,8 +204,8 @@ func NewGitUploadPackInfo() *GitUploadPackInfo {
 	return &GitUploadPackInfo{Capabilities: NewCapabilities()}
 }
 
-func (r *GitUploadPackInfo) Decode(d *pktline.Decoder) error {
-	if err := r.read(d); err != nil {
+func (r *GitUploadPackInfo) Decode(s *pktline.Scanner) error {
+	if err := r.read(s); err != nil {
 		if err == ErrEmptyGitUploadPack {
 			return core.NewPermanentError(err)
 		}
@@ -216,16 +216,29 @@ func (r *GitUploadPackInfo) Decode(d *pktline.Decoder) error {
 	return nil
 }
 
-func (r *GitUploadPackInfo) read(d *pktline.Decoder) error {
-	lines, err := d.ReadAll()
-	if err != nil {
-		return err
-	}
-
+func (r *GitUploadPackInfo) read(s *pktline.Scanner) error {
 	isEmpty := true
 	r.Refs = make(memory.ReferenceStorage, 0)
-	for _, line := range lines {
-		if !r.isValidLine(line) {
+	smartCommentIgnore := false
+	for s.Scan() {
+		line := string(s.Bytes())
+
+		if smartCommentIgnore {
+			// some servers like Github add a flush-pkt after the smart http comment
+			// that we must ignore to prevent a premature termination of the read.
+			if len(line) == 0 {
+				continue
+			}
+			smartCommentIgnore = false
+		}
+
+		// exit on first flush-pkt
+		if len(line) == 0 {
+			break
+		}
+
+		if isSmartHttpComment(line) {
+			smartCommentIgnore = true
 			continue
 		}
 
@@ -240,11 +253,11 @@ func (r *GitUploadPackInfo) read(d *pktline.Decoder) error {
 		return ErrEmptyGitUploadPack
 	}
 
-	return nil
+	return s.Err()
 }
 
-func (r *GitUploadPackInfo) isValidLine(line string) bool {
-	return line[0] != '#'
+func isSmartHttpComment(line string) bool {
+	return line[0] == '#'
 }
 
 func (r *GitUploadPackInfo) readLine(line string) error {
@@ -280,21 +293,28 @@ func (r *GitUploadPackInfo) String() string {
 }
 
 func (r *GitUploadPackInfo) Bytes() []byte {
-	e := pktline.NewEncoder()
-	e.AddLine("# service=git-upload-pack")
-	e.AddFlush()
-	e.AddLine(fmt.Sprintf("%s HEAD\x00%s", r.Head().Hash(), r.Capabilities.String()))
+	payloads := []string{}
+	payloads = append(payloads, "# service=git-upload-pack\n")
+	// inserting a flush-pkt here violates the protocol spec, but some
+	// servers do it, like Github.com
+	payloads = append(payloads, "")
+
+	firstLine := fmt.Sprintf("%s HEAD\x00%s\n", r.Head().Hash(), r.Capabilities.String())
+	payloads = append(payloads, firstLine)
 
 	for _, ref := range r.Refs {
 		if ref.Type() != core.HashReference {
 			continue
 		}
 
-		e.AddLine(fmt.Sprintf("%s %s", ref.Hash(), ref.Name()))
+		ref := fmt.Sprintf("%s %s\n", ref.Hash(), ref.Name())
+		payloads = append(payloads, ref)
 	}
 
-	e.AddFlush()
-	b, _ := ioutil.ReadAll(e.Reader())
+	payloads = append(payloads, "")
+	pktlines, _ := pktline.NewFromStrings(payloads...)
+	b, _ := ioutil.ReadAll(pktlines)
+
 	return b
 }
 
@@ -318,21 +338,25 @@ func (r *GitUploadPackRequest) String() string {
 }
 
 func (r *GitUploadPackRequest) Reader() *strings.Reader {
-	e := pktline.NewEncoder()
+	payloads := []string{}
+
 	for _, want := range r.Wants {
-		e.AddLine(fmt.Sprintf("want %s", want))
+		payloads = append(payloads, fmt.Sprintf("want %s\n", want))
 	}
 
 	for _, have := range r.Haves {
-		e.AddLine(fmt.Sprintf("have %s", have))
+		payloads = append(payloads, fmt.Sprintf("have %s\n", have))
 	}
 
 	if r.Depth != 0 {
-		e.AddLine(fmt.Sprintf("deepen %d", r.Depth))
+		payloads = append(payloads, fmt.Sprintf("deepen %d\n", r.Depth))
 	}
 
-	e.AddFlush()
-	e.AddLine("done")
+	payloads = append(payloads, "")
+	payloads = append(payloads, "done\n")
+
+	pktlines, _ := pktline.NewFromStrings(payloads...)
+	b, _ := ioutil.ReadAll(pktlines)
 
-	return e.Reader()
+	return strings.NewReader(string(b))
 }
diff --git a/clients/common/common_test.go b/clients/common/common_test.go
index cc6c6d1..66a49e1 100644
--- a/clients/common/common_test.go
+++ b/clients/common/common_test.go
@@ -5,9 +5,10 @@ import (
 	"encoding/base64"
 	"testing"
 
-	. "gopkg.in/check.v1"
 	"gopkg.in/src-d/go-git.v4/core"
-	"gopkg.in/src-d/go-git.v4/formats/pktline"
+	"gopkg.in/src-d/go-git.v4/formats/packp/pktline"
+
+	. "gopkg.in/check.v1"
 )
 
 func Test(t *testing.T) { TestingT(t) }
@@ -48,7 +49,7 @@ func (s *SuiteCommon) TestGitUploadPackInfo(c *C) {
 	b, _ := base64.StdEncoding.DecodeString(GitUploadPackInfoFixture)
 
 	i := NewGitUploadPackInfo()
-	err := i.Decode(pktline.NewDecoder(bytes.NewBuffer(b)))
+	err := i.Decode(pktline.NewScanner(bytes.NewBuffer(b)))
 	c.Assert(err, IsNil)
 
 	name := i.Capabilities.SymbolicReference("HEAD")
@@ -70,7 +71,7 @@ func (s *SuiteCommon) TestGitUploadPackInfoNoHEAD(c *C) {
 	b, _ := base64.StdEncoding.DecodeString(GitUploadPackInfoNoHEADFixture)
 
 	i := NewGitUploadPackInfo()
-	err := i.Decode(pktline.NewDecoder(bytes.NewBuffer(b)))
+	err := i.Decode(pktline.NewScanner(bytes.NewBuffer(b)))
 	c.Assert(err, IsNil)
 
 	name := i.Capabilities.SymbolicReference("HEAD")
@@ -86,7 +87,7 @@ func (s *SuiteCommon) TestGitUploadPackInfoEmpty(c *C) {
 	b := bytes.NewBuffer(nil)
 
 	i := NewGitUploadPackInfo()
-	err := i.Decode(pktline.NewDecoder(b))
+	err := i.Decode(pktline.NewScanner(b))
 	c.Assert(err, ErrorMatches, "permanent.*empty.*")
 }
 
diff --git a/clients/http/git_upload_pack.go b/clients/http/git_upload_pack.go
index 888d279..eb8db0b 100644
--- a/clients/http/git_upload_pack.go
+++ b/clients/http/git_upload_pack.go
@@ -2,6 +2,7 @@ package http
 
 import (
 	"bufio"
+	"bytes"
 	"fmt"
 	"io"
 	"net/http"
@@ -9,7 +10,7 @@ import (
 
 	"gopkg.in/src-d/go-git.v4/clients/common"
 	"gopkg.in/src-d/go-git.v4/core"
-	"gopkg.in/src-d/go-git.v4/formats/pktline"
+	"gopkg.in/src-d/go-git.v4/formats/packp/pktline"
 )
 
 // GitUploadPackService git-upoad-pack service over HTTP
@@ -77,7 +78,7 @@ func (s *GitUploadPackService) Info() (*common.GitUploadPackInfo, error) {
 	defer res.Body.Close()
 
 	i := common.NewGitUploadPackInfo()
-	return i, i.Decode(pktline.NewDecoder(res.Body))
+	return i, i.Decode(pktline.NewScanner(res.Body))
 }
 
 // Fetch request and returns a reader to a packfile
@@ -101,27 +102,22 @@ func (s *GitUploadPackService) Fetch(r *common.GitUploadPackRequest) (io.ReadClo
 		return nil, err
 	}
 
-	if err := s.discardResponseInfo(reader); err != nil {
+	if err := discardResponseInfo(reader); err != nil {
 		return nil, err
 	}
 
 	return reader, nil
 }
 
-func (s *GitUploadPackService) discardResponseInfo(r io.Reader) error {
-	decoder := pktline.NewDecoder(r)
-	for {
-		line, err := decoder.ReadLine()
-		if err != nil {
-			break
-		}
-
-		if line == "NAK\n" {
+func discardResponseInfo(r io.Reader) error {
+	s := pktline.NewScanner(r)
+	for s.Scan() {
+		if bytes.Equal(s.Bytes(), []byte{'N', 'A', 'K', '\n'}) {
 			break
 		}
 	}
 
-	return nil
+	return s.Err()
 }
 
 func (s *GitUploadPackService) doRequest(method, url string, content *strings.Reader) (*http.Response, error) {
diff --git a/clients/ssh/git_upload_pack.go b/clients/ssh/git_upload_pack.go
index d83aadb..513e528 100644
--- a/clients/ssh/git_upload_pack.go
+++ b/clients/ssh/git_upload_pack.go
@@ -11,7 +11,7 @@ import (
 	"strings"
 
 	"gopkg.in/src-d/go-git.v4/clients/common"
-	"gopkg.in/src-d/go-git.v4/formats/pktline"
+	"gopkg.in/src-d/go-git.v4/formats/packp/pktline"
 
 	"golang.org/x/crypto/ssh"
 )
@@ -123,7 +123,7 @@ func (s *GitUploadPackService) Info() (i *common.GitUploadPackInfo, err error) {
 	}
 
 	i = common.NewGitUploadPackInfo()
-	return i, i.Decode(pktline.NewDecoder(bytes.NewReader(out)))
+	return i, i.Decode(pktline.NewScanner(bytes.NewReader(out)))
 }
 
 // Disconnect the SSH client.