Address review notes on the refactor

179edaa→main· Soph·2w ago·4 files·+75 added/-67 removed

• parseMappings returned a non-nil empty slice when no --map was given, where the old inline loops left Scope.Mappings nil; preserve nil so the request shape is unchanged.
• gitproto/convert.go no longer contained conversions after the dead PushPlan path was removed; rename it to readers.go to match its content (CloseOnce, LimitPackReader).

Co-Authored-By: Claude Fable 5 noreply@anthropic.com

Sessions

1dd678f9643bView transcript

Changes

4

• cmd/git-sync

• Mflags.go+5

• internal/gitproto

• Dconvert.go-67

• Areaders.go+70

• Rreaders_test.go

179 unmodified lines

// parseMappings converts raw --map values (src:dst form) into ref mappings.
// Returns nil (not an empty slice) when no mappings were given, matching the
// zero value the request structs start with.
func parseMappings(raw []string) ([]gitsync.RefMapping, error) {
    if len(raw) == 0 {
        return nil, nil
    }
    out := make([]gitsync.RefMapping, 0, len(raw))
    for _, value := range raw {
        mapping, err := validation.ParseMapping(value)

package gitproto

import (
    "errors"
    "fmt"
    "io"
    "sync"
)

// CloseOnce wraps a ReadCloser so repeated Close calls only close the
// underlying reader once. Strategies use it for pack readers that are closed
// both by PushPack and by the caller's retry/error cleanup, so double closes
// do not surface spurious failures. Passing an already-wrapped or nil reader
// returns it unchanged.
func CloseOnce(rc io.ReadCloser) io.ReadCloser {
    if rc == nil {
        return nil
    }
    if _, ok := rc.(*closeOnceReadCloser); ok {
        return rc
    }
    return &closeOnceReadCloser{ReadCloser: rc}
}

type closeOnceReadCloser struct {
    io.ReadCloser

once sync.Once
}

func (c *closeOnceReadCloser) Close() error {
    var err error
    c.once.Do(func() {
        err = c.ReadCloser.Close()
    })
    if err != nil {
        return fmt.Errorf("close pack reader: %w", err)
    }
    return nil
}

// LimitPackReader wraps a ReadCloser with a byte limit. Shared across strategies.
func LimitPackReader(r io.ReadCloser, maxBytes int64) io.ReadCloser {
    if maxBytes <= 0 {
        return r
    }
    return &packLimitRC{ReadCloser: r, max: maxBytes}
}

type packLimitRC struct {
    io.ReadCloser

max  int64
    read int64
}

func (r *packLimitRC) Read(p []byte) (int, error) {
    n, err := r.ReadCloser.Read(p)
    r.read += int64(n)
    if r.read > r.max {
        return n, fmt.Errorf("source pack exceeded max-pack-bytes limit (%d)", r.max)
    }
    if err != nil && !errors.Is(err, io.EOF) {
        return n, fmt.Errorf("read: %w", err)
    }
    return n, err //nolint:wrapcheck // io.EOF must pass through for io.Reader contract
}

package gitproto

import (
    "errors"
    "fmt"
    "io"
    "sync"
)

// This file holds shared io.ReadCloser wrappers for pack streams produced
// and consumed by this package.
// CloseOnce wraps a ReadCloser so repeated Close calls only close the
// underlying reader once. Strategies use it for pack readers that are closed
// both by PushPack and by the caller's retry/error cleanup, so double closes
// do not surface spurious failures. Passing an already-wrapped or nil reader
// returns it unchanged.
func CloseOnce(rc io.ReadCloser) io.ReadCloser {
    if rc == nil {
        return nil
    }
    if _, ok := rc.(*closeOnceReadCloser); ok {
        return rc
    }
    return &closeOnceReadCloser{ReadCloser: rc}
}

type closeOnceReadCloser struct {
    io.ReadCloser

once sync.Once
}

type packLimitRC struct {
    io.ReadCloser

max  int64
    read int64
}

```go
No patch available.

Rinternal/gitproto/readers_test.go