Skip to content

Commit d1d02cb

Browse files
committed
fix(cache): a package's object layout must not depend on the consumer (#344)
A dependency's objects live in the global build cache under a key that deliberately excludes the consuming project — that exclusion is what makes an entry shareable across projects. Their LAYOUT inside the entry did not exclude it: the address was the consumer's build-dir path with `obj/` stripped, and build-dir object paths are decided by #233's basename disambiguation, whose census spans the WHOLE build directory. So `compat.zlib`'s compress.o was obj/compress.o alone obj/compat_zlib/zlib-1.3.2/compress.o beside compat.bzip2 under one key. Whichever project ran second asked the entry for a path the first had never written, and ninja rejected the graph before running any command: ninja: error: '<cache>/…/obj/compress.o', needed by 'obj/compress.o', missing and no known rule to make it one line after the CLI printed `Cached compat.zlib v1.3.2 (15 units)`. 32 of 47 mcpp-index workspace members across three platforms. #233 (compile edges collided), #240 (link inputs didn't follow the rename) and this are three products of one machine: a layout decided by a global census. The fix is not a fourth place to keep in sync — it is to take dependencies out of the census. * plan.cppm now emits TWO addresses from ONE derivation: `object` (build dir) and `packageObjectRel` (inside a cache entry, a pure function of the owning package). Dependency objects go to `obj/<pkg-slug>/<mirrored relDir>/<name>.o` unconditionally — cross-package collisions are structurally impossible, so dependencies need no census at all. The root project, never cached, keeps its flat `obj/<name>.o` and now censuses only its own sources. * prepare.cppm's `object_cache_path` — the second derivation — is deleted, along with its `filename()` fallback, which silently mapped distinct sources onto one entry address. * `is_cached(key)` validated the entry's OWN file list while the caller went on to read addresses it computed itself; the two answers were never compared. It is now `probe_cached(key, requested)`. Any divergence is a MISS, never a build failure, and is reported — a systematic recurrence would otherwise show up only as "the cache never hits", with no signal at all. * Cacheability is judged by where the payload is on disk, not by the `sourceKind` label. Multi-version mangling re-anchors a consumer package at `target/.mangled/…` and rewrites its sources while keeping that label; it stays out of the cache today only because axis F happens to differ. Containment is judged LEXICALLY (`path_is_under_any`): stores built out of symlinks are ordinary, and canonicalizing drops every such package out of the cache silently. * All-or-nothing per package: one unaddressable unit takes the whole package out, rather than leaving it half staged. * kCacheEpoch 1 → 2 (the artifact layout changed) and `cache verify` now reports obj addresses that escape their entry. Tests: unit ObjectAddress pins "a dependency's addresses are immune to the rest of the graph"; e2e 184 runs both orderings and asserts ZERO compile edges for the reused dependency — asserting only "it builds" would be satisfied by a build that quietly degraded every hit to a miss. e2e 123's assertion is generalized from "the consumer's main must be renamed" (the shape of #240's first fix) to "every link input is produced by some edge" (the property).
1 parent 5d71824 commit d1d02cb

13 files changed

Lines changed: 1487 additions & 120 deletions

.agents/docs/2026-08-03-issue344-cache-object-address-design.md

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

