Commit e14c4caeec

e14c4caeec78ed29351c36767d0377794b3fd9ab

parent: c07024b730

Verified · cmc ci/build: success ci/test: success

cmc <hello@cleberg.net> · 2026-10-19 04:53 UTC

ci: reuse a build only on the ref it ran for

queueJobs let a build of the same commit, or a passed build of the same
tree, on any ref stand for a push. A job sees GITBAY_REF, so a fast-forward
or a squash merge onto the default branch reused a branch build that had
not done what the default-branch build would: krz/orgo's pages job no-ops
off main and was never run after a merge.

Both lookups now take the ref (BuildsForCommitOnRef, SuccessBuildForTree).
A filter skip still leaves alone a status another ref's build reported on
the same commit. New push-shape row for the squash merge; the fast-forward
row now queues, as does each case in the e2e
TestFastForwardMergeBuildsOnTarget (was TestFastForwardMergeSkipsBuiltCommit).

Closes #335

Layout: unified · split

.gitbay/wiki/Architecture/07-CI-and-Supply-Chain.org +1 −1
@@ -23,7 +23,7 @@ commit instead of failing silently.
23231. *Queue.* The post-receive hook calls =queueJobs=
2424 (=internal/control/build.go=). Each job gets a =ci/<job>= status:
2525 =pending= when queued, =skipped= when path filters exclude it, or
26 =success= copied from an earlier trusted build of the same tree on the same image (#177, #258).
26 =success= copied from an earlier trusted build of the same tree on the same ref and image (#177, #258).
2727 Merge requests from forks are queued against the target repository
2828 with =trusted = false=.
29292. *Claim.* A runner calls =runner next= over SSH
.gitbay/wiki/CI.org +15 −8
@@ -2,10 +2,13 @@
22
33Three mechanisms decide what a push does to CI, and they interact:
44
5- *Dedupe.* A job's result is a property of the commit's tree. A commit
6 that already has a passed, queued or running build for a job is not
7 queued again; a commit whose tree already passed a job gets that
8 result as its status, naming the build it came from (#177). Only a
5- *Dedupe.* A job's result is a property of the commit's tree and the
6 ref it ran for, since a job sees =GITBAY_REF=. A commit that already
7 has a passed, queued or running build for a job on this ref is not
8 queued again; a commit whose tree already passed a job on this ref
9 gets that result as its status, naming the build it came from (#177).
10 A merge onto the default branch is built there even when the branch
11 passed. Only a
912 trusted build counts, and for tree reuse only one on the image the
1013 job names: a fork's green build does not stand for the repository's
1114 own, so its commit is built again when it lands on a branch (#258). A
@@ -168,7 +171,8 @@ push produced for that job; the commit's =ci/<job>= status follows:
168171| a new branch, no old sha | queued | queued | skipped | — | — | — |
169172| rebase onto a moved default branch, old not an ancestor | queued | queued | skipped | — | — | — |
170173| rewritten commit, same tree as a passed build | reused | reused | skipped | — | — | — |
171| fast-forward of a commit built on another branch | — | — | — | registered | — | — |
174| fast-forward of a commit built on another branch | queued | queued | — | registered | — | — |
175| squash merge onto the default branch, same tree as a branch build | queued | queued | skipped | registered | — | — |
172176| a commit whose earlier build failed, on a new branch | queued | queued | — | — | — | — |
173177| merge request head from a fork | queued, untrusted | queued, untrusted | queued, untrusted | — | — | — |
174178| tag push | — | — | — | — | queued | — |
@@ -190,9 +194,12 @@ Rows worth a second look:
190194 because the tree check comes before the filter. =notapp= never had a
191195 passed build to reuse, and the push changed nothing, so the filter
192196 skips it again.
193- A fast-forward of a commit built elsewhere queues nothing: the
194 builds belong to the commit, not the branch. The default-branch push
195 still registers the schedule.
197- A build stands for its commit, or its tree, only on the ref it ran
198 for: a job sees =GITBAY_REF= and may act on it, as one that publishes
199 only from the default branch does. A fast-forward or a squash merge
200 onto the default branch is built there although the branch passed.
201 =notapp= on the fast-forward keeps the skipped status the branch push
202 gave the same commit.
196203- A failed build does not stand for its commit. The same commit pushed
197204 to another branch runs again; =notapp= keeps its skipped status.
198205- A merge request head from a fork has no diff base and, unlike a
e2e/ci_dedupe_test.go +24 −22
@@ -7,9 +7,11 @@ import (
77 "testing"
88)
99
10// A fast-forward lands the exact commit that was already built on its
11// branch; that commit is not built again. A commit whose build failed is.
12func TestFastForwardMergeSkipsBuiltCommit(t *testing.T) {
10// A fast-forward lands a commit already built on its branch, and it is
11// built again for the target: a job sees GITBAY_REF, so the branch's
12// result does not say what the job does on main (#335). That holds for
13// a branch build that passed, one still queued, and one that failed.
14func TestFastForwardMergeBuildsOnTarget(t *testing.T) {
1315 t.Parallel()
1416 inst := startInstance(t)
1517 inst.runner = buildRunner(t)
@@ -20,6 +22,10 @@ func TestFastForwardMergeSkipsBuiltCommit(t *testing.T) {
2022 if _, errOut, code := inst.ssh(t, aliceKey, "", "repo", "create", "alice/app"); code != 0 {
2123 t.Fatalf("repo create: %s", errOut)
2224 }
25 mainBuilds := func() int {
26 out, _, _ := inst.ssh(t, aliceKey, "", "build", "list", "alice/app", "--ref", "main")
27 return strings.Count(out, "\n")
28 }
2329 work := t.TempDir()
2430 env := inst.gitEnv(aliceKey)
2531 mustGit(t, work, env, "clone", inst.sshURL("alice/app"), "w")
@@ -39,27 +45,22 @@ func TestFastForwardMergeSkipsBuiltCommit(t *testing.T) {
3945 mustGit(t, dir, env, "commit", "-q", "-m", "good change")
4046 mustGit(t, dir, env, "push", "-q", "origin", "good")
4147 inst.runnerOnce(t, runnerKey)
42 list := inst.buildList(t, aliceKey)
43 if !strings.Contains(list, "good\tunit\tsuccess") && !strings.Contains(list, "unit\tsuccess") {
48 if list := inst.buildList(t, aliceKey); !strings.Contains(list, "unit\tsuccess") {
4449 t.Fatalf("branch build did not pass:\n%s", list)
4550 }
4651 if _, errOut, code := inst.ssh(t, aliceKey, "", "mr", "create", "alice/app", "--source", "good", "--target", "main", "--title", "good"); code != 0 {
4752 t.Fatalf("mr create: %s", errOut)
4853 }
49 before := strings.Count(inst.buildList(t, aliceKey), "\n")
54 before := mainBuilds()
5055 if _, errOut, code := inst.ssh(t, aliceKey, "", "mr", "merge", "alice/app", "1", "--strategy", "ff"); code != 0 {
5156 t.Fatalf("mr merge: %s", errOut)
5257 }
53 if after := strings.Count(inst.buildList(t, aliceKey), "\n"); after != before {
54 t.Fatalf("fast-forward queued a build for an already-built commit:\n%s", inst.buildList(t, aliceKey))
55 }
56 sha := strings.Fields(strings.TrimSpace(mustGit(t, dir, env, "ls-remote", "origin", "refs/heads/main")))[0]
57 if status, _, _ := inst.ssh(t, aliceKey, "", "status", "list", "alice/app", sha); !strings.Contains(status, "success") || strings.Contains(status, "pending") {
58 t.Fatalf("statuses on the merged commit changed:\n%s", status)
58 if after := mainBuilds(); after != before+1 {
59 t.Fatalf("fast-forward of a passed commit queued %d builds on main, want 1:\n%s", after-before, inst.buildList(t, aliceKey))
5960 }
61 inst.runnerOnce(t, runnerKey)
6062
61 // A branch merged before its build has run: the queued build will
62 // say, so the merge queues nothing either.
63 // A branch merged before its build has run: main gets its own.
6364 mustGit(t, dir, env, "checkout", "-q", "-b", "early")
6465 os.WriteFile(filepath.Join(dir, "g.txt"), []byte("y\n"), 0o644)
6566 mustGit(t, dir, env, "add", ".")
@@ -68,16 +69,17 @@ func TestFastForwardMergeSkipsBuiltCommit(t *testing.T) {
6869 if _, errOut, code := inst.ssh(t, aliceKey, "", "mr", "create", "alice/app", "--source", "early", "--target", "main", "--title", "early"); code != 0 {
6970 t.Fatalf("mr create: %s", errOut)
7071 }
71 before = strings.Count(inst.buildList(t, aliceKey), "\n")
72 before = mainBuilds()
7273 if _, errOut, code := inst.ssh(t, aliceKey, "", "mr", "merge", "alice/app", "2", "--strategy", "ff"); code != 0 {
7374 t.Fatalf("mr merge early: %s", errOut)
7475 }
75 if after := strings.Count(inst.buildList(t, aliceKey), "\n"); after != before {
76 t.Fatalf("a merge before the branch build ran queued a second build:\n%s", inst.buildList(t, aliceKey))
76 if after := mainBuilds(); after != before+1 {
77 t.Fatalf("a merge before the branch build ran queued %d builds on main, want 1:\n%s", after-before, inst.buildList(t, aliceKey))
7778 }
78 inst.runnerOnce(t, runnerKey) // the branch's queued build, now the only one
79 inst.runnerOnce(t, runnerKey) // the branch's
80 inst.runnerOnce(t, runnerKey) // main's
7981
80 // A branch whose build fails is built again when it lands.
82 // A branch whose build fails.
8183 mustGit(t, dir, env, "checkout", "-q", "-b", "bad")
8284 os.WriteFile(filepath.Join(dir, "fail.txt"), []byte("x\n"), 0o644)
8385 os.Remove(filepath.Join(dir, "f.txt"))
@@ -91,11 +93,11 @@ func TestFastForwardMergeSkipsBuiltCommit(t *testing.T) {
9193 if _, errOut, code := inst.ssh(t, aliceKey, "", "mr", "create", "alice/app", "--source", "bad", "--target", "main", "--title", "bad"); code != 0 {
9294 t.Fatalf("mr create: %s", errOut)
9395 }
94 before = strings.Count(inst.buildList(t, aliceKey), "\n")
96 before = mainBuilds()
9597 if _, errOut, code := inst.ssh(t, aliceKey, "", "mr", "merge", "alice/app", "3", "--strategy", "ff"); code != 0 {
9698 t.Fatalf("mr merge bad: %s", errOut)
9799 }
98 if after := strings.Count(inst.buildList(t, aliceKey), "\n"); after != before+1 {
99 t.Fatalf("a failed commit was not rebuilt on landing:\n%s", inst.buildList(t, aliceKey))
100 if after := mainBuilds(); after != before+1 {
101 t.Fatalf("a failed commit queued %d builds on main, want 1:\n%s", after-before, inst.buildList(t, aliceKey))
100102 }
101103}
internal/control/build.go +29 −15
@@ -1020,20 +1020,28 @@ func queueJobs(
10201020 st.SetCommitStatus(repo.ID, sha, "ci/config", "failure", err.Error(), "", userID)
10211021 return
10221022 }
1023 // A build is a fact about a commit, not a ref: a job has no branch
1024 // filter, so a commit that already passed a job on another branch has
1025 // nothing left to prove when a fast-forward lands it here, and one
1026 // still queued or running there will say soon enough. A failed,
1027 // abandoned or cancelled build does not count; that commit runs again.
1028 built, err := st.BuildsForCommit(repo.ID, sha)
1023 // A build is a fact about a commit on a ref: a job sees GITBAY_REF and
1024 // may act on it, as a job that publishes only from the default branch
1025 // does, so a build for another ref does not stand for this one. A
1026 // commit already built for this ref, or queued or running for it, has
1027 // nothing left to prove. A failed, abandoned or cancelled build does
1028 // not count; that commit runs again.
1029 built, err := st.BuildsForCommitOnRef(repo.ID, sha, ref)
10291030 if err != nil {
10301031 built = nil
10311032 }
1032 // A job's result is a property of the tree, not the commit: a rebase
1033 // onto a base that touched nothing the branch did gives every commit
1034 // a new sha and the same tree, and re-running the suite over it
1035 // proves nothing it did not already prove (#177). A success recorded
1036 // against the tree stands for the new commit.
1033 // The ci/<job> status belongs to the commit, so a build of it for
1034 // another ref reports there too, and a skip here must not overwrite it.
1035 elsewhere, err := st.BuildsForCommit(repo.ID, sha)
1036 if err != nil {
1037 elsewhere = nil
1038 }
1039 // A job's result is a property of the tree and the ref, not the
1040 // commit: a rebase onto a base that touched nothing the branch did
1041 // gives every commit a new sha and the same tree, and re-running the
1042 // suite over it proves nothing it did not already prove (#177). A
1043 // success recorded against the tree on this ref stands for the new
1044 // commit; a squash merge onto another branch is built there.
10371045 tree, _ := gitutil.ResolveTree(dir, sha)
10381046 // The changed-file list a job's path filters run against, computed
10391047 // once and only if some job actually declares one. When the diff
@@ -1092,11 +1100,15 @@ func queueJobs(
10921100 // stands for it — unless this queue is trusted and that build was
10931101 // not: a fork's head that lands on a branch is built again as the
10941102 // repository's own (#258).
1095 if b, ok := built[j.Name]; ok && (b.Trusted || !trusted) &&
1096 (b.Status == "success" || b.Status == "pending" || b.Status == "running") {
1103 stands := func(m map[string]store.Build) bool {
1104 b, ok := m[j.Name]
1105 return ok && (b.Trusted || !trusted) &&
1106 (b.Status == "success" || b.Status == "pending" || b.Status == "running")
1107 }
1108 if stands(built) {
10971109 continue
10981110 }
1099 if prev, ok, _ := st.SuccessBuildForTree(repo.ID, tree, j.Name, j.Image); ok && prev.SHA != sha {
1111 if prev, ok, _ := st.SuccessBuildForTree(repo.ID, tree, ref, j.Name, j.Image); ok && prev.SHA != sha {
11001112 url := fmt.Sprintf("%s/%s/builds/%d", siteURL, repo.Path(), prev.Number)
11011113 st.SetCommitStatus(repo.ID, sha, "ci/"+j.Name, "success",
11021114 fmt.Sprintf("passed in build %d as %.10s, same tree", prev.Number, prev.SHA), url, userID)
@@ -1117,7 +1129,9 @@ func queueJobs(
11171129 // require_checks with a skipped status instead of leaving the
11181130 // commit with none at all, which the gate refuses outright (#172).
11191131 if filtered && !ci.Selected(j, changed) {
1120 st.SetCommitStatus(repo.ID, sha, "ci/"+j.Name, "skipped", skipReason(j, changed), "", userID)
1132 if !stands(elsewhere) {
1133 st.SetCommitStatus(repo.ID, sha, "ci/"+j.Name, "skipped", skipReason(j, changed), "", userID)
1134 }
11211135 continue
11221136 }
11231137 steps, _ := json.Marshal(j.Steps)
internal/hookd/pushshapes_test.go +14 −1
@@ -330,7 +330,20 @@ var pushShapes = []pushShape{
330330 f.git(f.src, "merge", "-q", "--ff-only", "feat")
331331 f.mark(c1)
332332 f.push("main", f.base, c1)
333 }, []string{"—", "—", "—", "registered", "—", "—"}},
333 }, []string{"queued", "queued", "—", "registered", "—", "—"}},
334
335 {"squash merge onto the default branch, same tree as a branch build", func(f *shapeFixture) {
336 f.git(f.src, "checkout", "-q", "-b", "feat")
337 c1 := f.appCommit("more")
338 f.push("feat", zeroSHA40, c1)
339 f.finish("success")
340 f.git(f.src, "checkout", "-q", "main")
341 f.git(f.src, "merge", "-q", "--squash", "feat")
342 f.git(f.src, "commit", "-q", "-m", "squashed")
343 c2 := f.sha("HEAD")
344 f.mark(c2)
345 f.push("main", f.base, c2)
346 }, []string{"queued", "queued", "skipped", "registered", "—", "—"}},
334347
335348 {"a commit whose earlier build failed, on a new branch", func(f *shapeFixture) {
336349 f.git(f.src, "checkout", "-q", "-b", "feat")
internal/store/builds.go +18 −5
@@ -437,7 +437,18 @@ func (s *Store) LatestBuild(repoID int64, job string) (Build, error) {
437437// request's checks are ci/<job> statuses; this is where their timing comes
438438// from, in one query rather than one per check.
439439func (s *Store) BuildsForCommit(repoID int64, sha string) (map[string]Build, error) {
440 rows, err := s.DB.Query(buildSelect+" WHERE repo_id = ? AND sha = ? ORDER BY number ASC", repoID, sha)
440 return s.newestPerJob(" WHERE repo_id = ? AND sha = ? ORDER BY number ASC", repoID, sha)
441}
442
443// BuildsForCommitOnRef returns the newest build per job for one commit
444// made for one ref. A job sees GITBAY_REF, so a build for another ref
445// says nothing about what the job does on this one.
446func (s *Store) BuildsForCommitOnRef(repoID int64, sha, ref string) (map[string]Build, error) {
447 return s.newestPerJob(" WHERE repo_id = ? AND sha = ? AND ref = ? ORDER BY number ASC", repoID, sha, ref)
448}
449
450func (s *Store) newestPerJob(where string, args ...any) (map[string]Build, error) {
451 rows, err := s.DB.Query(buildSelect+where, args...)
441452 if err != nil {
442453 return nil, err
443454 }
@@ -490,18 +501,20 @@ func (s *Store) CancelBuild(id int64) error {
490501
491502// SuccessBuildForTree finds a passed build of the job for a tree rather
492503// than a commit: a rebase that changes nothing in the tree has already
493// been built (#177). Only a trusted build on the image the job names
504// been built (#177). Only a build for the same ref counts, since a job
505// sees GITBAY_REF and may do something else on another one. Only a
506// trusted build on the image the job names
494507// counts: a fork's result, or one from an image the job has left, does
495508// not stand for the repository's own (#258). A job naming no image
496509// matches builds that named none, whichever default the runner used;
497510// the CI wiki page says so. An empty tree never matches.
498func (s *Store) SuccessBuildForTree(repoID int64, tree, job, image string) (Build, bool, error) {
511func (s *Store) SuccessBuildForTree(repoID int64, tree, ref, job, image string) (Build, bool, error) {
499512 if tree == "" {
500513 return Build{}, false, nil
501514 }
502515 b, err := scanBuild(s.DB.QueryRow(buildSelect+
503 " WHERE repo_id = ? AND tree = ? AND job = ? AND image = ? AND trusted = 1 AND status = 'success'"+
504 " ORDER BY number DESC LIMIT 1", repoID, tree, job, image))
516 " WHERE repo_id = ? AND tree = ? AND ref = ? AND job = ? AND image = ? AND trusted = 1 AND status = 'success'"+
517 " ORDER BY number DESC LIMIT 1", repoID, tree, ref, job, image))
505518 if errors.Is(err, sql.ErrNoRows) {
506519 return Build{}, false, nil
507520 }
internal/store/builds_test.go +11 −7
@@ -214,8 +214,9 @@ func TestBuildLogSaysWhenItTruncates(t *testing.T) {
214214 }
215215}
216216
217// A success is found by tree across commits; an empty tree never matches,
218// so builds queued without one (scheduled, tag) are never reused (#177).
217// A success is found by tree across commits on one ref; an empty tree
218// never matches, so builds queued without one (scheduled, tag) are never
219// reused (#177).
219220func TestSuccessBuildForTree(t *testing.T) {
220221 s := open(t)
221222 if err := s.MigrateUp(); err != nil {
@@ -239,13 +240,16 @@ func TestSuccessBuildForTree(t *testing.T) {
239240 if err := s.FinishBuild(b["unit"].ID, "success"); err != nil {
240241 t.Fatal(err)
241242 }
242 if prev, ok, _ := s.SuccessBuildForTree(repoID, "tree1", "unit", ""); !ok || prev.SHA != "aaa" {
243 if prev, ok, _ := s.SuccessBuildForTree(repoID, "tree1", "main", "unit", ""); !ok || prev.SHA != "aaa" {
243244 t.Fatalf("success not found by tree: ok=%v prev=%+v", ok, prev)
244245 }
245 if _, ok, _ := s.SuccessBuildForTree(repoID, "tree1", "other", ""); ok {
246 if _, ok, _ := s.SuccessBuildForTree(repoID, "tree1", "main", "other", ""); ok {
246247 t.Error("matched a different job")
247248 }
248 if _, ok, _ := s.SuccessBuildForTree(repoID, "", "unit", ""); ok {
249 if _, ok, _ := s.SuccessBuildForTree(repoID, "tree1", "feat", "unit", ""); ok {
250 t.Error("matched a build for another ref")
251 }
252 if _, ok, _ := s.SuccessBuildForTree(repoID, "", "main", "unit", ""); ok {
249253 t.Error("an empty tree matched")
250254 }
251255}
@@ -278,10 +282,10 @@ func TestSuccessReuseNeedsTrustAndImage(t *testing.T) {
278282 t.Fatal(err)
279283 }
280284 }
281 if prev, ok, _ := s.SuccessBuildForTree(repoID, "tree1", "unit", ""); ok {
285 if prev, ok, _ := s.SuccessBuildForTree(repoID, "tree1", "main", "unit", ""); ok {
282286 t.Fatalf("reused build %d: untrusted, or on another image", prev.Number)
283287 }
284 if prev, ok, _ := s.SuccessBuildForTree(repoID, "tree1", "unit", "localhost/old:1"); !ok || prev.SHA != "bbb" {
288 if prev, ok, _ := s.SuccessBuildForTree(repoID, "tree1", "main", "unit", "localhost/old:1"); !ok || prev.SHA != "bbb" {
285289 t.Fatalf("trusted build on the same image not found: ok=%v prev=%+v", ok, prev)
286290 }
287291 if _, ok, _ := s.SuccessBuildFor(repoID, "aaa", "unit"); ok {