Route slog and sideband progress through the live ticker

f396a8a→main·

Soph·2mo ago·8 files·+109 added/-29 removed

When --verbose and --progress were combined, two stderr streams fought for the same line: slog's INFO lines and go-git's sideband server progress ("Enumerating objects: ...", "Compressing objects: 36% (98/271)") collided mid-character with the in-place ticker frame. Verbose+progress was effectively unusable.

Add a sessionStderr writer that routes through progressReporter. notify() (clearing the frame, printing the message, re-arming the redraw on the next tick) when a reporter is attached to the session, and falls through to os.Stderr otherwise. Both '\n' and '\r' are treated as line ends so sideband percentage updates each become their own line above the ticker rather than overlapping with it.

Plumb the sink in two places: as the slog handler's writer in newSession when Verbose is set, and as Conn.ProgressOut on every gitproto connection so progressSink in fetch.go/push.go writes there instead of os.Stderr. The ticker's own '\r' in-place writes remain the only thing on the live line.

Sessions

36b4fd4a5690View transcript

?\can you rebase soph/progress-indicators onto soph/smart-subdivisionClaude Code·Opus 4.7[1m]·1 step

Changes

8

• internal

• gitproto

• Mfetch.go+9/-9

• Mfetch_test.go+9/-9

• Mpush.go+15/-7

• Mpush_test.go+3/-3

• Msmarthttp.go+8

• syncer

• Mprogress.go+34

• Mprogress_test.go+28

• Msyncer.go+3/-1

135 unmodified lines

136
137
138
139
139
140
141
142
51 unmodified lines

194
195
196
197
197
198
199
200
39 unmodified lines

240
241
242
243
243
244
245
246
1 unmodified line

248
249
250
251
251
252
253
254
23 unmodified lines

278
279
280
281
281
282
283
284
16 unmodified lines

301
302
303
304
304
305
306
307
16 unmodified lines

324
325
326
327
327
328
329
330
133 unmodified lines

464
465
466
467
467
468
469
470
28 unmodified lines

499
500
501
502
502
503
504
505

135 unmodified lines

}
    defer ioutil.CheckClose(reader, &err)
    // Commit-graph fetches are short and not user-facing; skip progress.
    return storeV2FetchPack(store, reader, false)
    return storeV2FetchPack(store, reader, false, nil)
}

// Capabilities returns the sorted capability list for display.
51 unmodified lines

return err
    }
    defer ioutil.CheckClose(reader, &err)
    return storeV2FetchPack(store, reader, verbose)
    return storeV2FetchPack(store, reader, verbose, conn.ProgressOut)
}

func fetchPackV2(
39 unmodified lines

if err != nil {
        return nil, err
    }
    packStream, err := openV2PackStream(reader, verbose)
    packStream, err := openV2PackStream(reader, verbose, conn.ProgressOut)
    if err != nil {
        _ = reader.Close()
        return nil, err
    }
    return packStream, nil
}