CHANGELOG.md

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,31 @@
33
> 本文件追踪 `mcpp-community/mcpp` 公开仓的版本演进。
44
> 格式参考 [Keep a Changelog](https://keepachangelog.com/zh-CN/1.1.0/)
55
6+
## [2026.8.3.4] — 2026-08-03
7+
8+
### 修复
9+
10+
- **全局 build cache:同一个 key 下第二个消费者必挂 `missing and no known rule to make it`(#344)。** 一个依赖的 `.o` 存在 cache 条目里的**路径**,此前取自消费方 build dir 的相对路径。而 build dir 里的对象布局由 #233 的 basename 消歧决定,消歧的判据是一次**跨越整个 build dir**的普查 —— 也就是说,它取决于消费方还拉了哪些别的包:
11+
12+
- 同时拉了 `compat.zlib``compat.bzip2`(两者上游各有一个 `compress.c`)→ `obj/compat_zlib/zlib-1.3.2/compress.o`
13+
- 只拉了 `compat.zlib``obj/compress.o`
14+
15+
cache key **刻意不含消费方**(这正是跨工程共享成立的前提),于是两种布局落进同一个条目,后跑的那个工程按自己的布局去取,必然缺一个文件。失败发生在 ninja 的 **graph 加载**阶段 —— 一条命令都还没跑,上一行却刚打印过 `Cached … (15 units)`。mcpp-index 全量 workspace 在三个平台上共 32 个成员因此失败。
16+
17+
条目内的对象地址现在是**包自身的纯函数**:镜像源文件相对它**自己**包根的路径,不含任何消费方信息。构建目录里,依赖包的对象一律落在 `obj/<pkg-slug>/…` 下 —— 跨包撞名结构性地不可能发生,依赖包因此完全退出消歧普查。根工程(永不入 cache)保持沿用至今的扁平 `obj/<name>.o`
18+
19+
#233(编译边撞名)、#240(链接输入未跟改名)与本条是同一台机器的三个产物:**布局由一次全局普查决定**。所以修的不是再补一处同步,而是把依赖包从普查里彻底拿掉。
20+
21+
### 改进
22+
23+
- **cache 条目与本次构建的布局分歧,现在降级为 miss 并明确报告,而不是让 ninja 崩在图加载阶段。** `is_cached` 此前校验的是条目**自述的**文件表,而消费方随后按**自己算的**地址去取 —— 两处独立推导,从不比对。命中判据现在校验「本次实际要读的那批产物」,任何不匹配都只是一次重编。同时新增一行 warning:一个系统性的分歧否则会表现为「cache 永远不命中」而毫无信号,这正是 v2026.7.30.2 之前那个假 `Cached` 骗了三个月的失败模态。
24+
25+
- **可缓存性改判磁盘出处,不再只认标签。** 规则一直写着「无法证明来自不可变的 xpkgs store 就不准入」,但代码实现的是更弱的代理(`sourceKind == "version"`)。多版本共存(mangling)会把消费方包的根重锚到 `<project>/target/.mangled/…`**改写其源码**,而标签仍是 `"version"` —— 它今天不出错只靠轴 F 侥幸。现在按包根的实际位置判定。同一批还加了「全有全无」:任何一个单元拿不到与机器无关的条目地址,整个包退出缓存,不留下半 staged 的包(那会表现为三条边之后的 BMI CRC mismatch)。
26+
27+
- **`mcpp cache verify` 现在会报告逃出条目的对象地址。** 让「条目地址必须是包内相对路径」这条不变量可以离线审计,而不是只能通过复现一次双工程构建才看得见。
28+
29+
- **`kCacheEpoch` 1 → 2。** 产物布局变了,旧条目描述的是本版本不会去要的布局。它们本来就会被判为 miss,但让两套布局共用一个目录会让 `cache gc` 的体积统计和 `cache verify` 的输出失去意义。用户侧表现为一次全量重建,无需任何手工步骤。
30+
631
## [2026.8.3.3] — 2026-08-03
732

833
### 修复

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.3.3"
3+
version = "2026.8.3.4"
44
description = "Modern C++ build & package management tool"
55
license = "Apache-2.0"
66
authors = ["mcpp-community"]

src/bmi_cache.cppm

Lines changed: 113 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,16 @@
55
// (the cache root is $MCPP_HOME/build-cache/v1 — see mcpp.home::cache_root)
66
// entry.json sentinel + self-description + file list
77
// bmi/<module>.{gcm,pcm}
8-
// obj/<relative>.o
8+
// obj/<package-internal path>.o
9+
//
10+
// The obj address is PACKAGE-INTERNAL: it mirrors the source's path relative to
11+
// its own package root and contains nothing about the consuming project
12+
// (mcpp#344). It used to be the consumer's build-dir path with `obj/` stripped,
13+
// which made the layout depend on which OTHER packages the consumer happened to
14+
// pull in — #233's basename disambiguation is triggered by a census over the
15+
// whole build dir — while the key deliberately excludes the consumer. Two
16+
// consumers then wrote and read incompatible layouts under one key and the
17+
// second one died in ninja's graph phase.
918
//
1019
// <key16> comes from mcpp.build.cache_key: a per-package Merkle key over the
1120
// toolchain, the language/dialect settings, the resolved profile, the package's
@@ -75,16 +84,63 @@ struct CacheKey {
7584
std::filesystem::path objDir() const { return dir() / "obj"; }
7685
};
7786

78-
// File names (basenames for BMIs, output-relative paths for objects) belonging
79-
// to one package's cache entry.
87+
// One cached object file. The two addresses are deliberately separate
88+
// (mcpp#344):
89+
//
90+
// cacheRel — where it lives INSIDE the entry (`<entry>/obj/<cacheRel>`).
91+
// Must be a pure function of the package, because the key
92+
// deliberately excludes the consumer. plan.cppm derives it.
93+
// buildRel — where THIS build produces it (`<buildDir>/<buildRel>`).
94+
// Consumer-side and therefore not recordable: two consumers of
95+
// one entry may legitimately place the same object at different
96+
// build-dir paths.
97+
//
98+
// Collapsing the two — recording the consumer's path as the entry's address —
99+
// is exactly what made the second consumer of an entry fail with ninja's
100+
// "missing and no known rule to make it". Only `cacheRel` ever reaches
101+
// entry.json; `buildRel` is populate-time input and is empty on read-back.
102+
struct ObjArtifact {
103+
std::string cacheRel;
104+
std::filesystem::path buildRel;
105+
};
106+
107+
// The artifacts belonging to one package's cache entry: BMI basenames plus the
108+
// objects above.
80109
struct DepArtifacts {
81-
std::vector<std::string> bmiFiles;
82-
std::vector<std::string> objFiles;
110+
std::vector<std::string> bmiFiles;
111+
std::vector<ObjArtifact> objFiles;
83112
};
84113

85-
// True when entry.json exists, its schema matches, its recorded inputs equal
86-
// `key.inputs` field for field, and every listed file is present on disk.
87-
bool is_cached(const CacheKey& key);
114+
// Why an entry could not serve this build.
115+
struct CacheProbe {
116+
bool ok = false;
117+
// Non-empty ONLY when the entry itself validated (schema, key, inputs) but
118+
// does not carry the artifacts THIS build asked for. After mcpp#344 that
119+
// shape should be unreachable, which is precisely why it must be reported
120+
// rather than silently folded into "miss": a systematic recurrence would
121+
// otherwise present as "the cache simply never hits", with no signal at
122+
// all — the same failure mode as the fake `Cached` that went unnoticed for
123+
// three months.
124+
std::vector<std::string> layoutMismatch;
125+
};
126+
127+
// Validate an entry AGAINST WHAT THIS BUILD WILL ACTUALLY READ.
128+
//
129+
// A hit requires all of: entry.json exists, its schema matches, its recorded
130+
// key matches, its recorded inputs equal `key.inputs` field for field, and
131+
// every artifact in `requested` is both listed by the entry and present on
132+
// disk. Checking only the entry's OWN file list — which is what this used to
133+
// do — validates a different question than the one the caller goes on to ask,
134+
// and the two answers diverged the moment the object layout stopped being a
135+
// function of the package alone.
136+
//
137+
// Anything short of a full match is a MISS. This function must never be the
138+
// reason a build fails: an unusable entry costs a recompile, and the staging
139+
// edges that would read it are never emitted.
140+
CacheProbe probe_cached(const CacheKey& key, const DepArtifacts& requested);
141+
142+
// probe_cached(...).ok
143+
bool is_cached(const CacheKey& key, const DepArtifacts& requested);
88144

89145
// The artifact list of a validated entry. Does NOT copy anything: the ninja
90146
// backend stages cached files through its own `stage_file` edges, so that a
@@ -137,12 +193,15 @@ std::optional<nlohmann::json> read_entry(const std::filesystem::path& p) {
137193
return j;
138194
}
139195

196+
// Read-back fills `cacheRel` only: `buildRel` is consumer-side and is not — and
197+
// must not be — recorded in the entry.
140198
DepArtifacts artifacts_from(const nlohmann::json& j) {
141199
DepArtifacts a;
142200
if (auto it = j.find("bmi"); it != j.end() && it->is_array())
143201
for (auto& v : *it) if (v.is_string()) a.bmiFiles.push_back(v.get<std::string>());
144202
if (auto it = j.find("obj"); it != j.end() && it->is_array())
145-
for (auto& v : *it) if (v.is_string()) a.objFiles.push_back(v.get<std::string>());
203+
for (auto& v : *it)
204+
if (v.is_string()) a.objFiles.push_back({v.get<std::string>(), {}});
146205
return a;
147206
}
148207

@@ -199,21 +258,39 @@ std::filesystem::path cached_obj_path(const CacheKey& key, std::string_view rel)
199258
return key.objDir() / std::filesystem::path(std::string(rel));
200259
}
201260

202-
bool is_cached(const CacheKey& key) {
261+
CacheProbe probe_cached(const CacheKey& key, const DepArtifacts& requested) {
262+
CacheProbe probe;
203263
auto j = read_entry(key.entryFile());
204-
if (!j) return false;
205-
if (j->value("schema", 0) != kEntrySchema) return false;
206-
if (j->value("key", std::string{}) != key.keyHex) return false;
264+
if (!j) return probe;
265+
if (j->value("schema", 0) != kEntrySchema) return probe;
266+
if (j->value("key", std::string{}) != key.keyHex) return probe;
207267
auto it = j->find("inputs");
208-
if (it == j->end() || !inputs_match(*it, key.inputs)) return false;
268+
if (it == j->end() || !inputs_match(*it, key.inputs)) return probe;
269+
270+
// The entry itself is valid. From here on, every remaining check is about
271+
// whether it holds what THIS build is going to read.
272+
auto recorded = artifacts_from(*j);
273+
std::set<std::string> haveBmi(recorded.bmiFiles.begin(), recorded.bmiFiles.end());
274+
std::set<std::string> haveObj;
275+
for (auto& o : recorded.objFiles) haveObj.insert(o.cacheRel);
209276

210-
auto arts = artifacts_from(*j);
211277
std::error_code ec;
212-
for (auto& g : arts.bmiFiles)
213-
if (!std::filesystem::exists(cached_bmi_path(key, g), ec)) return false;
214-
for (auto& o : arts.objFiles)
215-
if (!std::filesystem::exists(cached_obj_path(key, o), ec)) return false;
216-
return true;
278+
for (auto& g : requested.bmiFiles) {
279+
if (!haveBmi.contains(g)
280+
|| !std::filesystem::exists(cached_bmi_path(key, g), ec))
281+
probe.layoutMismatch.push_back(g);
282+
}
283+
for (auto& o : requested.objFiles) {
284+
if (!haveObj.contains(o.cacheRel)
285+
|| !std::filesystem::exists(cached_obj_path(key, o.cacheRel), ec))
286+
probe.layoutMismatch.push_back(o.cacheRel);
287+
}
288+
probe.ok = probe.layoutMismatch.empty();
289+
return probe;
290+
}
291+
292+
bool is_cached(const CacheKey& key, const DepArtifacts& requested) {
293+
return probe_cached(key, requested).ok;
217294
}
218295

219296
std::expected<DepArtifacts, std::string> resolve_cached(const CacheKey& key) {
@@ -250,7 +327,6 @@ populate_from(const CacheKey& key,
250327
std::filesystem::create_directories(cacheObj, ec);
251328

252329
auto projectBmi = projectTargetDir / key.bmiDirName;
253-
auto projectObj = projectTargetDir / "obj";
254330

255331
for (auto& g : arts.bmiFiles) {
256332
auto from = projectBmi / g;
@@ -263,15 +339,20 @@ populate_from(const CacheKey& key,
263339
"populate bmi '{}': {}", g, ec.message()));
264340
}
265341
}
342+
// Read from `buildRel`, write at `cacheRel`. These are NOT the same path in
343+
// general (mcpp#344): the build-dir layout partitions objects by package,
344+
// the entry's layout is package-internal, and a source that sits outside its
345+
// package root is re-anchored for the entry. Deriving one from the other
346+
// here is what this split exists to prevent.
266347
for (auto& o : arts.objFiles) {
267-
auto from = projectObj / o;
268-
if (!std::filesystem::exists(from)) {
348+
auto from = projectTargetDir / o.buildRel;
349+
if (o.buildRel.empty() || !std::filesystem::exists(from)) {
269350
return std::unexpected(std::format(
270351
"expected build output missing: {}", from.string()));
271352
}
272-
if (!copy_one(from, cached_obj_path(key, o), ec)) {
353+
if (!copy_one(from, cached_obj_path(key, o.cacheRel), ec)) {
273354
return std::unexpected(std::format(
274-
"populate obj '{}': {}", o, ec.message()));
355+
"populate obj '{}': {}", o.cacheRel, ec.message()));
275356
}
276357
}
277358

@@ -289,7 +370,13 @@ populate_from(const CacheKey& key,
289370
j["tag"] = key.manifestTag;
290371
j["inputs"] = key.inputs;
291372
j["bmi"] = arts.bmiFiles;
292-
j["obj"] = arts.objFiles;
373+
// Only the entry-internal addresses. Recording the consumer's build path
374+
// here is mcpp#344 in one line.
375+
{
376+
auto objs = nlohmann::json::array();
377+
for (auto& o : arts.objFiles) objs.push_back(o.cacheRel);
378+
j["obj"] = std::move(objs);
379+
}
293380
j["accessed"] = now_iso8601();
294381
return write_entry(key.entryFile(), j);
295382
}

src/bmi_cache/maintenance.cppm

Lines changed: 17 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -177,9 +177,24 @@ void check_pkg_files(Entry& e, const nlohmann::json& j) {
177177
if (auto it = j.find("obj"); it != j.end() && it->is_array()) {
178178
for (auto& v : *it) {
179179
if (!v.is_string()) continue;
180-
if (missing(e.dir / "obj" / v.get<std::string>())) {
180+
auto rel = v.get<std::string>();
181+
// mcpp#344: an entry's obj addresses must be package-internal —
182+
// downward, relative, no drive letter. An address that escapes the
183+
// entry is one that was derived from some consumer's build tree,
184+
// which is precisely the defect that made two consumers of one key
185+
// disagree about the layout. Report it here so the invariant is
186+
// auditable offline rather than only observable as a build that
187+
// dies in ninja's graph phase.
188+
if (rel.empty() || rel.starts_with("/") || rel.starts_with("..")
189+
|| rel.find(':') != std::string::npos) {
181190
e.complete = false;
182-
e.problem = std::format("missing obj/{}", v.get<std::string>());
191+
e.problem = std::format(
192+
"obj address is not package-internal: '{}'", rel);
193+
return;
194+
}
195+
if (missing(e.dir / "obj" / rel)) {
196+
e.complete = false;
197+
e.problem = std::format("missing obj/{}", rel);
183198
return;
184199
}
185200
}

src/build/cache_key.cppm

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -64,7 +64,13 @@ export namespace mcpp::build::cache_key {
6464
// Deliberately NOT the mcpp release number: folding the whole version in
6565
// orphaned every entry on every release, including plain C object files whose
6666
// validity has nothing to do with mcpp's version.
67-
inline constexpr int kCacheEpoch = 1;
67+
// 2 (mcpp#344): the artifact layout changed. An entry's obj addresses are now
68+
// package-internal instead of "the first consumer's build-dir path minus
69+
// `obj/`", so entries written by an older mcpp describe a layout this one does
70+
// not ask for. They would all miss anyway (probe_cached compares the REQUESTED
71+
// artifacts), but sharing a directory between two layouts makes `cache gc`'s
72+
// size accounting and `cache verify`'s output meaningless.
73+
inline constexpr int kCacheEpoch = 2;
6874

6975
// Axes A/B/C — identical for every package in one build, computed once.
7076
struct BuildAxes {

0 commit comments

Comments
 (0)