From c33eedf7bbb6707f959d369e7fad92e91b4b5e29 Mon Sep 17 00:00:00 2001 From: speak-agent <248744407+speak-agent@users.noreply.github.com> Date: Thu, 1 Oct 2026 01:04:43 +0800 Subject: [PATCH 01/26] docs: plan for member selection, build programs prepared once, a pack over several members, and the output streams (#748, #749, #750) --- ...r-selection-and-build-program-cost-plan.md | 826 ++++++++++++++++++ .agents/docs/README.md | 4 +- 2 files changed, 829 insertions(+), 1 deletion(-) create mode 100644 .agents/docs/2026-09-30-member-selection-and-build-program-cost-plan.md diff --git a/.agents/docs/2026-09-30-member-selection-and-build-program-cost-plan.md b/.agents/docs/2026-09-30-member-selection-and-build-program-cost-plan.md new file mode 100644 index 00000000..7a65cee3 --- /dev/null +++ b/.agents/docs/2026-09-30-member-selection-and-build-program-cost-plan.md @@ -0,0 +1,826 @@ +--- +subject: design +status: active +--- + +# Member selection, build programs prepared once, a pack over several members, and the output streams of `mcpp run`: the plan for the release after 2026.9.30.2 (#748, #749, #750) + +- Status: revision 3, in implementation. + - Revision 3 settles the open decisions by their recommendations (D3, D6, + D7), since the review directed that the plan be implemented. It removes + the stage-specific downstream verification from the plan, adds a review + from several angles (section 12), the tasks with their ownership and + dependencies (section 13), and the cross-repository work and its + verification (section 14). It folds #750, filed after revision 1, into S, + and records #751 as outside this release (section 8). + - Revision 1 was reviewed on 2026-10-01. D1, D2 and D5 were accepted. + - D4 was settled by the reviewer's rule: what the project owns is kept in + the project, and what comes from the index is kept globally. + - Status output moves to stderr on every command (R3, option (b)). + - D3 and D6 were asked about. Revision 2 explains D3 and replaces D6: the + revision 1 answer would have changed the meaning of a JSON field that + `docs/50-machine-output.md` §7 fixes. + - The review also asked for `mcpp run -q` to be aligned with established + practice. That is section 6, from a measurement. + - Section 11 is the self-review of revision 2. +- Date: 2026-09-30, revised 2026-10-01. +- Origin: + 1. A question on how to build or test several workspace members at once + (`mcpp test -p a -p b`), and the follow-up questions on what Cargo does + and what mcpp's own design admits. + 2. mcpp#748: a workspace's build programs are prepared and compiled one + after another, although only their runs have an order. + 3. mcpp#749: `mcpp pack` packs one member per invocation, so packing + several members plans the graph and runs the build programs once per + member. + 4. Review round 1: whether `mcpp run -q` follows the conventions of + comparable tools. + 5. mcpp#750, filed independently of this plan: a repeated `-p` keeps only + the last value (F1). +- Task: one plan for the next release that treats the four as one subject. + The subject is which members a command acts on, what a command pays once + per selection rather than once per member, and which stream carries what. + +## 0. Summary + +| # | Finding | Item | +|---|---|---| +| F1 | `-p` takes one value on every command. A repeated `-p` is accepted, and all but the last are dropped without a word (measured) | S1, S2 | +| F2 | The selection is computed by two functions. `build` and `emit` use `workspace_selection` and plan once per configuration group. `test --workspace` uses `workspace_fanout_members` and plans each member alone (code) | S1, S4 | +| F3 | Each build program compiles the bundled `mcpp` module, and every host module it imports, into its own directory before its own compile. #748 measured about 7.8 s per program on its runner, repeated for every program and every invocation. The attribution comes from code reading and is to be confirmed | B1, B2 | +| F4 | `pack` has one `-p`, no `--workspace`, and one stage directory per invocation (code, #749) | K1 | +| F5 | `mcpp run` writes its status lines to stdout. With `-q` it still writes an empty line to stdout before the program's output. A failed build exits 1, as a program that returns 1 does (measured) | R1, R2, R3 | + +The plan has four parts. No manifest key is added. + +- **S, the selection.** + - S1: one selection shared by every command. + - S2: `-p` is repeatable. + - S3: `--exclude`. + - S4: `test` plans a selection once. +- **B, the build programs (#748).** + - B1: the `mcpp` module and host modules are compiled once per agreeing + flag set. The output is kept globally when it comes from the engine or the + index, and in the workspace otherwise. + - B2: programs are compiled concurrently, after the process launcher is + made safe for concurrent callers (B2-0). + - B3: runs in dependency waves, deferred. +- **K, pack over a selection (#749).** + - K1: `pack --workspace` and a repeated `-p`, with one plan and one stage + directory per member. +- **R, the output streams.** + - R1: no empty line under `-q`. + - R2: a distinct exit status for a `run` whose build failed. + - R3: status output on stderr, on every command. + +Three existing behaviours change: + +- A repeated `-p` selects every member it names (F1). +- Status lines leave stdout (R3). +- A `run` whose build failed exits 101 (D7). + +## 1. Findings + +### F1. A repeated `-p` keeps the last value + +`src/cli.cppm` declares `package` with `.takes_value()` and without +`.multiple()`. It does so on `build` (390), `run` (436), `test` (538), `pack` +(660) and `describe` (707). + +Measured with mcpp 2026.9.30.1 on a virtual workspace of three members `a`, +`b` and `c`. The option declaration is the same at 8e00a183. + +``` +$ mcpp build -p a -p b + Workspace building member 'b' + ... + Compiling b v0.1.0 (b) + Finished dev [unoptimized + debuginfo] in 0.30s +``` + +`a` is not built, and nothing says so. #750 reports the same defect on +`mcpp test -p a -p b`, which tests `b` alone and exits 0. + +### F2. Two selection functions, and `test` plans each member alone + +`src/cli/cmd_build.cppm` has two functions: + +- `workspace_selection` (79) returns the workspace root and the selected + members. `build` (324) and `emit build-database` (612) pass the members to + `workspace_groups`. They then plan once per configuration group through + `BuildOverrides::workspace_members` (workspace design 2026-09-29, §15). +- `workspace_fanout_members` (50) returns the member list alone. `test` (998) + loops over it and calls `run_tests` once per member with + `package_filter = member`. Each call resolves the toolchain, plans that + member's closure, and runs the build programs of the closure. + +The planner already accepts a group together with the test targets of each +member (`BuildOverrides::member_targets`, read in +`src/build/prepare/manifest.cpp:187`). `--configure-only` and +`emit build-database` use it. `test` does not. + +This has two consequences, the same ones #749 states for `pack`: + +- A shared member's build program runs once per member that reaches it. +- A shared member's active features are those of one closure, not the union + across the selection. + +### F3. The preparation of a build program is repeated per program + +When the program's cache is stale, `run_build_program` +(`src/build/build_program.cppm`) performs four steps: + +1. It compiles the bundled `mcpp` module and its `mcpp.core` alias + (`build_mcpp_module`, 1401). +2. It obtains the `std` module, which is already cached globally by + `stdmod::ensure_built`. +3. It compiles every host module the program imports (`build_host_module`, + 1518). +4. It compiles `build.mcpp` and runs it. + +Steps 1 and 3 write into `/target/.build-mcpp`. Neither step checks +whether its output is already current. `program_compiling` is called only at +step 4 (1665), so the reported `ran` covers step 4 alone. Steps 1 to 3 are +counted as `plan` in `Finished`. `step9_member_build_programs` +(`src/build/prepare/target_side.cpp:1945`) runs the programs one after +another. + +#748 measured about 7.8 s outside `ran` for each of four programs on a +4-vCPU `windows-2025` runner, and about 8 s again in each `mcpp pack`. The +four programs import the same host module with the same flags. + +### F4. `pack` acts on one member + +`pack` has one `-p` and no `--workspace` (`src/cli.cppm:660`). The stage +directory is one value per invocation (`BuildOverrides::pack_stage_dir`, +`src/build/prepare.cppm:687`). `step9_member_build_programs` gives that one +value to every program (`bpEnv.packStageDir`, target_side.cpp:1992). + +#749 measured two consecutive `mcpp pack -p` invocations on a five-member +workspace at 146 s. Distribution accounts for about 35 s of that. Most of the remainder is +planning and build programs repeated per invocation, plus a 29.2 s gap +between the two processes. + +### F5. The output streams of `mcpp run` + +The measurements used mcpp 2026.9.30.2 on member `a` of the F1 workspace. The +program prints `OUT` on stdout and `ERR` on stderr. + +| Command | stdout | Note | +|---|---|---| +| `mcpp run 2>/dev/null` | the `Workspace`, `Resolving`, `Resolved`, `Target`, `Inferred`, `Finished` and `Running` lines, an empty line, then `OUT` | status is on stdout | +| `mcpp run -q 2>/dev/null` | `\n` `OUT` `\n` (checked with `od -c`) | one empty line before the program's output | +| `mcpp run -q -- x` (the program returns 3) | exit 3 | the program's status passes through | +| `mcpp run -q` with a compile error | exit 1, and the diagnostic is shown | the build's status | +| `mcpp run -q` (the program returns 1) | exit 1 | indistinguishable from the previous row | + +Where each behaviour comes from: + +- **Status on stdout.** This was decided on purpose by the observability + design of 2026-05-22: `status`, `info`, `finished` and progress go to + stdout, and `warning` and `error` go to stderr. +- **The empty line.** `src/build/execute.cppm:2183` prints `std::println("")` + after the `Running` line, and it does so unconditionally. Under `-q`, only + the empty line remains. +- **Arguments after `--`.** The `--quiet`/`-q` pre-scan (`src/cli.cppm:161`) + stops at `--`, so arguments after it reach the program. + +## 2. What the design already states, and what follows from it + +| # | Principle | Where | Consequence here | +|---|---|---|---| +| P1 | An input mcpp cannot serve, or cannot read unambiguously, is refused by name. mcpp does not guess | `docs/00-what-mcpp-is.md`, the guarantee; `docs/07-workspace.md` §5.3, where an ambiguous `-p` is refused, naming every match | F1 is a defect. A name that matches no member is refused, and so is a selection a command cannot act on | +| P2 | One graph per configuration. The selection only chooses the roots | `docs/07-workspace.md` §5.4; workspace design 2026-09-29 §15 | Several members are one plan, never N invocations. The selection is a set, so argument order does not change the plan | +| P3 | A command over several members continues past a failing member, reports each member, and ends with a summary | `docs/07-workspace.md` §5.3 | A multi-member `-p` inherits this report. There is no fail-fast switch | +| P4 | `-p` names a package, resolved among the members | `docs/07-workspace.md` §5.3 | `-p` does not select a dependency, unlike Cargo | +| P5 | The engine carries general capabilities. A new manifest key is a compatibility cost on engines already released | the engine-and-plugin rule; the `[c-abi]` precedent | No `default-members` key | +| P6 | A BMI is usable only by a compile that agrees with it. The agreement is produced from one set of flags, not checked afterwards | `build_host_module` (`src/build/hostprogram.cppm:918`) | B1 shares a BMI only between compiles whose key, which includes the flags, is equal | +| P7 | Only what comes from the immutable store may enter the global cache, and only when nothing it was built against is local | `src/build/prepare/plan.cpp:1964` | B1 applies the same rule: engine and index output is global, and everything else stays in the workspace | +| P8 | The JSON streams add fields and never remove or redefine one | `docs/50-machine-output.md` §7 | D6 adds records and fields. `build_ms` keeps its meaning | + +### Compared with Cargo + +| Cargo | mcpp in this plan | Reason | +|---|---|---| +| `-p a -p b` | same | P2 | +| `--workspace --exclude c` | same, and also with the implicit whole selection at a virtual root (D2) | a virtual root without `-p` already means every member | +| `-p` accepts a dependency | refused; members only | P4 | +| `-p 'foo-*'` | not in this release | a fourth resolution form would make a pattern ambiguous between a name and a path | +| `[workspace] default-members` | not in this release | P5 | +| stops at the first failing test binary unless `--no-fail-fast` | continues and reports every member (D1) | P3 | +| status on stderr, and stdout is the program's | the same after R3 | section 6 | +| a failed build exits 101, and a program's status passes through | the same after R2 (D7) | section 6 | + +## 3. The selection (S1 to S4) + +### S1. One selection, one function + +`workspace_fanout_members` and `workspace_selection` are replaced by one +function. + +- Input: `(wantAll, packages[], excludes[])`. +- Output: `{root, members}`. `members` is a set, kept in `[workspace] + members` order (D5). A rooted workspace's own package comes first, as `"."`. + +| Input | Members | +|---|---| +| `--workspace` | all | +| a virtual root, no `-p` | all | +| a rooted root, no `-p` | `"."` | +| inside member X, no `-p` | X | +| `-p X -p Y` | {X, Y}, each resolved by the §5.3 order | +| any "all" form above with `--exclude Z` | all minus Z | + +The following are refused before any planning: + +| Input | Refusal | +|---|---| +| `-p N`, where N matches no member | refused, listing the members | +| N is ambiguous | refused, naming every match, as today | +| `--exclude` together with `-p` | refused | +| `--exclude Z`, where Z matches no member | refused | +| every member excluded | refused | + +Two `-p` values that resolve to one member select it once. + +### S2. `-p` is repeatable + +`.multiple()` is added to `package` on `build`, `test`, `pack` and +`describe`. `BuildOverrides::package_filter` stays a single string. The plan +reads it in 56 places, and a selection of one member still sets it. A +selection of several members goes through `workspace_members`, as +`--workspace` does now. + +`run` keeps one member. On `run`, a second `-p` is refused, naming both. + +### S3. `--exclude` + +The option is added to `build`, `test`, `pack` and `describe`. Its value is +resolved like `-p`, and it may be repeated. It applies to every "all" form +(D2) and is refused with `-p`. + +### S4. `test` plans a selection once + +A `test` over more than one member proceeds in four steps: + +1. It plans once per configuration group, with each member's test targets in + `member_targets`, as `--configure-only` does. +2. It builds each group's graph once. +3. It runs each member's tests in member order, continuing past failures + (D1). +4. Test discovery stays scoped to each member. + +The report keeps its shape, with one change for the shared build (D6, +section 7). The timeouts are divided between the two phases: + +- `--build-timeout` bounds the build. +- `--workspace-timeout` bounds the runs. A member not started before the + deadline is listed as `not run`, as before. + +A member that fails to plan fails alone, by the R5.2 rule `emit` already +uses. The other members of its group are planned without it. + +## 4. Build programs (#748) + +### B0. The attribution, measured first + +Before B1 is written, `mcpp build --workspace` runs with `MCPP_VERBOSE=1` on +a fixture of #748's shape: four members, each with a build program that +imports one member host module, which imports the bundled `mcpp` module. The +`buildmcpp-host` lines are timestamped and summed per step: the `mcpp` +module, `std`, each host module, and `build.mcpp`. + +If steps 1 and 3 of F3 are not most of the preparation, B1 is re-scoped. The +measurement is recorded in section 15, which the landing revision adds. +B0 also records the `base` flags of the bundled module's compile (see +section 11, item 10). + +### B1. The `mcpp` module and host modules are compiled once per key, and kept by provenance + +Each output of `build_mcpp_module` and `build_host_module` moves from the +program's `bdir` to an entry addressed by a key. The key is built from: + +- the host compiler's identity (`compilerHash`); +- the standard flag, `base`, and the `use` flags of the compile; +- the SHA-256 of the interface file; +- for the bundled module, the mcpp version, which determines its text; +- for a host module, the providing package's identity (index, name, + version). + +Under P6, a program whose flags differ has a different key. Agreement is +therefore a consequence of the key and is never checked separately. + +**Where the entry lives (D4, as decided): by provenance, under P7.** + +| What | Where | Why | +|---|---|---| +| The bundled `mcpp` module and `mcpp.core` | the global cache, next to the `std` module | the engine owns its text. It is identical in every project for one mcpp version and one host compiler | +| A host module from an index package whose sources are in the immutable store | the global cache, beside the package's cached objects | the same admission rule as the dependency cache (`plan.cpp:1964`) | +| A host module from a path or git dependency, or from a workspace member | `/target/.build-mcpp/host-modules//` | its sources can change without its name and version changing | + +A host module compiled "alone" imports only `std` and `mcpp` +(`build_host_module`: "a rule package is a leaf by construction"). Its local +taint is therefore its own package's alone, so the dependency cache's rule +applies without a closure walk. + +The cache mode applies as it does for dependencies: + +- `--cache global`: global when admissible, otherwise the workspace. +- `local` and `off`: the workspace. Programs of one invocation still share + entries. + +The global entries reuse `mcpp.bmi_cache`: + +- the entry layout and `entry.json`, whose recorded inputs are compared field + by field on a hit; +- the LRU stamp, so `mcpp cache gc` collects them; +- the write to a temporary name followed by a rename. + +On GCC, a consumer stages BMIs into its `gcm.cache`, which `bmi_cache` +already does for cached dependencies. On clang and MSVC, the BMIs are +referenced by path. + +Expected effect, which is an estimate, not a measurement: + +- On a fresh runner, the first program pays steps 1 and 3 of F3, and every + later program of the build pays neither. For the #748 workspace this saves + 25 to 30 s. +- A later `mcpp pack` or `mcpp test` in the same checkout pays neither. +- On a developer machine, a second project with the same mcpp and host + compiler does not compile the bundled module again. + +### B2. Programs are compiled concurrently + +**B2-0, the launcher is safe under concurrent use.** Two threads that start +children at the same time must not pass one child's pipe to the other. + +- `capture_exec` on Linux and macOS creates its pipe with `::pipe`, without + `O_CLOEXEC` (`modules/platform/src/process.cppm`). +- The bounded launchers on both families create an inheritable write end and + start the child with handle inheritance + (`modules/platform/src/windows/bounded_process.cppm`, `CreateProcessA` with + `bInheritHandles = TRUE`). + +A child started by one thread while another thread's pipe is open inherits +that pipe's write end. The other thread's reader then waits for end of file +until the unrelated child exits. The pipes are therefore created close-on-exec +where the platform offers it (`pipe2` with `O_CLOEXEC`; `posix_spawn`'s `dup2` +clears the flag on the child's descriptors). Where the platform does not +offer it (macOS pipes, Windows handle inheritance), the creation of the pipe, +the start of the child and the parent's close of the write end form one +critical section under a process-wide mutex. The registry of children for +signal forwarding (`guard_group_on_signal`) is made safe for concurrent +callers in the same change. + +`step9_member_build_programs` is split into two phases: + +- **Phase A** compiles every stale program concurrently, up to the build's + job count, once B1's entries exist. No compile depends on another program's + run. +- **Phase B** runs the programs in the present serial order and applies + their directives in that order. The plan and `build.ninja` are therefore + byte-identical to the serial form's (#748 D). + +A compile's output is captured and printed as a whole. When several compiles +fail, the failure reported is the first in the serial order (#748 E). + +### B3. Runs in dependency waves: deferred (D3) + +B3 would run programs with no dependency between them at the same time. In +the #748 workspace, gpp.core and gpp.updater would run together, then gpp.cli +and gpp.gui. + +#748 estimates the saving at 12.9 s to 10.8 s. The cost is paid in +correctness: + +- Directives from concurrent runs would have to be buffered and applied in + the serial order. +- Programs write generated files into their packages. Nothing now states + that two programs' writes do not interfere, because they have always run + one at a time. +- Reports, the first reported failure, and program timeouts would each need + an ordering rule. + +B3 is reconsidered only if a measurement after B1 and B2 shows the runs to be +the dominant remaining cost. + +## 5. Pack over a selection (#749) + +### K1 + +`mcpp pack --workspace`, `--exclude`, and a repeated `-p` select members +through S1. For `pack`, "all" means every member with a packable target. + +1. **Refusals before any compile.** + - A selected member that does not provide the requested `--format` is + refused, and the refusal names it. + - `--output` with more than one member must name a directory. + - Two members whose staging writes one destination from different sources + are refused, naming both, by the rule `mcpp stage` applies. +2. **One plan per configuration group, and one build.** Each build program + runs once. +3. **A stage directory and a format per member.** + - `BuildOverrides::pack_stage_dir` becomes a map from member to + directory. + - `step9_member_build_programs` already visits one package at a time, and + sets `bpEnv.packStageDir` and `bpEnv.packFormat` from the map. +4. **Distribution per member, in member order.** A failing member is + reported, and the others continue (P3). The exit status is non-zero if any + member failed. +5. **`mcpp pack -p X` keeps its meaning.** It plans X's closure alone. + +## 6. The output streams (R1 to R3) + +### Established practice + +The practice below is recalled, not measured here. + +- **Cargo.** `cargo run` writes every status line (`Compiling`, + `Finished`, `Running`) to stderr. The program owns stdout. `-q` suppresses + cargo's own messages, not errors. A failed build exits 101, and a program's + exit status passes through. +- **`go run` and `zig run`.** Both print nothing of their own on success and + write diagnostics to stderr. +- **The common rule.** stdout carries a command's result: the program's + output for `run`, and the document for a command that emits one. Progress + goes to stderr, where a redirection or a pipe does not capture it. + +### R1. No empty line under `-q` + +The separator after the `Running` line is written only when the `Running` +line was written. It goes to the same stream as the `Running` line, which is +stderr after R3. + +Criterion R-A: `mcpp run -q 2>/dev/null` produces exactly the program's +stdout, compared byte for byte. + +### R2. A `run` whose build failed exits with a status of its own (D7) + +A `run` whose planning or build fails exits **101**. This is Cargo's value +and an established convention. It is also rare among programs' own exit +statuses. The program's own status continues to pass through unchanged. +`build`, `test` and `pack` keep their exit statuses; `test`'s 0, 1 and 2 are +a documented contract. + +Criterion R-B: with a compile error, `mcpp run` exits 101. With a program +that returns 1, it exits 1. + +### R3. Status output on stderr, on every command (option (b), decided) + +Every line the 2026-05-22 design sends to stdout moves to stderr: `status`, +`info`, `finished`, `line`, the progress bar, and the live region. This +covers every command, so no two commands differ. Four changes follow: + +- **The live region's terminal test** follows stderr instead of stdout. + Output revision 3 (2026-09-30) draws a frame in one write, and that is + unaffected. +- **stdout keeps only a command's result.** This means the program's output + for `run`, the JSON streams, and the documents that `emit`, `describe` and + `--list-runners` print. +- **JSON modes.** stdout would now stay clean without `set_quiet`. The + existing `set_quiet` calls are kept, so that a JSON run also leaves stderr + free of human status, as it does today. +- **Records.** This plan supersedes the stream table of the 2026-05-22 + design, and that record is not edited (the record rule). The user-facing + docs that describe streams are updated in the same pull request: + `docs/50-machine-output.md` and `docs/09-commands-by-scenario.md`. + +**R3-0, a census first.** 46 e2e scripts mention `Compiling`, `Finished` or +`Running`. Most capture `2>&1`, which is indifferent to the change. The +scripts that capture stdout alone and assert a status line are counted +before the change, and each is corrected in the same pull request. + +**User-facing change.** `mcpp build | tee log` no longer records the status +lines, and becomes `mcpp build 2>&1 | tee log`. CI logs, which capture both +streams, are unaffected. The CHANGELOG states this under a breaking-change +heading. + +Criterion R-C: on a warning-free project, `mcpp build >/dev/null` still +shows `Compiling` and `Finished`, and `mcpp build 2>/dev/null` writes +nothing. + +## 7. Decisions + +| # | Question | Outcome | +|---|---|---| +| D1 | Should a multi-member `test` continue past a failing member? | **Accepted**: yes, and there is no switch | +| D2 | May `--exclude` be used with every "all" form? | **Accepted**: yes. It is refused with `-p` | +| D3 | Should B3, runs in dependency waves, be deferred? | **Settled by the recommendation**: deferred (§4 B3) | +| D4 | Where does B1's output live? | **Decided by the reviewer's rule**: engine and index output is global, and project-owned output stays in the workspace (§4 B1) | +| D5 | Are reports and packs in manifest order or command-line order? | **Accepted**: manifest order | +| D6 | How is a shared build reported in `test`? | **Settled by the recommendation**: revised form below | +| D7 | Should a `run` whose build failed exit 101? | **Settled by the recommendation**: yes (§6 R2) | +| D8 | Should status move to stderr on one command or on every command? | **Decided**: every command, option (b) (§6 R3) | + +**D6, revised.** Revision 1 proposed reporting each member's `elapsed_ms` as +its run time alone. That redefines a field, which P8 forbids. Revision 2 +adds and does not redefine: + +- **A group record** comes before the group's first test record: + `{"group_build": {"group": 0, "members": [...], "build_ms": N}}`. +- **Each member's `build_ms`** is its group's build wall time. That is still + true to the field's definition: the wall time this member's tests waited + for their build. A new field, `build_group`, names the group. + - A consumer that sums `build_ms` over members deduplicates by + `build_group`. + - A consumer that does not sum is unaffected. +- **Each test's `duration_ms`** stays "build+run wall time of this test". + Its build part is the sum of that test binary's own edges in + `.ninja_log`. +- **The human report** prints one line per group: members, build time, and + the slowest edges, for example `slowest: libs/jsc link 88s`. That keeps the + signal the per-member split existed for: a member whose link, not its + tests, is slow. Each member's line then states its run time. + +## 8. What this release does not do + +| Item | Reason | +|---|---| +| `-p` selecting a dependency | P4 | +| glob patterns in `-p` and `--exclude` | a fourth resolution form, and no demand | +| `[workspace] default-members` | P5 | +| `--fail-fast` / `--no-fail-fast` | P3; `--workspace-timeout` bounds the fan-out | +| several members as N internal invocations | P2 | +| a multi-member `run` | an artifact to execute is one program | +| B3 | D3 | +| a global home for project-owned host modules | P7 | +| #751, a shared member recompiled when another member compiles a module of the same name | see below | + +**#751 is outside this release.** W10 of 2026.9.30.2 moves two providers of +one module name below their packages' directories only when the plan holds +both (`src/build/plan.cppm`, the block after the product directories). A +plan of `-p app` holds one provider and a plan of `--workspace` holds two, +so the shared member's command lines differ between the two selections. A +command line independent of the selection needs one of two things: + +- the set of module names of the whole workspace, including the closures of + members that are not selected, at every plan; or +- every module placed below its provider and named explicitly to every + importer. On GCC this means a module mapper file on every compile, which is + the default path of every GCC build. + +Both change the planning of every workspace build, and neither is a +consequence of this plan's items. #751 remains open for its own design. The +criteria S-B and S-G below are stated on fixtures in which every module name +has one provider. + +## 9. Delivery + +One pull request in mcpp carries every item of this plan, the documentation, +and the version. The items share `src/cli/cmd_build.cppm`, `src/cli.cppm` +and the e2e suite, and R3 changes what every e2e script reads, so separate +pull requests would correct the same scripts more than once. Section 13 +divides the work into tasks and states their order; section 14 states what +the other repositories do. + +## 10. Acceptance criteria + +Every new e2e fixture writes its paths through named `*_HOST` variables and +passes the `00` path lint. + +**Selection** +- S-A. `mcpp build -p a -p b` in a workspace of `a`, `b` and `c` builds `a` + and `b`. `c`'s object directory stays absent. +- S-B. `mcpp build -p a -p b`, followed by `mcpp build -p b -p a`, adds no + compile edge to `.ninja_log`. +- S-C. `mcpp build -p nosuch` exits non-zero before planning. Its message + names `nosuch` and lists the members. +- S-D. `mcpp run -p a -p b` is refused, naming both. +- S-E. `mcpp build --workspace --exclude c` builds `a` and `b`. + `--exclude nosuch` and `-p a --exclude b` are refused. +- S-F. `a` and `b` share `core`, whose build program appends a line to a + file under `OUT_DIR` on each run. After `mcpp test --workspace`, the file + has one line. Today it has two. +- S-G. After `mcpp build --workspace`, `mcpp test --workspace` adds no + compile edge for `core`'s sources. The fixture has no dev-dependency, so + the test build does not change `core`'s features. +- S-H. The JSON stream of S-F has one `group_build` record, and both + members' summaries carry its `build_group` and its `build_ms`. +- S-I. #750's reproduction: `mcpp test -p a -p b` runs the tests of `a` and + of `b`, and the workspace summary counts both members. + +**Build programs (#748)** +- B-A. Two members' programs import one host module from a path dependency + with identical flags. The module's compile appears once in the + `buildmcpp-host` verbose log, and its entry is under the workspace's + `target/`. +- B-B. A program whose standard differs gets its own entry. +- B-C. After one build, a second project with the same mcpp and host + compiler does not compile the bundled `mcpp` module. There is no + `mcpp module precompile` or `compile` line in its verbose log. +- B-D. A host module from a path dependency is never written under the + global cache root, with `--cache global` set. +- B-E. Two independent programs record their compile start and end. The two + intervals overlap when `-j` is 2 or more. The test compares timestamps, not + wall time. +- B-F. `build.ninja` and the applied directives are byte-identical to those + of a build at concurrency 1, and the `ran` lines are in the same order. +- B-G. When two programs fail to compile, the reported failure is the first + in the serial order. +- B-H. Two children started at once from two threads, one of which exits at + once and one of which sleeps: the reader of the first returns when the + first exits, not when the second does (B2-0). + +**Pack (#749)** +- K-A. #749's criteria A to E. The stage directories are read through + `pack_stage_dir()` and written to `OUT_DIR`. + +**Output streams** +- R-A, R-B and R-C (section 6). + +## 11. Self-review of revision 2 + +1. **The D4 rule versus the #748 measurement.** The rule sends the measured + workspace's host module to the workspace store, because that module is a + workspace member and its sources are local. It is still compiled once per + build, which is #748's saving. The global part adds the bundled module and + index rules such as `mcpp.plugins`. No gain claimed for #748 depends on the + global part. +2. **Taint of an index host module.** P7's dependency cache also requires + that nothing a package was built against is local. A host module is + compiled alone, against `std` and `mcpp` only. `std` is already global, + and the bundled module is global under B1. The rule therefore holds + without a walk. If host modules ever gain imports of other packages, which + `build_host_module` states they cannot, this must be re-derived. B-D + guards the local side. +3. **Upgrade debris.** Every mcpp release leaves one stale entry for the + bundled module per host compiler. It is collected by `mcpp cache gc`, as a + stale `std` entry is. There is no new eviction mechanism. +4. **R3 and the live region.** When stdout is a pipe and stderr is a + terminal, as in `mcpp run | less` or `mcpp emit ... > file`, the region is + now drawn. Today it is not drawn in that case. That is the intended + effect, and it is also Cargo's behaviour. The reverse case, stderr + redirected and stdout a terminal, loses the region, which is also + intended. +5. **R3 and `mcpp test`.** Test programs' stdout is captured into the JSON + stream or printed on failure; its routing is not changed by R3. Only + mcpp's own status lines move. R-C checks `build` only. A test-side + criterion is not added, because the human test report + (`test result ok. …`) is itself a result and stays on stdout. **This + boundary, the report as result or as status, is the one open point of R3, + and PR 4 settles it by the census, following Cargo, which prints + `test result` on stdout.** +6. **S4 and `--workspace-timeout`.** Today the timeout can stop the fan-out + between member builds. After S4, the group build is one step and cannot + be interrupted by it. A workspace whose build alone exceeds the deadline + used to stop early and now overruns until `--build-timeout`. This is + stated in `docs/07-workspace.md` §5.3 by PR 1. +7. **S-G under dev-dependencies.** Revision 1 stated S-G without condition. + A dev-dependency can activate a feature of a shared package and + legitimately recompile it, as in Cargo. Revision 2 states the fixture's + condition. +8. **The exit status 101 and `run` under a runner.** With + `[target.].runner` or `--runner`, the status that passes through + is the runner's. A runner that itself returns 101 would read as a failed + build. This is accepted, as in Cargo, and documented. +9. **Numbers are estimates.** The 25 to 30 s of B1 and #749's 30 to 45 s are + estimates. B0 replaces the first by a measurement in the landing + revision. +10. **Cross-project reuse depends on `base`.** The key includes `base`, the + flags the bundled module is compiled with. By code reading, `base` is + `host_base_flags(tc, macosDeploymentTarget)` + (`src/build/build_program.cppm:534`). It is built from the host toolchain + and the macOS deployment target only, and so carries no project path. + Cross-project reuse therefore holds. B0 records `base` to confirm this; + if a project path appears, the bundled module stays in the workspace until + that is resolved. + +## 12. Review from several angles (revision 3) + +**Architecture.** +- One selection function replaces two (S1). The selection is data, a set of + members, and the planner already accepts a set; no planning mode is added. +- B1 is one keyed store with two homes, chosen by provenance (P7). The global + home reuses `mcpp.bmi_cache`'s entry layout, its recorded inputs and its + LRU stamp. +- B2 splits `run_build_program` into a compile phase and a run phase. Only + `step9_member_build_programs` schedules the two apart. The root package's + program and the dependencies' programs keep the single call. +- K1 divides `build_and_pack` into a build, a stage per member, and a + dispatch. A single member is the same code with one member. +- R3 is one decision in `mcpp.ui`: the stream that narrates. Every narrating + function reads it. + +**Stability.** +- Directives are applied in the serial order, so the plan of a concurrent + build is byte-identical to the serial plan (B-F). +- Concurrency is confined to compiles. Runs stay serial (B3 deferred). +- The launcher is made safe for concurrent callers before any concurrent + caller exists (B2-0). +- A store entry is written under a temporary name and renamed into place. + Its `entry.json` is written last, so a partial entry is never read. +- S4 reuses the group planning that `emit build-database` and + `--configure-only` already exercise. +- A member that fails to plan fails alone. + +**Simplicity.** +- No manifest key is added. +- The options added are a repeatable `-p` and `--exclude`. +- There is no switch for failing fast, for the stream of status output, or + for the store's location. + +**User experience.** +- A repeated `-p` does what it states. A misspelt member is refused, and the + refusal lists the members. +- `mcpp run -q > file` writes exactly the program's output. A failed build is + distinguishable from a failing program (101). +- `test` and `pack` over several members plan once. +- A pipe or a redirection receives a command's result, and progress stays on + the terminal. + +**Compatibility.** +- A single `-p` keeps its meaning. A repeated `-p`, which acted on the last + member alone, now acts on every member it names; the old behaviour was the + defect #750 reports. +- The JSON streams gain a record and a field and lose nothing (P8). +- Status lines move from stdout to stderr. This is the one change a script + can observe. It is stated in the CHANGELOG with its migration (`2>&1`). + Machine consumers read the JSON forms, which do not move. Section 14 + records that no ecosystem consumer reads status lines from stdout. +- The exit status 101 applies to `run` alone. `build`, `test` and `pack` keep + theirs. +- Nothing is written to a manifest, so an older engine reads every project + this release reads. + +**Cross-platform.** +- B1 covers the three BMI families. GCC finds BMIs under the compile's + `gcm.cache`, so a store entry is staged there, as `bmi_cache` stages a + cached dependency. clang names a BMI with `-fmodule-file==`, and + MSVC with `/reference =`. +- Store keys use `u8string()`. A path given to a tool is narrowed with + `try_narrow`, by the path narrowing rule of the contributing guide. +- B2-0 differs by platform: `pipe2` on Linux, a mutex on macOS, and a mutex or + an explicit handle list on Windows. +- R3's live region follows the stream it is drawn on. The Windows console + test `can_move_cursor` is asked of stderr. + +**Consistency.** +- `build`, `test`, `pack`, `describe` and `emit build-database` accept the + same selection. `run` refuses a second member. +- Every report is in manifest order (D5). +- Every command narrates on stderr. + +**Upgrade.** +- No project needs an edit, and no cache needs a migration. +- The first build after the upgrade compiles the bundled module once per host + compiler, because the key holds the mcpp version. +- A script that read status lines from stdout adds `2>&1`. + +**Test coverage.** +- Each criterion of section 10 has an e2e script. The selection function and + the store key have unit tests. +- The census of R3-0 corrects the existing scripts that read status from + stdout. Each correction captures both streams. +- B1's clang and MSVC paths run in the macOS and Windows e2e shards. B2-0 has + a test that starts two children at once and requires each reader to finish + when its own child exits. + +## 13. Tasks, ownership and dependencies + +| Task | Items | Files owned | Depends on | +|---|---|---|---| +| T1 | S1 to S4, D6, #750 | `src/cli.cppm` (the `package` and `exclude` options of `build`, `run`, `test`, `describe`, `emit build-database`), `src/cli/cmd_build.cppm`, a new `src/cli/selection.cppm` (module `mcpp.cli.selection`), `src/build/execute.cppm` (`run_tests` and its summary only), `src/build/prepare/manifest.cpp` and `src/build/prepare.cppm` (test targets of a group), `src/project.cppm` | none | +| T2 | B0, B2-0, B1, B2 | `src/build/build_program.cppm`, `src/build/hostprogram.cppm`, `src/build/prepare/target_side.cpp` (`step9_member_build_programs`), `src/bmi_cache.cppm`, `modules/platform/src/process.cppm`, `modules/platform/src/unix/*`, `modules/platform/src/windows/*`, `src/build/progress.cppm` (program lines) | none | +| T3 | R1, R2, R3 | `src/ui.cppm`, `modules/platform/src/terminal.cppm`, `src/build/execute.cppm` (the `run` path only), the e2e scripts of the R3-0 census | none | +| T4 | K1 | `src/pack/pipeline.cppm`, `src/cli/cmd_publish.cppm`, `src/cli.cppm` (the options of `pack`), `src/build/prepare.cppm` (the stage directory per member), one hunk in `step9_member_build_programs` and in `src/build/prepare/features.cpp` (the stage directory a program receives) | T1 (the selection module), T2 (the final form of `step9`) | +| T5 | integration | `mcpp.toml` and `modules/versioning/src/version.cppm` (the version), `CHANGELOG.md`, `docs/` and `docs/zh/`, this record | T1 to T4 | + +- T1, T2 and T3 proceed at the same time, each on its own branch from the + integration branch. +- They are merged in the order T2, T1, T3, so that the census of T3 is taken + against the scripts T1 and T2 add. New scripts capture both streams when + they assert a status line, so that they hold before and after T3. +- T4 starts on the integration branch once T1 and T2 are merged. +- T5 takes the full e2e suite on the merged branch, locally in shards, then + opens the pull request. + +## 14. Cross-repository work and verification + +| Repository | Change | When | +|---|---|---| +| mcpp-community/mcpp | the pull request of section 9, then a release by tag | first | +| xlings-res/mcpp (GitHub and GitCode) | release assets mirrored by `publish-ecosystem`. The GitCode assets are uploaded with a local `gtc` as soon as each appears on the GitHub release | during the release | +| openxlings/xim-pkgindex | the bump pull request that `publish-ecosystem` opens, merged by a maintainer | after the mirror is verified by GET | +| mcpp-community/mcpp-index | `MCPP_VERSION` and `latest_mcpp` move to the new version, and a full scan runs by `workflow_dispatch` | after the index entry is live | +| openxlings/xlings | none. `kXlingsVersion` 2026.9.30.1 is xlings' latest release. xlings' CI reads mcpp's exit status and not its status lines | none | +| mcpp-language-server | none. It reads the `emit build-database --format json` envelope from stdout and the exit status | none | + +The release canaries (xlings, mcppls) build with the tagged mcpp before the +platform builds start. + +**Verification of the released artifact.** In an xlings subos sandbox, with +the released mcpp addressed at its store path and the CN mirror set inside +the sandbox (`mcpp self config --mirror CN`), a script runs: + +1. `mcpp test -p a -p b` on a three-member workspace: both members tested, + the third untouched (#750). +2. `--exclude`, and the refusal of an unknown member. +3. A workspace of four members whose programs import one host module: the + host module compiled once (#748). +4. `mcpp pack --workspace` over two members that share a member with a build + program: each program run once, and one package per member (#749). +5. `mcpp run -q` with stdout redirected: exactly the program's output, and + exit 101 for a compile error. + +Each section reports ok, failed or not run separately. The script's result is +posted on #748, #749 and #750, which are then closed. diff --git a/.agents/docs/README.md b/.agents/docs/README.md index 20380229..fa9ed62b 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 --- ``` -320 records. +321 records. ## By subject @@ -30,6 +30,7 @@ Records that declare one. Everything else is listed by date below. ### design +- [Member selection, build programs prepared once, a pack over several members, and the output streams of `mcpp run`: the plan for the release after 2026.9.30.2 (#748, #749, #750)](2026-09-30-member-selection-and-build-program-cost-plan.md) — active - [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 @@ -111,6 +112,7 @@ Records that declare one. Everything else is listed by date below. ### 2026-09 +- [Member selection, build programs prepared once, a pack over several members, and the output streams of `mcpp run`: the plan for the release after 2026.9.30.2 (#748, #749, #750)](2026-09-30-member-selection-and-build-program-cost-plan.md) — active - [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 From ecff5992e87bfd34d09985256fa39343ed1e56ec Mon Sep 17 00:00:00 2001 From: speak-agent <248744407+speak-agent@users.noreply.github.com> Date: Thu, 1 Oct 2026 01:21:15 +0800 Subject: [PATCH 02/26] platform: starting a child is safe for concurrent callers (#748, B2-0) A child inherits every descriptor (every inheritable handle on Windows) its parent holds, so a child started by one thread while another thread's capture pipe is open kept that pipe's write end open, and the other thread's reader waited for end of file until an unrelated child exited. Linux creates the pipes with pipe2(O_CLOEXEC). Where the platform has no such call (macOS) and on Windows (handle inheritance, _popen), the creation of the pipe, the start of the child and the parent's close of the write end form one critical section under a process-wide mutex. The registry of children that a signal terminates is serialized for its writers, so two threads no longer claim one slot, and holds 256 entries instead of 8, since one is held per running child. The Windows capture path sets a process environment variable only when its value differs, so concurrent callers passing one value stop mutating the environment block after the first. A test starts a quick and a slow child from two threads at one instant, 12 rounds, and requires the quick reader to return before the slow child exits. --- modules/platform/src/process.cppm | 83 +++++++++++-- .../platform/src/unix/bounded_process.cppm | 114 ++++++++++++++++-- .../platform/src/windows/bounded_process.cppm | 99 +++++++++++++-- .../test_process_concurrent_children.cpp | 105 ++++++++++++++++ 4 files changed, 367 insertions(+), 34 deletions(-) create mode 100644 modules/platform/tests/test_process_concurrent_children.cpp diff --git a/modules/platform/src/process.cppm b/modules/platform/src/process.cppm index d559cded..730f58d1 100644 --- a/modules/platform/src/process.cppm +++ b/modules/platform/src/process.cppm @@ -364,6 +364,16 @@ std::string windows_shell_command_line(std::string_view command) { namespace { +// The section every child start holds from the creation of its pipe to the +// parent's close of the write end (see mcpp.platform.unix.bounded_process, which +// states why). Each platform's module defines a class on every platform and +// empties the one that does not apply, so the uses below need no branch. +#if defined(_WIN32) +using LaunchSection = mcpp::platform::winproc::LaunchSection; +#else +using LaunchSection = mcpp::platform::unixproc::LaunchSection; +#endif + // Append a non-interactive stdin redirect to prevent child processes from // blocking on terminal input. // - POSIX: "< /dev/null" — fixes macOS xcrun / xcode-select hangs. @@ -537,7 +547,13 @@ RunResult capture(std::string_view command) { auto cmd = finalize_shell_command(command); RunResult result; - std::FILE* fp = ::popen(cmd.c_str(), "r"); + std::FILE* fp = nullptr; + { + // `popen` creates the pipe and starts the child in one call, so the + // section is exactly that call. + LaunchSection launch; + fp = ::popen(cmd.c_str(), "r"); + } if (!fp) { result.exit_code = -1; return result; @@ -564,8 +580,21 @@ RunResult capture_with_env( const std::vector>& env) { #if defined(_WIN32) - for (auto& [k, v] : env) - _putenv_s(k.c_str(), v.c_str()); + // This mutates the calling process's environment, as it always has, and it + // is called from several threads at once when a workspace's build programs + // are compiled together. A variable is therefore set only when its value + // differs, so callers that pass one value (the toolchain's INCLUDE and LIB, + // the same for every compile of a build) stop touching the environment block + // after the first, and the writes that remain are made under the section the + // children are started in. + { + LaunchSection launch; + for (auto& [k, v] : env) { + const char* now = std::getenv(k.c_str()); + if (now && v == now) continue; + _putenv_s(k.c_str(), v.c_str()); + } + } return capture(command); #else std::string prefixed; @@ -595,7 +624,11 @@ int run_streaming(std::string_view command, std::function on_line) { auto cmd = finalize_shell_command(command); - std::FILE* fp = ::popen(cmd.c_str(), "r"); + std::FILE* fp = nullptr; + { + LaunchSection launch; + fp = ::popen(cmd.c_str(), "r"); + } if (!fp) return -1; std::array buf{}; @@ -624,7 +657,11 @@ int run_streaming(std::string_view command, int run_passthrough(std::string_view command, std::string* output) { auto cmd = finalize_shell_command(command); - std::FILE* fp = ::popen(cmd.c_str(), "r"); + std::FILE* fp = nullptr; + { + LaunchSection launch; + fp = ::popen(cmd.c_str(), "r"); + } if (!fp) return -1; std::array buf{}; @@ -695,7 +732,13 @@ int run_exec(const std::vector& argv, ::posix_spawnattr_setflags(&attr, POSIX_SPAWN_SETPGROUP); pid_t pid = 0; - int sp = ::posix_spawnp(&pid, cargv[0], nullptr, &attr, cargv.data(), envp.data()); + int sp = 0; + { + // A child with no pipe of its own is still started outside the window + // another thread's pipe is open in. + LaunchSection launch; + sp = ::posix_spawnp(&pid, cargv[0], nullptr, &attr, cargv.data(), envp.data()); + } ::posix_spawnattr_destroy(&attr); if (sp != 0) { // Reported once: by the caller when it asked for the errno, here @@ -756,9 +799,6 @@ RunResult capture_exec( #if defined(__linux__) || defined(__APPLE__) // posix_spawn + a pipe; stdout and stderr both go to the pipe so the // captured text is combined (replaces the old `2>&1`). - int fds[2]; - if (::pipe(fds) != 0) { result.exit_code = 127; return result; } - auto envStore = merged_environ(extraEnv); std::vector envp; for (auto& s : envStore) envp.push_back(s.data()); @@ -767,6 +807,18 @@ RunResult capture_exec( for (auto& a : argv) cargv.push_back(const_cast(a.c_str())); cargv.push_back(nullptr); + // Several threads call this at once (a workspace's build programs are + // compiled together). The pipe is close-on-exec, so a child another thread + // starts while this one is open does not keep it open (see + // mcpp.platform.unix.bounded_process for the rule and for what a platform + // without `pipe2` does in addition). + LaunchSection launch; + int fds[2]; + if (mcpp::platform::unixproc::make_pipe(fds) != 0) { + result.exit_code = 127; + return result; + } + posix_spawn_file_actions_t fa; ::posix_spawn_file_actions_init(&fa); // Run the child in `cwd` when requested (e.g. build.mcpp, whose relative @@ -794,6 +846,7 @@ RunResult capture_exec( ::posix_spawnattr_destroy(&attr); ::posix_spawn_file_actions_destroy(&fa); ::close(fds[1]); + launch.release(); if (sp == 0) mcpp::platform::unixproc::guard_group_on_signal(pid); if (sp != 0) { ::close(fds[0]); @@ -845,9 +898,6 @@ RunResult capture_stdout( if (spawn_error) *spawn_error = 0; if (argv.empty()) { result.exit_code = 127; return result; } #if defined(__linux__) || defined(__APPLE__) - int fds[2]; - if (::pipe(fds) != 0) { result.exit_code = 127; return result; } - auto envStore = merged_environ(extraEnv); std::vector envp; for (auto& s : envStore) envp.push_back(s.data()); @@ -856,6 +906,14 @@ RunResult capture_stdout( for (auto& a : argv) cargv.push_back(const_cast(a.c_str())); cargv.push_back(nullptr); + // The same rule as capture_exec's. + LaunchSection launch; + int fds[2]; + if (mcpp::platform::unixproc::make_pipe(fds) != 0) { + result.exit_code = 127; + return result; + } + posix_spawn_file_actions_t fa; ::posix_spawn_file_actions_init(&fa); ::posix_spawn_file_actions_addopen(&fa, 0, "/dev/null", O_RDONLY, 0); @@ -876,6 +934,7 @@ RunResult capture_stdout( ::posix_spawnattr_destroy(&attr); ::posix_spawn_file_actions_destroy(&fa); ::close(fds[1]); + launch.release(); if (sp != 0) { ::close(fds[0]); result.exit_code = 127; diff --git a/modules/platform/src/unix/bounded_process.cppm b/modules/platform/src/unix/bounded_process.cppm index 9757652d..02a3237f 100644 --- a/modules/platform/src/unix/bounded_process.cppm +++ b/modules/platform/src/unix/bounded_process.cppm @@ -79,6 +79,43 @@ struct DeadlineRun { using OutputSink = void (*)(void* ctx, const char* data, unsigned long len); +// ─── Starting a child is safe for concurrent callers ───────────────────── +// +// Two threads that start children at the same time must not pass one child's +// pipe to the other. A child inherits every descriptor of its parent that is not +// close-on-exec, so a child started by thread B while thread A holds the write +// end of A's capture pipe keeps that end open for as long as it lives. A's +// reader then waits for end of file until B's unrelated child exits: a compile +// that finished at once is reported only when a longer one does, and a deadline +// the reader is meant to enforce is enforced late. +// +// The pipes are therefore created close-on-exec, and `posix_spawn`'s `dup2` +// action, which clears the flag on the descriptor it creates, is the only way a +// descriptor reaches the child. Where the platform has `pipe2` (Linux) that is +// one atomic call. Where it does not (macOS) the flag is set by a second call, +// which leaves a window in which another thread's child could be started; every +// start on such a platform therefore holds one process-wide section, from the +// creation of the pipe to the parent's close of its write end. On Linux the +// section is empty. +// +// `make_pipe` returns 0 on success and -1 on failure, as `pipe` does. Both ends +// are close-on-exec. The caller holds a `LaunchSection` across this call and the +// start of the child. +int make_pipe(int fds[2]); + +class LaunchSection { +public: + LaunchSection(); + ~LaunchSection(); + // Leaves the section before the destructor does: a caller that goes on to + // wait for its child must not hold it for the child's whole life. + void release(); + LaunchSection(const LaunchSection&) = delete; + LaunchSection& operator=(const LaunchSection&) = delete; +private: + bool held_ = false; +}; + // `argvEntries` is `argvCount` NUL-terminated strings; `envEntries` is // `envCount` "KEY=VALUE" strings applied on top of the current environment. // `cwd` may be null. A non-positive `deadlineMs` is rejected with @@ -173,6 +210,13 @@ void background_stop(long long group, long long graceMs); // Fixed capacity and no allocation: the handler reads this array and may not // allocate. Registering past capacity fails loudly rather than silently // dropping a group, because a dropped group is an orphan nobody will find. +// +// SAFE FOR CONCURRENT CALLERS. Children are started from several threads at once +// (a workspace's build programs are compiled concurrently), so claiming and +// releasing a slot is serialized by a mutex, and two threads can no longer claim +// the same slot. The handler takes no lock: it reads slots that are written one +// `sig_atomic_t` at a time. The capacity is 256, well above any job count, since +// one group is held per running child. void guard_group_on_signal(long long group); void unguard_group(long long group); void clear_group_guard(); @@ -262,8 +306,10 @@ DeadlineRun capture_with_deadline(const char* const* argvEntries, cargv.push_back(nullptr); const bool capture = (sink != nullptr); + // From the pipe to the parent's close of its write end: see LaunchSection. + LaunchSection launch; int fds[2] = {-1, -1}; - if (capture && ::pipe(fds) != 0) return out; + if (capture && make_pipe(fds) != 0) return out; posix_spawn_file_actions_t fa; ::posix_spawn_file_actions_init(&fa); @@ -292,6 +338,7 @@ DeadlineRun capture_with_deadline(const char* const* argvEntries, ::posix_spawnattr_destroy(&attr); ::posix_spawn_file_actions_destroy(&fa); if (capture) ::close(fds[1]); + launch.release(); if (sp != 0) { out.spawn_error = sp; if (capture) ::close(fds[0]); return out; } if (ownGroup) guard_group_on_signal(pid); @@ -364,8 +411,14 @@ namespace { // Read by a signal handler, so `volatile sig_atomic_t` and nothing else: the // handler may run between any two instructions and may not lock, allocate, or // call anything that is not async-signal-safe. 0 means "nothing to clean up". -constexpr int kMaxGuardedGroups = 8; +constexpr int kMaxGuardedGroups = 256; volatile sig_atomic_t g_guardedGroups[kMaxGuardedGroups] = {}; +// Serializes the WRITERS of the registry and of the handlers it installs. The +// handler never takes it. +std::mutex g_guardMutex; + +// The platforms without `pipe2` share one section across every child start. +std::mutex g_launchMutex; // The terminal whose mode the handler restores (-1: none), and that mode. volatile sig_atomic_t g_terminalFd = -1; @@ -396,6 +449,32 @@ extern "C" void background_signal_handler(int sig) { } // namespace +int make_pipe(int fds[2]) { +#if defined(__linux__) + return ::pipe2(fds, O_CLOEXEC); +#else + if (::pipe(fds) != 0) return -1; + ::fcntl(fds[0], F_SETFD, FD_CLOEXEC); + ::fcntl(fds[1], F_SETFD, FD_CLOEXEC); + return 0; +#endif +} + +LaunchSection::LaunchSection() { +#if !defined(__linux__) + g_launchMutex.lock(); + held_ = true; +#endif +} + +LaunchSection::~LaunchSection() { release(); } + +void LaunchSection::release() { + if (!held_) return; + held_ = false; + g_launchMutex.unlock(); +} + BackgroundChild spawn_background(const char* const* argvEntries, unsigned long argvCount, const char* cwd, @@ -430,8 +509,14 @@ BackgroundChild spawn_background(const char* const* argvEntries, ::posix_spawnattr_setflags(&attr, POSIX_SPAWN_SETPGROUP); pid_t pid = 0; - const int sp = ::posix_spawnp(&pid, cargv[0], &fa, &attr, - cargv.data(), current_environ()); + int sp = 0; + { + // A child that holds no pipe of its own must still not be started in + // the window another thread's pipe is open in (see LaunchSection). + LaunchSection launch; + sp = ::posix_spawnp(&pid, cargv[0], &fa, &attr, + cargv.data(), current_environ()); + } ::posix_spawn_file_actions_destroy(&fa); ::posix_spawnattr_destroy(&attr); if (sp != 0) return out; @@ -490,11 +575,14 @@ void background_stop(long long group, long long graceMs) { void guard_group_on_signal(long long group) { if (group <= 0) return; - for (int i = 0; i < kMaxGuardedGroups; ++i) { - if (g_guardedGroups[i] == 0) { - g_guardedGroups[i] = static_cast(group); - install_handlers(); - return; + { + std::lock_guard lock(g_guardMutex); + for (int i = 0; i < kMaxGuardedGroups; ++i) { + if (g_guardedGroups[i] == 0) { + g_guardedGroups[i] = static_cast(group); + install_handlers(); + return; + } } } // Out of slots. Say so rather than return silently: an unguarded group is @@ -510,6 +598,7 @@ void guard_group_on_signal(long long group) { // the guard an outer one still needs. void unguard_group(long long group) { if (group <= 0) return; + std::lock_guard lock(g_guardMutex); bool any = false; for (int i = 0; i < kMaxGuardedGroups; ++i) { if (g_guardedGroups[i] == static_cast(group)) @@ -525,6 +614,7 @@ void unguard_group(long long group) { } void clear_group_guard() { + std::lock_guard lock(g_guardMutex); for (int i = 0; i < kMaxGuardedGroups; ++i) g_guardedGroups[i] = 0; if (g_terminalFd >= 0) return; // the terminal's guard still needs the handler ::signal(SIGINT, SIG_DFL); @@ -533,12 +623,14 @@ void clear_group_guard() { } void guard_terminal_mode(int fd) { + std::lock_guard lock(g_guardMutex); if (fd < 0 || ::tcgetattr(fd, &g_terminalMode) != 0) return; g_terminalFd = fd; install_handlers(); } void unguard_terminal_mode() { + std::lock_guard lock(g_guardMutex); if (g_terminalFd < 0) return; ::tcsetattr(g_terminalFd, TCSANOW, &g_terminalMode); g_terminalFd = -1; @@ -575,6 +667,10 @@ BackgroundChild spawn_background(const char* const*, unsigned long, } int background_running(long long, int*) { return -1; } void background_stop(long long, long long) {} +int make_pipe(int[2]) { return -1; } +LaunchSection::LaunchSection() {} +LaunchSection::~LaunchSection() {} +void LaunchSection::release() {} void guard_group_on_signal(long long) {} void unguard_group(long long) {} void clear_group_guard() {} diff --git a/modules/platform/src/windows/bounded_process.cppm b/modules/platform/src/windows/bounded_process.cppm index 3fdb7444..dceebbf2 100644 --- a/modules/platform/src/windows/bounded_process.cppm +++ b/modules/platform/src/windows/bounded_process.cppm @@ -82,6 +82,36 @@ struct DeadlineRun { int spawn_error = 0; }; +// ─── Starting a child is safe for concurrent callers ───────────────────── +// +// `CreateProcess` with `bInheritHandles = TRUE` hands a child EVERY inheritable +// handle of its parent, and a capture pipe's write end has to be inheritable for +// its own child. A child started by thread B while thread A holds that write end +// keeps it open for as long as it lives, and A's reader then waits for end of +// file until B's unrelated child exits. (`SetHandleInformation` cannot close the +// window: a handle is inheritable from its creation to the moment the parent +// closes it, and the start of the child lies between the two.) +// +// So every start on this platform that can inherit handles, the ones below and +// the `_popen` calls of mcpp.platform.process, holds one process-wide section +// from the creation of the pipe to the parent's close of its write end. The +// alternative, `PROC_THREAD_ATTRIBUTE_HANDLE_LIST`, would restrict what OUR +// launchers pass and leave `_popen` and any third party passing everything; the +// section is one rule for all of them. +// +// On another platform the class exists and does nothing. +class LaunchSection { +public: + LaunchSection(); + ~LaunchSection(); + // Leaves the section before the destructor does. + void release(); + LaunchSection(const LaunchSection&) = delete; + LaunchSection& operator=(const LaunchSection&) = delete; +private: + bool held_ = false; +}; + // Receives stdout+stderr as it arrives. Called on the calling thread only. // // A NULL sink means "do not capture": the child inherits the caller's stdio and @@ -169,6 +199,10 @@ void background_stop(unsigned long long job, unsigned long long process, // A REGISTRY AND NOT ONE SLOT, for the reason the POSIX peer gives: a spanning // `[hooks]` command and the build's own ninja are guarded at the same time, and // a single slot lets the second registration disarm the first. +// +// SAFE FOR CONCURRENT CALLERS, as the POSIX peer is: claiming and releasing a +// slot is serialized by a mutex and the console handler takes no lock. The +// capacity is 256. void guard_job_on_signal(unsigned long long job); void unguard_job(unsigned long long job); void clear_job_guard(); @@ -238,6 +272,9 @@ std::string environment_block(const char* const* envEntries, return block; } +// The one section every child start shares (see LaunchSection). +std::mutex g_launchMutex; + struct Handle { HANDLE h = nullptr; Handle() = default; @@ -252,6 +289,19 @@ struct Handle { } // namespace +LaunchSection::LaunchSection() { + g_launchMutex.lock(); + held_ = true; +} + +LaunchSection::~LaunchSection() { release(); } + +void LaunchSection::release() { + if (!held_) return; + held_ = false; + g_launchMutex.unlock(); +} + DeadlineRun capture_with_deadline(const char* commandLine, const char* const* envEntries, unsigned long envCount, @@ -273,6 +323,9 @@ DeadlineRun capture_with_deadline(const char* commandLine, sa.nLength = sizeof(sa); sa.bInheritHandle = TRUE; + // From the creation of the pipe to the parent's close of its write end: no + // other child may be started while this pipe is inheritable. + LaunchSection launch; Handle readEnd, writeEnd; if (capture) { if (!::CreatePipe(&readEnd.h, &writeEnd.h, &sa, 0)) return out; @@ -350,6 +403,7 @@ DeadlineRun capture_with_deadline(const char* commandLine, // The parent must drop its copy of the write end or the pipe never reaches // EOF, even after every child has exited. writeEnd.reset(); + launch.release(); const auto started = std::chrono::steady_clock::now(); const auto until = deadlineMs > 0 @@ -414,8 +468,11 @@ namespace { // Read by a console control handler, which runs on a thread of the OS's // choosing. Only the handle is shared, and closing a job handle is atomic from // the caller's point of view. -constexpr int kMaxGuardedJobs = 8; +constexpr int kMaxGuardedJobs = 256; volatile unsigned long long g_guardedJobs[kMaxGuardedJobs] = {}; +// Serializes the WRITERS of the registry and of the handler it installs; the +// console handler never takes it. +std::mutex g_guardMutex; BOOL WINAPI background_console_handler(DWORD) { // TerminateJobObject, not CloseHandle: this handler races `background_stop` @@ -477,12 +534,19 @@ BackgroundChild spawn_background(const char* commandLine, // CREATE_NEW_PROCESS_GROUP is the peer of POSIX_SPAWN_SETPGROUP: the child // stops receiving the console's Ctrl-C, which is what makes the guard // below necessary and what stops a stray Ctrl-C from half-killing the tree. - const BOOL ok = ::CreateProcessA( - nullptr, cmdBuf.data(), nullptr, nullptr, - /*bInheritHandles=*/TRUE, - CREATE_SUSPENDED | CREATE_NEW_PROCESS_GROUP - | (inheritStdio ? 0u : CREATE_NO_WINDOW), - nullptr, (cwd && *cwd) ? cwd : nullptr, &si, &pi); + BOOL ok = FALSE; + { + // This child inherits every inheritable handle, another thread's + // capture pipe included, unless it is started outside the window that + // pipe is open in (see LaunchSection). + LaunchSection launch; + ok = ::CreateProcessA( + nullptr, cmdBuf.data(), nullptr, nullptr, + /*bInheritHandles=*/TRUE, + CREATE_SUSPENDED | CREATE_NEW_PROCESS_GROUP + | (inheritStdio ? 0u : CREATE_NO_WINDOW), + nullptr, (cwd && *cwd) ? cwd : nullptr, &si, &pi); + } if (!ok) { out.refused = ::GetLastError(); if (job) ::CloseHandle(job); @@ -555,19 +619,23 @@ void background_stop(unsigned long long job, unsigned long long process, void guard_job_on_signal(unsigned long long job) { if (!job) return; - for (int i = 0; i < kMaxGuardedJobs; ++i) { - if (g_guardedJobs[i] == 0) { - g_guardedJobs[i] = job; - ::SetConsoleCtrlHandler(background_console_handler, TRUE); - return; + { + std::lock_guard lock(g_guardMutex); + for (int i = 0; i < kMaxGuardedJobs; ++i) { + if (g_guardedJobs[i] == 0) { + g_guardedJobs[i] = job; + ::SetConsoleCtrlHandler(background_console_handler, TRUE); + return; + } } } - std::fputs("mcpp: internal: more than 8 concurrently guarded jobs; the " + std::fputs("mcpp: internal: more than 256 concurrently guarded jobs; the " "newest is NOT guarded and may outlive mcpp\n", stderr); } void unguard_job(unsigned long long job) { if (!job) return; + std::lock_guard lock(g_guardMutex); bool any = false; for (int i = 0; i < kMaxGuardedJobs; ++i) { if (g_guardedJobs[i] == job) g_guardedJobs[i] = 0; @@ -577,6 +645,7 @@ void unguard_job(unsigned long long job) { } void clear_job_guard() { + std::lock_guard lock(g_guardMutex); for (int i = 0; i < kMaxGuardedJobs; ++i) g_guardedJobs[i] = 0; ::SetConsoleCtrlHandler(background_console_handler, FALSE); } @@ -599,6 +668,10 @@ DeadlineRun capture_with_deadline(const char*, const char* const*, unsigned long return {}; } +LaunchSection::LaunchSection() {} +LaunchSection::~LaunchSection() {} +void LaunchSection::release() {} + BackgroundChild spawn_background(const char*, const char*, int) { return {}; } int background_running(unsigned long long, int*) { return -1; } void background_stop(unsigned long long, unsigned long long, long long) {} diff --git a/modules/platform/tests/test_process_concurrent_children.cpp b/modules/platform/tests/test_process_concurrent_children.cpp new file mode 100644 index 00000000..69f2d33d --- /dev/null +++ b/modules/platform/tests/test_process_concurrent_children.cpp @@ -0,0 +1,105 @@ +#include + +#if !defined(_WIN32) +#include +#include +#endif + +import std; +import mcpp.platform.process; +import mcpp.platform.unix.bounded_process; + +// SUBSYSTEM-LEVEL, and the contract is B2-0 of the member-selection and +// build-program-cost plan (#748): the launcher is safe for concurrent callers. +// +// A workspace's build programs are compiled together, so several threads start +// children at the same moment. A child inherits every descriptor (every +// inheritable handle on Windows) its parent holds, so a child started by one +// thread while another thread's capture pipe is open keeps that pipe's write end +// open for as long as it lives. The other thread's reader then waits for end of +// file until an unrelated child exits. + +namespace proc = mcpp::platform::process; + +namespace { + +using Clock = std::chrono::steady_clock; + +#if defined(_WIN32) +// `hostname` exits at once on every Windows; `ping -n 2` answers the second +// time one second after the first, which is the slowest child this needs. +const std::vector kQuick{"hostname"}; +const std::vector kSlow{"ping", "-n", "2", "127.0.0.1"}; +constexpr auto kSlowFor = std::chrono::milliseconds(1000); +constexpr int kRounds = 6; +#else +const std::vector kQuick{"/bin/sh", "-c", "echo quick"}; +const std::vector kSlow{"/bin/sh", "-c", "sleep 0.5"}; +constexpr auto kSlowFor = std::chrono::milliseconds(500); +constexpr int kRounds = 12; +#endif + +struct Outcome { + Clock::time_point end{}; + int exit_code = -1; + std::string output; +}; + +// Runs `argv` once the gate opens and records when its reader returned. +Outcome run_when_open(const std::atomic& gate, const std::vector& argv) { + while (!gate.load(std::memory_order_acquire)) std::this_thread::yield(); + auto r = proc::capture_exec(argv); + return {Clock::now(), r.exit_code, std::move(r.output)}; +} + +} // namespace + +// Two children started at once from two threads, one of which exits at once and +// one of which sleeps: the reader of the first returns when the first child +// exits, not when the second does. +// +// The window a leaked descriptor needs is the few hundred microseconds between +// the creation of a pipe and the parent's close of its write end, so one round +// cannot be expected to fall inside it. Each round releases both threads at one +// instant, which makes an overlap likely, and the rounds are repeated. With the +// launcher as it was, an overlap turns the quick reader's return into the slow +// child's exit; with it fixed, no round can. +TEST(ConcurrentChildren, AReaderReturnsWhenItsOwnChildExits) { + for (int round = 0; round < kRounds; ++round) { + std::atomic gate{false}; + Outcome quick, slow; + std::thread tSlow([&] { slow = run_when_open(gate, kSlow); }); + std::thread tQuick([&] { quick = run_when_open(gate, kQuick); }); + gate.store(true, std::memory_order_release); + tQuick.join(); + tSlow.join(); + + ASSERT_EQ(quick.exit_code, 0) << "round " << round; + ASSERT_EQ(slow.exit_code, 0) << "round " << round; + // Each reader is handed its own child's output and nothing else. + EXPECT_NE(quick.output.find("quick"), std::string::npos) << quick.output; + // The quick child was gone long before the slow one. Half the slow + // child's life is the margin: it does not depend on how fast the + // machine starts a process, only on the two readers not sharing a pipe. + const auto gap = std::chrono::duration_cast( + slow.end - quick.end); + EXPECT_GT(gap, kSlowFor / 2) + << "round " << round << ": the reader of the child that exited at once " + << "returned only " << gap.count() << " ms before the slow child did; " + << "it waited for a child another thread started"; + } +} + +#if !defined(_WIN32) +// The mechanism behind the test above, stated directly. A pipe the launcher +// creates is close-on-exec on both ends, so the only descriptors a child can +// inherit are the ones `posix_spawn` duplicates onto its standard streams. +TEST(ConcurrentChildren, APipeTheLauncherCreatesClosesOnExec) { + int fds[2] = {-1, -1}; + ASSERT_EQ(mcpp::platform::unixproc::make_pipe(fds), 0); + EXPECT_TRUE(::fcntl(fds[0], F_GETFD) & FD_CLOEXEC) << "the read end is inheritable"; + EXPECT_TRUE(::fcntl(fds[1], F_GETFD) & FD_CLOEXEC) << "the write end is inheritable"; + ::close(fds[0]); + ::close(fds[1]); +} +#endif From 380a6c9605bb051940031f1074bd9c0c1bea997a Mon Sep 17 00:00:00 2001 From: speak-agent <248744407+speak-agent@users.noreply.github.com> Date: Thu, 1 Oct 2026 01:32:24 +0800 Subject: [PATCH 03/26] feat: one member selection for build, test, emit build-database and run; a repeated -p selects every member it names, --exclude removes members, and test plans a selection once per configuration group Replace workspace_fanout_members and workspace_selection with mcpp.cli.selection: a set of members in [workspace] members order, refusals before planning (unknown, ambiguous, --exclude with -p, unknown --exclude, everything excluded). -p is repeatable on build, test, run and emit build-database; run refuses a second -p naming every member. mcpp test over several members plans once per configuration group with each member's tests in member_targets, builds Phase A and the test goals once, then runs each member's tests in member order, continuing past a member whose tests, package or plan fail. A member's tests run with that member's runtime directories. The stream gains a group_build record and a build_group field; build_ms is the group's build wall time. --- src/build/execute.cppm | 1178 +++++++++++++++++++------- src/cli.cppm | 26 +- src/cli/cmd_build.cppm | 282 +++--- src/cli/selection.cppm | 186 ++++ tests/unit/test_member_selection.cpp | 274 ++++++ 5 files changed, 1498 insertions(+), 448 deletions(-) create mode 100644 src/cli/selection.cppm create mode 100644 tests/unit/test_member_selection.cpp diff --git a/src/build/execute.cppm b/src/build/execute.cppm index be6b92ab..badc6fb8 100644 --- a/src/build/execute.cppm +++ b/src/build/execute.cppm @@ -669,9 +669,13 @@ compute_subos_env(const mcpp::build::BuildPlan& plan) { // tell an engine that states "nothing" from one that predates the variable. constexpr std::string_view kRuntimeFilesEnv = "MCPP_RUNTIME_FILES"; +// `owner`, for a plan of several workspace members, is the member whose +// artifact this is: the shared libraries another member's targets link are not +// this artifact's files. std::vector> runtime_files_for(const mcpp::build::BuildContext& ctx, - const std::filesystem::path& artifact) { + const std::filesystem::path& artifact, + std::string_view owner = {}) { std::vector> out; const auto artifactDir = artifact.parent_path().lexically_normal(); const auto artifactNorm = artifact.lexically_normal(); @@ -689,6 +693,7 @@ runtime_files_for(const mcpp::build::BuildContext& ctx, for (auto const& d : mcpp::build::compute_flags(ctx.plan).runtimeDeploy) add(d.dest); for (auto const& lu : ctx.plan.linkUnits) { if (lu.kind != mcpp::build::LinkUnit::SharedLibrary) continue; + if (!owner.empty() && !lu.memberOf.empty() && lu.memberOf != owner) continue; add(lu.output); for (auto const& alias : lu.runtimeAliases) add(alias); } @@ -2552,10 +2557,42 @@ export struct TestRunSummary { // `notRun` so the workspace total cannot add a stated build-only result to // a run that mcpp could not perform. int built = 0; - long long buildMs = 0; // Phase A + bulk pass + per-test drives + // The wall time this member's tests waited for their build. Phase A + bulk + // pass + per-test drives for a member planned alone; for a member planned + // with others, the build of its whole group (`buildGroup`), plus its own + // per-test drives. + long long buildMs = 0; long long runMs = 0; // the test binaries' own execution long long elapsedMs = 0; // wall clock for the whole member bool packageError = false; // Phase A failed: no test ever ran + // The configuration group whose one build this member's tests waited for, + // when `mcpp test` planned several members together; -1 for a member + // planned and built alone. Members of one group report the same + // `buildMs`, so a consumer that sums it over members deduplicates by this + // number. + int buildGroup = -1; +}; + +// One selected member of a `mcpp test` over several members, as the command +// layer found it: the path `[workspace] members` spells, and what discovery +// read from the member's own directory (two members may each have a +// `tests/main.cpp`). +export struct WorkspaceTestMember { + std::string path; + std::vector targets; + // Discovery failed: the member fails alone, with this message, and the + // others are planned without it. + std::string error; + // The line a member with no tests reports, naming where discovery looked. + std::string noTests; +}; + +// What the command layer reports around each member's run. `begin` is asked +// before a member's tests start, and a member it answers false for is not run +// (`--workspace-timeout`); `end` receives the member's exit status and summary. +export struct WorkspaceTestHooks { + std::function begin; + std::function end; }; // Minimal JSON string escaping for the --message-format json records. Same @@ -2580,127 +2617,148 @@ static std::string test_json_escape(std::string_view s) { return out; } -// `mcpp test` driver: discover tests/**/*.cpp, synthesize targets, build -// with dev-deps, run each test binary, summarize. -export int run_tests(std::span passthrough, - BuildOverrides overrides = {}, - TestOptions testOpts = {}, - TestRunSummary* summaryOut = nullptr) { - const bool json = (testOpts.format == TestMessageFormat::Json); - // The member this call is scoped to (empty outside a workspace). Threaded - // into every JSON record so a `--workspace` stream can be attributed: a - // bare test name is ambiguous the moment two members both have a `smoke`. - const std::string memberName = overrides.package_filter; - TestRunSummary summary; - struct SummaryWriter { - TestRunSummary* out; const TestRunSummary* src; - ~SummaryWriter() { if (out) *out = *src; } - } summaryWriter{summaryOut, &summary}; - // Wall clock for the WHOLE member, started before Phase A. The old `t0` - // sat after Phase A and the bulk pass, so `finished in` reported only the - // per-test loop: measured on one member, 6.53s printed against 93.5s - // actual — a 14x understatement, and worst exactly on the build-heavy - // members where the number matters. - auto tMember = std::chrono::steady_clock::now(); - auto member_ms = [&tMember] { - return std::chrono::duration_cast( - std::chrono::steady_clock::now() - tMember).count(); - }; - // JSON mode: stdout carries NDJSON only. All ui::status/info lines print - // to stdout, so silence them wholesale; errors already go to stderr. - if (json) mcpp::ui::set_quiet(true); - // The report covers the planning and the package's own build (Phase A); - // it is closed before the tests' own lines. - mcpp::build::progress::open(mcpp::log::is_verbose()); +// A test's result, as the records and the summary read it. +struct TestResult { + std::string name; + // `NotRun` (#544): built, and not executed — the host cannot load the + // artifact, or the declared runner could not be found or started. + // Reporting that as `RunFail (exit 127)` states that the test ran and + // returned 127, which is false and indistinguishable from a missing + // program; reporting it as a pass would be read as one. + // `Built` (`--no-run`): compiled and linked, and deliberately not + // executed. Distinct from `NotRun`, which means mcpp tried and could + // not --- the difference is whether anything was left unanswered. + enum class St { Pass, CompileFail, RunFail, NotRun, Built } status; + int exitCode = 0; + std::string compileOutput; + std::string runOutput; + long long durationMs = 0; // build+run wall time for THIS test + bool timedOut = false; // killed by --timeout + std::string reason; // NotRun only: why, in one sentence +}; - auto root = mcpp::project::find_manifest_root(std::filesystem::current_path()); - if (!root) { - mcpp::ui::error("no mcpp.toml found in current directory or any parent"); - return 2; - } +// Streaming NDJSON: one record per test, emitted as it finishes — a +// consumer (e.g. the d2x provider) sees progress live, and a crash +// mid-run still leaves the completed records on stdout. +static void emit_test_json(const std::string& memberName, const TestResult& r) { + const char* st = r.status == TestResult::St::Pass ? "pass" + : r.status == TestResult::St::CompileFail ? "compile_fail" + : r.status == TestResult::St::NotRun ? "not_run" + : r.status == TestResult::St::Built ? "built" + : "run_fail"; + std::string signal = (r.exitCode > 128 && r.exitCode < 128 + 65) + ? std::to_string(r.exitCode - 128) : "null"; + std::println("{{\"member\":\"{}\",\"test\":\"{}\",\"status\":\"{}\"," + "\"exit_code\":{},\"signal\":{}," + "\"duration_ms\":{},\"timed_out\":{}," + "\"compile_output\":\"{}\",\"run_output\":\"{}\"," + "\"reason\":\"{}\"}}", + test_json_escape(memberName), + test_json_escape(r.name), st, r.exitCode, signal, r.durationMs, + r.timedOut ? "true" : "false", + test_json_escape(r.compileOutput), test_json_escape(r.runOutput), + test_json_escape(r.reason)); + std::fflush(stdout); +} - auto discovered = mcpp::build::discover_test_targets( - *root, overrides.package_filter); - if (!discovered) { - mcpp::ui::error(discovered.error()); - return 2; - } - auto testRoot = discovered->packageRoot; - auto testTargets = std::move(discovered->targets); - if (testTargets.empty()) { - // Names where it looked when the manifest chose the place, so that a - // glob that matches nothing is not read as a project without tests. - if (discovered->discoverDeclared) { - std::string globs; - for (auto const& g : discovered->discover) - globs += std::format("{}\"{}\"", globs.empty() ? "" : ", ", g); - std::println("no tests found ([test] discover = [{}])", globs); - } else { - std::println("no tests found in tests/"); - } - return 0; - } - // --list: enumerate (filtered) tests and stop — no toolchain resolution, - // no build. Names/paths come straight from discovery, so this also works - // on tests that do not currently compile. - if (testOpts.list) { - std::size_t total = 0; - for (auto& t : testTargets) { - if (!testOpts.filter.empty() - && t.name.find(testOpts.filter) == std::string::npos) continue; - ++total; - auto abs = std::filesystem::absolute(testRoot / t.main) - .lexically_normal().generic_string(); - if (json) - std::println("{{\"member\":\"{}\",\"test\":\"{}\",\"main\":\"{}\"}}", - test_json_escape(memberName), - test_json_escape(t.name), test_json_escape(abs)); - else - std::println("{}", t.name); - } - if (json) { - std::println("{{\"summary\":{{\"total\":{}}}}}", total); - std::fflush(stdout); +// An edge ninja recorded in `.ninja_log`: what it made, and how long it took. +struct NinjaEdge { + std::string output; + long long ms = 0; +}; + +// The edges ninja appended to `log` after byte `from`: the ones the drives +// since that point ran. The log accumulates across invocations, so an edge +// that was not rebuilt has an old entry that says how long it took once; the +// offset taken before the first drive is what tells this run's edges from +// those. A log that ninja rewrote (recompaction) is shorter than the offset +// and reads as no edges, which is an absent measurement and not a zero. +// +// Format (ninja log v5): `start_ms TAB end_ms TAB mtime TAB output TAB hash`. +static std::vector ninja_edges_since(const std::filesystem::path& log, + std::uintmax_t from) { + std::vector edges; + std::error_code ec; + const auto size = std::filesystem::file_size(log, ec); + if (ec || size <= from) return edges; + std::ifstream is(log, std::ios::binary); + if (!is) return edges; + is.seekg(static_cast(from)); + std::string line; + while (std::getline(is, line)) { + std::array f{}; + std::size_t n = 0, b = 0; + std::string_view v = line; + while (n < f.size()) { + const auto t = v.find('\t', b); + f[n++] = v.substr(b, t == std::string_view::npos ? std::string_view::npos : t - b); + if (t == std::string_view::npos) break; + b = t + 1; } - return 0; + if (n < 4) continue; + long long start = 0, end = 0; + auto [p1, e1] = std::from_chars(f[0].data(), f[0].data() + f[0].size(), start); + auto [p2, e2] = std::from_chars(f[1].data(), f[1].data() + f[1].size(), end); + if (e1 != std::errc{} || e2 != std::errc{} || end < start) continue; + edges.push_back({std::string(f[3]), end - start}); } + return edges; +} - // 3. prepare_build with dev-deps enabled + synthetic targets. - // A test binary is executed, so the run tier applies here exactly as it - // does to `mcpp run` — `[xlings.workspace]` has no separate `test` tier - // because there is no separate need. - overrides.will_run = true; - auto ctx = prepare_build(/*print_fp=*/false, - /*includeDevDeps=*/true, - std::move(testTargets), - std::move(overrides)); - if (!ctx) { mcpp::ui::error(ctx.error()); return 2; } +// A planned test build, and what the tests run against it need: the plan, the +// backend that drives it, how long planning and the build took, and how the +// test binaries are executed. One per configuration group of a `mcpp test` +// over several members; `run_tests` holds one for its one member. +struct TestBuild { + std::optional ctx; + std::unique_ptr backend; + long long prepareMs = 0; + // Phase A, the bulk pass, and any attribution drive. + long long buildMs = 0; + bool bulkBuiltEverything = false; + std::filesystem::path ninjaLog; + std::uintmax_t logFrom = 0; + + // How the test binaries are executed, resolved once for the build + // (#544): every test of one build shares a target. + RunnerChoice runnerChoice; + std::vector runnerTmpl; + // Non-empty ⇒ no test is spawned; every one is reported NotRun with it. + std::string invocationNotRunReason; + // Non-zero: there is nothing to execute the tests with, and this is the + // exit status every member of the build returns. + int runnerFatal = 0; + // Set by the first worker whose spawn the kernel refused; every worker + // checks it before spawning. Workers already past the check may be + // refused the same way — harmless, a refused spawn has no side effects — + // and each such result is NotRun, not RunFail. The reason is printed once. + std::atomic hostCannotRun{false}; + std::string hostCannotRunReason; +}; - // Filter guard. The filter selects at the build/run stage ONLY — the plan - // above always contains every test, so build.ninja and - // compile_commands.json stay complete (clangd depends on the latter; a - // filtered run must not clobber it down to one entry). - auto filter_match = [&](const mcpp::build::LinkUnit& lu) { - return lu.kind == mcpp::build::LinkUnit::TestBinary - && (testOpts.filter.empty() - || lu.targetName.find(testOpts.filter) != std::string::npos); - }; - if (!testOpts.filter.empty()) { - bool any = false; - for (auto& lu : ctx->plan.linkUnits) - if (filter_match(lu)) { any = true; break; } - if (!any) { - if (json) - std::println("{{\"error\":\"no-tests-matched\",\"filter\":\"{}\"}}", - test_json_escape(testOpts.filter)); - mcpp::ui::error(std::format("no tests match '{}'", testOpts.filter)); - return 2; - } - } +// Reads `fn` with the plan describing one workspace member (workspace design +// 2026-09-29 §15): the member's link group is exchanged into the plan's own +// fields for the call, so what a member's tests run against -- its runtime +// directories, the files its runtime needs -- is the member's closure's, and +// no other member's. Outside a workspace plan, and for an empty `owner`, the +// plan is read as it is. The exchange is undone before the call returns: a +// drive emits the plan, and must see its own fields. +template +static void as_member(BuildContext& ctx, std::string_view owner, F&& fn) { + BuildPlan::LinkGroup* group = nullptr; + if (!owner.empty()) + for (auto& g : ctx.plan.linkGroups) + if (!g.linkOnly && g.member == owner) { group = &g; break; } + if (!group) { fn(); return; } + swap_link_group(ctx.plan, *group); + fn(); + swap_link_group(ctx.plan, *group); +} - // 4. "Compiling test_X (test)" lines for the test binaries. +// The "Compiling " lines the tests' own lines follow. +static void test_announce(const BuildContext& ctx) { std::map cachedUnits; - for (auto& dep : ctx->cachedDeps) cachedUnits[dep.name] = dep.units; + for (auto& dep : ctx.cachedDeps) cachedUnits[dep.name] = dep.units; auto announce = [&](const std::string& name, const mcpp::manifest::DependencySpec& spec, std::string_view suffix) { @@ -2716,184 +2774,235 @@ export int run_tests(std::span passthrough, } }; std::set announced; - announced.insert(ctx->manifest.package.name); + announced.insert(ctx.manifest.package.name); mcpp::ui::status("Compiling", std::format("{} v{} (.)", - ctx->manifest.package.name, ctx->manifest.package.version)); - for (auto& [name, spec] : ctx->manifest.dependencies) { + ctx.manifest.package.name, ctx.manifest.package.version)); + for (auto& [name, spec] : ctx.manifest.dependencies) { if (announced.contains(name)) continue; announced.insert(name); announce(name, spec, ""); } - for (auto& [name, spec] : ctx->manifest.devDependencies) { + for (auto& [name, spec] : ctx.manifest.devDependencies) { if (announced.contains(name)) continue; announced.insert(name); announce(name, spec, " (dev)"); } - // List test binaries. - // (Per-test "Compiling" lines print in Phase B, interleaved with each - // test's own result — announcing them all up front separated the three - // pieces of one test's story across the whole output.) - - // 5. Two-phase build. Phase A: package-level artifacts (everything that - // is not a test binary — libs, deps). A failure here is the PACKAGE's - // fault, not any single test's: report it as a build error, never as - // N red tests. Phase B (below): each test is built as its own ninja - // goal, so a compile failure is attributed to exactly that test and - // the rest still build and run. - struct TestResult { - std::string name; - // `NotRun` (#544): built, and not executed — the host cannot load the - // artifact, or the declared runner could not be found or started. - // Reporting that as `RunFail (exit 127)` states that the test ran and - // returned 127, which is false and indistinguishable from a missing - // program; reporting it as a pass would be read as one. - // `Built` (`--no-run`): compiled and linked, and deliberately not - // executed. Distinct from `NotRun`, which means mcpp tried and could - // not --- the difference is whether anything was left unanswered. - enum class St { Pass, CompileFail, RunFail, NotRun, Built } status; - int exitCode = 0; - std::string compileOutput; - std::string runOutput; - long long durationMs = 0; // build+run wall time for THIS test - bool timedOut = false; // killed by --timeout - std::string reason; // NotRun only: why, in one sentence - }; - std::vector results; - - // Streaming NDJSON: one record per test, emitted as it finishes — a - // consumer (e.g. the d2x provider) sees progress live, and a crash - // mid-run still leaves the completed records on stdout. - auto emit_json = [&](const TestResult& r) { - if (!json) return; - const char* st = r.status == TestResult::St::Pass ? "pass" - : r.status == TestResult::St::CompileFail ? "compile_fail" - : r.status == TestResult::St::NotRun ? "not_run" - : r.status == TestResult::St::Built ? "built" - : "run_fail"; - std::string signal = (r.exitCode > 128 && r.exitCode < 128 + 65) - ? std::to_string(r.exitCode - 128) : "null"; - std::println("{{\"member\":\"{}\",\"test\":\"{}\",\"status\":\"{}\"," - "\"exit_code\":{},\"signal\":{}," - "\"duration_ms\":{},\"timed_out\":{}," - "\"compile_output\":\"{}\",\"run_output\":\"{}\"," - "\"reason\":\"{}\"}}", - test_json_escape(memberName), - test_json_escape(r.name), st, r.exitCode, signal, r.durationMs, - r.timedOut ? "true" : "false", - test_json_escape(r.compileOutput), test_json_escape(r.runOutput), - test_json_escape(r.reason)); - std::fflush(stdout); - }; - - auto backend = mcpp::build::make_ninja_backend(); +} - // Phase A goal set: every shared prerequisite — all package/dep compile - // units EXCEPT the tests' own main TUs, plus any non-test link outputs. - // In test mode the lib link unit is skipped entirely (plan.cppm), so the - // package's module objects are the only place shared breakage can show - // up; building them here is what keeps a broken src/ module a PACKAGE - // error instead of N identical per-test compile failures. +// Phase A goal set: every shared prerequisite — all package/dep compile +// units EXCEPT the tests' own main TUs, plus any non-test link outputs. +// In test mode the lib link unit is skipped entirely (plan.cppm), so the +// package's module objects are the only place shared breakage can show +// up; building them here is what keeps a broken src/ module a PACKAGE +// error instead of N identical per-test compile failures. +static std::vector test_package_goals(const BuildContext& ctx) { std::set testMains; - for (auto& lu : ctx->plan.linkUnits) + for (auto& lu : ctx.plan.linkUnits) if (lu.kind == mcpp::build::LinkUnit::TestBinary && lu.entryMain) testMains.insert(*lu.entryMain); - std::vector pkgTargets; - for (auto& cu : ctx->plan.compileUnits) + std::vector goals; + for (auto& cu : ctx.plan.compileUnits) if (!testMains.contains(cu.source)) - pkgTargets.push_back(cu.object.generic_string()); - for (auto& lu : ctx->plan.linkUnits) + goals.push_back(cu.object.generic_string()); + for (auto& lu : ctx.plan.linkUnits) if (lu.kind != mcpp::build::LinkUnit::TestBinary) - pkgTargets.push_back(lu.output.generic_string()); - mcpp::build::progress::programs_done(); - if (!pkgTargets.empty()) { - mcpp::build::BuildOptions aOpts; - aOpts.ninjaTargets = pkgTargets; - aOpts.buildTimeoutSecs = static_cast(testOpts.buildTimeoutSecs); - // Phase A is the package's own build, and is reported as `mcpp build` - // reports one (build progress design 2026-09-29); the tests' own - // builds and runs below keep their per-test lines. - std::optional phaseReport; - if (!json && !mcpp::ui::is_quiet()) { - phaseReport.emplace(ctx->outputDir); - aOpts.progress = &*phaseReport; - } - auto tPhaseA = std::chrono::steady_clock::now(); - auto a = backend->build(ctx->plan, aOpts); - summary.buildMs += std::chrono::duration_cast( - std::chrono::steady_clock::now() - tPhaseA).count(); - if (!a) { - summary.packageError = true; - summary.elapsedMs = member_ms(); - std::fflush(stdout); - if (json) - std::println("{{\"error\":\"package\",\"compile_output\":\"{}\"}}", - test_json_escape(a.error().diagnosticOutput)); - // Surface the compiler/linker stderr (parity with run_build_plan) — - // otherwise `mcpp test` failures show only "build failed" with no - // diagnostic, which is undebuggable (notably on CI). A failed step - // was reported when it failed. - if (!a.error().reported) mcpp::ui::error(a.error().message); - mcpp::ui::block(a.error().diagnosticOutput); - return 1; - } + goals.push_back(lu.output.generic_string()); + return goals; +} - // M3.2: populate BMI cache for deps that did NOT hit cache — deps - // are package-level artifacts, so this belongs right after Phase A. - for (auto& task : ctx->depsToPopulate) { - auto pr = mcpp::bmi_cache::populate_from(task.key, ctx->outputDir, task.artifacts); - if (!pr) { - mcpp::ui::warning(std::format( - "bmi cache populate failed for {}@{}: {}", - task.key.packageName, task.key.version, pr.error())); - } +// The part of Phase A that one member's tests need: the objects of the +// member's closure, which are the ones its test binaries link other than the +// tests' own main TUs. A member whose package does not build is found by +// building these alone, so that a group's other members still run. +static std::vector member_package_goals(const BuildContext& ctx, + std::string_view owner) { + std::set testMains; + for (auto& lu : ctx.plan.linkUnits) + if (lu.kind == mcpp::build::LinkUnit::TestBinary && lu.entryMain) + testMains.insert(*lu.entryMain); + std::set mainObjects; + for (auto& cu : ctx.plan.compileUnits) + if (testMains.contains(cu.source)) mainObjects.insert(cu.object); + std::set seen; + std::vector goals; + for (auto& lu : ctx.plan.linkUnits) { + if (lu.kind != mcpp::build::LinkUnit::TestBinary || lu.memberOf != owner) continue; + for (auto& o : lu.objects) + if (!mainObjects.contains(o) && seen.insert(o).second) + goals.push_back(o.generic_string()); + } + return goals; +} + +// Phase A, run against `tb` (see the note at its two callers): everything +// every test shares, built once. Nullopt when it built; else the failure, +// which the caller reports. Its wall time is added to the build's. +static std::optional test_phase_a(TestBuild& tb, const TestOptions& testOpts, + bool json) { + auto* ctx = &*tb.ctx; + auto& backend = tb.backend; + auto pkgTargets = test_package_goals(*ctx); + if (pkgTargets.empty()) return std::nullopt; + mcpp::build::BuildOptions aOpts; + aOpts.ninjaTargets = pkgTargets; + aOpts.buildTimeoutSecs = static_cast(testOpts.buildTimeoutSecs); + // Phase A is the package's own build, and is reported as `mcpp build` + // reports one (build progress design 2026-09-29); the tests' own + // builds and runs below keep their per-test lines. + std::optional phaseReport; + if (!json && !mcpp::ui::is_quiet()) { + phaseReport.emplace(ctx->outputDir); + aOpts.progress = &*phaseReport; + } + auto tPhaseA = std::chrono::steady_clock::now(); + auto a = backend->build(ctx->plan, aOpts); + tb.buildMs += std::chrono::duration_cast( + std::chrono::steady_clock::now() - tPhaseA).count(); + if (!a) return std::move(a.error()); + + // M3.2: populate BMI cache for deps that did NOT hit cache — deps + // are package-level artifacts, so this belongs right after Phase A. + for (auto& task : ctx->depsToPopulate) { + auto pr = mcpp::bmi_cache::populate_from(task.key, ctx->outputDir, task.artifacts); + if (!pr) { + mcpp::ui::warning(std::format( + "bmi cache populate failed for {}@{}: {}", + task.key.packageName, task.key.version, pr.error())); } + } + + // No "Finished test" line here: Phase A only built the shared + // prerequisites. Printing a success banner right before per-test + // failures read as a contradiction; the final summary carries timing. + return std::nullopt; +} - // No "Finished test" line here: Phase A only built the shared - // prerequisites. Printing a success banner right before per-test - // failures read as a contradiction; the final summary carries timing. +// 6. Phase B. First a single keep-going bulk build over every selected +// test goal — ninja parallelizes across tests and a failing test does +// not stop the rest (-k 0). The result is deliberately ignored: the +// per-test loop below re-drives each goal so a failure is attributed to +// exactly one test. +// +// ...but ONLY when this bulk build failed. A re-drive was assumed to be +// a near no-op, and it is not: a drive re-emits build.ninja, rewrites +// compile_commands.json, spawns ninja and re-validates the runtime +// closure. Measured on the 83-test suite AFTER the rule E fix, that is +// still ~39ms x 83 = 3.2s of a 5.3s hot run — spent re-asking a question +// the bulk build just answered for every test at once. +// +// `-k 0` means the bulk exit code is 0 IFF every selected goal built, so +// it carries exactly the information the loop was re-deriving. When it +// is non-zero the loop runs as before and each failure still names its +// own test. +// `keep` says which test binaries are goals. +template +static void test_bulk(TestBuild& tb, const TestOptions& testOpts, Keep&& keep) { + auto* ctx = &*tb.ctx; + auto& backend = tb.backend; + mcpp::build::BuildOptions bulk; + bulk.keepGoing = true; + bulk.buildTimeoutSecs = static_cast(testOpts.buildTimeoutSecs); + for (auto& lu : ctx->plan.linkUnits) + if (keep(lu)) + bulk.ninjaTargets.push_back(lu.output.generic_string()); + if (!bulk.ninjaTargets.empty()) { + auto tBulk = std::chrono::steady_clock::now(); + tb.bulkBuiltEverything = backend->build(ctx->plan, bulk).has_value(); + tb.buildMs += std::chrono::duration_cast( + std::chrono::steady_clock::now() - tBulk).count(); } - // The tests' own lines follow, as they always have. - mcpp::build::progress::close(); +} - // 6. Phase B. First a single keep-going bulk build over every selected - // test goal — ninja parallelizes across tests and a failing test does - // not stop the rest (-k 0). The result is deliberately ignored: the - // per-test loop below re-drives each goal so a failure is attributed to - // exactly one test. - // - // ...but ONLY when this bulk build failed. A re-drive was assumed to be - // a near no-op, and it is not: a drive re-emits build.ninja, rewrites - // compile_commands.json, spawns ninja and re-validates the runtime - // closure. Measured on the 83-test suite AFTER the rule E fix, that is - // still ~39ms x 83 = 3.2s of a 5.3s hot run — spent re-asking a question - // the bulk build just answered for every test at once. - // - // `-k 0` means the bulk exit code is 0 IFF every selected goal built, so - // it carries exactly the information the loop was re-deriving. When it - // is non-zero the loop runs as before and each failure still names its - // own test. - bool bulkBuiltEverything = false; - { - mcpp::build::BuildOptions bulk; - bulk.keepGoing = true; - bulk.buildTimeoutSecs = static_cast(testOpts.buildTimeoutSecs); - for (auto& lu : ctx->plan.linkUnits) - if (filter_match(lu)) - bulk.ninjaTargets.push_back(lu.output.generic_string()); - if (!bulk.ninjaTargets.empty()) { - auto tBulk = std::chrono::steady_clock::now(); - bulkBuiltEverything = backend->build(ctx->plan, bulk).has_value(); - summary.buildMs += std::chrono::duration_cast( - std::chrono::steady_clock::now() - tBulk).count(); - } +// How the test binaries are executed — the SAME runner `mcpp run` uses, +// resolved ONCE per build (#544). One read point, two callers; and +// one lookup, because every test of a build shares a target, so +// "the runner's program is not there" is a fact about the build and +// is reported once rather than once per test. +// +// Nothing else about the test model changes, and that is a measured +// result rather than a simplification: semihosting propagates the +// firmware's `main` return value to the emulator's exit code +// (`return 7` → qemu exits 7, verified), so "exit code is the verdict" +// holds under a runner exactly as it does on the host. +static void test_resolve_runner(TestBuild& tb, const TestOptions& testOpts, bool json) { + auto* ctx = &*tb.ctx; + tb.runnerChoice = choose_runner(*ctx, testOpts.noRunner); + auto& runnerChoice = tb.runnerChoice; + if (runnerChoice.ignored && !json) + mcpp::ui::info("note", std::format( + "--no-runner: ignoring the runner declared for {}", runnerChoice.tripleKey)); + if (runnerChoice.fromManifest && !json) + mcpp::ui::info("note", std::format( + "[target.{}].runner overrides the runner a dependency supplied", + runnerChoice.tripleKey)); + if (runnerChoice.freestanding && runnerChoice.tmpl.empty()) { + std::println(stderr, "error: {}", + mcpp::freestanding::no_runner_message(runnerChoice.tripleKey)); + tb.runnerFatal = 2; + return; + } + tb.runnerTmpl = runnerChoice.tmpl; + auto& runnerTmpl = tb.runnerTmpl; + if (!runnerTmpl.empty()) { + const char* pathEnv = std::getenv("PATH"); + auto found = mcpp::build::runner_lookup::locate( + runnerTmpl.front(), ctx->xlingsDepBinDirs, pathEnv ? pathEnv : ""); + if (found.program) runnerTmpl.front() = found.program->string(); + else tb.invocationNotRunReason = mcpp::build::runner_lookup::not_found_message( + runnerChoice.tripleKey, runnerTmpl.front(), found.searched); } +} - // Then build + run each test in sequence; collect results. +// One member's tests, against a build that exists: the per-test builds that +// the bulk pass did not answer, then the runs, then the member's summary. +// `owner` is the member's package name in the plan, which picks its test +// binaries out of a plan that holds several members' (empty: every test +// binary of the plan). `carriedMs` is the time the member has already spent +// on planning and building, which `elapsed_ms` counts; `summary.buildMs` +// arrives holding the build's wall time. +static int test_run_member(TestBuild& tb, const TestOptions& testOpts, + std::span passthrough, bool json, + const std::string& memberName, const std::string& owner, + long long carriedMs, TestRunSummary& summary) { + auto* ctx = &*tb.ctx; + auto& backend = tb.backend; + const auto tLoop = std::chrono::steady_clock::now(); + if (tb.runnerFatal) return tb.runnerFatal; + const auto& runnerChoice = tb.runnerChoice; + auto& runnerTmpl = tb.runnerTmpl; + auto& invocationNotRunReason = tb.invocationNotRunReason; + auto& hostCannotRun = tb.hostCannotRun; + auto& hostCannotRunReason = tb.hostCannotRunReason; + const bool bulkBuiltEverything = tb.bulkBuiltEverything; + // Filter guard. The filter selects at the build/run stage ONLY — the plan + // always contains every test, so build.ninja and compile_commands.json + // stay complete (clangd depends on the latter; a filtered run must not + // clobber it down to one entry). + auto filter_match = [&](const mcpp::build::LinkUnit& lu) { + return lu.kind == mcpp::build::LinkUnit::TestBinary + && (owner.empty() || lu.memberOf == owner) + && (testOpts.filter.empty() + || lu.targetName.find(testOpts.filter) != std::string::npos); + }; + std::vector results; + auto emit_json = [&](const TestResult& r) { + if (!json) return; + emit_test_json(memberName, r); + }; + + // The runtime of THIS member: in a plan of several members, the plan's own + // directories are the union over every member, and a member's tests are + // told about its closure's alone. auto runtimeEnvKey = mcpp::platform::env::runtime_library_path_key(); - auto runtimeEnvValue = mcpp::platform::env::prepend_path_list( - runtimeEnvKey, ctx->plan.runtimeLibraryDirs); + std::string runtimeEnvValue; + bool hasRuntimeDirs = false; + as_member(*ctx, owner, [&] { + runtimeEnvValue = mcpp::platform::env::prepend_path_list( + runtimeEnvKey, ctx->plan.runtimeLibraryDirs); + hasRuntimeDirs = !ctx->plan.runtimeLibraryDirs.empty(); + }); // Read once for the whole run rather than per test: it is one file, and // every test in a run belongs to the same subos. const auto subosEnv = compute_subos_env(ctx->plan); @@ -2905,7 +3014,7 @@ export int run_tests(std::span passthrough, // here with a dyld error that names neither the cause nor the platform — // so say it out loud rather than leaving the difference silent. if constexpr (mcpp::platform::is_macos) { - if (runtimeEnvKey.empty() && !ctx->plan.runtimeLibraryDirs.empty()) { + if (runtimeEnvKey.empty() && hasRuntimeDirs) { mcpp::diag::warning("test/runtime-path", "macOS does not inject a runtime library path for test binaries " "(DYLD_LIBRARY_PATH is deliberately not set); dependencies must be " @@ -2945,6 +3054,7 @@ export int run_tests(std::span passthrough, std::string name; std::vector argv; std::vector> env; + long long buildMs = 0; // this test's own edges in .ninja_log }; std::vector runnable; @@ -2955,47 +3065,6 @@ export int run_tests(std::span passthrough, // the terminal interleaves them line by line, which does not just look // untidy — it makes a failing assertion unattributable, and the whole // reason the per-test loop exists is attribution. - // How the test binaries are executed — the SAME runner `mcpp run` uses, - // resolved ONCE per invocation (#544). One read point, two callers; and - // one lookup, because every test of an invocation shares a target, so - // "the runner's program is not there" is a fact about the invocation and - // is reported once rather than once per test. - // - // Nothing else about the test model changes, and that is a measured - // result rather than a simplification: semihosting propagates the - // firmware's `main` return value to the emulator's exit code - // (`return 7` → qemu exits 7, verified), so "exit code is the verdict" - // holds under a runner exactly as it does on the host. - const auto runnerChoice = choose_runner(*ctx, testOpts.noRunner); - if (runnerChoice.ignored && !json) - mcpp::ui::info("note", std::format( - "--no-runner: ignoring the runner declared for {}", runnerChoice.tripleKey)); - if (runnerChoice.fromManifest && !json) - mcpp::ui::info("note", std::format( - "[target.{}].runner overrides the runner a dependency supplied", - runnerChoice.tripleKey)); - if (runnerChoice.freestanding && runnerChoice.tmpl.empty()) { - std::println(stderr, "error: {}", - mcpp::freestanding::no_runner_message(runnerChoice.tripleKey)); - return 2; - } - std::vector runnerTmpl = runnerChoice.tmpl; - // Non-empty ⇒ no test is spawned; every one is reported NotRun with it. - std::string invocationNotRunReason; - if (!runnerTmpl.empty()) { - const char* pathEnv = std::getenv("PATH"); - auto found = mcpp::build::runner_lookup::locate( - runnerTmpl.front(), ctx->xlingsDepBinDirs, pathEnv ? pathEnv : ""); - if (found.program) runnerTmpl.front() = found.program->string(); - else invocationNotRunReason = mcpp::build::runner_lookup::not_found_message( - runnerChoice.tripleKey, runnerTmpl.front(), found.searched); - } - // Set by the first worker whose spawn the kernel refused; every worker - // checks it before spawning. Workers already past the check may be - // refused the same way — harmless, a refused spawn has no side effects — - // and each such result is NotRun, not RunFail. The reason is printed once. - std::atomic hostCannotRun{false}; - std::string hostCannotRunReason; auto run_tests_now = [&](std::vector& list) { if (list.empty()) return; @@ -3104,7 +3173,7 @@ export int run_tests(std::span passthrough, if (!json) mcpp::ui::warning(reason); } if (!json) mcpp::ui::plain(std::format("{} ... not run", r.name)); - results.push_back({r.name, TestResult::St::NotRun, 0, {}, {}, ms, false, + results.push_back({r.name, TestResult::St::NotRun, 0, {}, {}, ms + r.buildMs, false, reason}); std::fflush(stdout); emit_json(results.back()); @@ -3114,18 +3183,18 @@ export int run_tests(std::span passthrough, if (!json) mcpp::ui::plain(std::format( "{} ... FAIL (timeout after {}s)", r.name, testOpts.timeoutSecs)); results.push_back({r.name, TestResult::St::RunFail, exitCode, {}, - runOutput, ms, true}); + runOutput, ms + r.buildMs, true}); } else if (exitCode == 0) { if (!json) mcpp::ui::plain(std::format( "{} ... ok ({:.2f}s)", r.name, static_cast(ms) / 1000.0)); results.push_back({r.name, TestResult::St::Pass, 0, {}, - runOutput, ms}); + runOutput, ms + r.buildMs}); } else { if (!json) mcpp::ui::plain(std::format( "{} ... FAIL (exit {}, {:.2f}s)", r.name, exitCode, static_cast(ms) / 1000.0)); results.push_back({r.name, TestResult::St::RunFail, exitCode, {}, - runOutput, ms}); + runOutput, ms + r.buildMs}); } // The captured output belongs directly under its own line, or // it is attributable to nothing. @@ -3153,6 +3222,19 @@ export int run_tests(std::span passthrough, std::chrono::steady_clock::now() - tRunPhase).count(); }; + // The build part of a test's duration is that test binary's own edges, its + // main TU and its link, among the ones ninja logged for this build. Read + // once when the bulk pass built every test; after each drive otherwise, + // since a drive logs its own. + std::map edgeMs; + auto read_edges = [&] { + edgeMs.clear(); + for (auto& e : ninja_edges_since(tb.ninjaLog, tb.logFrom)) edgeMs[e.output] += e.ms; + }; + read_edges(); + std::map objectOf; + for (auto& cu : ctx->plan.compileUnits) objectOf[cu.source] = cu.object; + for (auto& lu : ctx->plan.linkUnits) { if (!filter_match(lu)) continue; @@ -3200,6 +3282,11 @@ export int run_tests(std::span passthrough, } auto exe = ctx->outputDir / lu.output; + if (!bulkBuiltEverything) read_edges(); + long long buildMsOfTest = edgeMs[lu.output.generic_string()]; + if (lu.entryMain) + if (auto o = objectOf.find(*lu.entryMain); o != objectOf.end()) + buildMsOfTest += edgeMs[o->second.generic_string()]; // Through the runner resolved once above, or bare. The runner's // program was located already; only the artifact changes per test. @@ -3219,8 +3306,9 @@ export int run_tests(std::span passthrough, // `mcpp run` hands them over (see `runtime_files_for`). Written here, // in the single-threaded pass, one list per test program. if (!runnerTmpl.empty()) { - if (auto listed = write_runtime_files_list(*ctx, exe, - runtime_files_for(*ctx, exe))) + std::vector> carried; + as_member(*ctx, owner, [&] { carried = runtime_files_for(*ctx, exe, owner); }); + if (auto listed = write_runtime_files_list(*ctx, exe, carried)) childEnv.emplace_back(std::string(kRuntimeFilesEnv), listed->string()); else if (invocationNotRunReason.empty()) invocationNotRunReason = listed.error(); @@ -3242,9 +3330,9 @@ export int run_tests(std::span passthrough, } } - runnable.push_back({lu.targetName, std::move(argv), std::move(childEnv)}); + runnable.push_back({lu.targetName, std::move(argv), std::move(childEnv), + buildMsOfTest}); } - // Pass 2: run them. Concurrently unless there is exactly one — see // `runJobs` for why the single-test case is deliberately different. // @@ -3259,7 +3347,8 @@ export int run_tests(std::span passthrough, } else { run_tests_now(runnable); } - summary.elapsedMs = member_ms(); + summary.elapsedMs = carriedMs + std::chrono::duration_cast( + std::chrono::steady_clock::now() - tLoop).count(); // 7. Summary. int passed = 0; @@ -3300,13 +3389,18 @@ export int run_tests(std::span passthrough, const int rc = failed ? 1 : (notRun ? 2 : 0); if (json) { + // `build_group` names the configuration group whose one build this + // member's tests waited for, when it was planned with others; its + // `build_ms` is then the group's (docs/50 §8). + const auto group = summary.buildGroup >= 0 + ? std::format(",\"build_group\":{}", summary.buildGroup) : std::string{}; std::println("{{\"summary\":{{\"member\":\"{}\",\"passed\":{},\"failed\":{}," "\"not_run\":{},\"not_run_reason\":\"{}\"," "\"built\":{}," - "\"elapsed_ms\":{},\"build_ms\":{},\"run_ms\":{}}}}}", + "\"elapsed_ms\":{},\"build_ms\":{},\"run_ms\":{}{}}}}}", test_json_escape(memberName), passed, failed, notRun, test_json_escape(notRunReason), built, - summary.elapsedMs, summary.buildMs, summary.runMs); + summary.elapsedMs, summary.buildMs, summary.runMs, group); std::fflush(stdout); return rc; } @@ -3340,6 +3434,448 @@ export int run_tests(std::span passthrough, return rc; } +// `mcpp test` driver: discover tests/**/*.cpp, synthesize targets, build +// with dev-deps, run each test binary, summarize. +export int run_tests(std::span passthrough, + BuildOverrides overrides = {}, + TestOptions testOpts = {}, + TestRunSummary* summaryOut = nullptr) { + const bool json = (testOpts.format == TestMessageFormat::Json); + // The member this call is scoped to (empty outside a workspace). Threaded + // into every JSON record so a `--workspace` stream can be attributed: a + // bare test name is ambiguous the moment two members both have a `smoke`. + const std::string memberName = overrides.package_filter; + TestRunSummary summary; + struct SummaryWriter { + TestRunSummary* out; const TestRunSummary* src; + ~SummaryWriter() { if (out) *out = *src; } + } summaryWriter{summaryOut, &summary}; + // Wall clock for the WHOLE member, started before Phase A. The old `t0` + // sat after Phase A and the bulk pass, so `finished in` reported only the + // per-test loop: measured on one member, 6.53s printed against 93.5s + // actual — a 14x understatement, and worst exactly on the build-heavy + // members where the number matters. + auto tMember = std::chrono::steady_clock::now(); + auto member_ms = [&tMember] { + return std::chrono::duration_cast( + std::chrono::steady_clock::now() - tMember).count(); + }; + // JSON mode: stdout carries NDJSON only. All ui::status/info lines print + // to stdout, so silence them wholesale; errors already go to stderr. + if (json) mcpp::ui::set_quiet(true); + // The report covers the planning and the package's own build (Phase A); + // it is closed before the tests' own lines. + mcpp::build::progress::open(mcpp::log::is_verbose()); + + auto root = mcpp::project::find_manifest_root(std::filesystem::current_path()); + if (!root) { + mcpp::ui::error("no mcpp.toml found in current directory or any parent"); + return 2; + } + + auto discovered = mcpp::build::discover_test_targets( + *root, overrides.package_filter); + if (!discovered) { + mcpp::ui::error(discovered.error()); + return 2; + } + auto testRoot = discovered->packageRoot; + auto testTargets = std::move(discovered->targets); + if (testTargets.empty()) { + // Names where it looked when the manifest chose the place, so that a + // glob that matches nothing is not read as a project without tests. + if (discovered->discoverDeclared) { + std::string globs; + for (auto const& g : discovered->discover) + globs += std::format("{}\"{}\"", globs.empty() ? "" : ", ", g); + std::println("no tests found ([test] discover = [{}])", globs); + } else { + std::println("no tests found in tests/"); + } + return 0; + } + // --list: enumerate (filtered) tests and stop — no toolchain resolution, + // no build. Names/paths come straight from discovery, so this also works + // on tests that do not currently compile. + if (testOpts.list) { + std::size_t total = 0; + for (auto& t : testTargets) { + if (!testOpts.filter.empty() + && t.name.find(testOpts.filter) == std::string::npos) continue; + ++total; + auto abs = std::filesystem::absolute(testRoot / t.main) + .lexically_normal().generic_string(); + if (json) + std::println("{{\"member\":\"{}\",\"test\":\"{}\",\"main\":\"{}\"}}", + test_json_escape(memberName), + test_json_escape(t.name), test_json_escape(abs)); + else + std::println("{}", t.name); + } + if (json) { + std::println("{{\"summary\":{{\"total\":{}}}}}", total); + std::fflush(stdout); + } + return 0; + } + // 3. prepare_build with dev-deps enabled + synthetic targets. + // A test binary is executed, so the run tier applies here exactly as it + // does to `mcpp run` — `[xlings.workspace]` has no separate `test` tier + // because there is no separate need. + overrides.will_run = true; + auto prepared = prepare_build(/*print_fp=*/false, + /*includeDevDeps=*/true, + std::move(testTargets), + std::move(overrides)); + if (!prepared) { mcpp::ui::error(prepared.error()); return 2; } + TestBuild tb; + tb.ctx.emplace(std::move(*prepared)); + tb.backend = mcpp::build::make_ninja_backend(); + tb.ninjaLog = tb.ctx->outputDir / ".ninja_log"; + { + std::error_code ec; + tb.logFrom = std::filesystem::file_size(tb.ninjaLog, ec); + if (ec) tb.logFrom = 0; + } + auto* ctx = &*tb.ctx; + + // Filter guard. The filter selects at the build/run stage ONLY — the plan + // above always contains every test, so build.ninja and + // compile_commands.json stay complete (clangd depends on the latter; a + // filtered run must not clobber it down to one entry). + auto filter_match = [&](const mcpp::build::LinkUnit& lu) { + return lu.kind == mcpp::build::LinkUnit::TestBinary + && (testOpts.filter.empty() + || lu.targetName.find(testOpts.filter) != std::string::npos); + }; + if (!testOpts.filter.empty()) { + bool any = false; + for (auto& lu : ctx->plan.linkUnits) + if (filter_match(lu)) { any = true; break; } + if (!any) { + if (json) + std::println("{{\"error\":\"no-tests-matched\",\"filter\":\"{}\"}}", + test_json_escape(testOpts.filter)); + mcpp::ui::error(std::format("no tests match '{}'", testOpts.filter)); + return 2; + } + } + + // 4. "Compiling test_X (test)" lines for the test binaries. + test_announce(*ctx); + // List test binaries. + // (Per-test "Compiling" lines print in Phase B, interleaved with each + // test's own result — announcing them all up front separated the three + // pieces of one test's story across the whole output.) + + // 5. Two-phase build. Phase A: package-level artifacts (everything that + // is not a test binary — libs, deps). A failure here is the PACKAGE's + // fault, not any single test's: report it as a build error, never as + // N red tests. Phase B (below): each test is built as its own ninja + // goal, so a compile failure is attributed to exactly that test and + // the rest still build and run. + mcpp::build::progress::programs_done(); + if (auto a = test_phase_a(tb, testOpts, json)) { + summary.packageError = true; + summary.buildMs = tb.buildMs; + summary.elapsedMs = member_ms(); + std::fflush(stdout); + if (json) + std::println("{{\"error\":\"package\",\"compile_output\":\"{}\"}}", + test_json_escape(a->diagnosticOutput)); + // Surface the compiler/linker stderr (parity with run_build_plan) — + // otherwise `mcpp test` failures show only "build failed" with no + // diagnostic, which is undebuggable (notably on CI). A failed step + // was reported when it failed. + if (!a->reported) mcpp::ui::error(a->message); + mcpp::ui::block(a->diagnosticOutput); + return 1; + } + // The tests' own lines follow, as they always have. + mcpp::build::progress::close(); + + test_bulk(tb, testOpts, filter_match); + test_resolve_runner(tb, testOpts, json); + summary.buildMs = tb.buildMs; + return test_run_member(tb, testOpts, passthrough, json, memberName, /*owner=*/"", + member_ms(), summary); +} + +// `mcpp test` over several members: the members are planned once per +// configuration group, each group's Phase A and test goals are built once, and +// then each member's tests run in member order, continuing past a failing +// member (member-selection design 2026-09-30, S4 and D1). +// +// `groups` holds the selected members by configuration, as `mcpp build` groups +// them, and `members` the same members in `[workspace] members` order with +// what discovery found in each. A member with no tests, or whose discovery +// failed, is not planned, as it never was: it reports its own result when its +// turn comes. A group that fails to plan is planned again member by member, +// so a member that fails to plan fails alone, and the members that plan are +// planned together again without it. +export void run_workspace_tests(std::span passthrough, + const BuildOverrides& base, + const TestOptions& testOpts, + const std::filesystem::path& wsRoot, + const std::vector>& groups, + std::vector members, + const WorkspaceTestHooks& hooks) { + const bool json = (testOpts.format == TestMessageFormat::Json); + if (json) mcpp::ui::set_quiet(true); + // The report covers the planning and the packages' own builds (Phase A); + // it is closed before the tests' own lines. + mcpp::build::progress::open(mcpp::log::is_verbose()); + + // What each member has to say when its turn comes, decided before any + // build: a member whose tests cannot be planned says why, and one with + // nothing to run says so. + struct Slot { + int session = -1; // the build that holds the member's tests + std::string owner; // the member's package name in that plan + std::string error; // it failed before its tests could run + std::string note; // it has no tests + bool noMatch = false; // no test matches the filter + bool packageFailed = false; + std::string packageOutput; // the diagnostics of its package's build + }; + std::vector slots(members.size()); + std::map indexOf; + std::vector planned(members.size(), false); + for (std::size_t i = 0; i < members.size(); ++i) { + indexOf[members[i].path] = i; + if (!members[i].error.empty()) { slots[i].error = members[i].error; continue; } + if (members[i].targets.empty()) { slots[i].note = members[i].noTests; continue; } + if (!testOpts.filter.empty() + && std::ranges::none_of(members[i].targets, [&](auto const& t) { + return t.name.find(testOpts.filter) != std::string::npos; })) { + slots[i].noMatch = true; + continue; + } + planned[i] = true; + } + + std::vector> sessions; + std::vector> sessionMembers; + + // One plan of the given members, with each member's tests. + auto plan_session = [&](const std::vector& who) + -> std::expected, std::string> { + BuildOverrides mo = base; + mo.package_filter.clear(); + mo.project_root = wsRoot; + mo.will_run = true; + mo.workspace_members.clear(); + mo.workspace_request.clear(); + mo.member_targets.clear(); + for (auto i : who) { + mo.workspace_members.push_back(members[i].path); + mo.member_targets[members[i].path] = members[i].targets; + } + for (auto const& m : members) mo.workspace_request.push_back(m.path); + const auto t0 = std::chrono::steady_clock::now(); + auto prepared = prepare_build(/*print_fp=*/false, /*includeDevDeps=*/true, + /*extraTargets=*/{}, std::move(mo)); + if (!prepared) return std::unexpected(prepared.error()); + auto tb = std::make_unique(); + tb->prepareMs = std::chrono::duration_cast( + std::chrono::steady_clock::now() - t0).count(); + tb->ctx.emplace(std::move(*prepared)); + tb->backend = mcpp::build::make_ninja_backend(); + tb->ninjaLog = tb->ctx->outputDir / ".ninja_log"; + std::error_code ec; + tb->logFrom = std::filesystem::file_size(tb->ninjaLog, ec); + if (ec) tb->logFrom = 0; + return tb; + }; + auto attach = [&](std::unique_ptr tb, const std::vector& who) { + mcpp::build::progress::programs_done(); + const int id = static_cast(sessions.size()); + for (auto i : who) { + slots[i].session = id; + for (auto const& wm : tb->ctx->workspaceMembers) + if (wm.memberPath == members[i].path) slots[i].owner = wm.name; + } + sessionMembers.push_back(who); + sessions.push_back(std::move(tb)); + }; + + for (auto const& g : groups) { + std::vector who; + for (auto const& mp : g) + if (auto it = indexOf.find(mp); it != indexOf.end() && planned[it->second]) + who.push_back(it->second); + if (who.empty()) continue; + auto tb = plan_session(who); + if (tb) { attach(std::move(*tb), who); continue; } + if (who.size() == 1) { slots[who.front()].error = tb.error(); continue; } + // The group did not plan, and the message does not say which member is + // to blame. Each member is planned alone to find out: the ones that + // plan are planned together again without the others, and the ones + // that do not are reported with their own reason. + std::vector survivors; + std::vector> alone; + for (auto i : who) { + auto one = plan_session({i}); + if (one) { survivors.push_back(i); alone.push_back(std::move(*one)); } + else slots[i].error = one.error(); + } + if (survivors.size() > 1) { + if (auto together = plan_session(survivors)) { + attach(std::move(*together), survivors); + continue; + } + } + for (std::size_t k = 0; k < survivors.size(); ++k) + attach(std::move(alone[k]), {survivors[k]}); + } + // The groups' compile databases are published once, as the union, below: + // a group's own build must not publish the root's as if it were the only + // one (`mcpp build` does the same). + if (sessions.size() > 1) + for (auto& s : sessions) s->ctx->plan.publishRootCompileDb = false; + + // Build every group: Phase A once, then one keep-going pass over the test + // goals of the members whose package built. + for (std::size_t s = 0; s < sessions.size(); ++s) { + auto& tb = *sessions[s]; + const auto& who = sessionMembers[s]; + test_announce(*tb.ctx); + if (auto a = test_phase_a(tb, testOpts, json)) { + // A failure of the package level is the package's fault, never N + // red tests. A group's Phase A stops at its first failure, and the + // failure says nothing of which member it belongs to: each + // member's own part of it is built alone, so that a member whose + // package builds still runs, and the one whose package does not is + // reported as failed, alone. + if (!json) { + if (!a->reported) mcpp::ui::error(a->message); + mcpp::ui::block(a->diagnosticOutput); + } + if (who.size() > 1) { + for (auto i : who) { + const auto goals = member_package_goals(*tb.ctx, slots[i].owner); + if (goals.empty()) continue; + mcpp::build::BuildOptions own; + own.ninjaTargets = goals; + own.buildTimeoutSecs = static_cast(testOpts.buildTimeoutSecs); + const auto t0 = std::chrono::steady_clock::now(); + auto r = tb.backend->build(tb.ctx->plan, own); + tb.buildMs += std::chrono::duration_cast( + std::chrono::steady_clock::now() - t0).count(); + if (!r) { + slots[i].packageFailed = true; + slots[i].packageOutput = r.error().diagnosticOutput; + } + } + } + // Nothing pointed at a member (a failure outside every member's + // own objects), or the group is one member: they fail together. + if (std::ranges::none_of(who, [&](auto i) { return slots[i].packageFailed; })) + for (auto i : who) { + slots[i].packageFailed = true; + slots[i].packageOutput = a->diagnosticOutput; + } + } + // The test goals of the members that can run. + std::set runnable; + for (auto i : who) + if (!slots[i].packageFailed) runnable.insert(slots[i].owner); + test_bulk(tb, testOpts, [&](const mcpp::build::LinkUnit& lu) { + return lu.kind == mcpp::build::LinkUnit::TestBinary + && runnable.contains(lu.memberOf) + && (testOpts.filter.empty() + || lu.targetName.find(testOpts.filter) != std::string::npos); + }); + if (!runnable.empty()) test_resolve_runner(tb, testOpts, json); + } + // The tests' own lines follow, as they always have. + mcpp::build::progress::close(); + + if (sessions.size() > 1) { + std::vector dirs; + for (auto& s : sessions) dirs.push_back(s->ctx->outputDir); + publish_workspace_compile_commands(wsRoot, dirs); + } + + // One record or line per group, before its first member's tests: the + // members it built and the wall time of the build (member-selection design + // D6). The time is the group's and not any member's, so it is stated once; + // each member's summary names the group it waited for. + for (std::size_t s = 0; s < sessions.size(); ++s) { + auto& tb = *sessions[s]; + const auto& who = sessionMembers[s]; + if (json) { + std::string list; + for (auto i : who) { + if (!list.empty()) list += ','; + list += std::format("\"{}\"", test_json_escape(members[i].path)); + } + std::println("{{\"group_build\":{{\"group\":{},\"members\":[{}],\"build_ms\":{}}}}}", + s, list, tb.buildMs); + std::fflush(stdout); + continue; + } + std::string names; + for (auto i : who) names += (names.empty() ? "" : ", ") + members[i].path; + // The edges that took the build its time, for the question the per + // member split used to answer: which member's link, and not its tests, + // is slow. + auto edges = ninja_edges_since(tb.ninjaLog, tb.logFrom); + std::ranges::sort(edges, [](auto const& a, auto const& b) { return a.ms > b.ms; }); + std::string slowest; + for (std::size_t k = 0; k < edges.size() && k < 3; ++k) { + if (edges[k].ms < 1000) break; + slowest += std::format("{}{} {:.1f}s", slowest.empty() ? "" : ", ", + edges[k].output, static_cast(edges[k].ms) / 1000.0); + } + mcpp::ui::status("Workspace", std::format( + "{}built {} {} in {:.2f}s{}", + sessions.size() > 1 ? std::format("group {}/{} ", s + 1, sessions.size()) + : std::string{}, + who.size() == 1 ? "member" : "members", names, + static_cast(tb.buildMs) / 1000.0, + slowest.empty() ? std::string{} : std::format("; slowest: {}", slowest))); + } + + for (std::size_t i = 0; i < members.size(); ++i) { + auto& m = members[i]; + auto& slot = slots[i]; + if (hooks.begin && !hooks.begin(i, m.path)) continue; + TestRunSummary sum; + int rc = 0; + if (!slot.error.empty()) { + mcpp::ui::error(slot.error); + rc = 2; + } else if (slot.noMatch) { + if (json) + std::println("{{\"error\":\"no-tests-matched\",\"filter\":\"{}\"}}", + test_json_escape(testOpts.filter)); + mcpp::ui::error(std::format("no tests match '{}'", testOpts.filter)); + rc = 2; + } else if (!slot.note.empty()) { + if (!json) std::println("{}", slot.note); + } else if (slot.session >= 0) { + auto& tb = *sessions[static_cast(slot.session)]; + sum.buildGroup = slot.session; + sum.buildMs = tb.buildMs; + if (slot.packageFailed) { + sum.packageError = true; + sum.elapsedMs = tb.prepareMs + tb.buildMs; + if (json) + std::println("{{\"error\":\"package\",\"member\":\"{}\",\"compile_output\":\"{}\"}}", + test_json_escape(m.path), test_json_escape(slot.packageOutput)); + mcpp::ui::error(std::format( + "member '{}': its package did not build, so its tests did not run", m.path)); + rc = 1; + } else { + rc = test_run_member(tb, testOpts, passthrough, json, m.path, slot.owner, + tb.prepareMs + tb.buildMs, sum); + } + } + if (hooks.end) hooks.end(i, m.path, rc, sum); + } +} + // `mcpp clean` driver. export int clean_project(bool wipe_bmi) { auto root = mcpp::project::find_manifest_root(std::filesystem::current_path()); diff --git a/src/cli.cppm b/src/cli.cppm index 212b96d7..ec696f22 100644 --- a/src/cli.cppm +++ b/src/cli.cppm @@ -387,8 +387,8 @@ int run(int argc, char** argv) { .help("Target no accelerator, ignoring [build] accel")) .option(cl::Option("static").help( "Force static linking (-static). On Linux, prefer pairing with --target -linux-musl")) - .option(cl::Option("package").short_name('p').takes_value().value_name("NAME") - .help("Build only the named workspace member (namespace.name or package name, then directory)")) + .option(cl::Option("package").short_name('p').takes_value().multiple().value_name("NAME") + .help("Build the named workspace member (namespace.name or package name, then directory); repeat to build several")) .option(cl::Option("profile").takes_value().value_name("NAME") .help("Build profile: dev (default) | release | dist | <[profile.*] name>")) .option(cl::Option("release").help("Shorthand for --profile release")) @@ -401,6 +401,8 @@ int run(int argc, char** argv) { .help("Treat manifest schema warnings (unknown feature/platform) as errors")) .option(cl::Option("workspace") .help("Build all workspace members")) + .option(cl::Option("exclude").takes_value().multiple().value_name("NAME") + .help("With --workspace (or at a virtual workspace root), leave the named member out; repeatable, refused with -p")) .option(cl::Option("play-game") .help("Play a game in the status row while it builds: --play-game=snake|stack|runner, or one at random")) .action(wrap_rc(cmd_build))) @@ -433,8 +435,8 @@ int run(int argc, char** argv) { // `run` accepted, and scripts written against it must keep working. .option(cl::Option("target-triple").takes_value().value_name("TRIPLE") .help("Alias for --target")) - .option(cl::Option("package").short_name('p').takes_value().value_name("NAME") - .help("Run only the named workspace member (namespace.name or package name, then directory; single-member, no --workspace fan-out)")) + .option(cl::Option("package").short_name('p').takes_value().multiple().value_name("NAME") + .help("Run only the named workspace member (namespace.name or package name, then directory; one member: a second -p is refused)")) // DECLARED ON THE THREE COMMANDS THAT BUILD BEFORE THEY ACT, AS ON // `build`. The value has always reached them: the pre-parse loop // above publishes it as MCPP_TOOLCHAIN for every command, and @@ -526,7 +528,7 @@ int run(int argc, char** argv) { .option(cl::Option("build-timeout").takes_value().value_name("SECS") .help("Kill a compile/link drive still running after SECS seconds (default 0 = no limit; POSIX only)")) .option(cl::Option("workspace-timeout").takes_value().value_name("SECS") - .help("Stop the --workspace fan-out after SECS seconds and report what did run (default 0 = no limit)")) + .help("Start no further member's tests after SECS seconds since the command began, and report what did not run (default 0 = no limit); the build is bounded by --build-timeout")) .option(cl::Option("profile").takes_value().value_name("NAME") .help("Build profile for the test build: dev (default) | release | dist | <[profile.*] name>")) .option(cl::Option("features").takes_value().value_name("LIST") @@ -535,8 +537,8 @@ int run(int argc, char** argv) { .help("Pin capability providers (e.g. blas=openblas,lapack=mkl)")) .option(cl::Option("strict") .help("Treat manifest schema warnings (unknown feature/platform) as errors")) - .option(cl::Option("package").short_name('p').takes_value().value_name("NAME") - .help("Run tests only for the named workspace member (namespace.name or package name, then directory)")) + .option(cl::Option("package").short_name('p').takes_value().multiple().value_name("NAME") + .help("Run the tests of the named workspace member (namespace.name or package name, then directory); repeat to test several")) .option(cl::Option("toolchain").takes_value().value_name("SPEC") .help("Build the tests with this toolchain for one invocation, e.g. llvm@22.1.8")) .option(cl::Option("cache").takes_value().value_name("MODE") @@ -545,6 +547,8 @@ int run(int argc, char** argv) { .help("Deprecated alias for --cache=off (also clears the build dir)")) .option(cl::Option("workspace") .help("Run tests for all workspace members")) + .option(cl::Option("exclude").takes_value().multiple().value_name("NAME") + .help("With --workspace (or at a virtual workspace root), leave the named member out; repeatable, refused with -p")) .option(cl::Option("play-game") .help("Play a game in the status row while it builds: --play-game=snake|stack|runner, or one at random")) .action(wrap_rc([&passthrough](const cl::ParsedArgs& p) { @@ -704,8 +708,8 @@ int run(int argc, char** argv) { .option(cl::Option("no-accel") .help("Describe the variant built for no accelerator")) .option(cl::Option("static").help("Describe the build with --static")) - .option(cl::Option("package").short_name('p').takes_value().value_name("NAME") - .help("Describe only the named workspace member (namespace.name or package name, then directory)")) + .option(cl::Option("package").short_name('p').takes_value().multiple().value_name("NAME") + .help("Describe the named workspace member (namespace.name or package name, then directory); repeat to describe several")) .option(cl::Option("profile").takes_value().value_name("NAME") .help("Build profile: dev (default) | release | dist | <[profile.*] name>")) .option(cl::Option("release").help("Shorthand for --profile release")) @@ -717,7 +721,9 @@ int run(int argc, char** argv) { .option(cl::Option("strict") .help("Treat manifest schema warnings (unknown feature/platform) as errors")) .option(cl::Option("workspace") - .help("Describe all workspace members in one document"))) + .help("Describe all workspace members in one document")) + .option(cl::Option("exclude").takes_value().multiple().value_name("NAME") + .help("With --workspace (or at a virtual workspace root), leave the named member out; repeatable, refused with -p"))) .action(wrap_rc([&dispatch_sub](const cl::ParsedArgs& p) { return dispatch_sub("emit", p, {{"xpkg", cmd_emit_xpkg}, {"sbom", mcpp::cli::cmd_sbom}, diff --git a/src/cli/cmd_build.cppm b/src/cli/cmd_build.cppm index 0e3f94de..4a3beb31 100644 --- a/src/cli/cmd_build.cppm +++ b/src/cli/cmd_build.cppm @@ -19,6 +19,7 @@ import mcpp.build.coff_exports; import mcpp.build.stage; import mcpp.build.schedule.detach_codegen; import mcpp.build.test_targets; +import mcpp.cli.selection; import mcpp.build.build_database; import mcpp.build.build_program; import mcpp.build.progress; // the report every building command opens @@ -39,83 +40,16 @@ import mcpp.wire; namespace mcpp::cli { -// Decide whether a build/test invocation acts on several workspace members, and -// if so which. It does when `--workspace` is given, or at a *virtual* workspace -// root with no `-p` (the intuitive "act on the whole workspace"). Returns the -// member paths as `[workspace] members` writes them -- a rooted workspace's own -// package first, as "." (workspace design 2026-09-29 §7.1) -- or nullopt for -// the single-package / single-`-p` / rooted-bare path. Inside a member, the -// workspace is the one that lists it. -std::optional> -workspace_fanout_members(bool wantAll, const std::string& package_filter) { - auto root = mcpp::project::find_manifest_root(std::filesystem::current_path()); - if (!root) return std::nullopt; - auto m = mcpp::manifest::load(*root / "mcpp.toml"); - if (m && !m->workspace.present && wantAll) { - auto wsRoot = mcpp::project::find_workspace_root(*root); - if (wsRoot.empty()) return std::nullopt; - m = mcpp::manifest::load(wsRoot / "mcpp.toml"); - } - if (!m || !m->workspace.present || m->workspace.members.empty()) return std::nullopt; - bool virtualWs = m->package.name.empty(); - if (!(wantAll || (virtualWs && package_filter.empty()))) return std::nullopt; - std::vector members; - if (!virtualWs) members.push_back("."); - members.insert(members.end(), m->workspace.members.begin(), m->workspace.members.end()); - return members; -} - -// The workspace a build command acts on and the members it selects (workspace -// design 2026-09-29 §7.1, §15): `--workspace`, and a virtual root without -// `-p`, select every member (a rooted workspace's own package first, as "."); -// `-p X` selects X; a command in a member's directory selects that member; a -// command at a rooted workspace's root selects the workspace's own package. -// nullopt outside a workspace. -struct WorkspaceSelection { - std::filesystem::path root; - std::vector members; -}; -std::expected, std::string> -workspace_selection(bool wantAll, const std::string& package_filter) { - auto root = mcpp::project::find_manifest_root(std::filesystem::current_path()); - if (!root) return std::optional{}; - auto m = mcpp::manifest::load(*root / "mcpp.toml", {.insideWorkspace = true}); - if (!m) return std::optional{}; - WorkspaceSelection sel{*root, {}}; - std::string inside; - if (!m->workspace.present) { - auto wsRoot = mcpp::project::find_workspace_root(*root); - if (wsRoot.empty()) return std::optional{}; - sel.root = wsRoot; - m = mcpp::manifest::load(wsRoot / "mcpp.toml"); - if (!m || !m->workspace.present) return std::optional{}; - const auto rel = root->lexically_normal() - .lexically_relative(wsRoot.lexically_normal()); - for (auto const& mp : m->workspace.members) - if (std::filesystem::path(mp).lexically_normal() == rel) inside = mp; - if (inside.empty()) return std::optional{}; - } - const bool rooted = !m->package.name.empty(); - std::vector all; - if (rooted) all.push_back("."); - all.insert(all.end(), m->workspace.members.begin(), m->workspace.members.end()); - if (wantAll) { sel.members = std::move(all); return sel; } - if (!package_filter.empty()) { - auto dir = mcpp::project::resolve_member_dir(*m, sel.root, package_filter); - if (!dir) return std::unexpected(dir.error()); - auto rel = dir->empty() ? std::string(".") - : dir->lexically_normal().lexically_relative(sel.root.lexically_normal()).generic_string(); - if (rel.empty()) rel = "."; - for (auto const& mp : m->workspace.members) - if (std::filesystem::path(mp).lexically_normal() == std::filesystem::path(rel)) - rel = mp; - sel.members = {rel}; - return sel; - } - if (!inside.empty()) { sel.members = {inside}; return sel; } - if (!rooted) { sel.members = std::move(all); return sel; } - sel.members = {"."}; - return sel; +// The selectors of a command line, as `mcpp::cli::select_members` reads them +// (member selection design 2026-09-30, S1): every command that acts on members +// reads its `-p`, `--workspace` and `--exclude` the same way, so the members a +// command plans are the ones the flags name whatever the command is. +mcpp::cli::MemberRequest member_request(const mcpplibs::cmdline::ParsedArgs& parsed) { + mcpp::cli::MemberRequest req; + req.all = parsed.is_flag_set("workspace"); + req.packages = parsed.option_or_empty("package").values; + req.excludes = parsed.option_or_empty("exclude").values; + return req; } // The workspace a fan-out acts on, and its members grouped by configuration @@ -138,6 +72,24 @@ workspace_groups(const std::filesystem::path& wsRoot, const std::vector +discover_member_tests(const std::filesystem::path& wsRoot, const std::string& mp) { + auto d = mcpp::build::discover_test_targets(wsRoot, mp); + if (!d) return d; + std::error_code ec; + const bool same = std::filesystem::equivalent(d->packageRoot, wsRoot / mp, ec); + if (!ec && !same) + return std::unexpected(std::format( + "the path '{}' is also the name of another member's package, so its tests " + "cannot be told from that member's; select it by its package name (-p )", + mp)); + return d; +} + // The tests of each member of a configuration group, for a plan of the group // that includes them (`--configure-only`, `mcpp emit build-database`): the // targets by member path, and the root and globs each member's discovery read. @@ -149,7 +101,7 @@ std::expected group_tests(const std::filesystem::path& wsRoot, const std::vector& group) { GroupTests out; for (auto const& mp : group) { - auto d = mcpp::build::discover_test_targets(wsRoot, mp); + auto d = discover_member_tests(wsRoot, mp); if (!d) return std::unexpected(std::format("{}: {}", mp, d.error())); if (!d->targets.empty()) out.targets[mp] = std::move(d->targets); out.discovery.emplace_back(d->packageRoot, std::move(d->discover)); @@ -321,7 +273,7 @@ export int cmd_build(const mcpplibs::cmdline::ParsedArgs& parsed) { // Groups are independent (a member reached from two groups is a node of // each), and a failed group does not stop the others; the first non-zero // exit wins. - auto selection = workspace_selection(parsed.is_flag_set("workspace"), ov.package_filter); + auto selection = mcpp::cli::select_members(member_request(parsed)); if (!selection) { mcpp::ui::error(std::format("{}", selection.error())); return 2; } if (*selection) { auto const& members = (*selection)->members; @@ -609,7 +561,8 @@ export int cmd_emit_build_database(const mcpplibs::cmdline::ParsedArgs& parsed) requests.push_back({mp, {}, std::move(one)}); }; std::filesystem::path wsRoot = *root; - auto selection = workspace_selection(parsed.is_flag_set("workspace"), ov.package_filter); + const auto request = member_request(parsed); + auto selection = mcpp::cli::select_members(request); if (!selection) return failed("MCPP_BUILD_DATABASE_PLAN_FAILED", selection.error()); if (*selection && (*selection)->members.size() > 1) { @@ -629,9 +582,10 @@ export int cmd_emit_build_database(const mcpplibs::cmdline::ParsedArgs& parsed) requests.push_back({g.size() == 1 ? g.front() : std::string{}, g, std::move(mo)}); } } - } else if (auto members = workspace_fanout_members(parsed.is_flag_set("workspace"), - ov.package_filter)) { - plan_alone(members->front()); + } else if (*selection && (*selection)->whole) { + // The whole workspace, which lists one member (or one is left by + // `--exclude`): that member's plan, as a selection of several plans. + plan_alone((*selection)->members.front()); } else { requests.push_back({std::string{}, {}, ov}); } @@ -825,12 +779,21 @@ export int cmd_emit_build_database(const mcpplibs::cmdline::ParsedArgs& parsed) } // One line per selector: a value never spans lines, and a `\x1f` separator // before `f`, `c` or `a` reads as a longer hex escape (clang refuses it). + // The members the flags name, each as written and in command-line order. + // One `-p` reads as it always has, so the fingerprint of a command that + // names one member does not change; `--exclude` adds a line only when it + // is given. + std::string packages; + for (auto const& p : request.packages) packages += (packages.empty() ? "" : ",") + p; + std::string excludes; + for (auto const& e : request.excludes) excludes += (excludes.empty() ? "" : ",") + e; const auto selector = std::format( "spec={}\ntarget={}\ntoolchain={}\nprofile={}\nfeatures={}\n" - "cap={}\naccel={}\nstatic={}\npackage={}\nworkspace={}", + "cap={}\naccel={}\nstatic={}\npackage={}\nworkspace={}{}", spec, ov.target_triple, mcpp::platform::env::get("MCPP_TOOLCHAIN").value_or(""), ov.profile, ov.features, ov.capabilities, ov.accel, ov.force_static, - ov.package_filter, parsed.is_flag_set("workspace")); + packages, parsed.is_flag_set("workspace"), + excludes.empty() ? std::string{} : std::format("\nexclude={}", excludes)); auto rendered = mcpp::build::database::render(members, failedMemberRoots, wsRoot, selector); // A note's severity is its own (E3's program-failure note is an error; @@ -884,9 +847,23 @@ export int cmd_run(const mcpplibs::cmdline::ParsedArgs& parsed, if (parsed.positional_count() > 0) targetName = parsed.positional(0); // -p/--package : scope to one workspace member, same flag/rule // as `mcpp build -p` / `mcpp test -p` (mcpp::project::resolve_member_dir). - // `mcpp run` is single-member only — no `--workspace` fan-out. + // `mcpp run` is single-member only — no `--workspace` fan-out — because an + // artifact to execute is one program. The option is repeatable on the + // commands that act on several members, so a second `-p` here is refused, + // naming every member asked for, and never read as "the last one". + const auto packages = parsed.option_or_empty("package").values; + if (packages.size() > 1) { + std::string named; + for (std::size_t i = 0; i < packages.size(); ++i) + named += std::format("{}'{}'", + i == 0 ? "" : (i + 1 == packages.size() ? " and " : ", "), packages[i]); + mcpp::ui::error(std::format( + "mcpp run runs one program, so it acts on one workspace member, and -p names {}: " + "pass one -p (mcpp build and mcpp test accept several)", named)); + return 2; + } std::string package_filter; - if (auto p = parsed.value("package")) package_filter = *p; + if (!packages.empty()) package_filter = packages.front(); std::string cache_mode; bool no_cache = parsed.is_flag_set("no-cache"); if (auto c = parsed.value("cache")) cache_mode = *c; @@ -992,11 +969,19 @@ export int cmd_test(const mcpplibs::cmdline::ParsedArgs& parsed, } } - // Workspace fan-out: test every member through run_tests (which scopes its - // discovery to the member). Continue-on-failure + per-member summary so one - // red member never hides the rest. - if (auto members = workspace_fanout_members(parsed.is_flag_set("workspace"), - ov.package_filter)) { + // The members this command tests, by the one selection every command reads + // (`mcpp::cli::select_members`). A selection of one member, named or implied + // by the directory, is that member's own test run, as it always was. A + // selection of several members, or of the whole workspace, fans out: the + // members are planned once per configuration group and each group is built + // once, then each member's tests run in member order, continuing past a + // failing member so that one red member never hides the rest (member + // selection design 2026-09-30, S4). + auto selection = mcpp::cli::select_members(member_request(parsed)); + if (!selection) { mcpp::ui::error(std::format("{}", selection.error())); return 2; } + if (*selection && ((*selection)->whole || (*selection)->members.size() > 1)) { + auto const& sel = **selection; + auto const& members = sel.members; const bool json = (to.format == mcpp::build::TestMessageFormat::Json); // Silence the ui BEFORE the first member, not inside run_tests. The // quiet flag used to be set by run_tests itself, so the fan-out's own @@ -1023,28 +1008,37 @@ export int cmd_test(const mcpplibs::cmdline::ParsedArgs& parsed, const long long wsDeadlineMs = static_cast(workspaceTimeoutSecs) * 1000; - std::size_t idx = 0; - for (auto& mp : *members) { - ++idx; - // Checked BEFORE starting a member rather than after: stopping - // mid-member would leave a half-built member reported as neither - // run nor skipped. + // Asked before a member's tests start. The deadline is checked BEFORE + // starting a member rather than after: stopping mid-member would leave + // a half-tested member reported as neither run nor skipped. It is + // measured from the start of the command, so the build the members + // share counts against it, and a member not started by then is listed + // as not run; the build itself is bounded by --build-timeout. + auto member_begin = [&](std::size_t i, const std::string& mp) -> bool { if (wsDeadlineMs > 0 && ws_ms() >= wsDeadlineMs) { notRun.push_back(mp); - continue; + return false; } - mcpp::build::BuildOverrides mo = ov; - mo.package_filter = mp; mcpp::ui::status("Workspace", - std::format("testing member '{}' ({}/{})", mp, idx, members->size())); - mcpp::build::TestRunSummary sum; - int r = mcpp::build::run_tests(passthrough, mo, to, &sum); + std::format("testing member '{}' ({}/{})", mp, i + 1, members.size())); + return true; + }; + // The member's line: how it ended, and how long its own tests ran. The + // build is the group's, stated once by the group's line, so a member's + // line does not state it again. + auto member_end = [&](std::size_t i, const std::string& mp, int r, + const mcpp::build::TestRunSummary& sum) { + const auto idx = i + 1; totalPassed += sum.passed; totalFailed += sum.failed; totalNotRun += sum.notRun; totalBuilt += sum.built; - memberTimes.emplace_back(mp, sum.elapsedMs); - auto secs = static_cast(sum.elapsedMs) / 1000.0; + // Ranked by its run, which is the member's own: a build a group + // shares belongs to no one member. + memberTimes.emplace_back(mp, sum.buildGroup >= 0 ? sum.runMs : sum.elapsedMs); + auto secs = static_cast(sum.buildGroup >= 0 ? sum.runMs : sum.elapsedMs) / 1000.0; + const char* took = sum.buildGroup >= 0 ? "run " : ""; + const char* in = sum.buildGroup >= 0 ? ", " : " in "; if (r == 2 && sum.failed == 0 && sum.notRun > 0) { // Built and not executed (#544): the member did not fail, and // it did not pass. 2 outranks 0 and yields to 1, as it does @@ -1052,25 +1046,79 @@ export int cmd_test(const mcpplibs::cmdline::ParsedArgs& parsed, if (rc == 0) rc = 2; unrunnable.push_back(mp); mcpp::ui::status("Workspace", - std::format("member '{}' ({}/{}) NOT RUN — {} passed, {} not run in {:.2f}s", - mp, idx, members->size(), sum.passed, sum.notRun, secs)); + std::format("member '{}' ({}/{}) NOT RUN — {} passed, {} not run{}{}{:.2f}s", + mp, idx, members.size(), sum.passed, sum.notRun, in, took, secs)); } else if (r != 0) { rc = r; failed.push_back(mp); mcpp::ui::status("Workspace", - std::format("member '{}' ({}/{}) FAILED — {} passed, {} failed in {:.2f}s", - mp, idx, members->size(), sum.passed, sum.failed, secs)); + std::format("member '{}' ({}/{}) FAILED — {} passed, {} failed{}{}{:.2f}s", + mp, idx, members.size(), sum.passed, sum.failed, in, took, secs)); } else { // Under `--no-run` nothing passed and nothing was meant to: // reporting "0 passed" for a member whose tests all built is // the same sentence a member with no tests would produce. mcpp::ui::status("Workspace", sum.built - ? std::format("member '{}' ({}/{}) ok — {} built, not run in {:.2f}s", - mp, idx, members->size(), sum.built, secs) - : std::format("member '{}' ({}/{}) ok — {} passed in {:.2f}s", - mp, idx, members->size(), sum.passed, secs)); + ? std::format("member '{}' ({}/{}) ok — {} built, not run{}{}{:.2f}s", + mp, idx, members.size(), sum.built, in, took, secs) + : std::format("member '{}' ({}/{}) ok — {} passed{}{}{:.2f}s", + mp, idx, members.size(), sum.passed, in, took, secs)); + } + }; + + if (to.list) { + // A listing builds nothing, so there is nothing to plan once: each + // member lists its own tests. + for (std::size_t i = 0; i < members.size(); ++i) { + if (!member_begin(i, members[i])) continue; + mcpp::build::BuildOverrides mo = ov; + mo.package_filter = members[i]; + mcpp::build::TestRunSummary sum; + int r = mcpp::build::run_tests(passthrough, mo, to, &sum); + member_end(i, members[i], r, sum); + } + } else { + // Each member's own tests, discovered from the member's own + // directory: two members may each have a `tests/main.cpp`. + std::vector inputs; + for (auto const& mp : members) { + mcpp::build::WorkspaceTestMember wm; + wm.path = mp; + auto d = discover_member_tests(sel.root, mp); + if (!d) { + wm.error = std::format("{}: {}", mp, d.error()); + } else { + wm.targets = std::move(d->targets); + if (wm.targets.empty()) { + // Names where it looked when the manifest chose the + // place, so that a glob that matches nothing is not + // read as a project without tests. + if (d->discoverDeclared) { + std::string globs; + for (auto const& g : d->discover) + globs += std::format("{}\"{}\"", globs.empty() ? "" : ", ", g); + wm.noTests = std::format("no tests found ([test] discover = [{}])", globs); + } else { + wm.noTests = "no tests found in tests/"; + } + } + } + inputs.push_back(std::move(wm)); } + // Members whose root-position values are equal are planned together, + // as `mcpp build` plans them. A member whose manifest cannot be read + // has no configuration: each member is then its own group, and it + // fails alone when it is planned. + std::vector> groups; + if (auto g = workspace_groups(sel.root, members)) groups = std::move(*g); + else for (auto const& mp : members) groups.push_back({mp}); + + mcpp::build::WorkspaceTestHooks hooks; + hooks.begin = member_begin; + hooks.end = member_end; + mcpp::build::run_workspace_tests(passthrough, ov, to, sel.root, groups, + std::move(inputs), hooks); } auto wsElapsed = ws_ms(); @@ -1101,7 +1149,7 @@ export int cmd_test(const mcpplibs::cmdline::ParsedArgs& parsed, "\"tests_not_run\":{},\"tests_built\":{}," "\"failed_members\":[{}],\"unrunnable_members\":[{}]," "\"not_run\":[{}],\"elapsed_ms\":{}}}}}", - members->size(), totalPassed, totalFailed, totalNotRun, + members.size(), totalPassed, totalFailed, totalNotRun, totalBuilt, join(failed), join(unrunnable), join(notRun), wsElapsed); std::fflush(stdout); @@ -1136,13 +1184,13 @@ export int cmd_test(const mcpplibs::cmdline::ParsedArgs& parsed, if (failed.empty() && notRun.empty() && unrunnable.empty()) mcpp::ui::status("workspace result", std::format("ok. {} member(s); {} passed; 0 failed{}; finished in {:.2f}s", - members->size(), totalPassed, notRunCounts, + members.size(), totalPassed, notRunCounts, static_cast(wsElapsed) / 1000.0)); else mcpp::ui::error(std::format( "workspace test: {}/{} member(s) failed; {} passed; {} failed{}; " "finished in {:.2f}s", - failed.size(), members->size(), totalPassed, totalFailed, notRunCounts, + failed.size(), members.size(), totalPassed, totalFailed, notRunCounts, static_cast(wsElapsed) / 1000.0)); if (!failed.empty()) mcpp::ui::plain(std::format(" failed members: {}", join_names(failed))); diff --git a/src/cli/selection.cppm b/src/cli/selection.cppm new file mode 100644 index 00000000..4e96749f --- /dev/null +++ b/src/cli/selection.cppm @@ -0,0 +1,186 @@ +// mcpp.cli.selection — which workspace members a command acts on. +// +// `build`, `test`, `mcpp emit build-database` and `pack` read the same +// selection from the same flags, so the members a command plans are the ones +// the flags name whatever the command is (member selection design 2026-09-30, +// S1). The selection is a SET of members: it is kept in `[workspace] members` +// order whatever order `-p` names them in, and a member named twice is +// selected once, so the plan does not depend on how the command line was +// spelled. +// +// The function is split in two. `select_members(ws, ...)` is pure in its +// inputs, which is what the unit tests exercise; the overload that takes a +// directory finds the workspace the directory belongs to and the member the +// directory is inside, and is the one the commands call. + +export module mcpp.cli.selection; + +import std; +import mcpp.manifest; +import mcpp.project; + +export namespace mcpp::cli { + +// The selectors of a command line, as written. +struct MemberRequest { + bool all = false; // --workspace + std::vector packages; // every -p/--package, in command-line order + std::vector excludes; // every --exclude, in command-line order +}; + +// The workspace a command acts on and the members it selects. +struct MemberSelection { + std::filesystem::path root; + // Each member as `[workspace] members` spells it, in that order; a rooted + // workspace's own package comes first, as "." (workspace design + // 2026-09-29 §7.1). + std::vector members; + // True when the selection is the workspace itself less what `--exclude` + // removed: `--workspace`, or a virtual root without `-p`. A command that + // reports per member (`test`) reports in its fan-out form for such a + // selection even when the workspace has one member. + bool whole = false; +}; + +// The member path `value` names, spelled as `[workspace] members` spells it +// ("." for a rooted workspace's own package). The value is resolved in the +// order docs/07 §5.3 states, by `mcpp::project::resolve_member_dir`: a +// qualified name, then a package name (refused, naming every match, when +// several members share it), then a directory. +std::expected +member_path_of(const mcpp::manifest::Manifest& ws, + const std::filesystem::path& wsRoot, + std::string_view value) { + auto dir = mcpp::project::resolve_member_dir(ws, wsRoot, value); + if (!dir) return std::unexpected(dir.error()); + std::string rel = "."; + if (!dir->empty()) { + const auto u8 = dir->lexically_normal() + .lexically_relative(wsRoot.lexically_normal()) + .generic_u8string(); + rel.assign(reinterpret_cast(u8.data()), u8.size()); + if (rel.empty()) rel = "."; + } + // The spelling the manifest uses, so that "./libs/core" and "libs/core" + // are one member to every reader of the selection. + for (auto const& mp : ws.workspace.members) + if (std::filesystem::path(mp).lexically_normal() == std::filesystem::path(rel)) + return mp; + return rel; +} + +// The selection over a workspace whose manifest is `ws` and whose root is +// `wsRoot`. `inside` is the member, as `[workspace] members` spells it, that +// the command's directory is inside; empty at the workspace root. +// +// | request | members | +// |------------------------------------------|----------------------------------| +// | `--workspace` | every member | +// | a virtual root, no `-p` | every member | +// | a rooted root, no `-p` | "." | +// | inside member X, no `-p` | X | +// | `-p X -p Y` | {X, Y} | +// | an "all" form above, with `--exclude Z` | every member but Z | +// +// Refused, before anything is planned: a `-p` that names no member (the +// refusal lists the members) or several (it names every match), `--exclude` +// together with `-p`, `--exclude` without an "all" form, an `--exclude` that +// names no member, and an `--exclude` that removes every member. +std::expected +select_members(const mcpp::manifest::Manifest& ws, + const std::filesystem::path& wsRoot, + std::string_view inside, + const MemberRequest& req) { + if (!req.excludes.empty() && !req.packages.empty()) + return std::unexpected(std::string( + "--exclude cannot be combined with -p: -p names the members a command acts on, " + "and --exclude removes members from a whole-workspace selection")); + + const bool rooted = !ws.package.name.empty(); + std::vector all; + if (rooted) all.push_back("."); + all.insert(all.end(), ws.workspace.members.begin(), ws.workspace.members.end()); + + MemberSelection sel{wsRoot, {}, false}; + if (req.all) { + sel.members = all; + sel.whole = true; + } else if (!req.packages.empty()) { + std::set named; + for (auto const& p : req.packages) { + auto mp = member_path_of(ws, wsRoot, p); + if (!mp) return std::unexpected(mp.error()); + named.insert(std::move(*mp)); + } + // Manifest order, and once each: the selection is a set. + for (auto const& mp : all) + if (named.contains(mp)) sel.members.push_back(mp); + } else if (!inside.empty()) { + sel.members = {std::string(inside)}; + } else if (!rooted) { + sel.members = all; + sel.whole = true; + } else { + sel.members = {"."}; + } + + if (sel.members.empty()) + return std::unexpected(std::string("the workspace lists no members")); + if (req.excludes.empty()) return sel; + + if (!sel.whole) + return std::unexpected(std::string( + "--exclude removes members from a whole-workspace selection: " + "add --workspace (a virtual workspace root selects every member without it)")); + std::set removed; + for (auto const& e : req.excludes) { + auto mp = member_path_of(ws, wsRoot, e); + if (!mp) return std::unexpected(std::format("--exclude '{}': {}", e, mp.error())); + removed.insert(std::move(*mp)); + } + std::erase_if(sel.members, [&](const std::string& mp) { return removed.contains(mp); }); + if (sel.members.empty()) + return std::unexpected(std::string( + "--exclude removes every member of the workspace, so nothing is left to act on")); + return sel; +} + +// The selection for a command run in `cwd`: nullopt outside a workspace (the +// command then acts on the one package it is in). Inside a member's directory +// the workspace is the one that lists the member, and `--workspace` there +// still means the whole of it. +std::expected, std::string> +select_members(const MemberRequest& req, + const std::filesystem::path& cwd = std::filesystem::current_path()) { + auto root = mcpp::project::find_manifest_root(cwd); + if (!root) return std::optional{}; + auto m = mcpp::manifest::load(*root / "mcpp.toml", {.insideWorkspace = true}); + // A manifest that cannot be read is reported by the planner, with its own + // diagnostic; it is not a selection question. + if (!m) return std::optional{}; + + auto outside = [&]() -> std::expected, std::string> { + if (!req.excludes.empty()) + return std::unexpected(std::string( + "--exclude names members of a workspace, and this directory is not in one")); + return std::optional{}; + }; + + std::filesystem::path wsRoot = *root; + std::string inside; + if (!m->workspace.present) { + wsRoot = mcpp::project::find_workspace_root(*root); + if (wsRoot.empty()) return outside(); + m = mcpp::manifest::load(wsRoot / "mcpp.toml"); + if (!m || !m->workspace.present) return outside(); + const auto rel = root->lexically_normal().lexically_relative(wsRoot.lexically_normal()); + for (auto const& mp : m->workspace.members) + if (std::filesystem::path(mp).lexically_normal() == rel) inside = mp; + if (inside.empty()) return outside(); + } + auto sel = select_members(*m, wsRoot, inside, req); + if (!sel) return std::unexpected(sel.error()); + return std::optional{std::move(*sel)}; +} + +} // namespace mcpp::cli diff --git a/tests/unit/test_member_selection.cpp b/tests/unit/test_member_selection.cpp new file mode 100644 index 00000000..1d356b55 --- /dev/null +++ b/tests/unit/test_member_selection.cpp @@ -0,0 +1,274 @@ +#include + +import std; +import mcpp.manifest; +import mcpp.cli.selection; + +// Member selection design 2026-09-30, S1 to S3. `select_members` is the one +// function every command that acts on workspace members reads its `-p`, +// `--workspace` and `--exclude` through, so the set a command plans does not +// depend on which command it is, or on the order `-p` names members in. The +// e2e halves (what the selected members build and test) are +// tests/e2e/852_… to 856_…. + +namespace { + +namespace fs = std::filesystem; + +struct Workspace { + fs::path root; + + explicit Workspace(std::string_view tag) { + root = fs::temp_directory_path() + / std::format("mcpp-select-{}-{:x}", tag, std::random_device{}()); + fs::create_directories(root); + } + ~Workspace() { + std::error_code ec; + fs::remove_all(root, ec); + } + Workspace(const Workspace&) = delete; + + void write(const fs::path& rel, std::string_view text) { + auto p = root / rel; + fs::create_directories(p.parent_path()); + std::ofstream(p) << text; + } + void member(std::string_view dir, std::string_view name, std::string_view ns = "") { + std::string toml = "[package]\n"; + if (!ns.empty()) toml += std::format("namespace = \"{}\"\n", ns); + toml += std::format("name = \"{}\"\nversion = \"0.1.0\"\n", name); + write(fs::path(dir) / "mcpp.toml", toml); + } + // A virtual workspace root (`[workspace]` alone) or a rooted one. + mcpp::manifest::Manifest virtual_root(std::string_view members) { + write("mcpp.toml", std::format("[workspace]\nmembers = [{}]\n", members)); + return load(); + } + mcpp::manifest::Manifest rooted_root(std::string_view members) { + write("mcpp.toml", std::format( + "[package]\nname = \"rootpkg\"\nversion = \"0.1.0\"\n\n[workspace]\nmembers = [{}]\n", + members)); + return load(); + } + mcpp::manifest::Manifest load() { + auto m = mcpp::manifest::load(root / "mcpp.toml"); + EXPECT_TRUE(m.has_value()) << (m ? "" : m.error().format()); + return m ? std::move(*m) : mcpp::manifest::Manifest{}; + } +}; + +using Members = std::vector; + +mcpp::cli::MemberRequest request(bool all, Members packages = {}, Members excludes = {}) { + return {all, std::move(packages), std::move(excludes)}; +} + +// Three members in `[workspace] members` order: a, b, c. +struct Three : Workspace { + Three() : Workspace("three") { + member("a", "a"); + member("b", "b"); + member("c", "c"); + } + mcpp::manifest::Manifest ws() { return virtual_root("\"a\", \"b\", \"c\""); } +}; + +} // namespace + +// S1: a virtual root without `-p` is the whole workspace, `--workspace` is +// the whole workspace anywhere, and both are the same set. +TEST(MemberSelection, AVirtualRootWithoutDashPAndWorkspaceAreEveryMember) { + Three f; + auto ws = f.ws(); + auto implicit = mcpp::cli::select_members(ws, f.root, "", request(false)); + ASSERT_TRUE(implicit.has_value()) << implicit.error(); + EXPECT_EQ(implicit->members, (Members{"a", "b", "c"})); + EXPECT_TRUE(implicit->whole); + + auto all = mcpp::cli::select_members(ws, f.root, "b", request(true)); + ASSERT_TRUE(all.has_value()) << all.error(); + EXPECT_EQ(all->members, (Members{"a", "b", "c"})); + EXPECT_TRUE(all->whole); +} + +// A rooted workspace's own package is "." and comes first. Without a selector +// it is the only member; `--workspace` adds the others. +TEST(MemberSelection, ARootedRootSelectsItsOwnPackageFirst) { + Workspace f("rooted"); + f.member("a", "a"); + f.member("b", "b"); + auto ws = f.rooted_root("\"a\", \"b\""); + + auto bare = mcpp::cli::select_members(ws, f.root, "", request(false)); + ASSERT_TRUE(bare.has_value()) << bare.error(); + EXPECT_EQ(bare->members, (Members{"."})); + EXPECT_FALSE(bare->whole); + + auto all = mcpp::cli::select_members(ws, f.root, "", request(true)); + ASSERT_TRUE(all.has_value()) << all.error(); + EXPECT_EQ(all->members, (Members{".", "a", "b"})); + + auto named = mcpp::cli::select_members(ws, f.root, "", request(false, {"b", "rootpkg"})); + ASSERT_TRUE(named.has_value()) << named.error(); + EXPECT_EQ(named->members, (Members{".", "b"})); +} + +// A command run inside a member's directory selects that member. +TEST(MemberSelection, InsideAMemberSelectsThatMember) { + Three f; + auto ws = f.ws(); + auto in = mcpp::cli::select_members(ws, f.root, "b", request(false)); + ASSERT_TRUE(in.has_value()) << in.error(); + EXPECT_EQ(in->members, (Members{"b"})); + EXPECT_FALSE(in->whole); +} + +// S1: `-p` names members, each resolved in the order docs/07 §5.3 states. The +// selection is a set kept in manifest order: the order `-p` was written in +// does not change it, and a member named twice, by two spellings, is one. +TEST(MemberSelection, SeveralDashPSelectTheirMembersOnceInManifestOrder) { + Workspace f("set"); + f.member("libs/a", "alpha"); + f.member("libs/b", "beta"); + f.member("libs/c", "gamma"); + auto ws = f.virtual_root("\"libs/a\", \"libs/b\", \"libs/c\""); + + auto ab = mcpp::cli::select_members(ws, f.root, "", request(false, {"alpha", "beta"})); + auto ba = mcpp::cli::select_members(ws, f.root, "", request(false, {"beta", "alpha"})); + ASSERT_TRUE(ab.has_value()) << ab.error(); + ASSERT_TRUE(ba.has_value()) << ba.error(); + EXPECT_EQ(ab->members, (Members{"libs/a", "libs/b"})); + EXPECT_EQ(ab->members, ba->members); + EXPECT_FALSE(ab->whole); + + // The package name, the member path and the directory's last segment are + // three spellings of one member. + auto same = mcpp::cli::select_members(ws, f.root, "", + request(false, {"alpha", "libs/a", "a", "./libs/a"})); + ASSERT_TRUE(same.has_value()) << same.error(); + EXPECT_EQ(same->members, (Members{"libs/a"})); +} + +// A `-p` that names no member is refused, and the refusal names it and lists +// the members; one that several members match is refused naming every match. +TEST(MemberSelection, AnUnknownOrAmbiguousDashPIsRefusedByName) { + Workspace f("refuse"); + f.member("a", "same", "ns1"); + f.member("b", "same", "ns2"); + f.member("c", "other"); + auto ws = f.virtual_root("\"a\", \"b\", \"c\""); + + auto unknown = mcpp::cli::select_members(ws, f.root, "", request(false, {"c", "nosuch"})); + ASSERT_FALSE(unknown.has_value()); + EXPECT_NE(unknown.error().find("nosuch"), std::string::npos) << unknown.error(); + EXPECT_NE(unknown.error().find("'c'"), std::string::npos) << unknown.error(); + + auto ambiguous = mcpp::cli::select_members(ws, f.root, "", request(false, {"same"})); + ASSERT_FALSE(ambiguous.has_value()); + EXPECT_NE(ambiguous.error().find("ns1.same"), std::string::npos) << ambiguous.error(); + EXPECT_NE(ambiguous.error().find("ns2.same"), std::string::npos) << ambiguous.error(); + + auto qualified = mcpp::cli::select_members(ws, f.root, "", + request(false, {"ns2.same", "ns1.same"})); + ASSERT_TRUE(qualified.has_value()) << qualified.error(); + EXPECT_EQ(qualified->members, (Members{"a", "b"})); +} + +// S3: `--exclude` removes members from either "all" form, by the same +// resolution as `-p`, and keeps manifest order. +TEST(MemberSelection, ExcludeRemovesMembersFromAnAllForm) { + Three f; + auto ws = f.ws(); + auto flag = mcpp::cli::select_members(ws, f.root, "", request(true, {}, {"c"})); + ASSERT_TRUE(flag.has_value()) << flag.error(); + EXPECT_EQ(flag->members, (Members{"a", "b"})); + EXPECT_TRUE(flag->whole); + + // The implicit whole selection of a virtual root, and two spellings. + auto implicit = mcpp::cli::select_members(ws, f.root, "", request(false, {}, {"a", "./c"})); + ASSERT_TRUE(implicit.has_value()) << implicit.error(); + EXPECT_EQ(implicit->members, (Members{"b"})); + + Workspace r("rooted-exclude"); + r.member("a", "a"); + auto rws = r.rooted_root("\"a\""); + auto root = mcpp::cli::select_members(rws, r.root, "", request(true, {}, {"rootpkg"})); + ASSERT_TRUE(root.has_value()) << root.error(); + EXPECT_EQ(root->members, (Members{"a"})); +} + +// S1, refusals: before anything is planned, `--exclude` with `-p`, `--exclude` +// that names no member, `--exclude` that leaves nothing, and `--exclude` +// where no "all" form applies. +TEST(MemberSelection, ExcludeIsRefusedWhereItHasNoMeaning) { + Three f; + auto ws = f.ws(); + + auto withP = mcpp::cli::select_members(ws, f.root, "", request(false, {"a"}, {"b"})); + ASSERT_FALSE(withP.has_value()); + EXPECT_NE(withP.error().find("--exclude"), std::string::npos) << withP.error(); + EXPECT_NE(withP.error().find("-p"), std::string::npos) << withP.error(); + + auto withPAndAll = mcpp::cli::select_members(ws, f.root, "", request(true, {"a"}, {"b"})); + EXPECT_FALSE(withPAndAll.has_value()); + + auto unknown = mcpp::cli::select_members(ws, f.root, "", request(true, {}, {"nosuch"})); + ASSERT_FALSE(unknown.has_value()); + EXPECT_NE(unknown.error().find("nosuch"), std::string::npos) << unknown.error(); + + auto everything = mcpp::cli::select_members(ws, f.root, "", request(true, {}, {"a", "b", "c"})); + ASSERT_FALSE(everything.has_value()); + EXPECT_NE(everything.error().find("every member"), std::string::npos) << everything.error(); + + // Inside a member, and at a rooted root, without `--workspace`: nothing is + // a whole-workspace selection to remove from. + auto inside = mcpp::cli::select_members(ws, f.root, "a", request(false, {}, {"b"})); + ASSERT_FALSE(inside.has_value()); + EXPECT_NE(inside.error().find("--workspace"), std::string::npos) << inside.error(); + + Workspace r("rooted-refuse"); + r.member("a", "a"); + auto rws = r.rooted_root("\"a\""); + EXPECT_FALSE(mcpp::cli::select_members(rws, r.root, "", request(false, {}, {"a"})).has_value()); +} + +// The overload that takes a directory finds the workspace the directory +// belongs to: from a member's directory, `-p` and `--workspace` still select +// among the members of the workspace that lists it. Outside a workspace there +// is no selection, and `--exclude` has nothing to name. +TEST(MemberSelection, TheDirectoryDecidesTheWorkspaceAndTheMemberItIsIn) { + Three f; + f.ws(); + + auto at_root = mcpp::cli::select_members(request(false), f.root); + ASSERT_TRUE(at_root.has_value()) << at_root.error(); + ASSERT_TRUE(at_root->has_value()); + EXPECT_EQ((*at_root)->members, (Members{"a", "b", "c"})); + + auto in_member = mcpp::cli::select_members(request(false), f.root / "b"); + ASSERT_TRUE(in_member.has_value()) << in_member.error(); + ASSERT_TRUE(in_member->has_value()); + EXPECT_EQ((*in_member)->members, (Members{"b"})); + EXPECT_EQ(std::filesystem::weakly_canonical((*in_member)->root), + std::filesystem::weakly_canonical(f.root)); + + auto several_from_member = mcpp::cli::select_members(request(false, {"a", "c"}), f.root / "b"); + ASSERT_TRUE(several_from_member.has_value()) << several_from_member.error(); + ASSERT_TRUE(several_from_member->has_value()); + EXPECT_EQ((*several_from_member)->members, (Members{"a", "c"})); + + auto all_from_member = mcpp::cli::select_members(request(true, {}, {"a"}), f.root / "b"); + ASSERT_TRUE(all_from_member.has_value()) << all_from_member.error(); + ASSERT_TRUE(all_from_member->has_value()); + EXPECT_EQ((*all_from_member)->members, (Members{"b", "c"})); + + Workspace alone("alone"); + alone.member(".", "solo"); + auto none = mcpp::cli::select_members(request(false), alone.root); + ASSERT_TRUE(none.has_value()) << none.error(); + EXPECT_FALSE(none->has_value()); + auto excluded = mcpp::cli::select_members(request(false, {}, {"x"}), alone.root); + ASSERT_FALSE(excluded.has_value()); + EXPECT_NE(excluded.error().find("not in one"), std::string::npos) << excluded.error(); +} From d0645c220612bbd65dde9907c6976632cc82c4db Mon Sep 17 00:00:00 2001 From: speak-agent <248744407+speak-agent@users.noreply.github.com> Date: Thu, 1 Oct 2026 01:32:27 +0800 Subject: [PATCH 04/26] test: e2e 852 to 856 for the member selection, --exclude, a test plan once per group, the group_build record and a failing member that fails alone --- ...ed_dash_p_selects_every_member_it_names.sh | 79 +++++++++ ..._one_member_and_exclude_removes_members.sh | 105 ++++++++++++ ...r_several_members_plans_and_builds_once.sh | 125 ++++++++++++++ ...d_test_build_is_reported_once_per_group.sh | 102 ++++++++++++ ...al_members_continues_past_a_failing_one.sh | 152 ++++++++++++++++++ 5 files changed, 563 insertions(+) create mode 100755 tests/e2e/852_a_repeated_dash_p_selects_every_member_it_names.sh create mode 100755 tests/e2e/853_run_keeps_one_member_and_exclude_removes_members.sh create mode 100755 tests/e2e/854_a_test_over_several_members_plans_and_builds_once.sh create mode 100755 tests/e2e/855_a_shared_test_build_is_reported_once_per_group.sh create mode 100755 tests/e2e/856_a_test_over_several_members_continues_past_a_failing_one.sh diff --git a/tests/e2e/852_a_repeated_dash_p_selects_every_member_it_names.sh b/tests/e2e/852_a_repeated_dash_p_selects_every_member_it_names.sh new file mode 100755 index 00000000..9474cea7 --- /dev/null +++ b/tests/e2e/852_a_repeated_dash_p_selects_every_member_it_names.sh @@ -0,0 +1,79 @@ +#!/usr/bin/env bash +# requires: unix-shell +# 852 -- a repeated `-p` selects every member it names, as a set, and a `-p` +# that names no member is refused before anything is planned. +# +# `-p` was declared with `takes_value()` and without `multiple()`, so a +# repeated `-p` kept the last value and dropped the others without a word +# (mcpp#750): `mcpp build -p a -p b` built `b` alone and exited 0. The +# selection is now one function for every command (member selection design +# 2026-09-30, S1 and S2); the members it returns are a set in `[workspace] +# members` order. +# +# Criteria: +# A. `mcpp build -p a -p b` in a workspace of `a`, `b` and `c` builds `a` and +# `b`. `c`'s object directory stays absent. +# B. `mcpp build -p a -p b` followed by `mcpp build -p b -p a` adds no +# compile edge to `.ninja_log`: the order `-p` was written in is not part +# of the selection, and a member named twice is one member. +# C. `mcpp build -p nosuch` exits non-zero before planning. Its message names +# `nosuch` and lists the members; a `-p` that is valid beside it does not +# rescue the command. +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" + +cat > mcpp.toml <<'EOF' +[workspace] +members = ["a", "b", "c"] +EOF +for m in a b c; do + mkdir -p $m/src + printf '[package]\nname = "%s"\nversion = "0.1.0"\n\n[targets.%s]\nkind = "lib"\n' $m $m > $m/mcpp.toml + printf 'export module sel852_%s;\nexport int %s_value() { return 1; }\n' $m $m > $m/src/$m.cppm +done + +# The build directory of a command: the one directory under target/ holding a +# build.ninja. +build_dir() { find target -name build.ninja -exec dirname {} \; | head -1; } +# How many times ninja built the object of member $2 in build directory $1. +object_edges() { awk -F'\t' -v m="$2" '$4 ~ ("(^|/)obj/" m "/src/" m "\\.m\\.o$")' "$1/.ninja_log" | wc -l | tr -d ' '; } + +# ── C ────────────────────────────────────────────────────────────────────── +# First, so that nothing has been planned, or built, that could be mistaken for +# what the refusal left behind. +rc=0 +"$MCPP" build -p a -p nosuch > c.log 2>&1 || rc=$? +[ "$rc" -ne 0 ] || fail "C: a -p that names no member was accepted" c.log +grep -q "nosuch" c.log || fail "C: the refusal does not name the member it could not find" c.log +for m in a b c; do + grep -qE "'$m'" c.log || fail "C: the refusal does not list member $m" c.log +done +! grep -q "Resolving toolchain" c.log || fail "C: the command planned before it refused" c.log +[ ! -d target ] || fail "C: the refused command left a build directory" c.log +echo "ok: C, an unknown -p is refused by name, with the members listed, before planning" + +# ── A ────────────────────────────────────────────────────────────────────── +"$MCPP" build -p a -p b > a.log 2>&1 || fail "A: -p a -p b did not build" a.log +dir=$(build_dir) +[ -n "$dir" ] || fail "A: no build directory" a.log +[ "$(object_edges "$dir" a)" = 1 ] || fail "A: a was not built" "$dir/.ninja_log" +[ "$(object_edges "$dir" b)" = 1 ] || fail "A: b was not built (the repeated -p kept one value)" "$dir/.ninja_log" +[ ! -e "$dir/obj/c" ] || fail "A: c was built although no -p named it" a.log +grep -q "Workspace building 2 members: a, b" a.log || fail "A: the plan does not state the two members" a.log +echo "ok: A, -p a -p b builds a and b, and c is untouched" + +# ── B ────────────────────────────────────────────────────────────────────── +"$MCPP" build -p b -p a > b.log 2>&1 || fail "B: -p b -p a did not build" b.log +[ "$(object_edges "$dir" a)" = 1 ] && [ "$(object_edges "$dir" b)" = 1 ] \ + || fail "B: the reverse order compiled an edge again" "$dir/.ninja_log" +"$MCPP" build -p a -p ./b -p b -p a > b2.log 2>&1 || fail "B: a member named twice did not build" b2.log +[ "$(object_edges "$dir" a)" = 1 ] && [ "$(object_edges "$dir" b)" = 1 ] \ + || fail "B: naming a member twice compiled an edge again" "$dir/.ninja_log" +[ "$(find target -name build.ninja | wc -l | tr -d ' ')" = 1 ] || fail "B: the order made a second build directory" b.log +echo "ok: B, the order of -p, and a member named twice, do not change the selection" + +echo "PASS: 852_a_repeated_dash_p_selects_every_member_it_names" diff --git a/tests/e2e/853_run_keeps_one_member_and_exclude_removes_members.sh b/tests/e2e/853_run_keeps_one_member_and_exclude_removes_members.sh new file mode 100755 index 00000000..5acfdedd --- /dev/null +++ b/tests/e2e/853_run_keeps_one_member_and_exclude_removes_members.sh @@ -0,0 +1,105 @@ +#!/usr/bin/env bash +# requires: unix-shell +# 853 -- `mcpp run` keeps one member, and `--exclude` removes members from a +# whole-workspace selection. +# +# `-p` is repeatable on `build`, `test` and `mcpp emit build-database`, and +# one selection function serves all of them (member selection design +# 2026-09-30, S1 to S3). `run` executes one program, so a second `-p` there is +# refused, naming both, and never read as "the last one". `--exclude ` +# removes members from the forms that select every member: `--workspace`, or a +# virtual root without `-p`. +# +# Criteria: +# D. `mcpp run -p a -p b` is refused, naming both, before anything is +# planned; `mcpp run -p a` runs `a`. +# E. `mcpp build --workspace --exclude c` builds `a` and `b` and leaves `c` +# untouched; so does `--exclude c` alone at a virtual root. `--exclude +# nosuch`, `-p a --exclude b`, and an `--exclude` that leaves no member +# are refused, each before planning; so is `--exclude` where no +# whole-workspace form applies. +# F. `mcpp test --workspace --exclude c` tests `a` and `b`, and `mcpp emit +# build-database` describes the members a selection names and no others. +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" + +cat > mcpp.toml <<'EOF' +[workspace] +members = ["a", "b", "c"] +EOF +for m in a b c; do + mkdir -p $m/src $m/tests + printf '[package]\nname = "%s"\nversion = "0.1.0"\n\n[targets.%s]\nkind = "bin"\nmain = "src/main.cpp"\n' $m $m > $m/mcpp.toml + printf '#include \nint main() { std::puts("hello from %s"); return 0; }\n' $m > $m/src/main.cpp + printf 'int main() { return 0; }\n' > $m/tests/smoke.cpp +done + +build_dir() { find target -name build.ninja -exec dirname {} \; | head -1; } +refused() { # refused