packfile/client: return maps of refs instead of slices

CHANGELOG.md

@@ -5,6 +5,13 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 [Unreleased]: https://github.com/gg-scm/gg-git/compare/v0.9.0...main
 
+## [Unreleased][]
+
+### Changed
+
+-  `*client.PullStream.ListRefs` and `*client.PushStream.Refs` now return a map
+   of refs instead of a slice.
+
 ## [0.9.0][] - 2021-01-26
 
 Version 0.9 adds a new package for interacting with remote Git repositories and

packfile/client/example_test.go

@@ -135,13 +135,7 @@ func ExamplePullStream_singleRef() {
 	if err != nil {
 		// handle error
 	}
-	var headRef *client.Ref
-	for _, r := range refs {
-		if r.Name == githash.Head {
-			headRef = r
-			break
-		}
-	}
+	headRef := refs[githash.Head]
 	if headRef == nil {
 		fmt.Fprintln(os.Stderr, "No HEAD found!")
 		return
@@ -275,14 +269,7 @@ func ExamplePushStream() {
 	// You should check that the ref is currently pointing to an ancestor of the
 	// commit you want to push to that ref. Otherwise, you are performing a
 	// force push.
-	mainRef := githash.BranchRef("main")
-	var curr *client.Ref
-	for _, r := range stream.Refs() {
-		if r.Name == mainRef {
-			curr = r
-			break
-		}
-	}
+	curr := stream.Refs()[githash.BranchRef("main")]
 	if curr == nil {
 		fmt.Fprintln(os.Stderr, "main branch not found!")
 		return
@@ -304,7 +291,7 @@ func ExamplePushStream() {
 
 	// Start the push. First inform the remote of refs that we intend to change.
 	err = stream.WriteCommands(&client.PushCommand{
-		RefName: mainRef,
+		RefName: curr.Name,
 		Old:     curr.ObjectID,
 		New:     newCommit.SHA1(),
 	})

packfile/client/pull.go

@@ -38,7 +38,7 @@ type PullStream struct {
 
 type puller interface {
 	negotiate(ctx context.Context, errPrefix string, req *PullRequest) (*PullResponse, error)
-	listRefs(ctx context.Context, refPrefixes []string) ([]*Ref, error)
+	listRefs(ctx context.Context, refPrefixes []string) (map[githash.Ref]*Ref, error)
 	capabilities() PullCapabilities
 	Close() error
 }
@@ -91,7 +91,7 @@ type Ref struct {
 //
 // If you need to call both ListRefs and Negotiate on a stream, you should call
 // ListRefs first. Older Git servers send their refs upfront
-func (p *PullStream) ListRefs(refPrefixes ...string) ([]*Ref, error) {
+func (p *PullStream) ListRefs(refPrefixes ...string) (map[githash.Ref]*Ref, error) {
 	refs, err := p.impl.listRefs(p.ctx, refPrefixes)
 	if err != nil {
 		return nil, fmt.Errorf("list refs for %s: %w", p.urlstr, err)

packfile/client/pull_test.go

@@ -41,7 +41,6 @@ import (
 	"gg-scm.io/pkg/git/object"
 	"gg-scm.io/pkg/git/packfile"
 	"github.com/google/go-cmp/cmp"
-	"github.com/google/go-cmp/cmp/cmpopts"
 )
 
 func TestPull(t *testing.T) {
@@ -94,30 +93,26 @@ func runPullTest(ctx context.Context, t *testing.T, u *url.URL, version int, obj
 		if err != nil {
 			t.Fatal("ListRefs:", err)
 		}
-		want := []*Ref{
-			{
+		want := map[githash.Ref]*Ref{
+			githash.Head: {
 				Name:         githash.Head,
 				ObjectID:     objects.commit2.SHA1(),
 				SymrefTarget: objects.mainRef,
 			},
-			{
+			objects.mainRef: {
 				Name:     objects.mainRef,
 				ObjectID: objects.commit2.SHA1(),
 			},
-			{
+			objects.ref1: {
 				Name:     objects.ref1,
 				ObjectID: objects.commit1.SHA1(),
 			},
-			{
+			objects.ref2: {
 				Name:     objects.ref2,
 				ObjectID: objects.commit2.SHA1(),
 			},
 		}
-		diff := cmp.Diff(
-			want, got,
-			cmpopts.SortSlices(func(r1, r2 *Ref) bool { return r1.Name < r2.Name }),
-		)
-		if diff != "" {
+		if diff := cmp.Diff(want, got); diff != "" {
 			t.Errorf("ListRefs() (-want +got):\n%s", diff)
 		}
 	})

packfile/client/pull_v1.go

@@ -56,7 +56,7 @@ type pullV1 struct {
 	refsReader *pktline.Reader
 	refsCloser io.Closer
 
-	refs      []*Ref
+	refs      map[githash.Ref]*Ref
 	refsError error
 }
 
@@ -70,7 +70,7 @@ func newPullV1(impl impl, refsReader *pktline.Reader, refsCloser io.Closer) *pul
 		refsCloser.Close()
 		return p
 	}
-	p.refs = []*Ref{ref0}
+	p.refs = map[githash.Ref]*Ref{ref0.Name: ref0}
 	p.refsReader = refsReader
 	p.refsCloser = refsCloser
 	return p
@@ -83,22 +83,25 @@ func (p *pullV1) Close() error {
 	return nil
 }
 
-func (p *pullV1) listRefs(ctx context.Context, refPrefixes []string) ([]*Ref, error) {
+func (p *pullV1) listRefs(ctx context.Context, refPrefixes []string) (map[githash.Ref]*Ref, error) {
 	if p.refsReader != nil {
-		p.refs, p.refsError = readOtherRefsV1(p.refs, p.caps.symrefs(), p.refsReader)
+		p.refsError = readOtherRefsV1(p.refs, p.caps.symrefs(), p.refsReader)
 		p.refsCloser.Close()
 		p.refsReader = nil
 		p.refsCloser = nil
 	}
+	refs := make(map[githash.Ref]*Ref, len(p.refs))
 	if len(refPrefixes) == 0 {
-		return append([]*Ref(nil), p.refs...), p.refsError
+		for k, v := range p.refs {
+			refs[k] = v
+		}
+		return refs, p.refsError
 	}
 	// Filter by given prefixes.
-	refs := make([]*Ref, 0, len(p.refs))
 	for _, r := range p.refs {
 		for _, prefix := range refPrefixes {
 			if strings.HasPrefix(string(r.Name), prefix) {
-				refs = append(refs, r)
+				refs[r.Name] = r
 			}
 		}
 	}
@@ -184,23 +187,23 @@ func parseFirstRefV1(line []byte) (*Ref, capabilityList, error) {
 // readOtherRefsV1 parses the second and subsequent refs in the version 1 refs
 // advertisement response. The caller is expected to have advanced r past the
 // first ref before calling readOtherRefsV1.
-func readOtherRefsV1(refs []*Ref, symrefs map[githash.Ref]githash.Ref, r *pktline.Reader) ([]*Ref, error) {
+func readOtherRefsV1(refs map[githash.Ref]*Ref, symrefs map[githash.Ref]githash.Ref, r *pktline.Reader) error {
 	for r.Next() && r.Type() != pktline.Flush {
 		line, err := r.Text()
 		if err != nil {
-			return nil, fmt.Errorf("read refs: %w", err)
+			return fmt.Errorf("read refs: %w", err)
 		}
 		ref, err := parseOtherRefV1(line)
 		if err != nil {
-			return nil, fmt.Errorf("read refs: %w", err)
+			return fmt.Errorf("read refs: %w", err)
 		}
 		ref.SymrefTarget = symrefs[ref.Name]
-		refs = append(refs, ref)
+		refs[ref.Name] = ref
 	}
 	if err := r.Err(); err != nil {
-		return refs, fmt.Errorf("read refs: %w", err)
+		return fmt.Errorf("read refs: %w", err)
 	}
-	return refs, nil
+	return nil
 }
 
 func parseOtherRefV1(line []byte) (*Ref, error) {

packfile/client/pull_v2.go

@@ -45,7 +45,7 @@ func (p *pullV2) Close() error {
 	return nil
 }
 
-func (p *pullV2) listRefs(ctx context.Context, refPrefixes []string) ([]*Ref, error) {
+func (p *pullV2) listRefs(ctx context.Context, refPrefixes []string) (map[githash.Ref]*Ref, error) {
 	if !p.caps.supports(listRefsV2Command) {
 		return nil, fmt.Errorf("unsupported by server")
 	}
@@ -63,7 +63,7 @@ func (p *pullV2) listRefs(ctx context.Context, refPrefixes []string) ([]*Ref, er
 		return nil, err
 	}
 	defer resp.Close()
-	var refs []*Ref
+	refs := make(map[githash.Ref]*Ref)
 	respReader := pktline.NewReader(resp)
 	for respReader.Next() && respReader.Type() != pktline.Flush {
 		line, err := respReader.Text()
@@ -90,7 +90,7 @@ func (p *pullV2) listRefs(ctx context.Context, refPrefixes []string) ([]*Ref, er
 				}
 			}
 		}
-		refs = append(refs, ref)
+		refs[ref.Name] = ref
 	}
 	if err := respReader.Err(); err != nil {
 		return nil, fmt.Errorf("parse response: %w", err)

packfile/client/push.go

@@ -36,7 +36,7 @@ const (
 // PushStream represents a git-receive-pack session.
 type PushStream struct {
 	urlstr string
-	refs   []*Ref
+	refs   map[githash.Ref]*Ref
 	caps   capabilityList
 	conn   receivePackConn
 
@@ -62,11 +62,10 @@ func (r *Remote) StartPush(ctx context.Context) (_ *PushStream, err error) {
 	if err != nil {
 		return nil, fmt.Errorf("push %s: %w", r.urlstr, err)
 	}
-	var refs []*Ref
+	var refs map[githash.Ref]*Ref
 	if ref0 != nil {
-		refs = append(refs, ref0)
-		refs, err = readOtherRefsV1(refs, caps.symrefs(), connReader)
-		if err != nil {
+		refs[ref0.Name] = ref0
+		if err := readOtherRefsV1(refs, caps.symrefs(), connReader); err != nil {
 			return nil, fmt.Errorf("push %s: %w", r.urlstr, err)
 		}
 	}
@@ -79,8 +78,8 @@ func (r *Remote) StartPush(ctx context.Context) (_ *PushStream, err error) {
 }
 
 // Refs returns the refs the remote sent when the stream started.
-// The caller must not modify the returned slice.
-func (p *PushStream) Refs() []*Ref {
+// The caller must not modify the returned map.
+func (p *PushStream) Refs() map[githash.Ref]*Ref {
 	return p.refs
 }