From 395c5ff1e4b723c4a112079111f7dc3c590f2e0b Mon Sep 17 00:00:00 2001 From: speak-agent <248744407+speak-agent@users.noreply.github.com> Date: Wed, 30 Sep 2026 12:08:00 +0800 Subject: [PATCH 01/15] build: state each planning phase's duration under build/stage Every phase of prepare_build logs its duration, as the backend's own steps do, whenever the log file or --verbose would show it. The backend's stage lines are recorded under the same condition, not only under --verbose. A planned edit of one source spent 3.05 s before ninja that could only be attributed from gaps between unrelated log lines. --- src/build/ninja_backend.cppm | 5 +++- src/build/prepare/driver.cpp | 50 ++++++++++++++++++++++++++++-------- 2 files changed, 43 insertions(+), 12 deletions(-) diff --git a/src/build/ninja_backend.cppm b/src/build/ninja_backend.cppm index 3e6a01e6..7226b488 100644 --- a/src/build/ninja_backend.cppm +++ b/src/build/ninja_backend.cppm @@ -3862,7 +3862,10 @@ std::expected NinjaBackend::build(const BuildPlan& plan // because the only number reported is the total. auto tStage = t0; auto stage = [&](std::string_view what) { - if (!mcpp::log::is_verbose()) { tStage = std::chrono::steady_clock::now(); return; } + if (!mcpp::log::is_verbose() && !mcpp::log::is_enabled(mcpp::log::Level::info)) { + tStage = std::chrono::steady_clock::now(); + return; + } auto now = std::chrono::steady_clock::now(); auto ms = std::chrono::duration_cast(now - tStage).count(); tStage = now; diff --git a/src/build/prepare/driver.cpp b/src/build/prepare/driver.cpp index f8b32e4a..90d56468 100644 --- a/src/build/prepare/driver.cpp +++ b/src/build/prepare/driver.cpp @@ -21,6 +21,7 @@ import mcpp.build.build_program; import mcpp.build.backend; // BuildOptions for the tool sub-build import mcpp.build.ninja; // make_ninja_backend — driving that sub-build import mcpp.platform; +import mcpp.log; namespace mcpp::build { @@ -59,21 +60,48 @@ prepare_build(bool print_fingerprint, return std::unexpected(std::move(message)); }; - if (auto r = phase0_manifest_and_workspace(state); !r) return fail(r.error()); + // WHERE PLANNING'S TIME GOES: each phase states its duration under + // `build/stage`, as the backend's own steps do, whenever the log file or + // --verbose would show it. Planning had no such record, and a planned + // edit of one source spent 3.05 s before ninja that could only be + // attributed from gaps between unrelated log lines (.agents/docs/ + // 2026-09-30-build-wall-time-progress-count-and-hang-plan.md, W9). + auto timed = [&](std::string_view phase, auto&& run) { + if (!mcpp::log::is_verbose() && !mcpp::log::is_enabled(mcpp::log::Level::info)) + return run(); + const auto t0 = std::chrono::steady_clock::now(); + auto r = run(); + const auto ms = std::chrono::duration_cast( + std::chrono::steady_clock::now() - t0).count(); + mcpp::log::verbose("build/stage", std::format("plan {}: {}ms", phase, ms)); + return r; + }; + + if (auto r = timed("manifest", [&] { return phase0_manifest_and_workspace(state); }); !r) + return fail(r.error()); if (auto r = check_engine_floors(state, /*rootOnly=*/true); !r) return fail(r.error()); - if (auto r = phase1_toolchain_spec_and_axes(state); !r) return fail(r.error()); - if (auto r = phase2_define_toolchain_resolver(state); !r) return fail(r.error()); - if (auto r = phase3_xlings_before_graph(state); !r) return fail(r.error()); - if (auto r = phase4a_graph_load(state); !r) return fail(r.error()); - if (auto r = phase4b_graph_worklist(state); !r) return fail(r.error()); + if (auto r = timed("toolchain request", [&] { return phase1_toolchain_spec_and_axes(state); }); !r) + return fail(r.error()); + if (auto r = timed("toolchain resolver", [&] { return phase2_define_toolchain_resolver(state); }); !r) + return fail(r.error()); + if (auto r = timed("xlings", [&] { return phase3_xlings_before_graph(state); }); !r) + return fail(r.error()); + if (auto r = timed("graph load", [&] { return phase4a_graph_load(state); }); !r) + return fail(r.error()); + if (auto r = timed("graph", [&] { return phase4b_graph_worklist(state); }); !r) + return fail(r.error()); if (auto r = check_engine_floors(state, /*rootOnly=*/false); !r) return fail(r.error()); - if (auto r = phase5_toolchain_after_graph(state); !r) return fail(r.error()); - if (auto r = phase6_features_and_host_tools(state); !r) return fail(r.error()); - if (auto r = phase9_target_side(state); !r) return fail(r.error()); - if (auto r = phase11_scan(state); !r) return fail(r.error()); + if (auto r = timed("toolchain", [&] { return phase5_toolchain_after_graph(state); }); !r) + return fail(r.error()); + if (auto r = timed("features and host tools", [&] { return phase6_features_and_host_tools(state); }); !r) + return fail(r.error()); + if (auto r = timed("target side", [&] { return phase9_target_side(state); }); !r) + return fail(r.error()); + if (auto r = timed("scan", [&] { return phase11_scan(state); }); !r) + return fail(r.error()); g_notesOnFailure.clear(); - return phase13_finish(state); + return timed("finish", [&] { return phase13_finish(state); }); } From e4778525116fa7af9b1ef2834a03b6677c858946 Mon Sep 17 00:00:00 2001 From: speak-agent <248744407+speak-agent@users.noreply.github.com> Date: Wed, 30 Sep 2026 12:08:00 +0800 Subject: [PATCH 02/15] ui: every frame of the stack animation returns A piece that lands with a cell outside the screen is not spawned, and a piece that adds no cell ends the fill loop, so every iteration grows the stack or ends the loop. Before, a stack whose holes reached the right edge short of the fraction's target spawned pieces onto cells it already held, the loop never ended while the ticker held the line lock, and the build hung after ninja. A property test drives every animation over 2000 seeds and every game over 500 under a watchdog. It fails on the previous stack animation (the stack leg does not return within 120 s) and passes in 4 s. --- src/ui/dots_screen/stack.cppm | 41 ++++++++++++++--- tests/unit/test_dots_screen.cpp | 80 +++++++++++++++++++++++++++++++++ 2 files changed, 115 insertions(+), 6 deletions(-) diff --git a/src/ui/dots_screen/stack.cppm b/src/ui/dots_screen/stack.cppm index b37f55ab..94f1dd05 100644 --- a/src/ui/dots_screen/stack.cppm +++ b/src/ui/dots_screen/stack.cppm @@ -12,6 +12,17 @@ namespace mcpp::ui::dots_screen { // stack where they leave the fewest holes; at most three fly at once, and a // burst of progress settles at once, so the stack's area follows the fraction // within four pieces (design §5.11). +// +// EVERY FILL LOOP GROWS THE STACK OR STOPS. The stack keeps holes, so it can +// reach the right edge with fewer cells than the fraction asks for. A piece +// that no longer fits inside the screen is therefore not spawned, and a piece +// that adds no cell ends the loop: termination follows from the loop, not +// from the shape of the stack. Before, such a piece landed at the right edge +// on cells an earlier one held, the map of cells stopped growing, and the +// loop never ended while the ticker held the line lock, so the build hung +// after ninja (mcpp 2026.9.30.1; .agents/docs/ +// 2026-09-30-build-wall-time-progress-count-and-hang-plan.md, F1). A full +// stack stays full until the build ends; the counts beside it state the build. class Stack final : public Animation { public: explicit Stack(std::uint64_t seed) : rnd_(seed) {} @@ -19,9 +30,18 @@ public: failed_ = in.failed; const auto target = static_cast(in.fraction * (kWidth - 6) * kHeight); if (!failed_) { - while (cells_.size() + 4 * flying_.size() + 16 <= target) lock(spawn()); - while (flying_.size() < 3 && cells_.size() + 4 * (flying_.size() + 1) <= target) - flying_.push_back(spawn()); + while (cells_.size() + 4 * flying_.size() + 16 <= target) { + auto piece = spawn(); + if (!piece) break; + const auto before = cells_.size(); + lock(*piece); + if (cells_.size() == before) break; + } + while (flying_.size() < 3 && cells_.size() + 4 * (flying_.size() + 1) <= target) { + auto piece = spawn(); + if (!piece) break; + flying_.push_back(std::move(*piece)); + } } const double speed = (5.0 + std::min(6.0, static_cast(in.finished) * 0.5)) * in.dt * 10; for (auto it = flying_.begin(); it != flying_.end();) { @@ -62,16 +82,22 @@ private: if (Cell{p.land + dx, dy} == c) return true; return false; } - int landing(const Shape& s) const { + // Where `s`, sliding in from the right, comes to rest; nothing when it + // would rest with a cell outside the screen. + std::optional landing(const Shape& s) const { int x = kWidth; auto fits = [&](int at) { for (auto [dx, dy] : s) if (taken({at + dx, dy})) return false; return true; }; while (x > 0 && fits(x - 1)) --x; + for (auto [dx, dy] : s) + if (x + dx >= kWidth) return std::nullopt; return x; } - Piece spawn() { + // The next piece at its best landing; nothing when no rotation and row of + // the chosen kind lands inside the screen. + std::optional spawn() { const auto& all = kinds(); const auto& [rotations, colour] = all[std::uniform_int_distribution(0, all.size() - 1)(rnd_)]; @@ -82,7 +108,9 @@ private: for (int oy = 0; oy + h <= kHeight; ++oy) { Shape s; for (auto [dx, dy] : r) s.push_back({dx, dy + oy}); - const int land = landing(s); + const auto landed = landing(s); + if (!landed) continue; + const int land = *landed; int front = 0, minx = kWidth; for (auto [dx, dy] : s) { front = std::max(front, land + dx); minx = std::min(minx, land + dx); } int holes = 0; @@ -94,6 +122,7 @@ private: if (!best || score < best->first) best = {score, Piece{s, colour, double(kWidth), land}}; } } + if (!best) return std::nullopt; return best->second; } void lock(const Piece& p) { diff --git a/tests/unit/test_dots_screen.cpp b/tests/unit/test_dots_screen.cpp index d116b9c3..f1bf2159 100644 --- a/tests/unit/test_dots_screen.cpp +++ b/tests/unit/test_dots_screen.cpp @@ -139,6 +139,86 @@ TEST(DotsScreen, TheChomperStandsAtTheFraction) { EXPECT_EQ(sc.at(kWidth - 5, 1), Colour::Yellow); } +// ─── Every frame returns (build wall-time plan, F1 and W1) ────────────── +// +// The ticker draws a frame while it holds the line lock, and the build joins +// the ticker when ninja ends: a frame that does not return hangs the build +// after it has finished. The stack animation did, in about 3% of builds that +// chose it (66 of 2000 seeded runs of this shape). The property is stated +// over seeds and input sequences, not by example, and a frame that never +// returns cannot be joined, so the watchdog ends the test binary. + +namespace { + +void returns_within(std::chrono::seconds limit, std::string what, + std::function body) { + auto done = std::make_shared>(); + auto finished = done->get_future(); + std::thread([body = std::move(body), done] { + body(); + done->set_value(); + }).detach(); + if (finished.wait_for(limit) != std::future_status::ready) { + ADD_FAILURE() << what << " did not return within " << limit.count() << " s"; + std::fflush(nullptr); + std::_Exit(1); + } +} + +// A build of `total` steps that finishes in bursts at ten frames a second, +// then `tail` more frames at its end; the failure input on one seed in eight. +void play_build(Animation& a, std::uint64_t seed, std::size_t total, int tail) { + std::mt19937_64 rnd(seed); + std::size_t done = 0; + const bool fails = seed % 8 == 7; + for (int frame = 0; done < total || tail-- > 0; ++frame) { + const std::size_t burst = rnd() % 7 == 0 ? rnd() % 12 : rnd() % 2; + const std::size_t before = done; + done = std::min(total, done + burst); + Input in; + in.dt = 0.1; + in.finished = done - before; + in.fraction = static_cast(done) / static_cast(total); + in.failed = fails && done * 2 > total; + a.update(in); + if (frame % 16 == 0) a.package(static_cast(rnd() % 6)); + } +} + +} // namespace + +TEST(DotsScreen, EveryAnimationReturnsFromEveryFrame) { + for (auto name : mcpp::ui::dots_screen::names()) { + const std::string n(name); + returns_within(std::chrono::seconds(120), "an animation '" + n + "'", [n] { + for (std::uint64_t seed = 0; seed < 2000; ++seed) { + auto a = make(n, seed); + play_build(*a, seed, 240, 120); + } + }); + } +} + +TEST(DotsScreenGames, EveryGameReturnsFromEveryFrame) { + for (auto name : game_names()) { + const std::string n(name); + returns_within(std::chrono::seconds(120), "a game '" + n + "'", [n] { + constexpr Key kKeys[] = {Key::Up, Key::Down, Key::Left, Key::Right, Key::Space}; + for (std::uint64_t seed = 0; seed < 500; ++seed) { + auto g = make_game(n, seed); + std::mt19937_64 rnd(seed); + Input in; + in.dt = 0.05; + for (int frame = 0; frame < 400; ++frame) { + if (rnd() % 3 == 0) g->key(kKeys[rnd() % 5]); + in.failed = frame > 300 && seed % 4 == 3; + g->update(in); + } + } + }); + } +} + // ─── The games of --play-game (design §5.14) ───────────────────────────── namespace { From 0ef96b45d7e0018e8562f710557fac4a7486053d Mon Sep 17 00:00:00 2001 From: speak-agent <248744407+speak-agent@users.noreply.github.com> Date: Wed, 30 Sep 2026 12:16:22 +0800 Subject: [PATCH 03/15] xlings: replace a vendored xlings from the newest source, once per process (#744) One function chooses the source: MCPP_VENDORED_XLINGS when set, otherwise the newer of the xlings released with this mcpp and the xlings on PATH, the released one on a tie. The check that decides whether to replace the vendored binary and the copy that replaces it both use its answer, so the version stated is the version copied. Before, the check took the first source that existed, so a released copy older than the pin hid a newer xlings on PATH and mcpp stated that no newer source was available. A home settled once in a process is not examined again, and Updating and Note are each stated at most once. The version of the vendored binary is asked once per process and kept under the home, keyed by the binary's path, size and modification time, so a command that loads the configuration no longer runs xlings --version (0.35 s measured) when the binary has not changed. e2e 846 covers the three source cases; it fails on 2026.9.30.1 with the false note of #744. --- src/config.cppm | 3 +- src/fallback/xlings_binary.cppm | 242 ++++++++++++------ ...ings_is_replaced_from_the_newest_source.sh | 112 ++++++++ tests/unit/test_xlings_version_pin.cpp | 191 ++++++++++++++ 4 files changed, 470 insertions(+), 78 deletions(-) create mode 100755 tests/e2e/846_the_vendored_xlings_is_replaced_from_the_newest_source.sh diff --git a/src/config.cppm b/src/config.cppm index f3e7d878..57f669e5 100644 --- a/src/config.cppm +++ b/src/config.cppm @@ -814,7 +814,8 @@ std::expected load_or_init( // 6. Acquire xlings binary if needed if (cfg.xlingsBinaryMode == "bundled") { auto xbin = mcpp::fallback::acquire_xlings_binary( - cfg.xlingsBinary, quiet, kXlingsPinnedVersion); + cfg.xlingsBinary, quiet, kXlingsPinnedVersion, + cfg.metaCacheDir / "vendored-xlings.versions"); if (!xbin) return std::unexpected(ConfigError{xbin.error()}); } else if (cfg.xlingsBinaryMode == "system") { auto sysPath = mcpp::platform::fs::which( diff --git a/src/fallback/xlings_binary.cppm b/src/fallback/xlings_binary.cppm index 9a11548b..2e64e8dd 100644 --- a/src/fallback/xlings_binary.cppm +++ b/src/fallback/xlings_binary.cppm @@ -1,10 +1,11 @@ // mcpp.fallback.xlings_binary — xlings binary acquisition chain. // -// Tries multiple strategies to obtain the xlings binary: -// 1. MCPP_VENDORED_XLINGS env var (explicit override) -// 2. the xlings released with this mcpp, `/registry/bin/xlings` -// 3. system `which xlings` -// 4. Fail with user-facing instructions +// One source is chosen (select_xlings_source) from: +// - MCPP_VENDORED_XLINGS (an explicit override, taken when set); +// - the xlings released with this mcpp, `/registry/bin/xlings`; +// - the xlings on PATH; +// the newer of the last two, the released one on a tie. With none, the error +// states how to provide one. module; #include @@ -36,10 +37,35 @@ std::string vendored_xlings_version(const std::filesystem::path& bin); // released binary is the one that satisfies the pin by construction. std::filesystem::path released_xlings_source(const std::filesystem::path& destBin); -// The version the acquisition chain WOULD install, without installing it. -// Empty when nothing is available. Replacing a vendored binary is only an -// improvement when this is newer than what is already there. -std::string candidate_source_version(const std::filesystem::path& destBin = {}); +// A place an xlings binary can be copied from, with the version it answers. +struct XlingsSource { + std::filesystem::path path; + std::string version; // empty when it cannot be read + std::string origin; // how the acquisition line names it +}; + +// THE ONE ANSWER TO "WHICH SOURCE" (mcpp#744). `MCPP_VENDORED_XLINGS` when it +// is set, as an explicit choice. Otherwise the newer of the xlings released +// with this mcpp and the xlings on PATH, the released one on a tie or when the +// PATH copy's version cannot be read. The check that decides whether to +// replace a vendored binary and the copy that replaces it both use this +// answer, so the version stated is the version copied. Before, the check took +// the first source that existed, not the newest: a released copy older than +// the pin hid a newer xlings on PATH, and mcpp stated that no newer source +// was available. +std::optional choose_xlings_source(std::optional override_, + std::optional released, + std::optional onPath); +// The same choice over this process's sources. +std::optional select_xlings_source(const std::filesystem::path& destBin = {}); + +// The version `bin` answers, asked at most once per process. With `memoFile`, +// the answer is also kept across processes, keyed by the binary's path, size +// and modification time: an update of xlings writes a new file and is asked +// again. Asking costs a process of xlings, measured at 0.35 s, and every +// command that loads the configuration asked (build wall-time plan, F6/W3). +std::string known_xlings_version(const std::filesystem::path& bin, + const std::filesystem::path& memoFile = {}); // True when `have` is strictly older than `want`, comparing dot-separated // numeric components. Anything unparseable answers false -- a version this @@ -63,14 +89,27 @@ bool version_is_older(std::string_view have, std::string_view want); // // Strictly-older, not not-equal: a user who put a newer xlings there on // purpose must not be downgraded by an mcpp that happens to pin an older one. +// +// ONCE PER PROCESS. The configuration is loaded from about ten call sites, and +// one `mcpp pack` over a workspace printed its note three times per member +// (mcpp#744). A home settled once in a process is not examined again, and +// `Updating` and `Note` are each stated at most once. std::expected acquire_xlings_binary(const std::filesystem::path& destBin, bool quiet = false, - std::string_view pinnedVersion = {}) { + std::string_view pinnedVersion = {}, + const std::filesystem::path& versionMemo = {}) { + static std::mutex m; + static std::set settled; + static bool noted = false, updated = false; + std::lock_guard lock(m); + if (settled.contains(destBin) && std::filesystem::exists(destBin)) return destBin; if (std::filesystem::exists(destBin)) { - auto have = vendored_xlings_version(destBin); + auto have = known_xlings_version(destBin, versionMemo); if (pinnedVersion.empty() || have.empty() - || !version_is_older(have, pinnedVersion)) + || !version_is_older(have, pinnedVersion)) { + settled.insert(destBin); return destBin; + } // Behind the pin -- but replacing is only an improvement if what we // would put there is actually newer. The acquisition chain below ends @@ -81,28 +120,46 @@ acquire_xlings_binary(const std::filesystem::path& destBin, bool quiet = false, // re-acquired, which replaced 2026.8.2.1 with the system's 0.4.51 -- // older still, and equally missing the feature the check exists to // restore. Look before leaping. - auto candidate = candidate_source_version(destBin); - if (candidate.empty() || !version_is_older(have, candidate)) { + auto candidate = select_xlings_source(destBin); + if (!candidate || candidate->version.empty() + || !version_is_older(have, candidate->version)) { // stderr, not stdout. This is a remark about the environment, // not output of the command that happens to be running -- and // `mcpp test --json` promises every stdout line is NDJSON, a // promise this line broke the moment a machine fell behind the // pin (e2e 155). - if (!quiet) + if (!quiet && !noted) std::println(stderr, "{:>12} vendored xlings {} is older than the " "pinned {}, but no newer source is available " "(keeping it; run `xlings self update`)", "Note", have, pinnedVersion); + noted = true; + settled.insert(destBin); return destBin; } - if (!quiet) - std::println(stderr, - "{:>12} vendored xlings {} -> {} (pinned {})", - "Updating", have, candidate, pinnedVersion); std::error_code rec; + std::filesystem::copy_file(candidate->path, destBin, + std::filesystem::copy_options::overwrite_existing, rec); + if (!rec) { + std::filesystem::permissions(destBin, + std::filesystem::perms::owner_exec + | std::filesystem::perms::group_exec + | std::filesystem::perms::others_exec, + std::filesystem::perm_options::add, rec); + if (!quiet && !updated) + std::println(stderr, + "{:>12} vendored xlings {} -> {} from {} (pinned {})", + "Updating", have, candidate->version, candidate->origin, + pinnedVersion); + updated = true; + // The version is known: it is the one just copied. + (void)known_xlings_version(destBin, versionMemo); + settled.insert(destBin); + return destBin; + } + // The copy failed; the chain below tries again from the start. std::filesystem::remove(destBin, rec); - // fall through and re-acquire } std::error_code ec; @@ -117,29 +174,11 @@ acquire_xlings_binary(const std::filesystem::path& destBin, bool quiet = false, std::println("{}{} {}", std::string(W - verb.size(), ' '), verb, msg); }; - // 1. Explicit override - if (auto* e = std::getenv("MCPP_VENDORED_XLINGS"); e && *e) { - std::filesystem::path src{e}; - if (std::filesystem::exists(src)) { - std::filesystem::copy_file(src, destBin, - std::filesystem::copy_options::overwrite_existing, ec); - if (!ec) { - std::filesystem::permissions(destBin, - std::filesystem::perms::owner_exec - | std::filesystem::perms::group_exec - | std::filesystem::perms::others_exec, - std::filesystem::perm_options::add, ec); - if (!quiet) print_status("Bundled", - std::format("xlings (from MCPP_VENDORED_XLINGS)")); - return destBin; - } - } - } - - // 2. The xlings released with this mcpp. Ahead of the system copy, which - // may be any version (see released_xlings_source). - if (auto released = released_xlings_source(destBin); !released.empty()) { - std::filesystem::copy_file(released, destBin, + // The first acquisition takes the source a replacement would take + // (select_xlings_source): the override, otherwise the newer of the + // released copy and the PATH copy. + if (auto src = select_xlings_source(destBin)) { + std::filesystem::copy_file(src->path, destBin, std::filesystem::copy_options::overwrite_existing, ec); if (!ec) { std::filesystem::permissions(destBin, @@ -148,34 +187,15 @@ acquire_xlings_binary(const std::filesystem::path& destBin, bool quiet = false, | std::filesystem::perms::others_exec, std::filesystem::perm_options::add, ec); if (!quiet) print_status("Bundled", - std::format("xlings (released with this mcpp: {})", released.string())); + std::format("xlings{} (from {}: {})", + src->version.empty() ? std::string{} : " " + src->version, + src->origin, src->path.string())); + settled.insert(destBin); return destBin; } - ec.clear(); - } - - // 3. Copy from system (`which xlings`) - auto xlings_name = std::string("xlings") + std::string(mcpp::platform::exe_suffix); - auto sysXlings = mcpp::platform::fs::which(xlings_name); - if (sysXlings) { - std::string p = sysXlings->string(); - if (!p.empty() && std::filesystem::exists(p)) { - std::filesystem::copy_file(p, destBin, - std::filesystem::copy_options::overwrite_existing, ec); - if (!ec) { - std::filesystem::permissions(destBin, - std::filesystem::perms::owner_exec - | std::filesystem::perms::group_exec - | std::filesystem::perms::others_exec, - std::filesystem::perm_options::add, ec); - if (!quiet) print_status("Bundled", - std::format("xlings (copied from system: {})", p)); - return destBin; - } - } } - // 3. Fail with instructions + // Nothing to copy: say how to provide one. return std::unexpected(std::format( "xlings binary not found. Either:\n" " - install via: curl -fsSL https://raw.githubusercontent.com/d2learn/xlings/refs/heads/main/tools/other/quick_install.sh | bash\n" @@ -259,18 +279,86 @@ std::filesystem::path released_xlings_source(const std::filesystem::path& destBi return released; } -std::string candidate_source_version(const std::filesystem::path& destBin) { +std::optional choose_xlings_source(std::optional override_, + std::optional released, + std::optional onPath) { + if (override_) return override_; + if (!released) return onPath; + if (!onPath || onPath->version.empty()) return released; + if (released->version.empty() || version_is_older(released->version, onPath->version)) + return onPath; + return released; +} + +std::optional select_xlings_source(const std::filesystem::path& destBin) { + std::optional override_, released, onPath; + std::error_code ec; if (const char* e = std::getenv("MCPP_VENDORED_XLINGS"); e && *e) { - std::error_code ec; - if (std::filesystem::exists(std::filesystem::path(e), ec)) - return vendored_xlings_version(std::filesystem::path(e)); + std::filesystem::path p{e}; + if (std::filesystem::exists(p, ec)) + override_ = XlingsSource{p, vendored_xlings_version(p), "MCPP_VENDORED_XLINGS"}; } - if (auto released = released_xlings_source(destBin); !released.empty()) - return vendored_xlings_version(released); - if (auto sys = mcpp::platform::fs::which( - std::string("xlings") + std::string(mcpp::platform::exe_suffix))) - return vendored_xlings_version(*sys); - return {}; + if (!override_) { + if (auto r = released_xlings_source(destBin); !r.empty()) + released = XlingsSource{r, vendored_xlings_version(r), "the release of this mcpp"}; + if (auto sys = mcpp::platform::fs::which( + std::string("xlings") + std::string(mcpp::platform::exe_suffix))) { + const bool isDest = !destBin.empty() && std::filesystem::equivalent(*sys, destBin, ec); + if (!isDest && std::filesystem::exists(*sys, ec)) + onPath = XlingsSource{*sys, vendored_xlings_version(*sys), "PATH"}; + } + } + return choose_xlings_source(std::move(override_), std::move(released), std::move(onPath)); +} + +std::string known_xlings_version(const std::filesystem::path& bin, + const std::filesystem::path& memoFile) { + std::error_code ec; + const auto size = std::filesystem::file_size(bin, ec); + if (ec) return {}; + const auto mtime = std::filesystem::last_write_time(bin, ec); + if (ec) return {}; + const auto u8 = bin.generic_u8string(); + const auto key = std::format("{}\t{}\t{}", + std::string(reinterpret_cast(u8.data()), u8.size()), + size, mtime.time_since_epoch().count()); + + static std::mutex m; + static std::map answered; // key -> version + std::lock_guard lock(m); + if (auto it = answered.find(key); it != answered.end()) return it->second; + + // The memo holds one line per binary: `\t\t\t`. + std::vector lines; + if (!memoFile.empty()) { + std::ifstream is(memoFile, std::ios::binary); + for (std::string line; std::getline(is, line);) { + if (line.starts_with(key + "\t")) { + auto v = line.substr(key.size() + 1); + if (!v.empty()) return answered[key] = v; + } + lines.push_back(std::move(line)); + } + } + auto version = vendored_xlings_version(bin); + answered[key] = version; + if (!memoFile.empty() && !version.empty()) { + // Replace this binary's line, keep the others, and write the file whole + // beside itself before renaming it into place. + const auto prefix = key.substr(0, key.find('\t') + 1); + std::erase_if(lines, [&](const std::string& l) { return l.starts_with(prefix); }); + lines.push_back(key + "\t" + version); + std::filesystem::create_directories(memoFile.parent_path(), ec); + auto tmp = memoFile; + tmp += std::format(".tmp-{}", std::chrono::steady_clock::now().time_since_epoch().count()); + { + std::ofstream os(tmp, std::ios::binary | std::ios::trunc); + for (auto const& l : lines) os << l << '\n'; + } + std::filesystem::rename(tmp, memoFile, ec); + if (ec) std::filesystem::remove(tmp, ec); + } + return version; } } // namespace mcpp::fallback diff --git a/tests/e2e/846_the_vendored_xlings_is_replaced_from_the_newest_source.sh b/tests/e2e/846_the_vendored_xlings_is_replaced_from_the_newest_source.sh new file mode 100755 index 00000000..4b7f34df --- /dev/null +++ b/tests/e2e/846_the_vendored_xlings_is_replaced_from_the_newest_source.sh @@ -0,0 +1,112 @@ +#!/usr/bin/env bash +# requires: +# 846 -- a vendored xlings older than the pin is replaced from the newest source +# (mcpp#744). +# +# The check that decides whether to replace the vendored binary took the first +# source that existed -- MCPP_VENDORED_XLINGS, then the xlings released beside +# the running mcpp, then the PATH -- not the newest. A released copy older than +# the pin therefore hid a newer xlings on the PATH, and mcpp stated that no +# newer source was available. One function now chooses: the override when set, +# otherwise the newer of the released copy and the PATH copy. +# +# The stand-in for an older xlings is the ninja payload, whose `--version` +# prints a dotted version older than any dated xlings (as in e2e 687); the +# newer one is the real xlings. Both are real executables, so the criteria +# hold on Windows as well. +# +# Criteria, each from an mcpp running from its release layout +# (`/bin/mcpp` beside `/registry/bin/xlings`): +# D. Released older, PATH newer: one `Updating` line naming the PATH, and the +# vendored binary is then an xlings. +# E. Released newer, PATH older: the released copy is taken. +# F. Everything older: one `Note` line, and the vendored binary is kept. +set -e + +TMP=$(mktemp -d) +trap 'rm -rf "$TMP"' EXIT + +fail() { echo "FAIL: $1"; shift; for f in "$@"; do echo "--- $f ---"; cat "$f" 2>/dev/null; done; exit 1; } + +EXE="" +case "$(uname -s)" in MINGW*|MSYS*|CYGWIN*) EXE=".exe" ;; esac + +export MCPP_HOME="$TMP/mcpp-home" +export MCPP_OFFLINE=1 +MCPP_INHERIT_CONFIG=0 source "$(dirname "$0")/_inherit_toolchain.sh" +cd "$TMP" + +VENDORED="$MCPP_HOME/registry/bin/xlings$EXE" +"$MCPP" self env > setup.out 2> setup.err || true +[ -f "$VENDORED" ] || fail "setup: the first command did not vendor xlings at $VENDORED" setup.out setup.err +"$VENDORED" --version 2>/dev/null | grep -q '^xlings ' \ + || fail "setup: the vendored binary does not answer as xlings" setup.err + +NINJA="" +for cand in "$MCPP_HOME"/registry/data/xpkgs/xim-x-ninja/*/ninja$EXE \ + "$MCPP_HOME"/registry/data/xpkgs/xim-x-ninja/*/bin/ninja$EXE; do + if [ -f "$cand" ]; then NINJA="$cand"; break; fi +done +[ -n "$NINJA" ] || fail "setup: no ninja binary to stand in for an older xlings" +older=$("$NINJA" --version 2>/dev/null | head -1) +case "$older" in + [0-9]*.*) ;; + *) fail "setup: the stand-in '$NINJA' answered '$older', not a dotted version" ;; +esac + +mkdir -p "$TMP/real" "$TMP/release/bin" "$TMP/release/registry/bin" "$TMP/pathbin" +cp "$VENDORED" "$TMP/real/xlings$EXE" +cp "$MCPP" "$TMP/release/bin/mcpp$EXE" +chmod +x "$TMP/release/bin/mcpp$EXE" 2>/dev/null || true + +BASE_PATH="/usr/bin:/bin" +if PATH="$BASE_PATH" command -v xlings > /dev/null 2>&1; then + fail "an xlings is reachable on $BASE_PATH, so the criteria cannot tell the sources apart" +fi + +# place : copy an executable into place. +place() { rm -f "$1"; cp "$2" "$1"; chmod +x "$1" 2>/dev/null || true; } +run() { # run : the release-layout mcpp, with the test PATH + env -u MCPP_VENDORED_XLINGS PATH="$TMP/pathbin:$BASE_PATH" \ + "$TMP/release/bin/mcpp$EXE" self env > "$1.out" 2> "$1.err" || true +} +count() { grep -c "$1" "$2" || true; } + +# ── D ────────────────────────────────────────────────────────────────────── +place "$VENDORED" "$NINJA" +place "$TMP/release/registry/bin/xlings$EXE" "$NINJA" +place "$TMP/pathbin/xlings$EXE" "$TMP/real/xlings$EXE" +run d +[ "$(count "vendored xlings $older -> " d.err)" = 1 ] \ + || fail "D: expected one Updating line for a vendored xlings answering $older" d.err +grep -q "vendored xlings $older -> .* from PATH" d.err \ + || fail "D: the replacement did not come from the newer xlings on the PATH" d.err +[ "$(count 'no newer source is available' d.err)" = 0 ] \ + || fail "D: mcpp stated that no newer source was available while one was on the PATH" d.err +"$VENDORED" --version 2>/dev/null | grep -q '^xlings ' \ + || fail "D: after the replacement the vendored binary is not xlings" d.err +echo "ok: D, a newer xlings on the PATH replaced the vendored one past an older released copy" + +# ── E ────────────────────────────────────────────────────────────────────── +place "$VENDORED" "$NINJA" +place "$TMP/release/registry/bin/xlings$EXE" "$TMP/real/xlings$EXE" +place "$TMP/pathbin/xlings$EXE" "$NINJA" +run e +grep -q "vendored xlings $older -> .* from the release of this mcpp" e.err \ + || fail "E: the newer released copy was not taken over an older PATH copy" e.err +"$VENDORED" --version 2>/dev/null | grep -q '^xlings ' \ + || fail "E: after the replacement the vendored binary is not xlings" e.err +echo "ok: E, the newer released copy was taken" + +# ── F ────────────────────────────────────────────────────────────────────── +place "$VENDORED" "$NINJA" +place "$TMP/release/registry/bin/xlings$EXE" "$NINJA" +place "$TMP/pathbin/xlings$EXE" "$NINJA" +run f +[ "$(count 'no newer source is available' f.err)" = 1 ] \ + || fail "F: expected exactly one Note when every source is older" f.err +[ "$(count 'vendored xlings .* -> ' f.err)" = 0 ] \ + || fail "F: a vendored binary was replaced by a source that is not newer" f.err +"$VENDORED" --version 2>/dev/null | grep -q '^xlings ' \ + && fail "F: the vendored binary changed although no source was newer" f.err +echo "ok: F, one Note and the vendored binary kept when no source is newer" diff --git a/tests/unit/test_xlings_version_pin.cpp b/tests/unit/test_xlings_version_pin.cpp index 9156ca3e..3df8c727 100644 --- a/tests/unit/test_xlings_version_pin.cpp +++ b/tests/unit/test_xlings_version_pin.cpp @@ -13,6 +13,7 @@ // one, and an unparseable version is not evidence of being behind. #include +#include #include import std; @@ -99,3 +100,193 @@ TEST(XlingsVersionPin, ProbeReadsStandardOutputThroughTheLauncher) { } } // namespace + +// ONE ANSWER TO "WHICH SOURCE" (mcpp#744). The check that decides whether to +// replace a vendored binary used the first source that existed, while the +// replacement ran the whole chain: a released copy older than the pin hid a +// newer xlings on PATH, and mcpp stated that no newer source was available. +namespace { +fb::XlingsSource src(std::string v, std::string origin) { + return fb::XlingsSource{std::filesystem::path(origin), std::move(v), origin}; +} +} // namespace + +TEST(XlingsSource, TheOverrideIsTakenAsAnExplicitChoice) { + auto c = fb::choose_xlings_source(src("2026.1.1.1", "override"), + src("2026.9.1.1", "released"), src("2026.9.9.1", "path")); + ASSERT_TRUE(c); + EXPECT_EQ(c->origin, "override"); +} + +TEST(XlingsSource, TheNewerOfTheReleasedAndThePathCopyIsTaken) { + auto a = fb::choose_xlings_source(std::nullopt, src("2026.9.29.1", "released"), + src("2026.9.30.1", "path")); + ASSERT_TRUE(a); + EXPECT_EQ(a->origin, "path"); + auto b = fb::choose_xlings_source(std::nullopt, src("2026.9.30.1", "released"), + src("2026.9.29.1", "path")); + ASSERT_TRUE(b); + EXPECT_EQ(b->origin, "released"); +} + +TEST(XlingsSource, TheReleasedCopyWinsATieAndAnUnreadablePathCopy) { + auto tie = fb::choose_xlings_source(std::nullopt, src("2026.9.30.1", "released"), + src("2026.9.30.1", "path")); + ASSERT_TRUE(tie); + EXPECT_EQ(tie->origin, "released"); + auto unreadable = fb::choose_xlings_source(std::nullopt, src("2026.9.30.1", "released"), + src("", "path")); + ASSERT_TRUE(unreadable); + EXPECT_EQ(unreadable->origin, "released"); + auto releasedUnreadable = fb::choose_xlings_source(std::nullopt, src("", "released"), + src("2026.9.30.1", "path")); + ASSERT_TRUE(releasedUnreadable); + EXPECT_EQ(releasedUnreadable->origin, "path"); +} + +TEST(XlingsSource, AMissingSourceLeavesTheOther) { + auto onlyPath = fb::choose_xlings_source(std::nullopt, std::nullopt, src("1.0", "path")); + ASSERT_TRUE(onlyPath); + EXPECT_EQ(onlyPath->origin, "path"); + EXPECT_FALSE(fb::choose_xlings_source(std::nullopt, std::nullopt, std::nullopt)); +} + +// THE VERSION IS ASKED ONCE. Asking costs a process of xlings (0.35 s +// measured), and every command that loads the configuration asked. Within a +// process the answer is kept; across processes it is kept in a file keyed by +// the binary's path, size and modification time. The stub counts its runs. +namespace { +struct CountingStub { + std::filesystem::path dir, bin, counter; + explicit CountingStub(std::string_view version) { + dir = std::filesystem::temp_directory_path() + / std::format("mcpp memo {}", std::chrono::steady_clock::now().time_since_epoch().count()); + std::filesystem::create_directories(dir); + counter = dir / "runs"; + write(version); + } + void write(std::string_view version) { +#if defined(_WIN32) + bin = dir / "xlings.bat"; + std::ofstream os(bin, std::ios::binary | std::ios::trunc); + os << "@echo off\r\necho run>>\"" << counter.string() << "\"\r\necho xlings " + << version << "\r\n"; +#else + bin = dir / "xlings"; + { + std::ofstream os(bin, std::ios::binary | std::ios::trunc); + os << "#!/bin/sh\necho run >> '" << counter.string() << "'\nprintf 'xlings " + << version << "\\n'\n"; + } + std::filesystem::permissions(bin, std::filesystem::perms::owner_all, + std::filesystem::perm_options::replace); +#endif + } + int runs() const { + std::ifstream is(counter); + int n = 0; + for (std::string line; std::getline(is, line);) ++n; + return n; + } + ~CountingStub() { + std::error_code ec; + std::filesystem::remove_all(dir, ec); + } +}; +std::string memo_key(const std::filesystem::path& bin) { + auto u8 = bin.generic_u8string(); + return std::format("{}\t{}\t{}", std::string(reinterpret_cast(u8.data()), u8.size()), + std::filesystem::file_size(bin), + std::filesystem::last_write_time(bin).time_since_epoch().count()); +} +} // namespace + +TEST(XlingsVersionMemo, AskedOncePerProcessAndStoredForTheNext) { + CountingStub stub("2026.1.2.3"); + const auto memo = stub.dir / "memo"; + EXPECT_EQ(fb::known_xlings_version(stub.bin, memo), "2026.1.2.3"); + EXPECT_EQ(fb::known_xlings_version(stub.bin, memo), "2026.1.2.3"); + EXPECT_EQ(stub.runs(), 1); + std::ifstream is(memo); + std::string line; + std::getline(is, line); + EXPECT_EQ(line, memo_key(stub.bin) + "\t2026.1.2.3"); +} + +TEST(XlingsVersionMemo, AStoredAnswerIsReadWithoutRunningTheBinary) { + CountingStub stub("2026.1.2.3"); + const auto memo = stub.dir / "memo"; + { + std::ofstream os(memo, std::ios::binary); + os << "/elsewhere/xlings\t1\t1\t2020.1.1.1\n" << memo_key(stub.bin) << "\t2026.7.7.7\n"; + } + EXPECT_EQ(fb::known_xlings_version(stub.bin, memo), "2026.7.7.7"); + EXPECT_EQ(stub.runs(), 0); +} + +TEST(XlingsVersionMemo, ANewFileIsAskedAgain) { + CountingStub stub("2026.1.2.3"); + const auto memo = stub.dir / "memo"; + EXPECT_EQ(fb::known_xlings_version(stub.bin, memo), "2026.1.2.3"); + // An update writes a new file: another size, and another modification time. + std::this_thread::sleep_for(std::chrono::milliseconds(20)); + stub.write("2026.10.20.30"); + EXPECT_EQ(fb::known_xlings_version(stub.bin, memo), "2026.10.20.30"); + EXPECT_EQ(stub.runs(), 2); + std::ifstream is(memo); + int lines = 0; + for (std::string l; std::getline(is, l);) ++lines; + EXPECT_EQ(lines, 1) << "the binary's earlier line is replaced, not kept"; +} + +// ONCE PER PROCESS (mcpp#744). The configuration is loaded from about ten call +// sites, and one `mcpp pack` over a workspace printed its note three times per +// member. A home settled once in a process is not examined again: the note is +// stated once and the version is asked once. +namespace { +class ScopedEnv { +public: + ScopedEnv(std::string name, const char* value) : name_(std::move(name)) { + if (const char* old = std::getenv(name_.c_str()); old) { had_ = true; old_ = old; } + apply(value); + } + ~ScopedEnv() { apply(had_ ? old_.c_str() : nullptr); } + ScopedEnv(const ScopedEnv&) = delete; + ScopedEnv& operator=(const ScopedEnv&) = delete; +private: + void apply(const char* value) { +#if defined(_WIN32) + ::_putenv_s(name_.c_str(), value ? value : ""); +#else + if (value) ::setenv(name_.c_str(), value, 1); + else ::unsetenv(name_.c_str()); +#endif + } + std::string name_; + bool had_ = false; + std::string old_; +}; +} // namespace + +TEST(XlingsAcquire, TheNoteIsStatedOncePerProcess) { + CountingStub stub("2026.1.1.1"); + const auto empty = stub.dir / "empty-path"; + std::filesystem::create_directories(empty); + // No newer source: no override, an empty PATH, and a test binary that does + // not run from a release layout (`/bin/`). + ScopedEnv override_("MCPP_VENDORED_XLINGS", nullptr); + ScopedEnv path("PATH", empty.string().c_str()); + testing::internal::CaptureStderr(); + auto first = fb::acquire_xlings_binary(stub.bin, /*quiet=*/false, "2026.9.30.1"); + auto second = fb::acquire_xlings_binary(stub.bin, /*quiet=*/false, "2026.9.30.1"); + const auto err = testing::internal::GetCapturedStderr(); + ASSERT_TRUE(first); + ASSERT_TRUE(second); + std::size_t notes = 0; + for (auto at = err.find("no newer source is available"); at != std::string::npos; + at = err.find("no newer source is available", at + 1)) + ++notes; + EXPECT_EQ(notes, 1u) << err; + EXPECT_EQ(stub.runs(), 1) << "the second load asked the version again"; +} + From 2beeec9ff7c09a02042c872e5f793afbbcd8f0b2 Mon Sep 17 00:00:00 2001 From: speak-agent <248744407+speak-agent@users.noreply.github.com> Date: Wed, 30 Sep 2026 12:23:47 +0800 Subject: [PATCH 04/15] modgraph: one walk per tree per process for glob expansion A glob's walk is kept per root and start, and each pattern is matched against the kept listing, first by the literal text after its last star. Every directory the walk entered is examined again before the listing is reused, and a directory modified within two seconds of the walk is never trusted, so files that planning writes are seen. Before, a pattern with an empty literal prefix walked the whole package tree: the 127 source patterns of compat.libarchive, expanded about three times per plan, opened its 35 directories 13,406 times. Measured on a planned edit of one xlings source: 30,372 directory opens of installed packages fall to 1,749, the scan phase from 0.91 s to 43 ms, and ninja starts at 0.66 s instead of 3.05 s. --- modules/manifest/src/glob.cppm | 11 ++- src/modgraph/scanner.cppm | 126 ++++++++++++++++++++++++++++----- 2 files changed, 117 insertions(+), 20 deletions(-) diff --git a/modules/manifest/src/glob.cppm b/modules/manifest/src/glob.cppm index e4a4e1b5..031a9dc7 100644 --- a/modules/manifest/src/glob.cppm +++ b/modules/manifest/src/glob.cppm @@ -127,6 +127,11 @@ void note_unnarrowable_path(const std::filesystem::path& p); // ("路径窄化不变式") and the user-facing behaviour in docs/04-mcpp-toml.md. std::vector take_unnarrowable_paths(); +// Does `relative`, a generic spelling relative to the glob's root, match +// `glob`? The matching half of path_matches_glob, for a caller that already +// holds the narrowed relative spelling (a cached directory listing). +bool relative_path_matches_glob(std::string_view relative, std::string_view glob); + // Does `candidate` match `glob`, interpreted relative to `root`? // // Supports "**" (any number of directory levels) and "*" (within one segment). @@ -149,7 +154,11 @@ bool path_matches_glob(const std::filesystem::path& candidate, note_unnarrowable_path(candidate); return false; } + return relative_path_matches_glob(*rel, glob); +} +bool relative_path_matches_glob(std::string_view relative, std::string_view glob) +{ auto match = [](std::string_view s, std::string_view p) -> bool { std::function rec = [&](std::size_t si, std::size_t pi) -> bool { @@ -185,7 +194,7 @@ bool path_matches_glob(const std::filesystem::path& candidate, }; return rec(0, 0); }; - return match(*rel, glob); + return match(relative, glob); } } // namespace mcpp::modgraph diff --git a/src/modgraph/scanner.cppm b/src/modgraph/scanner.cppm index 1bd173d5..f6ee2041 100644 --- a/src/modgraph/scanner.cppm +++ b/src/modgraph/scanner.cppm @@ -557,31 +557,56 @@ std::vector expand_braces(std::string_view glob, int depthGuard) { namespace { -// mcpp#225: the actual bounded recursive-directory walk for a SINGLE plain -// glob (no `{` — expand_glob below desugars brace alternation via -// expand_braces and calls this once per branch, unioning the results). -std::vector expand_glob_one(const std::filesystem::path& root, - std::string_view glob) -{ - namespace fs = std::filesystem; - std::vector out; - if (!fs::exists(root)) return out; +// A directory tree as a glob walk sees it, kept for the process. +// +// ONE WALK PER TREE (.agents/docs/2026-09-30-build-wall-time-progress-count- +// and-hang-plan.md, F5 and W8). A package's `sources` are patterns, and a +// pattern whose literal prefix is empty (`*/libarchive/archive_acl.c`) walked +// the whole package tree. 127 such patterns, expanded about three times per +// plan, opened libarchive's 35 directories 13,406 times, and 30,372 directory +// opens of installed packages preceded the compile of every edited file. A +// walk is now kept per root and start, and each pattern is matched against the +// kept list. +// +// A KEPT WALK IS CHECKED, NOT TRUSTED. Planning writes files (a build +// program's output, a descriptor's generated files), so every directory the +// walk entered is examined again before the list is reused: adding, removing +// or renaming an entry changes its directory's modification time. A directory +// modified within two seconds of the walk is never trusted, since a coarse +// clock could hide a change made in the same tick; a tree being edited is +// therefore walked every time, as before. +struct TreeListing { + struct File { + std::filesystem::path path; + std::optional relative; // to the glob root, generic and narrowed + }; + std::vector> dirs; + std::vector files; + bool trusted = true; +}; - // mcpp#225: bound the walk's start point to the glob's literal - // directory prefix instead of always walking the whole root. A prefix - // that doesn't exist means the glob can never match anything — return - // empty WITHOUT walking (not a full-tree fallback). - fs::path prefix = glob_literal_prefix(glob); - fs::path start = prefix.empty() ? root : root / prefix; - std::error_code startEc; - if (!fs::exists(start, startEc)) return out; +// mcpp#225: the bounded recursive-directory walk from `start`, with the +// exclusions and the symlink-cycle guard every glob walk has. +std::shared_ptr walk_tree(const std::filesystem::path& root, + const std::filesystem::path& start) { + namespace fs = std::filesystem; + auto listing = std::make_shared(); + const auto walkedAt = fs::file_time_type::clock::now(); + auto note_dir = [&](const fs::path& d) { + std::error_code tec; + const auto t = fs::last_write_time(d, tec); + if (tec) { listing->trusted = false; return; } + if (t > walkedAt - std::chrono::seconds(2)) listing->trusted = false; + listing->dirs.emplace_back(d, t); + }; + note_dir(start); // Follow directory symlinks (vendored trees are often symlink farms). // Cycle guard: a directory whose canonical path is already on the // CURRENT recursion chain is a link loop — only that is pruned; the same // real directory reached via a second lexical path (dir + link to it) // still walks, because glob matching is lexical. Files reachable twice - // are deduped by canonical identity afterwards. + // are deduped by canonical identity by the caller. std::vector chain; // canonical dirs of the recursion stack std::error_code ec, eec; // ec: iteration; eec: per-entry probes { @@ -604,15 +629,78 @@ std::vector expand_glob_one(const std::filesystem::path& it.disable_recursion_pending(); // link cycle } else { chain.push_back(eec ? e.path() : c); + note_dir(e.path()); } continue; } if (!e.is_regular_file(eec) || eec) continue; - if (path_matches_glob(e.path(), root, glob)) out.push_back(e.path()); + auto rel = try_narrow(e.path().lexically_relative(root)); + // A name the code page cannot spell can never match a glob, and is + // recorded as path_matches_glob records it. + if (!rel) note_unnarrowable_path(e.path()); + listing->files.push_back({e.path(), std::move(rel)}); + } + if (ec) listing->trusted = false; + return listing; +} + +bool listing_current(const TreeListing& listing) { + if (!listing.trusted) return false; + std::error_code ec; + for (auto const& [dir, time] : listing.dirs) { + const auto now = std::filesystem::last_write_time(dir, ec); + if (ec || now != time) return false; + } + return true; +} + +std::shared_ptr tree_listing(const std::filesystem::path& root, + const std::filesystem::path& start) { + static std::mutex m; + static std::map, + std::shared_ptr> kept; + std::lock_guard lock(m); + auto key = std::pair{root, start}; + if (auto it = kept.find(key); it != kept.end() && listing_current(*it->second)) + return it->second; + auto listing = walk_tree(root, start); + kept[key] = listing; + return listing; +} + +// mcpp#225: the walk for a SINGLE plain glob (no `{` — expand_glob below +// desugars brace alternation via expand_braces and calls this once per +// branch, unioning the results), over the kept listing of its start. +std::vector expand_glob_one(const std::filesystem::path& root, + std::string_view glob) +{ + namespace fs = std::filesystem; + std::vector out; + if (!fs::exists(root)) return out; + + // mcpp#225: bound the walk's start point to the glob's literal + // directory prefix instead of always walking the whole root. A prefix + // that doesn't exist means the glob can never match anything — return + // empty WITHOUT walking (not a full-tree fallback). + fs::path prefix = glob_literal_prefix(glob); + fs::path start = prefix.empty() ? root : root / prefix; + std::error_code startEc; + if (!fs::exists(start, startEc)) return out; + + // Every match ends with the text after the glob's last `*`, which the + // matcher reads literally: a cheap test that turns most entries away. + const auto star = glob.find_last_of('*'); + const std::string_view tail = star == std::string_view::npos ? glob : glob.substr(star + 1); + auto listing = tree_listing(root, start); + for (auto const& f : listing->files) { + if (!f.relative) continue; + if (!tail.empty() && !f.relative->ends_with(tail)) continue; + if (relative_path_matches_glob(*f.relative, glob)) out.push_back(f.path); } std::sort(out.begin(), out.end()); // Dedup files reachable through more than one directory link (first // lexical occurrence wins). + std::error_code eec; std::set seenFiles; out.erase(std::remove_if(out.begin(), out.end(), [&](const fs::path& p) { auto c = fs::canonical(p, eec); From 94603a99568d9473240647f71cc53abed47ef2e2 Mon Sep 17 00:00:00 2001 From: speak-agent <248744407+speak-agent@users.noreply.github.com> Date: Wed, 30 Sep 2026 12:47:08 +0800 Subject: [PATCH 05/15] modgraph: resolve a module name in the importer's closure (#732) A module name identifies one module within one program: GCC and clang name a module's entities and its initializer after it, so a program cannot link two modules of one name (measured: multiple definition of value@common()), and two programs may each have one. A build holds several programs, a package and the programs it ships through artifacts or the members of a workspace, and mcpp refused a name that two packages of one build provided whichever programs they belonged to. The prepare phase computes each compiled package's closure (code and member edges; not artifacts, tools or build-dependencies; no closure for a workspace's virtual root). One resolver, choose_provider and resolve_provider in mcpp.modgraph, answers which provider an import means, and the scanner, the interface check, the packer and the backend's std reachability all use it. Two providers of one name are refused only when one closure holds both, and one file reached twice is recognised by identity rather than spelling. When two packages provide a name, their BMIs lie below their packages' directories and every unit that may import the name is bound to its provider: a module map for GCC, whose mapper does not fall back for an unlisted name, -fmodule-file= for clang and /reference for MSVC; mcpp dyndep reads the same map through --module-map. With every name provided once nothing changes: build.ninja and compile_commands.json of the xlings workspace are byte-identical to 2026.9.30.1's. A package that provides a collided name is compiled rather than served from the global cache. The plan's unread module map is removed. e2e 847 (GCC and clang) and 848 (MSVC) cover an artifacts updater, two workspace members, the reported layout, and the two refusals; 847 fails on 2026.9.30.1 with the error of #732. --- docs/05-dependencies.md | 29 +++ docs/07-workspace.md | 8 +- docs/zh/05-dependencies.md | 19 ++ docs/zh/07-workspace.md | 5 +- modules/dyndep/src/dyndep.cppm | 39 +++- modules/toolchain-model/src/model.cppm | 7 + src/build/configure.cppm | 14 +- src/build/ninja_backend.cppm | 88 +++++-- src/build/plan.cppm | 127 +++++++++- src/build/prepare/plan.cpp | 5 + src/build/prepare/scan.cpp | 67 ++++-- src/cli.cppm | 2 + src/cli/cmd_build.cppm | 13 ++ src/modgraph/graph.cppm | 65 +++++- src/modgraph/scanner.cppm | 117 +++++++--- src/pack/interface.cppm | 23 +- ..._module_name_is_unique_within_a_program.sh | 221 ++++++++++++++++++ ...48_msvc_binds_a_module_name_per_program.sh | 53 +++++ tests/unit/test_modgraph.cpp | 99 ++++++++ 19 files changed, 906 insertions(+), 95 deletions(-) create mode 100755 tests/e2e/847_a_module_name_is_unique_within_a_program.sh create mode 100755 tests/e2e/848_msvc_binds_a_module_name_per_program.sh diff --git a/docs/05-dependencies.md b/docs/05-dependencies.md index 64bac73b..27d04486 100644 --- a/docs/05-dependencies.md +++ b/docs/05-dependencies.md @@ -525,6 +525,35 @@ updater = { path = "../updater", artifacts = ["updater"] } > by nothing that made a decision: writing it produced a manifest that loaded, > no diagnostic, and no effect. +### One module per name in each program (mcpp 2026.9.30.2+) + +A module name identifies one module within one program. The compilers name a +module's entities and its initializer after the module, so a program cannot +link two modules of one name. A build can hold several programs (a package and +the programs it ships through `artifacts`, the members of a workspace), and +each of them may have its own module of one name. + +- An import is resolved within the importing package's closure: the package + and every package it reaches through its dependencies. The closure does not + follow `artifacts`, `tools` or `[build-dependencies]` edges, whose programs + are built separately. +- Two packages that one program links may not provide the same name. The build + is refused, and the message names the package whose closure holds both. +- When two packages of one build provide a name, each BMI lies below its + package's directory in the build directory, and every compile that may import + the name is told which one it means: through a module map with GCC, through + `-fmodule-file=` with Clang, and through `/reference` with MSVC. When every + name has one provider, the build directory and every command are as they + were before. +- A package that provides such a name is compiled in the project, not served + from the global dependency cache. +- clangd finds a module by its name in the compilation database, so for a name + two packages provide it may show the other program's module. The build is + not affected. + +A module that several programs share is best provided by one package that the +others depend on; it is then compiled once. + ## Current limitations - **Two things need the network, and only two:** resolving a branch that has no diff --git a/docs/07-workspace.md b/docs/07-workspace.md index 28fe06a2..16e564f0 100644 --- a/docs/07-workspace.md +++ b/docs/07-workspace.md @@ -451,9 +451,11 @@ member that several members use is compiled once. `compile_commands.json` once, as the union of their databases (2026.9.29.5+). - **No-op builds.** A command repeated with nothing changed is answered by one check per configuration, without planning. -- **Module names.** Members built in one graph share one module namespace: - two members that each provide a module of the same name cannot be built in - one `--workspace` command; build each with `-p`. +- **Module names.** A module name is unique within one program, not within one + graph (2026.9.30.2+). Two members that share no program may each provide a + module of the same name, and one `--workspace` command builds both; a member + that links both is refused. See + [05 — One module per name in each program](05-dependencies.md#one-module-per-name-in-each-program-mcpp-20269302). ## 6. Directory Layout diff --git a/docs/zh/05-dependencies.md b/docs/zh/05-dependencies.md index 6b667990..afd61014 100644 --- a/docs/zh/05-dependencies.md +++ b/docs/zh/05-dependencies.md @@ -478,6 +478,25 @@ updater = { path = "../updater", artifacts = ["updater"] } > 这个段很早就能被解析，而直到 2026.8.29.1，没有任何做决定的代码读过它： > 写下它得到的是一份能加载的 manifest、零诊断、零效果。 +### 每个程序里一个名字只对应一个模块（mcpp 2026.9.30.2+） + +模块名在一个程序之内标识一个模块。编译器以模块名命名模块的实体与初始化函数，因此一个程序 +不能链接两个同名模块。一次构建可以包含多个程序（一个包，以及它经 `artifacts` 发布的程序； +工作区的各个成员），其中每个程序都可以有自己的同名模块。 + +- import 在导入方所在包的闭包内解析：该包，以及它经依赖到达的每个包。闭包不沿 + `artifacts`、`tools` 与 `[build-dependencies]` 边延伸，这些边对应的程序另行构建。 +- 被同一个程序链接的两个包不得提供同一个名字。此时构建被拒绝，消息点名其闭包同时包含两者的包。 +- 同一次构建中有两个包提供同一个名字时，各自的 BMI 位于构建目录中其所属包的子目录下， + 每一个可能导入该名字的编译都会被告知它指哪一个：GCC 经模块映射文件，Clang 经 + `-fmodule-file=`，MSVC 经 `/reference`。每个名字只有一个提供方时，构建目录与每条命令都与 + 以前相同。 +- 提供这种名字的包在项目内编译，不从全局依赖缓存取用。 +- clangd 在编译数据库中按名字查找模块，因此对两个包提供的同一个名字，编辑器可能显示另一个 + 程序的模块。构建不受影响。 + +几个程序共用的模块，最好由一个包提供、其余的包依赖它；这样它只编译一次。 + ## 当前边界 - **只有两件事需要网络，而且只有这两件：** 解析一个在 lock 里没有 commit 的 diff --git a/docs/zh/07-workspace.md b/docs/zh/07-workspace.md index df23764d..6ed6777d 100644 --- a/docs/zh/07-workspace.md +++ b/docs/zh/07-workspace.md @@ -414,8 +414,9 @@ mcpp test --workspace --workspace-timeout 1800 # whole fan-out (default 0 = no 多个配置的命令只写一次根目录的 `compile_commands.json`，内容为各配置数据库的并集 （2026.9.29.5+）。 - **无事可做的构建。** 在没有任何改动时重复执行的命令，每个配置只做一次检查，不重新规划。 -- **模块名。** 在同一张图中构建的成员共享一个模块命名空间：两个成员各自提供同名模块时，不能在 - 同一条 `--workspace` 命令中构建；分别用 `-p` 构建。 +- **模块名。** 模块名在一个程序之内唯一，而不是在一张图之内唯一（2026.9.30.2+）。不共享任何 + 程序的两个成员可以各自提供同名模块，一条 `--workspace` 命令会把两者都构建出来；同时链接两者的 + 成员会被拒绝。见 [05 —— 每个程序里一个名字只对应一个模块](05-dependencies.md#每个程序里一个名字只对应一个模块mcpp-20269302)。 ## 6. 目录布局 diff --git a/modules/dyndep/src/dyndep.cppm b/modules/dyndep/src/dyndep.cppm index e94dd800..145602d8 100644 --- a/modules/dyndep/src/dyndep.cppm +++ b/modules/dyndep/src/dyndep.cppm @@ -54,8 +54,18 @@ struct DyndepOptions { // A unit that provides nothing (implementation unit, plain .cpp) is not // split, so it keeps its single record. bool splitModuleEdges = false; + // mcpp#732: module name -> BMI path, for a unit whose package's closure + // holds a name two packages provide (the plan's module map). A name it + // lists takes that path; any other takes `/`. + const std::map>* moduleMap = nullptr; }; +// The BMI path `name` takes under `opts`: the module map's, or the flat one. +std::string bmi_path_for(std::string_view name, const DyndepOptions& opts); + +// Parse a module map: ` ` per line. +std::map> parse_module_map(std::string_view body); + // Parse a single .ddi JSON body to a UnitInfo. Returns unexpected on JSON error. std::expected parse_ddi(std::string_view body); @@ -188,6 +198,26 @@ std::size_t find_key(std::string_view s, std::size_t start, std::string_view key } // namespace +std::string bmi_path_for(std::string_view name, const DyndepOptions& opts) { + if (opts.moduleMap) + if (auto it = opts.moduleMap->find(name); it != opts.moduleMap->end()) return it->second; + return std::string(opts.bmiDir) + "/" + bmi_basename(name, opts.bmiExt); +} + +std::map> parse_module_map(std::string_view body) { + std::map> out; + while (!body.empty()) { + auto nl = body.find('\n'); + auto line = body.substr(0, nl); + body.remove_prefix(nl == std::string_view::npos ? body.size() : nl + 1); + while (!line.empty() && (line.back() == '\r' || line.back() == ' ')) line.remove_suffix(1); + auto sp = line.find(' '); + if (sp == std::string_view::npos || sp == 0) continue; + out.emplace(std::string(line.substr(0, sp)), std::string(line.substr(sp + 1))); + } + return out; +} + std::string bmi_basename(std::string_view logicalName, std::string_view ext) { std::string out; @@ -207,8 +237,7 @@ namespace { std::vector dyndep_targets(const UnitInfo& u, const DyndepOptions& opts) { std::vector t; if (opts.splitModuleEdges && !u.provides.empty()) { - t.push_back(std::string(opts.bmiDir) + "/" - + bmi_basename(u.provides.front(), opts.bmiExt)); + t.push_back(bmi_path_for(u.provides.front(), opts)); } if (!u.primaryOutput.empty()) t.push_back(u.primaryOutput.string()); return t; @@ -314,8 +343,7 @@ std::string emit_dyndep(const std::vector& units, bool selfProvides = false; for (auto& p : u.provides) if (p == r) { selfProvides = true; break; } if (selfProvides) continue; - std::string bmiDir(opts.bmiDir); - add_implicit(bmiDir + "/" + bmi_basename(r, opts.bmiExt)); + add_implicit(bmi_path_for(r, opts)); } line += "\n restat = 1\n"; out += line; @@ -367,8 +395,7 @@ emit_dyndep_single(const std::filesystem::path& ddiPath, for (auto& p : u->provides) if (p == r) { selfProvides = true; break; } if (selfProvides) continue; if (firstImplicit) { line += " |"; firstImplicit = false; } - std::string bmiDir(opts.bmiDir); - line += " " + bmiDir + "/" + bmi_basename(r, opts.bmiExt); + line += " " + bmi_path_for(r, opts); } line += "\n restat = 1\n"; out += line; diff --git a/modules/toolchain-model/src/model.cppm b/modules/toolchain-model/src/model.cppm index 27f359dc..2612170c 100644 --- a/modules/toolchain-model/src/model.cppm +++ b/modules/toolchain-model/src/model.cppm @@ -441,6 +441,10 @@ struct BmiTraits { std::string_view compileModulesFlag; // " -fmodules" (GCC) | "" std::string_view stdBmiUsePrefix; // "" | " -fmodule-file=std=" | " /reference std=" std::string_view stdCompatBmiUsePrefix; // "" | " -fmodule-file=std.compat=" | " /reference std.compat=" + // Binds one module name to one BMI file for one compile, as `=` + // (mcpp#732: two modules of one name in one build directory). "" for GCC, + // which binds names through a mapper file instead. + std::string_view moduleFileUsePrefix; // "" | " -fmodule-file=" | " /reference " std::string_view moduleOutputPrefix; // "" | " -fmodule-output=" | " /ifcOutput " std::string_view bmiSearchPrefix; // "" | " -fprebuilt-module-path=" | " /ifcSearchDir " // How this compiler is TOLD that a translation unit is a module interface. @@ -647,6 +651,7 @@ BmiTraits bmi_traits(const Toolchain& tc) { .compileModulesFlag = "", .stdBmiUsePrefix = " /reference std=", .stdCompatBmiUsePrefix = " /reference std.compat=", + .moduleFileUsePrefix = " /reference ", .moduleOutputPrefix = " /ifcOutput ", .bmiSearchPrefix = " /ifcSearchDir ", // Pre-existing behaviour, unchanged: cl has always been told @@ -670,6 +675,7 @@ BmiTraits bmi_traits(const Toolchain& tc) { .compileModulesFlag = "", .stdBmiUsePrefix = " -fmodule-file=std=", .stdCompatBmiUsePrefix = " -fmodule-file=std.compat=", + .moduleFileUsePrefix = " -fmodule-file=", .moduleOutputPrefix = " -fmodule-output=", .bmiSearchPrefix = " -fprebuilt-module-path=", .moduleInterfaceLangFlag = " -x c++-module", @@ -689,6 +695,7 @@ BmiTraits bmi_traits(const Toolchain& tc) { .compileModulesFlag = " -fmodules", .stdBmiUsePrefix = "", .stdCompatBmiUsePrefix = "", + .moduleFileUsePrefix = "", .moduleOutputPrefix = "", .bmiSearchPrefix = "", // GCC decides interface-ness from the content (`export module`), so diff --git a/src/build/configure.cppm b/src/build/configure.cppm index 6b996020..db9334c1 100644 --- a/src/build/configure.cppm +++ b/src/build/configure.cppm @@ -62,11 +62,15 @@ stage_configure_prerequisites(const BuildPlan& plan) { if (!unit.servedFromCache || unit.providesModule.empty() || unit.cachedBmi.empty()) continue; - std::string fileName; - fileName.reserve(unit.providesModule.size() + traits.bmiExt.size()); - for (char ch : unit.providesModule) - fileName.push_back(ch == ':' ? '-' : ch); - fileName += traits.bmiExt; + // Where the plan placed the unit's BMI: below its package's directory + // when two packages provide its module name (mcpp#732). + std::string fileName = unit.bmiFile; + if (fileName.empty()) { + fileName.reserve(unit.providesModule.size() + traits.bmiExt.size()); + for (char ch : unit.providesModule) + fileName.push_back(ch == ':' ? '-' : ch); + fileName += traits.bmiExt; + } auto result = stage_one( unit.cachedBmi, plan.outputDir / traits.bmiDir / fileName, diff --git a/src/build/ninja_backend.cppm b/src/build/ninja_backend.cppm index 7226b488..71d1e02d 100644 --- a/src/build/ninja_backend.cppm +++ b/src/build/ninja_backend.cppm @@ -1483,12 +1483,16 @@ std::string emit_ninja_string(const BuildPlan& plan, std::string* placements, // are actually SPLIT bind their record to the BMI. An implementation unit // or a plain .cpp still compiles in one edge whose output is the object, // and a `--target-bmi` there would name an edge nobody declared. + // + // `$module_map` (mcpp#732) names the package's module map when two packages + // of the plan provide one module name; the rule carries it only then, so + // a plan without such a name writes the file it always wrote. append(std::format( "rule cxx_dyndep\n" - " command = $mcpp dyndep --single --bmi-dir {} --bmi-ext {} $bind $expect --output $out $in\n" + " command = $mcpp dyndep --single --bmi-dir {} --bmi-ext {} $bind $expect{} --output $out $in\n" " description = DYNDEP $out\n" " restat = 1\n\n", - traits.bmiDir, traits.bmiExt)); + traits.bmiDir, traits.bmiExt, plan.moduleScopes.empty() ? "" : " $module_map")); // P2: cxx_module preserves BMI timestamps when interface is unchanged. // GCC always updates the .gcm timestamp even if content is identical. @@ -2234,21 +2238,43 @@ std::string emit_ninja_string(const BuildPlan& plan, std::string* placements, // exactly one symbol (`_ZGIW3std`, measured) and an importing TU references // it, so a missed unit is an undefined symbol at link time rather than a // silent miscompile. - std::unordered_map byModule; + // The module map of each package whose closure holds a name two packages + // provide (mcpp#732): module name -> BMI path, as the plan resolved it. + std::map>, std::less<>> scopeBmis; + for (auto const& [pkg, scope] : plan.moduleScopes) { + auto& m = scopeBmis[pkg]; + std::istringstream lines(scope.content); + for (std::string name, path; lines >> name >> path;) m[name] = path; + } + std::unordered_map> byModule; for (auto& cu : plan.compileUnits) - if (!cu.providesModule.empty()) byModule.emplace(cu.providesModule, &cu); + if (!cu.providesModule.empty()) byModule[cu.providesModule].push_back(&cu); + // The unit `importer`'s import of `name` means: the one provider, or the + // one its package's module map names. + auto provider_of = [&](const CompileUnit& importer, + const std::string& name) -> const CompileUnit* { + auto it = byModule.find(name); + if (it == byModule.end()) return nullptr; + if (it->second.size() == 1) return it->second.front(); + auto sc = scopeBmis.find(importer.packageName); + if (sc == scopeBmis.end()) return nullptr; + auto m = sc->second.find(name); + if (m == sc->second.end()) return nullptr; + for (auto const* c : it->second) + if (std::string(traits.bmiDir) + "/" + c->bmiFile == m->second) return c; + return nullptr; + }; auto reaches_std = [&](const CompileUnit& start) { std::vector stack{&start}; - std::unordered_set seen; + std::unordered_set seen; while (!stack.empty()) { const CompileUnit* cu = stack.back(); stack.pop_back(); for (auto& imp : cu->imports) { if (imp == "std" || imp == "std.compat") return true; - if (!seen.insert(imp).second) continue; - if (auto it = byModule.find(imp); it != byModule.end()) - stack.push_back(it->second); + auto const* next = provider_of(*cu, imp); + if (next && seen.insert(next).second) stack.push_back(next); } } return false; @@ -2329,6 +2355,19 @@ std::string emit_ninja_string(const BuildPlan& plan, std::string* placements, s += traits.bmiExt; return s; }; + // The BMI a unit provides, where the plan placed it: below its package's + // directory when two packages provide its module name (mcpp#732). + auto unit_bmi = [&](const mcpp::build::CompileUnit& cu) { + return cu.bmiFile.empty() ? bmi_path(cu.providesModule) + : std::string(traits.bmiDir) + "/" + cu.bmiFile; + }; + // The BMI `cu`'s import of `name` means: its package's module map when it + // has one, and the module's own name otherwise. + auto import_bmi = [&](const mcpp::build::CompileUnit& cu, std::string_view name) { + if (auto sc = scopeBmis.find(cu.packageName); sc != scopeBmis.end()) + if (auto it = sc->second.find(name); it != sc->second.end()) return it->second; + return bmi_path(name); + }; // Rule selection is a pure function of the unit's KIND — never of its // extension. mcpp#272 fixed link-object collection while this stayed @@ -2357,7 +2396,7 @@ std::string emit_ninja_string(const BuildPlan& plan, std::string* placements, traits.moduleInterfaceLangFlag); if (traits.needsExplicitModuleOutput) v += std::format(" module_output ={}{}\n", traits.moduleOutputPrefix, - bmi_path(cu.providesModule)); + unit_bmi(cu)); return v; }; @@ -2425,7 +2464,7 @@ std::string emit_ninja_string(const BuildPlan& plan, std::string* placements, append(" verify = --verify size\n"); staged.push_back(obj); if (!cu.providesModule.empty() && !cu.cachedBmi.empty()) { - auto bmi = bmi_path(cu.providesModule); + auto bmi = unit_bmi(cu); append(std::format("build {} : stage_file {}\n", bmi, escape_ninja_path(cu.cachedBmi))); append(" verify = --verify size\n"); @@ -2557,7 +2596,7 @@ std::string emit_ninja_string(const BuildPlan& plan, std::string* placements, append(std::format(" compile_target = {}\n", escape_ninja_path(cu.object))); append(std::format(" deps_target = {}\n", splitBmi && !cu.providesModule.empty() - ? bmi_path(cu.providesModule) + ? unit_bmi(cu) : escape_ninja_path(cu.object))); if (auto includes = local_include_flags(cu, dial); !includes.empty()) append(std::format(" local_includes ={}\n", includes)); @@ -2645,6 +2684,9 @@ std::string emit_ninja_string(const BuildPlan& plan, std::string* placements, append(std::format("build {} : cxx_dyndep {}\n", dd, ddi)); if (two_phase_ddi.contains(ddi)) append(" bind = --split-module\n"); + if (auto sc = plan.moduleScopes.find(ddiOwner[ddi]); sc != plan.moduleScopes.end()) + append(std::format(" module_map = --module-map {}\n", + escape_ninja_path(sc->second.mapFile))); if (auto it = ddi_expect.find(ddi); it != ddi_expect.end()) append(std::format(" expect = {}\n", it->second)); } @@ -2660,7 +2702,7 @@ std::string emit_ninja_string(const BuildPlan& plan, std::string* placements, if (splitBmi && !cu.providesModule.empty() && cu.kind == mcpp::SourceKind::ModuleInterface) { - const auto bmi = bmi_path(cu.providesModule); + const auto bmi = unit_bmi(cu); const auto obj = escape_ninja_path(cu.object); const auto slot = obj + ".sched"; const auto ddi = (cu.object.parent_path() / cu.source.filename()) @@ -2725,7 +2767,7 @@ std::string emit_ninja_string(const BuildPlan& plan, std::string* placements, if (twoPhase && !cu.providesModule.empty() && cu.kind == mcpp::SourceKind::ModuleInterface) { - const auto bmi = bmi_path(cu.providesModule); + const auto bmi = unit_bmi(cu); const auto obj = escape_ninja_path(cu.object); const auto ddi = (cu.object.parent_path() / cu.source.filename()) .string() + ".ddi"; @@ -2757,7 +2799,7 @@ std::string emit_ninja_string(const BuildPlan& plan, std::string* placements, std::string out_line = "build " + escape_ninja_path(cu.object); if (!cu.providesModule.empty()) { - out_line += " | " + bmi_path(cu.providesModule); + out_line += " | " + unit_bmi(cu); } out_line += std::format(" : {} {}", rule, escape_ninja_path(cu.source)); if (!is_scan_exempt(cu)) { @@ -2769,7 +2811,7 @@ std::string emit_ninja_string(const BuildPlan& plan, std::string* placements, out_line += "\n dyndep = " + it->second; // P2: set bmi_out for the copy_if_different logic in cxx_module. if (!cu.providesModule.empty()) { - out_line += "\n bmi_out = " + bmi_path(cu.providesModule); + out_line += "\n bmi_out = " + unit_bmi(cu); } out_line += "\n"; if (rule == "cxx_module") out_line += module_edge_vars(cu); @@ -2817,7 +2859,7 @@ std::string emit_ninja_string(const BuildPlan& plan, std::string* placements, implicit += " " + escape_ninja_path(std_bmi_dst); continue; } - implicit += " " + bmi_path(imp); + implicit += " " + import_bmi(cu, imp); } } @@ -2825,7 +2867,7 @@ std::string emit_ninja_string(const BuildPlan& plan, std::string* placements, if (!cu.providesModule.empty()) { // Use implicit output (|) so $out only contains the .o file. // GCC writes BMI implicitly; Clang uses -fmodule-output=$bmi_out. - out_line += " | " + bmi_path(cu.providesModule); + out_line += " | " + unit_bmi(cu); } out_line += std::format(" : {} {}", rule, escape_ninja_path(cu.source)); if (!implicit.empty()) @@ -2846,7 +2888,7 @@ std::string emit_ninja_string(const BuildPlan& plan, std::string* placements, } // Clang needs $bmi_out to emit -fmodule-output=$bmi_out if (!cu.providesModule.empty()) { - out_line += " bmi_out = " + bmi_path(cu.providesModule) + "\n"; + out_line += " bmi_out = " + unit_bmi(cu) + "\n"; } if (rule == "cxx_module") out_line += module_edge_vars(cu); append(std::move(out_line)); @@ -3920,6 +3962,16 @@ std::expected NinjaBackend::build(const BuildPlan& plan std::ofstream(listPath, std::ios::binary | std::ios::trunc) << placements; } } + // mcpp#732: the module maps the units of a package read when two packages + // provide one module name. The content's hash is in the name, so a file + // that exists is already right; a changed resolution names a new file. + for (auto const& [pkg, scope] : plan.moduleScopes) { + const auto path = plan.outputDir / scope.mapFile; + std::error_code mec; + if (std::filesystem::exists(path, mec)) continue; + std::filesystem::create_directories(path.parent_path(), mec); + std::ofstream(path, std::ios::binary | std::ios::trunc) << scope.content; + } // Command-length backstop (see // .agents/docs/2026-08-06-command-length-architecture.md). The structural diff --git a/src/build/plan.cppm b/src/build/plan.cppm index 72773f47..a7c5c0bf 100644 --- a/src/build/plan.cppm +++ b/src/build/plan.cppm @@ -56,6 +56,12 @@ struct CompileUnit { // (`_SMF_control` has no matching constructor), and the error names this // struct from whichever unit copies a CompileUnit first. std::string providesModule; + // The unit's BMI, relative to the toolchain's BMI directory: the module's + // name (`common.gcm`) when one package of the plan provides it, and below + // the provider's directory (`gpp.updater/common.gcm`) when two do + // (mcpp#732). Empty on a unit that provides nothing, and on a unit built + // by hand, which then takes the name. + std::string bmiFile; std::vector imports; // logical names imported // Unit came from a scan_overrides declaration — plan-vs-ddi // verification is mandatory for it (ninja_backend emits --expect-*). @@ -310,6 +316,20 @@ struct PlanPackage { std::string source; }; +// mcpp#732: a package whose closure holds a module name that two packages of +// the plan provide. Its units are told which BMI each name means: through +// `mapFile` for GCC, whose mapper does not fall back for a name it does not +// list, and by one flag per such name for clang and MSVC. The dyndep step +// reads `mapFile` on every compiler. +struct ModuleScope { + // Relative to the build directory, with a hash of `content` in its name, + // so a changed resolution changes the flag that names it. + std::filesystem::path mapFile; + // ` ` lines, one + // per name the package's units may import, sorted by name. + std::string content; +}; + struct BuildPlan { mcpp::manifest::Manifest manifest; // Every package of the graph, as the report names it; the backend writes @@ -417,6 +437,10 @@ struct BuildPlan { std::filesystem::path stdBmiPath; // absolute path to prebuilt std.gcm std::filesystem::path stdObjectPath; // absolute path to prebuilt std.o std::filesystem::path stdCompatBmiPath; // absolute path to prebuilt std.compat.pcm + // mcpp#732: by qualified package name. Empty when every module name of the + // plan has one provider, and then the build directory is laid out, and + // every command spelled, exactly as before. + std::map> moduleScopes; std::filesystem::path stdCompatObjectPath; // absolute path to prebuilt std.compat.o std::filesystem::path scanDepsPath; // clang-scan-deps binary (Clang only) // NASM assembly (.asm sources). Both resolved in prepare AFTER the plan @@ -2106,14 +2130,6 @@ make_plan(const mcpp::manifest::Manifest& manifest, } } - // 2. Build map of module-name → compile unit (for inter-unit dep resolution) - std::map producerOf; - for (std::size_t i = 0; i < plan.compileUnits.size(); ++i) { - if (!plan.compileUnits[i].providesModule.empty()) { - producerOf[plan.compileUnits[i].providesModule] = i; - } - } - // 3. Compute the set of all targets' entry .cpp files. Each entry is // exclusive to its target — when assembling another target's link // image we must NOT pull in foreign entries (they each define @@ -3162,6 +3178,101 @@ make_plan(const mcpp::manifest::Manifest& manifest, if (lu.kind == LinkUnit::SharedLibrary) { plan.needsPic = true; break; } } + // MODULE NAMES ARE RESOLVED IN THE IMPORTER'S CLOSURE (mcpp#732). A module + // name identifies one module within one program, and a plan holds several + // programs (a package and the programs it ships through `artifacts`, a + // workspace's members), so two packages of one plan may each provide + // `common` when no closure holds both (the scanner refuses the rest). + // Every compiler finds a BMI by name in one directory, so the two BMIs go + // below their providers' directories, and the units that may import such a + // name are told which one it means. With every name provided once, none of + // this applies: no unit moves, no flag is added. At the end of make_plan, + // after every producer of a compile unit (a target's `main` included). + { + const auto traits = mcpp::toolchain::bmi_traits(tc); + std::set> collided; + for (auto const& [name, units] : graph.providersOf) + if (units.size() > 1) collided.insert(name); + auto basename = [&](std::string_view name) { + std::string out; + for (char c : name) out.push_back(c == ':' ? '-' : c); + out += traits.bmiExt; + return out; + }; + auto bmi_file = [&](std::string_view provider, std::string_view name) { + return collided.contains(name) ? std::string(provider) + "/" + basename(name) + : basename(name); + }; + for (auto& cu : plan.compileUnits) + if (!cu.providesModule.empty()) + cu.bmiFile = bmi_file(cu.packageName, cu.providesModule); + + if (!collided.empty()) { + const bool gcc = traits.moduleFileUsePrefix.empty(); + std::set> packagesWithUnits; + for (auto const& cu : plan.compileUnits) packagesWithUnits.insert(cu.packageName); + std::map, std::less<>> flagsOf; + for (auto const& pkg : packagesWithUnits) { + std::vector> entries; // name -> BMI + std::vector> bound; // collided ones + for (auto const& [name, units] : graph.providersOf) { + auto provider = mcpp::modgraph::resolve_provider(graph, pkg, name); + if (!provider) continue; + const auto path = std::string(traits.bmiDir) + "/" + + bmi_file(graph.units[*provider].packageName, name); + entries.emplace_back(name, path); + if (collided.contains(name)) bound.emplace_back(name, path); + } + if (bound.empty()) continue; // sees no collided name + // GCC's mapper answers only what it lists: the standard + // library modules are listed where GCC's own mapper puts them. + if (gcc) { + entries.emplace_back("std", std::string(traits.bmiDir) + "/" + basename("std")); + entries.emplace_back("std.compat", + std::string(traits.bmiDir) + "/" + basename("std.compat")); + } + std::sort(entries.begin(), entries.end()); + ModuleScope scope; + for (auto const& [name, path] : entries) scope.content += name + " " + path + "\n"; + scope.mapFile = std::filesystem::path("modmap") + / std::format("{}-{}.map", pkg, + mcpp::toolchain::hash_string(scope.content).substr(0, 8)); + auto& flags = flagsOf[pkg]; + if (gcc) { + flags.push_back(mcpp::manifest::flag_element( + "-fmodule-mapper=" + scope.mapFile.generic_string())); + } else { + // `-fmodule-file==` (clang) or `/reference + // =` (MSVC), with an absolute path so the + // compile databases, whose directory is the project, + // name the same file. + std::string_view prefix = traits.moduleFileUsePrefix; + while (!prefix.empty() && prefix.front() == ' ') prefix.remove_prefix(1); + const bool separate = !prefix.empty() && prefix.back() == ' '; + if (separate) prefix.remove_suffix(1); + for (auto const& [name, path] : bound) { + const auto value = name + "=" + (outputDir / path).string(); + if (separate) { + flags.push_back(std::string(prefix)); + flags.push_back(mcpp::manifest::flag_element(value)); + } else { + flags.push_back(mcpp::manifest::flag_element(std::string(prefix) + value)); + } + } + } + plan.moduleScopes.emplace(pkg, std::move(scope)); + } + for (auto& cu : plan.compileUnits) { + if (cu.kind != mcpp::SourceKind::ModuleInterface + && cu.kind != mcpp::SourceKind::Cxx) + continue; + if (auto f = flagsOf.find(cu.packageName); f != flagsOf.end()) + cu.packageCxxflags.insert(cu.packageCxxflags.end(), + f->second.begin(), f->second.end()); + } + } + } + return plan; } diff --git a/src/build/prepare/plan.cpp b/src/build/prepare/plan.cpp index db34831f..dc08617c 100644 --- a/src/build/prepare/plan.cpp +++ b/src/build/prepare/plan.cpp @@ -2049,6 +2049,11 @@ static std::expected step13_dependency_cache(PrepareState& st // consumer three edges away, which is far harder to read than // one extra compile. if (cu.packageObjectRel.empty()) { addressable = false; break; } + // A BMI below its package's directory (a module name two + // packages of the plan provide, mcpp#732) has no address in + // the entry, whose BMIs are named by module: the package + // compiles here instead of being cached. + if (cu.bmiFile.find('/') != std::string::npos) { addressable = false; break; } if (!cu.providesModule.empty()) { std::string bmi; diff --git a/src/build/prepare/scan.cpp b/src/build/prepare/scan.cpp index 4d845574..94ef2ac1 100644 --- a/src/build/prepare/scan.cpp +++ b/src/build/prepare/scan.cpp @@ -75,6 +75,43 @@ static std::expected step11_scan_sources(PrepareState& state) for (std::size_t i = 0; i < state.packages.size(); ++i) if (state.compilesHere(i)) scannedPackages.push_back(state.packages[i]); + + // THE CLOSURE OF EACH PACKAGE THIS PLAN COMPILES (mcpp#732): the package + // and every package it reaches through code and workspace-member edges. A + // module name is unique within one program, and a program links its root + // package's closure. An `artifacts` edge ships a separate program and a + // `[build-dependencies]` edge serves the build, so neither is followed. A + // workspace plan's virtual root compiles nothing and has no closure, so + // two members that share no program may each provide a module of one name. + mcpp::modgraph::Closures closures; + { + auto qualified = [](const mcpp::manifest::Manifest& m) { + return m.package.namespace_.empty() ? m.package.name + : m.package.namespace_ + "." + m.package.name; + }; + std::map> codeEdges; + for (auto const& e : state.dependencyEdges) { + if (!e.requestedArtifacts.empty() || e.buildOnly) continue; + codeEdges[e.consumerPackageIndex].push_back(e.dependencyPackageIndex); + } + for (std::size_t i = 0; i < state.packages.size(); ++i) { + if (!state.compilesHere(i)) continue; + if (i == 0 && state.m->package.virtualRoot) continue; + std::set names; + std::set seen{i}; + std::vector work{i}; + while (!work.empty()) { + const auto k = work.back(); + work.pop_back(); + names.insert(qualified(state.packages[k].manifest)); + if (auto it = codeEdges.find(k); it != codeEdges.end()) + for (auto j : it->second) + if (seen.insert(j).second) work.push_back(j); + } + closures[qualified(state.packages[i].manifest)] = std::move(names); + } + } + state.scan = [&] { const char* sel = std::getenv("MCPP_SCANNER"); if (sel && std::string_view(sel) == "p1689") { @@ -82,9 +119,9 @@ static std::expected step11_scan_sources(PrepareState& state) / std::format("mcpp_p1689_{}", std::random_device{}()); std::filesystem::create_directories(tmp); return mcpp::modgraph::scan_packages_p1689(scannedPackages, *state.tc, tmp, - state.stdFlagAndDialect); + state.stdFlagAndDialect, closures); } - return mcpp::modgraph::scan_packages(scannedPackages); + return mcpp::modgraph::scan_packages(scannedPackages, closures); }(); if (!state.scan.errors.empty()) { std::string msg = "scanner errors:\n"; @@ -879,14 +916,18 @@ static void step11_public_module_check(PrepareState& state) { }; const std::string rootName = qualified(state.packages[0].manifest); - std::map providerOf; // primary module -> package + // The package a root import means, by the resolver every other reader + // uses (mcpp#732): a name two packages provide means the one in the + // root's closure. + auto providerOf = [&](const std::string& prim) -> std::optional { + if (auto p = mcpp::modgraph::resolve_provider(g, rootName, prim)) + return g.units[*p].packageName; + return std::nullopt; + }; std::map> unitsOf; for (auto const& u : g.units) { if (!u.provides) continue; - const auto prim = primary(u.provides->logicalName); - unitsOf[prim].push_back(&u); - if (u.provides->logicalName.find(':') == std::string::npos) - providerOf.emplace(prim, u.packageName); + unitsOf[primary(u.provides->logicalName)].push_back(&u); } // A member of the root's own workspace is built from source together with @@ -920,9 +961,9 @@ static void step11_public_module_check(PrepareState& state) { if (u.packageName != rootName) continue; for (auto const& req : u.requires_) { const auto prim = primary(req.logicalName); - auto prov = providerOf.find(prim); - if (prov == providerOf.end() || prov->second == rootName) continue; - auto pub = publicOf.find(prov->second); + auto prov = providerOf(prim); + if (!prov || *prov == rootName) continue; + auto pub = publicOf.find(*prov); if (pub == publicOf.end() || pub->second.contains(prim)) continue; if (!warned.insert(prim).second) continue; std::string names; @@ -931,10 +972,10 @@ static void step11_public_module_check(PrepareState& state) { mcpp::diag::Severity::Warning, "build/interface", std::format("'{}' imports '{}' of '{}', which is not one of that " "package's public modules", u.relPath.generic_string(), - prim, prov->second), + prim, *prov), std::format("the build succeeds from source, and fails against the " - "packed form of '{}'", prov->second), - std::format("import a public module of '{}': {}", prov->second, names)); + "packed form of '{}'", *prov), + std::format("import a public module of '{}': {}", *prov, names)); } } } diff --git a/src/cli.cppm b/src/cli.cppm index 9222788c..212b96d7 100644 --- a/src/cli.cppm +++ b/src/cli.cppm @@ -928,6 +928,8 @@ int run(int argc, char** argv) { .help("BMI cache directory name (default: gcm.cache)")) .option(cl::Option("bmi-ext").takes_value().value_name("EXT") .help("BMI file extension (default: .gcm)")) + .option(cl::Option("module-map").takes_value().value_name("FILE") + .help("Module name -> BMI path, one ` ` per line (mcpp#732)")) .option(cl::Option("split-module") .help("Also emit a record for the provided BMI (two-phase " "schedule: BMI and object are separate edges)")) diff --git a/src/cli/cmd_build.cppm b/src/cli/cmd_build.cppm index 3c237523..0e3f94de 100644 --- a/src/cli/cmd_build.cppm +++ b/src/cli/cmd_build.cppm @@ -1222,6 +1222,19 @@ export int cmd_dyndep(const mcpplibs::cmdline::ParsedArgs& parsed) { if (!bmiExtStorage.empty()) opts.bmiExt = bmiExtStorage; opts.splitModuleEdges = parsed.is_flag_set("split-module"); + // mcpp#732: the package's module map, when two packages of the plan + // provide one module name. + std::map> moduleMap; + if (auto mm = parsed.option_or_empty("module-map").value(); !mm.empty()) { + std::ifstream is{mcpp::platform::fs::extended_length(std::filesystem::path{mm})}; + if (!is) { + std::println(stderr, "error: cannot read module map '{}'", mm); + return 1; + } + std::string mapBody{std::istreambuf_iterator(is), {}}; + moduleMap = mcpp::dyndep::parse_module_map(mapBody); + opts.moduleMap = &moduleMap; + } std::expected body; if (single) { diff --git a/src/modgraph/graph.cppm b/src/modgraph/graph.cppm index ea3dab88..b30c9ade 100644 --- a/src/modgraph/graph.cppm +++ b/src/modgraph/graph.cppm @@ -113,14 +113,45 @@ struct SourceUnit { bool scanOverridden = false; }; +// A package's closure: the qualified names of the packages whose modules its +// units may import, itself included (mcpp#732). It is the package and every +// package it reaches through code and workspace-member edges; an `artifacts` +// edge ships a separate program and is not followed. +using Closures = std::map, std::less<>>; + struct Graph { std::vector units; - // logical-name -> index into units + // logical-name -> index into units, for a name ONE unit of the graph + // provides. A name two packages provide (mcpp#732) is in providersOf only. std::map> producerOf; + // logical-name -> every unit that provides it, in unit order. + std::map, std::less<>> providersOf; + // The closure of each package the plan compiles. Empty when the caller has + // none to give, and then a name has to be unique in the whole graph. + Closures closures; // edges as (consumer-index, producer-index) std::vector> edges; }; +// WHICH PROVIDER AN IMPORT MEANS (mcpp#732). A module name identifies one +// module within one program: GCC and clang mangle a module's entities with its +// name and give it one initializer named after it, so two modules of one name +// cannot be linked into one program, and two programs may each have one. A +// build configuration holds several programs (a package and the programs it +// ships through `artifacts`, a workspace's members), so a name is resolved in +// the importer's closure, not in the whole configuration. +// +// Among `providerPackages`, the packages that provide one name, the index of +// the one `importer` means: the only provider, or else the only one in the +// importer's closure. Nothing when the closure holds none of them or more than +// one. The one rule the scanner, the plan, the backend and the packer use. +std::optional choose_provider(std::span providerPackages, + std::string_view importer, + const Closures& closures); +// The unit of `g` that `importer`'s import of `name` means. +std::optional resolve_provider(const Graph& g, std::string_view importer, + std::string_view name); + // Topological order: returns indices of units in producer-before-consumer order. // Returns std::unexpected with the cycle if any, as the ordered path that // walks it (see mcpp.graph) rather than merely the units left over. @@ -133,6 +164,38 @@ std::expected, CycleError> topo_sort(const Graph& g); namespace mcpp::modgraph { +std::optional choose_provider(std::span providerPackages, + std::string_view importer, + const Closures& closures) { + if (providerPackages.size() == 1) return 0; + auto closure = closures.find(importer); + if (closure == closures.end()) return std::nullopt; + std::optional found; + for (std::size_t i = 0; i < providerPackages.size(); ++i) { + if (!closure->second.contains(providerPackages[i])) continue; + if (found) return std::nullopt; + found = i; + } + return found; +} + +std::optional resolve_provider(const Graph& g, std::string_view importer, + std::string_view name) { + auto it = g.providersOf.find(name); + if (it == g.providersOf.end() || it->second.empty()) { + // A graph built without `providersOf` (by hand, in a test) states its + // single providers in `producerOf`. + if (auto p = g.producerOf.find(name); p != g.producerOf.end()) return p->second; + return std::nullopt; + } + std::vector packages; + packages.reserve(it->second.size()); + for (auto i : it->second) packages.push_back(g.units[i].packageName); + auto chosen = choose_provider(packages, importer, g.closures); + if (!chosen) return std::nullopt; + return it->second[*chosen]; +} + std::expected, CycleError> topo_sort(const Graph& g) { // g.edges: (consumer, producer) pairs, "consumer depends on producer". // The unit order is the order of the objects on a link line, which diff --git a/src/modgraph/scanner.cppm b/src/modgraph/scanner.cppm index f6ee2041..3e4950d7 100644 --- a/src/modgraph/scanner.cppm +++ b/src/modgraph/scanner.cppm @@ -157,7 +157,10 @@ struct PackageRoot { bool selectedMember = false; std::string memberProducts; }; -ScanResult scan_packages(const std::vector& packages); +// `closures` states each compiled package's closure (mcpp#732); a module name +// is then unique per closure, and without them in the whole graph. +ScanResult scan_packages(const std::vector& packages, + const Closures& closures = {}); // Drop-in replacement that delegates per-file scanning to GCC's P1689r5 // (.ddi) output instead of regex parsing. Same ScanResult shape — used by @@ -165,7 +168,8 @@ ScanResult scan_packages(const std::vector& packages); ScanResult scan_packages_p1689(const std::vector& packages, const mcpp::toolchain::Toolchain& tc, const std::filesystem::path& tmpDir, - std::string_view cppStandardFlag); + std::string_view cppStandardFlag, + const Closures& closures = {}); } // namespace mcpp::modgraph @@ -1506,45 +1510,94 @@ void scan_one_into(ScanResult& result, } } -// Phase 2: producerOf + edges over already-collected units. +// Phase 2: the providers of each name, the check that a closure holds one of +// them, and the edges, over already-collected units (mcpp#732). void resolve_graph(ScanResult& result) { auto& g = result.graph; - for (std::size_t i = 0; i < g.units.size(); ++i) { - auto& u = g.units[i]; - if (u.provides) { - auto [it, inserted] = g.producerOf.emplace(u.provides->logicalName, i); - if (!inserted) { - // Name both packages: the same file reached as two packages - // and two packages that happen to pick one module name are - // different defects, and only the package names tell them - // apart. - auto const& first = g.units[it->second]; - result.errors.push_back(ScanError{ - u.path, 0, - std::format("module '{}' is provided by package '{}' ({}) " - "and by package '{}' ({}){}", - u.provides->logicalName, - first.packageName, first.path.string(), - u.packageName, u.path.string(), - first.path == u.path - ? "; one file is reached as two packages" - : "")}); - } + for (std::size_t i = 0; i < g.units.size(); ++i) + if (g.units[i].provides) + g.providersOf[g.units[i].provides->logicalName].push_back(i); + for (auto const& [name, units] : g.providersOf) + if (units.size() == 1) g.producerOf.emplace(name, units.front()); + + // TWO PROVIDERS OF ONE NAME MAY NOT MEET IN ONE CLOSURE. A program links + // the closure of its root package, and a program that links two modules + // of one name defines their entities twice (`value@common()`, and the + // module's initializer). Two programs may each have one. Without closures + // the name has to be unique in the whole graph, as before. + // + // Name both packages: the same file reached as two packages and two + // packages that happen to pick one module name are different defects, and + // only the package names tell them apart. The same file is decided by + // identity, not spelling: `a/../x.ixx` and `b/../x.ixx` are one file. + auto same_file = [](const std::filesystem::path& a, const std::filesystem::path& b) { + std::error_code ec; + return a.lexically_normal() == b.lexically_normal() + || std::filesystem::equivalent(a, b, ec); + }; + auto refuse = [&](std::string_view name, const SourceUnit& first, const SourceUnit& second, + std::string_view where) { + result.errors.push_back(ScanError{ + second.path, 0, + std::format("module '{}' is provided by package '{}' ({}) and by package '{}' ({}){}{}", + name, first.packageName, first.path.string(), + second.packageName, second.path.string(), where, + same_file(first.path, second.path) + ? "; one file is reached as two packages, and a package that " + "both depend on would provide it once" + : "")}); + }; + std::set> withUnits; + for (auto const& u : g.units) withUnits.insert(u.packageName); + for (auto const& [name, units] : g.providersOf) { + if (units.size() < 2) continue; + if (g.closures.empty()) { + refuse(name, g.units[units[0]], g.units[units[1]], ""); + continue; + } + for (auto const& [package, closure] : g.closures) { + if (!withUnits.contains(package)) continue; + std::vector inClosure; + for (auto u : units) + if (closure.contains(g.units[u].packageName)) inClosure.push_back(u); + if (inClosure.size() < 2) continue; + refuse(name, g.units[inClosure[0]], g.units[inClosure[1]], + std::format(", and both are in the closure of package '{}': a program " + "that links both defines the module twice", package)); + break; // one statement per name } } + for (std::size_t i = 0; i < g.units.size(); ++i) { auto& u = g.units[i]; for (auto const& req : u.requires_) { - auto it = g.producerOf.find(req.logicalName); - if (it == g.producerOf.end()) { - if (req.logicalName == "std" || req.logicalName == "std.compat") continue; + if (auto p = resolve_provider(g, u.packageName, req.logicalName)) { + g.edges.emplace_back(i, *p); + continue; + } + if (req.logicalName == "std" || req.logicalName == "std.compat") continue; + auto it = g.providersOf.find(req.logicalName); + if (it == g.providersOf.end()) { result.warnings.push_back(ScanError{ u.path, 0, std::format("module '{}' imported but not provided in this build", req.logicalName)}); continue; } - g.edges.emplace_back(i, it->second); + // Two or more providers, and none in the importer's closure (two in + // it, or any two without closures, are refused above). + if (g.closures.empty()) continue; + std::size_t inClosure = 0; + if (auto c = g.closures.find(u.packageName); c != g.closures.end()) + for (auto p : it->second) + if (c->second.contains(g.units[p].packageName)) ++inClosure; + if (inClosure > 1) continue; + result.errors.push_back(ScanError{ + u.path, 0, + std::format("module '{}' is provided by {} packages, and none of them is " + "a dependency of package '{}'; declare the dependency on the " + "one it means", req.logicalName, it->second.size(), + u.packageName)}); } } } @@ -1563,8 +1616,10 @@ ScanResult scan_package(const std::filesystem::path& root, return result; } -ScanResult scan_packages(const std::vector& packages) { +ScanResult scan_packages(const std::vector& packages, + const Closures& closures) { ScanResult result; + result.graph.closures = closures; for (auto const& p : packages) { auto localIncludeDirs = p.usageResolved ? p.privateBuild.includeDirs @@ -1601,9 +1656,11 @@ ScanResult scan_packages(const std::vector& packages) { ScanResult scan_packages_p1689(const std::vector& packages, const mcpp::toolchain::Toolchain& tc, const std::filesystem::path& tmpDir, - std::string_view cppStandardFlag) + std::string_view cppStandardFlag, + const Closures& closures) { ScanResult result; + result.graph.closures = closures; for (auto const& p : packages) { // Same contract as scan_one_into: each package's own table. const auto extTable = diff --git a/src/pack/interface.cppm b/src/pack/interface.cppm index c9be2518..8142af6a 100644 --- a/src/pack/interface.cppm +++ b/src/pack/interface.cppm @@ -120,18 +120,23 @@ interface_closure(const mcpp::modgraph::Graph& graph, return u.packageName == packageName; }; - auto rootIt = graph.producerOf.find(rootModule); - if (rootIt == graph.producerOf.end()) { + // The package's own import of a name, resolved as every import is + // (mcpp#732): a name two packages provide means the one in its closure. + auto provider = [&](std::string_view name) { + return mcpp::modgraph::resolve_provider(graph, packageName, name); + }; + const auto root = provider(rootModule); + if (!root) { return std::unexpected(std::format( "no module interface unit in this build provides '{}'", rootModule)); } - if (!owned(graph.units[rootIt->second])) { + if (!owned(graph.units[*root])) { return std::unexpected(std::format( "module '{}' is provided by package '{}', not '{}'", rootModule, - graph.units[rootIt->second].packageName, packageName)); + graph.units[*root].packageName, packageName)); } - std::vector stack{ rootIt->second }; + std::vector stack{ *root }; std::set seen; std::set unresolved; @@ -154,8 +159,8 @@ interface_closure(const mcpp::modgraph::Graph& graph, } for (auto const& req : u.requires_) { - auto it = graph.producerOf.find(req.logicalName); - if (it == graph.producerOf.end()) { + const auto found = provider(req.logicalName); + if (!found) { // Only OUR module's partitions are our problem. A bare name // with no producer is a dependency's module (or `std`), which // this package does not publish and must not complain about. @@ -163,8 +168,8 @@ interface_closure(const mcpp::modgraph::Graph& graph, if (ours) unresolved.insert(req.logicalName); continue; } - if (!owned(graph.units[it->second])) continue; // a dependency's unit - if (!seen.contains(it->second)) stack.push_back(it->second); + if (!owned(graph.units[*found])) continue; // a dependency's unit + if (!seen.contains(*found)) stack.push_back(*found); } } diff --git a/tests/e2e/847_a_module_name_is_unique_within_a_program.sh b/tests/e2e/847_a_module_name_is_unique_within_a_program.sh new file mode 100755 index 00000000..de5a4b13 --- /dev/null +++ b/tests/e2e/847_a_module_name_is_unique_within_a_program.sh @@ -0,0 +1,221 @@ +#!/usr/bin/env bash +# requires: +# 847 -- a module name is unique within one program, not within one build +# (mcpp#732). +# +# GCC and clang mangle a module's entities with its name and give the module +# one initializer named after it, so two modules of one name cannot be linked +# into one program (`multiple definition of value@common()`), and two programs +# may each have one. A build holds several programs -- a package and the +# programs it ships through `artifacts`, a workspace's members -- and mcpp +# refused a name two packages of one build provided, whichever programs they +# belonged to. An import is now resolved in the importer's closure, the two +# BMIs lie below their packages' directories, and each compile is told which +# one a name means. +# +# Criteria, with the default toolchain (GCC on Linux, clang on macOS and +# Windows): +# A. An app and its `artifacts` updater each provide a different module `boost`: +# the build succeeds, each program prints its own module's value, and the +# two BMIs lie below their packages' directories. +# B. Editing the updater's `boost` rebuilds the updater and not the app. +# C. Two independent members of one workspace, each with its own `boost`: +# `--workspace` builds both, each with its own value. +# D. The reported layout: one file listed by two packages through `..`, in +# two programs. Each program has one `boost`, so it builds. +# E. Two packages that one program links both provide `boost`: refused, +# naming the program's package. +# F. One file reached twice within one closure: refused, and named as one +# file reached as two packages. +# G. A build whose module names each have one provider writes no module map +# and no binding flag: its build directory is laid out as before. +set -e + +TMP=$(mktemp -d) +trap 'rm -rf "$TMP"' EXIT +fail() { echo "FAIL: $1"; shift; for f in "$@"; do echo "--- $f ---"; cat "$f" 2>/dev/null; done; exit 1; } + +EXE="" +case "$(uname -s)" in MINGW*|MSYS*|CYGWIN*) EXE=".exe" ;; esac +cd "$TMP" + +# module_file : the module `boost`, whose value() is . +# main_file : a main that prints the value of the `boost` it imports. +# (`common`, the name in mcpp#732, is one of the top-level names mcpp's +# naming rule refuses; the reporter's own module is `boost`.) +module_file() { printf 'export module boost;\nexport int value() { return %s; }\n' "$2" > "$1"; } +main_file() { + printf '#include \nimport boost;\nint main() { std::printf("%%d\\n", value()); }\n' > "$1" +} +bin_of() { find "$1" -path "*/bin/*" -name "$2$EXE" -type f | head -1; } + +# ── A ────────────────────────────────────────────────────────────────────── +mkdir -p a/app/src a/updater/src +module_file a/app/src/boost.cppm 1 +main_file a/app/src/main.cpp +module_file a/updater/src/boost.cppm 2 +main_file a/updater/src/main.cpp +cat > a/updater/mcpp.toml <<'EOF' +[package] +name = "updater" +version = "0.1.0" + +[targets.updater] +kind = "bin" +main = "src/main.cpp" +EOF +cat > a/app/mcpp.toml <<'EOF' +[package] +name = "app" +version = "0.1.0" + +[dependencies] +updater = { path = "../updater", artifacts = ["updater"] } + +[targets.app] +kind = "bin" +main = "src/main.cpp" +EOF +(cd a/app && "$MCPP" build > "$TMP/a.log" 2>&1) || fail "A: the build was refused" a.log +app=$(bin_of a/app/target app); upd=$(bin_of a/app/target updater) +[ -n "$app" ] && [ -n "$upd" ] || fail "A: a program is missing" a.log +[ "$("$app" | tr -d '\r')" = 1 ] || fail "A: the app does not print its own module's value" +[ "$("$upd" | tr -d '\r')" = 2 ] || fail "A: the updater does not print its own module's value" +nb=$(find a/app/target -path '*.cache/*/boost.*' -type f | wc -l) +[ "$nb" -eq 2 ] || { find a/app/target -name 'boost.*'; fail "A: expected two BMIs below their packages' directories, found $nb"; } +echo "ok: A, an app and its artifacts updater each have their own boost" + +# ── B ────────────────────────────────────────────────────────────────────── +touch "$TMP/marker" +sleep 1 +module_file a/updater/src/boost.cppm 3 +(cd a/app && "$MCPP" build > "$TMP/b.log" 2>&1) || fail "B: the rebuild failed" b.log +[ "$("$upd" | tr -d '\r')" = 3 ] || fail "B: the updater did not take its edited module" +[ "$("$app" | tr -d '\r')" = 1 ] || fail "B: the app changed its value" +[ -z "$(find "$(dirname "$app")" -name "app$EXE" -newer "$TMP/marker")" ] \ + || fail "B: editing the updater's module relinked the app" b.log +echo "ok: B, an edit of one boost rebuilt its own program only" + +# ── C ────────────────────────────────────────────────────────────────────── +mkdir -p c/one/src c/two/src +module_file c/one/src/boost.cppm 5; main_file c/one/src/main.cpp +module_file c/two/src/boost.cppm 6; main_file c/two/src/main.cpp +cat > c/mcpp.toml <<'EOF' +[workspace] +members = ["one", "two"] +EOF +for m in one two; do + cat > c/$m/mcpp.toml < "$TMP/c.log" 2>&1) || fail "C: the workspace build was refused" c.log +one=$(bin_of c/target one); two=$(bin_of c/target two) +[ -n "$one" ] && [ -n "$two" ] || fail "C: a member's program is missing" c.log +[ "$("$one" | tr -d '\r')" = 5 ] && [ "$("$two" | tr -d '\r')" = 6 ] \ + || fail "C: a member does not print its own module's value" +echo "ok: C, two workspace members each have their own boost" + +# ── D ────────────────────────────────────────────────────────────────────── +mkdir -p d/shared d/gui/src d/upd/src +module_file d/shared/boost.cppm 9 +main_file d/gui/src/main.cpp; main_file d/upd/src/main.cpp +cat > d/upd/mcpp.toml <<'EOF' +[package] +name = "upd" +version = "0.1.0" + +[build] +sources = ["../shared/boost.cppm"] + +[targets.upd] +kind = "bin" +main = "src/main.cpp" +EOF +cat > d/gui/mcpp.toml <<'EOF' +[package] +name = "gui" +version = "0.1.0" + +[dependencies] +upd = { path = "../upd", artifacts = ["upd"] } + +[build] +sources = ["../shared/boost.cppm"] + +[targets.gui] +kind = "bin" +main = "src/main.cpp" +EOF +(cd d/gui && "$MCPP" build > "$TMP/d.log" 2>&1) || fail "D: one file in two programs was refused" d.log +[ "$("$(bin_of d/gui/target gui)" | tr -d '\r')" = 9 ] && [ "$("$(bin_of d/gui/target upd)" | tr -d '\r')" = 9 ] \ + || fail "D: a program does not run" d.log +echo "ok: D, one file compiled in two programs builds" + +# ── E ────────────────────────────────────────────────────────────────────── +mkdir -p e/lib1/src e/lib2/src e/prog/src +module_file e/lib1/src/boost.cppm 1; module_file e/lib2/src/boost.cppm 2 +printf '#include \nint main() { std::printf("x\\n"); }\n' > e/prog/src/main.cpp +for l in lib1 lib2; do + printf '[package]\nname = "%s"\nversion = "0.1.0"\n' "$l" > e/$l/mcpp.toml +done +cat > e/prog/mcpp.toml <<'EOF' +[package] +name = "prog" +version = "0.1.0" + +[dependencies] +lib1 = { path = "../lib1" } +lib2 = { path = "../lib2" } + +[targets.prog] +kind = "bin" +main = "src/main.cpp" +EOF +if (cd e/prog && "$MCPP" build > "$TMP/e.log" 2>&1); then fail "E: two providers in one program were accepted" e.log; fi +grep -q "module 'boost' is provided by package" e.log && grep -q "closure of package 'prog'" e.log \ + || fail "E: the refusal does not name the program's package" e.log +echo "ok: E, two providers in one program are refused" + +# ── F ────────────────────────────────────────────────────────────────────── +mkdir -p f/shared f/lib3 f/lib4 f/prog/src +module_file f/shared/boost.cppm 1 +printf '#include \nint main() { std::printf("x\\n"); }\n' > f/prog/src/main.cpp +for l in lib3 lib4; do + printf '[package]\nname = "%s"\nversion = "0.1.0"\n\n[build]\nsources = ["../shared/boost.cppm"]\n' "$l" > f/$l/mcpp.toml +done +cat > f/prog/mcpp.toml <<'EOF' +[package] +name = "prog" +version = "0.1.0" + +[dependencies] +lib3 = { path = "../lib3" } +lib4 = { path = "../lib4" } + +[targets.prog] +kind = "bin" +main = "src/main.cpp" +EOF +if (cd f/prog && "$MCPP" build > "$TMP/f.log" 2>&1); then fail "F: one file twice in one program was accepted" f.log; fi +grep -q "one file is reached as two packages" f.log \ + || fail "F: the refusal does not say that one file is reached as two packages" f.log +echo "ok: F, one file reached twice in one program is refused as such" + +# ── G ────────────────────────────────────────────────────────────────────── +(cd a/updater && "$MCPP" build > "$TMP/g.log" 2>&1) || fail "G: a plain build failed" g.log +[ -z "$(find a/updater/target -type d -name modmap)" ] || fail "G: a plan without a collision wrote a module map" +ninja=$(find a/updater/target -name build.ninja | head -1) +[ -n "$ninja" ] || fail "G: no build.ninja" +if grep -q 'module-map\|fmodule-mapper\|cache/[^ ]*/boost\.' "$ninja"; then + fail "G: a plan without a collision binds a module name" "$ninja" +fi +echo "ok: G, a plan without a collision is laid out as before" + +echo "PASS: 847_a_module_name_is_unique_within_a_program" diff --git a/tests/e2e/848_msvc_binds_a_module_name_per_program.sh b/tests/e2e/848_msvc_binds_a_module_name_per_program.sh new file mode 100755 index 00000000..fdc72ccb --- /dev/null +++ b/tests/e2e/848_msvc_binds_a_module_name_per_program.sh @@ -0,0 +1,53 @@ +#!/usr/bin/env bash +# requires: msvc +# 848 -- under MSVC, two programs of one build each have their own module of +# one name (mcpp#732), bound with `/reference =`. +# +# e2e 847 states the rule with the default toolchains (GCC's mapper file, and +# clang's `-fmodule-file=`). cl.exe finds a BMI through `/ifcSearchDir`, and an +# explicit `/reference` is what tells one program's units which of the two +# `.ifc` files a name means; this is the leg that measures it. +# +# Criterion: an app and its `artifacts` updater each provide a different module +# `boost`; `--toolchain msvc` builds both, and each prints its own value. +set -e + +TMP=$(mktemp -d) +trap 'rm -rf "$TMP"' EXIT +fail() { echo "FAIL: $1"; shift; for f in "$@"; do echo "--- $f ---"; cat "$f" 2>/dev/null; done; exit 1; } +cd "$TMP" + +mkdir -p app/src updater/src +printf 'export module boost;\nexport int value() { return 1; }\n' > app/src/boost.cppm +printf 'export module boost;\nexport int value() { return 2; }\n' > updater/src/boost.cppm +for d in app updater; do + printf '#include \nimport boost;\nint main() { std::printf("%%d\\n", value()); }\n' > $d/src/main.cpp +done +cat > updater/mcpp.toml <<'EOF' +[package] +name = "updater" +version = "0.1.0" + +[targets.updater] +kind = "bin" +main = "src/main.cpp" +EOF +cat > app/mcpp.toml <<'EOF' +[package] +name = "app" +version = "0.1.0" + +[dependencies] +updater = { path = "../updater", artifacts = ["updater"] } + +[targets.app] +kind = "bin" +main = "src/main.cpp" +EOF +(cd app && "$MCPP" build --toolchain msvc > "$TMP/b.log" 2>&1) || fail "the build under MSVC was refused" b.log +app=$(find app/target -path '*/bin/*' -name 'app.exe' -type f | head -1) +upd=$(find app/target -path '*/bin/*' -name 'updater.exe' -type f | head -1) +[ -n "$app" ] && [ -n "$upd" ] || fail "a program is missing" b.log +[ "$("$app" | tr -d '\r')" = 1 ] || fail "the app does not print its own module's value" b.log +[ "$("$upd" | tr -d '\r')" = 2 ] || fail "the updater does not print its own module's value" b.log +echo "PASS: 848_msvc_binds_a_module_name_per_program" diff --git a/tests/unit/test_modgraph.cpp b/tests/unit/test_modgraph.cpp index a75c761e..5e3e6516 100644 --- a/tests/unit/test_modgraph.cpp +++ b/tests/unit/test_modgraph.cpp @@ -1334,3 +1334,102 @@ TEST(Scanner, WellFormedNamesSurviveTheIdentityGuard) { EXPECT_EQ(u->provides->logicalName, provides) << decl; } } + +// ─── A module name is resolved in the importer's closure (mcpp#732) ───────── +// +// GCC and clang mangle a module's entities with its name and give it one +// initializer named after it, so one program cannot link two modules of one +// name, and two programs may each have one. A build holds several programs, so +// an import is resolved in the importing package's closure, and a closure may +// hold one provider of a name. + +TEST(ModuleResolution, OneProviderIsTakenWhereverItIs) { + const std::vector providers{"lib"}; + EXPECT_EQ(choose_provider(providers, "app", {}), std::optional{0}); +} + +TEST(ModuleResolution, TwoProvidersResolveInTheImportersClosure) { + const std::vector providers{"app", "updater"}; + const Closures closures{{"app", {"app"}}, {"updater", {"updater"}}}; + EXPECT_EQ(choose_provider(providers, "app", closures), std::optional{0}); + EXPECT_EQ(choose_provider(providers, "updater", closures), std::optional{1}); +} + +TEST(ModuleResolution, TwoProvidersInOneClosureOrNoneResolveToNothing) { + const std::vector providers{"lib1", "lib2"}; + const Closures closures{{"prog", {"prog", "lib1", "lib2"}}, {"other", {"other"}}}; + EXPECT_FALSE(choose_provider(providers, "prog", closures)); + EXPECT_FALSE(choose_provider(providers, "other", closures)); + EXPECT_FALSE(choose_provider(providers, "prog", {})); +} + +namespace { +// Two packages under `dir`, each providing module `boost` and importing it. +std::vector two_boost_packages(const std::filesystem::path& dir) { + std::vector out; + for (auto name : {"app", "updater"}) { + write(dir / name / "src" / "boost.cppm", "export module boost;\nexport int value();\n"); + write(dir / name / "src" / "use.cpp", "import boost;\nint use() { return value(); }\n"); + mcpp::manifest::Manifest m; + m.package.name = name; + m.modules.sources = {"src/*.cppm", "src/*.cpp"}; + out.push_back(PackageRoot{dir / name, m}); + } + return out; +} +std::string all_errors(const ScanResult& r) { + std::string s; + for (auto const& e : r.errors) s += e.message + "\n"; + return s; +} +} // namespace + +TEST(ModuleResolution, TwoProgramsEachResolveTheirOwnProvider) { + auto dir = make_tempdir("mcpp-732-two-programs"); + const Closures closures{{"app", {"app"}}, {"updater", {"updater"}}}; + auto r = scan_packages(two_boost_packages(dir), closures); + ASSERT_TRUE(r.errors.empty()) << all_errors(r); + ASSERT_EQ(r.graph.providersOf.at("boost").size(), 2u); + EXPECT_FALSE(r.graph.producerOf.contains("boost")); + // Every edge joins a unit to the provider of its own package. + ASSERT_EQ(r.graph.edges.size(), 2u); + for (auto [consumer, producer] : r.graph.edges) + EXPECT_EQ(r.graph.units[consumer].packageName, r.graph.units[producer].packageName); + std::filesystem::remove_all(dir); +} + +TEST(ModuleResolution, TwoProvidersInOneClosureAreRefused) { + auto dir = make_tempdir("mcpp-732-one-closure"); + const Closures closures{{"app", {"app", "updater"}}, {"updater", {"updater"}}}; + auto r = scan_packages(two_boost_packages(dir), closures); + const auto errors = all_errors(r); + EXPECT_NE(errors.find("module 'boost' is provided by package"), std::string::npos) << errors; + EXPECT_NE(errors.find("closure of package 'app'"), std::string::npos) << errors; + std::filesystem::remove_all(dir); +} + +TEST(ModuleResolution, WithoutClosuresANameIsUniqueInTheGraph) { + auto dir = make_tempdir("mcpp-732-no-closures"); + auto r = scan_packages(two_boost_packages(dir)); + const auto errors = all_errors(r); + EXPECT_NE(errors.find("module 'boost' is provided by package"), std::string::npos) << errors; + std::filesystem::remove_all(dir); +} + +TEST(ModuleResolution, OneFileReachedTwiceIsNamedAsSuchWhateverItsSpelling) { + auto dir = make_tempdir("mcpp-732-one-file"); + write(dir / "shared" / "boost.cppm", "export module boost;\n"); + std::vector packages; + for (auto name : {"lib3", "lib4"}) { + std::filesystem::create_directories(dir / name); + mcpp::manifest::Manifest m; + m.package.name = name; + m.modules.sources = {"../shared/boost.cppm"}; + packages.push_back(PackageRoot{dir / name, m}); + } + const Closures closures{{"lib3", {"lib3", "lib4"}}, {"lib4", {"lib4"}}}; + auto r = scan_packages(packages, closures); + const auto errors = all_errors(r); + EXPECT_NE(errors.find("one file is reached as two packages"), std::string::npos) << errors; + std::filesystem::remove_all(dir); +} From cf844c9f200a4f91bc364eb8001f7762348c4696 Mon Sep 17 00:00:00 2001 From: speak-agent <248744407+speak-agent@users.noreply.github.com> Date: Wed, 30 Sep 2026 13:05:40 +0800 Subject: [PATCH 06/15] build: state the duration of each step of the plan's finish phase The finish phase took 301 ms of a planned edit of one xlings source; its steps now state their durations under build/stage like the phases: make plan 112 ms and the dependency cache 176 ms. --- src/build/prepare/plan.cpp | 65 +++++++++++++++++++++++++++----------- 1 file changed, 47 insertions(+), 18 deletions(-) diff --git a/src/build/prepare/plan.cpp b/src/build/prepare/plan.cpp index dc08617c..a442b616 100644 --- a/src/build/prepare/plan.cpp +++ b/src/build/prepare/plan.cpp @@ -2273,24 +2273,53 @@ std::expected phase13_finish(PrepareState& state) { ctx.projectRoot= *state.root; ctx.outputDir = target_dir(*state.tc, state.fp, state.workRoot); - if (auto r = step13_source_packages(state, ctx); !r) return std::unexpected(r.error()); - if (auto r = step13_runner_and_xlings(state, ctx); !r) return std::unexpected(r.error()); - if (auto r = step13_prebuilt_check(state, ctx); !r) return std::unexpected(r.error()); - if (auto r = step13_link_forms(state, ctx); !r) return std::unexpected(r.error()); - if (auto r = step13_make_plan(state, ctx); !r) return std::unexpected(r.error()); - if (auto r = step13_cxx_private_runtime(state, ctx); !r) return std::unexpected(r.error()); - if (auto r = step13_cxx_process_runtime(state, ctx); !r) return std::unexpected(r.error()); - step13_graph_and_schedule(state, ctx); - if (auto r = step13_build_graph_actions(state, ctx); !r) return std::unexpected(r.error()); - if (auto r = step13_assembly_units(state, ctx); !r) return std::unexpected(r.error()); - if (auto r = step13_windows_resources(state, ctx); !r) return std::unexpected(r.error()); - if (auto r = step13_dependency_cache(state, ctx); !r) return std::unexpected(r.error()); - if (auto r = step13_lockfile(state, ctx); !r) return std::unexpected(r.error()); - if (auto r = step13_runtime_provider_overrides(state, ctx); !r) return std::unexpected(r.error()); - if (auto r = step13_abi_enforcement(state, ctx); !r) return std::unexpected(r.error()); - step13_resolution_json(state, ctx); - if (auto r = step13_empty_link_check(state, ctx); !r) return std::unexpected(r.error()); - step13_report_packages(state, ctx); + // Each step states its duration under `build/stage`, as the phases do + // (build wall-time plan, W9), when the log file or --verbose would show it. + const bool timing = mcpp::log::is_verbose() || mcpp::log::is_enabled(mcpp::log::Level::info); + auto timed = [&](std::string_view step, auto&& run) { + if (!timing) return run(); + const auto t0 = std::chrono::steady_clock::now(); + auto r = run(); + mcpp::log::verbose("build/stage", std::format("plan finish {}: {}ms", step, + std::chrono::duration_cast( + std::chrono::steady_clock::now() - t0).count())); + return r; + }; + + if (auto r = timed("source packages", [&] { return step13_source_packages(state, ctx); }); !r) + return std::unexpected(r.error()); + if (auto r = timed("runner and xlings", [&] { return step13_runner_and_xlings(state, ctx); }); !r) + return std::unexpected(r.error()); + if (auto r = timed("prebuilt check", [&] { return step13_prebuilt_check(state, ctx); }); !r) + return std::unexpected(r.error()); + if (auto r = timed("link forms", [&] { return step13_link_forms(state, ctx); }); !r) + return std::unexpected(r.error()); + if (auto r = timed("make plan", [&] { return step13_make_plan(state, ctx); }); !r) + return std::unexpected(r.error()); + if (auto r = timed("cxx private runtime", [&] { return step13_cxx_private_runtime(state, ctx); }); !r) + return std::unexpected(r.error()); + if (auto r = timed("cxx process runtime", [&] { return step13_cxx_process_runtime(state, ctx); }); !r) + return std::unexpected(r.error()); + timed("graph and schedule", [&] { step13_graph_and_schedule(state, ctx); return 0; }); + if (auto r = timed("build graph actions", [&] { return step13_build_graph_actions(state, ctx); }); !r) + return std::unexpected(r.error()); + if (auto r = timed("assembly units", [&] { return step13_assembly_units(state, ctx); }); !r) + return std::unexpected(r.error()); + if (auto r = timed("windows resources", [&] { return step13_windows_resources(state, ctx); }); !r) + return std::unexpected(r.error()); + if (auto r = timed("dependency cache", [&] { return step13_dependency_cache(state, ctx); }); !r) + return std::unexpected(r.error()); + if (auto r = timed("lockfile", [&] { return step13_lockfile(state, ctx); }); !r) + return std::unexpected(r.error()); + if (auto r = timed("runtime provider overrides", [&] { return step13_runtime_provider_overrides(state, ctx); }); !r) + return std::unexpected(r.error()); + if (auto r = timed("abi enforcement", [&] { return step13_abi_enforcement(state, ctx); }); !r) + return std::unexpected(r.error()); + timed("resolution json", [&] { step13_resolution_json(state, ctx); return 0; }); + if (auto r = timed("empty link check", [&] { return step13_empty_link_check(state, ctx); }); !r) + return std::unexpected(r.error()); + timed("report packages", [&] { step13_report_packages(state, ctx); return 0; }); + ctx.planNotes = std::move(state.planNotes); return ctx; From 39f7b54efd73bb698d9907646cb0a9b332619017 Mon Sep 17 00:00:00 2001 From: speak-agent <248744407+speak-agent@users.noreply.github.com> Date: Wed, 30 Sep 2026 13:05:40 +0800 Subject: [PATCH 07/15] build: the status row counts the work of the build A clean build of xlings counted 1195 steps on its status row, of which 503 were placements of the cache pass and 460 were dependency scans, and read 967/1195 when its first compile began. Ninja passes now have a kind. The cache pass is a placement pass: its Cached lines are written as before and its steps are not counted. The scans that wait on no action run in a pass of their own, before the main pass, shown as Scanning f/t; a scan that waits on a package's prepare or check action stays in the main pass, which keeps such an action from holding back every compile. Building f/t is the main pass: 0/232 at the first compile of the same build. The fast path runs the same passes. A failed scan pass ends the build with its own output. A build of named goals scans in its main pass. Measured on xlings: the scan pass takes 20 ms on an edit and about 0.3 s on a clean build, whose scans all ended in the first quarter second of the main pass before. --- src/build/execute.cppm | 27 ++++++++-- src/build/ninja_backend.cppm | 84 ++++++++++++++++++++++++++++-- src/build/progress.cppm | 57 +++++++++++++++----- tests/unit/test_build_progress.cpp | 37 +++++++++++++ 4 files changed, 185 insertions(+), 20 deletions(-) diff --git a/src/build/execute.cppm b/src/build/execute.cppm index 849fdb7f..be6b92ab 100644 --- a/src/build/execute.cppm +++ b/src/build/execute.cppm @@ -1290,10 +1290,29 @@ std::optional run_ninja_fast(const std::string& ninjaProgram, int status = 0; bool reported = false; const auto prefixes = read_ninja_command_prefixes(ninjaPath); + // The scan pass comes first here as on the full path (build wall-time + // plan, W2): the same passes, so the same counts. + std::optional> scanArgv; + { + std::ifstream in(ninjaPath, std::ios::binary); + std::string text{std::istreambuf_iterator(in), {}}; + if (text.find("\nbuild " + std::string(mcpp::build::kScannedGoal) + " : phony") + != std::string::npos) { + scanArgv = argv; + scanArgv->push_back(std::string(mcpp::build::kScannedGoal)); + } + } if (reporting) { mcpp::build::progress::Build report(outputDir); - auto run = mcpp::build::run_ninja_reporting(argv, childEnv, std::chrono::milliseconds{0}, - report, verbose, prefixes); + mcpp::build::NinjaRun run; + if (scanArgv) + run = mcpp::build::run_ninja_reporting(*scanArgv, childEnv, std::chrono::milliseconds{0}, + report, verbose, prefixes, + mcpp::build::progress::PassKind::Scan); + // A failed scan ends the build with its own output. + if (run.exitCode == 0 && !run.timedOut) + run = mcpp::build::run_ninja_reporting(argv, childEnv, std::chrono::milliseconds{0}, + report, verbose, prefixes); out = std::move(run.output); status = run.exitCode; reported = run.reported; @@ -1305,7 +1324,9 @@ std::optional run_ninja_fast(const std::string& ninjaProgram, // Nobody reads this ninja's progress: it reports no action start // (build progress design 2026-09-29, §6.4). childEnv.emplace_back(std::string(mcpp::build::progress::kStartsEnv), ""); - auto r = mcpp::platform::process::capture_exec(argv, childEnv); + mcpp::platform::process::RunResult r; + if (scanArgv) r = mcpp::platform::process::capture_exec(*scanArgv, childEnv); + if (r.exit_code == 0) r = mcpp::platform::process::capture_exec(argv, childEnv); out = std::move(r.output); status = r.exit_code; } diff --git a/src/build/ninja_backend.cppm b/src/build/ninja_backend.cppm index 71d1e02d..a2a2a5b4 100644 --- a/src/build/ninja_backend.cppm +++ b/src/build/ninja_backend.cppm @@ -95,7 +95,14 @@ NinjaRun run_ninja_reporting(const std::vector& argv, std::chrono::milliseconds deadline, mcpp::build::progress::Build& progress, bool verbose, - std::span commandPrefixes); + std::span commandPrefixes, + mcpp::build::progress::PassKind kind = + mcpp::build::progress::PassKind::Work); + +// The goal of the scan pass (build wall-time plan, W2): the dyndep file of +// every unit whose scan waits on no action. Present in build.ninja when the +// graph scans such a unit. +inline constexpr std::string_view kScannedGoal = "_mcpp_scanned"; // The step record of a plan (design §6.3): how the plan names its packages, // and which package each statement of `attribution` is for. @@ -1175,10 +1182,11 @@ NinjaRun run_ninja_reporting(const std::vector& argv, std::chrono::milliseconds deadline, mcpp::build::progress::Build& progress, bool verbose, - std::span commandPrefixes) { + std::span commandPrefixes, + mcpp::build::progress::PassKind kind) { NinjaRun run; for (auto& kv : progress.environment()) env.push_back(std::move(kv)); - progress.pass_begin(); + progress.pass_begin(kind); // The lines after `FAILED:` up to the next status line are the failed // step's command and output; the lines after a status line alone are a // successful step's output, which only --verbose shows (as before). @@ -2692,6 +2700,22 @@ std::string emit_ninja_string(const BuildPlan& plan, std::string* placements, } append("\n"); + // THE SCAN PASS'S GOAL (build wall-time plan, W2): the dyndep file of + // every unit whose scan waits on no action. A scan waits on its + // package's actions that precede compilation (`order_only_for`), and + // a pass over those scans would hold every compile behind the longest + // such action; they stay in the main pass. + { + std::string goal; + for (auto& ddi : ddi_paths) { + auto it = actionOutputsByPackage.find(ddiOwner[ddi]); + if (it != actionOutputsByPackage.end() && !it->second.empty()) continue; + goal += " " + ddi + ".dd"; + } + if (!goal.empty()) + append(std::format("build {} : phony{}\n\n", kScannedGoal, goal)); + } + // ── Phase 3: compile edges with per-file dyndep. ──────────────── // Each compile edge references its OWN .dd file instead of a global one. // P2: module compile edges get a $bmi_out variable for BMI preservation. @@ -4286,7 +4310,8 @@ std::expected NinjaBackend::build(const BuildPlan& plan std::vector pre{ninjaProgram, "-C", plan.outputDir.string(), std::string(kStagedCacheGoal)}; (void)run_ninja_reporting(pre, nenv, preDeadline, *opts.progress, opts.verbose, - command_prefixes(flags, plan)); + command_prefixes(flags, plan), + mcpp::build::progress::PassKind::Placement); } else { std::vector pre{ninjaProgram, "--quiet", "-C", plan.outputDir.string(), std::string(kStagedCacheGoal)}; @@ -4299,13 +4324,62 @@ std::expected NinjaBackend::build(const BuildPlan& plan stage("ninja-staged-cache"); } + // THE SCANS THAT WAIT ON NO ACTION RUN IN A PASS OF THEIR OWN, after the + // cache pass and before the main pass (build wall-time plan, W2). The + // main pass's count then states the work of the build, its compiles, + // links, archives and actions, instead of 460 scans of which all end in + // the first quarter second; and every dyndep file of those units is + // current when the main pass loads them, as the cache pass arranges for + // its placements (ninja-build/ninja#2662). A build of named goals (a test, + // a target) scans in its main pass: a pass over every scan would scan + // units its goals never compile. bool buildTimedOut = false; bool reported = false; std::string out; int ninjaExit = 0; + bool scanFailed = false; + if (goalArg.empty() + && manifest.find("\nbuild " + std::string(kScannedGoal) + " : phony") != std::string::npos) { + const auto scanDeadline = + std::chrono::milliseconds(static_cast(opts.buildTimeoutSecs) * 1000); + std::vector scan{ninjaProgram}; + if (!opts.verbose && !opts.progress) scan.push_back("--quiet"); + scan.insert(scan.end(), {std::string("-C"), plan.outputDir.string()}); + if (opts.verbose) scan.push_back("-v"); + scan.push_back(std::string(kScannedGoal)); + if (opts.parallelJobs) scan.push_back(std::format("-j{}", opts.parallelJobs)); + if (opts.progress) { + auto run = run_ninja_reporting(scan, nenv, scanDeadline, *opts.progress, opts.verbose, + command_prefixes(flags, plan), + mcpp::build::progress::PassKind::Scan); + if (run.exitCode != 0 || run.timedOut) { + out = std::move(run.output); + ninjaExit = run.exitCode; + buildTimedOut = run.timedOut; + reported = run.reported; + scanFailed = true; + } + } else { + auto scanEnv = nenv; + scanEnv.emplace_back(std::string(mcpp::build::progress::kStartsEnv), ""); + auto cap = mcpp::platform::process::capture_exec_deadline( + scan, scanEnv, scanDeadline, &buildTimedOut); + if (cap.exit_code != 0 || buildTimedOut) { + out = std::move(cap.output); + ninjaExit = cap.exit_code; + scanFailed = true; + } + } + stage("ninja-scan"); + } + + // A failed scan ends the build with its own output: the main pass would + // run the failed step again and state its diagnostics a second time. const auto deadline = std::chrono::milliseconds(static_cast(opts.buildTimeoutSecs) * 1000); - if (opts.progress) { + if (scanFailed) { + // `out`, `ninjaExit`, `buildTimedOut` and `reported` are the scan's. + } else if (opts.progress) { auto run = run_ninja_reporting(nargv, nenv, deadline, *opts.progress, opts.verbose, command_prefixes(flags, plan)); out = std::move(run.output); diff --git a/src/build/progress.cppm b/src/build/progress.cppm index a734a6cb..f1fa3ae9 100644 --- a/src/build/progress.cppm +++ b/src/build/progress.cppm @@ -190,6 +190,8 @@ enum class ProgramOutcome { Ran, Cached, Failed }; void open(bool verbose); // Closes it: the region is erased. Idempotent. void close(); +// The status row as the region would draw it now. +std::string status_row(); // The number of configurations the command builds: with more than one, a // package line names its configuration. void configurations(std::size_t n); @@ -215,6 +217,16 @@ void defer_finished(); void finish_deferred(); // One build directory's ninja runs within this command. +// WHAT A NINJA PASS IS FOR, AND WHETHER ITS STEPS ARE COUNTED (build wall-time +// plan, W2). `Building f/t` states the work of the build: its compiles, links, +// archives and actions. The cache pass places files the global cache serves, +// which its `Cached` lines report; counted, its 503 placements and the 460 +// dependency scans of a clean build of xlings put the count at 81% when the +// first compile began. A placement pass is therefore read but not counted, and +// the scans that wait on no action run in a pass of their own, shown as +// `Scanning f/t`. +enum class PassKind { Placement, Scan, Work }; + class Build { public: // Opaque: its definition is this module's own. @@ -234,8 +246,8 @@ public: // The environment ninja runs with: NINJA_STATUS and the start file. std::vector> environment() const; - // One ninja invocation. - void pass_begin(); + // One ninja invocation, of the kind `kind` (see PassKind). + void pass_begin(PassKind kind = PassKind::Work); void status(const StatusLine& line); // A `FAILED: ` line. For the command's first failure, returns // the failed step's package as a line names it (empty when the step is @@ -634,7 +646,7 @@ void record_action_start(std::string_view stamp) { // ─── The model ─────────────────────────────────────────────────────────── // Not exported, and not TU-local either: `Build::Impl` holds them. -enum class Phase { Planning, Programs, Building, Stopping, Checking }; +enum class Phase { Planning, Programs, Scanning, Building, Stopping, Checking }; struct Program { std::string name; @@ -672,6 +684,7 @@ struct Build::Impl { std::size_t finished = 0, total = 0; // this pass std::optional unstarted; // this pass, from `%u` bool inPass = false; + PassKind kind = PassKind::Work; // this pass; only Work passes count // The log of this pass. std::filesystem::path logPath; std::string logTail; // the file's last bytes when the pass began @@ -991,12 +1004,18 @@ std::string phase_status(Report& r, std::string_view cells) { std::string current; long long oldest = std::numeric_limits::max(); std::size_t done = 0, total = 0, remaining = 0; + std::size_t scanDone = 0, scanTotal = 0; bool tail = true; // every build in a pass has started its last step bool anyPass = false; for (auto const& b : live_builds(r)) { - done += b->doneBefore + b->finished; - total += b->totalBefore + b->total; - if (b->inPass) { + const bool work = b->kind == PassKind::Work; + done += b->doneBefore + (work ? b->finished : 0); + total += b->totalBefore + (work ? b->total : 0); + if (b->inPass && b->kind == PassKind::Scan) { + scanDone += b->finished; + scanTotal += b->total; + } + if (b->inPass && work) { anyPass = true; if (!b->unstarted || *b->unstarted > 0) tail = false; else remaining += b->total > b->finished ? b->total - b->finished : 0; @@ -1026,6 +1045,10 @@ std::string phase_status(Report& r, std::string_view cells) { counts = std::format("{}/{}", finishedPrograms, r.programs.size()); break; } + case Phase::Scanning: + phase = "Scanning"; + if (scanTotal > 0) counts = std::format("{}/{}", scanDone, scanTotal); + break; case Phase::Building: case Phase::Stopping: phase = r.phase == Phase::Building ? "Building" : "Stopping"; @@ -1066,8 +1089,9 @@ mcpp::ui::Frame frame() { } else if (r.animation) { std::size_t done = 0, total = 0; for (auto const& b : live_builds(r)) { - done += b->doneBefore + b->finished; - total += b->totalBefore + b->total; + const bool work = b->kind == PassKind::Work; + done += b->doneBefore + (work ? b->finished : 0); + total += b->totalBefore + (work ? b->total : 0); } const auto now = now_ms(); screen::Input in; @@ -1281,6 +1305,10 @@ void checking() { mcpp::ui::touch_region(); } +std::string status_row() { + return frame().status; +} + void defer_finished() { auto& r = report(); std::lock_guard lock(r.m); @@ -1394,18 +1422,23 @@ std::vector> Build::environment() const { {std::string(kStartsEnv), (impl_->dir / kStartsFile).string()}}; } -void Build::pass_begin() { +void Build::pass_begin(PassKind kind) { auto& r = report(); { std::lock_guard lock(r.m); auto& b = *impl_; - b.doneBefore += b.finished; - b.totalBefore += b.total; + if (b.kind == PassKind::Work) { + b.doneBefore += b.finished; + b.totalBefore += b.total; + } + b.kind = kind; b.finished = b.total = 0; b.unstarted.reset(); b.passStart = now_ms(); if (!r.buildStart) r.buildStart = b.passStart; - r.phase = Phase::Building; + // A placement pass keeps the phase it found (`Planning`). + if (kind == PassKind::Work) r.phase = Phase::Building; + else if (kind == PassKind::Scan) r.phase = Phase::Scanning; b.inPass = true; b.ends.clear(); b.logPath = b.dir / ".ninja_log"; diff --git a/tests/unit/test_build_progress.cpp b/tests/unit/test_build_progress.cpp index 1bd6f11f..441dc080 100644 --- a/tests/unit/test_build_progress.cpp +++ b/tests/unit/test_build_progress.cpp @@ -390,6 +390,43 @@ TEST(ProgressModel, APackageTheCacheServesIsNamedCachedWithItsUnits) { EXPECT_EQ(count(out, "compat.ftxui"), 1u) << out; } +// ─── What the count counts (build wall-time plan, W2) ──────────────────── +// +// `Building f/t` states the work of the build. A clean build of xlings counted +// 1195 steps, 503 of them placements of the cache pass and 460 dependency +// scans, and read 967/1195 when its first compile began. A placement pass is +// read but not counted, a scan pass shows as `Scanning f/t`, and the main pass +// alone is `Building f/t`. +TEST(ProgressModel, OnlyTheMainPassIsCountedAsBuilding) { + mcpp::ui::disable_color(); + Tmp tmp; + Record rec; + rec.packages = {{"app", true, "app", 0, 1, "v0.1.0 (.)", "project"}}; + rec.steps = 1; + Build b(tmp.path); + b.set_record(rec); + testing::internal::CaptureStdout(); + b.pass_begin(mcpp::build::progress::PassKind::Placement); + b.status({503, 503, 1, 0, {}}); + const auto placing = mcpp::build::progress::status_row(); + b.pass_end(); + b.pass_begin(mcpp::build::progress::PassKind::Scan); + b.status({200, 460, 2, 100, {}}); + const auto scanning = mcpp::build::progress::status_row(); + b.pass_end(); + b.pass_begin(mcpp::build::progress::PassKind::Work); + b.status({1, 232, 3, 200, {}}); + const auto building = mcpp::build::progress::status_row(); + b.pass_end(); + b.finish(true); + testing::internal::GetCapturedStdout(); + EXPECT_EQ(placing.find("503"), std::string::npos) << placing; + EXPECT_NE(scanning.find("Scanning"), std::string::npos) << scanning; + EXPECT_NE(scanning.find("200/460"), std::string::npos) << scanning; + EXPECT_NE(building.find("Building"), std::string::npos) << building; + EXPECT_NE(building.find(" 1/232"), std::string::npos) << building; +} + TEST(ProgressModel, AFailureNamesItsPackageOnce) { mcpp::ui::disable_color(); Tmp tmp; From 1db3dc1028de7978c7b84953c8f3b0d901f2bf8e Mon Sep 17 00:00:00 2001 From: speak-agent <248744407+speak-agent@users.noreply.github.com> Date: Wed, 30 Sep 2026 13:09:58 +0800 Subject: [PATCH 08/15] build: a scan pass names no package at its end Every package scans in the scan pass, and naming each package that finished a step when that pass ended put a package before the one it imports (e2e 842). A package that only scanned is named when the main pass ends, as before. --- src/build/progress.cppm | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/build/progress.cppm b/src/build/progress.cppm index f1fa3ae9..e52da6b3 100644 --- a/src/build/progress.cppm +++ b/src/build/progress.cppm @@ -1523,7 +1523,10 @@ void Build::pass_end() { std::lock_guard lock(r.m); auto& b = *impl_; read_log(r, b, out); - if (b.record) + // A package that only scanned is named when the main pass ends. A + // scan pass names nobody at its end: every package scans there, and + // naming them all at once put a package before the one it imports. + if (b.record && b.kind != PassKind::Scan) for (std::size_t i = 0; i < b.packages.size(); ++i) if (b.packages[i].finished > 0) announce(r, b, i, out); b.inPass = false; From a0ef3c2725c6a5d2b2a6bb45e678f2dc3e91544a Mon Sep 17 00:00:00 2001 From: speak-agent <248744407+speak-agent@users.noreply.github.com> Date: Wed, 30 Sep 2026 13:12:14 +0800 Subject: [PATCH 09/15] 2026.9.30.2: version, CHANGELOG, documentation and the design record The version is 2026.9.30.2 in mcpp.toml and MCPP_VERSION. The CHANGELOG entry is the release's notes. The commands guide states what the status row counts, the dependencies guide states that a module name is unique within one program, and the workspace guide no longer refuses two members that share a module name and no program; both languages. The design record states the measurements, the plan, the tasks across repositories, and the implementation record, including W4's deferral on its own gate. --- ...-wall-time-progress-count-and-hang-plan.md | 1015 +++++++++++++++++ .agents/docs/README.md | 4 +- CHANGELOG.md | 60 + docs/09-commands-by-scenario.md | 13 +- docs/zh/09-commands-by-scenario.md | 4 +- mcpp.toml | 2 +- modules/versioning/src/version.cppm | 2 +- 7 files changed, 1092 insertions(+), 8 deletions(-) create mode 100644 .agents/docs/2026-09-30-build-wall-time-progress-count-and-hang-plan.md diff --git a/.agents/docs/2026-09-30-build-wall-time-progress-count-and-hang-plan.md b/.agents/docs/2026-09-30-build-wall-time-progress-count-and-hang-plan.md new file mode 100644 index 00000000..f7a12ea6 --- /dev/null +++ b/.agents/docs/2026-09-30-build-wall-time-progress-count-and-hang-plan.md @@ -0,0 +1,1015 @@ +--- +subject: design +status: landed +--- + +# The build's wall time, its progress count, a hang after the build, and #732 and #744: measurements and a remediation plan + +- Status: implemented, revision 4, released as 2026.9.30.2. Section 9 splits + the work into tasks across repositories; section 10 records what was built, + what was measured, and where the implementation departs from sections 4 and + 5 (W4 is deferred on its own gate). Revisions 1 and 2 were reviewed on + 2026-09-30. + - Round 1 settled D1 to D5 (section 6), narrowed W6 and W7, and asked + whether #732 is a usage problem. Revision 2 applied those answers and a + self-review, which changed W2, W4, W8 and W10. + - Round 2 asked for the root cause of #732 and for a fundamental solution + that is elegant, stable and compatible. Revision 3 answers with + measurements on GCC 16 and clang 22 (F7), and replaces revision 2's + rule with W10. +- Date: 2026-09-30 +- Origin: five points reported on 2026.9.30.1 while building xlings. + 1. Once, `mcpp build` did not exit after the animated status row stopped. + 2. A build with the animated row appears a few seconds slower than + 2026.9.28.2. The report asks whether the old timer was inaccurate or + whether the time before the build can be reduced. + 3. The status row's count includes the packages served from the cache. + The `Cached` lines are correct; the count should state only the work the + build performs. + 4. mcpp#744 and mcpp#732. + 5. The build is slow in general. +- Task: separate what is not mcpp's, and plan what is mcpp's or what mcpp's + own design requires, as one pull request. + +## 0. Summary + +| # | Report | Finding | Whose | Item | +|---|---|---|---|---| +| 1 | Hang after the animation | `Stack::update` does not terminate; the ticker holds the line lock, and `close_region()` joins it forever (F1) | mcpp, new in 2026.9.30.1 | W1 | +| 2 | Slower than 2026.9.28.2 | Wall time is equal within the run-to-run spread; 2026.9.28.2's `Finished` omitted about 3.9 s of work before ninja (F2) | no regression | none; the 4 s is addressed by W3, W4, W8 | +| 3 | Count includes cached packages | 1195 counted steps: 503 cache placements, 460 dependency scans, 232 compiles and links (F3) | mcpp design (revision 3, §7.2) | W2 | +| 4a | #744 | Confirmed; the probe also costs 0.35 s on every planned command (F6) | mcpp | W3 | +| 4b | #732 | The language and the ABI make a module name unique per program. mcpp makes it unique per build configuration, which holds several programs. GCC and clang can bind imports per importer, measured (F7) | within one program: a usage error, still refused. Across programs: mcpp's identity model | W10 | +| 5 | Slow in general | Clean build: 30 of 35 s is the project's import chain (F4). Edit of one file: 3.05 s of planning precedes the one compile, and the planner walks each dependency's tree about 380 times (F5) | chain: project and compiler; planning and probes: mcpp | W4, W8, W9 | + +**One pull request carries W1, W2, W3, W8, W9 and W10** (section 5). W4 was +deferred on its own gate after W8 was measured (section 10.3). + +| Item | Status | +|---|---| +| W5, the split schedule | stays opt-in | +| W6, a faster linker | recorded and not done (D3) | +| W7 | dropped (D4) | +| W11 | its Windows reading is taken from this pull request's run; the change waits for that reading | + +## 1. Method + +- **Machine.** 32 cores, Linux 6.8.0. `perf` is unavailable to the user + (`perf_event_paranoid = 4`) and `ptrace_scope = 1`, so the in-process time + is resolved to phases by log timestamps and system-call traces, not to + functions. +- **Project.** xlings at c4b6cef: a workspace of eight members, thirteen + index dependencies served from the global cache, 230 translation units of + its own. The tree was copied twice into a scratch directory (without + `tests/` and `target/`), one copy per mcpp version, so that each version's + lock file and build directory stay its own. +- **Versions.** The released 2026.9.28.2 and 2026.9.30.1 binaries from the + xlings store, run by path, with the same home (`~/.mcpp`), gcc 16.1.0, + ninja 1.12.1 and binutils 2.42. +- **Runs.** Clean builds (`mcpp clean && mcpp build`), one warm-up per + version, then three rounds alternating the versions under a pty (the + animated row on) and three through a pipe. Wall time is measured outside + mcpp. +- **Instruments.** + - `.ninja_log`, split into passes and deduplicated per step; + - the dyndep files (`*.dd`), which give the module edges, for the critical + path; + - `strace -f --seccomp-bpf -e trace=execve` for process launches, and + `-y -e trace=openat` for the paths the planner opens; + - `MCPP_LOG_LEVEL=info` timestamps for the planning phases; + - a harness that compiles the `Stack` class verbatim and drives it; + - the final link command replayed with `ld.bfd` and with `ld.lld`; + - `MCPP_BMI_SCHEDULE=on` against the default. + +## 2. Findings + +### F1. The hang: `Stack::update` does not terminate + +- **The loop.** `src/ui/dots_screen/stack.cppm:22`: + `while (cells_.size() + 4 * flying_.size() + 16 <= target) lock(spawn());` + terminates only if every `lock(spawn())` adds cells. + - `landing()` (`stack.cppm:65-72`) starts at `x = kWidth` and tests only + `fits(x - 1)`. + - When column 47 is occupied in the rows a piece spans, the piece lands at + `x = 48`, outside the 48-column screen, on cells that an earlier such + piece already holds. + - `cells_` is a `std::map`, so its size stops growing. `target` does not + fall, and the loop never ends. + - The screen holds 192 dots and the target at the end of a build is 168 + (`(kWidth - 6) * kHeight`). A stack whose holes leave it at 152 cells or + fewer when it reaches the right edge therefore keeps the condition true + for ever. +- **Measured.** The class compiled verbatim in a harness, with 2000 seeded + runs of a 240-step build that finishes in bursts at ten frames per second: + 66 runs (3.3%) never return from `update()`. They stall at 146 to 152 cells + against a target of 162 to 168. +- **Why the process hangs.** + - The ticker thread takes `line_mutex()` (`src/ui.cppm:599`) and draws: + `redraw_locked` → `current_rows_locked` → the model's `frame()` → + `r.animation->update(in)` (`src/build/progress.cppm:1080`). + - After ninja, `close_region()` (`src/ui.cppm:1029`) requests a stop and + joins the ticker. The ticker never returns to its stop check, so the join + blocks and `Finished` is never written. + - No frame completes after the loop starts, so the row stops moving: this + is what the report describes as the animation having ended. +- **How often.** `stack` is one of four animations chosen at random when + `MCPP_PROGRESS` is unset (`dots_screen.cppm:42`, `progress.cppm:1120-1134`). + The hang needs that one-in-four choice and a stack shape that reaches the + right edge before the end: about 1% of interactive builds, and more of the + long ones. +- **Ruled out, with evidence.** + - The stream reader: it reads non-blockingly and drains once after + `waitpid` (`modules/platform/src/unix/bounded_process.cppm:311-345`), so a + grandchild holding the pipe cannot block mcpp. + - Lock order: no holder of the model's lock calls into `mcpp.ui`; lines + are written after the lock is released. + - The keyboard reader of `--play-game`: it is non-blocking and runs inside + `poll` on the ticker thread. + - The post-build steps: only the freestanding size report and hooks run + after the region closes. + - The other animations and the three games: every loop in them is bounded + (checked: `snake`, `ions`, `chomp`, `stack_game`, `snake_game`, + `runner_game`). + +### F2. "Slower than 2026.9.28.2" is the timer + +| Version | Wall, mean (runs) | `Finished`, mean | ninja main pass, mean | Outside ninja | +|---|---|---|---|---| +| 2026.9.28.2 | 35.88 s (6: 34.11 to 37.82) | 31.98 s | 31.89 s | 3.99 s | +| 2026.9.30.1 | 35.39 s (5: 35.02 to 35.91) | 35.37 s | 31.20 s | 4.19 s | + +- **2026.9.28.2 under-reports by 3.90 s.** Its clock starts near ninja and + excludes resolution, the build programs and planning. Since 2026.9.29.5, + `Finished` counts from the start of the command and equals the wall time + within 0.02 s. +- **The wall times are equal within the spread.** The pty and pipe runs do + not differ measurably, so the animated row has no measurable cost. +- **One outlier.** One 2026.9.30.1 run took 43.06 s. Its ninja pass alone + took 38.65 s against about 31 s in the others, which is compile-time + variance on a shared machine. It is excluded from the mean. +- **No change to the timer is proposed.** The four seconds outside ninja are + real and are addressed by W3, W4 and W8. + +### F3. The count: 1195 steps, of which 232 compile or link + +The status row's `f/t` in a clean build of xlings (2026.9.30.1): + +| Steps | Count | Time | +|---|---|---| +| Cache placements (`stage_file`, the cache pass) | 503 | 80 ms of ninja time | +| Dependency scans (`.ddi`) | 230 | all 460 scans and collations end 0.21 to 0.24 s into the main pass | +| Collations (`.dd`) | 230 | (included above) | +| Compiles | 231 | about 30 s of the main pass | +| Link | 1 | 1.5 to 2.1 s | + +- **The recorded row.** It read `Building … 967/1195 · 0:04` when the first + compiles began: 81% of the count was reached at the start of the work. It + then took 25 s to go from 81% to 100%. The animations take their fraction + from the same numbers (`progress.cppm:1066-1076`). +- **Cause, first half: the design.** Revision 3 states "The cache pass + counts in `Building f/t` like the main pass" + (`2026-09-30-build-output-refinement-design.md:1152`), and + `Build::pass_begin` carries every pass's counts forward + (`progress.cppm:1397-1404`). +- **Cause, second half: ninja's `%t`.** It counts the scans and collations, + which are as numerous as the compiles and finish in the first quarter + second. Excluding the cache pass alone would still leave 460 of the + remaining 692 counted steps as scans. + +### F4. A clean build: 30 of 35 s is the project's import chain + +- **The critical path.** The module edges from the dyndep files and the + measured step durations (2026.9.28.2, final run) give a chain of 18 + compiles summing to 30.13 s, in a main pass of 32.48 s. + - It starts at `modules/json` (4.64 s), runs through `core/config`, `xim`, + `xself` and `subos` to `cli.m`, and ends in `cli.o` (6.01 s). + - The link (1.5 to 2.1 s) follows. + - Busy cores are between 14 and 32 for the first 12 s, 10 or fewer after + 17 s, and 1 for the last 5 s. +- **What bounds the build.** ninja's scheduling and `-j` are not the bound. + The bound is the depth of the import chain and GCC's time per module + interface, which belong to the project and the compiler. +- **Levers inside mcpp's design, measured.** + - *The split schedule.* `MCPP_BMI_SCHEDULE=on` releases importers when GCC + publishes the BMI, at about 22% of the compile + (`src/build/schedule/policy.cppm:215-218`). On xlings it measured + 34.04 s against 35.55 s (2 runs each; main pass 29.94 s against + 31.42 s), a gain of 1.5 s, or 4%. + - *The linker.* The final link replayed from `build.ninja` takes 1.49 to + 1.59 s with binutils `ld.bfd` and 0.19 to 0.20 s with `ld.lld` from the + llvm payload, three runs each. Both binaries run. +- **Outside ninja (about 4 s, both versions).** + + | Cost | Time | Note | + |---|---|---| + | `xlings --version` | 0.35 s | F6 | + | Recompiling a dependency's build program after `mcpp clean` | 0.79 s | its artifacts live in the consuming project's `target/.build-mcpp`, `build_program.cppm:586-595` | + | Planning | about 2.7 s | F5 | + +### F5. An edit: 3.05 s of planning precede the one compile + +A `touch` of `src/core/xself/doctor.cpp` and `mcpp build` (2026.9.30.1, +10.47 s; 2026.9.28.2 took 11.57 s), from the exec trace and the phase log: + +| From (s) | To (s) | Span | What | +|---|---|---|---| +| 0.00 | 0.02 | 0.02 s | start | +| 0.02 | 0.37 | 0.35 s | `xlings --version` (F6) | +| 0.37 | 0.38 | 0.01 s | three compiler probes | +| 0.38 | 1.04 | 0.66 s | graph load up to the build program's cache hit | +| 1.04 | 1.69 | 0.65 s | features, host tools, target side | +| 1.69 | 2.60 | 0.91 s | module scan of every package that compiles here | +| 2.60 | 3.05 | 0.45 s | plan, emit, records | +| 3.05 | 8.60 | 5.55 s | compile `doctor.cpp` | +| 8.60 | about 10.4 | about 1.8 s | link (`ld.bfd`) | + +- **Why every edit plans.** The fast path declines when any source is newer + than `build.ninja` (`src/build/execute.cppm:1654-1656`, the rule of + mcpp#225). A body edit that changes no module declaration, no import and + no file set cannot change `build.ninja`; planning it again is waste. + Revision 3 recorded this in §12 and deferred it to a design of its own. +- **A second reason, found in the self-review.** Even with fresh sources, + the fast path declines whenever ninja relinks an artifact ("ninja relinked + an artifact, whose closure the full path validates"). The post-link runtime + validation reads four facts from the plan: + - the runtime binding; + - the target triple; + - the library search directories; + - whether host libraries are allowed + (`src/build/runtime_validation.cppm:640-776`). + + The fast-path record carries only the runtime binding. A body edit always + relinks, so a fast path for edits must carry the rest. +- **Where the planning goes: the planner walks the dependencies' trees + hundreds of times.** + - Between the start and the first ninja, mcpp opens 30,967 paths. 30,372 + of them are directories inside installed index packages, and none is a + file read: + + | Tree | Directory opens | Directories | Walks | + |---|---|---|---| + | compat.libarchive 3.8.7 | 13,406 | 35 | about 383 | + | compat.xz 5.8.3 | 11,551 | 50 | about 231 | + | compat.zlib, zstd, lua, mbedtls, ftxui and others | 5,415 | | | + + - The cause is in `expand_glob_one` (`src/modgraph/scanner.cppm:563`). It + walks recursively from a pattern's literal prefix and calls + `fs::canonical` on every directory. + - The libarchive descriptor lists 127 sources of the form + `*/libarchive/archive_acl.c`, whose literal prefix is empty, so each + pattern walks the whole tree. The expansion runs about three times per + plan. This is inferred from 35 × 127 × 3 = 13,335 against 13,406 + measured. + - The same walk code timed in isolation costs 0.39 s for libarchive's 381 + walks and 0.13 s for xz's 222, with a warm page cache. That is before the + per-file glob match, which the harness reduces to a suffix comparison. + - The walks run across the whole planning window, at 2,000 to 4,900 + directory opens per quarter second. +- **Scanning.** The module scan reads the sources of every package that + compiles here, including the thirteen packages whose units the cache then + serves. The cache decision is made after the scan + (`src/build/prepare/plan.cpp:2080`). + +### F6. #744, and the probe costs 0.35 s per planned command + +- **Confirmed as reported.** + - `candidate_source_version` takes the first source that exists, not the + newest (`src/fallback/xlings_binary.cppm:262`). + - `load_or_init` is not memoised, and it runs the check on every load + (`src/config.cppm:816`). +- **The probe's cost.** `vendored_xlings_version` spawns `xlings --version` + (`xlings_binary.cppm:196`) on every configuration load, and that spawn + takes 0.35 to 0.49 s on this machine. + - In `mcpp build` it runs once, and it is the first 0.35 s of every + command that does not take the fast path. + - The fast path does not load the configuration: a no-op build takes + 0.05 s. +- **Whose.** That `xlings --version` needs 0.35 s is xlings's matter + (section 3). That mcpp asks the question on every command is mcpp's. + +### F7. #732: a module name identifies a module within a program, and mcpp made it an identity of the whole build configuration + +**What the language and the ABI require (measured).** + +- The ABI makes the program the boundary. GCC and clang mangle a + module-attached entity with its module's name, and a module has one + initializer, named after it. The objects of both test modules define + `_ZW6common5valuev` (`value@common()`). +- Linking two different `common` modules into one program therefore fails: + `multiple definition of value@common()` and of + `initializer for module common` (GCC 16.1.0, binutils 2.42). +- Two programs do not share symbols, so each may have its own `common`. A + module name is unique within one program, by the standard and by the ABI, + and not beyond it. + +**What mcpp does: the name is an identity of the whole configuration, at +four levels.** + +| Level | What mcpp does | Where | +|---|---|---| +| Resolution | Four places each keep a map from module name to one provider: the scanner's refuses a second provider, the plan's keeps the last one, and the backend's keeps the first, so the two would disagree | `scanner.cppm:1427-1436` (read by `pack/interface.cppm:123-158`); `prepare/scan.cpp:882-924`; `plan.cppm:2110-2113`; `ninja_backend.cppm:2237-2250` | +| Location | A BMI's path is `/`. The staging of cached BMIs uses the same form | `bmi_path`, `ninja_backend.cppm:2324`; `configure.cppm:72` | +| Compiler lookup | Every compiler finds a BMI by name in one directory: GCC by its default mapper, which also chooses where the BMI is written (`gcm.cache/.gcm`); clang by `-fprebuilt-module-path`; MSVC by `/ifcSearchDir` | `modules/toolchain-model/src/model.cppm:641-693`; `flags.cppm:1176-1222` | +| Collation | `mcpp dyndep --bmi-dir --bmi-ext` derives an import's BMI path from its name | `ninja_backend.cppm:1487-1491` | + +A build configuration holds several programs: a package and the programs it +ships through `artifacts`, a workspace's members, and test binaries. +mcpp's boundary is therefore wider than the one the language and the ABI +draw. + +**What the compilers can do instead (measured on the same two modules and a +third module `util`).** + +- **GCC 16.1.0.** + - With `-fmodule-mapper=` (lines of the form `name path`), the + provider writes its BMI to the mapped path and the importer reads it + from there. + - Two `common` modules lived in one build directory + (`gcm.cache/a/common.gcm`, `gcm.cache/b/common.gcm`), and the two + programs returned 41 and 42, as intended. + - A mapper file has no fallback. A module it does not list fails with + `unknown compiled module interface: no such module`, so a map must list + every module the unit can import. +- **clang 22.1.8.** + - The provider takes `-fmodule-output=`, as mcpp already passes it. + - `-fmodule-file=common=` overrides `-fprebuilt-module-path` for that + one name, while other names are still found in the directory. + - The programs returned 41 and 42. +- **MSVC.** `/ifcOutput` and `/reference name=path` are the documented + equivalents. They were not measured on this machine. + +**The cases of #732, read against this.** + +- **Two providers inside one program.** Invalid, by the ABI, and mcpp + rightly refuses it. This is a usage error. +- **Case B, the minimal example: two different `common` modules in two + independent programs.** Valid C++, and not a usage problem. mcpp refuses + it only because of its configuration-wide identity. The same holds for two + workspace members that do not share a program; the workspace design + records that refusal (`2026-09-29-workspace-build-graph-design.md:532`). +- **Case A, the reported project.** `gpp.core` (in the GUI program) and + `gpp.updater` (in the updater program) both compile + `3rdParty/3rdModule/boost.ixx`. Each program still contains one `boost`, + so the build is valid. The layout compiles one file twice where a package + that both depend on would compile it once, but it violates no rule. +- **A second defect in case A.** The scanner's "one file is reached as two + packages" hint compares paths lexically (`first.path == u.path`, + `scanner.cppm:1440`). `GalTranslPP/../3rdParty/…` and + `Updater/../3rdParty/…` differ lexically, so the hint was not printed. + +**Root cause.** The language keys a module by (program, name). mcpp keys it +by (configuration, name) and derives its BMI path, its lookup and its +resolution from the name alone. What identifies a BMI is its provider, the +package whose unit declares the module. What decides which provider an +import means is the importer's dependency closure. + +## 3. Excluded: not mcpp's + +- **E1. The depth of xlings's import chain and GCC's time per module + interface.** These account for 30.1 of 35.4 s. Observations for xlings + (not mcpp work): + - `modules/json` wraps a large header, is imported by almost everything, + and costs 4.6 s at the root of the chain; + - `cli.o` (6.0 s), `core/config.o` (8.6 s) and `xself/doctor.o` (7.8 s) + are the longest single compiles. +- **E2. `xlings --version` takes 0.35 s.** A version query should not load + more than the binary. This belongs to xlings, as its own issue; W3 removes + mcpp's dependence on it. +- **E3. The 43 s run.** It is variance in the compile steps themselves. +- **E4. 2026.9.28.2's `Finished`.** It is already superseded (F2). + +## 4. The plan + +Each item states the change, its criterion, and the expected gain on the +measured xlings builds. Items W1 to W4, W8, W9 and W10 form the pull request +of section 5. + +**W1. Every animation and game terminates in every frame (F1).** + +- **The change.** + - `landing()` tests the landing column itself. + - `spawn()` returns `std::nullopt` when no rotation and offset lands + inside the screen. + - Both fill loops in `update()` stop when `spawn()` returns nothing or a + locked piece adds no cell. Every iteration then either grows `cells_` or + ends the loop, so termination follows from the structure of the loop, + not from the shape of the stack. + - When the stack can hold no more pieces, it stays full until the build + ends. The display is decorative, and the counts beside it carry the + facts. +- **Criterion.** A property test over every name in `names()` and + `game_names()`, 2000 seeds each (the count at which the harness found 66 + hangs), with inputs that ramp to 1 in bursts and a failure input. Each + seed runs under a watchdog, and a timeout fails the test process. The test fails on 2026.9.30.1: the + harness reproduced 66 hangs in 2000 runs. This follows the repository's + rule that an invariant is stated as a property test, not as an example. +- **Alternative rejected.** Drawing the frame outside `line_mutex()`, or + joining the ticker with a timeout, only moves the hang: the ticker is a + static `jthread` whose destructor joins again at exit, and a spinning + thread still occupies a core. + +**W2. The count states the work the build performs (F3).** + +- **The cache pass is reported but not counted.** + - Its `Cached` lines are unchanged. + - The row keeps its phase, `Planning`, during the cache pass, which took + 80 ms in the measured build (D1). +- **Scans that wait on no action run in a pass of their own, before the + main pass.** + - The pass builds a goal `_mcpp_scanned`, made of the `.dd` files of every + unit whose package has no action preceding compilation. It is emitted as + `_mcpp_staged_cache` is. + - The row shows the pass as `Scanning f/t` (D1). + - The restriction is required. A scan waits on its package's `prepare` + and `check` actions (`order_only_for`, `ninja_backend.cppm:2505-2511`). + A pass over every scan would hold every compile of every package behind + the longest such action: 12 min 49 s of CMake in the validation project. + That was the defect of revision 1's version of this item (section 7). + - Scans that wait on an action stay in the main pass, where they are + counted. There are none in xlings. +- **The main pass counts work.** + - After the scan pass, the main pass's `%t` counts compiles, links, + archives and actions, plus those remaining scans and the few runtime + placements beside a program. + - The measured clean build reads `Building 0/232` at its first compile. A + body edit reads `Building 0/2`. + - The animations take their fraction from the counted passes only. + - Every dyndep file of a pre-scanned unit is current when the main pass + starts, so ninja loads them at start-up. This is the order the cache + pass already relies on (ninja-build/ninja#2662). The growth of `%t` that + the #742 design measured (12 to 13 when `std.pcm` appeared) should + disappear, and the criterion checks it. +- **Both paths run the same passes.** The fast path runs ninja through + `run_ninja_reporting` too (`execute.cppm:1228-1300`). The passes live in + one function that both paths call. + - The fast path does not run the cache pass: it replays only a graph whose + staged files are current. + - A build with explicit goals (`mcpp test`, a named target) scans only the + `.dd` files of the units its goals compile, derived from the link units + the goals name when the goal phony is written. +- **Cost, stated.** + - A clean build: at most about 0.25 s. All scans and collations currently + end 0.21 to 0.24 s into the main pass; the root of the critical chain + already waits for its own scan; one more ninja load takes about 15 ms + (measured: the no-op cache pass took 15 ms). + - A no-op build: one more ninja load, from 0.05 s to about 0.07 s. + - A project of thousands of units on a small machine: compiles wait for + the last quick scan instead of overlapping it. The delay is bounded by + the scan pass's duration, which the pull request's timers (W9) state. +- **Alternative rejected: subtracting scans from `%t` with a dry run of the + scan goal.** It keeps one pass, but it is not exact. The collation after + an unchanged scan is pruned by `restat`, which lowers `%t` in a way mcpp + cannot attribute to a scan or to a compile. +- **Criterion.** + - An e2e test with a cache-served dependency. The first `Building` line + reads `0/N`, where N is the number of compile, link, archive and action + steps of the packages the cache does not serve, counted from + `steps.tsv`. The last reads `N/N`, and `%t` does not change in between. + The `Cached` lines are unchanged. + - An e2e test with a `prepare` action. Scans of the other packages do not + wait for it, and its package's compiles still start after it. + - The measured clean build of xlings grows by no more than 0.3 s. +- **Compatibility.** The e2e tests that read revision 3's counts (842, 843 + and those listed in #742 and #743) change. Revision 3's §7.2 statement + on the cache pass is superseded. + +**W3. #744, and no process spawned to learn a version already known (F6).** + +- **As the issue proposes.** + - One function selects the source: `MCPP_VENDORED_XLINGS` when set, + otherwise the newer of the released copy and the `PATH` copy, with the + released copy on a tie. + - `Updating` and `Note` are each stated at most once per process. + - Only a strictly newer source replaces the vendored binary. +- **The version memo.** The version of a binary is memoised per process. It + is also stored under the home, keyed by path, size and modification time, + and by inode where the platform provides one. + - An update of xlings writes a new file, which invalidates the entry. + - A stale entry can at worst delay an update until the file changes, + because a replacement still requires a newer candidate. +- **Criterion.** + - The issue's four e2e cases. + - An exec trace of a second planned `mcpp build` contains no + `xlings --version`. +- **Gain.** 0.35 s on every planned command. + +**W4. A plan is reused when an edit cannot change it (F5; D2 settled).** + +- **The principle.** `build.ninja` is a function of the manifests, the lock + file, the toolchain, the overrides, the set of source files, and each + unit's module interface: its module declaration, its partition, its + imports and its header units. A body edit changes none of these. A header + cannot supply a module declaration, and an `import` in an included header + is already invisible to the planner's scanner today, so a header edit + cannot change the plan either. +- **The change.** + - The fast-path record stores, per scanned unit of the project's own + packages, the interface the scanner extracted (D2), and the file set of + each glob. + - When a source is newer than `build.ninja`, the fast path rescans only + the newer files, re-expands the project's globs, and compares. If the + signatures and the file sets are equal, it runs the passes of W2 on the + recorded `build.ninja`. + - The checks the fast path already makes stay: the inputs of the build + programs, the resources, the `path` dependencies, the runtime manifest + and the request tag. +- **The post-link checks run from a record, on both paths.** + - Every check the full path runs after ninja on a relinked artifact moves + into one function: the runtime closure validation, and the check of the + surface the artifact walk cannot reach. + - That function takes a record, not the plan. The record holds the + runtime binding, the target triple, the library search directories and + whether host libraries are allowed. + - Both paths call it. Without it, every body edit would relink and the + fast path would decline (F5), so W4 would save nothing. +- **Scope.** The same decision serves `mcpp run`'s fast path and a + workspace's per-group records. A project with active `[hooks]` keeps + declining the fast path, as it does today. +- **Compatibility.** + - A record written before this pull request lacks the new fields and + declines once, the existing pattern for new record fields + (`depSourceRootsRecorded`). + - The new fields form an optional block that an older mcpp ignores. +- **Risk.** A missed input skips a plan silently. The matrix below is the + guard, and it is run with the comparison disabled to prove that it + detects the loss. +- **Criterion.** An e2e matrix. + - A body edit takes the fast path, and an exec trace shows no compiler + probe and no `xlings`. + - Each of the following takes the full path and builds correctly: + - adding an import; + - removing an import; + - renaming a module; + - adding a partition; + - turning a module unit into a non-module unit; + - adding a file; + - removing a file; + - editing `build.mcpp` or a declared input; + - editing `mcpp.toml`. + - A relinked artifact is validated on the fast path. A fixture whose + runtime closure is broken fails on the fast path as it does on the full + path. + - A revert probe: with the signature comparison disabled, the matrix + fails. +- **Gain.** The measured edit falls from 10.47 s to about 7.4 s: the compile + (5.5 s) and the `ld.bfd` link (1.8 s) remain. + +**W8. One walk per tree per plan (F5).** + +- **The change.** + - `expand_glob` takes a package's pattern list and groups the patterns by + literal prefix. + - It walks each distinct start once, and matches every entry against the + group's patterns. + - The expansions are memoised for the process by root and pattern list, + so the planner's three expansions of one package share one walk. +- **Scope.** Memoising across processes is not part of this pull request. + - An installed index tree is not strictly immutable at its version: + payload revisions (2026.9.27.1) reinstall a version in place. + - A cross-process key would need a tree identity that mcpp does not + record today. + - Skipping the scan of cache-served packages depends on such a key, and is + left to a later measurement with W9. +- **Criterion.** + - The directory opens in the planning window of the measured edit fall + from 30,372 to below 1,000. + - The file lists are unchanged, checked by the glob unit tests and by a + byte comparison of `build.ninja` before and after on xlings and on the + e2e fixtures. +- **Gain.** At least 0.5 s per planned build (measured in isolation); the + full figure comes from W9. + +**W9. Planning states its phases (observability).** + +- Each phase of `prepare_build` (`src/build/prepare/driver.cpp:61-73`) logs + its duration under `build/stage`, as `ninja_backend`'s `stage()` already + does for its own steps. +- W2's scan pass is logged the same way. +- Behaviour does not change. W8's and W2's criteria read these lines. + +**W10. #732 at its cause: a module is identified by its provider, and an +import is resolved in the importer's closure (F7).** + +- **The rule, in one sentence.** An import is resolved within the + dependency closure of the importing package, and each package's closure + provides a module name at most once. + - *The closure* is the package and every package it reaches through + code and workspace-member edges. For the package's test units it + also includes the dev edges. It excludes `artifacts` and `tools` + edges, whose programs are separate, and build dependencies, which + build in a sub-build of their own. + - *Uniqueness per closure* is the ABI's program rule stated at the package + level: every program's objects are the closure of its root package. + - Closures nest: a dependency's closure is contained in its consumer's. A + name that is unique in a consumer's closure therefore resolves to the + same provider for the consumer and for all of its dependencies, so no + BMI is ever read against a different module than the one it was built + against. +- **One resolver.** `mcpp.modgraph` gains the only answer to "which + provider": `providers(name)`, `resolve(importerPackage, name)` and + `bmi_path(unit)`. The four maps of F7 become calls to it. The plan's + last-wins map and the backend's first-wins map disappear, and with them + the chance that the two disagree. + - When the configuration has one provider of a name, `resolve` returns + it, whether or not it lies in the importer's closure. That is today's + behaviour, kept so that no existing import is newly refused. + - Otherwise `resolve` returns the one provider in the importer's closure. + It refuses when there is none, naming the providers and the closure. + - The scanner's check becomes per closure. The message states the + consequence the ABI gives it: the program would define `value@common()` + twice. The "one file reached as two packages" hint compares file + identity (`std::filesystem::equivalent`) instead of spelling. +- **Location: disambiguate only on collision.** + - `bmi_path(unit)` stays `/` when the configuration has + one provider of the name. + - It becomes `//` when it has more + than one. + - This is the rule mcpp#233 already applies to object paths: flat, unless + two files would collide. + - The staging of a cached BMI (`configure.cppm:72`) uses the same + function, and the global cache's entries do not change. +- **Binding the compilers, only in affected packages.** An affected package + is one whose closure contains a name the configuration provides more than + once. + - **GCC.** One mapper file per affected package (`modmap/.map`, + GCC's `name path` format), used by all of that package's units through + `-fmodule-mapper=`. + - It lists every named module of the closure, `std` and `std.compat` + included, because GCC has no fallback for an unlisted name (F7). The + list is derived from the resolver over the closure, not from what + the units happen to import, so it is complete by construction. + - Header units are refused by the scanner (`scanner.cppm:1046`), so + named modules are all a map must hold. + - Providers write their BMI through the same file. The split schedule's + `bmi-compile --bmi` reads the same `bmi_path`. + - **clang.** An affected unit gets `-fmodule-file==` for each + collided name of its closure. Other names are still found through + `-fprebuilt-module-path` (F7). Providers already take + `-fmodule-output=`. + - **MSVC.** `/reference =` for the same names. Providers + already take `/ifcOutput`. + - **Collation.** `mcpp dyndep` gains `--module-map `: a mapped name + resolves to its path, and any other name to `--bmi-dir` and `--bmi-ext`, + as today. The GCC-format map file serves it on every compiler. + - The map files are written only when their content changes, and they are + inputs of the edges that read them, as `placements.list` is (#734 E4). +- **What does not change.** + - When no name has two providers, no map is written, no flag is added and + no path moves. `build.ninja`, `compile_commands.json`, the BMI cache and + the fast-path record are byte-identical. + - Every existing project that builds today is in that case, and so are + all 172 packages of the index. +- **What the change allows.** + - Case B: an `artifacts` program, or a workspace member, with its own + `common`. + - Case A as it stands: each program compiles `boost.ixx` in its own + package. A note states that one file is compiled as two packages, and + that a package both depend on would compile it once. +- **What stays refused.** Two providers of one name in one closure, + including one file reached twice within one closure: the ABI cannot link + that program. +- **Known limit.** clangd keys the providers of a compilation database by + module name. In a project that uses a collided name, the editor may + therefore resolve an importer to the other program's module; the build + stays correct. The limitation is stated in the docs, next to the rule. +- **Criterion.** The probes of F7 as e2e fixtures, on each compiler family + of the CI matrix (GCC, clang, and MSVC on Windows, where `/reference` + precedence is measured for the first time): + - an app and its `artifacts` updater, each with a different `common`; + - two independent workspace members, each with a different `common`; + - case A with the `..` spellings. + + Each program returns its own module's value. The criterion also covers: + - the refusal of two providers in one closure, and of one file reached + twice in one closure; + - an affected GCC unit that imports `std`, a unique module and the + collided module, which shows that the map is complete; + - an edit of one `common`, which rebuilds only its own closure's + importers; + - a byte comparison of `build.ninja` and `compile_commands.json` before + and after the change, for every e2e fixture without a collision and for + xlings. +- **Alternatives rejected.** + - *Revision 2's rule* (refuse a name twice per configuration). It leaves + the cause in place and refuses valid programs. + - *A sub-build per program.* It compiles shared dependencies twice, which + contradicts #711's "nothing built twice" and the workspace's one graph + per configuration. + - *Explicit maps for every unit*, as CMake writes them. The approach is + uniform, but it changes every `build.ninja`, every database entry and + every BMI path, with no gain for the projects that have no collision. + +### Recorded and not in this pull request + +- **W5. The split schedule stays opt-in.** + - Measured gain on xlings: 1.5 s of 35.5 s (4%). + - Its policy requires verification on every platform before it becomes a + default, because a wrong schedule is wrong silently + (`policy.cppm:196-201`). + - A gain of 4% does not justify adding that risk to a pull request that + already changes the pass structure (W2) and the fast path (W4). +- **W6. A faster linker (D3: recorded, not done).** + - `ld.lld` links xlings in 0.19 s against 1.54 s for `ld.bfd`. + - A toolchain is not composed from another package's payload: the lld + inside the llvm payload is not used for a GNU toolchain. The option + exists only once the ecosystem has an independent `lld` package, and + it would then be an opt-in key resolved to that package. +- **W7. Dropped (D4).** + - A build program stays in the project's build directory, a dependency's + in the consuming project's (`build_program.cppm:586-595`). + - `mcpp clean` removes it, as it does today. + - The 0.79 s rebuild of xlings's one dependency build program after + `mcpp clean` is the accepted cost of that rule. +- **W11. Cache placement on Windows.** + - The cache pass launches one `mcpp stage` per file: 504 processes in the + measured build, 80 ms on Linux. + - On Windows, #734 E4 measured 4.5 s for 1270 per-file placements against + 0.5 s for one process, and moved runtime placements to `stage_list`. + - This pull request's Windows validation run records the cache pass's + duration from the log line mcpp already writes + (`build/stage: ninja-staged-cache`). The change waits for that reading. + +## 5. The pull request + +One pull request, in this order of commits. Each commit builds and passes +the unit tests, so a bisect over the pull request stays meaningful. + +| Order | Commit | Why here | +|---|---|---| +| 1 | W9, planning phase timers | Every later commit is measured with them | +| 2 | W1, animation termination, with the property test | Independent, and the most urgent | +| 3 | W3, #744 and the version memo | Independent | +| 4 | W8, one walk per tree | Changes planning cost only; `build.ninja` is byte-identical | +| 5 | W10, module identity by provider and resolution by closure | `build.ninja` is byte-identical without a collision; before W2 and W4 so that their records carry the resolver's paths | +| 6 | W2, the pass structure and the count | Needed by W4's fast path | +| 7 | W4, plan reuse and post-link checks from a record | Last, measured after W8; deferred on that measurement (section 10.3) | + +Verification before merge, following the repository's practice: + +- the unit tests and the e2e suite on Linux, macOS and Windows CI; +- the sandbox ecosystem run; +- the validation project's cross-verification from the pull request's + branch. It checks W2 against a project with 12-minute `prepare` actions + and supplies W11's Windows reading. + +The pull request's description states the before-and-after readings for +the three builds of section 5.1. + +### 5.1 Expected figures for xlings on this machine + +| Build | Today | After the pull request | +|---|---|---| +| Clean | 35.4 s | about 34.8 s: −0.35 s (W3), −0.5 s or more (W8), up to +0.25 s (W2) | +| Edit of one leaf `.cpp` | 10.5 s | about 7.4 s: planning removed (W3, W4); the compile and the link remain. Measured with W3 and W8 and without W4: 8.0 s (section 10.2) | +| No-op | 0.05 s | about 0.07 s: one more ninja load (W2) | + +The clean build stays near 35 s because of the import chain (E1). The edit +stays above 7 s because of the one compile (5.5 s) and `ld.bfd` (1.8 s). + +## 6. Decisions + +Settled in review round 1: + +- **D1.** The row keeps its phase, `Planning`, without a count during the + cache pass, and shows `Scanning f/t` during the scan pass. +- **D2.** W4's signature is the interface the scanner extracts. +- **D3.** W6 is recorded and not done. A toolchain is not composed from + another package's payload; only an independent `lld` package would make + the option possible. +- **D4.** `mcpp clean` removes build programs. W7 is dropped, and build + programs stay per project. +- **D5.** One pull request. + +Open for review round 2: + +- **D6.** #732 (F7, W10): + - an import is resolved in the importer's closure, and a name is unique + per closure; + - BMI paths and compiler bindings change only for a collided name; + - four maps become one resolver; + - clangd's per-name view of a collided name is a stated limit. +- **D7.** W5 stays opt-in and outside this pull request. +- **D8.** W2's scan pass, with its stated costs: up to 0.25 s on the clean + build, about 15 ms on a no-op, and a bounded delay for very large projects. + +## 7. Self-review + +Revisions 1 and 2 were checked against the code, against the measurements +and across items. Five statements were wrong or incomplete; each is +corrected above. + +| # | Earlier text | What the check found | Now | +|---|---|---|---| +| 1 | W2: scans run in a pass over every `.dd` | A scan waits on its package's preceding actions (`ninja_backend.cppm:2505-2511`). The pass would hold every compile behind the longest `prepare` action (12 min 49 s in the validation project) | The scan pass covers only scans that wait on no action | +| 2 | W4: rescan and compare, then replay ninja | The fast path declines after any relink, because the post-link validation reads the plan (`runtime_validation.cppm:640-776`). A body edit always relinks, so W4 as written would have saved nothing and run ninja twice | The post-link checks run from a record, on both paths | +| 3 | W8: memoise index trees across processes as immutable | Payload revisions reinstall a version in place | Per-process memo only. The cross-process part waits for a tree identity | +| 4 | Revision 1's W10: per-program scopes for every graph. Revision 2's W10: refuse a name twice per configuration | Revision 1 moved every BMI path and database entry. Revision 2 refused valid programs and left four disagreeing maps in place. The measurements of F7 show that GCC needs a complete map and clang an explicit flag, only for the collided names, and that the ABI draws the boundary per program | Revision 3's W10: resolution by closure, one resolver, and disambiguation only on collision; byte-identical when no name collides | +| 5 | W7: move build programs to the store | Contradicts D4, and the measured 0.79 s belongs to a dependency's build program, which is already per project by design | Dropped | + +The checks that found nothing to change: + +- **Measurements.** + - F2's means use the same six-run sets for both versions. The one outlier + is explained by its own ninja pass. + - F4's critical path uses the dyndep edges the build itself used. + - F3's composition sums to the logged total: 503 + 230 + 230 + 231 + 1 = + 1195, which equals `progress: 1195 steps` in the log. +- **Platforms.** + - W1, W3 and W8 have no platform branch. + - W10 binds each compiler family in its own spelling. GCC 16 and clang 22 + were measured; MSVC's `/reference` precedence over `/ifcSearchDir` is + measured for the first time by W10's fixtures on the Windows leg of CI. + - W3's key uses the inode only where the platform provides one. + - W2 adds one process launch per build, which costs more on Windows but is + bounded to one. + - W4 uses the fast path that exists on every platform since 2026.9.28.3. +- **Workspaces.** + - W2 runs per configuration graph. + - W4 extends the per-group records of the workspace fast path. + - W10 allows two members with one module name when they share no + program, and still refuses the case where they do. +- **Interactions between items.** + - W2 and W4 share the pass function. + - W8 speeds W4's re-expansion of the project's globs. + - W9 is the instrument for W2's and W8's criteria. + - W3's memo is read before any plan, so W4's fast path does not depend on + it. +- **Compatibility.** + - `build.ninja` is byte-identical after W8, and after W10 for every graph + without a collided name. + - W2 changes the counts that e2e tests read. They are updated in the same + commit. + - W4's record gains an optional block. An old record declines the fast + path once. + - No manifest key and no index format changes. +- **W10 in particular.** + - *Consistency.* Closures nest. A name that is unique in a consumer's + closure resolves to the same provider for all of its dependencies, so + no unit reads a BMI built against another module of the same name. + - *GCC's missing fallback.* The map lists every named module of the + closure, `std` and `std.compat` included. It is derived from the + resolver, not from the imports a scan found. + - *Header units.* The scanner refuses them (`scanner.cppm:1046`), so a map + holds named modules only. + - *The scan step.* A scan wraps its unit's compile command + (`ninja_backend.cppm:2148-2150`), so the mapper flag reaches the scan as + well. + - *The split schedule.* `bmi-compile --bmi` takes its path from + `bmi_path`, the same function the mapper file is written from. + - *The BMI cache.* Its entries do not change. Staging already moves a BMI + from the cache to the build directory, so a BMI does not depend on its + location; only the destination comes from `bmi_path`. + - *W4.* A changed module declaration changes the unit's signature, so W4 + plans again, and the map files are regenerated by that plan. + - *The one limit found.* clangd's view of a collided name (W10, known + limit). +- **Documentation and prose.** + - W2's counts and phases, W10's rule and its clangd limit are stated in + `docs/` and `docs/zh/`. + - The CHANGELOG entry and the commit messages are English. + +## 9. Tasks, their dependencies, and the repositories + +| Task | Repository | Depends on | Criterion | +|---|---|---|---| +| T1 W9, phase timers (and the finish steps) | mcpp | none | `build/stage` lines in the log of a planned build | +| T2 W1, animation termination | mcpp | none | property test; fails on 2026.9.30.1 | +| T3 W3, #744 and the version memo | mcpp | none | unit tests, e2e 846; 846 fails on 2026.9.30.1 | +| T4 W8, one walk per tree | mcpp | T1 | directory opens of a planned edit; modgraph tests | +| T5 W10, #732 | mcpp | none | e2e 847 (GCC, clang), 848 (MSVC); byte comparison without a collision | +| T6 W2, pass kinds and the count | mcpp | T5 (the scan goal reads the scopes' owners) | e2e 842, 843; unit test | +| T7 W4, plan reuse | mcpp | T4, T6 | deferred (section 10) | +| T8 documentation, CHANGELOG, version | mcpp | T2 to T6 | docs in both languages; version pins | +| T9 one pull request, CI on every platform | mcpp | T8 | every required check | +| T10 release and the GitCode mirror | mcpp, xlings-res | T9 | four archives, GET on both hosts | +| T11 the index entry | openxlings/xim-pkgindex | T10 | the bot's bump merged; `latest` read back | +| T12 the index's CI pin | mcpp-community/mcpp-index | T11 | `validate.yml` and `latest_mcpp` on 2026.9.30.2 | +| T13 ecosystem verification in a sandbox | local | T11 | `xlings subos use --sandbox` with CN mirrors | +| T14 issues | mcpp, xlings | T13 | #744 and #732 closed with the evidence; xlings#638 opened for E2 | +| T15 acceptance on the validation project | Sunrisepeak/GalTranslPP | T11 | the pull request's CI with the released mcpp | + +xlings needs no change in this round: its pin is already the latest release +(2026.9.30.1). The start-up cost of `xlings --version` (E2) is xlings's, and is +stated as openxlings/xlings#638; W3 removes mcpp's dependence on it. + +## 10. Implementation record + +### 10.1 What was built + +- **W9.** Every phase of `prepare_build`, and every step of its last phase, + logs `plan : ` under `build/stage` when the log is at info level + or `--verbose` is on; the backend's own stage lines follow the same gate. +- **W1.** `landing()` answers nothing for a piece that would rest outside the + screen, `spawn()` answers nothing when no candidate lands inside, and both + fill loops end when a spawn fails or a locked piece adds no cell. The + property test drives the four animations over 2000 seeds and the three games + over 500 (not 10,000: 2000 is the count at which the harness found 66 hangs, + and the test runs in 4 s). It fails on 2026.9.30.1 (`an animation 'stack' + did not return within 120 s`). +- **W3.** `choose_xlings_source` is the pure choice and `select_xlings_source` + gathers the candidates; the first acquisition and the replacement both use + it, and the replacement copies the chosen file. The memo is + `/cache/vendored-xlings.versions`, one ` + ` line per binary. A home settled in a process is not examined + again. +- **W8.** The walk is kept per (root, start) and matched by the text after the + pattern's last `*` before the matcher runs. The same kept walk serves the + scanner, features, graph loading and every other caller of `expand_glob`, + which is why the graph and feature phases shrank as well as the scan. +- **W10.** As in section 4, with three findings from the implementation: + - mcpp's naming rule refuses `common` as a top-level module name (it is one + of `core`, `util`, `common`, `std`, `detail`, `internal`, `base`), so the + minimal example of #732 is refused by that rule before this one; the + fixtures use `boost`, the reporter's own module. + - The reported layout (case A) builds without a note: it violates no rule, + and a note on every build of a valid layout would be noise. + - A package that provides a collided name is not placed in the global cache, + whose entries name BMIs by module; it compiles in the project. +- **W2.** `PassKind` (`Placement`, `Scan`, `Work`) on every pass. Two + corrections from the tests: a failed scan pass ends the build with its own + output (the main pass would report the failed step twice), and a scan pass + names no package at its end (it named every package at once, out of order; + e2e 842). A build of named goals scans in its main pass. + +### 10.2 Measured (xlings, this machine) + +| Build | 2026.9.30.1 | 2026.9.30.2 | +|---|---|---| +| Clean, wall time (mean of 3 or more) | 35.4 s | 34.4 s | +| Clean, ninja starts at | about 4.5 s | about 1.3 s | +| Edit of `doctor.cpp` | 10.5 s | 8.0 s | +| No-op | 0.05 s | 0.05 s | +| Planning of that edit | 3.05 s | 0.43 s | +| Directory opens before ninja, that edit | 30,372 | 1,749 | + +The planning that remains: make plan 112 ms, the dependency cache 176 ms, the +other phases under 30 ms each. After ninja: runtime validation 78 ms, loader +tags 74 ms, symbol provision 136 ms. The status row of the clean build reads +`Scanning 46/460`, then `Building 20/232` at 0:02, rising steadily to +`231/232` at 0:34. + +### 10.3 W4, deferred on its own gate + +Section 5 placed W4 last, to be measured after W8. W8 and W3 removed 2.6 s of +the 3.05 s W4 was to save; what W4 could still save is about 0.5 s of an +8.0 s edit (planning and emission), against a new class of silent staleness +and a record-based post-link check on both paths. It is not in this pull +request. The next measured target, if planning becomes material again, is the +dependency cache step (176 ms), whose keys could be kept per tree identity +without a new staleness class. + +### 10.4 Verification + +- Unit tests of the touched subsystems: dots screen, xlings version, + modgraph, pack interface, dyndep, build progress. +- e2e 687, 842, 843, 845, 846, 847 (under GCC 16 and clang 22), 801, 805, + 806, 19, 172, 196 and 114 on this machine. 212 fails on this machine with + 2026.9.30.1 as well: its criterion reads GCC's `gcm.cache`, and this home's + default toolchain is llvm. +- The byte comparison of section 4 (W10): `build.ninja` (3,798 lines) and + `compile_commands.json` of the xlings workspace, 2026.9.30.1 against the + branch, with the binary's own path normalised: identical. +- Revert probes: the W1 property test, e2e 846 and e2e 847 each fail on + 2026.9.30.1. + +## 8. Appendix: readings + +- **Clean builds, wall time (s).** + - 2026.9.28.2, pty: 37.82, 35.08, 34.11; pipe: 37.31, 34.53, 36.40; + warm-up: 38.09. + - 2026.9.30.1, pty: 35.91, 35.35, 35.04; pipe: 35.62, 35.02, 43.06 + (ninja 38.65); warm-up: 36.94. +- **`Finished` (s).** + - 2026.9.28.2: 33.63, 31.41, 30.37, 33.08, 30.78, 32.61. + - 2026.9.30.1: equal to the wall time within 0.02 s. +- **Split schedule (2026.9.30.1, clean, s).** + - Default: 35.23, 35.86 (ninja 31.00, 31.83). + - `MCPP_BMI_SCHEDULE=on`: 33.38, 34.70 (ninja 29.40, 30.47). +- **Link of `bin/xlings` (s).** `ld.bfd`: 1.59, 1.54, 1.49. `ld.lld` 22.1.8: + 0.19, 0.20, 0.19. +- **Incremental builds (s).** + + | Build | 2026.9.30.1 | 2026.9.28.2 | + |---|---|---| + | No-op | 0.05 | 0.03 | + | Edit of `doctor.cpp` | 10.47 | 11.57 | + | `touch` of `cancellation.cppm` | 5.30 | 6.49 | + | Second no-op | 0.05 | 3.56 (not the fast path) | +- **The hang harness.** The body of `class Stack` from `stack.cppm`, with its + first fill loop capped at 100,000 iterations and the cap reported, driven + by 2000 seeds over a 240-step build at ten frames per second: 66 runs + reached the cap. +- **Exec counts in the clean build.** + + | Process | Count | + |---|---| + | `sh` | 1199 | + | `mcpp stage` | 504 | + | `g++` | 469 | + | `cc1plus` | 463 | + | `rm` | 356 | + | `as` | 233 | + | `mcpp dyndep` | 230 | + | `awk` | 230 | + | `ninja` | 2 | + | `xlings --version` | 1 | diff --git a/.agents/docs/README.md b/.agents/docs/README.md index e9acfb82..20380229 100644 --- a/.agents/docs/README.md +++ b/.agents/docs/README.md @@ -18,7 +18,7 @@ superseded_by: 2026-09-07-....md # when status is superseded --- ``` -319 records. +320 records. ## By subject @@ -30,6 +30,7 @@ Records that declare one. Everything else is listed by date below. ### design +- [The build's wall time, its progress count, a hang after the build, and #732 and #744: measurements and a remediation plan](2026-09-30-build-wall-time-progress-count-and-hang-plan.md) — landed - [Build output, revision 3: every package that does work is named, the live display is one line drawn in one write, and a repeated warning is stated once per file](2026-09-30-build-output-refinement-design.md) — landed - [The workspace as the unit of build: one graph per configuration, one scheduler, product directories, and a reusable graph module](2026-09-29-workspace-build-graph-design.md) — landed - [Build progress: each step's line states its outcome, and one status line states the build](2026-09-29-build-progress-display-design.md) — landed @@ -110,6 +111,7 @@ Records that declare one. Everything else is listed by date below. ### 2026-09 +- [The build's wall time, its progress count, a hang after the build, and #732 and #744: measurements and a remediation plan](2026-09-30-build-wall-time-progress-count-and-hang-plan.md) — landed - [Build output, revision 3: every package that does work is named, the live display is one line drawn in one write, and a repeated warning is stated once per file](2026-09-30-build-output-refinement-design.md) — landed - [The workspace as the unit of build: one graph per configuration, one scheduler, product directories, and a reusable graph module](2026-09-29-workspace-build-graph-design.md) — landed - [Build progress: each step's line states its outcome, and one status line states the build](2026-09-29-build-progress-display-design.md) — landed diff --git a/CHANGELOG.md b/CHANGELOG.md index 6cac3cee..82fe3846 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,66 @@ > Each `## []` section is that release's notes. Entries are written in English > from 2026.9.28.3 on; earlier entries remain as written. +## [2026.9.30.2] - 2026-09-30 + +This release answers five reports on 2026.9.30.1 while building xlings +(`.agents/docs/2026-09-30-build-wall-time-progress-count-and-hang-plan.md`): a +build that did not exit after its status row stopped, a status row whose count +was mostly bookkeeping, planning that preceded every edit's compile by three +seconds, mcpp#744, and mcpp#732. On a clean build of xlings the command starts +ninja at 1.3 s instead of 4.5 s; an edit of one source builds in 8.0 s instead +of 10.5 s; a build with nothing to do is unchanged at 0.05 s. + +### Fixed + +- **A build no longer hangs after ninja.** The stack animation could spawn + pieces onto cells it already held once its stack reached the right edge + short of its target, and its loop then never ended while the status row held + the terminal's line lock; the build joined the row's thread and waited for + ever (about 1% of interactive builds). Every loop of the animation now grows + the stack or ends. A property test drives every animation over 2000 seeds and + every game over 500 under a watchdog; it fails on the previous animation. +- **The status row counts the work of the build.** A clean build of xlings + counted 1195 steps, of which 503 placed files the global cache serves and 460 + were dependency scans, and read 967/1195 when its first compile began. The + cache pass is reported by its `Cached` lines and not counted; the scans that + wait on no action run first, shown as `Scanning f/t`; and `Building f/t` + counts the compiles, links, archives and actions: 0/232 at the first compile + of the same build. The fast path runs the same passes (e2e 842, 843). +- **A vendored xlings is replaced from the newest source (mcpp#744).** + `MCPP_VENDORED_XLINGS` when set, otherwise the newer of the xlings released + with mcpp and the xlings on `PATH`; the note that no newer source is + available was false when a newer xlings was on `PATH`. `Updating` and `Note` + are each stated once per process (e2e 846). +- **A module name is unique within one program, not within one build + (mcpp#732).** An `artifacts` program, or a workspace member that shares no + program with another, may provide a module of the same name as another + program of the build. An import is resolved in the importing package's + closure; two BMIs of one name lie below their packages' directories, and each + compile is told which one a name means (a module map for GCC, + `-fmodule-file=` for Clang, `/reference` for MSVC). Two providers that one + program links are refused, naming that program's package, and one file + reached as two packages is recognised whatever its spelling. When every name + has one provider, `build.ninja` and `compile_commands.json` are byte-identical + to 2026.9.30.1's (e2e 847, 848). + +### Changed + +- **Planning walks each package tree once.** A source pattern with an empty + literal prefix walked the whole package tree: the 127 patterns of + `compat.libarchive`, expanded about three times per plan, opened its 35 + directories 13,406 times. A walk is now kept per tree for the command and + revalidated by its directories' modification times. A planned edit of one + xlings source opens 1,749 package directories instead of 30,372, and its + scan phase takes 43 ms instead of 0.91 s. +- **The version of the vendored xlings is asked once.** It is kept per + process, and under the home keyed by the binary's path, size and + modification time, so a command that loads its configuration no longer runs + `xlings --version` (0.35 s) when the binary has not changed. +- **Planning states where its time goes.** Each phase of planning, and each + step of its last phase, logs its duration under `build/stage` whenever the + log file or `--verbose` would show it. + ## [2026.9.30.1] - 2026-09-30 This release revises what a build prints, from a report on `mcpp build` in the diff --git a/docs/09-commands-by-scenario.md b/docs/09-commands-by-scenario.md index 36861ae4..dca833e5 100644 --- a/docs/09-commands-by-scenario.md +++ b/docs/09-commands-by-scenario.md @@ -277,8 +277,9 @@ On a terminal one status row is drawn below the output and updated in place: Building ⠀⠀⠀⠀⠀⠀⠀⠀⠀⠀⢾⡷⠀⠰⣿⠆⠄⠄⠄⠄⠄⠄⠄⠄ 612/707 · 0:35 · gpp.gui: CMAKE ElaWidgetTools 6:10 ``` -- The phase (`Planning`, `Running` for the build programs, `Building`, - `Stopping` after a failure, `Checking`) is aligned with the verbs above it. +- The phase (`Planning`, `Running` for the build programs, `Scanning` for the + dependency scans, `Building`, `Stopping` after a failure, `Checking`) is + aligned with the verbs above it. - Beside it, a screen of 24 braille cells plays one of four animations, chosen per command: a chomper whose position is the progress, a snake that eats a food in the colour of each package that starts, Tetris on its side @@ -286,7 +287,13 @@ On a terminal one status row is drawn below the output and updated in place: bar. An animation moves slowly while mcpp works and faster as steps finish, and it stands still while the build waits. - Then come the steps finished and planned, and the time since the command - started. When no step is left to start, `last N running` follows. + started. When no step is left to start, `last N running` follows. The + steps of `Building` are the work of the build: its compiles, links, + archives and actions (2026.9.30.2+). Placing what the global cache serves is + reported by the `Cached` lines and not counted, and the dependency scans run + first, as `Scanning` with a count of their own; a scan that waits for a + package's `prepare` or `check` action runs with the build and is counted + there. - Last comes the longest-running `check` or `prepare` action. ninja reports every other step only when it finishes. - The row is first drawn half a second into the command. Every change leaves diff --git a/docs/zh/09-commands-by-scenario.md b/docs/zh/09-commands-by-scenario.md index 92f8a36e..0dc141c0 100644 --- a/docs/zh/09-commands-by-scenario.md +++ b/docs/zh/09-commands-by-scenario.md @@ -247,7 +247,7 @@ $ mcpp build Building ⠀⠀⠀⠀⠀⠀⠀⠀⠀⠀⢾⡷⠀⠰⣿⠆⠄⠄⠄⠄⠄⠄⠄⠄ 612/707 · 0:35 · gpp.gui: CMAKE ElaWidgetTools 6:10 ``` -- **阶段**：阶段（`Planning`，构建程序阶段为 `Running`，`Building`，失败后为 `Stopping`，`Checking`）与上方的动词对齐。 +- **阶段**：阶段（`Planning`，构建程序阶段为 `Running`，依赖扫描阶段为 `Scanning`，`Building`，失败后为 `Stopping`，`Checking`）与上方的动词对齐。 - **点阵屏**：阶段之后是由 24 个盲文点字格组成的点阵屏，每次命令随机播放四种动画之一。 - 吃豆人：位置即进度。 - 贪吃蛇：每有一个包开始，就吃下一颗该包来源颜色的食物。 @@ -255,7 +255,7 @@ $ mcpp build - 离子发射器：离子堆积成进度条。 动画在 mcpp 工作时缓慢移动，有步骤完成时加快，构建等待时静止。 -- **计数与时间**：随后是已完成与计划的步骤数，以及自命令开始的时间。当没有待启动的步骤时，追加 `last N running`。 +- **计数与时间**：随后是已完成与计划的步骤数，以及自命令开始的时间。当没有待启动的步骤时，追加 `last N running`。`Building` 的步骤是构建的工作：编译、链接、归档与 action（2026.9.30.2+）。放置全局缓存提供的内容由 `Cached` 行报告，不计入；依赖扫描先行，以 `Scanning` 显示，并有自己的计数；需要等待某个包的 `prepare` 或 `check` action 的扫描随构建一起运行，并计入构建。 - **正在运行的动作**：最后是运行最久的 `check` 或 `prepare` 动作。其余步骤只在完成时由 ninja 报告。 - **绘制方式**：状态行在命令开始半秒后才首次绘制；每次更新都以一次写入原地覆盖，不会闪烁。 diff --git a/mcpp.toml b/mcpp.toml index 4d977991..0b595e0a 100644 --- a/mcpp.toml +++ b/mcpp.toml @@ -1,6 +1,6 @@ [package] name = "mcpp" -version = "2026.9.30.1" +version = "2026.9.30.2" description = "Modern C++ build & package management tool" license = "Apache-2.0" authors = ["mcpp-community"] diff --git a/modules/versioning/src/version.cppm b/modules/versioning/src/version.cppm index 204e13f2..2b3e3e89 100644 --- a/modules/versioning/src/version.cppm +++ b/modules/versioning/src/version.cppm @@ -31,6 +31,6 @@ import std; export namespace mcpp { -inline constexpr std::string_view MCPP_VERSION = "2026.9.30.1"; +inline constexpr std::string_view MCPP_VERSION = "2026.9.30.2"; } // namespace mcpp From 95c52dc0cbbdabaa13a25b878a19e2004a0ef422 Mon Sep 17 00:00:00 2001 From: speak-agent <248744407+speak-agent@users.noreply.github.com> Date: Wed, 30 Sep 2026 13:21:54 +0800 Subject: [PATCH 10/15] docs: the sandbox verification script of 2026.9.30.2 Eight sections against the published release: the three #732 cases, index packages built with it, the status row's count against the units the cache placed, #744's source choice, and the planning timers. Run locally against the branch: 8 passed; against 2026.9.30.1 as the control, every section marked CHANGE fails. --- .../docs/2026-09-30-build-wall-time-verify.sh | 151 ++++++++++++++++++ 1 file changed, 151 insertions(+) create mode 100644 .agents/docs/2026-09-30-build-wall-time-verify.sh diff --git a/.agents/docs/2026-09-30-build-wall-time-verify.sh b/.agents/docs/2026-09-30-build-wall-time-verify.sh new file mode 100644 index 00000000..6ba4d821 --- /dev/null +++ b/.agents/docs/2026-09-30-build-wall-time-verify.sh @@ -0,0 +1,151 @@ +#!/usr/bin/env bash +# Sandbox verification of mcpp 2026.9.30.2 (.agents/docs/2026-09-30-build- +# wall-time-progress-count-and-hang-plan.md), run against the PUBLISHED release +# inside an xlings sandbox: +# +# B64=$(base64 -w0 .agents/docs/2026-09-30-build-wall-time-verify.sh) +# xlings subos new v9302 2>/dev/null || true +# xlings subos use v9302 --sandbox --cmd "echo $B64 | base64 -d > /tmp/v.sh && VER=2026.9.30.2 bash /tmp/v.sh" +# +# VER selects the release under test. Running it with VER=2026.9.30.1 is the +# control: every section marked CHANGE must fail there, and every other section +# must pass on both. +# +# Every probe directory is removed at the start of its section, because the +# sandbox's $HOME persists between runs of the same subos. A section that does +# not run is reported as SKIP and counted separately from a pass. +set -u +VER="${VER:-2026.9.30.2}" +W="$HOME/v9302" +fails=0; passes=0; skips=0 +pass() { echo "PASS $1"; passes=$((passes+1)); } +fail() { echo "FAIL $1"; [ -n "${2:-}" ] && [ -f "$2" ] && tail -15 "$2"; fails=$((fails+1)); } +skip() { echo "SKIP $1"; skips=$((skips+1)); } + +# ── 0. the release under test, from the published channel ────────────────── +if [ -n "${MCPP_OVERRIDE:-}" ]; then + MCPP="$MCPP_OVERRIDE" +else + xlings config --mirror CN >/dev/null 2>&1 || true + xlings update >/dev/null 2>&1 || true + xlings install "mcpp@$VER" -y > /tmp/v9302-install.log 2>&1 || true + MCPP="$HOME/.xlings/data/xpkgs/xim-x-mcpp/$VER/bin/mcpp" +fi +if [ ! -x "$MCPP" ]; then + echo "FATAL: mcpp $VER is not installable from the index"; tail -20 /tmp/v9302-install.log; exit 2 +fi +got=$("$MCPP" --version 2>&1 | head -1) +case "$got" in *"$VER"*) pass "0 installed: $got";; *) fail "0 version: $got";; esac +"$MCPP" self config --mirror CN >/dev/null 2>&1 || true +mkdir -p "$W" + +module_file() { printf 'export module boost;\nexport int value() { return %s; }\n' "$2" > "$1"; } +main_file() { printf '#include \nimport boost;\nint main() { std::printf("%%d\\n", value()); }\n' > "$1"; } +bin_of() { find "$1" -path '*/bin/*' -name "$2" -type f | head -1; } + +# ── 1. CHANGE: #732, an artifacts program with its own module of one name ── +rm -rf "$W/s1"; mkdir -p "$W/s1/app/src" "$W/s1/updater/src"; cd "$W/s1" +module_file app/src/boost.cppm 1; main_file app/src/main.cpp +module_file updater/src/boost.cppm 2; main_file updater/src/main.cpp +printf '[package]\nname = "updater"\nversion = "0.1.0"\n\n[targets.updater]\nkind = "bin"\nmain = "src/main.cpp"\n' > updater/mcpp.toml +printf '[package]\nname = "app"\nversion = "0.1.0"\n\n[dependencies]\nupdater = { path = "../updater", artifacts = ["updater"] }\n\n[targets.app]\nkind = "bin"\nmain = "src/main.cpp"\n' > app/mcpp.toml +if (cd app && "$MCPP" build > ../s1.log 2>&1) \ + && [ "$("$(bin_of app/target app)")" = 1 ] && [ "$("$(bin_of app/target updater)")" = 2 ]; then + pass "1 CHANGE: the app and its artifacts updater each have their own module boost" +else fail "1 CHANGE: two programs, one module name" s1.log; fi + +# ── 2. #732, two providers in one program are refused ─────────────────────── +rm -rf "$W/s2"; mkdir -p "$W/s2/lib1/src" "$W/s2/lib2/src" "$W/s2/prog/src"; cd "$W/s2" +module_file lib1/src/boost.cppm 1; module_file lib2/src/boost.cppm 2 +for l in lib1 lib2; do printf '[package]\nname = "%s"\nversion = "0.1.0"\n' "$l" > $l/mcpp.toml; done +printf 'int main() { return 0; }\n' > prog/src/main.cpp +printf '[package]\nname = "prog"\nversion = "0.1.0"\n\n[dependencies]\nlib1 = { path = "../lib1" }\nlib2 = { path = "../lib2" }\n\n[targets.prog]\nkind = "bin"\nmain = "src/main.cpp"\n' > prog/mcpp.toml +if (cd prog && "$MCPP" build > ../s2.log 2>&1); then fail "2 two providers in one program were accepted" s2.log +elif grep -q "module 'boost' is provided by package" s2.log; then pass "2 two providers in one program are refused" +else fail "2 the refusal does not name the module" s2.log; fi + +# ── 3. CHANGE: #732, two workspace members with one module name ───────────── +rm -rf "$W/s3"; mkdir -p "$W/s3/one/src" "$W/s3/two/src"; cd "$W/s3" +module_file one/src/boost.cppm 5; main_file one/src/main.cpp +module_file two/src/boost.cppm 6; main_file two/src/main.cpp +printf '[workspace]\nmembers = ["one", "two"]\n' > mcpp.toml +for m in one two; do printf '[package]\nname = "%s"\nversion = "0.1.0"\n\n[targets.%s]\nkind = "bin"\nmain = "src/main.cpp"\n' "$m" "$m" > $m/mcpp.toml; done +if "$MCPP" build --workspace > s3.log 2>&1 \ + && [ "$("$(bin_of target one)")" = 5 ] && [ "$("$(bin_of target two)")" = 6 ]; then + pass "3 CHANGE: two workspace members each have their own module boost" +else fail "3 CHANGE: two workspace members, one module name" s3.log; fi + +# ── 4. index packages build and run with the release ─────────────────────── +rm -rf "$W/s4"; mkdir -p "$W/s4/src"; cd "$W/s4" +printf '[package]\nname = "eco"\nversion = "0.1.0"\n\n[dependencies]\n"compat.zlib" = "*"\n"mcpplibs.cmdline" = "*"\n\n[targets.eco]\nkind = "bin"\nmain = "src/main.cpp"\n' > mcpp.toml +cat > src/main.cpp <<'EOF' +#include +#include +import mcpplibs.cmdline; +int heavy(); +int main() { std::printf("zlib %s\n", zlibVersion()); return heavy() == 0; } +EOF +# One unit that takes a few seconds to compile, so that section 5's build +# outlives the half second before the status row is first drawn. +cat > src/heavy.cpp <<'EOF' +#include +#include +#include +int heavy() { + std::regex r("([a-z]+)-([0-9]+)"); + std::smatch m; + std::string s = std::format("{}-{}", "mcpp", 2026); + return std::regex_match(s, m, r) ? static_cast(m.size()) : 0; +} +EOF +if "$MCPP" build > s4.log 2>&1 && "$(bin_of target eco)" | grep -q '^zlib '; then + pass "4 compat.zlib and mcpplibs.cmdline from the index build and run" +else fail "4 index packages" s4.log; fi + +# ── 5. CHANGE: the status row counts the work of the build ───────────────── +# A pty makes the status row appear; the project of section 4 has a +# dependency the global cache serves after its first build. +cd "$W/s4" +if command -v script > /dev/null 2>&1 && [ -f s4.log ] && grep -q 'Finished' s4.log; then + "$MCPP" clean > /dev/null 2>&1 + MCPP_PROGRESS=plain script -qefc "stty cols 160 rows 40; $MCPP build" s5.pty > /dev/null 2>&1 + rows=$(sed 's/\x1b\[[0-9;?]*[A-Za-z]//g' s5.pty | tr '\r' '\n') + # The units the cache placed, from the `Cached ... (N units)` lines; the + # Building total must not include them (2026.9.30.1 counted each one). + placed=$(echo "$rows" | grep -a '^ *Cached ' | grep -ao '[0-9]* units\?)' | grep -o '^[0-9]*' \ + | awk '{s += $1} END {print s + 0}') + building=$(echo "$rows" | grep -ao 'Building [0-9]*/[0-9]*' | head -1) + total=${building##*/} + if [ -n "$total" ] && [ "$placed" -gt 0 ] && [ "$total" -lt "$placed" ]; then + pass "5 CHANGE: Building counts $total steps, not the $placed units placed from the cache" + else fail "5 CHANGE: the status row (first Building: '$building', units placed: $placed)" s5.pty; fi +else skip "5 no script(1) or section 4 did not build"; fi + +# ── 6. CHANGE: #744, the vendored xlings comes from the newest source ────── +rm -rf "$W/s6"; mkdir -p "$W/s6/rel/bin" "$W/s6/rel/registry/bin" "$W/s6/pathbin"; cd "$W/s6" +NINJA=$(ls "$HOME"/.mcpp/registry/data/xpkgs/xim-x-ninja/*/ninja 2>/dev/null | head -1) +REAL="$HOME/.mcpp/registry/bin/xlings" +if [ -n "$NINJA" ] && [ -x "$REAL" ]; then + older=$("$NINJA" --version | head -1) + export_home="$W/s6/home" + cp "$MCPP" rel/bin/mcpp + cp "$NINJA" rel/registry/bin/xlings + cp "$REAL" pathbin/xlings + MCPP_HOME="$export_home" MCPP_OFFLINE=1 rel/bin/mcpp self env > setup.log 2>&1 || true + mkdir -p "$export_home/registry/bin"; rm -f "$export_home/registry/bin/xlings"; cp "$NINJA" "$export_home/registry/bin/xlings" + env -u MCPP_VENDORED_XLINGS MCPP_HOME="$export_home" MCPP_OFFLINE=1 PATH="$W/s6/pathbin:/usr/bin:/bin" \ + rel/bin/mcpp self env > s6.out 2> s6.err || true + if grep -q "vendored xlings $older -> .* from PATH" s6.err; then + pass "6 CHANGE: a newer xlings on PATH replaced the vendored one past an older released copy" + else fail "6 CHANGE: #744" s6.err; fi +else skip "6 no ninja payload or vendored xlings to stand in"; fi + +# ── 7. planning states its phases ────────────────────────────────────────── +cd "$W/s4" && touch src/main.cpp +if MCPP_VERBOSE=1 "$MCPP" build > s7.log 2>&1 && grep -q 'build/stage: plan scan' s7.log; then + pass "7 CHANGE: planning states its phases under build/stage" +else fail "7 CHANGE: the planning phase timers" s7.log; fi + +echo +echo "RESULT: $passes passed, $fails failed, $skips skipped (mcpp $VER)" +[ "$fails" -eq 0 ] From d0c1b6e8f453bb36598b7e64ec1eecf1c6525264 Mon Sep 17 00:00:00 2001 From: speak-agent <248744407+speak-agent@users.noreply.github.com> Date: Wed, 30 Sep 2026 13:23:40 +0800 Subject: [PATCH 11/15] docs: the verification builds xlings from its main branch The ninth section builds the largest consumer of mcpp, xlings, from its main branch with the release under test and runs the program. Locally against the branch: 9 passed. --- .agents/docs/2026-09-30-build-wall-time-verify.sh | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/.agents/docs/2026-09-30-build-wall-time-verify.sh b/.agents/docs/2026-09-30-build-wall-time-verify.sh index 6ba4d821..585eb0c3 100644 --- a/.agents/docs/2026-09-30-build-wall-time-verify.sh +++ b/.agents/docs/2026-09-30-build-wall-time-verify.sh @@ -146,6 +146,15 @@ if MCPP_VERBOSE=1 "$MCPP" build > s7.log 2>&1 && grep -q 'build/stage: plan scan pass "7 CHANGE: planning states its phases under build/stage" else fail "7 CHANGE: the planning phase timers" s7.log; fi +# ── 8. the largest consumer: xlings builds from its main branch ──────────── +rm -rf "$W/s8"; mkdir -p "$W/s8"; cd "$W/s8" +if git clone -q --depth 1 https://github.com/openxlings/xlings.git xlings > s8-clone.log 2>&1; then + cd xlings + if "$MCPP" build > ../s8.log 2>&1 && "$(bin_of target xlings)" --version 2>/dev/null | grep -q '^xlings '; then + pass "8 xlings builds from its main branch and runs: $(grep -a 'Finished' ../s8.log | tail -1 | sed 's/\x1b\[[0-9;]*m//g; s/^ *//')" + else fail "8 xlings from its main branch" ../s8.log; fi +else skip "8 xlings could not be cloned"; fi + echo echo "RESULT: $passes passed, $fails failed, $skips skipped (mcpp $VER)" [ "$fails" -eq 0 ] From 101f6b8dd3b2a8f9fa2486abe8be206564bf98d5 Mon Sep 17 00:00:00 2001 From: speak-agent <248744407+speak-agent@users.noreply.github.com> Date: Wed, 30 Sep 2026 13:30:31 +0800 Subject: [PATCH 12/15] review fixes: the tail prefilter, the scan pass's options, the xlings upgrade, the GCC map - A `**/` also matches no directory, so the literal tail of `**/name` does not start with its slash; a root-level match of `**/main.cpp` was dropped. - Under -k the scans run in the main pass; the scan pass takes MCPP_NINJA_DEBUG, and -j precedes its goal. - The versions of the vendored xlings's candidate sources are read through the memo, and an upgrade copies beside the binary and renames over it, so a failed copy keeps the old binary. - The GCC module map also lists a name a unit imports that no unit provides, at GCC's own path; the clang and MSVC paths use native separators. --- ...-wall-time-progress-count-and-hang-plan.md | 17 ++++++++ src/build/ninja_backend.cppm | 13 ++++++- src/build/plan.cppm | 25 +++++++++--- src/fallback/xlings_binary.cppm | 39 ++++++++++++------- src/modgraph/scanner.cppm | 9 ++++- tests/unit/test_modgraph.cpp | 16 ++++++++ 6 files changed, 96 insertions(+), 23 deletions(-) diff --git a/.agents/docs/2026-09-30-build-wall-time-progress-count-and-hang-plan.md b/.agents/docs/2026-09-30-build-wall-time-progress-count-and-hang-plan.md index f7a12ea6..3eb0133f 100644 --- a/.agents/docs/2026-09-30-build-wall-time-progress-count-and-hang-plan.md +++ b/.agents/docs/2026-09-30-build-wall-time-progress-count-and-hang-plan.md @@ -972,6 +972,23 @@ without a new staleness class. - Revert probes: the W1 property test, e2e 846 and e2e 847 each fail on 2026.9.30.1. +### 10.5 Review of the implementation + +An independent review of the pull request's diff found the following, each +resolved as stated. + +| Finding | Resolution | +|---|---| +| W8: the literal-tail prefilter dropped a root-level match of `**/name`, since `**/` also matches no directory | The `/` after a `**` is not part of the tail; `Scanner.ADoubleStarSlashTailMatchesAtTheRoot` | +| W2: the scan pass took neither `-k` nor `MCPP_NINJA_DEBUG`, and put `-j` after its goal | Under `-k` the scans run in the main pass; `-d` is passed; `-j` precedes the goal | +| W3: the candidates' versions were asked without the memo on the path where the vendored binary is behind the pin | They are read through the memo | +| W3: a failed copy during an upgrade removed the working binary | The copy is written beside the binary and renamed over it; a failure keeps the old one | +| W10: the GCC map listed only names a unit provides | A name a unit of the package imports that no unit provides is listed at GCC's own path | +| W10: the clang and MSVC paths mixed separators | Native separators | +| W10: the GCC map's relative path in the compile database | Not a defect: the database's directory is the build directory (`compile_commands.cppm:484`) | +| W2: the phase is one value for the whole command, so two configurations built at once show the phase of the pass that began last | Recorded; cosmetic | +| W8: a file symlink whose target is deleted during planning stays listed | Recorded; the per-pattern walk had the same window between its walk and its use | + ## 8. Appendix: readings - **Clean builds, wall time (s).** diff --git a/src/build/ninja_backend.cppm b/src/build/ninja_backend.cppm index a2a2a5b4..16f6c400 100644 --- a/src/build/ninja_backend.cppm +++ b/src/build/ninja_backend.cppm @@ -4338,16 +4338,25 @@ std::expected NinjaBackend::build(const BuildPlan& plan std::string out; int ninjaExit = 0; bool scanFailed = false; - if (goalArg.empty() + // Under `-k` the build goes on past a failure and states every one, so + // the scans run in the main pass there: a failed scan pass would end the + // build at its first failure, and running both would state a failed scan + // twice. + if (goalArg.empty() && !opts.keepGoing && manifest.find("\nbuild " + std::string(kScannedGoal) + " : phony") != std::string::npos) { const auto scanDeadline = std::chrono::milliseconds(static_cast(opts.buildTimeoutSecs) * 1000); + // The main pass's options, and its goal last. std::vector scan{ninjaProgram}; if (!opts.verbose && !opts.progress) scan.push_back("--quiet"); scan.insert(scan.end(), {std::string("-C"), plan.outputDir.string()}); if (opts.verbose) scan.push_back("-v"); - scan.push_back(std::string(kScannedGoal)); + if (const char* topics = std::getenv("MCPP_NINJA_DEBUG"); topics && *topics) { + scan.push_back("-d"); + scan.push_back(topics); + } if (opts.parallelJobs) scan.push_back(std::format("-j{}", opts.parallelJobs)); + scan.push_back(std::string(kScannedGoal)); if (opts.progress) { auto run = run_ninja_reporting(scan, nenv, scanDeadline, *opts.progress, opts.verbose, command_prefixes(flags, plan), diff --git a/src/build/plan.cppm b/src/build/plan.cppm index a7c5c0bf..be1001d4 100644 --- a/src/build/plan.cppm +++ b/src/build/plan.cppm @@ -3224,12 +3224,23 @@ make_plan(const mcpp::manifest::Manifest& manifest, if (collided.contains(name)) bound.emplace_back(name, path); } if (bound.empty()) continue; // sees no collided name - // GCC's mapper answers only what it lists: the standard - // library modules are listed where GCC's own mapper puts them. + // GCC's mapper answers only what it lists. The standard + // library modules, and any name a unit of the package imports + // that no unit of the graph provides (a BMI placed by other + // means), are listed where GCC's own mapper puts them. if (gcc) { - entries.emplace_back("std", std::string(traits.bmiDir) + "/" + basename("std")); - entries.emplace_back("std.compat", - std::string(traits.bmiDir) + "/" + basename("std.compat")); + std::set> listed; + for (auto const& [name, path] : entries) listed.insert(name); + auto flat = [&](const std::string& name) { + if (listed.insert(name).second) + entries.emplace_back(name, std::string(traits.bmiDir) + "/" + basename(name)); + }; + flat("std"); + flat("std.compat"); + for (auto const& cu : plan.compileUnits) + if (cu.packageName == pkg) + for (auto const& imp : cu.imports) + if (!graph.providersOf.contains(imp)) flat(imp); } std::sort(entries.begin(), entries.end()); ModuleScope scope; @@ -3251,7 +3262,9 @@ make_plan(const mcpp::manifest::Manifest& manifest, const bool separate = !prefix.empty() && prefix.back() == ' '; if (separate) prefix.remove_suffix(1); for (auto const& [name, path] : bound) { - const auto value = name + "=" + (outputDir / path).string(); + auto bmi = outputDir / std::filesystem::path(path); + bmi.make_preferred(); + const auto value = name + "=" + bmi.string(); if (separate) { flags.push_back(std::string(prefix)); flags.push_back(mcpp::manifest::flag_element(value)); diff --git a/src/fallback/xlings_binary.cppm b/src/fallback/xlings_binary.cppm index 2e64e8dd..d17d6479 100644 --- a/src/fallback/xlings_binary.cppm +++ b/src/fallback/xlings_binary.cppm @@ -56,8 +56,10 @@ struct XlingsSource { std::optional choose_xlings_source(std::optional override_, std::optional released, std::optional onPath); -// The same choice over this process's sources. -std::optional select_xlings_source(const std::filesystem::path& destBin = {}); +// The same choice over this process's sources. Their versions are read +// through `versionMemo` (known_xlings_version). +std::optional select_xlings_source(const std::filesystem::path& destBin = {}, + const std::filesystem::path& versionMemo = {}); // The version `bin` answers, asked at most once per process. With `memoFile`, // the answer is also kept across processes, keyed by the binary's path, size @@ -120,7 +122,7 @@ acquire_xlings_binary(const std::filesystem::path& destBin, bool quiet = false, // re-acquired, which replaced 2026.8.2.1 with the system's 0.4.51 -- // older still, and equally missing the feature the check exists to // restore. Look before leaping. - auto candidate = select_xlings_source(destBin); + auto candidate = select_xlings_source(destBin, versionMemo); if (!candidate || candidate->version.empty() || !version_is_older(have, candidate->version)) { // stderr, not stdout. This is a remark about the environment, @@ -138,15 +140,21 @@ acquire_xlings_binary(const std::filesystem::path& destBin, bool quiet = false, settled.insert(destBin); return destBin; } + // Beside the binary first and renamed over it, so a copy that fails + // (a full disk, a running binary) leaves the old one in place. std::error_code rec; - std::filesystem::copy_file(candidate->path, destBin, + auto staged = destBin; + staged += ".new"; + std::filesystem::copy_file(candidate->path, staged, std::filesystem::copy_options::overwrite_existing, rec); - if (!rec) { - std::filesystem::permissions(destBin, + if (!rec) + std::filesystem::permissions(staged, std::filesystem::perms::owner_exec | std::filesystem::perms::group_exec | std::filesystem::perms::others_exec, std::filesystem::perm_options::add, rec); + if (!rec) std::filesystem::rename(staged, destBin, rec); + if (!rec) { if (!quiet && !updated) std::println(stderr, "{:>12} vendored xlings {} -> {} from {} (pinned {})", @@ -158,8 +166,11 @@ acquire_xlings_binary(const std::filesystem::path& destBin, bool quiet = false, settled.insert(destBin); return destBin; } - // The copy failed; the chain below tries again from the start. - std::filesystem::remove(destBin, rec); + // The copy failed: the old binary stays, as it would with no source. + std::error_code sec; + std::filesystem::remove(staged, sec); + settled.insert(destBin); + return destBin; } std::error_code ec; @@ -177,7 +188,7 @@ acquire_xlings_binary(const std::filesystem::path& destBin, bool quiet = false, // The first acquisition takes the source a replacement would take // (select_xlings_source): the override, otherwise the newer of the // released copy and the PATH copy. - if (auto src = select_xlings_source(destBin)) { + if (auto src = select_xlings_source(destBin, versionMemo)) { std::filesystem::copy_file(src->path, destBin, std::filesystem::copy_options::overwrite_existing, ec); if (!ec) { @@ -290,22 +301,24 @@ std::optional choose_xlings_source(std::optional ove return released; } -std::optional select_xlings_source(const std::filesystem::path& destBin) { +std::optional select_xlings_source(const std::filesystem::path& destBin, + const std::filesystem::path& versionMemo) { std::optional override_, released, onPath; std::error_code ec; if (const char* e = std::getenv("MCPP_VENDORED_XLINGS"); e && *e) { std::filesystem::path p{e}; if (std::filesystem::exists(p, ec)) - override_ = XlingsSource{p, vendored_xlings_version(p), "MCPP_VENDORED_XLINGS"}; + override_ = XlingsSource{p, known_xlings_version(p, versionMemo), "MCPP_VENDORED_XLINGS"}; } if (!override_) { if (auto r = released_xlings_source(destBin); !r.empty()) - released = XlingsSource{r, vendored_xlings_version(r), "the release of this mcpp"}; + released = XlingsSource{r, known_xlings_version(r, versionMemo), + "the release of this mcpp"}; if (auto sys = mcpp::platform::fs::which( std::string("xlings") + std::string(mcpp::platform::exe_suffix))) { const bool isDest = !destBin.empty() && std::filesystem::equivalent(*sys, destBin, ec); if (!isDest && std::filesystem::exists(*sys, ec)) - onPath = XlingsSource{*sys, vendored_xlings_version(*sys), "PATH"}; + onPath = XlingsSource{*sys, known_xlings_version(*sys, versionMemo), "PATH"}; } } return choose_xlings_source(std::move(override_), std::move(released), std::move(onPath)); diff --git a/src/modgraph/scanner.cppm b/src/modgraph/scanner.cppm index 3e4950d7..275f6863 100644 --- a/src/modgraph/scanner.cppm +++ b/src/modgraph/scanner.cppm @@ -692,9 +692,14 @@ std::vector expand_glob_one(const std::filesystem::path& if (!fs::exists(start, startEc)) return out; // Every match ends with the text after the glob's last `*`, which the - // matcher reads literally: a cheap test that turns most entries away. + // matcher reads literally: a cheap test that turns most entries away. A + // `**/` also matches no directory at all (`**/main.cpp` matches a root + // `main.cpp`), so the `/` after a `**` is not part of the tail. const auto star = glob.find_last_of('*'); - const std::string_view tail = star == std::string_view::npos ? glob : glob.substr(star + 1); + std::string_view tail = star == std::string_view::npos ? glob : glob.substr(star + 1); + if (star != std::string_view::npos && star > 0 && glob[star - 1] == '*' + && tail.starts_with('/')) + tail.remove_prefix(1); auto listing = tree_listing(root, start); for (auto const& f : listing->files) { if (!f.relative) continue; diff --git a/tests/unit/test_modgraph.cpp b/tests/unit/test_modgraph.cpp index 5e3e6516..252e36a5 100644 --- a/tests/unit/test_modgraph.cpp +++ b/tests/unit/test_modgraph.cpp @@ -1433,3 +1433,19 @@ TEST(ModuleResolution, OneFileReachedTwiceIsNamedAsSuchWhateverItsSpelling) { EXPECT_NE(errors.find("one file is reached as two packages"), std::string::npos) << errors; std::filesystem::remove_all(dir); } + +// A kept walk matched by the literal tail of a glob: `**/` also matches no +// directory, so `**/main.cpp` matches a `main.cpp` at the root as well as one +// below it, as the per-pattern walk did. +TEST(Scanner, ADoubleStarSlashTailMatchesAtTheRoot) { + auto dir = make_tempdir("mcpp-glob-tail"); + write(dir / "main.cpp", "int main() {}\n"); + write(dir / "sub" / "main.cpp", "int main() {}\n"); + write(dir / "sub" / "other.cpp", "int x;\n"); + auto files = expand_glob(dir, "**/main.cpp"); + ASSERT_EQ(files.size(), 2u); + EXPECT_EQ(files[0], dir / "main.cpp"); + EXPECT_EQ(files[1], dir / "sub" / "main.cpp"); + std::filesystem::remove_all(dir); +} + From 77c1f5f939ba615cf7ff285b5deeab82c053cf73 Mon Sep 17 00:00:00 2001 From: speak-agent <248744407+speak-agent@users.noreply.github.com> Date: Wed, 30 Sep 2026 13:32:19 +0800 Subject: [PATCH 13/15] docs(spec): SPEC-005 v1.7, R3.8a: two providers of one module name in one configuration When two packages of one configuration provide a module name, both units list it, and a unit that may import it carries the binding the build uses in its arguments; a consumer that looks providers up by name may take the other program's module. With one provider per name the document is unchanged (#732). --- docs/specs/build-database.md | 18 +++++++++++++----- 1 file changed, 13 insertions(+), 5 deletions(-) diff --git a/docs/specs/build-database.md b/docs/specs/build-database.md index d6234f24..dc8568fe 100644 --- a/docs/specs/build-database.md +++ b/docs/specs/build-database.md @@ -4,12 +4,12 @@ |---|---| | 规范编号 | SPEC-005 | | 标题 | mcpp 输出的构建数据库:内容、取值规则与不写工程目录的保证 | -| 状态 | 评审中 v1.6 | -| 版本 | 1.6 | -| 最后修改 | 2026-09-29 | -| 对应实现 | mcpp >= 2026.9.15.1;v1.3 修改的 R2.5、R3.7、R3.8、R4.1、R5.2 为 mcpp >= 2026.9.26.2;v1.4 修改的 R2.5 为 mcpp >= 2026.9.27.1;v1.5 修改的 R3.7、R3.12、R5.1、R5.2 为 mcpp >= 2026.9.28.1;v1.6 修改的 R2.1、R3.3、R3.4、R3.5、R4.1、R5.2 为 mcpp >= 2026.9.29.5 | +| 状态 | 评审中 v1.7 | +| 版本 | 1.7 | +| 最后修改 | 2026-09-30 | +| 对应实现 | mcpp >= 2026.9.15.1;v1.3 修改的 R2.5、R3.7、R3.8、R4.1、R5.2 为 mcpp >= 2026.9.26.2;v1.4 修改的 R2.5 为 mcpp >= 2026.9.27.1;v1.5 修改的 R3.7、R3.12、R5.1、R5.2 为 mcpp >= 2026.9.28.1;v1.6 修改的 R2.1、R3.3、R3.4、R3.5、R4.1、R5.2 为 mcpp >= 2026.9.29.5;v1.7 新增的 R3.8a 为 mcpp >= 2026.9.30.2 | | 相关设计文档 | `.agents/docs/2026-09-14-636-build-database-and-the-latest-xlings.md`
`.agents/docs/2026-09-26-compile-database-and-issue-699-design.md` | -| 相关 issue | #636, #648, #655, #699, #702, #707 | +| 相关 issue | #636, #648, #655, #699, #702, #707, #732 | | 依据的外部规范 | S1「C++ Build Database: IDE Profile」profile 0.3.0(§7.2 的 `generated`,Sunrisepeak/mcpp-language-server#28;此前为 0.2.0)与 S2 0.2.0 §3.4,取自 https://github.com/Sunrisepeak/lsp-mcpp-private 提交 `b82859d`(schema 自提交 `28ecd6e` 起未变);S2 0.3.0 §3.4 的部分回答(S2-3.4-12、S2-3.4-13,Sunrisepeak/mcpp-language-server#25);JSON Compilation Database | ## 0. 适用范围 @@ -126,6 +126,13 @@ mcpp 输出的 S1 文档满足 S1 等级 2,不输出 `ide.options`。等级 3 真的编译就能得到(§3.4)。`ide.toolchains..build-id` 给出编译器的构建标识, 取自 mcpp 已经算出的驱动身份(工具链指纹的同一个字段),同一工具链的两次运行 之间保持稳定。**已实现** +- **R3.8a** 一个模块名在一个程序之内标识一个模块,而一个配置可以包含多个程序。同一 + 配置中两个包提供同一个模块名时(两者不在同一个包的闭包中,#732),两个单元的 + `provides` 都列出这个名字;可能导入它的每个单元,其 `arguments` 带有构建所用的 + 绑定:GCC 为 `-fmodule-mapper=<映射文件>`(相对 `work-directory`),Clang 为 + `-fmodule-file=<名字>=<路径>`,MSVC 为 `/reference <名字>=<路径>`。只按名字在文档中 + 查找提供方的消费方,因此可能取到另一个程序的模块;构建本身不受影响。每个名字只有 + 一个提供方时,文档与此前逐字相同。**已实现** - **R3.9** `ide.role` 取自扫描器读到的模块声明形式: | 声明 | `ide.role` | @@ -233,3 +240,4 @@ mcpp 输出的 S1 文档满足 S1 等级 2,不输出 `ide.options`。等级 3 | 1.4 | 2026-09-26 | R2.5:命令不构建宿主工具;工具库中没有的工具被推迟,输出说明 `MCPP_BUILD_DATABASE_HOST_TOOL_DEFERRED`,取代 1.3 的警告 `MCPP_BUILD_DATABASE_HOST_TOOL_UNBUILT`(#707)。 | | 1.5 | 2026-09-28 | R3.7:规则声明的设备源不是编译单元,不进入 S1 与 `compile_commands.json`(#724)。新增 R3.12:集合的 `ide.generated` 列出规则生成的文件与目录,给出构建写入的路径与生成它的步骤,S1 0.3.0(#724,Sunrisepeak/mcpp-language-server#28)。R5.1:S1 版本为 0.3.0。R5.2:以构建程序的指令为前提的检查不对其构建程序已失败的包运行,失败路径保留已记录的说明(#724)。 | | 1.6 | 2026-09-29 | 工作区按配置规划,与 `mcpp build` 相同(R2.1、R3.3、R3.4、R5.2):成员共用的包在一个配置中只描述一次;集合名的前缀由 `<成员>/` 改为只在文档描述多个配置时出现的 `<配置>/`;一个配置的规划失败时逐成员规划。R3.5:被选成员的集合按其目标给出 `ide.kind`。R4.1:同一文件与输出一条条目。 | +| 1.7 | 2026-09-30 | 新增 R3.8a:同一配置中两个包提供同一个模块名时,两个单元都列出它,可能导入它的单元的 `arguments` 带有构建所用的绑定(#732)。 | From 9b8cdd7fa6b89e54b6f975ca0d466a5b8426708d Mon Sep 17 00:00:00 2001 From: speak-agent <248744407+speak-agent@users.noreply.github.com> Date: Wed, 30 Sep 2026 13:44:54 +0800 Subject: [PATCH 14/15] build: the planning timers go to the log file, not the terminal Under --verbose the thirty planning lines reached the terminal, and e2e 198 on the Linux-to-Windows leg read one of them, `plan finish windows resources`, as a non-PE build speaking about resources. The planning phases and the finish phase's steps are recorded with log::info, in the log file that --verbose or MCPP_LOG_LEVEL=info enables; a record to read afterwards is not output of the build. The verification reads them from the file. --- ...d-wall-time-progress-count-and-hang-plan.md | 9 +++++++-- .../docs/2026-09-30-build-wall-time-verify.sh | 5 ++++- CHANGELOG.md | 5 +++-- src/build/prepare/driver.cpp | 18 ++++++++++-------- src/build/prepare/plan.cpp | 8 ++++---- 5 files changed, 28 insertions(+), 17 deletions(-) diff --git a/.agents/docs/2026-09-30-build-wall-time-progress-count-and-hang-plan.md b/.agents/docs/2026-09-30-build-wall-time-progress-count-and-hang-plan.md index 3eb0133f..5ca0c1f2 100644 --- a/.agents/docs/2026-09-30-build-wall-time-progress-count-and-hang-plan.md +++ b/.agents/docs/2026-09-30-build-wall-time-progress-count-and-hang-plan.md @@ -897,8 +897,13 @@ stated as openxlings/xlings#638; W3 removes mcpp's dependence on it. ### 10.1 What was built - **W9.** Every phase of `prepare_build`, and every step of its last phase, - logs `plan : ` under `build/stage` when the log is at info level - or `--verbose` is on; the backend's own stage lines follow the same gate. + logs `plan : ` under `build/stage` in the log file, which + `--verbose` or `MCPP_LOG_LEVEL=info` enables; the backend's own stage lines + are also recorded in the file at info level. The planning lines went to the + terminal under `--verbose` at first, and e2e 198 failed on the Linux-to- + Windows leg: its check that a non-PE build says nothing about resources read + `plan finish windows resources: 0ms`. A record of thirty lines a build + belongs in the file, where no check of what a build says reads it. - **W1.** `landing()` answers nothing for a piece that would rest outside the screen, `spawn()` answers nothing when no candidate lands inside, and both fill loops end when a spawn fails or a locked piece adds no cell. The diff --git a/.agents/docs/2026-09-30-build-wall-time-verify.sh b/.agents/docs/2026-09-30-build-wall-time-verify.sh index 585eb0c3..7a8b709d 100644 --- a/.agents/docs/2026-09-30-build-wall-time-verify.sh +++ b/.agents/docs/2026-09-30-build-wall-time-verify.sh @@ -142,7 +142,10 @@ else skip "6 no ninja payload or vendored xlings to stand in"; fi # ── 7. planning states its phases ────────────────────────────────────────── cd "$W/s4" && touch src/main.cpp -if MCPP_VERBOSE=1 "$MCPP" build > s7.log 2>&1 && grep -q 'build/stage: plan scan' s7.log; then +LOGFILE="$HOME/.mcpp/log/mcpp.log" +before=$(wc -c < "$LOGFILE" 2>/dev/null || echo 0) +if MCPP_LOG_LEVEL=info "$MCPP" build > s7.log 2>&1 \ + && tail -c +$((before + 1)) "$LOGFILE" 2>/dev/null | grep -q 'build/stage: plan scan'; then pass "7 CHANGE: planning states its phases under build/stage" else fail "7 CHANGE: the planning phase timers" s7.log; fi diff --git a/CHANGELOG.md b/CHANGELOG.md index 82fe3846..b85a31a5 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -61,8 +61,9 @@ of 10.5 s; a build with nothing to do is unchanged at 0.05 s. modification time, so a command that loads its configuration no longer runs `xlings --version` (0.35 s) when the binary has not changed. - **Planning states where its time goes.** Each phase of planning, and each - step of its last phase, logs its duration under `build/stage` whenever the - log file or `--verbose` would show it. + step of its last phase, logs its duration under `build/stage` in the log + file, which `--verbose` or `MCPP_LOG_LEVEL=info` enables. The backend's own + stage lines are recorded in the file under the same condition. ## [2026.9.30.1] - 2026-09-30 diff --git a/src/build/prepare/driver.cpp b/src/build/prepare/driver.cpp index 90d56468..20b56e05 100644 --- a/src/build/prepare/driver.cpp +++ b/src/build/prepare/driver.cpp @@ -61,19 +61,21 @@ prepare_build(bool print_fingerprint, }; // WHERE PLANNING'S TIME GOES: each phase states its duration under - // `build/stage`, as the backend's own steps do, whenever the log file or - // --verbose would show it. Planning had no such record, and a planned - // edit of one source spent 3.05 s before ninja that could only be - // attributed from gaps between unrelated log lines (.agents/docs/ - // 2026-09-30-build-wall-time-progress-count-and-hang-plan.md, W9). + // `build/stage` in the log file, which --verbose or MCPP_LOG_LEVEL=info + // enables. Planning had no such record, and a planned edit of one source + // spent 3.05 s before ninja that could only be attributed from gaps + // between unrelated log lines (.agents/docs/ + // 2026-09-30-build-wall-time-progress-count-and-hang-plan.md, W9). The + // file and not the terminal: thirty lines a build are a record to read + // afterwards, not output, and their words would reach every check that + // reads what a verbose build says. auto timed = [&](std::string_view phase, auto&& run) { - if (!mcpp::log::is_verbose() && !mcpp::log::is_enabled(mcpp::log::Level::info)) - return run(); + if (!mcpp::log::is_enabled(mcpp::log::Level::info)) return run(); const auto t0 = std::chrono::steady_clock::now(); auto r = run(); const auto ms = std::chrono::duration_cast( std::chrono::steady_clock::now() - t0).count(); - mcpp::log::verbose("build/stage", std::format("plan {}: {}ms", phase, ms)); + mcpp::log::info("build/stage", std::format("plan {}: {}ms", phase, ms)); return r; }; diff --git a/src/build/prepare/plan.cpp b/src/build/prepare/plan.cpp index a442b616..13bcc360 100644 --- a/src/build/prepare/plan.cpp +++ b/src/build/prepare/plan.cpp @@ -2273,14 +2273,14 @@ std::expected phase13_finish(PrepareState& state) { ctx.projectRoot= *state.root; ctx.outputDir = target_dir(*state.tc, state.fp, state.workRoot); - // Each step states its duration under `build/stage`, as the phases do - // (build wall-time plan, W9), when the log file or --verbose would show it. - const bool timing = mcpp::log::is_verbose() || mcpp::log::is_enabled(mcpp::log::Level::info); + // Each step states its duration under `build/stage` in the log file, as + // the phases do (build wall-time plan, W9; see prepare_build). + const bool timing = mcpp::log::is_enabled(mcpp::log::Level::info); auto timed = [&](std::string_view step, auto&& run) { if (!timing) return run(); const auto t0 = std::chrono::steady_clock::now(); auto r = run(); - mcpp::log::verbose("build/stage", std::format("plan finish {}: {}ms", step, + mcpp::log::info("build/stage", std::format("plan finish {}: {}ms", step, std::chrono::duration_cast( std::chrono::steady_clock::now() - t0).count())); return r; From ac4beb8867791d9b73f26f6e2d7f147d5717e4dc Mon Sep 17 00:00:00 2001 From: speak-agent <248744407+speak-agent@users.noreply.github.com> Date: Wed, 30 Sep 2026 14:32:57 +0800 Subject: [PATCH 15/15] build: a staged BMI waits for the modules it imports that compile here A package's cache entry holds the units below its root. A module its build program generates lies below the consumer's target directory and compiles in every build, also when the rest of the package is staged (xpkg's lua_stdlib, imported by its cached executor). The consumer's dyndep named the staged BMI only and the stage edge had no input but the cache entry, so a fresh build could compile the consumer first: the aarch64-linux-musl cross build of xlings failed with `failed to read compiled module`. The stage edge of such a BMI now waits for the BMIs it imports that compile here, and leaves the aggregate every compile waits for, which would otherwise form a cycle (NinjaBackend.AStagedBmiWaitsForTheModulesItImportsThatCompileHere, e2e 849). e2e 846: on Windows the test's PATH holds System32, where `where` lies. --- ...-wall-time-progress-count-and-hang-plan.md | 25 +++ CHANGELOG.md | 8 + src/build/ninja_backend.cppm | 44 ++++- ...ings_is_replaced_from_the_newest_source.sh | 3 + ...ed_bmi_waits_for_a_module_compiled_here.sh | 157 ++++++++++++++++++ tests/unit/test_ninja_backend.cpp | 102 ++++++++++++ 6 files changed, 334 insertions(+), 5 deletions(-) create mode 100755 tests/e2e/849_a_staged_bmi_waits_for_a_module_compiled_here.sh diff --git a/.agents/docs/2026-09-30-build-wall-time-progress-count-and-hang-plan.md b/.agents/docs/2026-09-30-build-wall-time-progress-count-and-hang-plan.md index 5ca0c1f2..eb190c1c 100644 --- a/.agents/docs/2026-09-30-build-wall-time-progress-count-and-hang-plan.md +++ b/.agents/docs/2026-09-30-build-wall-time-progress-count-and-hang-plan.md @@ -994,6 +994,31 @@ resolved as stated. | W2: the phase is one value for the whole command, so two configurations built at once show the phase of the pass that began last | Recorded; cosmetic | | W8: a file symlink whose target is deleted during planning stays listed | Recorded; the per-pattern walk had the same window between its walk and its use | +### 10.6 Found by CI + +- **A staged BMI that imports a module compiled here.** The aarch64-linux-musl + cross build of xlings failed with `mcpplibs.xpkg.lua_stdlib: failed to read + compiled module: No such file or directory`. xpkg's build program generates + `lua_stdlib` below the consumer's target directory, so the package's cache + entry holds its other five units, and `lua_stdlib` compiles in every build. + The consumer's dyndep named the staged `executor` BMI only, and its stage + edge had no input but the cache entry: nothing ordered the consumer after + `lua_stdlib`. The defect predates this pull request; it needs a warm cache + and a fresh build directory (after one build, GCC's depfile records the + transitive BMI), and the scan pass changed the schedule so that the consumer + compiled first. The stage edge of such a BMI now waits for the BMIs it + imports that compile here, and leaves the aggregate that every compile waits + for, which would otherwise be a cycle. On xlings, with the generated BMI, + one consumer's object and `.ninja_deps` removed, `ninja` asked for that + object alone reproduces the CI error with the previous binary and builds + with this one; e2e 849 states the same criterion on a fixture and fails on + the previous binary under clang. +- **e2e 846 on Windows.** Leg D found no xlings on the test's `PATH`, because + mcpp finds it with `where`, which lies in System32, and the test's `PATH` + held only `/usr/bin:/bin`. The test's `PATH` now holds System32, as every + Windows `PATH` does, and as the other Windows tests with a restricted `PATH` + do. + ## 8. Appendix: readings - **Clean builds, wall time (s).** diff --git a/CHANGELOG.md b/CHANGELOG.md index b85a31a5..dae8cb7d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -46,6 +46,14 @@ of 10.5 s; a build with nothing to do is unchanged at 0.05 s. reached as two packages is recognised whatever its spelling. When every name has one provider, `build.ninja` and `compile_commands.json` are byte-identical to 2026.9.30.1's (e2e 847, 848). +- **A BMI served from the global cache waits for the modules it imports that + the build compiles.** A module that a package's build program generates lies + below the consumer's target directory and is compiled in every build, also + when the rest of the package is staged from the cache (xpkg's `lua_stdlib`, + imported by its cached `executor`). Nothing ordered a consumer of the staged + BMI after that compile, so a fresh build could compile the consumer first and + fail with `failed to read compiled module`. The stage edge of such a BMI now + waits for those BMIs (e2e 849). ### Changed diff --git a/src/build/ninja_backend.cppm b/src/build/ninja_backend.cppm index 16f6c400..ae772855 100644 --- a/src/build/ninja_backend.cppm +++ b/src/build/ninja_backend.cppm @@ -2459,13 +2459,42 @@ std::string emit_ninja_string(const BuildPlan& plan, std::string* placements, // changed BMI still invalidates its consumers — this adds sequencing, not // dirtiness. The cost is that a handful of copies finish before compilation // starts, which is what used to happen anyway when those units were built. + // + // The other direction. A staged BMI is read together with the BMIs of the + // modules it imports, and one of those can be compiled HERE: a unit of the + // package that its cache entry does not hold, such as a source the + // package's build program writes below the consumer's target directory + // (xpkg's `lua_stdlib`), or a module of a package that is not cached. The + // consumer's dyndep names the staged BMI only, so the stage edge itself + // waits for those BMIs, and a staged BMI that imports such a stage waits + // for it in turn. Such a stage stays out of the aggregate: every compile + // edge waits for the aggregate, and the compile the stage waits for is one + // of them. std::string stagedOrderOnly; { + const auto is_staged = [](const CompileUnit& cu) { + return cu.servedFromCache && !cu.cachedObject.empty(); + }; + // Whether the BMI of `cu` exists only after a compile of this build: + // it is compiled here, or it is staged and imports such a BMI. + std::unordered_map late; + std::function after_a_compile = + [&](const CompileUnit& cu) -> bool { + if (!is_staged(cu)) return true; + if (auto it = late.find(&cu); it != late.end()) return it->second; + late[&cu] = false; // module imports form no cycle + bool waits = false; + for (auto& imp : cu.imports) + if (auto const* p = provider_of(cu, imp); p && after_a_compile(*p)) { + waits = true; + break; + } + return late[&cu] = waits; + }; std::vector staged; for (auto& cu : plan.compileUnits) { attribute(cu.packageName); - if (!cu.servedFromCache) continue; - if (cu.cachedObject.empty()) continue; + if (!is_staged(cu)) continue; auto obj = escape_ninja_path(cu.object); append(std::format("build {} : stage_file {}\n", obj, escape_ninja_path(cu.cachedObject))); @@ -2473,10 +2502,15 @@ std::string emit_ninja_string(const BuildPlan& plan, std::string* placements, staged.push_back(obj); if (!cu.providesModule.empty() && !cu.cachedBmi.empty()) { auto bmi = unit_bmi(cu); - append(std::format("build {} : stage_file {}\n", bmi, - escape_ninja_path(cu.cachedBmi))); + std::string waitsFor; + for (auto& imp : cu.imports) + if (auto const* p = provider_of(cu, imp); p && after_a_compile(*p)) + waitsFor += " " + import_bmi(cu, imp); + append(std::format("build {} : stage_file {}{}\n", bmi, + escape_ninja_path(cu.cachedBmi), + waitsFor.empty() ? "" : " ||" + waitsFor)); append(" verify = --verify size\n"); - staged.push_back(bmi); + if (waitsFor.empty()) staged.push_back(bmi); } } if (!staged.empty()) { diff --git a/tests/e2e/846_the_vendored_xlings_is_replaced_from_the_newest_source.sh b/tests/e2e/846_the_vendored_xlings_is_replaced_from_the_newest_source.sh index 4b7f34df..0099d326 100755 --- a/tests/e2e/846_the_vendored_xlings_is_replaced_from_the_newest_source.sh +++ b/tests/e2e/846_the_vendored_xlings_is_replaced_from_the_newest_source.sh @@ -60,6 +60,9 @@ cp "$MCPP" "$TMP/release/bin/mcpp$EXE" chmod +x "$TMP/release/bin/mcpp$EXE" 2>/dev/null || true BASE_PATH="/usr/bin:/bin" +# On Windows mcpp finds the PATH copy with `where`, which lies in System32, a +# directory every Windows PATH holds. +case "$(uname -s)" in MINGW*|MSYS*|CYGWIN*) BASE_PATH="$BASE_PATH:/c/Windows/System32" ;; esac if PATH="$BASE_PATH" command -v xlings > /dev/null 2>&1; then fail "an xlings is reachable on $BASE_PATH, so the criteria cannot tell the sources apart" fi diff --git a/tests/e2e/849_a_staged_bmi_waits_for_a_module_compiled_here.sh b/tests/e2e/849_a_staged_bmi_waits_for_a_module_compiled_here.sh new file mode 100755 index 00000000..a9c23ad4 --- /dev/null +++ b/tests/e2e/849_a_staged_bmi_waits_for_a_module_compiled_here.sh @@ -0,0 +1,157 @@ +#!/usr/bin/env bash +# requires: +# 849 -- a BMI served from the global cache waits for the modules it imports +# that this build compiles. +# +# A package's cache entry holds the units below its root. A module its build +# program generates lies below the consumer's target directory, so it is +# compiled in every build, also when the rest of the package is staged from +# the cache (xpkg's `lua_stdlib`, imported by its cached `executor`). The +# consumer's dyndep names the staged BMI only, and the stage edge had no input +# but the cache entry, so nothing ordered the consumer after the generated +# module's compile. A fresh build compiled the consumer first whenever the +# schedule allowed it: +# +# mcpplibs.xpkg.lua_stdlib: error: failed to read compiled module: No such file or directory +# mcpplibs.xpkg.executor: error: failed to read compiled module: Bad import dependency +# +# (the aarch64-linux-musl cross build of xlings, once the scan pass of +# 2026.9.30.2 changed the schedule). The stage edge of such a BMI now waits for +# the BMIs it imports that compile here. +# +# Criteria: +# A. The second build of the project stages the package from the cache. +# B. The stage edge of the cached BMI names the generated module's BMI after +# `||`, and the aggregate every compile waits for does not hold it. +# C. The order is carried by the graph and not by the schedule or by a +# depfile of an earlier build: with the generated BMI, the consumer's +# object and the recorded depfiles removed, ninja asked for the +# consumer's object alone builds the generated module first. +set -e +source "$(dirname "$0")/_host_path.sh" + +TMP=$(mktemp -d) +trap 'rm -rf "$TMP"' EXIT +fail() { echo "FAIL: $1"; shift; for f in "$@"; do echo "--- $f ---"; cat "$f" 2>/dev/null; done; exit 1; } + +export MCPP_HOME="$TMP/mcpp-home" +source "$(dirname "$0")/_inherit_toolchain.sh" + +INDEX_DIR="$TMP/local-index" +INDEX_DIR_HOST="$(host_path "$INDEX_DIR")" +mkdir -p "$INDEX_DIR/pkgs/g" +cat > "$INDEX_DIR/pkgs/g/gen-dep.lua" <<'EOF' +package = { + spec = "1", + name = "gen-dep", + description = "A package whose module imports a module its build program generates", + licenses = {"MIT"}, + type = "package", + xpm = { + linux = { + ["1.0.0"] = { + url = "https://example.invalid/gen-dep-1.0.0.tar.gz", + sha256 = "0000000000000000000000000000000000000000000000000000000000000000", + }, + }, + }, +} +EOF + +mkdir -p "$TMP/app/src" +PAYLOAD="$TMP/app/.mcpp/.xlings/data/xpkgs/local-dev.gen-dep/1.0.0" +mkdir -p "$PAYLOAD/src" +cat > "$PAYLOAD/mcpp.toml" <<'EOF' +[package] +name = "gen-dep" +version = "1.0.0" + +[targets.gen-dep] +kind = "lib" +EOF +cat > "$PAYLOAD/build.mcpp" <<'EOF' +#include +#include +import mcpp; +int main() { + std::string p = std::string(mcpp::out_dir()) + "/gen-table.cppm"; + std::FILE* f = std::fopen(p.c_str(), "w"); + std::fputs("export module gen.dep.table;\nexport int table_value() { return 42; }\n", f); + std::fclose(f); + mcpp::generated(p.c_str()); + return 0; +} +EOF +cat > "$PAYLOAD/src/gen.dep.cppm" <<'EOF' +export module gen.dep; +import gen.dep.table; +export int dep_value() { return table_value(); } +EOF +cat > "$TMP/app/src/main.cpp" <<'EOF' +#include +import gen.dep; +int main() { std::printf("%d\n", dep_value()); } +EOF +cat > "$TMP/app/mcpp.toml" < first.log 2>&1 || fail "the first build failed (a fixture problem)" first.log + +# ── A ────────────────────────────────────────────────────────────────────── +"$MCPP" clean > /dev/null 2>&1 +"$MCPP" build > hit.log 2>&1 || fail "A: the build that stages the package failed" hit.log +grep -qE 'Cached local-dev\.gen-dep v1\.0\.0' hit.log \ + || fail "A: the package was not staged from the cache, so nothing below is exercised" hit.log +[ "$(./target/*/*/bin/app | tr -d '\r')" = 42 ] || fail "A: the program does not print 42" hit.log +echo "ok: A, the package is staged from the cache" + +# ── B ────────────────────────────────────────────────────────────────────── +N=$(find target -name build.ninja | head -1) +[ -n "$N" ] || fail "B: no build.ninja" +D=$(dirname "$N") +# The BMI directory and extension are the toolchain's (gcm.cache/*.gcm for +# GCC, pcm.cache/*.pcm for clang); they are read from the stage edge. +stage=$(grep -E '^build [a-z]+\.cache/gen\.dep\.[a-z]+ : stage_file ' "$N" || true) +[ -n "$stage" ] || fail "B: the cached BMI has no stage edge" "$N" +bmi=$(echo "$stage" | awk '{print $2}') +bmidir=${bmi%%/*}; ext=${bmi##*.} +table="$bmidir/gen.dep.table.$ext" +case "$stage" in + *"|| $table"*) ;; + *) fail "B: the stage edge does not wait for the generated module's BMI: $stage" ;; +esac +phony=$(grep -E '^build _mcpp_staged_cache : phony' "$N" || true) +case " $phony " in + *" $bmi "*) fail "B: the aggregate holds a stage that waits for a compile: $phony" ;; +esac +echo "ok: B, the stage edge waits for the generated module" + +# ── C ────────────────────────────────────────────────────────────────────── +NINJA="" +for cand in "$MCPP_HOME"/registry/data/xpkgs/xim-x-ninja/*/ninja \ + "$MCPP_HOME"/registry/data/xpkgs/xim-x-ninja/*/bin/ninja; do + if [ -f "$cand" ]; then NINJA="$cand"; break; fi +done +[ -n "$NINJA" ] || fail "C: no ninja binary" +main_obj=$(grep -E '^build [^ ]*main\.[a-z.]+ : cxx_object ' "$N" | head -1 | awk '{print $2}') +[ -n "$main_obj" ] || fail "C: no compile edge for main.cpp" "$N" +rm -f "$D/.ninja_deps" "$D/$table" "$D/$main_obj" +"$NINJA" -C "$D" "$main_obj" > c.log 2>&1 || fail "C: the consumer compiled before the module it reaches" c.log +[ -f "$D/$table" ] || fail "C: the generated module was not compiled" c.log +echo "ok: C, the graph orders the consumer after the generated module" + +echo "PASS: 849_a_staged_bmi_waits_for_a_module_compiled_here" diff --git a/tests/unit/test_ninja_backend.cpp b/tests/unit/test_ninja_backend.cpp index d41e09b9..20d0e28c 100644 --- a/tests/unit/test_ninja_backend.cpp +++ b/tests/unit/test_ninja_backend.cpp @@ -1456,6 +1456,108 @@ TEST(NinjaBackend, NonCachedEdgesOrderAfterEveryStagedArtifact) { << consumerLine; } +// The other direction of the same ordering. A staged BMI can import a module +// compiled HERE: a unit of the package that the cache entry does not hold, +// such as a source its build program writes below the consumer's target +// directory (xpkg's `lua_stdlib`). The consumer's dyndep names the staged BMI +// only, and the stage edge had no input but the cache entry, so a consumer +// could compile while that module's BMI did not yet exist (`failed to read +// compiled module: No such file or directory`, observed in CI once the scan +// pass changed the schedule). The stage edge now waits for those BMIs, and it +// leaves the aggregate, because every compile edge waits for the aggregate and +// the compile it waits for is one of them. +TEST(NinjaBackend, AStagedBmiWaitsForTheModulesItImportsThatCompileHere) { + auto plan = minimal_plan(); + // The generated unit of the cached package, compiled here. + plan.compileUnits.push_back({ + .source = "target/.build-mcpp/deps/dep/out/gen.cppm", + .kind = mcpp::SourceKind::ModuleInterface, + .object = "obj/gen.m.o", + .packageName = "dep", + .providesModule = "dep.gen", + }); + // The package's primary interface, served from the cache, imports it. + plan.compileUnits.push_back({ + .source = "/store/dep/src/dep.cppm", + .kind = mcpp::SourceKind::ModuleInterface, + .object = "obj/dep.m.o", + .packageName = "dep", + .providesModule = "dep", + .imports = {"dep.gen"}, + .servedFromCache = true, + .cachedObject = "/bc/obj/dep.m.o", + .cachedBmi = "/bc/bmi/dep.gcm", + }); + // A second staged interface that imports the first: it waits as well. + plan.compileUnits.push_back({ + .source = "/store/dep/src/api.cppm", + .kind = mcpp::SourceKind::ModuleInterface, + .object = "obj/api.m.o", + .packageName = "dep", + .providesModule = "dep.api", + .imports = {"dep"}, + .servedFromCache = true, + .cachedObject = "/bc/obj/api.m.o", + .cachedBmi = "/bc/bmi/dep.api.gcm", + }); + // A staged interface whose imports are all staged: unchanged. + plan.compileUnits.push_back({ + .source = "/store/dep/src/util.cppm", + .kind = mcpp::SourceKind::ModuleInterface, + .object = "obj/util.m.o", + .packageName = "dep", + .providesModule = "dep.util", + .servedFromCache = true, + .cachedObject = "/bc/obj/util.m.o", + .cachedBmi = "/bc/bmi/dep.util.gcm", + }); + plan.compileUnits.push_back({ + .source = "src/main.cpp", + .kind = mcpp::SourceKind::Cxx, + .object = "obj/main.o", + .packageName = "objc_rule_test", + .imports = {"dep.api", "dep.util"}, + }); + + auto ninja = emit_ninja_string(plan); + auto line_of = [&](std::string_view head) { + auto at = ninja.find(head); + if (at == std::string::npos) return std::string{}; + return ninja.substr(at, ninja.find('\n', at) - at); + }; + + auto dep = line_of("build gcm.cache/dep.gcm : stage_file"); + ASSERT_FALSE(dep.empty()) << ninja; + EXPECT_NE(dep.find("|| gcm.cache/dep.gen.gcm"), std::string::npos) << dep; + auto api = line_of("build gcm.cache/dep.api.gcm : stage_file"); + ASSERT_FALSE(api.empty()) << ninja; + EXPECT_NE(api.find("|| gcm.cache/dep.gcm"), std::string::npos) << api; + auto util = line_of("build gcm.cache/dep.util.gcm : stage_file"); + ASSERT_FALSE(util.empty()) << ninja; + EXPECT_EQ(util.find("||"), std::string::npos) << util; + + // The aggregate holds the BMIs that wait for nothing, and every object. + auto phony = line_of("build _mcpp_staged_cache : phony"); + ASSERT_FALSE(phony.empty()) << ninja; + auto words = [](const std::string& s) { + std::set w; + std::istringstream is(s); + for (std::string t; is >> t;) w.insert(t); + return w; + }; + auto held = words(phony); + EXPECT_FALSE(held.contains("gcm.cache/dep.gcm")) << phony; + EXPECT_FALSE(held.contains("gcm.cache/dep.api.gcm")) << phony; + EXPECT_TRUE(held.contains("gcm.cache/dep.util.gcm")) << phony; + for (auto* o : {"obj/dep.m.o", "obj/api.m.o", "obj/util.m.o"}) + EXPECT_TRUE(held.contains(o)) << o << " missing from: " << phony; + + // The generated unit still compiles after the aggregate: no cycle. + auto gen = line_of("build obj/gen.m.o"); + ASSERT_FALSE(gen.empty()) << ninja; + EXPECT_NE(gen.find("|| _mcpp_staged_cache"), std::string::npos) << gen; +} + TEST(NinjaBackend, NoStagedPhonyWhenNothingIsCached) { auto plan = minimal_plan(); plan.compileUnits.push_back({