Commit `33045f8a24`

33045f8a24accf9f7b141bff7f403525dbeb91dd

parent: fde83cbf5f

Verified · cmc

cmc <hello@cleberg.net> · 2026-09-23 06:53 UTC

build log --follow: re-check read access, coalesce reads after a wake

Ref #251

internal/control/build.go +2 −2

		@@ -200,12 +200,12 @@ func runBuildLog(c *Ctx, args []string) int {
200	200	if err != nil {
201	201	return c.fail(protocol.ExitUsage, "%v", err)
202	202	}
203		_, b, code := buildRef(c, f.Pos)
	203	repo, b, code := buildRef(c, f.Pos)
204	204	if code >= 0 {
205	205	return code
206	206	}
207	207	if f.Has("--follow") {
208		return followBuildLog(c, b)
	208	return followBuildLog(c, repo, b)
209	209	}
210	210	log, err := c.Store.BuildLog(b.ID)
211	211	if err != nil {

internal/control/buildfollow.go +47 −2

		@@ -1,10 +1,12 @@
1	1	package control
2	2
3	3	import (
	4	"errors"
4	5	"fmt"
5	6	"sync"
6	7	"time"
7	8
	9	"gitbay.org/gitbay/internal/policy"
8	10	"gitbay.org/gitbay/internal/protocol"
9	11	"gitbay.org/gitbay/internal/store"
10	12	)
		@@ -28,6 +30,11 @@ var (
28	30	// reaper's deadline, so a follow needs its own limit for the queued
29	31	// case or it never ends.
30	32	followQueued = 10 * time.Minute
	33	// followCoalesce is the pause between a wake and the read it causes.
	34	// A runner appends a chunk per read of its output, often a line, and
	35	// each read loads the whole stored log; one read after a short pause
	36	// takes a burst of appends together.
	37	followCoalesce = 200 * time.Millisecond
31	38	)
32	39
33	40	var (
		@@ -53,10 +60,30 @@ func dropFollow(uid int64) {
53	60	}
54	61	}
55	62
	63	// mayStillRead reports whether the follower can still read the build's
	64	// repository. It looks the repository up by id, so a rename mid-follow
	65	// does not end the follow.
	66	func mayStillRead(c *Ctx, repoID int64) (bool, error) {
	67	repo, err := c.Store.RepoByID(repoID)
	68	if errors.Is(err, store.ErrNotFound) {
	69	return false, nil
	70	}
	71	if err != nil {
	72	return false, err
	73	}
	74	grant, err := c.Store.AccessRole(repo.ID, c.User.ID)
	75	if err != nil {
	76	return false, err
	77	}
	78	return policy.CanRead(c.User, repo, grant), nil
	79	}
	80
56	81	// followBuildLog writes the build's log as it grows and returns once the
57	82	// build has an outcome and its last bytes are written. The outcome goes
58		// to stderr, so stdout is the log byte for byte.
59		func followBuildLog(c *Ctx, b store.Build) int {
	83	// to stderr, so stdout is the log byte for byte. Read access is checked
	84	// again every followPoll: a repository made private, or a grant revoked,
	85	// ends the follow with the answer a new request would get.
	86	func followBuildLog(c *Ctx, repo store.Repo, b store.Build) int {
60	87	if !takeFollow(c.User.ID) {
61	88	return c.fail(protocol.ExitDenied, "%d follows are already open for this account; close one and retry", maxFollows)
62	89	}
		@@ -65,7 +92,18 @@ func followBuildLog(c *Ctx, b store.Build) int {
65	92	var off int64
66	93	var settleBy time.Time
67	94	var queuedSince time.Time
	95	checked := time.Now()
68	96	for {
	97	if time.Since(checked) >= followPoll {
	98	ok, err := mayStillRead(c, b.RepoID)
	99	if err != nil {
	100	return c.fail(protocol.ExitFailure, "%v", err)
	101	}
	102	if !ok {
	103	return c.fail(protocol.ExitNotFound, "repository %s not found", repo.Path())
	104	}
	105	checked = time.Now()
	106	}
69	107	wake := c.Store.BuildLogWait(b.ID)
70	108	status, chunk, err := c.Store.BuildLogFrom(b.ID, off)
71	109	if err != nil {
		@@ -107,6 +145,13 @@ func followBuildLog(c *Ctx, b store.Build) int {
107	145	t := time.NewTimer(wait)
108	146	select {
109	147	case <-wake:
	148	t.Reset(followCoalesce)
	149	select {
	150	case <-t.C:
	151	case <-c.Done:
	152	t.Stop()
	153	return protocol.ExitFailure
	154	}
110	155	case <-t.C:
111	156	case <-c.Done:
112	157	t.Stop()

internal/control/buildfollow_test.go +45 −11

		@@ -46,6 +46,20 @@ func follow(t testing.T, st store.Store, uid int64, repo store.Repo, done <-ch
46	46	return &out, &errOut, res
47	47	}
48	48
	49	// waitOutput waits until the follower has written want, which is how a
	50	// test knows the follow is past its first read.
	51	func waitOutput(t testing.T, out syncBuffer, want string) {
	52	t.Helper()
	53	deadline := time.After(2 * time.Second)
	54	for !strings.Contains(out.String(), want) {
	55	select {
	56	case <-deadline:
	57	t.Fatalf("follow never wrote %q", want)
	58	case <-time.After(10 * time.Millisecond):
	59	}
	60	}
	61	}
	62
49	63	func waitExit(t *testing.T, res chan int) int {
50	64	t.Helper()
51	65	select {
		@@ -58,9 +72,9 @@ func waitExit(t *testing.T, res chan int) int {
58	72	}
59	73
60	74	func shortFollowTimers(t *testing.T) {
61		settle, poll, queued := followSettle, followPoll, followQueued
62		followSettle, followPoll = 200time.Millisecond, 50time.Millisecond
63		t.Cleanup(func() { followSettle, followPoll, followQueued = settle, poll, queued })
	75	settle, poll, queued, coalesce := followSettle, followPoll, followQueued, followCoalesce
	76	followSettle, followPoll, followCoalesce = 200time.Millisecond, 50time.Millisecond, 10*time.Millisecond
	77	t.Cleanup(func() { followSettle, followPoll, followQueued, followCoalesce = settle, poll, queued, coalesce })
64	78	}
65	79
66	80	// The follow prints the stored log, then what arrives, and ends with the
		@@ -134,14 +148,7 @@ func TestBuildLogFollowDone(t *testing.T) {
134	148	done := make(chan struct{})
135	149	out, _, res := follow(t, st, uid, repo, done)
136	150
137		deadline := time.After(2 * time.Second)
138		for !strings.Contains(out.String(), "step one") {
139		select {
140		case <-deadline:
141		t.Fatal("follow never read the appended line")
142		case <-time.After(10 * time.Millisecond):
143		}
144		}
	151	waitOutput(t, out, "step one")
145	152	close(done)
146	153	if code := waitExit(t, res); code != protocol.ExitFailure {
147	154	t.Fatalf("exit %d, want %d", code, protocol.ExitFailure)
		@@ -166,6 +173,33 @@ func TestBuildLogFollowQueued(t *testing.T) {
166	173	}
167	174	}
168	175
	176	// A follower who loses read access mid-follow is ended with the answer
	177	// a new request would get: the repository is not found.
	178	func TestBuildLogFollowLosesAccess(t *testing.T) {
	179	shortFollowTimers(t)
	180	st, repo, _ := newQueueTestRepo(t)
	181	bob, err := st.CreateUser("bob", false)
	182	if err != nil {
	183	t.Fatal(err)
	184	}
	185	id, err := st.CreateBuild(repo.ID, "unit", "abc", "main", `["true"]`, "", "", true)
	186	if err != nil {
	187	t.Fatal(err)
	188	}
	189	st.AppendBuildLog(id, []byte("step one\n"))
	190	out, errOut, res := follow(t, st, bob, repo, nil)
	191	waitOutput(t, out, "step one")
	192	if err := st.SetRepoVisibility(repo.ID, "private"); err != nil {
	193	t.Fatal(err)
	194	}
	195	if code := waitExit(t, res); code != protocol.ExitNotFound {
	196	t.Fatalf("exit %d, want %d: %s", code, protocol.ExitNotFound, errOut)
	197	}
	198	if !strings.Contains(errOut.String(), "repository "+repo.Path()+" not found") {
	199	t.Errorf("stderr %q", errOut)
	200	}
	201	}
	202
169	203	// An account holding maxFollows is refused another.
170	204	func TestBuildLogFollowCap(t *testing.T) {
171	205	st, repo, uid := newQueueTestRepo(t)

Commit 33045f8a24

Commit `33045f8a24`