func storeV2FetchPack(store storer.Storer, r io.Reader, verbose bool) error {
func storeV2FetchPack(store storer.Storer, r io.Reader, verbose bool, progressOut io.Writer) error {
    reader := NewPacketReader(r)
    expectPackfile := false
    for {
23 unmodified lines

switch line {
        case "packfile\n":
            demux := sideband.NewDemuxer(sideband.Sideband64k, reader.BufReader())
            demux.Progress = progressSink(verbose, "source: ")
            demux.Progress = progressSink(verbose, "source: ", progressOut)
            if err := packfile.UpdateObjectStorage(store, demux); err != nil {
                return fmt.Errorf("update object storage: %w", err)
            }
    16 unmodified lines

}

func openV2PackStream(body io.ReadCloser, verbose bool) (io.ReadCloser, error) {
func openV2PackStream(body io.ReadCloser, verbose bool, progressOut io.Writer) (io.ReadCloser, error) {
    reader := NewPacketReader(body)
    for {
        kind, payload, err := reader.ReadPacket()
16 unmodified lines

switch line {
        case "packfile\n":
            demux := sideband.NewDemuxer(sideband.Sideband64k, reader.BufReader())
            demux.Progress = progressSink(verbose, "source: ")
            demux.Progress = progressSink(verbose, "source: ", progressOut)
            return &wrappedRC{
                Reader: demux,
                Closer: body,
133 unmodified lines

if drainErr := drainTrailingNAKs(buffered); drainErr != nil {
        return fmt.Errorf("drain server response: %w", drainErr)
    }
    sbReader := buildSidebandReader(caps, buffered, progressSink(verbose, "source: "))
    sbReader := buildSidebandReader(caps, buffered, progressSink(verbose, "source: ", conn.ProgressOut))
    if err := packfile.UpdateObjectStorage(store, sbReader); err != nil {
        return fmt.Errorf("update object storage: %w", err)
    }
28 unmodified lines

return nil, fmt.Errorf("drain server response: %w", drainErr)
    }
    return &wrappedRC{
        Reader: buildSidebandReader(caps, buffered, progressSink(verbose, "source: ")),
        Reader: buildSidebandReader(caps, buffered, progressSink(verbose, "source: ", conn.ProgressOut)),
        Closer: reader,
    }, nil
}
``

Minternal/gitproto/fetch.go+9/-9

136 unmodified lines

137 138 139 140 140 141 142 142 143 144 144 145 146 146 147 148 149 663 unmodified lines

813 814 815 816 816 817 818 819 8 unmodified lines

828 829 830 831 831 832 833 834 14 unmodified lines

849 850 851 852 852 853 854 855 14 unmodified lines

870 871 872 873 873 874 875 876 17 unmodified lines

894 895 896 897 897 898 899 900

136 unmodified lines

}

func TestProgressWriter(t *testing.T) { w := progressWriter(false) w := progressWriter(false, nil) if w != nil { t.Error("progressWriter(false) should return nil") t.Error("progressWriter(false, nil) should return nil") } w = progressWriter(true) w = progressWriter(true, nil) if w == nil { t.Error("progressWriter(true) should return non-nil writer") t.Error("progressWriter(true, nil) should return non-nil writer") } }

663 unmodified lines

t.Fatalf("write remote error: %v", err) }

error := storeV2FetchPack(memory.NewStorage(), &wire, false) error := storeV2FetchPack(memory.NewStorage(), &wire, false, nil) if err == nil { t.Fatal("expected remote error") } 8 unmodified lines

t.Fatalf("write remote error: %v", err) }

_, err := openV2PackStream(io.NopCloser(&wire), false) _, err := openV2PackStream(io.NopCloser(&wire), false, nil) if err == nil { t.Fatal("expected remote error") } 14 unmodified lines

t.Fatalf("write flush: %v", err) } error := storeV2FetchPack(memory.NewStorage(), &wire, false) error := storeV2FetchPack(memory.NewStorage(), &wire, false, nil) if err == nil { t.Fatal("expected missing packfile error") } 14 unmodified lines

t.Fatalf("write flush: %v", err) } _, err := openV2PackStream(io.NopCloser(&wire), false) _, err := openV2PackStream(io.NopCloser(&wire), false, nil) if err == nil { t.Fatal("expected missing packfile error") } 17 unmodified lines

t.Fatalf("write flush: %v", err) } error := storeV2FetchPack(memory.NewStorage(), &wire, false) error := storeV2FetchPack(memory.NewStorage(), &wire, false, nil) if err == nil { t.Fatal("expected missing packfile error") }

Minternal/gitproto/fetch_test.go+9/-9

124 unmodified lines

125 126 127 128 128 129 130 131 132 132 133 134 135 97 unmodified lines

233 234 235 236 236 237 238 239 240 240 241 242 243 244 245 246 247 245 246 248 249 250 251 252 253 254 250 255 256 257 258 259 260 261

124 unmodified lines

switch { case req.Capabilities.Supports(capability.Sideband64k): dem := sideband.NewDemuxer(sideband.Sideband64k, reader) dem.Progress = progressSink(verbose, "target: ") dem.Progress = progressSink(verbose, "target: ", conn.ProgressOut) respReader = dem case req.Capabilities.Supports(capability.Sideband): dem := sideband.NewDemuxer(sideband.Sideband, reader) dem.Progress = progressSink(verbose, "target: ") dem.Progress = progressSink(verbose, "target: ", conn.ProgressOut) respReader = dem }

97 unmodified lines

return sendReceivePack(ctx, conn, req, nil, verbose) }

func progressWriter(verbose bool) io.Writer { func progressWriter(verbose bool, dest io.Writer) io.Writer { if !verbose { return nil } return os.Stderr if dest == nil { dest = os.Stderr } return dest }

// progressSink returns a line-prefixing io.Writer suitable for // sideband.Demuxer.Progress. When verbose is false it returns nil so the // demuxer discards progress frames without allocating. func progressSink(verbose bool, prefix string) io.Writer { // demuxer discards progress frames without allocating. Passing a non-nil // dest routes the prefixed lines through that writer instead of os.Stderr, // which lets a live progress reporter coordinate output. func progressSink(verbose bool, prefix string, dest io.Writer) io.Writer { if !verbose { return nil } return &prefixedLineWriter{w: os.Stderr, prefix: prefix, atLineStart: true} if dest == nil { dest = os.Stderr } return &prefixedLineWriter{w: dest, prefix: prefix, atLineStart: true} }

// prefixedLineWriter prepends a fixed prefix to each line of input written

Minternal/gitproto/push.go+15/-7

70 unmodified lines

71 72 73 74 74 75 76 77 77 78 79 80 5 unmodified lines

86 87 88 89 89 90 91 92

70 unmodified lines

}

