fix: Finished once per command, the host tool's sub-build states its own output, module maps rewritten when they differ, and a workspace addresses a member's outside source by its place in the workspace

Author: speak-agent

.agents/docs/2026-10-01-pack-drive-and-selection-independent-compile-design.md

@@ -597,6 +597,26 @@ command lines that T7's criteria compare.
   command file: the second CI round of #754 failed e2e 258, 262 and 848 on
   the Windows legs with `D8004: '/reference' requires an argument`. A unit
   test now writes both compilers' files and reads them back.
+- **The review of the pull request** (section 15.3) found five more places,
+  each fixed with a unit test:
+  - `mcpp run --format <name>` wrote `Finished` twice, once for the pack and
+    once for the build it drives again to resolve the runner. `Finished` is
+    now written once per command, in the progress model.
+  - An eleventh drive site, the sub-build of a dependency's host tool inside
+    planning (`prepare/features.cpp`), took the new default and was reported
+    as lines of the command, while its caller folds its output into an error
+    message. It states `Caller`.
+  - An argument file holds the build directory's absolute path while its name
+    hashes relative paths, so a moved or restored build directory kept the
+    old file. Both module map files are now written whenever their content
+    differs.
+  - The root was recognised by the name `workspace`; a member of that name
+    would have kept its BMIs at their names. A virtual root is recognised by
+    `virtualRoot`.
+  - A member's file inside another member's directory was addressed under
+    the containing package, which the virtual root replaced when the selection
+    did not hold that member. A workspace plan addresses such a file by its
+    place in the workspace (`obj/<member>/__ws/<path>`).
 - **B2's predicate** is "ELF and not freestanding", read from the target
   triple with the host triple when the target is empty. Mach-O is excluded
   because its compilers default to PIC; WebAssembly refuses shared objects.
@@ -631,3 +651,12 @@ command lines that T7's criteria compare.
   on the host against this branch: 9 of 9 sections pass, xlings built from its
   source among them; against 2026.10.1.1 every CHANGE section fails.
 
+### 15.3 Review
+
+A review of the full diff, independent of the author, read the plan, backend,
+pack, cache and progress code against this record. Its three findings above
+medium confidence and its two below were confirmed in the code and fixed
+(section 15.1). It found the per-unit maps, their keys, the dyndep and staging
+paths, the cache population, both argument-file writers, the job count and the
+token reclaim consistent with sections 3 and 4.
+

docs/specs/build-database.md

