Public: anyone can read this; the crew can change it.
A fork's home reads its Following card under 5 ms when neither tip has moved #6137
Check · Review · Land · Live done — Landed and live in production.
scout/forge:scout/6127-a-fork-s-home-reads-its-following-card-u → main 3 commits fork 1 thread · 1 unresolved
3 files
changed +223
−14
src/core/following.ts +79 −14
| @@ -23,7 +23,6 @@ import type { EventBus, ForgeEvent } from "./events.ts"; | ||
| 23 | 23 | import { getPull, listOpenPullsFromHead, type Pull } from "./pulls.ts"; |
| 24 | 24 | import { getRepoById, listForks, type Repo } from "./repos.ts"; |
| 25 | 25 | import { |
| 26 | aheadCount, | |
| 27 | 26 | fetchInto, |
| 28 | 27 | merge, |
| 29 | 28 | mergeBase, |
| @@ -32,6 +31,8 @@ import { | ||
| 32 | 31 | writeRef, |
| 33 | 32 | type MergeIdentity, |
| 34 | 33 | } from "../git/merge.ts"; |
| 34 | import { runGit } from "../git/env.ts"; | |
| 35 | import { readTip } from "../git/tip.ts"; | |
| 35 | 36 | import { repoDir } from "../git/repos.ts"; |
| 36 | 37 | import { createLog } from "../kernel/log.ts"; |
| 37 | 38 | import { err, ok, type Result } from "../kernel/result.ts"; |
| @@ -128,8 +129,9 @@ export function follow(forge: Following): () => void { | ||
| 128 | 129 | * nobody has touched; `behind` has nothing of its own and can simply move; |
| 129 | 130 | * `ahead` has its own work and the parent has not moved; `diverged` has |
| 130 | 131 | * both, and git says whether they still merge cleanly and, if not, which |
| 131 | * files need a decision. Every number is read from git each time — the | |
| 132 | * database never caches a relationship it could get wrong. | |
| 132 | * files need a decision. The tips are read from the ref files each time and | |
| 133 | * the comparison is memoized on that pair of tips, so a relationship is | |
| 134 | * recomputed only when either side moves; the database never holds it. | |
| 133 | 135 | */ |
| 134 | 136 | export type RelationState = "synced" | "behind" | "ahead" | "diverged"; |
| 135 | 137 | |
| @@ -162,8 +164,8 @@ export async function forkRelation( | ||
| 162 | 164 | const parentDir = repoDir(forge.reposRoot, p.ownerName, p.name); |
| 163 | 165 | const forkDir = repoDir(forge.reposRoot, fork.ownerName, fork.name); |
| 164 | 166 | const [parentTip, forkTip] = await Promise.all([ |
| 165 | resolveOid(parentDir, `refs/heads/${p.defaultBranch}`), | |
| 166 | resolveOid(forkDir, `refs/heads/${fork.defaultBranch}`), | |
| 167 | tipOf(parentDir, `refs/heads/${p.defaultBranch}`), | |
| 168 | tipOf(forkDir, `refs/heads/${fork.defaultBranch}`), | |
| 167 | 169 | ]); |
| 168 | 170 | const at = ( |
| 169 | 171 | state: RelationState, |
| @@ -184,24 +186,87 @@ export async function forkRelation( | ||
| 184 | 186 | // An empty side has nothing to follow or to differ by. |
| 185 | 187 | if (parentTip === undefined || forkTip === undefined || parentTip === forkTip) |
| 186 | 188 | return at("synced", 0, 0); |
| 189 | // The answer is a function of the two tips: git is asked again only when | |
| 190 | // either moves (#6127). | |
| 191 | const key = `${forkDir}\0${parentDir}\0${forkTip}\0${parentTip}`; | |
| 192 | const known = relationCache.get(key); | |
| 193 | if (known !== undefined) return at(...known); | |
| 194 | const { standing, sure } = await compare(forkDir, forkTip, parentTip); | |
| 195 | // Only what git actually answered is kept: a timeout or a spawn failure | |
| 196 | // is wrong for this read, not for as long as the tips stand. | |
| 197 | if (sure) { | |
| 198 | if (relationCache.size >= RELATION_CACHE_MAX) relationCache.clear(); | |
| 199 | relationCache.set(key, standing); | |
| 200 | } | |
| 201 | return at(...standing); | |
| 202 | } | |
| 203 | ||
| 204 | /** The tip from the ref files; git is asked only when they cannot say. */ | |
| 205 | async function tipOf(dir: string, ref: string): Promise<string | undefined> { | |
| 206 | return (await readTip(dir, ref)) ?? (await resolveOid(dir, ref)); | |
| 207 | } | |
| 208 | ||
| 209 | type Standing = [ | |
| 210 | RelationState, | |
| 211 | behind: number, | |
| 212 | ahead: number, | |
| 213 | clean: boolean, | |
| 214 | conflicts: readonly string[], | |
| 215 | ]; | |
| 216 | ||
| 217 | const RELATION_CACHE_MAX = 500; | |
| 218 | const relationCache = new Map<string, Standing>(); | |
| 219 | ||
| 220 | /** How many times git was asked to compare two tips (for tests). */ | |
| 221 | export const comparisons = { count: 0 }; | |
| 222 | ||
| 223 | async function compare( | |
| 224 | forkDir: string, | |
| 225 | forkTip: string, | |
| 226 | parentTip: string, | |
| 227 | ): Promise<{ standing: Standing; sure: boolean }> { | |
| 228 | comparisons.count++; | |
| 229 | const at = ( | |
| 230 | state: RelationState, | |
| 231 | behind: number, | |
| 232 | ahead: number, | |
| 233 | clean = true, | |
| 234 | conflicts: readonly string[] = [], | |
| 235 | sure = true, | |
| 236 | ) => ({ | |
| 237 | standing: [state, behind, ahead, clean, conflicts] as Standing, | |
| 238 | sure, | |
| 239 | }); | |
| 187 | 240 | // The parent's objects reach the fork through its alternates, so every |
| 188 | 241 | // question here is answered inside the fork without a fetch. |
| 189 | const base = await mergeBase(forkDir, forkTip, parentTip); | |
| 190 | if (base === undefined) return at("diverged", 0, 0, false); | |
| 191 | const [behind, ahead] = await Promise.all([ | |
| 192 | aheadCount(forkDir, forkTip, parentTip), | |
| 193 | aheadCount(forkDir, parentTip, forkTip), | |
| 242 | const base = await runGit(["merge-base", forkTip, parentTip], { cwd: forkDir }); | |
| 243 | if (!base.ok) { | |
| 244 | // Exit 1 with nothing said is git's answer for histories with no common | |
| 245 | // ancestor; any other failure is not an answer. | |
| 246 | const unrelated = base.error.code === 1 && base.error.stderr === ""; | |
| 247 | return at("diverged", 0, 0, false, [], unrelated); | |
| 248 | } | |
| 249 | const count = (from: string, to: string) => | |
| 250 | runGit(["rev-list", "--count", `${from}..${to}`], { cwd: forkDir }); | |
| 251 | const [b, a] = await Promise.all([ | |
| 252 | count(forkTip, parentTip), | |
| 253 | count(parentTip, forkTip), | |
| 194 | 254 | ]); |
| 195 | if (ahead === 0) return at("behind", behind, 0); | |
| 196 | if (behind === 0) return at("ahead", 0, ahead); | |
| 255 | const sure = b.ok && a.ok; | |
| 256 | const behind = b.ok ? Number(b.value) || 0 : 0; | |
| 257 | const ahead = a.ok ? Number(a.value) || 0 : 0; | |
| 258 | if (ahead === 0) return at("behind", behind, 0, true, [], sure); | |
| 259 | if (behind === 0) return at("ahead", 0, ahead, true, [], sure); | |
| 197 | 260 | const merged = await mergeable(forkDir, forkTip, parentTip); |
| 198 | if (merged.ok) return at("diverged", behind, ahead, true); | |
| 261 | if (merged.ok) return at("diverged", behind, ahead, true, [], sure); | |
| 262 | const conflict = merged.error.kind === "conflict"; | |
| 199 | 263 | return at( |
| 200 | 264 | "diverged", |
| 201 | 265 | behind, |
| 202 | 266 | ahead, |
| 203 | 267 | false, |
| 204 | merged.error.kind === "conflict" ? merged.error.files : [], | |
| 268 | conflict ? merged.error.files : [], | |
| 269 | sure && conflict, | |
| 205 | 270 | ); |
| 206 | 271 | } |
| 207 | 272 |
src/git/tip.ts added +33
| @@ -0,0 +1,33 @@ | ||
| 1 | /** | |
| 2 | * A branch tip read from the repository's own files, without a process: | |
| 3 | * the loose ref, else the line in packed-refs. Anything else (a symbolic | |
| 4 | * ref, a missing file, a surprise) is `undefined`, and the caller asks git. | |
| 5 | */ | |
| 6 | ||
| 7 | import { readFile } from "node:fs/promises"; | |
| 8 | import { join } from "node:path"; | |
| 9 | import { isSafeOid, isSafeRef } from "./guard.ts"; | |
| 10 | ||
| 11 | async function text(path: string): Promise<string | undefined> { | |
| 12 | try { | |
| 13 | return await readFile(path, "utf8"); | |
| 14 | } catch { | |
| 15 | return undefined; | |
| 16 | } | |
| 17 | } | |
| 18 | ||
| 19 | export async function readTip( | |
| 20 | dir: string, | |
| 21 | ref: string, | |
| 22 | ): Promise<string | undefined> { | |
| 23 | if (!isSafeRef(ref) || !ref.startsWith("refs/heads/")) return undefined; | |
| 24 | const loose = (await text(join(dir, ref)))?.trim(); | |
| 25 | if (loose !== undefined) return isSafeOid(loose) ? loose : undefined; | |
| 26 | const packed = await text(join(dir, "packed-refs")); | |
| 27 | if (packed === undefined) return undefined; | |
| 28 | for (const line of packed.split("\n")) { | |
| 29 | const [oid, name] = line.split(" "); | |
| 30 | if (name === ref && oid !== undefined && isSafeOid(oid)) return oid; | |
| 31 | } | |
| 32 | return undefined; | |
| 33 | } |
test/following-cache.test.ts added +111
| @@ -0,0 +1,111 @@ | ||
| 1 | /** | |
| 2 | * A fork's Following card is a function of the two default tips (#6127): | |
| 3 | * warm reads do not ask git again, and a push to the parent changes the | |
| 4 | * next read. | |
| 5 | */ | |
| 6 | import { afterAll, beforeAll, expect, test } from "bun:test"; | |
| 7 | import { mkdtemp, rm } from "node:fs/promises"; | |
| 8 | import { tmpdir } from "node:os"; | |
| 9 | import { join } from "node:path"; | |
| 10 | import { commitToBranch, createRepo, forkRepo } from "../src/actions/repos.ts"; | |
| 11 | import { relationOf } from "../src/actions/forks.ts"; | |
| 12 | import { createApp, type App } from "../src/app.ts"; | |
| 13 | import { comparisons } from "../src/core/following.ts"; | |
| 14 | import { getRepo } from "../src/core/repos.ts"; | |
| 15 | import { getUserByName } from "../src/core/users.ts"; | |
| 16 | import { createUser } from "../src/core/users.ts"; | |
| 17 | import { promote } from "./helpers/members.ts"; | |
| 18 | ||
| 19 | let dataDir: string; | |
| 20 | let app: App; | |
| 21 | ||
| 22 | beforeAll(async () => { | |
| 23 | dataDir = await mkdtemp(join(tmpdir(), "forge-following-cache-")); | |
| 24 | app = createApp({ dataDir, port: 0 }); | |
| 25 | }, 60_000); | |
| 26 | ||
| 27 | afterAll(async () => { | |
| 28 | app.close(); | |
| 29 | await rm(dataDir, { recursive: true, force: true }); | |
| 30 | }); | |
| 31 | ||
| 32 | test("warm reads do not compare again and a push to the parent changes the next read", async () => { | |
| 33 | const mk = async (username: string, first = false) => { | |
| 34 | const u = await createUser(app.db, { | |
| 35 | origin: "seed", | |
| 36 | trusted: true, | |
| 37 | username, | |
| 38 | email: `${username}@forge.local`, | |
| 39 | password: "a-good-passphrase", | |
| 40 | adminIfFirst: first, | |
| 41 | }); | |
| 42 | if (!u.ok) throw new Error(u.error); | |
| 43 | return u.value; | |
| 44 | }; | |
| 45 | const alice = await mk("alice", true); | |
| 46 | const bob = await mk("bob"); | |
| 47 | promote(app.db, bob); | |
| 48 | const lib = await createRepo(app, alice, { name: "lib", visibility: "public" }); | |
| 49 | if (!lib.ok) throw new Error(String(lib.error)); | |
| 50 | const commit = async (path: string) => { | |
| 51 | const r = await commitToBranch(app, alice, getRepo(app.db, "alice", "lib")!, { | |
| 52 | message: `Add ${path}`, | |
| 53 | files: [{ path, content: `${path}\n` }], | |
| 54 | }); | |
| 55 | if (!r.ok) throw new Error(JSON.stringify(r.error)); | |
| 56 | }; | |
| 57 | await commit("a.txt"); | |
| 58 | const forked = await forkRepo(app, bob, lib.value); | |
| 59 | if (!forked.ok) throw new Error(String(forked.error)); | |
| 60 | const fork = getRepo(app.db, "bob", "lib")!; | |
| 61 | ||
| 62 | // Behind-only needs a git comparison; the first read pays for it. | |
| 63 | await commit("b.txt"); | |
| 64 | const cold = await relationOf(app, bob, fork); | |
| 65 | expect(cold?.state).toBe("behind"); | |
| 66 | expect(cold?.behind).toBe(1); | |
| 67 | ||
| 68 | const before = comparisons.count; | |
| 69 | const warm = await relationOf(app, bob, fork); | |
| 70 | expect(warm?.state).toBe("behind"); | |
| 71 | expect(comparisons.count).toBe(before); | |
| 72 | ||
| 73 | await commit("c.txt"); | |
| 74 | const moved = await relationOf(app, bob, fork); | |
| 75 | expect(moved?.parentTip).not.toBe(warm?.parentTip); | |
| 76 | expect(moved?.state).toBe("behind"); | |
| 77 | expect(comparisons.count).toBe(before + 1); | |
| 78 | }, 60_000); | |
| 79 | ||
| 80 | test("readTip reads loose and packed refs without git, and declines the rest", async () => { | |
| 81 | const { readTip } = await import("../src/git/tip.ts"); | |
| 82 | const { mkdir, writeFile } = await import("node:fs/promises"); | |
| 83 | const dir = join(dataDir, "bare"); | |
| 84 | const a = "a".repeat(40); | |
| 85 | const b = "b".repeat(40); | |
| 86 | await mkdir(join(dir, "refs/heads"), { recursive: true }); | |
| 87 | await writeFile(join(dir, "refs/heads/main"), `${a}\n`); | |
| 88 | await writeFile(join(dir, "packed-refs"), `# pack-refs\n${b} refs/heads/old\n^${a}\n`); | |
| 89 | expect(await readTip(dir, "refs/heads/main")).toBe(a); | |
| 90 | expect(await readTip(dir, "refs/heads/old")).toBe(b); | |
| 91 | expect(await readTip(dir, "refs/heads/none")).toBeUndefined(); | |
| 92 | expect(await readTip(dir, "refs/tags/x")).toBeUndefined(); | |
| 93 | await writeFile(join(dir, "refs/heads/sym"), "ref: refs/heads/main\n"); | |
| 94 | expect(await readTip(dir, "refs/heads/sym")).toBeUndefined(); | |
| 95 | }); | |
| 96 | ||
| 97 | test("an answer git failed to give is not remembered", async () => { | |
| 98 | const { writeFile } = await import("node:fs/promises"); | |
| 99 | const { repoDir } = await import("../src/git/repos.ts"); | |
| 100 | const fork = getRepo(app.db, "bob", "lib")!; | |
| 101 | const bob = getUserByName(app.db, "bob")!; | |
| 102 | // A tip whose objects are missing: git cannot compare it with anything. | |
| 103 | await writeFile( | |
| 104 | join(repoDir(app.reposRoot, "bob", "lib"), `refs/heads/${fork.defaultBranch}`), | |
| 105 | `${"c".repeat(40)}\n`, | |
| 106 | ); | |
| 107 | const before = comparisons.count; | |
| 108 | await relationOf(app, bob, fork); | |
| 109 | await relationOf(app, bob, fork); | |
| 110 | expect(comparisons.count).toBe(before + 2); | |
| 111 | }); |
new above
offline
High: every result of
compareis stored here, including the ones the git helpers made up after a failure (mergeBase→ undefined gives diverged/unclean,aheadCount→ 0,mergeablegit error gives unclean with no conflicts). They stay until a tip moves, andtakeUpdatesreads them too. Store only answers git actually returned.