Simplify after review: drop dead field, dedupe, serialize discovery · Entire

Simplify after review: drop dead field, dedupe, serialize discovery

c0dde3e·

Soph·2mo ago·5 files·+77 added/-120 removed

- Drop TargetFeatures.HeadTarget. Receive-pack adverts omit HEAD by protocol design (server-side addReferences passes addHead=false for forPush), so the field was structurally always empty in real use. Real target HEAD lives on targetSession.headTarget from DiscoverHEAD. - Serialize the target upload-pack discovery instead of running it concurrently with receive-pack. Both calls share targetConn, and RequestInfoRefs mutates conn.Endpoint.Scheme/Host under FollowInfoRefsRedirect — concurrent use raced on a shared *url.URL. Cost: one extra round-trip wall time. Correctness > the 50-100ms save. - Add (s *syncSession) heads() helper; the six Result/ProbeResult construction sites previously repeated the same SourceHEAD/TargetHEAD field pair. - Extract seedTargetWithFeatureHEAD test helper; the new sync and probe HEAD tests had an identical 18-line target-seed dance. - Trim verbose docstrings on Result, DiscoverHEAD, the discovery block, the new test docstrings, and lsRefsCoversHead.

Refs #45.

Co-Authored-By: Claude Opus 4.7 (1M context) noreply@anthropic.com

Sessions

535cfb3709fdView transcript

Changes

5

200 unmodified lines

201
202
203
204
205
206
207
208
209
210
211
212
204
205
206
207
208

200 unmodified lines

}
}

// DiscoverHEAD reads HEAD's symref target from an upload-pack info-refs
// advertisement against conn. Used to find the target's default branch
// since the receive-pack advertisement we already query doesn't include
// HEAD (server-side `addReferences` passes addHead=false for forPush).
//
// Returns empty when HEAD is detached, the target advertisement omits it
// (typical for empty bare repos where HEAD's underlying ref doesn't yet
// exist), or any error occurs. Errors are returned for telemetry but
// callers typically treat this as best-effort.
// DiscoverHEAD reads HEAD's symref target from conn's upload-pack info-refs
// advertisement. Returns empty (no error) when HEAD is detached or unadvertised.
func DiscoverHEAD(ctx context.Context, conn *Conn) (plumbing.ReferenceName, error) {
    adv, err := AdvertisedRefsV1(ctx, conn, transport.UploadPackService)
    if err != nil {
```go
Minternal/gitproto/refs.go+2/-9

```go
1
2
3
4
4
5
6
8 unmodified lines

15
16
17
19
20
21
22
18
19
20
10 unmodified lines

31
32
33
39
34
35

package gitproto

import (
    "github.com/go-git/go-git/v6/plumbing"
    "github.com/go-git/go-git/v6/plumbing/protocol/packp"
    "github.com/go-git/go-git/v6/plumbing/protocol/packp/capability"
)
8 unmodified lines

ReportStatus bool `json:"reportStatus"`
    Sideband     bool `json:"sideband"`
    Sideband64k  bool `json:"sideband64k"`
    // HeadTarget is the branch the target's HEAD symref points at, if the
    // advertisement carried a symref=HEAD:<ref> capability. Empty when HEAD
    // is detached or not advertised.
    HeadTarget plumbing.ReferenceName `json:"headTarget,omitempty"`
}

// TargetFeaturesFromAdvRefs derives the target-side feature summary from a
10 unmodified lines

ReportStatus: adv.Capabilities.Supports(capability.ReportStatus),
        Sideband:     adv.Capabilities.Supports(capability.Sideband),
        Sideband64k:  adv.Capabilities.Supports(capability.Sideband64k),
        HeadTarget:   headTargetFromAdv(adv),
    }
}

Minternal/gitproto/target_features.go-6

```go
2 unmodified lines

3
4
5
6
6
7
8
16 unmodified lines

25
26
27
29
30
31
32
33
34
35
36
37
38
28
29
30

2 unmodified lines

import (
    "testing"

"github.com/go-git/go-git/v6/plumbing"
    "github.com/go-git/go-git/v6/plumbing/protocol/packp"
    "github.com/go-git/go-git/v6/plumbing/protocol/packp/capability"
    "github.com/stretchr/testify/require"
16 unmodified lines

}
}

func TestTargetFeaturesFromAdvRefsHeadTarget(t *testing.T) {
    adv := packp.NewAdvRefs()
    require.NoError(t, adv.Capabilities.Set(capability.SymRef, "HEAD:refs/heads/main"))

got := TargetFeaturesFromAdvRefs(adv)
    if got.HeadTarget != plumbing.ReferenceName("refs/heads/main") {
        t.Fatalf("HeadTarget = %q, want refs/heads/main", got.HeadTarget)
    }
}

func TestTargetFeaturesFromAdvRefsNil(t *testing.T) {
    if got := TargetFeaturesFromAdvRefs(nil); got != (TargetFeatures{}) {
        t.Fatalf("expected zero features for nil adv, got %+v", got)
```go
Minternal/gitproto/target_features_test.go-11

