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