fix(pregate): parse Cargo manifests as TOML; follow workspace membership

The line scan of Cargo.toml missed dependents (a false green) and picked the
wrong workspace:
- an inherited renamed dependency (`base = { workspace = true }` with
  `[workspace.dependencies] base = { package = "alpha", path = ... }`);
- dotted keys (`alpha.path = "../a"`), `[dependencies.<name>]` tables and
  target-specific tables, matched by key name only;
- a crate the workspace excludes ran `cargo test -p` from the workspace root.

Manifests are now parsed with Bun.TOML. A dependency resolves by path
(relative to the manifest declaring it, or to the workspace root when
inherited), else by package name. A crate runs from the workspace it is a
member of under Cargo's rules (package.workspace, the nearest [workspace],
members globs, exclude, literal members, path dependencies of members),
else from its own directory. Nothing runs cargo.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
vilano93andClaude Opus 5.5 authored and garrytan committed 2026-10-24 20:08:40 +00:00
1 parent e495951205
commit b85593e6bc
3 files changed
+190 -84

No files matched your search

+6 -6
View File
@@ -16,10 +16,10 @@ import * as os from 'node:os';
import * as path from 'node:path';
import { baseInputs, sha256 } from '../context';
import { PREGATE_CONFIG_PATH } from '../config';
import { isDeviceDart, readRepoFile } from '../import-graph';
import { isDeviceDart, listRepoFiles, readRepoFile } from '../import-graph';
import { selectLanes, type Selection } from '../lane-selection';
import { launchRuntime } from '../runtime';
import { cargoWorkspaceRoot, crateOf, type ManifestCache } from '../rust';
import { cargoModel, type CargoModel } from '../rust';
import { adapterFor, readRange, supervise, type Counter, type Supervised } from '../supervise';
import { matchesAny } from '../../ship-policy';
import type { CheckDef, CheckResult, PregateContext, RemoteObligation } from '../types';
@@ -160,8 +160,7 @@ export function planRuns(ctx: PregateContext, files: readonly string[]): RunPlan
const packages = new Map<string, string[]>();
const seen = new Map<string, string | null>();
const seenPackage = new Map<string, string | null>();
const manifests: ManifestCache = new Map();
const read = (rel: string) => readRepoFile(ctx.repoRoot, rel);
let model: CargoModel | null = null;
const pkgRunners = new Map<string, ReturnType<typeof packageRunner>>();
const rest: string[] = [];
const noPubspec: string[] = [];
@@ -171,9 +170,10 @@ export function planRuns(ctx: PregateContext, files: readonly string[]): RunPlan
const under = (root: string, f: string) => (root ? f.slice(root.length + 1) : f);
for (const f of files) {
if (f.endsWith('.rs')) {
const crate = crateOf(read, f, manifests);
model ??= cargoModel(listRepoFiles(ctx.repoRoot), rel => readRepoFile(ctx.repoRoot, rel));
const crate = model.crateOf(f);
if (!crate) { unclaimed.push(f); continue; }
const cwd = cargoWorkspaceRoot(read, crate.root, manifests);
const cwd = crate.workspace;
const key = `${cwd}\0${crate.name}`;
if (!cargo.has(key)) cargo.set(key, { name: crate.name, cwd, files: [] });
cargo.get(key)!.files.push(under(cwd, f));
+129 -78
View File
@@ -5,43 +5,137 @@
* a `#[cfg(test)]` (in-source unit tests). Cargo runs tests per crate, never
* per file, so the graph maps at crate level: every `.rs` file of a crate,
* and its Cargo.toml, select that crate's tests and the tests of every crate
* that depends on it (transitively, through the manifests' dependency keys).
* Uncertainty widens: a renamed dependency counts under both names, and any
* cfg predicate naming `test` makes a file a test.
* that depends on it, transitively.
* Manifests are parsed as TOML (Bun.TOML, as lib/model-policy.ts does), so
* dotted keys, `[dependencies.<name>]` tables, target-specific tables and
* `{ workspace = true }` inheritance read as Cargo reads them. A dependency
* resolves by `path` (relative to the manifest declaring it) to a tracked
* crate, else by package name (`package = ` or the key); a name match only
* widens. The workspace a crate runs in follows Cargo's membership rules:
* `package.workspace`, else the nearest `[workspace]` above it, when that
* workspace is the crate's own manifest, lists it in `members` or reaches it
* by a path dependency, and does not `exclude` it; otherwise the crate
* directory. Nothing here runs cargo: pregate reads files only.
*/
import * as path from 'node:path';
import { matchesAny } from '../ship-policy';
export interface CargoManifest { name: string | null; workspace: boolean; deps: string[] }
export interface Crate { root: string; name: string }
/** Reads a repo-relative file's text, or null when it is absent or unreadable. */
export type ReadFile = (rel: string) => string | null;
const DEP_TABLE = /^(?:target\..+\.)?(?:dependencies|dev-dependencies|build-dependencies)$/;
const DEP_SUBTABLE = /^(?:target\..+\.)?(?:dependencies|dev-dependencies|build-dependencies)\.["']?([A-Za-z0-9_-]+)["']?$/;
const PACKAGE_KEY = /\bpackage\s*=\s*["']([A-Za-z0-9_-]+)["']/;
type Table = Record<string, unknown>;
const isTable = (v: unknown): v is Table => typeof v === 'object' && v !== null && !Array.isArray(v);
const strings = (v: unknown): string[] => (Array.isArray(v) ? v.filter((x): x is string => typeof x === 'string') : []);
const DEP_TABLES = ['dependencies', 'dev-dependencies', 'build-dependencies'] as const;
/** A Cargo.toml's `[package]` name, whether it declares a `[workspace]`, and every dependency key and `package =` rename, in a line scan (no TOML parser; comments dropped). */
export function parseCargoManifest(text: string): CargoManifest {
let section = '';
let name: string | null = null;
let workspace = false;
const deps = new Set<string>();
for (const raw of text.replace(/^/, '').split('\n')) {
const line = raw.replace(/(^|\s)#.*$/, '$1').trim();
const header = /^\[\[?\s*([^\]]+?)\s*\]\]?$/.exec(line);
if (header) {
section = header[1]!.replace(/\s+/g, '');
if (section === 'workspace' || section.startsWith('workspace.')) workspace = true;
const sub = DEP_SUBTABLE.exec(section);
if (sub) deps.add(sub[1]!);
continue;
/** One dependency: the crate directory its `path` names (repo-relative, normalized) and the package name it asks for. */
interface DepSpec { path: string | null; name: string }
export interface Crate {
root: string;
name: string;
/** Where `cargo test -p <name>` runs: the root of the workspace it is a member of, else the crate directory. */
workspace: string;
/** Roots of the tracked crates it depends on. */
deps: string[];
}
export interface CargoModel { crates: Map<string, Crate>; crateOf(file: string): Crate | null }
interface Manifest { dir: string; toml: Table }
const dirOf = (manifest: string) => (path.posix.dirname(manifest) === '.' ? '' : path.posix.dirname(manifest));
/** `dir` joined with a manifest-relative path, normalized; null when it leaves the repository. */
function joinRel(dir: string, rel: string): string | null {
const joined = path.posix.normalize(path.posix.join(dir || '.', rel.replace(/\\/g, '/'))).replace(/\/+$/, '');
if (joined === '..' || joined.startsWith('../') || path.posix.isAbsolute(joined)) return null;
return joined === '.' ? '' : joined;
}
const inside = (dir: string, root: string) => root === '' || dir === root || dir.startsWith(`${root}/`);
const relTo = (dir: string, root: string) => (root === '' ? dir : dir === root ? '' : dir.slice(root.length + 1));
/** Every dependency of a manifest: `[*dependencies]` and `[target.<cfg>.*dependencies]`, `{ workspace = true }` read from `ws`'s `[workspace.dependencies]` (its paths relative to the workspace root). */
function depSpecs(m: Manifest, ws: Manifest | null): DepSpec[] {
const tables: Table[] = [];
for (const key of DEP_TABLES) if (isTable(m.toml[key])) tables.push(m.toml[key] as Table);
if (isTable(m.toml.target)) for (const cfg of Object.values(m.toml.target)) if (isTable(cfg)) for (const key of DEP_TABLES) if (isTable(cfg[key])) tables.push(cfg[key] as Table);
const inherited = ws && isTable(ws.toml.workspace) && isTable(ws.toml.workspace.dependencies) ? ws.toml.workspace.dependencies : {};
const out: DepSpec[] = [];
for (const table of tables) {
for (const [key, raw] of Object.entries(table)) {
const fromWorkspace = isTable(raw) && raw.workspace === true;
const spec = fromWorkspace ? inherited[key] : raw;
const renamed = [spec, raw].find(s => isTable(s) && typeof s.package === 'string') as Table | undefined;
const p = isTable(spec) && typeof spec.path === 'string' ? joinRel(fromWorkspace ? ws!.dir : m.dir, spec.path) : null;
out.push({ path: p, name: (renamed?.package as string | undefined) ?? key });
}
if (section === 'package') { const m = /^name\s*=\s*["']([A-Za-z0-9_-]+)["']/.exec(line); if (m) name = m[1]!; continue; }
if (DEP_TABLE.test(section)) {
const key = /^["']?([A-Za-z0-9_-]+)["']?\s*=/.exec(line);
if (key) deps.add(key[1]!);
}
if (DEP_TABLE.test(section) || DEP_SUBTABLE.test(section)) { const renamed = PACKAGE_KEY.exec(line); if (renamed) deps.add(renamed[1]!); }
}
return { name, workspace, deps: [...deps] };
return out;
}
/** The tracked crates among `files` (the paths `git ls-files` reports), what each depends on, and the workspace each runs in. */
export function cargoModel(files: readonly string[], read: ReadFile): CargoModel {
const manifests = new Map<string, Manifest>();
for (const f of files) {
if (path.posix.basename(f) !== 'Cargo.toml') continue;
const text = read(f);
if (text === null) continue;
try { const toml = Bun.TOML.parse(text); if (isTable(toml)) manifests.set(dirOf(f), { dir: dirOf(f), toml }); } catch { /* an unparsable manifest names no crate */ }
}
const nameOf = (dir: string) => { const pkg = manifests.get(dir)?.toml.package; return isTable(pkg) && typeof pkg.name === 'string' ? pkg.name : null; };
const roots = [...manifests.keys()].filter(d => nameOf(d) !== null);
const workspaces = [...manifests.values()].filter(m => isTable(m.toml.workspace));
const byName = new Map<string, string[]>();
for (const r of roots) byName.set(nameOf(r)!, [...(byName.get(nameOf(r)!) ?? []), r]);
// `package.workspace` names the workspace; otherwise Cargo consults only the nearest [workspace] at or above the crate.
const candidate = (root: string): Manifest | null => {
const pkg = manifests.get(root)!.toml.package as Table;
if (typeof pkg.workspace === 'string') { const w = manifests.get(joinRel(root, pkg.workspace) ?? '\0'); return w && isTable(w.toml.workspace) ? w : null; }
return workspaces.filter(w => inside(root, w.dir)).sort((a, b) => b.dir.length - a.dir.length)[0] ?? null;
};
const memberCache = new Map<string, Set<string>>();
const membersOf = (ws: Manifest): Set<string> => {
const cached = memberCache.get(ws.dir);
if (cached) return cached;
const conf = ws.toml.workspace as Table;
const norm = (g: string) => joinRel('', g) ?? '\0';
const globs = strings(conf.members).map(norm);
const exclude = strings(conf.exclude).map(norm);
const own = new Set(roots.filter(r => candidate(r)?.dir === ws.dir));
const excluded = (r: string) => exclude.some(e => { const rel = relTo(r, ws.dir); return rel === e || rel.startsWith(`${e}/`); });
// Cargo's rules: the root package and a literally listed member always belong; a glob match or a path dependency of a
// member inside the workspace belongs unless `exclude` names it or a directory above it.
const literal = (r: string) => globs.some(g => !/[*?[]/.test(g) && relTo(r, ws.dir) === g);
const members = new Set([...own].filter(r => r === ws.dir || literal(r) || (matchesAny(globs, relTo(r, ws.dir)) && !excluded(r))));
memberCache.set(ws.dir, members);
const queue = [...members];
while (queue.length) {
for (const dep of depSpecs(manifests.get(queue.shift()!)!, ws)) {
if (dep.path === null || members.has(dep.path) || !own.has(dep.path) || !inside(dep.path, ws.dir) || excluded(dep.path)) continue;
members.add(dep.path);
queue.push(dep.path);
}
}
return members;
};
// A path names one crate; without a path (or when it names no tracked crate) every crate of that package name is a candidate.
const resolve = (specs: DepSpec[]) => [...new Set(specs.flatMap(s => (s.path !== null && nameOf(s.path) !== null ? [s.path] : byName.get(s.name) ?? [])))];
const crates = new Map<string, Crate>();
for (const root of roots) {
const ws = candidate(root);
const member = ws !== null && membersOf(ws).has(root) ? ws : null;
crates.set(root, { root, name: nameOf(root)!, workspace: member?.dir ?? root, deps: resolve(depSpecs(manifests.get(root)!, member)).filter(d => d !== root) });
}
const crateOf = (file: string): Crate | null => {
for (let d = path.posix.dirname(file); ; d = path.posix.dirname(d)) {
const c = crates.get(d === '.' ? '' : d);
if (c) return c;
if (d === '.' || d === '/' || d === '') return null;
}
};
return { crates, crateOf };
}
/** Whether Rust source holds a `#[cfg(...)]` whose predicate names `test` outside a string (`cfg(test)`, `cfg(all(test, unix))`; never `cfg(feature = "test")`). */
@@ -50,40 +144,6 @@ export function hasCfgTest(source: string): boolean {
return false;
}
/** Reads a repo-relative file's text, or null when it is absent or unreadable. */
export type ReadFile = (rel: string) => string | null;
/** A memo of manifests by repo-relative directory (`''` for the repository root). */
export type ManifestCache = Map<string, CargoManifest | null>;
function manifestAt(read: ReadFile, dir: string, cache: ManifestCache): CargoManifest | null {
if (!cache.has(dir)) { const text = read(path.posix.join(dir, 'Cargo.toml')); cache.set(dir, text === null ? null : parseCargoManifest(text)); }
return cache.get(dir)!;
}
function ancestors(file: string): string[] {
const out: string[] = [];
for (let d = path.posix.dirname(file); ; d = path.posix.dirname(d)) {
out.push(d === '.' ? '' : d);
if (d === '.' || d === '/' || d === '') return out;
}
}
/** The crate owning `file`: the nearest directory at or above it whose Cargo.toml names a `[package]`; null outside any crate. */
export function crateOf(read: ReadFile, file: string, cache: ManifestCache = new Map()): Crate | null {
for (const dir of ancestors(file)) { const m = manifestAt(read, dir, cache); if (m?.name) return { root: dir, name: m.name }; }
return null;
}
/** Where `cargo test -p` runs for a crate: the nearest directory at or above its root whose Cargo.toml declares `[workspace]`, else the crate root. */
export function cargoWorkspaceRoot(read: ReadFile, crateRoot: string, cache: ManifestCache = new Map()): string {
for (const dir of ancestors(path.posix.join(crateRoot, 'Cargo.toml'))) if (manifestAt(read, dir, cache)?.workspace) return dir;
return crateRoot;
}
/** Whether a crate file is a test by its path alone: under `<crate>/tests/`. */
function inTestsDir(crate: Crate, file: string): boolean { return file.startsWith(crate.root ? `${crate.root}/tests/` : 'tests/'); }
export interface RustGraph {
tests: string[];
edges: Array<{ dependency: string; dependent: string; via: 'import' | 'convention' }>;
@@ -96,29 +156,20 @@ export interface RustGraph {
* (`convention`) and of every crate depending on it, transitively (`import`).
*/
export function rustEdges(files: readonly string[], read: ReadFile): RustGraph {
const cache: ManifestCache = new Map();
const manifests = files.filter(f => path.posix.basename(f) === 'Cargo.toml');
const crates = new Map<string, Crate & { deps: string[] }>();
for (const f of manifests) {
const dir = path.posix.dirname(f) === '.' ? '' : path.posix.dirname(f);
const m = manifestAt(read, dir, cache);
if (m?.name) crates.set(dir, { root: dir, name: m.name, deps: m.deps });
}
const { crates, crateOf } = cargoModel(files, read);
const out: RustGraph = { tests: [], edges: [] };
if (!crates.size) return out;
const testsOf = new Map<string, string[]>([...crates.keys()].map(k => [k, []]));
const manifestOf = (root: string) => (root ? `${root}/Cargo.toml` : 'Cargo.toml');
for (const f of files) {
if (!f.endsWith('.rs')) continue;
const crate = crateOf(read, f, cache);
if (!crate || !crates.has(crate.root)) continue;
const crate = crateOf(f);
if (!crate) continue;
out.edges.push({ dependency: f, dependent: manifestOf(crate.root), via: 'convention' });
if (inTestsDir(crate, f) || hasCfgTest(read(f) ?? '')) { out.tests.push(f); testsOf.get(crate.root)!.push(f); }
if (f.startsWith(crate.root ? `${crate.root}/tests/` : 'tests/') || hasCfgTest(read(f) ?? '')) { out.tests.push(f); testsOf.get(crate.root)!.push(f); }
}
const byName = new Map<string, string[]>();
for (const c of crates.values()) byName.set(c.name, [...(byName.get(c.name) ?? []), c.root]);
const dependents = new Map<string, Set<string>>();
for (const c of crates.values()) for (const d of c.deps) for (const root of byName.get(d) ?? []) if (root !== c.root) dependents.set(root, (dependents.get(root) ?? new Set()).add(c.root));
for (const c of crates.values()) for (const d of c.deps) dependents.set(d, (dependents.get(d) ?? new Set()).add(c.root));
for (const root of crates.keys()) {
const reach = new Set<string>([root]);
const queue = [root];
+55
View File
@@ -149,6 +149,61 @@ describe('Rust runner and summary (pure)', () => {
});
});
/**
* Manifest shapes a line scan misreads (review of the first cut): a renamed
* dependency inherited from `[workspace.dependencies]`, TOML dotted keys, the
* `[dependencies.<name>]` table form, target-specific tables, and a crate the
* workspace excludes. Dependencies resolve by path first, then by package name.
*/
const MANIFEST_FILES: Record<string, string> = {
'Cargo.toml': '[workspace]\nmembers = ["crates/*"]\nexclude = ["crates/helper"]\n\n[workspace.dependencies]\nbase = { package = "alpha", path = "crates/a" }\n',
'crates/a/Cargo.toml': '[package]\nname = "alpha"\nversion = "0.1.0"\n',
'crates/a/src/lib.rs': 'pub fn a() {}\n',
'crates/a/tests/a.rs': '#[test]\nfn a() {}\n',
'crates/beta/Cargo.toml': '[package]\nname = "beta"\nversion = "0.1.0"\n\n[dependencies]\nbase = { workspace = true }\n',
'crates/beta/tests/b.rs': '#[test]\nfn b() {}\n',
'crates/dotted/Cargo.toml': '[package]\nname = "dotted"\nversion = "0.1.0"\n\n[dependencies]\nfoo.path = "../a"\n',
'crates/dotted/tests/d.rs': '#[test]\nfn d() {}\n',
'crates/table/Cargo.toml': '[package]\nname = "table"\nversion = "0.1.0"\n\n[dev-dependencies.whatever]\npath = "../a"\n',
'crates/table/tests/t.rs': '#[test]\nfn t() {}\n',
'crates/targeted/Cargo.toml': '[package]\nname = "targeted"\nversion = "0.1.0"\n\n[target.\'cfg(unix)\'.build-dependencies]\nrenamed = { path = "../a" }\n',
'crates/targeted/tests/x.rs': '#[test]\nfn x() {}\n',
'crates/plain/Cargo.toml': '[package]\nname = "plain"\nversion = "0.1.0"\n',
'crates/plain/tests/p.rs': '#[test]\nfn p() {}\n',
'crates/helper/Cargo.toml': '[package]\nname = "helper"\nversion = "0.1.0"\n',
'crates/helper/src/lib.rs': '#[cfg(test)]\nmod tests {\n #[test]\n fn h() {}\n}\n',
};
function manifestRepo(): FixtureRepo {
const repo = makeFixtureRepo({ pregate: null });
repos.push(repo);
commit(repo, MANIFEST_FILES, 'cargo manifests');
return repo;
}
describe('Cargo manifests are parsed as TOML', () => {
// Value: protects=every dependency spelling Cargo accepts links a crate to its dependents: an inherited renamed workspace dependency,
// a dotted `foo.path` key, a `[dev-dependencies.<name>]` table and a target-specific build dependency, all resolved by path;
// fails_when=the manifest is read by a line scan again and alpha's change misses these dependents (a false green);
// why_new=the first cut matched dependency keys against package names only; seam=none; no_claim=cargo's own resolver
test('renamed, inherited, dotted, table-form and target-specific dependencies all select the dependent crates', () => {
const g = buildGraph(manifestRepo().work);
expect(testsFor(g, 'crates/a/src/lib.rs').tests).toEqual(['crates/a/tests/a.rs', 'crates/beta/tests/b.rs', 'crates/dotted/tests/d.rs', 'crates/table/tests/t.rs', 'crates/targeted/tests/x.rs']);
});
// Value: protects=a crate the workspace excludes runs from its own directory, a member from the workspace root; fails_when=the
// nearest [workspace] is taken without checking members/exclude and `cargo test -p helper` runs where helper is no member
// (cargo fails, or tests a same-named member); why_new=membership was not read; seam=none; no_claim=a real cargo (plan only)
test('an excluded crate runs from its own directory; a member runs from the workspace root', () => {
const repo = manifestRepo();
const ctx = { repoRoot: repo.work, config: DEFAULT_CONFIG, env: cargoShim(repo) } as unknown as PregateContext;
expect(planRuns(ctx, ['crates/beta/tests/b.rs', 'crates/helper/src/lib.rs']).groups).toEqual([
{ runner: 'cargo test -p beta', cwd: '', files: ['crates/beta/tests/b.rs'] },
{ runner: 'cargo test -p helper', cwd: 'crates/helper', files: ['src/lib.rs'] },
]);
});
});
describe('gstack-pregate on a Rust workspace', () => {
const run = (repo: FixtureRepo, env: Record<string, string>) => runBin(repo, 'gstack-pregate', ['--base', 'main', '--allow-repo-commands', '--stage', 'tests', '--only', 'lanes'], env);
const log = (out: string) => fs.readFileSync(/log: (\S+)/.exec(out)![1]!, 'utf8');