```go
1849 unmodified lines

1850
1851
1852
1853
1854
1855
1853
1854
1855
1856
1857
1858
1860
1861
1862
1863
1864
1865
1866
1867
1868
1870
1871
1872
1873
1874
1875
1876
1877
1878
1879
1880
984 unmodified lines

2865
2866
2867
2890
2891
2892
2893
2868
2869
2870
2871
2872
2873
2898
2899
2900
2901
2902
2903
2904
2905
2906
2907
2908
2909
2910
2911
2912
2913
2914
2915
2916
2917
2874
2875
2876
12 unmodified lines

2889
2890
2891
2937
2938
2939
2940
2941
2942
2892
2893
2894
2895
2896
632 unmodified lines

3529
3530
3531
3532
3533
3534
3535
3536
3537
3538
3539
3540
3541
3542
3543
3544
3545
3546
3547
3548
3549
3550
3551
3552
3553
3554
3555
3556
3557
3558
3559
3560
3561
3562
603 unmodified lines

4166
4167
4168
4190
4191
4192
4169
4170
4171
4172

1849 unmodified lines

}

// ProbeResult.TargetHEAD covers the same upload-pack round-trip the sync
// session does; `git-sync probe --target-url X` should let users see the
// target's default branch before mutating anything.
// Probe surfaces the target's HEAD symref via the upload-pack round-trip.
func TestProbe_IntegrationSurfacesTargetHEAD(t *testing.T) {
    sourceRepo, sourceFS := newSourceRepo(t)
    makeCommits(t, sourceRepo, sourceFS, 1)
targetRepo, featureRef := seedTargetWithFeatureHEAD(t, sourceRepo)

targetRepo, err := git.Init(memory.NewStorage())
    if err != nil {
        t.Fatalf("init target repo: %v", err)
    }
    if err := copyRefsAndObjects(sourceRepo.Storer, targetRepo.Storer, []plumbing.ReferenceName{plumbing.NewBranchReferenceName(testBranch)}); err != nil {
        t.Fatalf("seed target: %v", err)
    }
    featureRef := plumbing.NewBranchReferenceName("feature")
    head, err := targetRepo.Reference(plumbing.NewBranchReferenceName(testBranch), true)
    if err != nil {
        t.Fatalf("resolve seeded head: %v", err)
    }
    if err := targetRepo.Storer.SetReference(plumbing.NewHashReference(featureRef, head.Hash())); err != nil {
        t.Fatalf("set target feature ref: %v", err)
    }
    if err := targetRepo.Storer.SetReference(plumbing.NewSymbolicReference(plumbing.HEAD, featureRef)); err != nil {
        t.Fatalf("retarget target HEAD: %v", err)
    }

sourceServer := newSmartHTTPRepoServerV2(t, sourceRepo)
targetServer := newSmartHTTPRepoServer(t, targetRepo)
def
sourceServer.Close()
12 unmodified lines

}

// ProbeResult.SourceHEAD lets `git-sync probe` show the source's default
// branch without performing a sync. Same protocol limitation as the sync
// path: target HEAD isn't exposed by receive-pack advertisements.
// Probe surfaces the source's HEAD symref.
func TestProbe_IntegrationSurfacesSourceHEAD(t *testing.T) {
    sourceRepo, sourceFS := newSourceRepo(t)
    makeCommits(t, sourceRepo, sourceFS, 1)
984 unmodified lines

assertHeadsMatch(t, sourceRepo, targetRepo, testBranch)
// Result.TargetHEAD comes from a separate upload-pack info-refs round-trip
// against the target URL (the receive-pack advertisement we already query
doesn't include HEAD by protocol design). The session does this lookup
// alongside the existing target setup; failures are non-fatal.
// Sync surfaces the target's HEAD symref alongside the source's.
func TestRun_IntegrationSyncSurfacesTargetHEADInResult(t *testing.T) {
    sourceRepo, sourceFS := newSourceRepo(t)
    makeCommits(t, sourceRepo, sourceFS, 1)
targetRepo, featureRef := seedTargetWithFeatureHEAD(t, sourceRepo)

// Seed target so its HEAD's underlying ref exists; otherwise the
    // upload-pack advertisement skips HEAD entirely.
targetRepo, err := git.Init(memory.NewStorage())
    if err != nil {
        t.Fatalf("init target repo: %v", err)
    }
    if err := copyRefsAndObjects(sourceRepo.Storer, targetRepo.Storer, []plumbing.ReferenceName{plumbing.NewBranchReferenceName(testBranch)}); err != nil {
        t.Fatalf("seed target: %v", err)
    }
    featureRef := plumbing.NewBranchReferenceName("feature")
    head, err := targetRepo.Reference(plumbing.NewBranchReferenceName(testBranch), true)
    if err != nil {
        t.Fatalf("resolve seeded head: %v", err)
    }
    if err := targetRepo.Storer.SetReference(plumbing.NewHashReference(featureRef, head.Hash())); err != nil {
        t.Fatalf("set target feature ref: %v", err)
    }
    if err := targetRepo.Storer.SetReference(plumbing.NewSymbolicReference(plumbing.HEAD, featureRef)); err != nil {
        t.Fatalf("retarget target HEAD: %v", err)
    }

sourceServer := newSmartHTTPRepoServerV2(t, sourceRepo)
targetServer := newSmartHTTPRepoServer(t, targetRepo)
def
sourceServer.Close()
12 unmodified lines

}

// Result.SourceHEAD carries the source's symref HEAD target, parsed from
// the v2 ls-refs response (or v1 advertisement's symref capability), so
// library callers can compare against the target's intended default
// branch without out-of-band metadata. Target HEAD is not exposed: the
// receive-pack advertisement we already query doesn't include HEAD, and
// detecting it would need a separate upload-pack round-trip.
// Sync surfaces the source's HEAD symref via the existing upload-pack
// discovery — no extra round-trip needed for this side.
func TestRun_IntegrationSyncSurfacesSourceHEADInResult(t *testing.T) {
    sourceRepo, sourceFS := newSourceRepo(t)
    makeCommits(t, sourceRepo, sourceFS, 1)
632 unmodified lines

syncertest.MakeCommits(t, repo, fs, count)
}

// seedTargetWithFeatureHEAD returns an in-memory target seeded from source's
// testBranch with refs/heads/feature pointing at the same commit and HEAD
// pointing at feature. Used by the HEAD-discovery tests: an empty target
// has no advertised HEAD because HEAD's underlying ref doesn't exist yet,
// so we need a target with at least one resolvable ref to observe it.
func seedTargetWithFeatureHEAD(t *testing.T, sourceRepo *git.Repository) (*git.Repository, plumbing.ReferenceName) {
    t.Helper()
targetRepo, err := git.Init(memory.NewStorage())
    if err != nil {
        t.Fatalf("init target repo: %v", err)
    }
    if err := copyRefsAndObjects(sourceRepo.Storer, targetRepo.Storer, []plumbing.ReferenceName{plumbing.NewBranchReferenceName(testBranch)}); err != nil {
        t.Fatalf("seed target: %v", err)
    }
    featureRef := plumbing.NewBranchReferenceName("feature")
    head, err := targetRepo.Reference(plumbing.NewBranchReferenceName(testBranch), true)
    if err != nil {
        t.Fatalf("resolve seeded head: %v", err)
    }
    if err := targetRepo.Storer.SetReference(plumbing.NewHashReference(featureRef, head.Hash())); err != nil {
        t.Fatalf("set target feature ref: %v", err)
    }
    if err := targetRepo.Storer.SetReference(plumbing.NewSymbolicReference(plumbing.HEAD, featureRef)); err != nil {
        t.Fatalf("retarget target HEAD: %v", err)
    }
    return targetRepo, featureRef
}

func makeLargeCommits(t *testing.T, repo *git.Repository, fs billy.Filesystem, count int, blobSize int) {
syncertest.MakeLargeCommits(t, repo, fs, count, blobSize)
}
603 unmodified lines

strings.Contains(msg, "connection reset by peer")

// lsRefsCoversHead returns true when prefixes is empty (matches everything)
// or any prefix is "HEAD" itself (the only way ref-prefix syntax selects HEAD,
// since "H", "HE" etc. aren't valid ref names a client would use).
// lsRefsCoversHead reports whether prefixes would include HEAD in an ls-refs response.
func lsRefsCoversHead(prefixes []string) bool {
    if len(prefixes) == 0 {
        return true