@@ -128,10 +128,11 @@ mcpp 输出的 S1 文档满足 S1 等级 2,不输出 `ide.options`。等级 3 
   之间保持稳定。**已实现**
 - **R3.8a** 一个模块名在一个程序之内标识一个模块,而一个配置可以包含多个程序
   (#732)。除根包之外,每个包的 BMI 都位于其所属包的子目录下(2026.10.1.2+,#751)。
-  导入其他包模块的单元,以及提供这样一个模块的单元,其 `arguments` 带有构建所用的
-  一份模块映射:GCC 为 `-fmodule-mapper=<映射文件>`(相对 `work-directory`),Clang
-  与 MSVC 为一个参数文件 `@<绝对路径>/modmap/<包>-<哈希>.modmap`,其中每行一个
-  `-fmodule-file=<名字>=<路径>` 或 `/reference <名字>=<路径>`。映射只列出该单元经其
+  导入其他包模块的单元,其 `arguments` 带有构建所用的一份模块映射:GCC 为
+  `-fmodule-mapper=<映射文件>`(相对 `work-directory`;提供这样一个模块的单元也带,
+  因为 GCC 经映射写出 BMI),Clang 与 MSVC 为一个参数文件
+  `@<绝对路径>/modmap/<包>-<哈希>.modmap`,其中每行一个 `-fmodule-file=<名字>=<路径>`
+  或 `/reference <名字>=<路径>`。映射只列出该单元经其
   导入所能到达的模块,因此同一单元在一个配置的每一种成员选择下,`arguments` 逐字相同。
   `.modmap` 后缀与 CMake 模块映射相同,读者可据此把未展开的参数文件识别为模块机制。
   同一个名字由两个包提供时,两个单元的 `provides` 都列出它;只按名字在文档中查找提供方

src/build/ninja_backend.cppm

@@ -4733,22 +4733,27 @@ std::unique_ptr<Backend> make_ninja_backend() {
 }
 
 // The module maps of the units that reach a BMI below its provider's directory
-// (B1). The key in a file's name is a hash of what the file holds, so a file
-// that exists is already right, and a changed resolution names a new file.
+// (B1). The key in a file's name is a hash of the relative paths the file
+// holds, so it is the same in every build directory. The argument file also
+// holds the build directory's absolute path, so a file is written whenever
+// what it holds differs from what is there: a build directory that was moved
+// or restored at another path would otherwise keep pointing at the old one.
 void write_module_maps(const BuildPlan& plan) {
     const bool msvc = msvc_module_spelling(plan);
-    for (auto const& [key, scope] : plan.moduleScopes) {
+    auto write_if_changed = [](const std::filesystem::path& path, const std::string& text) {
         std::error_code ec;
-        const auto path = plan.outputDir / scope.mapFile;
-        if (!std::filesystem::exists(path, ec)) {
-            std::filesystem::create_directories(path.parent_path(), ec);
-            std::ofstream(path, std::ios::binary | std::ios::trunc) << scope.content;
+        if (std::filesystem::exists(path, ec)) {
+            std::ifstream in(path, std::ios::binary);
+            if (std::string(std::istreambuf_iterator<char>(in), {}) == text) return;
         }
-        if (scope.argsFile.empty()) continue;
-        const auto args = plan.outputDir / scope.argsFile;
-        if (std::filesystem::exists(args, ec)) continue;
-        std::ofstream(args, std::ios::binary | std::ios::trunc)
-            << module_map_arguments(scope.arguments, msvc);
+        std::filesystem::create_directories(path.parent_path(), ec);
+        std::ofstream(path, std::ios::binary | std::ios::trunc) << text;
+    };
+    for (auto const& [key, scope] : plan.moduleScopes) {
+        write_if_changed(plan.outputDir / scope.mapFile, scope.content);
+        if (!scope.argsFile.empty())
+            write_if_changed(plan.outputDir / scope.argsFile,
+                             module_map_arguments(scope.arguments, msvc));
     }
 }
 

src/build/plan.cppm

@@ -1954,6 +1954,19 @@ make_plan(const mcpp::manifest::Manifest&         manifest,
     };
     auto outside_prefix = [&](const std::filesystem::path& src)
         -> std::filesystem::path {
+        // IN A WORKSPACE PLAN, BY ITS PLACE IN THE WORKSPACE (pack drive and
+        // selection design 2026-10-01, B1). The package that contains the file
+        // may be a member the selection does not hold, and then the virtual
+        // root contains it instead: a member that lists a file of another
+        // member's directory had one address under `-p` and another under
+        // `--workspace`. The workspace's directory is the same in every
+        // selection.
+        if (manifest.package.virtualRoot) {
+            std::error_code ec;
+            auto rel = std::filesystem::relative(src, projectRoot, ec);
+            if (!ec && !rel.empty() && !rel.generic_string().starts_with(".."))
+                return std::filesystem::path("__ws") / safe_object_prefix({}, rel.parent_path());
+        }
         if (auto c = container_of(src)) {
             std::error_code ec;
             auto rel = std::filesystem::relative(src, packages[*c].root, ec);
@@ -3275,7 +3288,10 @@ make_plan(const mcpp::manifest::Manifest&         manifest,
     // after every producer of a compile unit (a target's `main` included).
     {
         const auto traits = mcpp::toolchain::bmi_traits(tc);
-        const auto rootPackage = qualified_package_name(manifest);
+        // A workspace plan's root is virtual and provides nothing, so every
+        // member is placed below its own directory, whatever it is named.
+        const auto rootPackage = manifest.package.virtualRoot
+            ? std::string{} : qualified_package_name(manifest);
         auto basename = [&](std::string_view name) {
             std::string out;
             for (char c : name) out.push_back(c == ':' ? '-' : c);

src/build/prepare/features.cpp

@@ -1727,6 +1727,10 @@ step6_build_tool(PrepareState& state, HostToolCtx& ctx) {
     auto be = mcpp::build::make_ninja_backend();
     mcpp::build::BuildOptions bopt;
     bopt.ninjaTargets = { goal.generic_string() };
+    // A build inside planning: its output is this function's error message,
+    // not lines of the command's report (pack drive and selection design
+    // 2026-10-01, A2).
+    bopt.report = mcpp::build::BuildOptions::Report::Caller;
     // Unfiltered inner output on demand: the filter drops
     // ninja's own progress and command echoes, which is right
     // for a normal build and wrong when the question is "what

src/build/progress.cppm

@@ -220,6 +220,7 @@ void programs_done();
 void checking();
 
 // `Finished` (design §4.5): the whole command's time, and how it was spent.
+// Written once per command; a later call does nothing.
 void finished(std::string_view profile, std::string_view descriptor);
 // A command that builds several configurations writes one `Finished`, after
 // all of them: `finished` then only records what it was given, and
@@ -739,6 +740,8 @@ struct Report {
     bool failureReported = false;
     bool deferred = false;
     std::optional<std::pair<std::string, std::string>> deferredFinish;
+    // `Finished` was written: a command states it once.
+    bool finishedWritten = false;
     // The status row's screen (revision 3, §5.9 to §5.13): an animation fed
     // by the build, or none.
     std::unique_ptr<screen::Animation> animation;
@@ -1362,6 +1365,13 @@ void finished(std::string_view profile, std::string_view descriptor) {
             r.deferredFinish.emplace(std::string(profile), std::string(descriptor));
             return;
         }
+        // ONCE PER COMMAND. `mcpp run --format <name>` packs, which states its
+        // build and writes `Finished`, and then drives the build again to
+        // resolve the runner, a scan that finds nothing to do and would write
+        // a second `Finished` counting the first's time again (pack drive and
+        // selection design 2026-10-01, A3).
+        if (r.finishedWritten) return;
+        r.finishedWritten = true;
         // The breakdown and the longest step explain a wait; a command shorter
         // than a minute has none worth a longer line (build output design
         // revision 3, §7.3).

tests/unit/test_build_progress.cpp

@@ -475,3 +475,16 @@ TEST(ProgressModel, AStepWhoseEntriesAreReadInTwoPiecesCountsOnce) {
     auto out = testing::internal::GetCapturedStderr();
     EXPECT_EQ(count(out, "Compiling lib"), 1u) << out;
 }
+
+// `Finished` is written once per command. `mcpp run --format <name>` packs, and
+// the pack states its build with `Finished`; the run then drives the same build
+// again, and its second `Finished` repeated the line with the first's time
+// counted twice (pack drive and selection design 2026-10-01, A3).
+TEST(ProgressModel, FinishedIsWrittenOncePerCommand) {
+    mcpp::ui::disable_color();
+    testing::internal::CaptureStderr();
+    mcpp::build::progress::finished("release", "optimized");
+    mcpp::build::progress::finished("release", "optimized");
+    auto out = testing::internal::GetCapturedStderr();
+    EXPECT_EQ(count(out, "Finished"), 1u) << out;
+}

tests/unit/test_module_address.cpp

@@ -334,6 +334,9 @@ TEST(ModuleAddress, ArgumentFilesAreWrittenAsEachCompilerReadsThem) {
     ms.arguments = {"/reference", "m=C:/build/ifc.cache/core/m.ifc",
                     "/reference", "n=C:/build/ifc.cache/core/n.ifc"};
     msvc.moduleScopes.emplace("core-1", ms);
+    // A file a moved or restored build directory left behind is rewritten.
+    std::filesystem::create_directories(msvc.outputDir / "modmap");
+    std::ofstream(msvc.outputDir / "modmap/core-1.modmap") << "/reference m=D:/old/m.ifc\n";
     write_module_maps(msvc);
     EXPECT_EQ(read(msvc.outputDir / "modmap/core-1.modmap"),
               "\xEF\xBB\xBF/reference m=C:/build/ifc.cache/core/m.ifc\n"

tests/unit/test_object_address.cpp

@@ -415,3 +415,43 @@ TEST(ObjectAddress, AMembersOutsideSourceIsImmuneToAnotherMemberListingIt) {
     // Filed under its declaring member, not at the root's flat address.
     EXPECT_NE(alone.generic_string().find("obj/core/"), std::string::npos) << alone;
 }
+
+// The same for a file inside ANOTHER member's directory: the member that
+// contains it may be outside the selection, and then the virtual root contains
+// the file instead. The address is the file's place in the workspace.
+TEST(ObjectAddress, AMembersSourceInAnotherMembersDirectoryIsImmuneToTheSelection) {
+    Tmp t;
+    const auto ws = t.path / "ws";
+    auto plan_with = [&](bool withLib) -> std::filesystem::path {
+        mcpp::manifest::Manifest root;
+        root.package.name        = "workspace";
+        root.package.version     = "0.0.0";
+        root.package.standard    = "c++23";
+        root.package.virtualRoot = true;
+        std::vector<mcpp::modgraph::PackageRoot> packages;
+        auto rootPkg = makePackage(ws, "workspace");
+        rootPkg.manifest = root;
+        packages.push_back(rootPkg);
+        packages.push_back(makePackage(ws / "app", "app"));
+        if (withLib) packages.push_back(makePackage(ws / "lib", "lib"));
+
+        mcpp::modgraph::Graph graph;
+        graph.units.push_back(unitFor(ws / "app", "../lib/src/shared.c", "app"));
+        if (withLib) graph.units.push_back(unitFor(ws / "lib", "src/lib.c", "lib"));
+        std::vector<std::size_t> topo;
+        for (std::size_t i = 0; i < graph.units.size(); ++i) topo.push_back(i);
+        auto plan = make_plan(root, gccLike(), {}, graph, topo, packages, ws,
+                              ws / "target" / "t", {}, {}, {});
+        EXPECT_TRUE(plan) << (plan ? std::string{} : plan.error());
+        if (!plan) return {};
+        for (auto const& cu : plan->compileUnits)
+            if (cu.packageName == "app") return cu.object;
+        ADD_FAILURE() << "app's unit is not in the plan";
+        return {};
+    };
+    const auto alone  = plan_with(false);
+    const auto beside = plan_with(true);
+    EXPECT_FALSE(alone.empty());
+    EXPECT_EQ(alone, beside);
+    EXPECT_NE(alone.generic_string().find("obj/app/__ws/lib/src/"), std::string::npos) << alone;
+}