Public: anyone can read this; the crew can change it.
/changelog asks git about each merged pull once per running commit: a pull that is not live yet costs no git on the next read #8397
Check · Review · Land · Live done — Landed and live in production.
scout/forge:scout/8380-changelog-asks-git-about-each-merged-pul → main 1 commit fork
2 files
changed +107
−6
src/release/landings.ts +19 −6
| @@ -70,6 +70,8 @@ interface LiveState { | ||
| 70 | 70 | readonly own: string; |
| 71 | 71 | /** Merge commits known to be in the running commit. */ |
| 72 | 72 | readonly oids: ReadonlySet<string>; |
| 73 | /** Merge commits found not to be in the running commit; they cannot become so while it stays. */ | |
| 74 | readonly misses: ReadonlySet<string>; | |
| 73 | 75 | /** When the running commit was made; stands in for a merge commit the pull never recorded. */ |
| 74 | 76 | readonly at: number; |
| 75 | 77 | /** The running commit these were asked of. */ |
| @@ -79,7 +81,7 @@ let kept: LiveState | undefined; | ||
| 79 | 81 | |
| 80 | 82 | /** Keep what the running commit contains; a test says it directly. */ |
| 81 | 83 | export function keepLive(own: string, oids: Iterable<string>, at: number): void { |
| 82 | kept = { own, oids: new Set(oids), at, commit: "" }; | |
| 84 | kept = { own, oids: new Set(oids), misses: new Set(), at, commit: "" }; | |
| 83 | 85 | } |
| 84 | 86 | |
| 85 | 87 | /** Forget it, so the alarm is silent until the next read. */ |
| @@ -166,22 +168,33 @@ async function askLive(app: App, rows: readonly MergedRow[]): Promise<void> { | ||
| 166 | 168 | const head = running().commit ?? (await runGit(["rev-parse", "--verify", "-q", `refs/heads/${repo.defaultBranch}`], { cwd: dir })); |
| 167 | 169 | const commit = typeof head === "string" ? head : head.ok ? head.value : undefined; |
| 168 | 170 | if (commit === undefined) return; |
| 169 | const made = await runGit(["show", "-s", "--format=%ct", commit], { cwd: dir }); | |
| 170 | if (!made.ok) return; | |
| 171 | 171 | const named = `${repo.ownerName}/${repo.name}`; |
| 172 | // A live commit's ancestors stay ancestors; only a new running commit voids what was kept. | |
| 172 | // For one running commit an ancestor stays one and a non-ancestor never becomes one: | |
| 173 | // only a new running commit voids what was kept. | |
| 173 | 174 | const before = kept?.own === named && kept.commit === commit ? kept : undefined; |
| 175 | let at = before?.at; | |
| 176 | if (at === undefined) { | |
| 177 | const made = await runGit(["show", "-s", "--format=%ct", commit], { cwd: dir }); | |
| 178 | if (!made.ok) return; | |
| 179 | at = Number(made.value) * 1000; | |
| 180 | } | |
| 174 | 181 | const oids = new Set(before?.oids ?? []); |
| 182 | const misses = new Set(before?.misses ?? []); | |
| 175 | 183 | const wanted = rows |
| 176 | .filter((r) => r.repo.toLowerCase() === named.toLowerCase() && r.oid !== "" && !oids.has(r.oid)) | |
| 184 | .filter((r) => r.repo.toLowerCase() === named.toLowerCase() && r.oid !== "" && !oids.has(r.oid) && !misses.has(r.oid)) | |
| 177 | 185 | .slice(0, ASKED); |
| 178 | 186 | await Promise.all( |
| 179 | 187 | wanted.map(async (r) => { |
| 180 | 188 | const ancestor = await runGit(["merge-base", "--is-ancestor", r.oid, commit], { cwd: dir }); |
| 181 | 189 | if (ancestor.ok) oids.add(r.oid); |
| 190 | // Exit 1 is git's "no"; any other failure may pass, so it is asked again. | |
| 191 | else if (ancestor.error.code === 1) misses.add(r.oid); | |
| 182 | 192 | }), |
| 183 | 193 | ); |
| 184 | kept = { own: named, oids, at: Number(made.value) * 1000, commit }; | |
| 194 | // Misses are kept only for the merged rows still listed. | |
| 195 | const listed = new Set(rows.map((r) => r.oid)); | |
| 196 | for (const o of misses) if (!listed.has(o)) misses.delete(o); | |
| 197 | kept = { own: named, oids, misses, at, commit }; | |
| 185 | 198 | } |
| 186 | 199 | |
| 187 | 200 | /** Every landing, live or on its way, as of the running commit. */ |
test/landings-asked-once.test.ts added +88
| @@ -0,0 +1,88 @@ | ||
| 1 | import { afterEach, describe, expect, test } from "bun:test"; | |
| 2 | import { chmodSync, mkdirSync, mkdtempSync, readFileSync, rmSync, writeFileSync } from "node:fs"; | |
| 3 | import { tmpdir } from "node:os"; | |
| 4 | import { join } from "node:path"; | |
| 5 | import { createRepo } from "../src/core/repos.ts"; | |
| 6 | import { createApp } from "../src/app.ts"; | |
| 7 | import { createUser } from "../src/core/users.ts"; | |
| 8 | import { repoDir } from "../src/git/repos.ts"; | |
| 9 | import { id } from "../src/kernel/id.ts"; | |
| 10 | import { forgetLive, readLandings } from "../src/release/landings.ts"; | |
| 11 | ||
| 12 | const dirs: string[] = []; | |
| 13 | const saved = { path: process.env.PATH, commit: process.env.FORGE_COMMIT }; | |
| 14 | afterEach(() => { | |
| 15 | process.env.PATH = saved.path; | |
| 16 | if (saved.commit === undefined) delete process.env.FORGE_COMMIT; | |
| 17 | else process.env.FORGE_COMMIT = saved.commit; | |
| 18 | forgetLive(); | |
| 19 | for (const d of dirs.splice(0)) rmSync(d, { recursive: true, force: true }); | |
| 20 | }); | |
| 21 | ||
| 22 | const TREE = "4b825dc642cb6eb9a060e54bf8d69288fbee4904"; | |
| 23 | function git(cwd: string, ...args: string[]): string { | |
| 24 | const p = Bun.spawnSync(["git", ...args], { | |
| 25 | cwd, | |
| 26 | env: { ...process.env, GIT_AUTHOR_NAME: "t", GIT_AUTHOR_EMAIL: "t@t", GIT_COMMITTER_NAME: "t", GIT_COMMITTER_EMAIL: "t@t" }, | |
| 27 | }); | |
| 28 | return p.stdout.toString().trim(); | |
| 29 | } | |
| 30 | ||
| 31 | describe("landings ask git once per merge commit", () => { | |
| 32 | test("a pull on its way costs no git on the next read; a new one is asked once", async () => { | |
| 33 | const dir = mkdtempSync(join(tmpdir(), "forge-asked-once-")); | |
| 34 | dirs.push(dir); | |
| 35 | const app = createApp({ dataDir: dir, port: 0 }); | |
| 36 | const u = await createUser(app.db, { username: "alice", email: "[email protected]", password: "a-good-passphrase", origin: "seed", trusted: true, adminIfFirst: true }); | |
| 37 | if (!u.ok) throw new Error(u.error); | |
| 38 | const repo = await createRepo(app.db, app.reposRoot, { ownerId: u.value.id, name: "r", visibility: "public", description: "r" }); | |
| 39 | if (!repo.ok) throw new Error(repo.error); | |
| 40 | const gitDir = repoDir(app.reposRoot, "alice", "r"); | |
| 41 | const a = git(gitDir, "commit-tree", TREE, "-m", "a"); | |
| 42 | const b = git(gitDir, "commit-tree", TREE, "-p", a, "-m", "b"); | |
| 43 | const c = git(gitDir, "commit-tree", TREE, "-m", "c"); | |
| 44 | const d = git(gitDir, "commit-tree", TREE, "-m", "d"); | |
| 45 | let n = 0; | |
| 46 | const merge = (oid: string) => { | |
| 47 | n++; | |
| 48 | const at = new Date(Date.now() - (10 - n) * 60_000).toISOString(); | |
| 49 | const issueId = id(); | |
| 50 | app.db.query(`INSERT INTO issue (id, repo_id, number, kind, title, body, author_id, state, created_at) VALUES (?1, ?2, ?3, 'pull', 'P', '', ?4, 'closed', ?5)`).run(issueId, repo.value.id, n, u.value.id, at); | |
| 51 | app.db.query(`INSERT INTO pull (issue_id, base_branch, head_branch, base_oid, head_oid, head_repo_id, merged_at, merged_oid) VALUES (?1, ?2, ?3, '', '', ?4, ?5, ?6)`).run(issueId, repo.value.defaultBranch, `b/${n}`, repo.value.id, at, oid); | |
| 52 | }; | |
| 53 | ||
| 54 | // A `git` that logs its arguments, then runs the real one. | |
| 55 | const bin = join(dir, "bin"); | |
| 56 | const log = join(dir, "git.log"); | |
| 57 | mkdirSync(bin); | |
| 58 | const real = Bun.which("git") ?? "/usr/bin/git"; | |
| 59 | writeFileSync(join(bin, "git"), `#!/bin/sh\necho "$@" >> ${log}\nexec ${real} "$@"\n`); | |
| 60 | chmodSync(join(bin, "git"), 0o755); | |
| 61 | process.env.PATH = `${bin}:${saved.path ?? ""}`; | |
| 62 | process.env.FORGE_COMMIT = b; | |
| 63 | (app.config as { forgeRepo?: string }).forgeRepo = "alice/r"; | |
| 64 | const asks = (oid: string) => readFileSync(log, "utf8").split("\n").filter((l) => l.startsWith("merge-base") && l.includes(oid)).length; | |
| 65 | const mergeBases = () => readFileSync(log, "utf8").split("\n").filter((l) => l.startsWith("merge-base")).length; | |
| 66 | ||
| 67 | merge(a); | |
| 68 | merge(c); // not in the running commit: on its way | |
| 69 | const first = await readLandings(app); | |
| 70 | expect(first.live.map((l) => l.number)).toEqual([1]); | |
| 71 | expect(first.onItsWay.map((l) => l.number)).toEqual([2]); | |
| 72 | expect(asks(a)).toBe(1); | |
| 73 | expect(asks(c)).toBe(1); | |
| 74 | ||
| 75 | const before = readFileSync(log, "utf8"); | |
| 76 | const second = await readLandings(app); | |
| 77 | expect(second.onItsWay.map((l) => l.number)).toEqual([2]); | |
| 78 | expect(readFileSync(log, "utf8")).toBe(before); // no git at all | |
| 79 | expect(mergeBases()).toBe(2); | |
| 80 | ||
| 81 | merge(d); | |
| 82 | const third = await readLandings(app); | |
| 83 | expect(third.onItsWay.map((l) => l.number).sort()).toEqual([2, 3]); | |
| 84 | expect(asks(d)).toBe(1); | |
| 85 | expect(asks(c)).toBe(1); | |
| 86 | expect(mergeBases()).toBe(3); | |
| 87 | }); | |
| 88 | }); |
new above
offline