Merge pull request #552 from mcarmonaa/fix/packp-adv-refs-encode

packp: fixed encoding when HEAD is not a valid ref

2 files changed, 56 insertions, 38 deletions

diff --git a/plumbing/protocol/packp/advrefs_encode.go b/plumbing/protocol/packp/advrefs_encode.go
index e981120..cb93d46 100644
--- a/plumbing/protocol/packp/advrefs_encode.go
+++ b/plumbing/protocol/packp/advrefs_encode.go
@@ -2,6 +2,7 @@ package packp
 
 import (
 	"bytes"
+	"fmt"
 	"io"
 	"sort"
 
@@ -21,9 +22,13 @@ func (a *AdvRefs) Encode(w io.Writer) error {
 }
 
 type advRefsEncoder struct {
-	data *AdvRefs         // data to encode
-	pe   *pktline.Encoder // where to write the encoded data
-	err  error            // sticky error
+	data         *AdvRefs         // data to encode
+	pe           *pktline.Encoder // where to write the encoded data
+	firstRefName string           // reference name to encode in the first pkt-line (HEAD if present)
+	firstRefHash plumbing.Hash    // hash referenced to encode in the first pkt-line (HEAD if present)
+	sortedRefs   []string         // hash references to encode ordered by increasing order
+	err          error            // sticky error
+
 }
 
 func newAdvRefsEncoder(w io.Writer) *advRefsEncoder {
@@ -34,6 +39,8 @@ func newAdvRefsEncoder(w io.Writer) *advRefsEncoder {
 
 func (e *advRefsEncoder) Encode(v *AdvRefs) error {
 	e.data = v
+	e.sortRefs()
+	e.setFirstRef()
 
 	for state := encodePrefix; state != nil; {
 		state = state(e)
@@ -42,6 +49,32 @@ func (e *advRefsEncoder) Encode(v *AdvRefs) error {
 	return e.err
 }
 
+func (e *advRefsEncoder) sortRefs() {
+	if len(e.data.References) > 0 {
+		refs := make([]string, 0, len(e.data.References))
+		for refName := range e.data.References {
+			refs = append(refs, refName)
+		}
+
+		sort.Strings(refs)
+		e.sortedRefs = refs
+	}
+}
+
+func (e *advRefsEncoder) setFirstRef() {
+	if e.data.Head != nil {
+		e.firstRefName = head
+		e.firstRefHash = *e.data.Head
+		return
+	}
+
+	if len(e.sortedRefs) > 0 {
+		refName := e.sortedRefs[0]
+		e.firstRefName = refName
+		e.firstRefHash = e.data.References[refName]
+	}
+}
+
 type encoderStateFn func(*advRefsEncoder) encoderStateFn
 
 func encodePrefix(e *advRefsEncoder) encoderStateFn {
@@ -61,33 +94,27 @@ func encodePrefix(e *advRefsEncoder) encoderStateFn {
 }
 
 // Adds the first pkt-line payload: head hash, head ref and capabilities.
-// Also handle the special case when no HEAD ref is found.
+// If HEAD ref is not found, the first reference ordered in increasing order will be used.
+// If there aren't HEAD neither refs, the first line will be "PKT-LINE(zero-id SP "capabilities^{}" NUL capability-list)".
+// See: https://github.com/git/git/blob/master/Documentation/technical/pack-protocol.txt
+// See: https://github.com/git/git/blob/master/Documentation/technical/protocol-common.txt
 func encodeFirstLine(e *advRefsEncoder) encoderStateFn {
-	head := formatHead(e.data.Head)
-	separator := formatSeparator(e.data.Head)
+	const formatFirstLine = "%s %s\x00%s\n"
+	var firstLine string
 	capabilities := formatCaps(e.data.Capabilities)
 
-	if e.err = e.pe.Encodef("%s %s\x00%s\n", head, separator, capabilities); e.err != nil {
-		return nil
-	}
+	if e.firstRefName == "" {
+		firstLine = fmt.Sprintf(formatFirstLine, plumbing.ZeroHash.String(), "capabilities^{}", capabilities)
+	} else {
+		firstLine = fmt.Sprintf(formatFirstLine, e.firstRefHash.String(), e.firstRefName, capabilities)
 
-	return encodeRefs
-}
-
-func formatHead(h *plumbing.Hash) string {
-	if h == nil {
-		return plumbing.ZeroHash.String()
 	}
 
-	return h.String()
-}
-
-func formatSeparator(h *plumbing.Hash) string {
-	if h == nil {
-		return noHead
+	if e.err = e.pe.EncodeString(firstLine); e.err != nil {
+		return nil
 	}
 
-	return head
+	return encodeRefs
 }
 
 func formatCaps(c *capability.List) string {
@@ -101,8 +128,11 @@ func formatCaps(c *capability.List) string {
 // Adds the (sorted) refs: hash SP refname EOL
 // and their peeled refs if any.
 func encodeRefs(e *advRefsEncoder) encoderStateFn {
-	refs := sortRefs(e.data.References)
-	for _, r := range refs {
+	for _, r := range e.sortedRefs {
+		if r == e.firstRefName {
+			continue
+		}
+
 		hash, _ := e.data.References[r]
 		if e.err = e.pe.Encodef("%s %s\n", hash.String(), r); e.err != nil {
 			return nil
@@ -118,16 +148,6 @@ func encodeRefs(e *advRefsEncoder) encoderStateFn {
 	return encodeShallow
 }
 
-func sortRefs(m map[string]plumbing.Hash) []string {
-	ret := make([]string, 0, len(m))
-	for k := range m {
-		ret = append(ret, k)
-	}
-	sort.Strings(ret)
-
-	return ret
-}
-
 // Adds the (sorted) shallows: "shallow" SP hash EOL
 func encodeShallow(e *advRefsEncoder) encoderStateFn {
 	sorted := sortShallows(e.data.Shallows)
diff --git a/plumbing/protocol/packp/advrefs_encode_test.go b/plumbing/protocol/packp/advrefs_encode_test.go
index f901440..3ae84a7 100644
--- a/plumbing/protocol/packp/advrefs_encode_test.go
+++ b/plumbing/protocol/packp/advrefs_encode_test.go
@@ -99,8 +99,7 @@ func (s *AdvRefsEncodeSuite) TestRefs(c *C) {
 	}
 
 	expected := pktlines(c,
-		"0000000000000000000000000000000000000000 capabilities^{}\x00\n",
-		"a6930aaee06755d1bdcfd943fbf614e4d92bb0c7 refs/heads/master\n",
+		"a6930aaee06755d1bdcfd943fbf614e4d92bb0c7 refs/heads/master\x00\n",
 		"5dc01c595e6c6ec9ccda4f6f69c131c0dd945f8c refs/tags/v2.6.11-tree\n",
 		"1111111111111111111111111111111111111111 refs/tags/v2.6.12-tree\n",
 		"2222222222222222222222222222222222222222 refs/tags/v2.6.13-tree\n",
@@ -129,8 +128,7 @@ func (s *AdvRefsEncodeSuite) TestPeeled(c *C) {
 	}
 
 	expected := pktlines(c,
-		"0000000000000000000000000000000000000000 capabilities^{}\x00\n",
-		"a6930aaee06755d1bdcfd943fbf614e4d92bb0c7 refs/heads/master\n",
+		"a6930aaee06755d1bdcfd943fbf614e4d92bb0c7 refs/heads/master\x00\n",
 		"5dc01c595e6c6ec9ccda4f6f69c131c0dd945f8c refs/tags/v2.6.11-tree\n",
 		"1111111111111111111111111111111111111111 refs/tags/v2.6.12-tree\n",
 		"5555555555555555555555555555555555555555 refs/tags/v2.6.12-tree^{}\n",