Skip to content

Commit 5ac794b

Browse files
committed
fix(build): sequence staged artifacts before compilation, and stop hiding std entries
Three fixes from the first CI round. Module partitions. Replacing a package's compile edges with stage edges also removes the ordering those compile edges carried. A consumer that imports `pkg` has `pkg`'s BMI in its dyndep and nothing else — the partition BMI `pkg:part` was reached only because `pkg`'s own compile edge depended on it. With independent stage edges ninja may start the consumer while the partition is still unstaged: error: failed to find module file for module 'mcpplibs.cmdline:options' macOS CI hit it; Linux won the race, which is why it is now an invariant rather than a scheduling accident. Every staged artifact becomes an ORDER-ONLY prerequisite of every non-staged edge, aggregated through one phony so no edge repeats the list (mcpp#274: long ninja lines are how a 50781-character command line blew past cmd.exe's 8191 limit). Order-only is the right strength — the real content dependencies are still declared where they always were, so a changed BMI still invalidates its consumers; this adds sequencing, not dirtiness. Verified locally on both GCC and Clang against mcpplibs.cmdline, which has a `:options` partition. Ages were computed against the wrong epoch. file_time_type is std::chrono::file_clock, whose epoch is not the Unix epoch, so `cache list` printed "74509d ago". Converted through clock_cast. Test fallout, all of it real: - 22_doctor_cache_publish asserted `cache list` was empty after `mcpp self doctor`, which precompiles a std module. That assertion only held because the old `cache list` walked dep entries and skipped std ones — and hiding them is how 16 GB of duplicated std BMIs went unnoticed. The empty-cache check moved ahead of doctor; the std entry is now asserted to be visible, with a bound on the age column that would have caught the epoch bug. - 40_llvm_bmi_cache used `--no-cache` to force a cold first build, then expected the second build to reuse it. `--no-cache` is now an alias for `--cache=off`, which means neither read NOR write — so it left nothing to reuse. The test's actual intent (fresh MCPP_HOME is already cold) is now what it says, and it additionally asserts zero compile edges and the partition sequencing. The semantic tightening is called out in the CHANGELOG: a mode named `off` that still writes the cache would not be defensible. - 98_reflection_import_std grepped $MCPP_HOME/bmi, the pre-v1 root. It passed locally purely because this machine still holds 26 GB of legacy entries, one of which happened to record -freflection — a false green of exactly the kind the plan warned about. It and two siblings now select the std entry by CONTENT instead of `find | head -1`, which is no longer well-defined: one MCPP_HOME can hold several std identities now, and that is the point.
1 parent bbd2eeb commit 5ac794b

12 files changed

Lines changed: 227 additions & 43 deletions

CHANGELOG.md

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -63,6 +63,8 @@
6363

6464
`--no-cache` 保留为 `off` 的兼容别名。它的旧 help 文案「Force-clear target/ before building」两处不准:它清的是**构建目录**(`target/<triple>/<fp>/`)而不是整个 `target/`,而且名字与缓存无关。`mcpp run` / `mcpp test` 一并补上这两个 flag(此前它们连 `--no-cache` 都没有)。
6565

66+
⚠️ **`--no-cache` 的语义有一处收紧**:此前它只清构建目录、**仍然会回填全局缓存**;现在它等于 `off`,即**不读也不写**。想要「从零重编但仍然刷新缓存」的,用 `mcpp clean``rm -rf target` 后正常构建。这个收紧是为了让三个模式正交:一个叫 `off` 的模式还偷偷写缓存是说不通的。
67+
6668
- **`mcpp cache` 补齐到可运维。** `cache dir`(缓存到底在哪 —— 此前 `cache *`/`doctor`/`clean --bmi-cache` 各自解析根目录,而 config 的 reset 路径用 `GlobalConfig::bmiCacheDir`,两者可能不是同一个目录)、`cache gc --max-size <N>{MiB,GiB} / --older-than <N>{s,m,h,d}`(**真 LRU**)、`cache clean --deps|--std|--all|--legacy``cache list --json``cache verify`(逐条目校验清单与磁盘,残缺条目非零退出)。`cache info` 现在打印该条目的键输入 —— 怀疑命中错了时第一件想看的东西。
6769