func TestProgressSinkNilWhenNotVerbose(t *testing.T) { if got := progressSink(false, "anything: "); got != nil { if got := progressSink(false, "anything: ", nil); got != nil { t.Fatalf("progressSink(false) = %T, want nil", got) } if got := progressSink(true, "source: "); got == nil { if got := progressSink(true, "source: ", nil); got == nil { t.Fatal("progressSink(true) returned nil, want non-nil writer") } }

5 unmodified lines

rc, err := openV2PackStream(body, false) rc, err := openV2PackStream(body, false, nil) if err != nil { t.Fatalf("openV2PackStream: %v", err) } }

Minternal/gitproto/push_test.go+3/-3

85 unmodified lines

86 87 88 89 90 91 92 93 94 95 96 97 98 99

85 unmodified lines

// contains the repo path. Off by default to preserve behaviour for // callers that rely on Endpoint being stable. FollowInfoRefsRedirect bool

// ProgressOut is the destination for verbose sideband progress // messages ("Enumerating objects: ...", "Resolving deltas: ..." // streamed by upload-pack and receive-pack). Nil falls back to // os.Stderr. Callers driving a live progress ticker can plug in a // coordinated writer here so server-side progress lines don't // clobber the in-place ticker frame. ProgressOut io.Writer }

// NewConn creates a new connection to the given endpoint. ``

Minternal/gitproto/smarthttp.go+8

193 unmodified lines

194
195
196
197
198
199
200
201
202
203
204
205
206
207
208
209
210
211
212
213
214
215
216
217
218
219
220
221
222
223
224
225
226
227
228
229
230
231
232
233

193 unmodified lines

return host[:width-1] + "…"
// sessionStderr is an io.Writer that hands writes to the live progress
// reporter when one is attached to the syncSession, so verbose slog
// lines and server-side sideband progress ("Resolving deltas …") land
// above the in-place ticker frame instead of clobbering it. Falls back
// to os.Stderr when no reporter is active.
//
// Each Write may carry multiple lines or carriage-returned in-place
// updates; both '\n' and '\r' are treated as line ends so the reporter
// receives one notify per logical line.
type sessionStderr struct{ s *syncSession }

func (w sessionStderr) Write(b []byte) (int, error) {
    if w.s == nil || w.s.progress == nil {
        n, err := os.Stderr.Write(b)
        if err != nil {
            return n, fmt.Errorf("stderr write: %w", err)
        }
        return n, nil
    }
    s := string(b)
    for s != "" {
        i := strings.IndexAny(s, "\r\n")
        if i < 0 {
            w.s.progress.notify(s)
            break
        }
        if i > 0 {
            w.s.progress.notify(s[:i])
        }
        s = s[i+1:]
    }
    return len(b), nil
}

// stderrIsTTY reports whether stderr is attached to a terminal. The
// progress ticker is suppressed otherwise because '\r' updates only make
// sense on a TTY and would otherwise spam log files.
``

Minternal/syncer/progress.go+34

204 unmodified lines

205 206 207 208 209 210 211 212 213 214 215 216 217 218 219 220 221 222 223 224 225 226 227 228 229 230 231 232 233 234 235 236 237 238

204 unmodified lines

} }

func TestSessionStderrRoutesMultilineThroughNotify(t *testing.T) { t.Parallel() stats := newStats(true) stats.setSideDisplay("source", "github.com") stats.side("source").bytes.Store(1024)

var buf bytes.Buffer p := newProgressReporter(&buf, stats, 0) p.render(false) // give notify something to clear

sess := &syncSession{progress: p} sink := sessionStderr{s: sess}

// Multi-line write (e.g. a slog line followed by a sideband line) // must produce one notify per logical line — both '\n' and '\r' are // treated as line ends so sideband '\r'-driven percentage updates // don't clobber the live progress frame. if _, err := sink.Write([]byte("first line\nsecond line\rthird line\n")); err != nil { t.Fatalf("write: %v", err) }

for _, want := range []string{"first line", "second line", "third line"} { if !strings.Contains(buf.String(), want) { t.Errorf("output missing %q: %q", want, buf.String()) } } }

func TestProgressReporterNotifyClearsAndRedraws(t *testing.T) { t.Parallel() stats := newStats(true) }