Sharpen --all-refs semantics: branch-filter override, Pusher pointer, prune coverage

3cf7d2c→main·

Third-pass review caught four sharpening items, all addressed:

1. --branch foo --all-refs was inconsistent: branches got filtered to foo, but tags and other-kind refs were unconditionally included. The doc claimed "in addition to whatever existing flags select," but for tags the implementation overrode the existing scope. New helper normalizeAllRefs(cfg) clears cfg.Branches at the entry of the three plan builders so AllRefs really means every branch. replicate-CanBootstrap mirrors the same logic via a branchScopeCovers predicate. Test pins the override.

2. NewPusher returned Pusher by value with value-receiver methods. The wiring at session construction (set OnRejection on the stored value, then later pass it to strategies) only worked because field assignment came before strategies captured copies. A future refactor that reordered those steps would break BestEffort silently. Switch to *Pusher with pointer receivers — strategies now hold a pointer so any later OnRejection mutation flows through. Strategy interfaces still satisfied since *Pusher.PushPack matches the same shape.

3. BestEffort only covers target-side receive-pack rejections, not source-side upload-pack failures. A server that advertises a hidden ref but refuses to serve a want for its tip (Gerrit refs/changes/* is the common case) errors out the whole fetch with no warn granularity. usage.md now spells this out so users don't expect per-ref warn semantics for source rejections.

4. sync --all-refs --prune lacked a dedicated regression. The replicate path was covered, and addPruneCandidates is shared, but the sync side wasn't pinned. New TestRun_IntegrationAllRefsSyncPrune-DeletesStaleOtherRef seeds a stale refs/notes/stale on target and asserts sync --all-refs --prune deletes it.

Sessions

830978bf4f5cView transcript

Changes

6

• docs

  • Musage.md +4/-1

• internal

  • gitproto
    • Mpush.go +9/-5
  • planner
    • Mplanner.go +14
    • Mplanner_test.go +24
  • syncer
    • Mintegration_test.go +48
    • Msyncer.go +5/-2

// Pusher wraps target-side receive-pack state behind a smaller execution API.
// When OnRejection is non-nil, per-ref ng statuses invoke it instead of erroring;
// pack-level unpack failure remains fatal.

// Returned by NewPusher as a pointer so callers can attach OnRejection after
// construction without worrying about whether downstream strategies have
// already captured a value copy.
type Pusher struct {
    Conn        *Conn
    Adv         *packp.AdvRefs
}

// NewPusher builds a target-side push executor.
func NewPusher(conn *Conn, adv *packp.AdvRefs, verbose bool) *Pusher {
    return &Pusher{Conn: conn, Adv: adv, Verbose: verbose}
}

// PushPack streams a pack to the target.
func (p *Pusher) PushPack(ctx context.Context, commands []PushCommand, pack io.ReadCloser) error {
    return PushPack(ctx, p.Conn, p.Adv, commands, pack, p.Verbose, p.OnRejection)
}

Sync's prune logic must extend to other-kind refs under AllRefs the same way replicate's does. This pins the sync side: a stale notes ref on target with no source counterpart gets deleted under sync --all-refs --prune.