From 5f8e7493c8ad0b2090448448ebcb8327a88aebbc Mon Sep 17 00:00:00 2001 From: ragnarok-clanker Date: Mon, 26 Oct 2026 19:31:30 +0100 Subject: [PATCH] Find header includers from the sources, not only from earlier compiles unit_deps holds only the units some merge compiled, so a change to a header no merge touched compiled none of its includers. Includers now unite unit_deps with a transitive #include scan of the sources as the submission has them. --- src/server/app.js | 2 +- src/server/includers.js | 58 +++++++++++++++++++++++++++++++++++++++++ src/server/verify.js | 23 +++++++++++----- test/server.test.js | 12 +++++++++ 4 files changed, 88 insertions(+), 7 deletions(-) create mode 100644 src/server/includers.js diff --git a/src/server/app.js b/src/server/app.js index 4bbb0b7..4212b21 100644 --- a/src/server/app.js +++ b/src/server/app.js @@ -341,7 +341,7 @@ export function createApp(ctx) { auth(req); const body = await readJson(req, 64 * 1024); const headers = (Array.isArray(body.headers) ? body.headers : []).slice(0, 200).map(String); - return { includers: Object.fromEntries(headers.map((h) => [h, store.all('SELECT unit FROM unit_deps WHERE header = ? ORDER BY unit', h).map((r) => r.unit)])) }; + return { includers: Object.fromEntries(headers.map((h) => [h, ctx.verifier.includersOf([h])])) }; }); router.on('POST', '/api/me/password', async ({ req }) => { diff --git a/src/server/includers.js b/src/server/includers.js new file mode 100644 index 0000000..2af322f --- /dev/null +++ b/src/server/includers.js @@ -0,0 +1,58 @@ +// Units that include a header, transitively, read from the sources themselves (#include "..."), +// not from what earlier compiles recorded: unit_deps holds only units some merge compiled, so a +// header nobody's merge touched yet shows no includers there. + +import { listSources } from '../shared/project.js'; + +const INCLUDE_RE = /^\s*#\s*include\s+"([^"]+)"/gm; + +/** + * @param {import('./git.js').Repo} repo + * @param {object} project sourceDirs, includeDirs, compile.units + * @param {string[]} headers repo paths of the changed headers + * @param {Record} files submitted contents (override what main has) + * @returns {string[]} units (compile.json .cpp files) that include any of `headers` + */ +export function sourceIncluders(repo, project, headers, files = {}) { + if (!headers.length) return []; + const read = (f) => files[f] ?? repo.show('HEAD', f); + const roots = [...project.sourceDirs, ...project.includeDirs].map((d) => d.replace(/\/+$/, '')); + const known = new Set([...listSources(repo.dir, project), ...Object.keys(files)]); + const resolve = (from, spec) => { + const dir = from.slice(0, from.lastIndexOf('/') + 1); + for (const base of [dir.replace(/\/$/, ''), ...roots]) { + const p = normalize(base ? `${base}/${spec}` : spec); + if (known.has(p)) return p; + } + return null; + }; + const edges = new Map(); + const includesOf = (f) => { + if (!edges.has(f)) { + const text = read(f) ?? ''; + edges.set(f, [...text.matchAll(INCLUDE_RE)].map((m) => resolve(f, m[1])).filter(Boolean)); + } + return edges.get(f); + }; + const want = new Set(headers); + const reaches = new Map(); + const hits = (f, seen = new Set()) => { + if (reaches.has(f)) return reaches.get(f); + if (seen.has(f)) return false; + seen.add(f); + const r = includesOf(f).some((h) => want.has(h) || hits(h, seen)); + reaches.set(f, r); + return r; + }; + const units = listSources(repo.dir, project, { unitsOnly: true }); + return units.filter((u) => hits(u)); +} + +function normalize(p) { + const out = []; + for (const part of p.split('/')) { + if (part === '..') out.pop(); + else if (part && part !== '.') out.push(part); + } + return out.join('/'); +} diff --git a/src/server/verify.js b/src/server/verify.js index f0a2b1a..1911a86 100644 --- a/src/server/verify.js +++ b/src/server/verify.js @@ -11,6 +11,7 @@ import { STATUS, sameOriginal } from '../shared/compare.js'; import { listSources, isUnit } from '../shared/project.js'; import { changedNames, unitsMentioning } from './affected.js'; import { undefinedDeclarations } from './undefined.js'; +import { sourceIncluders } from './includers.js'; import { functionRegions } from './split.js'; import { className } from './grouping.js'; import { now } from './db.js'; @@ -116,6 +117,17 @@ export class Verifier { } } + /** + * Units that include any of `headers`: what compiles recorded (unit_deps: the compiler's own + * include list), plus what the sources say (includers.js), which also covers units no merge has + * compiled yet. As `files` change them. + */ + includersOf(headers, files = {}) { + const out = new Set(sourceIncluders(this.repo, this.project, headers, files)); + for (const h of headers) for (const r of this.store.all('SELECT unit FROM unit_deps WHERE header = ?', h)) out.add(r.unit); + return [...out].sort(); + } + /** * Compiles `files` (already merged onto HEAD) plus every unit they can affect * and compares against the current target version. @@ -135,12 +147,11 @@ export class Verifier { const allProjectUnits = () => listSources(this.repo.dir, this.project, { unitsOnly: true }); if (allUnits) for (const u of allProjectUnits()) units.add(u); else if (changedHeaders.length && !onlyUnits) { - // Units that include a changed header; all of them until a rebuild has recorded dependencies. - // Without dependents, a header-only change compiles one includer per header. - const known = this.store.get('SELECT COUNT(*) AS n FROM unit_deps').n > 0; + // Units that include a changed header, read from the sources (unit_deps only knows units a + // merge compiled). Without dependents, a header-only change compiles one includer per header. const headerOnly = !units.size; for (const h of changedHeaders) { - const deps = known ? this.store.all('SELECT unit FROM unit_deps WHERE header = ? ORDER BY unit', h).map((r) => r.unit) : allProjectUnits(); + const deps = this.includersOf([h], files); if (dependents) for (const u of deps) units.add(u); else if (headerOnly && deps.length) units.add(deps[0]); // A header expands the same way in every unit that includes it: one of them shows what its @@ -176,7 +187,7 @@ export class Verifier { } const names = [...(args.size ? args : classes)]; for (const h of changedHeaders) { - const deps = this.store.all('SELECT unit FROM unit_deps WHERE header = ? ORDER BY unit', h).map((r) => r.unit); + const deps = this.includersOf([h], files); const hits = names.length ? unitsMentioning(this.repo, deps, names, files) ?? [] : []; for (const u of (hits.length ? hits : deps).slice(0, 16)) units.add(u); } @@ -308,7 +319,7 @@ export class Verifier { for (const h of Object.keys(files).filter((f) => !isUnit(f))) { const before = this.repo.show(head, h); if (before === files[h]) continue; - const deps = this.store.all('SELECT unit FROM unit_deps WHERE header = ?', h).map((r) => r.unit); + const deps = this.includersOf([h], files); const narrowed = before === null ? null : unitsMentioning(this.repo, deps, changedNames(before, files[h]), files); const pick = narrowed && narrowed.length <= 300 ? narrowed : deps; if (pick !== deps) this.log.info(`batch ${batch.id}: ${h} changed; compiling ${pick.length} of its ${deps.length} includers (the ones that mention what changed)`); diff --git a/test/server.test.js b/test/server.test.js index b28d2dc..734e117 100644 --- a/test/server.test.js +++ b/test/server.test.js @@ -473,6 +473,18 @@ test('a header batch is verified on its own units and merged with every includer } }); +test('header includers come from the sources too, also for units no merge has compiled', () => { + ctx.repo.commitFiles({ 'src/deep.h': 'int deep();\n', 'src/mid.h': '#include "deep.h"\n', 'src/user.cpp': '#include "mid.h"\nint u() { return deep(); }\n', 'src/other.cpp': 'int o() { return 0; }\n' }, 'include chain', 'x '); + const units = ctx.project.compile?.units; + if (units) Object.assign(units, { 'src/user.cpp': 0, 'src/other.cpp': 0 }); + try { + assert.deepEqual(ctx.verifier.includersOf(['src/deep.h']).filter((u) => u.startsWith('src/user') || u.startsWith('src/other')), ['src/user.cpp'], 'a transitive includer, found though no merge recorded it'); + assert.deepEqual(ctx.verifier.includersOf(['src/deep.h'], { 'src/user.cpp': 'int u() { return 0; }\n' }).filter((u) => u === 'src/user.cpp'), [], 'as the submission changes it'); + } finally { + if (units) for (const u of ['src/user.cpp', 'src/other.cpp']) delete units[u]; + } +}); + test('agreeing peers accept a function without human review (three ahead)', async () => { await activeUser('aaauthor'); const author = ctx.accounts.byName('aaauthor');