From fe4db9185228703f188439f16661aa5b305337ad Mon Sep 17 00:00:00 2001 From: Ross Light Date: Sat, 6 Feb 2021 08:32:17 -0800 Subject: [PATCH] packfile/client: return maps of refs instead of slices --- CHANGELOG.md | 7 +++++++ packfile/client/example_test.go | 19 +++---------------- packfile/client/pull.go | 4 ++-- packfile/client/pull_test.go | 17 ++++++----------- packfile/client/pull_v1.go | 29 ++++++++++++++++------------- packfile/client/pull_v2.go | 6 +++--- packfile/client/push.go | 13 ++++++------- 7 files changed, 43 insertions(+), 52 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index ea9b482..c94e4e4 100644 --- a/CHANGELOG.md +++ b/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 diff --git a/packfile/client/example_test.go b/packfile/client/example_test.go index d59881e..a05ae47 100644 --- a/packfile/client/example_test.go +++ b/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(), }) diff --git a/packfile/client/pull.go b/packfile/client/pull.go index 40ee6de..53827f8 100644 --- a/packfile/client/pull.go +++ b/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) diff --git a/packfile/client/pull_test.go b/packfile/client/pull_test.go index 7d997ef..39359c2 100644 --- a/packfile/client/pull_test.go +++ b/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) } }) diff --git a/packfile/client/pull_v1.go b/packfile/client/pull_v1.go index f07cab6..feed96d 100644 --- a/packfile/client/pull_v1.go +++ b/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) { diff --git a/packfile/client/pull_v2.go b/packfile/client/pull_v2.go index 8d9dde6..dec2b9f 100644 --- a/packfile/client/pull_v2.go +++ b/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) diff --git a/packfile/client/push.go b/packfile/client/push.go index 3de99c0..3dfdc9f 100644 --- a/packfile/client/push.go +++ b/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 }