6870
`prune` 此前按**目录 mtime** 排序,而那只记录条目被**写入**的时间:一个每次构建都命中的热包,和一个一个月没人碰过的冷包一样「陈旧」。`entry.json``accessed` 由每次命中刷新(只重写 `entry.json`,**不动产物 mtime** —— 那些 mtime 参与 ninja 的 restat 判定),`gc` 按它排序。`cache clean` 开头那句 `remove_all(<root>/"deps")` 指向一个从不存在的路径(dep 条目在 `<root>/<fp>/deps`),是死代码。

src/bmi_cache/maintenance.cppm

Lines changed: 9 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -128,14 +128,20 @@ std::int64_t stamp_of(const nlohmann::json& j, const char* field) {
128128
return 0;
129129
}
130130

131-
// mtime fallback for entries whose stamp is missing (written by an mcpp that
132-
// predates `accessed`). Reported as such rather than silently treated as fresh.
131+
// mtime fallback for entries with no `accessed` stamp (std entries, and package
132+
// entries written by an mcpp that predates the field).
133+
//
134+
// file_time_type is std::chrono::file_clock, whose epoch is NOT the Unix epoch —
135+
// on libstdc++ it is 1970 shifted, so reading time_since_epoch() and comparing
136+
// it against system_clock produced ages like "74509d ago". Convert through the
137+
// clock rather than assuming a shared epoch.
133138
std::int64_t dir_mtime_seconds(const std::filesystem::path& p) {
134139
std::error_code ec;
135140
auto t = std::filesystem::last_write_time(p, ec);
136141
if (ec) return 0;
142+
auto sys = std::chrono::clock_cast<std::chrono::system_clock>(t);
137143
return std::chrono::duration_cast<std::chrono::seconds>(
138-
t.time_since_epoch()).count();
144+
sys.time_since_epoch()).count();
139145
}
140146

