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.
This commit is contained in:
parent
8d472dd8ee
commit
5f8e7493c8
4 changed files with 88 additions and 7 deletions
|
|
@ -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 }) => {
|
||||
|
|
|
|||
58
src/server/includers.js
Normal file
58
src/server/includers.js
Normal file
|
|
@ -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<string,string>} 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('/');
|
||||
}
|
||||
|
|
@ -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)`);
|
||||
|
|
|
|||
|
|
@ -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 <x@y.invalid>');
|
||||
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');
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue