Commit 5671f4a322

5671f4a32294b8ea2aa6b3b7cfffd0e9964b258e

parent: e0b9ff1afa

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

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

ci: cover the DiffFiles failure path, tighten path-filter comments

QueueBranchBuilds fails open when the old sha resolves to nothing, not
just when it is empty or all zeros; a real bare repo with a well-formed
but nonexistent old sha exercises the actual git-diff failure, proving
a job whose paths would otherwise exclude the change still queues.

Job.Paths documents that it's inert on a scheduled or tag job, and the
QueueBranchBuilds comment now names both empty and all-zero old as the
new-branch case.

Ref #169

Layout: unified · split

internal/ci/ci.go +6 −4
@@ -31,10 +31,12 @@ const (
3131var jobName = regexp.MustCompile(`^[a-z0-9][a-z0-9_-]{0,39}$`)
3232
3333type Job struct {
34 Name string
35 Steps []string
36 Schedule string // cron expression; scheduled jobs run on schedule, not on push
37 Tags string // tag glob (e.g. "v*"); tag jobs run on matching tag pushes only
34 Name string
35 Steps []string
36 Schedule string // cron expression; scheduled jobs run on schedule, not on push
37 Tags string // tag glob (e.g. "v*"); tag jobs run on matching tag pushes only
38 // Paths and PathsIgnore only gate a job queued on push; a scheduled
39 // or tag job ignores them.
3840 Paths []string // globs; the job runs only when a changed file matches one
3941 PathsIgnore []string // globs; the job is skipped when every changed file matches one
4042}
internal/control/build.go +3 −2
@@ -501,8 +501,9 @@ func runRunnerDone(c *Ctx, args []string) int {
501501// Both paths that move a branch call this: post-receive for a push, and
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
504// base a job's path filters run against; an empty old, from a new
505// branch, cannot be diffed and runs every job.
504// 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.
506507func QueueBranchBuilds(
507508 st *store.Store, root, siteURL string,
508509 repo store.Repo, userID int64, branch, old, sha string, now time.Time,
internal/control/build_test.go added +91
@@ -0,0 +1,91 @@
1package control
2
3import (
4 "os"
5 "os/exec"
6 "path/filepath"
7 "strings"
8 "testing"
9 "time"
10
11 "gitbay.org/gitbay/internal/store"
12)
13
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) {
18 st, err := store.Open(":memory:")
19 if err != nil {
20 t.Fatal(err)
21 }
22 defer st.Close()
23 if err := st.MigrateUp(); err != nil {
24 t.Fatal(err)
25 }
26 uid, err := st.CreateUser("alice", false)
27 if err != nil {
28 t.Fatal(err)
29 }
30 repoID, err := st.CreateRepo("user", uid, "app", "public")
31 if err != nil {
32 t.Fatal(err)
33 }
34 repo, err := st.RepoByID(repoID)
35 if err != nil {
36 t.Fatal(err)
37 }
38
39 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 }
55
56 // A job whose paths would exclude a docs-only change, so the test
57 // proves something: without fail-open, the diff failure would leave
58 // the filter unevaluated and this build would never queue.
59 src := filepath.Join(root, "src")
60 os.MkdirAll(filepath.Join(src, ".gitbay"), 0o755)
61 os.MkdirAll(filepath.Join(src, "docs"), 0o755)
62 os.WriteFile(filepath.Join(src, ".gitbay", "ci.yml"), []byte(
63 "jobs:\n unit:\n paths:\n - src/**\n steps:\n - echo hi\n"), 0o644)
64 os.WriteFile(filepath.Join(src, "docs", "x.md"), []byte("# x\n"), 0o644)
65 git(root, "init", "-q", "-b", "main", "src")
66 git(src, "add", ".")
67 git(src, "commit", "-q", "-m", "base")
68 os.WriteFile(filepath.Join(src, "docs", "x.md"), []byte("# x changed\n"), 0o644)
69 git(src, "add", ".")
70 git(src, "commit", "-q", "-m", "docs only")
71 newSHA := strings.TrimSpace(git(src, "rev-parse", "HEAD"))
72
73 dir := RepoDir(root, repo.OwnerName, repo.Name)
74 os.MkdirAll(filepath.Dir(dir), 0o755)
75 git(root, "clone", "-q", "--bare", src, dir)
76
77 // Well-formed but names no object in this repo: git diff itself
78 // fails, rather than the empty/all-zero short-circuit HasDiffBase
79 // already covers.
80 old := strings.Repeat("1", 40)
81
82 QueueBranchBuilds(st, root, "https://x.test", repo, uid, "main", old, newSHA, time.Now())
83
84 builds, err := st.ListBuilds(repo.ID, 10)
85 if err != nil || len(builds) != 1 {
86 t.Fatalf("builds after queue: %v %v", builds, err)
87 }
88 if builds[0].Job != "unit" || builds[0].Status != "pending" || builds[0].SHA != newSHA {
89 t.Fatalf("queued build wrong: %+v", builds[0])
90 }
91}