141147
void measure(Entry& e) {

src/build/ninja_backend.cppm

Lines changed: 49 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -872,6 +872,11 @@ std::string emit_ninja_string(const BuildPlan& plan) {
872872
append("\n");
873873
}
874874

875+
// Aggregate target for everything staged out of the global cache. Named
876+
// with a leading underscore so it cannot collide with a module or target
877+
// name (both of which are identifiers or paths).
878+
constexpr std::string_view kStagedCachePhony = "_mcpp_staged_cache";
879+
875880
auto bmi_path = [&traits](std::string_view name) {
876881
std::string s(traits.bmiDir);
877882
s += '/';
@@ -916,24 +921,55 @@ std::string emit_ninja_string(const BuildPlan& plan) {
916921
// this package's own config and its dependencies' keys, so for a given key
917922
// equal size IS equivalence — and size comes from directory metadata, which
918923
// stays readable even when a holder denies opening the file.
924+
//
925+
// ORDERING. Replacing a package's compile edges with stage edges also
926+
// removes the ordering those compile edges carried. A module partition is
927+
// the case that breaks: a consumer imports `pkg`, so its dyndep declares
928+
// `pkg`'s BMI and nothing else — the partition BMI `pkg:part` was reached
929+
// only because `pkg`'s own compile edge depended on it. With independent
930+
// stage edges, ninja may finish staging `pkg` and start the consumer while
931+
// `pkg:part` is still unstaged, and Clang then fails with
932+
// `failed to find module file for module 'pkg:part'`. (Observed on macOS
933+
// CI; Linux happened to win the race, which is exactly why this is stated
934+
// as an invariant rather than left to scheduling.)
935+
//
936+
// So every staged artifact becomes an ORDER-ONLY prerequisite of every edge
937+
// that is not itself staged. Order-only (`||`) is the right strength: the
938+
// real content dependencies are still declared where they always were (a
939+
// dyndep-supplied implicit input, or the static-mode implicit list), so a
940+
// changed BMI still invalidates its consumers — this adds sequencing, not
941+
// dirtiness. The cost is that a handful of copies finish before compilation
942+
// starts, which is what used to happen anyway when those units were built.
943+
std::string stagedOrderOnly;
919944
{
920-
bool any = false;
945+
std::vector<std::string> staged;
921946
for (auto& cu : plan.compileUnits) {
922947
if (!cu.servedFromCache) continue;
923948
if (cu.cachedObject.empty()) continue;
924-
any = true;
925-
append(std::format("build {} : stage_file {}\n",
926-
escape_ninja_path(cu.object),
949+
auto obj = escape_ninja_path(cu.object);
950+
append(std::format("build {} : stage_file {}\n", obj,
927951
escape_ninja_path(cu.cachedObject)));
928952
append(" verify = --verify size\n");
953+
staged.push_back(obj);
929954
if (cu.providesModule && !cu.cachedBmi.empty()) {
930-
append(std::format("build {} : stage_file {}\n",
931-
bmi_path(*cu.providesModule),
955+
auto bmi = bmi_path(*cu.providesModule);
956+
append(std::format("build {} : stage_file {}\n", bmi,
932957
escape_ninja_path(cu.cachedBmi)));
933958
append(" verify = --verify size\n");
959+
staged.push_back(bmi);
934960
}
935961
}
936-
if (any) append("\n");
962+
if (!staged.empty()) {
963+
append("\n");
964+
// One phony aggregates them so each consuming edge names a single
965+
// prerequisite instead of repeating the whole list (mcpp#274: long
966+
// ninja lines are how a 50781-character command line blew past
967+
// cmd.exe's 8191 limit on Windows).
968+
append("build " + std::string(kStagedCachePhony) + " : phony");
969+
for (auto& s2 : staged) append(" " + s2);
970+
append("\n\n");
971+
stagedOrderOnly = " || " + std::string(kStagedCachePhony);
972+
}
937973
}
938974

939975
if (dyndep) {
@@ -955,8 +991,8 @@ std::string emit_ninja_string(const BuildPlan& plan) {
955991
continue;
956992
auto ddi = (cu.object.parent_path() / cu.source.filename()).string() + ".ddi";
957993
ddi_paths.push_back(ddi);
958-
append(std::format("build {} : cxx_scan {}\n", escape_ninja_path(ddi),
959-
escape_ninja_path(cu.source)));
994+
append(std::format("build {} : cxx_scan {}{}\n", escape_ninja_path(ddi),
995+
escape_ninja_path(cu.source), stagedOrderOnly));
960996
append(std::format(" compile_target = {}\n", escape_ninja_path(cu.object)));
961997
if (auto includes = local_include_flags(cu, msvcDeps); !includes.empty())
962998
append(std::format(" local_includes ={}\n", includes));
@@ -1027,17 +1063,18 @@ std::string emit_ninja_string(const BuildPlan& plan) {
10271063
auto it = ddi_to_dd.find(ddi);
10281064
if (it != ddi_to_dd.end()) {
10291065
out_line += " | " + it->second;
1066+
out_line += stagedOrderOnly;
10301067
out_line += "\n dyndep = " + it->second;
10311068
// P2: set bmi_out for the copy_if_different logic in cxx_module.
10321069
if (cu.providesModule) {
10331070
out_line += "\n bmi_out = " + bmi_path(*cu.providesModule);
10341071
}
10351072
out_line += "\n";
10361073
} else {
1037-
out_line += "\n";
1074+
out_line += stagedOrderOnly + "\n";
10381075
}
10391076
} else {
1040-
out_line += "\n";
1077+
out_line += stagedOrderOnly + "\n";
10411078
}
10421079
if (auto includes = local_include_flags(cu, msvcDeps); !includes.empty())
10431080
out_line += " local_includes =" + includes + "\n";
@@ -1089,6 +1126,7 @@ std::string emit_ninja_string(const BuildPlan& plan) {
10891126
out_line += std::format(" : {} {}", rule, escape_ninja_path(cu.source));
10901127
if (!implicit.empty())
10911128
out_line += " |" + implicit;
1129+
out_line += stagedOrderOnly;
10921130
out_line += "\n";
10931131
if (auto includes = local_include_flags(cu, msvcDeps); !includes.empty())
10941132
out_line += " local_includes =" + includes + "\n";

src/cli.cppm

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -54,7 +54,7 @@ void print_usage() {
5454
std::println(" mcpp build [options] Build the current package");
5555
std::println(" mcpp run [target] [-- args...] Build + run a binary target");
5656
std::println(" mcpp test [pattern] [-- args...] Build + run tests/**/*.cpp (--list, --timeout, --message-format json)");
57-
std::println(" mcpp clean [--bmi-cache] Remove target/ (and optionally BMI cache)");
57+
std::println(" mcpp clean [--bmi-cache] Remove target/ (and optionally the build cache)");
5858
std::println(" mcpp add <pkg>[@<ver>] Add a dependency to mcpp.toml");
5959
std::println(" mcpp remove <pkg> Remove a dependency from mcpp.toml");
6060
std::println(" mcpp update [pkg] Re-resolve deps and rewrite mcpp.lock");
@@ -286,8 +286,8 @@ int run(int argc, char** argv) {
286286
return cmd_test(p, std::span<const std::string>(passthrough));
287287
})))
288288
.subcommand(cl::App("clean")
289-
.description("Remove target/ (and optionally the global BMI cache)")
290-
.option(cl::Option("bmi-cache").help("Also wipe the global BMI cache"))
289+
.description("Remove target/ (and optionally the global build cache)")
290+
.option(cl::Option("bmi-cache").help("Also wipe the global build cache (see `mcpp cache clean`)"))
291291
.action(wrap_rc(cmd_clean)))
292292
.subcommand(cl::App("why")
293293
.description("Explain how the toolchain / runtime / deps were resolved")

src/config.cppm

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,8 @@
66
// registry/ XLINGS_HOME for mcpp's xlings
77
// bin/xlings vendored xlings binary (= <XLINGS_HOME>/bin/xlings)
88
// .xlings.json seeded with index_repos = [mcpplibs]
9-
// bmi/<fp>/ BMI cache (existing)
9+
// build-cache/v1/ cross-project build cache (pkg/ + std/)
10+
// bmi/ pre-v1 build cache; unused, `cache clean --legacy`
1011
// cache/ metadata caches
1112
// config.toml this module's input
1213
//

src/doctor.cppm

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -461,10 +461,11 @@ export int explain_code(std::string_view code) {
461461
"The [toolchain] pin in mcpp.toml does not match the detected toolchain.\n"
462462
"Either install the pinned toolchain (xlings install ...) or relax the\n"
463463
"pin (e.g. \"gcc@>=15\" instead of \"gcc@15.1.0\")."},
464-
{"E0005", "BMI cache corruption",
465-
"A cached BMI file referenced by manifest.txt is missing on disk. Run\n"
466-
"`mcpp cache prune --older-than 0d` to drop stale entries; the next build\n"
467-
"will repopulate."},
464+
{"E0005", "build cache corruption",
465+
"A file listed in a cache entry's entry.json is missing on disk. Such an\n"
466+
"entry is treated as a miss and rebuilt, so this is never wrong output —\n"
467+
"only wasted space. `mcpp cache verify` lists every affected entry and\n"
468+
"`mcpp cache gc --older-than 0s` reclaims them."},
468469
{"E0006", "index requires a newer mcpp",
469470
"The package index declares (index.toml [index].min_mcpp) that its\n"
470471
"descriptors need a newer mcpp than this binary — parsing them would\n"

tests/e2e/100_cppfly_reflection.sh

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -104,10 +104,12 @@ out=$("$binary")
104104
}
105105

106106
# The std BMI prebuild carries the same dialect (scan/prebuild/compile agree).
107-
metadata="$(find "$MCPP_HOME/build-cache/v1/std" -name std-module.json | head -1)"
108-
grep -q '"std_flag": "-std=c++26 -freflection' "$metadata" || {
109-
cat "$metadata"
110-
echo "FAIL: std-module.json std_flag lacks -std=c++26 -freflection"
107+
# Recursive grep, not `find | head -1`: one MCPP_HOME can hold several std
108+
# identities now, so "the first one" is arbitrary.
109+
grep -rl '"std_flag": "-std=c++26 -freflection' "$MCPP_HOME/build-cache/v1/std" >/dev/null 2>&1 || {
110+
find "$MCPP_HOME/build-cache/v1/std" -name std-module.json \
111+
-exec grep -H '"std_flag"' {} \; 2>/dev/null
112+
echo "FAIL: no std-module.json records std_flag -std=c++26 -freflection"
111113
exit 1
112114
}
113115

tests/e2e/22_doctor_cache_publish.sh

Lines changed: 25 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,14 @@ TMP=$(mktemp -d)
1111
trap "rm -rf $TMP" EXIT
1212
export MCPP_HOME="$TMP/mcpp-home"
1313

14+
# 0) cache list on a genuinely empty cache: friendly message.
15+
# This has to come BEFORE doctor — doctor resolves a build plan, which
16+
# precompiles the std module, and `cache list` now reports std entries too.
17+
# Hiding them is how 16 GB of duplicated std BMIs went unnoticed, so listing
18+
# them is the fix, not a regression.
19+
out=$("$MCPP" cache list 2>&1)
20+
[[ "$out" == *"empty"* ]] || { echo "cache list on empty cache: '$out'"; exit 1; }
21+
1422
# 1) doctor: should always run, exit 0 or 1 (warn ok), never 2
1523
rc=0
1624
"$MCPP" self doctor > doctor.log 2>&1 || rc=$?
@@ -21,9 +29,24 @@ grep -q 'Checking registry' doctor.log || { cat doctor.log; echo "no regis
2129
grep -q 'Checking cache health' doctor.log || { cat doctor.log; echo "no cache check"; exit 1; }
2230
grep -q 'Doctor result' doctor.log || { cat doctor.log; echo "no result line"; exit 1; }
2331

24-
# 2) cache list (empty): friendly message
32+
# 2) after doctor, the std module it precompiled must be VISIBLE. std entries
33+
# dominate the cache's size, and `cache list` used to walk only dep entries.
2534
out=$("$MCPP" cache list 2>&1)
26-
[[ "$out" == *"empty"* ]] || { echo "cache list empty: '$out'"; exit 1; }
35+
[[ "$out" == *"std"* ]] || { echo "cache list omits the std entry: '$out'"; exit 1; }
36+
# The age column must be plausible: file_time_type's epoch is not the Unix
37+
# epoch, and reading it as if it were printed ages like "74509d ago".
38+
if [[ "$out" =~ ([0-9]+)d\ ago ]] && (( BASH_REMATCH[1] > 3650 )); then
39+
echo "cache list reports an implausible age (clock epoch bug): '$out'"
40+
exit 1
41+
fi
42+
43+
# 2b) cache dir must point at the cache the rest of the tool uses.
44+
out=$("$MCPP" cache dir 2>&1)
45+
[[ "$out" == "$MCPP_HOME/build-cache/v1"* ]] || {
46+
echo "cache dir '$out' != '$MCPP_HOME/build-cache/v1'"; exit 1; }
47+
48+
# 2c) verify must pass on a healthy cache.
49+
"$MCPP" cache verify > /tmp/_v.log 2>&1 || { cat /tmp/_v.log; echo "verify failed"; exit 1; }
2750

2851
# 3) publish dry-run on a fresh package. Publish uses `git archive` for the
2952
# source tarball, so we git-init + commit first. We also need a non-empty

tests/e2e/40_llvm_bmi_cache.sh

Lines changed: 31 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -60,22 +60,45 @@ int main() {
6060
}
6161
EOF
6262

63-
# First build — should compile the dependency
64-
out1=$("$MCPP" build --no-cache 2>&1)
63+
# First build — populates the cache. Deliberately NOT `--no-cache`: that is now
64+
# an alias for `--cache=off`, which means neither read NOR write, so a build that
65+
# used it would leave nothing for the second build to reuse. MCPP_HOME is fresh
66+
# here, so this build is already cold.
67+
out1=$("$MCPP" build 2>&1)
6568
echo "$out1" | grep -q "Compiling.*mcpplibs.cmdline" || {
66-
# It's OK if it says "Cached" because global cache may exist
67-
echo "$out1" | grep -q "Cached.*mcpplibs.cmdline" || {
68-
echo "FAIL: mcpplibs.cmdline not mentioned in first build: $out1"
69-
exit 1
70-
}
69+
echo "FAIL: mcpplibs.cmdline not compiled in the first (cold) build: $out1"
70+
exit 1
7171
}
7272

73-
# Second build (clean target, keep BMI cache) — dependency should be cached
73+
# Second build, clean target dir, cache kept — the dependency must be reused.
7474
rm -rf target
7575
out2=$("$MCPP" build 2>&1)
7676
echo "$out2" | grep -q "Cached.*mcpplibs.cmdline" || {
7777
echo "FAIL: mcpplibs.cmdline not cached on second build: $out2"
7878
exit 1
7979
}
8080

81+
# ...and reuse must mean "not recompiled". The status line alone used to be
82+
# printed while ninja rebuilt every unit behind it, so assert on the graph.
83+
NINJA="$(find target -name build.ninja | head -1)"
84+
[[ -n "$NINJA" ]] || { echo "FAIL: no build.ninja"; exit 1; }
85+
if grep -qE ': (cxx_module|cxx_object|cxx_scan) .*mcpplibs' "$NINJA"; then
86+
echo "FAIL: cached dependency still has compile edges"
87+
grep -nE ': (cxx_module|cxx_object|cxx_scan) .*mcpplibs' "$NINJA" | head
88+
exit 1
89+
fi
90+
91+
# Clang + module partitions: mcpplibs.cmdline has a `:options` partition, which a
92+
# consumer never imports directly. Its stage edge must be sequenced before the
93+
# consumer's compile, or clang fails with `failed to find module file for module
94+
# 'mcpplibs.cmdline:options'` — a race Linux won and macOS lost.
95+
grep -q '_mcpp_staged_cache' "$NINJA" || {
96+
echo "FAIL: staged artifacts are not sequenced before compilation"
97+
exit 1
98+
}
99+
100+
# And the whole thing must actually link and run.
101+
out3=$("$MCPP" run 2>&1) || { echo "FAIL: run: $out3"; exit 1; }
102+
echo "$out3" | grep -q 'cache test ok' || { echo "FAIL: run output: $out3"; exit 1; }
103+
81104
echo "OK"

tests/e2e/59_cpp_standard_config.sh

Lines changed: 8 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -71,11 +71,14 @@ if grep -q -- "-std=c++23" compile_commands.json; then
7171
exit 1
7272
fi
7373

74-
metadata="$(find "$MCPP_HOME/build-cache/v1/std" -name std-module.json | head -1)"
75-
[[ -n "$metadata" ]] || { echo "FAIL: std module metadata missing"; exit 1; }
76-
grep -q '"cpp_standard": "c++26"' "$metadata" || {
77-
echo "FAIL: std module metadata missing C++26 standard"
78-
cat "$metadata"
74+
# The entry recording c++26, selected by CONTENT rather than by `find | head -1`:
75+
# one MCPP_HOME can hold several std identities now.
76+
metadata="$(grep -rl '"cpp_standard": "c++26"' \
77+
"$MCPP_HOME/build-cache/v1/std" 2>/dev/null | head -1)"
78+
[[ -n "$metadata" ]] || {
79+
echo "FAIL: no std module metadata records c++26"
80+
find "$MCPP_HOME/build-cache/v1/std" -name std-module.json \
81+
-exec grep -H '"cpp_standard"' {} \; 2>/dev/null
7982
exit 1
8083
}
8184
grep -q '"std_flag": "-std=c++26"' "$metadata" || {

0 commit comments

Comments
 (0)