Skip to content

feat(cli): add per-canister build lock to prevent parallel clobbering #472

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
Kamirus merged 3 commits into from
Apr 1, 2026
Merged

feat(cli): add per-canister build lock to prevent parallel clobbering #472

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
00f5b40
feat(cli): add per-canister build lock to prevent parallel clobbering
Kamirus Apr 1, 2026
7fec91e
fix: address review findings for build lock
Kamirus Apr 1, 2026
3f0ad00
refactor
Kamirus Apr 1, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
219 changes: 127 additions & 92 deletions cli/commands/build.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ import { execa } from "execa";
import { exists } from "fs-extra";
import { mkdir, readFile, writeFile } from "node:fs/promises";
import { join } from "node:path";
import { lock, unlockSync } from "proper-lockfile";
import { cliError } from "../error.js";
import { isCandidCompatible } from "../helpers/is-candid-compatible.js";
import { resolveCanisterConfigs } from "../helpers/resolve-canisters.js";
Expand Down Expand Up @@ -70,115 +71,149 @@ export async function build(
motokoPath = resolveConfigPath(motokoPath);
const wasmPath = join(outputDir, `${canisterName}.wasm`);
const mostPath = join(outputDir, `${canisterName}.most`);
let args = [
"-c",
"--idl",
"--stable-types",
"-o",
wasmPath,
motokoPath,
...(await sourcesArgs()).flat(),
...getGlobalMocArgs(config),
];
args.push(
...collectExtraArgs(config, canister, canisterName, options.extraArgs),
);

const isPublicCandid = true; // always true for now to reduce corner cases
const candidVisibility = isPublicCandid ? "icp:public" : "icp:private";
if (isPublicCandid) {
args.push("--public-metadata", "candid:service");
args.push("--public-metadata", "candid:args");

// per-canister lock to prevent parallel builds of the same canister from clobbering output files
const lockTarget = join(outputDir, `.${canisterName}.buildlock`);
await writeFile(lockTarget, "", { flag: "a" });

let release: (() => Promise<void>) | undefined;
try {
release = await lock(lockTarget, {
stale: 300_000,
retries: { retries: 60, minTimeout: 500, maxTimeout: 5_000 },
});
} catch {
cliError(
`Failed to acquire build lock for canister ${canisterName} — another build may be stuck`,
);
}

// proper-lockfile registers its own signal-exit handler, but it doesn't reliably
// fire on process.exit(). This manual handler covers that gap. Double-unlock is
Copy link
Copy Markdown
Collaborator Author

@Kamirus Kamirus Apr 1, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We could get rid of this exitCleanup after refactoring the cliError to throw an exception and not process.exit, but it is a huge refactor so I don't know if we want to commit to it : #473

Copy link
Copy Markdown
Collaborator

@rvanasa rvanasa Apr 1, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok, we can evaluate it separately after merging this PR. Exit cleanup seems like a reasonable / acceptable way to do this imo.

// harmless (the second call throws and is caught).
const exitCleanup = () => {
try {
unlockSync(lockTarget);
} catch {}
};
process.on("exit", exitCleanup);

try {
if (options.verbose) {
console.log(chalk.gray(mocPath, JSON.stringify(args)));
let args = [
"-c",
"--idl",
"--stable-types",
"-o",
wasmPath,
motokoPath,
...(await sourcesArgs()).flat(),
...getGlobalMocArgs(config),
];
args.push(
...collectExtraArgs(config, canister, canisterName, options.extraArgs),
);

const isPublicCandid = true; // always true for now to reduce corner cases
const candidVisibility = isPublicCandid ? "icp:public" : "icp:private";
if (isPublicCandid) {
args.push("--public-metadata", "candid:service");
args.push("--public-metadata", "candid:args");
}
const result = await execa(mocPath, args, {
stdio: options.verbose ? "inherit" : "pipe",
reject: false,
});
try {
if (options.verbose) {
console.log(chalk.gray(mocPath, JSON.stringify(args)));
}
const result = await execa(mocPath, args, {
stdio: options.verbose ? "inherit" : "pipe",
reject: false,
});

if (result.exitCode !== 0) {
if (!options.verbose) {
if (result.stderr) {
console.error(chalk.red(result.stderr));
}
if (result.stdout?.trim()) {
console.error(chalk.yellow("Build output:"));
console.error(result.stdout);
if (result.exitCode !== 0) {
if (!options.verbose) {
if (result.stderr) {
console.error(chalk.red(result.stderr));
}
if (result.stdout?.trim()) {
console.error(chalk.yellow("Build output:"));
console.error(result.stdout);
}
}
cliError(
`Build failed for canister ${canisterName} (exit code: ${result.exitCode})`,
);
}
cliError(
`Build failed for canister ${canisterName} (exit code: ${result.exitCode})`,
);
}

if (options.verbose && result.stdout && result.stdout.trim()) {
console.log(result.stdout);
}

options.verbose &&
console.log(chalk.gray(`Stable types written to ${mostPath}`));
if (options.verbose && result.stdout && result.stdout.trim()) {
console.log(result.stdout);
}

const generatedDidPath = join(outputDir, `${canisterName}.did`);
const resolvedCandidPath = canister.candid
? resolveConfigPath(canister.candid)
: null;
options.verbose &&
console.log(chalk.gray(`Stable types written to ${mostPath}`));

if (resolvedCandidPath) {
try {
const compatible = await isCandidCompatible(
generatedDidPath,
resolvedCandidPath,
);
const generatedDidPath = join(outputDir, `${canisterName}.did`);
const resolvedCandidPath = canister.candid
? resolveConfigPath(canister.candid)
: null;

if (!compatible) {
cliError(
`Candid compatibility check failed for canister ${canisterName}`,
if (resolvedCandidPath) {
try {
const compatible = await isCandidCompatible(
generatedDidPath,
resolvedCandidPath,
);
}

if (options.verbose) {
console.log(
chalk.gray(
`Candid compatibility check passed for canister ${canisterName}`,
),
if (!compatible) {
cliError(
`Candid compatibility check failed for canister ${canisterName}`,
);
}

if (options.verbose) {
console.log(
chalk.gray(
`Candid compatibility check passed for canister ${canisterName}`,
),
);
}
} catch (err: any) {
cliError(
`Error during Candid compatibility check for canister ${canisterName}${err?.message ? `\n${err.message}` : ""}`,
);
}
} catch (err: any) {
cliError(
`Error during Candid compatibility check for canister ${canisterName}${err?.message ? `\n${err.message}` : ""}`,
);
}
}

options.verbose &&
console.log(chalk.gray(`Adding metadata to ${wasmPath}`));
const candidPath = resolvedCandidPath ?? generatedDidPath;
const candidText = await readFile(candidPath, "utf-8");
const customSections: CustomSection[] = [
{ name: `${candidVisibility} candid:service`, data: candidText },
];
if (canister.initArg) {
customSections.push({
name: `${candidVisibility} candid:args`,
data: canister.initArg,
});
}
const wasmBytes = await readFile(wasmPath);
const newWasm = getWasmBindings().add_custom_sections(
wasmBytes,
customSections,
);
await writeFile(wasmPath, newWasm);
} catch (err: any) {
if (err.message?.includes("Build failed for canister")) {
throw err;
options.verbose &&
console.log(chalk.gray(`Adding metadata to ${wasmPath}`));
const candidPath = resolvedCandidPath ?? generatedDidPath;
const candidText = await readFile(candidPath, "utf-8");
const customSections: CustomSection[] = [
{ name: `${candidVisibility} candid:service`, data: candidText },
];
if (canister.initArg) {
customSections.push({
name: `${candidVisibility} candid:args`,
data: canister.initArg,
});
}
const wasmBytes = await readFile(wasmPath);
const newWasm = getWasmBindings().add_custom_sections(
wasmBytes,
customSections,
);
await writeFile(wasmPath, newWasm);
} catch (err: any) {
if (err.message?.includes("Build failed for canister")) {
throw err;
}
cliError(
`Error while compiling canister ${canisterName}${err?.message ? `\n${err.message}` : ""}`,
);
}
cliError(
`Error while compiling canister ${canisterName}${err?.message ? `\n${err.message}` : ""}`,
);
} finally {
process.removeListener("exit", exitCleanup);
try {
await release?.();
} catch {}
}
}

Expand Down
45 changes: 45 additions & 0 deletions cli/package-lock.json
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 2 additions & 0 deletions cli/package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -86,6 +86,7 @@
"prettier-plugin-motoko": "0.13.0",
"promisify-child-process": "4.1.2",
"prompts": "2.4.2",
"proper-lockfile": "4.1.2",
"semver": "7.7.1",
"stream-to-promise": "3.0.0",
"string-width": "7.2.0",
Expand All @@ -102,6 +103,7 @@
"@types/ncp": "2.0.8",
"@types/node": "24.0.3",
"@types/prompts": "2.4.9",
"@types/proper-lockfile": "4.1.4",
"@types/semver": "7.5.8",
"@types/stream-to-promise": "2.2.4",
"@types/tar": "6.1.13",
Expand Down
17 changes: 17 additions & 0 deletions cli/tests/build.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -100,6 +100,23 @@ describe("build", () => {
}
});

test("parallel builds of the same canister both succeed", async () => {
const cwd = path.join(import.meta.dirname, "build/success");
try {
const [a, b] = await Promise.all([
cli(["build", "foo"], { cwd }),
cli(["build", "foo"], { cwd }),
]);
expect(a.exitCode).toBe(0);
expect(b.exitCode).toBe(0);
expect(existsSync(path.join(cwd, ".mops/.build/foo.wasm"))).toBe(true);
expect(existsSync(path.join(cwd, ".mops/.build/foo.did"))).toBe(true);
expect(existsSync(path.join(cwd, ".mops/.build/foo.most"))).toBe(true);
} finally {
cleanFixture(cwd);
}
});

// Regression: bin/mops.js must route through environments/nodejs/cli.js
// so that setWasmBindings() is called before any command runs.
// The dev entry point (npm run mops) uses tsx and always worked;
Expand Down
1 change: 1 addition & 0 deletions cli/tests/build/success/.gitignore
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,2 +1,3 @@
.mops/
cli-output-test/
*.most
Loading