Skip to content

Commit f519dfb

Browse files
committed
fix(build): a package's generated inputs come before its compiles; an empty link is refused (2026.8.30.1)
Three defects, one change of mind about evidence. Each of them was a place where something answered "is this ready?" from a proxy instead of from the artifact. ── mcpp#534: `role = "source"` did not order a generated header ───────────── The engine's own comment claimed ordering needed no special handling because "a Source action's outputs ARE the compile edge's inputs". That is true of a generated `.cpp` and false of a generated `.h`: a header is reached through `-I`, never appears as an edge input, and the depfile that would record it does not exist until a compile has already succeeded. So an action whose outputs were all headers had a node in build.ninja that nothing could reach — not `default` (Source outputs are excluded on purpose), not the goal phony (objects and link outputs only), and no consuming edge. It never ran. The issue was filed as an intermittent race; it is deterministic, and five consecutive builds reproduce it identically. What made it look like a race is that `prepare_actions` wrote a zero-byte placeholder for every declared Source output, headers included — so the file was on disk whether or not the generator had run. * `BuildAction` records the package that declared it, spelled the same way `CompileUnit::packageName` is (`qualified_package_name`, now exported so the two cannot drift). * Each package that declares a gating action gets one phony over its outputs, and every compile edge of that package takes it as an order-only prerequisite. Per package, not per build: `include_dir` colours only the declaring package's own TUs, and a build-wide edge would express a dependency that does not exist while landing on the critical path. * `check_action_ordering` scans the emitted manifest and fails the build if any such edge is missing one — including the denominator, because "every edge that should carry it does" is vacuously true when none does, which was exactly the previous state. Seven call sites append the string today; being careful at seven sites is not a mechanism. * `blocking` on a `check` now does what it has been documented to do since it was introduced. It was typed, emitted over the build-program protocol, parsed, documented in two languages and demonstrated in a shipped example — and read by nothing. * Placeholders are no longer written for outputs that are not translation units. The scan never reads a header, and the empty file only ever turned "the generator did not run" into "the header is empty". ── mcpp#533: an empty link unit, reported as a shell error ────────────────── A dependency whose `install()` was skipped over a package-identity collision left a version directory with no sources. mcpp planned its shared library anyway and the user was shown `/bin/sh: 1: -shared: not found`. * A link unit with no inputs is refused at plan time, naming the target. The static case is why this is an error rather than a better linker message: `ar rcs` with no members exits 0 and writes an 8-byte archive, so the build REPORTED SUCCESS and every consumer failed later with undefined symbols. * `cc` is emitted unconditionally, for the reason `c_ldflags` twenty-six lines below already carried (mcpp#426). The rule had been written down for one variable of `c_link`/`c_shared` and not for the other. * `check_rule_commands_name_a_program` scans the manifest for the class: a rule's command must begin with a program. Deliberately not the more obvious "no undefined variables" — ninja's empty expansion is a feature several rules rely on (`$soname_flag`, `$unit_ldflags`), and that check would have needed an allowlist of exceptions. * `.mcpp_ok` is no longer written from the installer's exit code plus the existence of a directory the installer creates before doing any work. It now requires one entry that neither mcpp nor xlings wrote. Withheld rather than fatal, because a package may legitimately install no payload. * The same predicate runs on the fast path, so a store already poisoned by this bug heals on the next build instead of requiring the user to know which directory to delete. * The lib-root warning asked `has_lib_target` — "does this produce a library" — when the property it wants is "is this a C++ module library". A source-built C package warned that `src/<name>.cppm` was missing in every consumer's build. ── Tests ─────────────────────────────────────────────────────────────────── e2e 314 (a dependency generating its only header), 315 (blocking gates the compile, non-blocking does not), 316 (empty shared AND static targets refused; a populated one still builds). All three were run against the pre-fix binary and all three fail there, so they discriminate rather than describe. Unit: 14 new cases over the two emitter guards, the ordering denominator, and the install-marker evidence — including the poisoned-store heal. Analysis and cross-repo plan: .agents/docs/2026-08-30-*.md Refs #533, #534
1 parent ab1da5d commit f519dfb

19 files changed

Lines changed: 2550 additions & 52 deletions

.agents/docs/2026-08-30-cross-repo-fix-plan-532-533-534.md

Lines changed: 837 additions & 0 deletions
Large diffs are not rendered by default.

.agents/docs/2026-08-30-issues-532-533-534-analysis.md

Lines changed: 443 additions & 0 deletions
Large diffs are not rendered by default.

docs/07-build-mcpp.md

Lines changed: 18 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -311,14 +311,27 @@ attach:
311311

