Commit f994d5a21a
Verified · cmc ci/build: success ci/sonar: success ci/test: success ci/vuln: success
Layout: unified · split
e2e/buildorphan_test.go added +72
| @@ -0,0 +1,72 @@ | ||
| 1 | package e2e | |
| 2 | ||
| 3 | import ( | |
| 4 | "os" | |
| 5 | "path/filepath" | |
| 6 | "strings" | |
| 7 | "testing" | |
| 8 | ) | |
| 9 | ||
| 10 | // Signed commits mean only fast-forward merges are allowed, so a branch | |
| 11 | // whose target advances gets rebased and force-pushed — orphaning whatever | |
| 12 | // was queued for the old head. That build must not run and fail at clone | |
| 13 | // looking like a real failure; it is cancelled when a runner claims it, and | |
| 14 | // the runner gets the real build behind it instead, in the same poll. | |
| 15 | func TestBuildOrphanedByForcePushCancelledAtClaim(t *testing.T) { | |
| 16 | inst := startInstance(t) | |
| 17 | inst.runner = buildRunner(t) | |
| 18 | aliceKey := inst.newKey(t, "alice") | |
| 19 | runnerKey := inst.newKey(t, "ci") | |
| 20 | inst.admin(t, "admin", "user", "create", "alice", "--key", aliceKey+".pub") | |
| 21 | inst.admin(t, "admin", "user", "create", "ci", "--key", runnerKey+".pub", "--admin") | |
| 22 | if _, _, code := inst.ssh(t, aliceKey, "", "repo", "create", "alice/app"); code != 0 { | |
| 23 | t.Fatal("repo create failed") | |
| 24 | } | |
| 25 | work := t.TempDir() | |
| 26 | env := inst.gitEnv(aliceKey) | |
| 27 | mustGit(t, work, env, "clone", inst.sshURL("alice/app"), "w") | |
| 28 | dir := filepath.Join(work, "w") | |
| 29 | os.MkdirAll(filepath.Join(dir, ".gitbay"), 0o755) | |
| 30 | os.WriteFile(filepath.Join(dir, ".gitbay", "ci.yml"), []byte("jobs:\n unit:\n steps:\n - echo fine\n"), 0o644) | |
| 31 | mustGit(t, dir, env, "checkout", "-q", "-b", "main") | |
| 32 | mustGit(t, dir, env, "add", ".") | |
| 33 | mustGit(t, dir, env, "commit", "-q", "-m", "base") | |
| 34 | mustGit(t, dir, env, "push", "-q", "origin", "main") | |
| 35 | orphanSHA := strings.TrimSpace(mustGit(t, dir, env, "rev-parse", "HEAD")) | |
| 36 | ||
| 37 | // Amend and force-push: build 1 (queued for orphanSHA) is left behind, | |
| 38 | // unreachable once main moves; build 2 queues for the amended commit. | |
| 39 | mustGit(t, dir, env, "commit", "-q", "--amend", "-m", "amended") | |
| 40 | mustGit(t, dir, env, "push", "-q", "--force", "origin", "main") | |
| 41 | realSHA := strings.TrimSpace(mustGit(t, dir, env, "rev-parse", "HEAD")) | |
| 42 | if orphanSHA == realSHA { | |
| 43 | t.Fatal("amend produced the same sha; test setup is broken") | |
| 44 | } | |
| 45 | ||
| 46 | out, _, _ := inst.ssh(t, aliceKey, "", "build", "list", "alice/app") | |
| 47 | if !strings.Contains(out, "1\tunit\tpending") || !strings.Contains(out, "2\tunit\tpending") { | |
| 48 | t.Fatalf("expected both builds queued and pending:\n%s", out) | |
| 49 | } | |
| 50 | ||
| 51 | // One claim: the orphaned build is cancelled and the real one runs, | |
| 52 | // without a second poll. | |
| 53 | inst.runnerOnce(t, runnerKey) | |
| 54 | ||
| 55 | out, _, _ = inst.ssh(t, aliceKey, "", "build", "list", "alice/app") | |
| 56 | if !strings.Contains(out, "1\tunit\tcancelled") { | |
| 57 | t.Fatalf("orphaned build not cancelled:\n%s", out) | |
| 58 | } | |
| 59 | if !strings.Contains(out, "2\tunit\tsuccess") { | |
| 60 | t.Fatalf("real build behind it did not run:\n%s", out) | |
| 61 | } | |
| 62 | ||
| 63 | log, _, _ := inst.ssh(t, aliceKey, "", "build", "log", "alice/app", "1") | |
| 64 | if !strings.Contains(log, "not reachable") { | |
| 65 | t.Fatalf("cancelled build's log does not explain why:\n%s", log) | |
| 66 | } | |
| 67 | ||
| 68 | st, _, _ := inst.ssh(t, aliceKey, "", "status", "list", "alice/app", orphanSHA) | |
| 69 | if !strings.Contains(st, "ci/unit") || strings.Contains(st, "pending") { | |
| 70 | t.Fatalf("orphaned commit's status left pending:\n%s", st) | |
| 71 | } | |
| 72 | } | |
internal/control/build.go +77 −18
| @@ -315,6 +315,16 @@ func requireRunner(c *Ctx) int { | ||
| 315 | 315 | return -1 |
| 316 | 316 | } |
| 317 | 317 | |
| 318 | // maxOrphanSkip bounds how many claimed builds runRunnerNext will find | |
| 319 | // unreachable and cancel in one call before giving up. Only fast-forward | |
| 320 | // merges are allowed here, so any branch whose target advances gets | |
| 321 | // rebased and force-pushed, and a stack of branches can do that repeatedly | |
| 322 | // in one sitting — the issue this guards saw five in an afternoon. The cap | |
| 323 | // is well above that, so a real backlog is never cut short, while a | |
| 324 | // repository whose queue is orphaned end to end still returns rather than | |
| 325 | // walking it forever. | |
| 326 | const maxOrphanSkip = 50 | |
| 327 | ||
| 318 | 328 | func runRunnerNext(c *Ctx, args []string) int { |
| 319 | 329 | if code := requireRunner(c); code >= 0 { |
| 320 | 330 | return code |
| @@ -330,19 +340,45 @@ func runRunnerNext(c *Ctx, args []string) int { | ||
| 330 | 340 | } |
| 331 | 341 | repoIDs = append(repoIDs, repo.ID) |
| 332 | 342 | } |
| 333 | b, ok, err := c.Store.ClaimBuild(repoIDs) | |
| 334 | if err != nil { | |
| 335 | return c.fail(protocol.ExitFailure, "%v", err) | |
| 343 | var b store.Build | |
| 344 | var repo store.Repo | |
| 345 | var ok bool | |
| 346 | var err error | |
| 347 | for attempt := 0; attempt < maxOrphanSkip; attempt++ { | |
| 348 | b, ok, err = c.Store.ClaimBuild(repoIDs) | |
| 349 | if err != nil { | |
| 350 | return c.fail(protocol.ExitFailure, "%v", err) | |
| 351 | } | |
| 352 | if !ok { | |
| 353 | break | |
| 354 | } | |
| 355 | repo, err = c.Store.RepoByID(b.RepoID) | |
| 356 | if err != nil { | |
| 357 | return c.fail(protocol.ExitFailure, "%v", err) | |
| 358 | } | |
| 359 | // Only fast-forward merges are allowed here, so a target that | |
| 360 | // advances gets rebased and force-pushed, orphaning whatever was | |
| 361 | // queued for the old head: the runner would clone the repo and | |
| 362 | // fail at checkout with a git internal error that reads exactly | |
| 363 | // like a real failure. Catch it here instead. A check that itself | |
| 364 | // fails is not evidence of anything — the build runs for real and | |
| 365 | // is left to fail on its own terms, never cancelled on a guess. | |
| 366 | reachable, err := gitutil.Reachable(RepoDir(c.Cfg.Server.Root, repo.OwnerName, repo.Name), b.SHA) | |
| 367 | if err != nil || reachable { | |
| 368 | break | |
| 369 | } | |
| 370 | if code := cancelOrphanedBuild(c, repo, b); code >= 0 { | |
| 371 | return code | |
| 372 | } | |
| 373 | // Cancelled, not claimed: if the cap is hit right here, the runner | |
| 374 | // heartbeat below must not record this build as the one handed out. | |
| 375 | b, ok = store.Build{}, false | |
| 336 | 376 | } |
| 337 | 377 | // The poll itself is the runner's heartbeat: admin runners reads it. |
| 338 | 378 | c.Store.TouchRunner(c.User.ID, strings.Join(args, ","), b.ID) |
| 339 | 379 | if !ok { |
| 340 | 380 | return c.emit(map[string]any{}, func(w io.Writer) { fmt.Fprintln(w, "no pending builds") }) |
| 341 | 381 | } |
| 342 | repo, err := c.Store.RepoByID(b.RepoID) | |
| 343 | if err != nil { | |
| 344 | return c.fail(protocol.ExitFailure, "%v", err) | |
| 345 | } | |
| 346 | 382 | var steps []string |
| 347 | 383 | json.Unmarshal([]byte(b.Steps), &steps) |
| 348 | 384 | // Secrets ride the claim: this channel is admin-only and the values |
| @@ -652,6 +688,38 @@ func queueJobs( | ||
| 652 | 688 | } |
| 653 | 689 | } |
| 654 | 690 | |
| 691 | // resolveCancelledCommitStatus sets the commit status for a build that was | |
| 692 | // just cancelled: if the commit already passed this job on another ref, | |
| 693 | // that result stands again; otherwise the context reports the | |
| 694 | // cancellation as an error, so the queued status left behind is never | |
| 695 | // pending forever. | |
| 696 | func resolveCancelledCommitStatus(c *Ctx, repo store.Repo, b store.Build) { | |
| 697 | if prev, ok, err := c.Store.SuccessBuildFor(repo.ID, b.SHA, b.Job); err == nil && ok { | |
| 698 | url := fmt.Sprintf("%s/%s/builds/%d", c.Cfg.Server.SiteURL, repo.Path(), prev.Number) | |
| 699 | c.Store.SetCommitStatus(repo.ID, b.SHA, "ci/"+b.Job, "success", | |
| 700 | fmt.Sprintf("passed in build %d on %s", prev.Number, prev.Ref), url, c.User.ID) | |
| 701 | return | |
| 702 | } | |
| 703 | url := fmt.Sprintf("%s/%s/builds/%d", c.Cfg.Server.SiteURL, repo.Path(), b.Number) | |
| 704 | c.Store.SetCommitStatus(repo.ID, b.SHA, "ci/"+b.Job, "error", "cancelled", url, c.User.ID) | |
| 705 | } | |
| 706 | ||
| 707 | // cancelOrphanedBuild withdraws a build runRunnerNext claimed and then | |
| 708 | // found unreachable. It leaves the same shape behind as a build cancel a | |
| 709 | // person runs by hand: CancelBuild's status, a log line saying why, and | |
| 710 | // the commit status resolved rather than left pending. Returns -1 to mean | |
| 711 | // "handled, keep going"; anything else is the exit code to return. | |
| 712 | func cancelOrphanedBuild(c *Ctx, repo store.Repo, b store.Build) int { | |
| 713 | if err := c.Store.CancelBuild(b.ID); err != nil { | |
| 714 | return c.fail(protocol.ExitFailure, "%v", err) | |
| 715 | } | |
| 716 | c.Store.AppendBuildLog(b.ID, []byte(fmt.Sprintf( | |
| 717 | "cancelled: %.10s is not reachable from any ref; the sha was likely orphaned by a force-push\n", b.SHA))) | |
| 718 | resolveCancelledCommitStatus(c, repo, b) | |
| 719 | c.Store.RecordEvent(repo.ID, c.User.ID, "build.cancelled", fmt.Sprintf(`{"number":%d,"job":%q}`, b.Number, b.Job)) | |
| 720 | return -1 | |
| 721 | } | |
| 722 | ||
| 655 | 723 | func runBuildCancel(c *Ctx, args []string) int { |
| 656 | 724 | repo, b, code := buildRef(c, args) |
| 657 | 725 | if code >= 0 { |
| @@ -675,17 +743,8 @@ func runBuildCancel(c *Ctx, args []string) int { | ||
| 675 | 743 | } else { |
| 676 | 744 | c.Store.AppendBuildLog(b.ID, []byte(fmt.Sprintf("cancelled by %s before a runner claimed it\n", c.User.Username))) |
| 677 | 745 | } |
| 678 | // The queued status replaced whatever the commit had for this job. If | |
| 679 | // the commit passed the job on another ref, that result stands again; | |
| 680 | // otherwise the context says it was withdrawn. | |
| 681 | if prev, ok, err := c.Store.SuccessBuildFor(repo.ID, b.SHA, b.Job); err == nil && ok { | |
| 682 | url := fmt.Sprintf("%s/%s/builds/%d", c.Cfg.Server.SiteURL, repo.Path(), prev.Number) | |
| 683 | c.Store.SetCommitStatus(repo.ID, b.SHA, "ci/"+b.Job, "success", | |
| 684 | fmt.Sprintf("passed in build %d on %s", prev.Number, prev.Ref), url, c.User.ID) | |
| 685 | } else { | |
| 686 | url := fmt.Sprintf("%s/%s/builds/%d", c.Cfg.Server.SiteURL, repo.Path(), b.Number) | |
| 687 | c.Store.SetCommitStatus(repo.ID, b.SHA, "ci/"+b.Job, "error", "cancelled", url, c.User.ID) | |
| 688 | } | |
| 746 | // The queued status replaced whatever the commit had for this job. | |
| 747 | resolveCancelledCommitStatus(c, repo, b) | |
| 689 | 748 | c.Store.RecordEvent(repo.ID, c.User.ID, "build.cancelled", fmt.Sprintf(`{"number":%d,"job":%q}`, b.Number, b.Job)) |
| 690 | 749 | return c.emit(map[string]any{"number": b.Number, "job": b.Job, "status": "cancelled", "was": b.Status}, func(w io.Writer) { |
| 691 | 750 | if b.Status == "running" { |
internal/control/runnernext_test.go added +217
| @@ -0,0 +1,217 @@ | ||
| 1 | package control | |
| 2 | ||
| 3 | import ( | |
| 4 | "bytes" | |
| 5 | "fmt" | |
| 6 | "os" | |
| 7 | "path/filepath" | |
| 8 | "strings" | |
| 9 | "testing" | |
| 10 | ||
| 11 | "gitbay.org/gitbay/internal/config" | |
| 12 | "gitbay.org/gitbay/internal/protocol" | |
| 13 | "gitbay.org/gitbay/internal/store" | |
| 14 | ) | |
| 15 | ||
| 16 | // runnerCtx builds a Ctx good enough to run runRunnerNext directly: an | |
| 17 | // admin user (requireRunner accepts admin as well as scope "runner"), a | |
| 18 | // server root that matches where the test's bare repo lives. | |
| 19 | func runnerCtx(st *store.Store, uid int64, root string) (*Ctx, *bytes.Buffer) { | |
| 20 | var out bytes.Buffer | |
| 21 | c := &Ctx{ | |
| 22 | User: store.User{ID: uid, Username: "ci", IsAdmin: true}, | |
| 23 | Store: st, | |
| 24 | Cfg: config.Config{Server: config.Server{Root: root, SiteURL: "https://x.test"}}, | |
| 25 | Stdin: strings.NewReader(""), | |
| 26 | Stdout: &out, | |
| 27 | Stderr: &out, | |
| 28 | } | |
| 29 | return c, &out | |
| 30 | } | |
| 31 | ||
| 32 | // A queued build's sha is orphaned by rewinding "main" past it — the shape | |
| 33 | // a force-push leaves, without needing an actual git-receive-pack round | |
| 34 | // trip. The base commit stays reachable, giving one real build behind it. | |
| 35 | func setupOrphanRepo(t *testing.T) (*store.Store, store.Repo, int64, string, string, string) { | |
| 36 | t.Helper() | |
| 37 | st, repo, uid := newQueueTestRepo(t) | |
| 38 | git := gitRunner(t) | |
| 39 | root := t.TempDir() | |
| 40 | ||
| 41 | src := filepath.Join(root, "src") | |
| 42 | os.MkdirAll(src, 0o755) | |
| 43 | git(root, "init", "-q", "-b", "main", "src") | |
| 44 | git(src, "commit", "-q", "--allow-empty", "-m", "base") | |
| 45 | baseSHA := strings.TrimSpace(git(src, "rev-parse", "HEAD")) | |
| 46 | git(src, "commit", "-q", "--allow-empty", "-m", "orphaned") | |
| 47 | orphanSHA := strings.TrimSpace(git(src, "rev-parse", "HEAD")) | |
| 48 | ||
| 49 | dir := RepoDir(root, repo.OwnerName, repo.Name) | |
| 50 | os.MkdirAll(filepath.Dir(dir), 0o755) | |
| 51 | git(root, "clone", "-q", "--bare", src, dir) | |
| 52 | git(dir, "update-ref", "refs/heads/main", baseSHA) | |
| 53 | ||
| 54 | return st, repo, uid, root, baseSHA, orphanSHA | |
| 55 | } | |
| 56 | ||
| 57 | // A build queued for a sha a force-push orphaned is cancelled at claim | |
| 58 | // time, and the runner gets the next real build instead of an impossible | |
| 59 | // one. | |
| 60 | func TestRunnerNextSkipsOrphanedBuildAndClaimsNext(t *testing.T) { | |
| 61 | st, repo, uid, root, baseSHA, orphanSHA := setupOrphanRepo(t) | |
| 62 | ||
| 63 | orphanedID, err := st.CreateBuild(repo.ID, "unit", orphanSHA, "main", "[]", true) | |
| 64 | if err != nil { | |
| 65 | t.Fatal(err) | |
| 66 | } | |
| 67 | if err := st.SetCommitStatus(repo.ID, orphanSHA, "ci/unit", "pending", "queued", "https://x.test", uid); err != nil { | |
| 68 | t.Fatal(err) | |
| 69 | } | |
| 70 | realID, err := st.CreateBuild(repo.ID, "unit", baseSHA, "main", "[]", true) | |
| 71 | if err != nil { | |
| 72 | t.Fatal(err) | |
| 73 | } | |
| 74 | ||
| 75 | c, out := runnerCtx(st, uid, root) | |
| 76 | code := runRunnerNext(c, nil) | |
| 77 | if code != protocol.ExitOK { | |
| 78 | t.Fatalf("runner next: exit %d, output:\n%s", code, out.String()) | |
| 79 | } | |
| 80 | if !strings.Contains(out.String(), fmt.Sprintf("build %d: ", realID)) || !strings.Contains(out.String(), baseSHA[:10]) { | |
| 81 | t.Fatalf("expected the real build handed out, got:\n%s", out.String()) | |
| 82 | } | |
| 83 | ||
| 84 | orphaned, err := st.BuildByNumber(repo.ID, orphanedID) | |
| 85 | if err != nil { | |
| 86 | t.Fatal(err) | |
| 87 | } | |
| 88 | if orphaned.Status != "cancelled" { | |
| 89 | t.Fatalf("orphaned build status = %q, want cancelled", orphaned.Status) | |
| 90 | } | |
| 91 | log, _ := st.BuildLog(orphaned.ID) | |
| 92 | if !strings.Contains(string(log), "not reachable") { | |
| 93 | t.Fatalf("orphaned build log missing the reason:\n%s", log) | |
| 94 | } | |
| 95 | statuses, err := st.ListCommitStatuses(repo.ID, orphanSHA) | |
| 96 | if err != nil { | |
| 97 | t.Fatal(err) | |
| 98 | } | |
| 99 | if len(statuses) != 1 || statuses[0].State == "pending" { | |
| 100 | t.Fatalf("orphaned build's commit status still pending: %+v", statuses) | |
| 101 | } | |
| 102 | ||
| 103 | real, err := st.BuildByNumber(repo.ID, realID) | |
| 104 | if err != nil { | |
| 105 | t.Fatal(err) | |
| 106 | } | |
| 107 | if real.Status != "running" { | |
| 108 | t.Fatalf("real build status = %q, want running (claimed)", real.Status) | |
| 109 | } | |
| 110 | } | |
| 111 | ||
| 112 | // A build whose sha is genuinely reachable is claimed exactly as before: | |
| 113 | // the reachability check must never reject a healthy build. | |
| 114 | func TestRunnerNextClaimsReachableBuildNormally(t *testing.T) { | |
| 115 | st, repo, uid, root, baseSHA, _ := setupOrphanRepo(t) | |
| 116 | id, err := st.CreateBuild(repo.ID, "unit", baseSHA, "main", "[]", true) | |
| 117 | if err != nil { | |
| 118 | t.Fatal(err) | |
| 119 | } | |
| 120 | ||
| 121 | c, out := runnerCtx(st, uid, root) | |
| 122 | code := runRunnerNext(c, nil) | |
| 123 | if code != protocol.ExitOK { | |
| 124 | t.Fatalf("runner next: exit %d, output:\n%s", code, out.String()) | |
| 125 | } | |
| 126 | if strings.Contains(out.String(), "no pending builds") { | |
| 127 | t.Fatalf("a reachable build was not handed out:\n%s", out.String()) | |
| 128 | } | |
| 129 | b, err := st.BuildByNumber(repo.ID, id) | |
| 130 | if err != nil { | |
| 131 | t.Fatal(err) | |
| 132 | } | |
| 133 | if b.Status != "running" { | |
| 134 | t.Fatalf("reachable build status = %q, want running", b.Status) | |
| 135 | } | |
| 136 | log, _ := st.BuildLog(b.ID) | |
| 137 | if strings.Contains(string(log), "cancelled") { | |
| 138 | t.Fatalf("a healthy build was cancelled:\n%s", log) | |
| 139 | } | |
| 140 | } | |
| 141 | ||
| 142 | // A queue built entirely of one orphaned sha, past the loop's cap, still | |
| 143 | // terminates and reports no pending builds — never a spin, never an | |
| 144 | // error — while everything up to the cap is actually resolved rather than | |
| 145 | // left claimed and dangling. | |
| 146 | func TestRunnerNextOrphanedQueuePastCapReportsNoPendingBuilds(t *testing.T) { | |
| 147 | st, repo, uid, root, _, orphanSHA := setupOrphanRepo(t) | |
| 148 | total := maxOrphanSkip + 1 | |
| 149 | for i := 0; i < total; i++ { | |
| 150 | if _, err := st.CreateBuild(repo.ID, fmt.Sprintf("job%d", i), orphanSHA, "main", "[]", true); err != nil { | |
| 151 | t.Fatal(err) | |
| 152 | } | |
| 153 | } | |
| 154 | ||
| 155 | c, out := runnerCtx(st, uid, root) | |
| 156 | code := runRunnerNext(c, nil) | |
| 157 | if code != protocol.ExitOK { | |
| 158 | t.Fatalf("runner next: exit %d, output:\n%s", code, out.String()) | |
| 159 | } | |
| 160 | if !strings.Contains(out.String(), "no pending builds") { | |
| 161 | t.Fatalf("expected no pending builds, got:\n%s", out.String()) | |
| 162 | } | |
| 163 | ||
| 164 | builds, err := st.ListBuilds(repo.ID, total+1) | |
| 165 | if err != nil { | |
| 166 | t.Fatal(err) | |
| 167 | } | |
| 168 | var cancelled, pending, other int | |
| 169 | for _, b := range builds { | |
| 170 | switch b.Status { | |
| 171 | case "cancelled": | |
| 172 | cancelled++ | |
| 173 | case "pending": | |
| 174 | pending++ | |
| 175 | default: | |
| 176 | other++ | |
| 177 | } | |
| 178 | } | |
| 179 | if other != 0 { | |
| 180 | t.Fatalf("a build was left claimed rather than resolved: cancelled=%d pending=%d other=%d", cancelled, pending, other) | |
| 181 | } | |
| 182 | if cancelled != maxOrphanSkip { | |
| 183 | t.Fatalf("cancelled %d builds, want the cap of %d", cancelled, maxOrphanSkip) | |
| 184 | } | |
| 185 | if pending != total-maxOrphanSkip { | |
| 186 | t.Fatalf("pending %d builds, want %d left behind by the cap", pending, total-maxOrphanSkip) | |
| 187 | } | |
| 188 | } | |
| 189 | ||
| 190 | // Reachable erroring — no repository on disk at all — must not read as | |
| 191 | // "unreachable": the ambiguous case is claimable, never cancelled. | |
| 192 | func TestRunnerNextClaimsBuildWhenReachabilityCannotBeChecked(t *testing.T) { | |
| 193 | st, repo, uid := newQueueTestRepo(t) | |
| 194 | // No RepoDir created on disk at all: Reachable will fail to even stat | |
| 195 | // the repository, which must not be read as "orphaned". | |
| 196 | root := t.TempDir() | |
| 197 | id, err := st.CreateBuild(repo.ID, "unit", strings.Repeat("a", 40), "main", "[]", true) | |
| 198 | if err != nil { | |
| 199 | t.Fatal(err) | |
| 200 | } | |
| 201 | ||
| 202 | c, out := runnerCtx(st, uid, root) | |
| 203 | code := runRunnerNext(c, nil) | |
| 204 | if code != protocol.ExitOK { | |
| 205 | t.Fatalf("runner next: exit %d, output:\n%s", code, out.String()) | |
| 206 | } | |
| 207 | if strings.Contains(out.String(), "no pending builds") { | |
| 208 | t.Fatalf("a build was not handed out when reachability could not be checked:\n%s", out.String()) | |
| 209 | } | |
| 210 | b, err := st.BuildByNumber(repo.ID, id) | |
| 211 | if err != nil { | |
| 212 | t.Fatal(err) | |
| 213 | } | |
| 214 | if b.Status != "running" { | |
| 215 | t.Fatalf("build status = %q, want running: an unchecked build must still be claimable", b.Status) | |
| 216 | } | |
| 217 | } | |
internal/gitutil/endofoptions_test.go +1
| @@ -32,6 +32,7 @@ func TestRefsAreNotOptions(t *testing.T) { | ||
| 32 | 32 | "ListTree": func() error { _, err := ListTree(dir, ref, ""); return err }, |
| 33 | 33 | "ReadBlob": func() error { _, err := ReadBlob(dir, ref, "f.txt", 1<<20); return err }, |
| 34 | 34 | "ResolveRef": func() error { _, err := ResolveRef(dir, ref); return err }, |
| 35 | "Reachable": func() error { _, err := Reachable(dir, ref); return err }, | |
| 35 | 36 | "Archive": func() error { return Archive(dir, ref, "x", &sink) }, |
| 36 | 37 | "Grep": func() error { _, err := Grep(dir, ref, "hi", 10); return err }, |
| 37 | 38 | "MergeBase": func() error { _, err := MergeBase(dir, ref, "main"); return err }, |
internal/gitutil/gitutil.go +49
| @@ -70,6 +70,55 @@ func IsAncestor(dir, old, new string) (bool, error) { | ||
| 70 | 70 | return false, err |
| 71 | 71 | } |
| 72 | 72 | |
| 73 | // Reachable reports whether sha is reachable from some ref in the | |
| 74 | // repository at dir — the condition a clone must find true to have any | |
| 75 | // chance of checking it out. That includes refs outside refs/heads and | |
| 76 | // refs/tags: a merge request head fetched from a fork lives at | |
| 77 | // refs/merge-requests/<n>/head, and is exactly as fetchable as a branch | |
| 78 | // tip, so it must count as reachable too — a first pass of this function | |
| 79 | // restricted the check to branches and tags and read every fork MR's | |
| 80 | // queued build as unreachable, cancelling it. false comes from two | |
| 81 | // shapes, and Reachable does not need to tell them apart: the object is | |
| 82 | // already gone (pruned), or it is still in the object store but nothing | |
| 83 | // points at it any more (a force-push moved the branch, gc has not run | |
| 84 | // yet). Either way the answer is the same: no ref reaches it. | |
| 85 | // | |
| 86 | // A non-nil error means the check itself did not run to a clean answer — | |
| 87 | // missing repository, git failing for its own reasons — and false is | |
| 88 | // meaningless in that case. The caller must not read err as "unreachable": | |
| 89 | // that would cancel a build the check never actually looked at. | |
| 90 | func Reachable(dir, sha string) (bool, error) { | |
| 91 | if _, err := os.Stat(dir); err != nil { | |
| 92 | return false, fmt.Errorf("reachable %s: %w", sha, err) | |
| 93 | } | |
| 94 | // cat-file -e <object>, unpeeled, is git's own existence predicate with | |
| 95 | // a documented exit code: 1 means the object is not there, the same | |
| 96 | // contract IsAncestor above already trusts from merge-base | |
| 97 | // --is-ancestor. Peeling to ^{commit} breaks that contract: a missing | |
| 98 | // object then exits 128, indistinguishable from "not a git repository" | |
| 99 | // or a corrupted one — the ambiguous case that must never read as | |
| 100 | // "unreachable" and cancel a build the check never actually looked at. | |
| 101 | err := exec.Command(toolpath.Look("git"), "-C", dir, "cat-file", "-e", "--end-of-options", sha).Run() | |
| 102 | if err != nil { | |
| 103 | if ee, ok := err.(*exec.ExitError); ok && ee.ExitCode() == 1 { | |
| 104 | return false, nil | |
| 105 | } | |
| 106 | return false, fmt.Errorf("cat-file -e %s: %w", sha, err) | |
| 107 | } | |
| 108 | // The object exists; --contains lists every ref whose history includes | |
| 109 | // it, with no namespace restriction — a branch, a tag, or a | |
| 110 | // refs/merge-requests/<n>/head are equally "a ref reaches this", and | |
| 111 | // that is the actual question, not whether it happens to be a branch | |
| 112 | // or a tag. Empty output with no error is the force-push case: present | |
| 113 | // in the object store, reachable from nothing. | |
| 114 | out, err := exec.Command(toolpath.Look("git"), "-C", dir, "for-each-ref", | |
| 115 | "--count=1", "--format=x", "--contains="+sha).Output() | |
| 116 | if err != nil { | |
| 117 | return false, fmt.Errorf("for-each-ref --contains %s: %w", sha, err) | |
| 118 | } | |
| 119 | return len(out) > 0, nil | |
| 120 | } | |
| 121 | ||
| 73 | 122 | // ZeroSHA reports whether s is an all-zero object id (SHA-1 or SHA-256). |
| 74 | 123 | func ZeroSHA(s string) bool { |
| 75 | 124 | if len(s) != 40 && len(s) != 64 { |
internal/gitutil/reachable_test.go added +143
| @@ -0,0 +1,143 @@ | ||
| 1 | package gitutil | |
| 2 | ||
| 3 | import ( | |
| 4 | "path/filepath" | |
| 5 | "strings" | |
| 6 | "testing" | |
| 7 | ) | |
| 8 | ||
| 9 | func mustResolve(t *testing.T, dir, ref string) string { | |
| 10 | t.Helper() | |
| 11 | sha, err := ResolveRef(dir, ref) | |
| 12 | if err != nil { | |
| 13 | t.Fatalf("resolving %s: %v", ref, err) | |
| 14 | } | |
| 15 | return sha | |
| 16 | } | |
| 17 | ||
| 18 | // A commit still an ancestor of a branch, or a branch tip itself, is | |
| 19 | // reachable — the ordinary case a claimed build's sha is in. | |
| 20 | func TestReachableAncestorAndTip(t *testing.T) { | |
| 21 | dir := t.TempDir() | |
| 22 | git(t, dir, "init", "-q", "-b", "main") | |
| 23 | write(t, dir, "f.txt", "one\n") | |
| 24 | git(t, dir, "add", ".") | |
| 25 | git(t, dir, "commit", "-q", "-m", "one") | |
| 26 | first := mustResolve(t, dir, "HEAD") | |
| 27 | write(t, dir, "f.txt", "two\n") | |
| 28 | git(t, dir, "add", ".") | |
| 29 | git(t, dir, "commit", "-q", "-m", "two") | |
| 30 | tip := mustResolve(t, dir, "HEAD") | |
| 31 | ||
| 32 | if ok, err := Reachable(dir, first); err != nil || !ok { | |
| 33 | t.Fatalf("ancestor: ok=%v err=%v", ok, err) | |
| 34 | } | |
| 35 | if ok, err := Reachable(dir, tip); err != nil || !ok { | |
| 36 | t.Fatalf("tip: ok=%v err=%v", ok, err) | |
| 37 | } | |
| 38 | } | |
| 39 | ||
| 40 | // A force-push moves a branch off a commit, and the old commit's object | |
| 41 | // sticks around until the next gc: exactly what a rebase-and-force-push | |
| 42 | // leaves a queued build pointed at. Reachable must say false here, not | |
| 43 | // error — a git error would let the caller read it as "cannot tell" and | |
| 44 | // hand the build to a runner that fails at clone. | |
| 45 | func TestReachableOrphanedByForcePushNotYetPruned(t *testing.T) { | |
| 46 | dir := t.TempDir() | |
| 47 | git(t, dir, "init", "-q", "-b", "main") | |
| 48 | write(t, dir, "f.txt", "one\n") | |
| 49 | git(t, dir, "add", ".") | |
| 50 | git(t, dir, "commit", "-q", "-m", "one") | |
| 51 | base := mustResolve(t, dir, "HEAD") | |
| 52 | write(t, dir, "f.txt", "two\n") | |
| 53 | git(t, dir, "add", ".") | |
| 54 | git(t, dir, "commit", "-q", "-m", "orphaned") | |
| 55 | orphaned := mustResolve(t, dir, "HEAD") | |
| 56 | ||
| 57 | git(t, dir, "update-ref", "refs/heads/main", base) | |
| 58 | ||
| 59 | if ok, err := Reachable(dir, orphaned); err != nil || ok { | |
| 60 | t.Fatalf("orphaned, object still present: ok=%v err=%v", ok, err) | |
| 61 | } | |
| 62 | // The rewound branch itself is untouched. | |
| 63 | if ok, err := Reachable(dir, base); err != nil || !ok { | |
| 64 | t.Fatalf("base after rewind: ok=%v err=%v", ok, err) | |
| 65 | } | |
| 66 | } | |
| 67 | ||
| 68 | // Once gc has actually removed the object, the object no longer exists at | |
| 69 | // all. Still false, still no error: pruned is a stronger form of orphaned, | |
| 70 | // not a different outcome. | |
| 71 | func TestReachablePrunedObject(t *testing.T) { | |
| 72 | dir := t.TempDir() | |
| 73 | git(t, dir, "init", "-q", "-b", "main") | |
| 74 | write(t, dir, "f.txt", "one\n") | |
| 75 | git(t, dir, "add", ".") | |
| 76 | git(t, dir, "commit", "-q", "-m", "one") | |
| 77 | base := mustResolve(t, dir, "HEAD") | |
| 78 | write(t, dir, "f.txt", "two\n") | |
| 79 | git(t, dir, "add", ".") | |
| 80 | git(t, dir, "commit", "-q", "-m", "orphaned") | |
| 81 | orphaned := mustResolve(t, dir, "HEAD") | |
| 82 | ||
| 83 | git(t, dir, "update-ref", "refs/heads/main", base) | |
| 84 | git(t, dir, "reflog", "expire", "--expire=now", "--all") | |
| 85 | git(t, dir, "gc", "--prune=now", "-q") | |
| 86 | ||
| 87 | if ok, err := Reachable(dir, orphaned); err != nil || ok { | |
| 88 | t.Fatalf("pruned: ok=%v err=%v", ok, err) | |
| 89 | } | |
| 90 | } | |
| 91 | ||
| 92 | // A check that cannot run at all — no repository at the path — must not | |
| 93 | // come back as "unreachable": that would read as a real answer and cancel | |
| 94 | // a build that was never actually checked. | |
| 95 | func TestReachableErrorsRatherThanFalseWhenItCannotCheck(t *testing.T) { | |
| 96 | dir := filepath.Join(t.TempDir(), "no-such-repo") | |
| 97 | if ok, err := Reachable(dir, strings.Repeat("a", 40)); err == nil { | |
| 98 | t.Fatalf("expected an error for a missing repository, got ok=%v", ok) | |
| 99 | } | |
| 100 | } | |
| 101 | ||
| 102 | // A directory that exists but holds no git repository at all — corrupted, | |
| 103 | // mid-restore from a backup, or simply never initialized — must error the | |
| 104 | // same way: cat-file -e on a well-formed sha exits 1 only when the object | |
| 105 | // is genuinely absent from a real repository. Outside a repository it | |
| 106 | // exits 128, the same code a peeled ^{commit} lookup uses for a missing | |
| 107 | // object, so collapsing "any exit" to false would read a broken | |
| 108 | // repository as an orphaned queue and cancel every build in it. | |
| 109 | func TestReachableErrorsWhenDirectoryIsNotAGitRepository(t *testing.T) { | |
| 110 | dir := t.TempDir() // exists, but no `git init` ever ran here | |
| 111 | if ok, err := Reachable(dir, strings.Repeat("a", 40)); err == nil { | |
| 112 | t.Fatalf("expected an error for a non-repository directory, got ok=%v", ok) | |
| 113 | } | |
| 114 | } | |
| 115 | ||
| 116 | // A merge request head fetched from a fork lives at | |
| 117 | // refs/merge-requests/<n>/head — no branch and no tag ever points at it. | |
| 118 | // It must still read as reachable: it is fetched into the target | |
| 119 | // repository and a runner's clone reaches it exactly like a branch tip | |
| 120 | // does. Restricting the ancestry check to refs/heads and refs/tags (a | |
| 121 | // first pass of Reachable did this) would read every such build as | |
| 122 | // unreachable and cancel it, which is worse than the bug this whole | |
| 123 | // change fixes — CI silently stops running on every fork merge request. | |
| 124 | func TestReachableFromMergeRequestHeadRef(t *testing.T) { | |
| 125 | dir := t.TempDir() | |
| 126 | git(t, dir, "init", "-q", "-b", "main") | |
| 127 | write(t, dir, "f.txt", "one\n") | |
| 128 | git(t, dir, "add", ".") | |
| 129 | git(t, dir, "commit", "-q", "-m", "base") | |
| 130 | ||
| 131 | git(t, dir, "checkout", "-q", "-b", "fork-head") | |
| 132 | write(t, dir, "f.txt", "two\n") | |
| 133 | git(t, dir, "add", ".") | |
| 134 | git(t, dir, "commit", "-q", "-m", "mr head") | |
| 135 | mrSHA := mustResolve(t, dir, "HEAD") | |
| 136 | git(t, dir, "update-ref", "refs/merge-requests/1/head", mrSHA) | |
| 137 | git(t, dir, "checkout", "-q", "main") | |
| 138 | git(t, dir, "branch", "-D", "fork-head") | |
| 139 | ||
| 140 | if ok, err := Reachable(dir, mrSHA); err != nil || !ok { | |
| 141 | t.Fatalf("mr head reachable only via refs/merge-requests: ok=%v err=%v", ok, err) | |
| 142 | } | |
| 143 | } | |