Commit 9a2a62e575

9a2a62e575135a9a70e2284c03d65b6d873c9e90

parent: 00033f022f

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

cmc <hello@cleberg.net> · 2026-09-05 06:34 UTC

ci: derive a diff base from the merge base on a branch's first push

Path filters (#169) fell back to running every job whenever old was
empty or all-zero, which is what a new branch's first push always
sends. Since branch-then-merge-request is the normal workflow, that
made filters apply only to a second push on an already-existing
branch — not the shape most changes have.

queueJobs now falls back to the merge base of the default branch and
the new sha when there is no old sha, computed at most once per call
and only when some job declares paths or paths-ignore. Still fails
open when the merge base can't be computed (unrelated histories), or
equals the new sha itself (a fresh repository's first push, which
moves the default branch with nothing to diff against).

QueueMRBuilds keeps its existing no-fallback, fail-open behavior:
require_checks refuses a merge with no statuses at all, and filtering
an MR head down to zero jobs would make it unmergeable rather than
just unfiltered (#172, not fixed here).

Closes #171

Layout: unified · split

e2e/cipaths_test.go +56
@@ -62,3 +62,59 @@ func TestCIPaths(t *testing.T) {
6262 t.Fatalf("src push did not queue the unit job:\n%s", out)
6363 }
6464}
65
66// A branch's first push has no old sha, but path filters must still
67// apply: the normal workflow here is branch, commit, open an MR, and
68// that first push is always a new branch. Without a merge-base fallback,
69// a docs-only branch queues the full suite anyway (#171).
70func TestCIPathsNewBranch(t *testing.T) {
71 inst := startInstance(t)
72 aliceKey := inst.newKey(t, "alice")
73 inst.admin(t, "admin", "user", "create", "alice", "--key", aliceKey+".pub")
74
75 if _, errOut, code := inst.ssh(t, aliceKey, "", "repo", "create", "alice/app"); code != 0 {
76 t.Fatalf("repo create: %s", errOut)
77 }
78 work := t.TempDir()
79 env := inst.gitEnv(aliceKey)
80 mustGit(t, work, env, "clone", inst.sshURL("alice/app"), "w")
81 dir := filepath.Join(work, "w")
82 os.MkdirAll(filepath.Join(dir, ".gitbay"), 0o755)
83 os.MkdirAll(filepath.Join(dir, "src"), 0o755)
84 os.MkdirAll(filepath.Join(dir, "docs"), 0o755)
85 os.WriteFile(filepath.Join(dir, ".gitbay", "ci.yml"), []byte(
86 "jobs:\n unit:\n paths-ignore:\n - docs/**\n steps:\n - echo fine\n"), 0o644)
87 os.WriteFile(filepath.Join(dir, "src", "x.go"), []byte("package x\n"), 0o644)
88 os.WriteFile(filepath.Join(dir, "docs", "x.md"), []byte("# x\n"), 0o644)
89 mustGit(t, dir, env, "checkout", "-q", "-b", "main")
90 mustGit(t, dir, env, "add", ".")
91 mustGit(t, dir, env, "commit", "-q", "-m", "base")
92 mustGit(t, dir, env, "push", "-q", "origin", "main")
93 before := strings.Count(inst.buildList(t, aliceKey), "\n")
94
95 // A brand-new branch whose only commit touches an ignored path: the
96 // filter must apply on this, its first push, not just on later ones.
97 mustGit(t, dir, env, "checkout", "-q", "-b", "docs-branch")
98 os.WriteFile(filepath.Join(dir, "docs", "x.md"), []byte("# x changed\n"), 0o644)
99 mustGit(t, dir, env, "add", ".")
100 mustGit(t, dir, env, "commit", "-q", "-m", "docs only")
101 mustGit(t, dir, env, "push", "-q", "origin", "docs-branch")
102 if out := inst.buildList(t, aliceKey); strings.Count(out, "\n") != before {
103 t.Fatalf("new branch's docs-only push queued a build:\n%s", out)
104 }
105
106 // A brand-new branch touching a matched path still queues, on its
107 // first push.
108 mustGit(t, dir, env, "checkout", "-q", "-b", "src-branch")
109 os.WriteFile(filepath.Join(dir, "src", "x.go"), []byte("package x\n\nvar y int\n"), 0o644)
110 mustGit(t, dir, env, "add", ".")
111 mustGit(t, dir, env, "commit", "-q", "-m", "src change")
112 mustGit(t, dir, env, "push", "-q", "origin", "src-branch")
113 out := inst.buildList(t, aliceKey)
114 if strings.Count(out, "\n") != before+1 {
115 t.Fatalf("new branch's src push did not queue a build:\n%s", out)
116 }
117 if !strings.Contains(out, "unit\tpending") {
118 t.Fatalf("new branch's src push did not queue the unit job:\n%s", out)
119 }
120}
internal/control/build.go +29 −9
@@ -502,13 +502,15 @@ func runRunnerDone(c *Ctx, args []string) int {
502502// the merge path for a merge, which updates the ref directly and so never
503503// reaches a hook. old is the branch's sha before this update, the diff
504504// base a job's path filters run against; a new branch has no prior
505// commit and sends old as empty or all zeros, either of which cannot be
506// diffed and runs every job.
505// commit and sends old as empty or all zeros. queueJobs falls back to
506// the merge base with the default branch in that case, so a filter
507// still applies to a branch's first push — the shape most changes have,
508// since branch-then-MR is the normal workflow here.
507509func QueueBranchBuilds(
508510 st *store.Store, root, siteURL string,
509511 repo store.Repo, userID int64, branch, old, sha string, now time.Time,
510512) {
511 queueJobs(st, root, siteURL, repo, userID, branch, old, sha, now, true, branch == repo.DefaultBranch)
513 queueJobs(st, root, siteURL, repo, userID, branch, old, sha, now, true, branch == repo.DefaultBranch, true)
512514}
513515
514516// QueueMRBuilds queues the push jobs for a merge request head fetched
@@ -522,15 +524,18 @@ func QueueMRBuilds(
522524 st *store.Store, root, siteURL string,
523525 repo store.Repo, userID, n int64, sha string,
524526) {
525 // No old sha: fails open and runs every job, matching today's
526 // behaviour for a merge request head.
527 queueJobs(st, root, siteURL, repo, userID, mrHeadRef(n), "", sha, time.Now(), false, false)
527 // No old sha, and unlike QueueBranchBuilds, no merge-base fallback
528 // either: this deliberately keeps failing open and running every
529 // job. require_checks refuses a merge when an MR head has no
530 // statuses at all (mr.go), so filtering a head down to zero jobs
531 // would make it unmergeable rather than just unfiltered (#172).
532 queueJobs(st, root, siteURL, repo, userID, mrHeadRef(n), "", sha, time.Now(), false, false, false)
528533}
529534
530535func queueJobs(
531536 st *store.Store, root, siteURL string,
532537 repo store.Repo, userID int64, ref, old, sha string, now time.Time,
533 trusted, syncSchedules bool,
538 trusted, syncSchedules, deriveMergeBase bool,
534539) {
535540 dir := RepoDir(root, repo.OwnerName, repo.Name)
536541 raw, err := gitutil.ReadBlob(dir, sha, ci.ConfigPath, 1<<16)
@@ -556,14 +561,29 @@ func queueJobs(
556561 // base does not exist or the diff itself fails, filtered stays
557562 // false and every job runs: a filter that cannot be evaluated must
558563 // not silently skip CI.
564 //
565 // A branch's first push has no old sha, but a diff base still
566 // exists: the merge base with the default branch. Without deriving
567 // one, every job runs on every new branch, and since branch-then-MR
568 // is the normal workflow, that is the push path filters matter most
569 // for. The merge base of the default branch's tip with itself is
570 // the tip, carrying no diff — that covers the default branch's own
571 // first push on a fresh repository, and must fail open rather than
572 // read as "nothing changed".
559573 filtered := false
560574 var changed []string
561575 for _, j := range jobs {
562576 if len(j.Paths) == 0 && len(j.PathsIgnore) == 0 {
563577 continue
564578 }
565 if ci.HasDiffBase(old) {
566 if files, err := gitutil.DiffFiles(dir, old, sha); err == nil {
579 diffOld := old
580 if !ci.HasDiffBase(diffOld) && deriveMergeBase {
581 if base, err := gitutil.MergeBase(dir, "refs/heads/"+repo.DefaultBranch, sha); err == nil && base != sha {
582 diffOld = base
583 }
584 }
585 if ci.HasDiffBase(diffOld) {
586 if files, err := gitutil.DiffFiles(dir, diffOld, sha); err == nil {
567587 changed, filtered = files, true
568588 }
569589 }
internal/control/build_test.go +270 −20
@@ -11,15 +11,42 @@ import (
1111 "gitbay.org/gitbay/internal/store"
1212)
1313
14// A DiffFiles failure must not turn into a silent skip: when the old sha
15// on record cannot be diffed against, every job runs regardless of what
16// it names in paths, the same as when there is no diff base at all.
17func TestQueueBranchBuildsFailsOpenOnDiffFailure(t *testing.T) {
14const testZeroSHA = "0000000000000000000000000000000000000000"
15
16// gitTestEnv sets up an isolated git identity so tests never touch a
17// developer's real config.
18func gitTestEnv() []string {
19 return append(os.Environ(),
20 "GIT_CONFIG_NOSYSTEM=1", "GIT_CONFIG_GLOBAL=/dev/null",
21 "GIT_AUTHOR_NAME=t", "GIT_AUTHOR_EMAIL=t@example.test",
22 "GIT_COMMITTER_NAME=t", "GIT_COMMITTER_EMAIL=t@example.test")
23}
24
25func gitRunner(t *testing.T) func(dir string, args ...string) string {
26 t.Helper()
27 env := gitTestEnv()
28 return func(dir string, args ...string) string {
29 t.Helper()
30 cmd := exec.Command("git", args...)
31 cmd.Dir = dir
32 cmd.Env = env
33 out, err := cmd.CombinedOutput()
34 if err != nil {
35 t.Fatalf("git %v: %v\n%s", args, err, out)
36 }
37 return string(out)
38 }
39}
40
41// newQueueTestRepo returns a store with one public repo (default branch
42// "main", matching the schema default) and the uid to queue builds as.
43func newQueueTestRepo(t *testing.T) (*store.Store, store.Repo, int64) {
44 t.Helper()
1845 st, err := store.Open(":memory:")
1946 if err != nil {
2047 t.Fatal(err)
2148 }
22 defer st.Close()
49 t.Cleanup(func() { st.Close() })
2350 if err := st.MigrateUp(); err != nil {
2451 t.Fatal(err)
2552 }
@@ -35,23 +62,17 @@ func TestQueueBranchBuildsFailsOpenOnDiffFailure(t *testing.T) {
3562 if err != nil {
3663 t.Fatal(err)
3764 }
65 return st, repo, uid
66}
67
68// A DiffFiles failure must not turn into a silent skip: when the old sha
69// on record cannot be diffed against, every job runs regardless of what
70// it names in paths, the same as when there is no diff base at all.
71func TestQueueBranchBuildsFailsOpenOnDiffFailure(t *testing.T) {
72 st, repo, uid := newQueueTestRepo(t)
73 git := gitRunner(t)
3874
3975 root := t.TempDir()
40 env := append(os.Environ(),
41 "GIT_CONFIG_NOSYSTEM=1", "GIT_CONFIG_GLOBAL=/dev/null",
42 "GIT_AUTHOR_NAME=t", "GIT_AUTHOR_EMAIL=t@example.test",
43 "GIT_COMMITTER_NAME=t", "GIT_COMMITTER_EMAIL=t@example.test")
44 git := func(dir string, args ...string) string {
45 t.Helper()
46 cmd := exec.Command("git", args...)
47 cmd.Dir = dir
48 cmd.Env = env
49 out, err := cmd.CombinedOutput()
50 if err != nil {
51 t.Fatalf("git %v: %v\n%s", args, err, out)
52 }
53 return string(out)
54 }
5576
5677 // A job whose paths would exclude a docs-only change, so the test
5778 // proves something: without fail-open, the diff failure would leave
@@ -89,3 +110,232 @@ func TestQueueBranchBuildsFailsOpenOnDiffFailure(t *testing.T) {
89110 t.Fatalf("queued build wrong: %+v", builds[0])
90111 }
91112}
113
114// A new branch's first push carries no old sha, but a diff base still
115// exists: the merge base with the default branch. A push whose commits
116// only touch paths a job ignores must not queue that job, or path
117// filters never do anything on the ordinary branch-then-MR workflow.
118func TestQueueBranchBuildsNewBranchIgnoredPathSkips(t *testing.T) {
119 st, repo, uid := newQueueTestRepo(t)
120 git := gitRunner(t)
121 root := t.TempDir()
122
123 src := filepath.Join(root, "src")
124 os.MkdirAll(filepath.Join(src, ".gitbay"), 0o755)
125 os.MkdirAll(filepath.Join(src, "docs"), 0o755)
126 os.WriteFile(filepath.Join(src, ".gitbay", "ci.yml"), []byte(
127 "jobs:\n unit:\n paths-ignore:\n - docs/**\n steps:\n - echo hi\n"), 0o644)
128 os.WriteFile(filepath.Join(src, "docs", "x.md"), []byte("# x\n"), 0o644)
129 git(root, "init", "-q", "-b", "main", "src")
130 git(src, "add", ".")
131 git(src, "commit", "-q", "-m", "base")
132
133 git(src, "checkout", "-q", "-b", "feature")
134 os.WriteFile(filepath.Join(src, "docs", "x.md"), []byte("# x changed\n"), 0o644)
135 git(src, "add", ".")
136 git(src, "commit", "-q", "-m", "docs only")
137 featureSHA := strings.TrimSpace(git(src, "rev-parse", "HEAD"))
138
139 dir := RepoDir(root, repo.OwnerName, repo.Name)
140 os.MkdirAll(filepath.Dir(dir), 0o755)
141 git(root, "clone", "-q", "--bare", src, dir)
142
143 QueueBranchBuilds(st, root, "https://x.test", repo, uid, "feature", testZeroSHA, featureSHA, time.Now())
144
145 builds, err := st.ListBuilds(repo.ID, 10)
146 if err != nil {
147 t.Fatal(err)
148 }
149 if len(builds) != 0 {
150 t.Fatalf("docs-only push on a new branch queued a build: %+v", builds)
151 }
152}
153
154// The same new-branch push, but touching a path the job cares about:
155// the merge-base diff must still let it through.
156func TestQueueBranchBuildsNewBranchMatchedPathQueues(t *testing.T) {
157 st, repo, uid := newQueueTestRepo(t)
158 git := gitRunner(t)
159 root := t.TempDir()
160
161 src := filepath.Join(root, "src")
162 os.MkdirAll(filepath.Join(src, ".gitbay"), 0o755)
163 os.MkdirAll(filepath.Join(src, "src"), 0o755)
164 os.WriteFile(filepath.Join(src, ".gitbay", "ci.yml"), []byte(
165 "jobs:\n unit:\n paths:\n - src/**\n steps:\n - echo hi\n"), 0o644)
166 os.WriteFile(filepath.Join(src, "src", "x.go"), []byte("package x\n"), 0o644)
167 git(root, "init", "-q", "-b", "main", "src")
168 git(src, "add", ".")
169 git(src, "commit", "-q", "-m", "base")
170
171 git(src, "checkout", "-q", "-b", "feature")
172 os.WriteFile(filepath.Join(src, "src", "x.go"), []byte("package x\n\nvar y int\n"), 0o644)
173 git(src, "add", ".")
174 git(src, "commit", "-q", "-m", "src change")
175 featureSHA := strings.TrimSpace(git(src, "rev-parse", "HEAD"))
176
177 dir := RepoDir(root, repo.OwnerName, repo.Name)
178 os.MkdirAll(filepath.Dir(dir), 0o755)
179 git(root, "clone", "-q", "--bare", src, dir)
180
181 QueueBranchBuilds(st, root, "https://x.test", repo, uid, "feature", testZeroSHA, featureSHA, time.Now())
182
183 builds, err := st.ListBuilds(repo.ID, 10)
184 if err != nil || len(builds) != 1 {
185 t.Fatalf("builds after queue: %v %v", builds, err)
186 }
187 if builds[0].Job != "unit" || builds[0].SHA != featureSHA {
188 t.Fatalf("queued build wrong: %+v", builds[0])
189 }
190}
191
192// When the new branch shares no history with the default branch, the
193// merge base cannot be computed. That must fail open, same as any other
194// diff base that cannot be evaluated.
195func TestQueueBranchBuildsNewBranchFailsOpenWithoutMergeBase(t *testing.T) {
196 st, repo, uid := newQueueTestRepo(t)
197 git := gitRunner(t)
198 root := t.TempDir()
199
200 dir := RepoDir(root, repo.OwnerName, repo.Name)
201 os.MkdirAll(filepath.Dir(dir), 0o755)
202 git(root, "init", "-q", "--bare", dir)
203
204 mainSrc := filepath.Join(root, "main-src")
205 os.MkdirAll(filepath.Join(mainSrc, ".gitbay"), 0o755)
206 os.WriteFile(filepath.Join(mainSrc, ".gitbay", "ci.yml"), []byte(
207 "jobs:\n unit:\n paths-ignore:\n - docs/**\n steps:\n - echo hi\n"), 0o644)
208 git(root, "init", "-q", "-b", "main", "main-src")
209 git(mainSrc, "add", ".")
210 git(mainSrc, "commit", "-q", "-m", "base")
211 git(mainSrc, "push", "-q", dir, "main")
212
213 // An unrelated repository: no common commit with main.
214 otherSrc := filepath.Join(root, "other-src")
215 os.MkdirAll(filepath.Join(otherSrc, ".gitbay"), 0o755)
216 os.MkdirAll(filepath.Join(otherSrc, "docs"), 0o755)
217 os.WriteFile(filepath.Join(otherSrc, ".gitbay", "ci.yml"), []byte(
218 "jobs:\n unit:\n paths-ignore:\n - docs/**\n steps:\n - echo hi\n"), 0o644)
219 os.WriteFile(filepath.Join(otherSrc, "docs", "x.md"), []byte("# x\n"), 0o644)
220 git(root, "init", "-q", "-b", "feature", "other-src")
221 git(otherSrc, "add", ".")
222 git(otherSrc, "commit", "-q", "-m", "unrelated docs-only")
223 otherSHA := strings.TrimSpace(git(otherSrc, "rev-parse", "HEAD"))
224 git(otherSrc, "push", "-q", dir, "feature")
225
226 QueueBranchBuilds(st, root, "https://x.test", repo, uid, "feature", testZeroSHA, otherSHA, time.Now())
227
228 builds, err := st.ListBuilds(repo.ID, 10)
229 if err != nil || len(builds) != 1 {
230 t.Fatalf("expected fail-open to queue the job: %v %v", builds, err)
231 }
232}
233
234// The first push to a brand-new repository moves the default branch
235// itself with no prior commit: the merge base of the default branch
236// against its own tip is the tip, which carries no diff. That must
237// fail open rather than read as "nothing changed".
238func TestQueueBranchBuildsFreshDefaultBranchFailsOpen(t *testing.T) {
239 st, repo, uid := newQueueTestRepo(t)
240 git := gitRunner(t)
241 root := t.TempDir()
242
243 src := filepath.Join(root, "src")
244 os.MkdirAll(filepath.Join(src, ".gitbay"), 0o755)
245 os.MkdirAll(filepath.Join(src, "docs"), 0o755)
246 os.WriteFile(filepath.Join(src, ".gitbay", "ci.yml"), []byte(
247 "jobs:\n unit:\n paths:\n - src/**\n steps:\n - echo hi\n"), 0o644)
248 os.WriteFile(filepath.Join(src, "docs", "x.md"), []byte("# x\n"), 0o644)
249 git(root, "init", "-q", "-b", "main", "src")
250 git(src, "add", ".")
251 git(src, "commit", "-q", "-m", "initial")
252 sha := strings.TrimSpace(git(src, "rev-parse", "HEAD"))
253
254 dir := RepoDir(root, repo.OwnerName, repo.Name)
255 os.MkdirAll(filepath.Dir(dir), 0o755)
256 git(root, "clone", "-q", "--bare", src, dir)
257
258 QueueBranchBuilds(st, root, "https://x.test", repo, uid, "main", testZeroSHA, sha, time.Now())
259
260 builds, err := st.ListBuilds(repo.ID, 10)
261 if err != nil || len(builds) != 1 {
262 t.Fatalf("expected fail-open on the repository's first commit: %v %v", builds, err)
263 }
264}
265
266// An ordinary push with a genuine old sha is unaffected by the
267// new-branch handling: filtering still works exactly as it did before.
268func TestQueueBranchBuildsOrdinaryPushStillFilters(t *testing.T) {
269 st, repo, uid := newQueueTestRepo(t)
270 git := gitRunner(t)
271 root := t.TempDir()
272
273 src := filepath.Join(root, "src")
274 os.MkdirAll(filepath.Join(src, ".gitbay"), 0o755)
275 os.MkdirAll(filepath.Join(src, "docs"), 0o755)
276 os.WriteFile(filepath.Join(src, ".gitbay", "ci.yml"), []byte(
277 "jobs:\n unit:\n paths-ignore:\n - docs/**\n steps:\n - echo hi\n"), 0o644)
278 os.WriteFile(filepath.Join(src, "docs", "x.md"), []byte("# x\n"), 0o644)
279 git(root, "init", "-q", "-b", "main", "src")
280 git(src, "add", ".")
281 git(src, "commit", "-q", "-m", "base")
282 oldSHA := strings.TrimSpace(git(src, "rev-parse", "HEAD"))
283
284 os.WriteFile(filepath.Join(src, "docs", "x.md"), []byte("# x changed\n"), 0o644)
285 git(src, "add", ".")
286 git(src, "commit", "-q", "-m", "docs only")
287 newSHA := strings.TrimSpace(git(src, "rev-parse", "HEAD"))
288
289 dir := RepoDir(root, repo.OwnerName, repo.Name)
290 os.MkdirAll(filepath.Dir(dir), 0o755)
291 git(root, "clone", "-q", "--bare", src, dir)
292
293 QueueBranchBuilds(st, root, "https://x.test", repo, uid, "main", oldSHA, newSHA, time.Now())
294
295 builds, err := st.ListBuilds(repo.ID, 10)
296 if err != nil {
297 t.Fatal(err)
298 }
299 if len(builds) != 0 {
300 t.Fatalf("docs-only push with a real old sha queued a build: %+v", builds)
301 }
302}
303
304// QueueMRBuilds keeps failing open with no diff base at all: deriving a
305// merge base for the MR head is deliberately out of scope here (#172 —
306// filtering a head down to zero jobs leaves it with no statuses, which
307// the require_checks gate reads as unmergeable). This test documents
308// and locks in that choice.
309func TestQueueMRBuildsStillFailsOpen(t *testing.T) {
310 st, repo, uid := newQueueTestRepo(t)
311 git := gitRunner(t)
312 root := t.TempDir()
313
314 src := filepath.Join(root, "src")
315 os.MkdirAll(filepath.Join(src, ".gitbay"), 0o755)
316 os.MkdirAll(filepath.Join(src, "docs"), 0o755)
317 os.WriteFile(filepath.Join(src, ".gitbay", "ci.yml"), []byte(
318 "jobs:\n unit:\n paths-ignore:\n - docs/**\n steps:\n - echo hi\n"), 0o644)
319 os.WriteFile(filepath.Join(src, "docs", "x.md"), []byte("# x\n"), 0o644)
320 git(root, "init", "-q", "-b", "main", "src")
321 git(src, "add", ".")
322 git(src, "commit", "-q", "-m", "base")
323
324 git(src, "checkout", "-q", "-b", "pr")
325 os.WriteFile(filepath.Join(src, "docs", "x.md"), []byte("# x changed\n"), 0o644)
326 git(src, "add", ".")
327 git(src, "commit", "-q", "-m", "docs only")
328 prSHA := strings.TrimSpace(git(src, "rev-parse", "HEAD"))
329
330 dir := RepoDir(root, repo.OwnerName, repo.Name)
331 os.MkdirAll(filepath.Dir(dir), 0o755)
332 git(root, "clone", "-q", "--bare", src, dir)
333 git(dir, "update-ref", "refs/merge-requests/1/head", prSHA)
334
335 QueueMRBuilds(st, root, "https://x.test", repo, uid, 1, prSHA)
336
337 builds, err := st.ListBuilds(repo.ID, 10)
338 if err != nil || len(builds) != 1 {
339 t.Fatalf("expected the MR head to fail open and queue a build: %v %v", builds, err)
340 }
341}