312312
| `role` | Outputs | Ordering | Typical |
313313
|---|---|---|---|
314-
| `source` | join the compile set | the compile edge consumes them | protoc, a transpiler |
315-
| `check` | a stamp file, written by mcpp | runs **alongside** compilation (set `blocking = true` to gate it) | clang-tidy, a format or ABI check |
314+
| `source` | compilable ones join the compile set; the rest are produced but not compiled | **every compile edge of the declaring package waits for them** | protoc, a transpiler, a protocol/IDL generator |
315+
| `check` | a stamp file, written by mcpp | runs **alongside** compilation; `blocking = true` makes the package's compile edges wait for it | clang-tidy, a format or ABI check |
316316
| `object` | join the **link** set | the link edge consumes them | a resource compiler, `objcopy` embedding a blob, a generated `.def`, a pre-built `.o` |
317317
| `artifact` | a new file | its *inputs* are link outputs, so it runs after the link | codesign, packaging, size budgets |
318318

319-
No phase machinery is involved: ninja's own file dependencies do the
320-
sequencing, which is also why an `artifact` action cannot double-apply itself
321-
the way a naive "post-build hook" would.
319+
No phase machinery is involved. `object` and `artifact` are sequenced by
320+
ninja's own file dependencies — which is also why an `artifact` action cannot
321+
double-apply itself the way a naive "post-build hook" would. `source` and a
322+
blocking `check` are sequenced by an order-only edge from the declaring
323+
package's compile edges to that package's action outputs.
324+
325+
> **Why `source` needs the edge (mcpp 2026.8.30.2+).** A generated `.cpp`
326+
> becomes an input of the edge that compiles it, so it was ordered for free. A
327+
> generated **header** never does: it is reached through `-I`, and the depfile
328+
> that would record it does not exist until a compile has already succeeded.
329+
> Before this, an action whose outputs were all headers had a node in
330+
> `build.ninja` that nothing could reach — not `default`, not the goal set, no
331+
> consuming edge — so it never ran, and what the compiler read was the empty
332+
> placeholder mcpp writes for a declared output. The ordering is **per
333+
> package**, because `include_dir` colours only the declaring package's own
334+
> translation units.
322335
323336
**A check's command does not have to write its stamp** (mcpp 2026.8.29.1+).
324337
The verdict is the exit code; the stamp is bookkeeping the graph needs, and

docs/zh/07-build-mcpp.md

Lines changed: 13 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -277,13 +277,22 @@ int main() {
277277

278278
| `role` | 输出 | 顺序 | 典型 |
279279
|---|---|---|---|
280-
| `source` | 进编译集 | 编译边消费它们 | protoc、转译器 |
281-
| `check` | 一个 stamp 文件,由 mcpp 写入 | **与编译并行**(`blocking = true` 才前置) | clang-tidy、格式/ABI 检查 |
280+
| `source` | 可编译的进编译集,其余只产出、不编译 | **声明它的那个包的每条编译边都等它** | protoc、转译器、协议/IDL 生成器 |
281+
| `check` | 一个 stamp 文件,由 mcpp 写入 | 与编译并行;`blocking = true` 让该包的编译边等它 | clang-tidy、格式/ABI 检查 |
282282
| `object` |**链接**| 链接边消费它们 | 资源编译器、`objcopy` 嵌 blob、生成的 `.def`、预编译 `.o` |
283283
| `artifact` | 一个新文件 | 它的**输入**是链接产物,所以在链接之后跑 | 签名、打包、size budget |
284284

285-
全程不涉及任何 phase 机制:顺序由 ninja 自己的文件依赖决定 —— 这也是为什么
286-
`artifact` 不会像朴素的「post 构建钩子」那样把自己重复施加一遍。
285+
全程不涉及任何 phase 机制。`object``artifact` 由 ninja 自己的文件依赖定序 ——
286+
这也是为什么 `artifact` 不会像朴素的「post 构建钩子」那样把自己重复施加一遍。
287+
`source` 与 blocking 的 `check` 则由一条 order-only 边定序:从声明它的那个包的
288+
编译边,指向该包的 action 产物。
289+
290+
> **`source` 为什么需要这条边(mcpp 2026.8.30.2+)。** 生成的 `.cpp` 会成为编译它
291+
> 那条边的输入,所以顺序是白得的。生成的**头文件**永远不会:它是通过 `-I` 找到的,
292+
> 而能记录它的 depfile 要等到某次编译成功之后才存在。在此之前,一个产物全是头文件的
293+
> action 在 `build.ninja` 里有节点却无人可达 —— 不在 `default`、不在 goal 集、没有
294+
> 任何边消费它 —— 于是它从不执行,而编译器读到的是 mcpp 为已声明产物写下的那个空占位
295+
> 文件。这条边**按包**划分,因为 `include_dir` 只染色声明它的那个包自己的 TU。
287296
288297
**check 的命令不必自己写 stamp**(mcpp 2026.8.29.1+)。判定是退出码,stamp 是**构建图**
289298
需要的记账;命令成功时由 mcpp 创建它。在此之前每个 check 都需要一个包装脚本去 touch

mcpp.toml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
[package]
22
name = "mcpp"
3-
version = "2026.8.29.1"
3+
version = "2026.8.30.1"
44
description = "Modern C++ build & package management tool"
55
license = "Apache-2.0"
66
authors = ["mcpp-community"]

modules/buildmcpp/src/directives.cppm

Lines changed: 17 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -387,8 +387,17 @@ std::string action_error(const Directives& d);
387387
// Never truncates an existing file: after the first build the real content is
388388
// there, and rewriting it would make ninja think the input changed on every
389389
// prepare.
390+
//
391+
// ⚠️ ONLY FOR OUTPUTS THAT ARE TRANSLATION UNITS, which is why this needs the
392+
// table. A placeholder exists so the SCAN has something to read, and the scan
393+
// never reads a header — but writing one anyway turned "the generator did not
394+
// run" into "the header is empty", and mcpp#534 was diagnosed as a race for
395+
// exactly that reason: the file was on disk, so the action looked like it had
396+
// run. A missing file is the honest report, and after the ordering fix the
397+
// generator runs before anything reads it either way.
390398
void prepare_actions(std::vector<mcpp::manifest::BuildAction>& actions,
391-
const std::filesystem::path& pkgRoot);
399+
const std::filesystem::path& pkgRoot,
400+
const mcpp::ExtensionTable& extensions);
392401

393402
// Does this action output belong in the COMPILE set?
394403
//
@@ -773,7 +782,8 @@ bool is_compilable_output(const fs::path& p, const mcpp::ExtensionTable& t) {
773782
}
774783

775784
void prepare_actions(std::vector<mcpp::manifest::BuildAction>& actions,
776-
const fs::path& pkgRoot) {
785+
const fs::path& pkgRoot,
786+
const mcpp::ExtensionTable& extensions) {
777787
for (auto& a : actions) {
778788
auto absolutize = [&](std::vector<std::string>& v) {
779789
for (auto& p : v) {
@@ -788,6 +798,11 @@ void prepare_actions(std::vector<mcpp::manifest::BuildAction>& actions,
788798
if (a.role != mcpp::manifest::BuildAction::Role::Source) continue;
789799
for (auto const& o : a.outputs) {
790800
if (o.find("${mcpp.") != std::string::npos) continue;
801+
// A placeholder exists so the scan has a translation unit to read.
802+
// A header is not one — nothing scans it, and the empty file it
803+
// used to leave behind is what made a generator that never ran
804+
// look like one that had (mcpp#534).
805+
if (!is_compilable_output(o, extensions)) continue;
791806
std::error_code ec;
792807
fs::path p(o);
793808
if (fs::exists(p, ec)) continue; // real content already there

modules/manifest/src/types.cppm

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -300,6 +300,18 @@ struct BuildAction {
300300
enum class Role { Source, Check, Object, Artifact };
301301

302302
std::string id; // diagnostics + edge naming
303+
// Which package's `build.mcpp` declared this. Filled by the engine when
304+
// actions are collected into the plan, NOT by the build program — the
305+
// program does not know, and the engine already does.
306+
//
307+
// Load-bearing, not bookkeeping: the ordering edge an action needs is
308+
// scoped to the declaring package, because `include_dir` colours only that
309+
// package's own translation units. A build-wide ordering would express a
310+
// dependency that does not exist and put it on the critical path of a
311+
// build whose wall clock is dominated by one. Spelled the same way
312+
// `CompileUnit::packageName` is (`qualified_package_name`), because the
313+
// two are matched against each other.
314+
std::string packageName;
303315
Role role = Role::Source;
304316
std::vector<std::string> inputs; // absolute or package-relative
305317
std::vector<std::string> outputs; // ditto; declared, see INV-D

modules/versioning/src/version.cppm

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,6 @@ import std;
3131

3232
export namespace mcpp {
3333

34-
inline constexpr std::string_view MCPP_VERSION = "2026.8.29.1";
34+
inline constexpr std::string_view MCPP_VERSION = "2026.8.30.1";
3535

3636
} // namespace mcpp

0 commit comments

Comments
 (0)