diff --git a/.agents/docs/2026-08-30-project-build-hooks-owned-intervals.md b/.agents/docs/2026-08-30-project-build-hooks-owned-intervals.md new file mode 100644 index 00000000..8a9a855b --- /dev/null +++ b/.agents/docs/2026-08-30-project-build-hooks-owned-intervals.md @@ -0,0 +1,239 @@ +# Project build hooks as owned intervals (#496) + +## The question this design answers + +The first shape of `[hooks]` gave `mcpp build` three commands — `build_start`, +`build_finished`, `build_failed` — each run to completion with a timeout, each +judged by its exit code. + +The request that followed was a background command: music that plays *for the +duration of the build* and stops when it ends, optionally restarting when the +player exits. Written as a knob it looks like + +```toml +build_start = { cmd = "play bgm.mp3", loop = true } +``` + +but `loop` is not the new thing. The new thing is that this command's life is +**longer than the moment that started it**. Adding `background = true` to +`build_start` would give that key two incompatible meanings and would silently +change what `timeout_seconds` and `side_effect` mean for one of them — a table +whose keys mean different things depending on a sibling key is the shape this +design exists to avoid. + +## The model + +> A hook is a command mcpp **owns for an interval**. The event names the +> interval. mcpp starts the command when the interval opens and ends it when +> the interval closes. + +| Event | The interval opens | The interval closes | +|---|---|---| +| `build_start` | after preparation, before the build | when the command exits | +| `build_finished` | after a build that succeeded | when the command exits | +| `build_failed` | after a build that failed | when the command exits | +| `during_build` | after preparation, before the build | after the build, before the terminal hook | + +The first three have **self-closing** intervals. "Synchronous" is not a +separate mode in this model — it is what an interval closed by the command +itself looks like. `during_build` is the one interval closed by something else, +and everything that reads as a special case for it falls out of that single +difference rather than being declared: + +- **`timeout_seconds` bounds one run of the command.** For a self-closing + interval that is the whole hook. For `during_build` the build already bounds + it, so the key is *rejected* there rather than accepted and reinterpreted — + a per-run cap would only be enforceable when `loop` is on (nothing is + polling otherwise), and a key that works under one sibling setting and not + another is the hole this design is trying not to dig. +- **`loop` restarts a command that exits before its interval closes.** A + self-closing interval ends *when the command exits*, so `loop` can never fire + there. It is rejected on those events rather than accepted and ignored, with + a message naming `during_build`. +- **`side_effect` is unchanged**: does a hook failure fail the build. For + `during_build`, "failure" means *failed to start*, or *failed to stay up* + (below). Being stopped because the interval closed is not a failure. + +Ordering inside the lifecycle: + +```text +during_build opens +build_start + ├─ build succeeds → during_build closes → build_finished + └─ build fails → during_build closes → build_failed +``` + +`during_build` closes **before** the terminal hook, not after. A "build +finished" sound playing over the background music it was supposed to replace is +the whole reason the order is fixed rather than incidental. + +## Schema + +Every event value is a string or a table; the string is sugar. + +```toml +[hooks] +build_start = "echo start" # = { cmd = "echo start" } +build_finished = "notify-send 'build finished'" +during_build = { cmd = "mcpp-hooks-audioplayer bgm", loop = true } + +# Table-level, unchanged. +timeout_seconds = 10 +enabled = true +side_effect = true +``` + +Per-event table keys: `cmd` (required, non-empty), `timeout_seconds` +(overrides the table default for this event), `loop` (`during_build` only). + +The string-or-table pattern is already how `[dependencies]` and +`[resources].version-info` are spelled, so it introduces no new parsing +semantics — Appendix A of docs/05 is satisfied for the same reason the first +shape satisfied it: fixed keys, open values, and no key that duplicates an +answer another section already gives. + +## What the feature actually costs + +The schema is the small half. A process whose life spans the build needs three +things mcpp does not have. + +### 1. A process group, because SIGKILL on a pid is not enough + +`modules/platform/src/unix/bounded_process.cppm:216` kills the direct child: + +```cpp +::kill(pid, SIGKILL); +``` + +For `sh -c "sleep 5"` that is sufficient — the shell execs the command. For +`sh -c "while :; do play a.mp3; done"` it is not: the shell dies and `play` +survives. **Music that cannot be stopped, from a process the user cannot name, +is the worst failure this feature can have**, and it is the default outcome +without process groups. + +So the POSIX side gains `posix_spawnattr_setpgroup` + `killpg`. Windows already +has the right shape: a Job object with `JOB_OBJECT_LIMIT_KILL_ON_JOB_CLOSE` +(`windows/bounded_process.cppm:205`), which takes the whole tree — and takes it +even if mcpp dies, because the handle closes with the process. + +This is worth doing on its own terms: the same gap is why +`mcpp test --timeout` and `[build] build_program_timeout` can leave +grandchildren behind today. + +### 2. A signal handler, because a process group stops receiving Ctrl-C + +The two requirements fight each other. A child in its own process group no +longer receives the terminal's SIGINT — which is what makes `killpg` possible +and *also* what makes Ctrl-C leave the music playing. Both halves are needed: +its own group, plus a SIGINT/SIGTERM handler in mcpp that stops the group +before re-raising. + +mcpp installs no signal handlers today. The one added here is the minimum that +is async-signal-safe: a `volatile sig_atomic_t` holding the process-group id, a +handler that calls `killpg` (which is async-signal-safe) and re-raises the +default action. It is installed only while a spanning hook is running. + +Windows needs no equivalent for correctness — the job object already covers +process death — but `SetConsoleCtrlHandler` is installed for a clean stop. + +### 3. A restart floor, because `loop` on a typo is a fork bomb + +`loop = true` with `play /nonexistant` restarts thousands of times per second +for the length of the build. Two bounds, both fixed in v1 rather than +configurable, because a knob whose wrong value is a spin is not a knob: + +- **250 ms between runs.** +- **Five consecutive runs that exited non-zero in under a second** stops the + loop and reports a hook failure: *"during_build command failed to stay up".* + Whether that fails the build is `side_effect`, as everywhere else. + +A supervisor thread exists **only when `loop = true`**. Without it, a spanning +hook is a spawn and a stop, and no thread is created. + +## Stopping + +POSIX: `SIGTERM` to the group, a 2 s grace period, then `SIGKILL`. A player +asked to stop should get to close its audio device. + +Windows: the job object is closed, which terminates the tree at once. There is +no graceful equivalent that does not require enumerating the job's processes +and posting window messages; the asymmetry is stated rather than hidden. + +⚠️ The obvious symmetry — "ask with `GenerateConsoleCtrlEvent(CTRL_BREAK_EVENT, +pid)` first" — was written, and it is wrong in a way no local run shows. That +call addresses a process GROUP attached to the caller's console, not a process. +When the id does not name a live group of ours (and it does not, once the child +has exited — `start /b`-style commands exit immediately) the event reaches +everything sharing the console. Measured on the Windows e2e runner: the entire +suite died eleven seconds into the hooks test with exit code `-1073741510` +(`0xC000013A`, `STATUS_CONTROL_C_EXIT`) and printed no summary, because mcpp had +sent Ctrl-Break to its own console. The design said Windows has no graceful stop; +the first implementation did not believe it. The code now matches the design. + +## Output + +A self-closing hook inherits stdio, which is safe because nothing else is +writing at that moment. A spanning hook writes **concurrently with ninja** and +would interleave into the middle of a compiler diagnostic. Its output is +therefore discarded by default, and inherited under `--verbose` — no new schema +key, and the answer to "why is there no music" is one flag away. + +## Shipping it experimental + +The feature ships with `side_effect` defaulting to **false** and `true` +*refused* by the manifest parser. A hook therefore cannot change whether a +build succeeded: every failure is a warning and `mcpp build` keeps the result +it earned on its own. + +Refused rather than downgraded, because both silent behaviours are worse than +an error. Honouring `true` ships an experimental feature with a veto over every +build. Ignoring it leaves a project believing its build is gated on a notifier +when nothing is — the "accepted and does nothing" shape this design rejects +everywhere else (`loop` on a self-closing event, `timeout_seconds` on +`during_build`), so it would be inconsistent to make an exception for the one +key whose wrong answer is invisible. + +The key stays in the schema so manifests do not have to change when the feature +is promoted, and the mechanism under it already implements both values — the +`sideEffect == true` branch in `mcpp.hooks` is unreachable today ON PURPOSE. +**Promotion is the deletion of one block in the parser**, not a +reconstruction. That is the property to preserve when editing either side. + +Two further limits are permanent rather than provisional and should not be read +as part of the experiment: + +- Only the ROOT project's hooks run. A dependency's `[hooks]` is inert by + construction — there is exactly one `Span` construction and there are exactly + two `invoke` call sites, all in `run_build_with_hooks`, all fed from the + context's own manifest. +- Only `mcpp build` runs hooks. `mcpp run`, `mcpp test` and + `mcpp build --configure-only` build too, and deliberately do not. + +## Criteria + +The assertions this design has to earn are about *state*, not about log lines — +"we called stop" is not evidence that anything stopped. + +1. **It runs during the build.** The command appends a heartbeat line every + 200 ms; the file is non-empty when the build ends. +2. **It is stopped.** Record the heartbeat file's size after `mcpp build` + returns, wait one second, read it again: unchanged. A log line saying + "stopped" would pass whether or not the process died. +3. **`loop` restarts it.** A command that exits immediately produces a heartbeat + count that grows across the build; without `loop` it produces exactly one. +4. **The whole tree dies, not just the shell.** The command is + `sh -c '... & wait'`, so the writer is a grandchild. This is the case + `kill(pid)` misses and `killpg` catches, and it is the only assertion that + distinguishes the fix from the bug. +5. **The failure cap trips.** A command that fails instantly stops after five + attempts and reports; the heartbeat count is bounded, not "large". +6. **Ctrl-C leaves nothing behind.** POSIX only: `mcpp build` in the + background, `kill -INT`, then criterion 2. Sending Ctrl-C to another process + on Windows needs a helper this suite does not have; the gap is declared + rather than papered over with a test that passes vacuously. +7. **A self-closing event rejects `loop`.** Otherwise the key is accepted, + does nothing, and the user concludes the feature does not work. +8. **`side_effect = true` is refused, and every failure mode is a warning.** + Two assertions, not one: the exit code says the hook had no vote, and the + warning says the failure was not swallowed. A test that checked only the + exit code would pass just as well if hooks had stopped running altogether. diff --git a/docs/05-mcpp-toml.md b/docs/05-mcpp-toml.md index 3dc5ce0c..4b2810f8 100644 --- a/docs/05-mcpp-toml.md +++ b/docs/05-mcpp-toml.md @@ -2094,6 +2094,172 @@ See [07 — build.mcpp](07-build-mcpp.md). Naming such a file in command: nothing tracks it, and editing the file produces `ninja: no work to do`. +### 2.16 `[hooks]` — Project Build Lifecycle Commands (experimental) + +> **Experimental.** A hook cannot currently decide whether a build succeeded. +> Every hook failure is reported as a **warning** and `mcpp build` keeps the +> result it earned on its own; `side_effect = true` is refused with an error +> rather than honoured. The key stays in the schema so that manifests written +> today do not have to change when the feature is promoted. Two further limits +> are permanent rather than provisional: only the root project's hooks run, and +> only `mcpp build` runs them. + +A hook is a command `mcpp build` **owns for an interval**, and the event names +the interval: + +```toml +[hooks] +build_start = "echo build started" +build_failed = "notify-send 'build failed'" +build_finished = "notify-send 'build finished'" + +# Runs alongside the build and is stopped when it ends. +during_build = { cmd = "mcpp-hooks-audioplayer bgm", loop = true } + +# Optional; these are the defaults. +timeout_seconds = 10 +enabled = true +side_effect = false # `true` is refused while this is experimental +``` + +| Key | Type | Default | The interval it names | +|---|---|---:|---| +| `build_start` | command | — | Opens after project preparation, closes when the command exits | +| `build_finished` | command | — | Opens after a build that succeeded, closes when the command exits | +| `build_failed` | command | — | Opens after a build that failed, closes when the command exits | +| `during_build` | command | — | Opens before the build, closes after it | +| `timeout_seconds` | integer, 1–86400 | `10` | Bounds one run of a command | +| `enabled` | bool | `true` | Enables all commands in this table | +| `side_effect` | bool | `false` | Whether a hook failure makes the build fail. **Reserved** — only `false` is accepted while this is experimental | + +The first three intervals are **self-closing** — they end when the command +does. "Synchronous" is not a separate mode here; it is what a self-closing +interval looks like. `during_build` is the one interval closed by something +else, and the two keys that only make sense for one shape follow from that +rather than being exceptions. + +A command is a string, or a table when it needs options: + +| Table key | Applies to | Meaning | +|---|---|---| +| `cmd` | every event | The command. Required. | +| `timeout_seconds` | self-closing events | Overrides the table default for this event | +| `loop` | `during_build` | Restart the command if it exits before the build ends | + +`loop` on a self-closing event and `timeout_seconds` on `during_build` are both +**errors**, not ignored keys: a self-closing interval ends when its command +exits, so there is nothing to restart, and `during_build` is already bounded by +the build. A key that is accepted and does nothing reads as a broken feature. + +Commands run through the host shell (`/bin/sh` or `cmd.exe`), with the +**project root** as their working directory — not the directory `mcpp build` +was typed in, so a relative path in a hook means the same thing wherever the +build was started. A self-closing command keeps ordinary terminal +input/output. Missing event commands are skipped. + +The lifecycle is: + +```text +during_build opens +build_start + ├─ build succeeds → during_build closes → build_finished + └─ build fails → during_build closes → build_failed +``` + +`during_build` closes **before** the terminal hook, so a "build finished" sound +is not competing with the background music it replaces. + +`build_failed` and `build_finished` are mutually exclusive, and both are +reachable only after `build_start` has run. A project that cannot be *prepared* +— an invalid manifest, an unresolvable dependency, no usable toolchain — fires +nothing: it has not started building, and its hook program may be exactly what +preparation would have installed. + +A hook command that cannot start, returns non-zero, or exceeds its timeout is a +hook failure. For `during_build` there is one more: a looped command that +**fails to stay up** — five consecutive runs ending unsuccessfully within a +second — stops being restarted and is reported. (A command that finishes +quickly and *successfully* is doing exactly what `loop` was asked to repeat, +and is not a failure.) Every one of those is reported as a **warning**, and the +build keeps the result it earned on its own — while `[hooks]` is experimental +it does not get a vote. A hook's own failure does not trigger another hook. + +`side_effect = true` is what will change that, and asking for it today is an +error: + +```text +error: mcpp.toml: error: [hooks].side_effect = true is not available yet: +[hooks] is experimental and cannot decide whether a build succeeded. … +``` + +Refused rather than quietly downgraded, because both silent options are worse: +honouring it would give an experimental feature a veto over every build, and +ignoring it would leave a project believing its build is gated on a notifier +when nothing is. When the feature is promoted, `true` will mean "a hook failure +fails the build" — and a build that failed on its own will still keep its own +exit code, so `mcpp build` never reports a compile error as a notifier problem. + +Two things are worth knowing about a `during_build` command specifically: + +- **Its output is discarded**, because it writes concurrently with the build + and would otherwise land in the middle of a compiler diagnostic. Run + `mcpp build --verbose` to see it. +- **It is stopped as a process tree**, not as a process. `player & wait` makes + the player a grandchild of the command mcpp started, and stopping only the + latter would leave the audio device held after the build. mcpp puts the + command in its own process group (a job object on Windows) and stops that, + including when the build is interrupted with Ctrl-C. + +Scope, precisely: + +- Only `mcpp build` runs hooks. `mcpp run`, `mcpp test` and + `mcpp build --configure-only` build too, and deliberately do not. +- Hooks belong to the **package being built**. In a workspace fan-out that is + each member in turn — its own `[hooks]`, around its own build, in its own + root. A *virtual* workspace root (`[workspace]` with no `[package]`) builds + nothing, so a `[hooks]` table there never fires. +- A dependency's `[hooks]` is **skipped**, always. Only the root project's run. + Every manifest mcpp parses carries the section, a dependency's included, and + nothing reads it — which is what keeps `mcpp add` from meaning "run this + author's shell command on my next build". This is a property of the design, + not a default awaiting a switch. +- Declaring an active hook opts the project out of the no-op fast path, because + `build_start` is specified to run after preparation. Expect `mcpp build` on an + already-current hooked project to cost a preparation pass rather than + milliseconds. + +An unrecognised key in `[hooks]`, or inside one event's table, is a warning (an +error under `--strict`), so a manifest written for a newer mcpp still loads. An +unrecognised *value* — a missing or non-string `cmd`, a `timeout_seconds` +outside 1–86400, a key offered to the wrong interval — is a manifest error. + +> **A hook is code, and `mcpp.toml` is part of the repository.** Building a +> freshly cloned project runs whatever its `[hooks]` say, with the privileges +> of whoever invoked `mcpp build`. This is the same trust `build.mcpp` already +> asks for ([07 — build.mcpp](07-build-mcpp.md)); `[hooks]` widens its reach +> rather than introducing it. + +Hook programs can be installed as ordinary xlings dependencies. For example, +an audio notifier can keep its sound files inside its own executable rather +than adding media handling to mcpp: + +```toml +[hooks] +during_build = { cmd = "mcpp-hooks-audioplayer bgm", loop = true } +build_finished = "mcpp-hooks-audioplayer niulai-mm" +build_failed = "mcpp-hooks-audioplayer niulai-niulai" +side_effect = false + +[xlings] +deps = ["xim:mcpp-hooks-audioplayer@0.0.1"] +``` + +Background music for the length of the build, and a different sound for how it +ended. `side_effect = false` is written out rather than left to the default: +it is the value this manifest wants on its own terms — a missing audio device +should never fail a build — so it will still say so once the key has more than +one accepted value. + ## Appendix A. Schema Ownership Principle (admission criteria for new fields) > **Closed syntax, open vocabulary**: whoever owns the parsing semantics defines the keys; whoever owns the domain knowledge defines the values. diff --git a/docs/zh/05-mcpp-toml.md b/docs/zh/05-mcpp-toml.md index 1ff98fc0..2cbde30e 100644 --- a/docs/zh/05-mcpp-toml.md +++ b/docs/zh/05-mcpp-toml.md @@ -1791,6 +1791,143 @@ o.arg("./mkblob.sh").arg("blob.bin").arg("${mcpp.out_dir}/blob.o") 但 ldflags 是链接命令里的一串字符:没有任何东西跟踪它,改了它得到的是 `ninja: no work to do`。 +### 2.16 `[hooks]` —— 项目构建生命周期命令(实验性) + +> **实验性。** Hook 目前**不能**决定一次构建成功与否。所有 Hook 失败都以 +> **warning** 报出,`mcpp build` 保留它自己挣来的结果;`side_effect = true` +> 会被报错拒绝,而不是被采纳。这个键保留在 schema 里,这样今天写下的 manifest +> 在该功能转正时无需改动。另有两条限制是永久的、不是临时的:**只有根项目的 Hook +> 会执行**,而且**只有 `mcpp build` 会执行它们**。 + +Hook 是 `mcpp build` **在一段区间内持有**的命令,事件名就是那段区间: + +```toml +[hooks] +build_start = "echo build started" +build_failed = "notify-send 'build failed'" +build_finished = "notify-send 'build finished'" + +# 与构建并行,构建结束时被停止。 +during_build = { cmd = "mcpp-hooks-audioplayer bgm", loop = true } + +# 可选;以下是默认值。 +timeout_seconds = 10 +enabled = true +side_effect = false # 实验期内 `true` 会被拒绝 +``` + +| 键 | 类型 | 默认值 | 它命名的区间 | +|---|---|---:|---| +| `build_start` | 命令 | — | 项目准备完成后开启,命令退出时闭合 | +| `build_finished` | 命令 | — | 构建成功后开启,命令退出时闭合 | +| `build_failed` | 命令 | — | 构建失败后开启,命令退出时闭合 | +| `during_build` | 命令 | — | 构建开始前开启,构建结束后闭合 | +| `timeout_seconds` | 整数,1–86400 | `10` | 单次运行的时限 | +| `enabled` | 布尔 | `true` | 是否启用本表中的全部命令 | +| `side_effect` | 布尔 | `false` | Hook 失败是否让本次构建失败。**保留键**——实验期内只接受 `false` | + +前三个区间是**自闭合**的——命令退出,区间就结束。"同步"在这里不是一种单独的模式, +它就是自闭合区间的样子。`during_build` 是唯一由别的东西闭合的区间,而那两个只对其中 +一种形状有意义的键,是从这一点推出来的,不是额外规定的例外。 + +命令写成字符串;需要选项时写成表: + +| 表内键 | 适用于 | 含义 | +|---|---|---| +| `cmd` | 所有事件 | 命令本身,必填 | +| `timeout_seconds` | 自闭合事件 | 覆盖本表默认值 | +| `loop` | `during_build` | 命令在区间闭合前退出时重新启动 | + +`loop` 写在自闭合事件上、`timeout_seconds` 写在 `during_build` 上,都是**错误**而不是 +被忽略的键:自闭合区间随命令退出而结束,没有东西可重启;而 `during_build` 已经由构建 +定界。一个被接受却什么都不做的键,读起来就是"这功能坏了"。 + +命令通过宿主 Shell(`/bin/sh` 或 `cmd.exe`)执行,工作目录是**项目根目录**——不是敲 +`mcpp build` 的那个目录,所以 Hook 里的相对路径在哪儿发起构建都指同一处。自闭合命令 +的标准输入、输出和错误沿用普通终端行为。没有配置的事件直接跳过。 + +生命周期为: + +```text +during_build 开启 +build_start + ├─ 构建成功 → during_build 闭合 → build_finished + └─ 构建失败 → during_build 闭合 → build_failed +``` + +`during_build` 在终止 Hook **之前**闭合,这样"构建完成"的提示音不会和它要替换掉的 +背景音乐撞在一起。 + +`build_failed` 与 `build_finished` 互斥,而且两者都只在 `build_start` 已经执行之后 +才可达。项目**准备**阶段就失败的情况——manifest 非法、依赖无法解析、没有可用工具链 +——一个 Hook 都不触发:此时构建尚未开始,而 Hook 程序本身可能正是准备阶段要装的东西。 + +命令无法启动、返回非零或超过时限均视为 Hook 失败。`during_build` 还多一种:开了 `loop` +的命令**起不来**——连续五次在一秒内以非零状态结束——就不再重启,并被报出来。(很快就 +成功结束的命令,正是 `loop` 被要求重复的那件事,不算失败。)以上每一种都以 **warning** +报出,构建保留它自己挣来的结果——`[hooks]` 还在实验期,它没有投票权。Hook 自身失败不会 +再触发另一个 Hook。 + +改变这一点的正是 `side_effect = true`,而今天写它是一个错误: + +```text +error: mcpp.toml: error: [hooks].side_effect = true is not available yet: +[hooks] is experimental and cannot decide whether a build succeeded. … +``` + +是拒绝而不是悄悄降级,因为两种沉默的做法都更糟:采纳它等于让一个实验性功能对每一次 +构建都有否决权;忽略它则让项目以为自己的构建被通知程序把着关,而实际上没有。功能转正 +后,`true` 的含义是"Hook 失败让构建失败"——而构建自身失败时仍保留它自己的退出码,所以 +`mcpp build` 不会把一次编译错误报成通知程序的问题。 + +关于 `during_build` 有两件事值得单独知道: + +- **它的输出被丢弃**,因为它与构建并发写出,否则会插进某条编译诊断的中间。要看它的 + 输出就跑 `mcpp build --verbose`。 +- **停止的单位是进程树,不是进程。** `player & wait` 让播放器成为 mcpp 所启动那条命令 + 的孙子进程,只停掉后者会让音频设备在构建结束后仍被占着。mcpp 把命令放进它自己的 + 进程组(Windows 上是 job object)并停止整组,构建被 Ctrl-C 打断时也一样。 + +作用范围: + +- 只有 `mcpp build` 执行 Hook。`mcpp run`、`mcpp test` 和 + `mcpp build --configure-only` 同样会构建,但有意不执行。 +- Hook 属于**被构建的那个包**。workspace 展开时就是逐个成员:各自的 `[hooks]`、 + 各自的构建、各自的根目录。**虚拟** workspace 根(只有 `[workspace]` 没有 + `[package]`)不构建任何东西,写在那里的 `[hooks]` 永不触发。 +- 依赖的 `[hooks]` **一律跳过**,只有根项目的会执行。mcpp 解析的每一份 manifest 都 + 带着这一节,依赖的也带,而没有任何东西去读它——这正是"装一个包"不会变成"在我下次 + 构建时跑包作者的 Shell 命令"的原因。这是设计的性质,不是一个等着被打开的默认值。 +- 声明了生效的 Hook 就等于让项目放弃空转快路径,因为 `build_start` 规定在准备阶段之后 + 执行。对已经是最新状态的带 Hook 项目,`mcpp build` 的代价是一次准备,而不是毫秒级。 + +`[hooks]` 里、以及某个事件表里不认识的**键**都是 warning(`--strict` 下为错误),所以为更新版 mcpp 写的 +manifest 在这一版仍能加载;不认识的**值**——`cmd` 缺失或不是字符串、`timeout_seconds` 不在 +1–86400 之间、键写给了错误的区间——是 manifest 错误。 + +> **Hook 是代码,而 `mcpp.toml` 是仓库的一部分。** 构建一个刚克隆下来的项目,会以 +> 执行 `mcpp build` 的那个账户的权限,运行它 `[hooks]` 里写的任何东西。这与 +> `build.mcpp`([07 — build.mcpp](07-build-mcpp.md))已经要求的信任是同一份; +> `[hooks]` 扩大的是它的范围,而不是引入了一份新的信任。 + +Hook 程序可以作为普通 xlings 依赖安装。例如,音频通知程序可以把音频内置进自己的 +可执行文件,无需让 mcpp 处理媒体资源: + +```toml +[hooks] +during_build = { cmd = "mcpp-hooks-audioplayer bgm", loop = true } +build_finished = "mcpp-hooks-audioplayer niulai-mm" +build_failed = "mcpp-hooks-audioplayer niulai-niulai" +side_effect = false + +[xlings] +deps = ["xim:mcpp-hooks-audioplayer@0.0.1"] +``` + +构建全程的背景音乐,加上一段区分结果的提示音。`side_effect = false` 写出来而不是靠 +默认值:它是这份 manifest 自己就想要的值——缺个音频设备不该让构建失败——所以等这个键 +有了不止一个可接受的值之后,它仍然会这么写。 + ## 附录 A. Schema 所有权原则(新字段准入标准) > **语法封闭,词汇开放**:谁拥有解析语义谁定义键;谁拥有领域知识谁定义值。 diff --git a/modules/manifest/src/toml.cppm b/modules/manifest/src/toml.cppm index c20773fa..1c61e222 100644 --- a/modules/manifest/src/toml.cppm +++ b/modules/manifest/src/toml.cppm @@ -1520,6 +1520,172 @@ std::expected parse_string(std::string_view content, } } + // [hooks] — project build lifecycle commands (#496). Parsed HERE rather + // than by the module that runs them, for the reason Appendix A of + // docs/05-mcpp-toml.md states: mcpp.toml has one grammar and one parser. + // A second reader of the same file would report ITS syntax errors in its + // own vocabulary — a typo in [package] arriving as "invalid hook + // configuration" — and would sit outside the warning/--strict policy every + // other section is subject to. + if (auto* hooksValue = doc->get("hooks"); + hooksValue && !hooksValue->is_table()) { + return std::unexpected(error(origin, + "[hooks] must be a table of lifecycle commands")); + } + if (auto* ht = doc->get_table("hooks")) { + // Bounded above as well as below: the value becomes a + // std::chrono::seconds deadline, and "a timeout so large it is not + // one" is a mistake worth naming rather than honouring. One constant, + // used by both the table-level default and the per-event override, so + // the two cannot disagree about what they accept. + constexpr std::int64_t kMaxHookTimeout = 24 * 60 * 60; + + // Values are the author's own and visible in front of them: a wrong + // type is an error, not a silent default. An unrecognised KEY is a + // warning (--strict makes it an error), same split as [build] — so a + // manifest written for a later mcpp still loads on this one. + // + // A command is a string or a table. `spanning` says which INTERVAL the + // event names, and that decides which table keys exist: a self-closing + // interval can be bounded (`timeout_seconds`) but can never be + // restarted (`loop`), and a spanning one is the reverse. A key offered + // to the wrong event is an error naming the right one — accepted and + // ignored, it would read as "the feature does not work". + auto read_command = [&](std::string_view key, HookCommand& out, + bool spanning) -> std::optional { + auto it = ht->find(key); + if (it == ht->end()) return std::nullopt; + + auto const& value = it->second; + if (value.is_string()) { + if (value.as_string().empty()) + return error(origin, std::format( + "[hooks].{} must be a non-empty command string", key)); + out.cmd = value.as_string(); + return std::nullopt; + } + if (!value.is_table()) + return error(origin, std::format( + "[hooks].{} must be a command string or a table with `cmd`", + key)); + + auto const& t = value.as_table(); + auto ci = t.find("cmd"); + if (ci == t.end() || !ci->second.is_string() + || ci->second.as_string().empty()) + return error(origin, std::format( + "[hooks].{}.cmd must be a non-empty command string", key)); + out.cmd = ci->second.as_string(); + + if (auto ti = t.find("timeout_seconds"); ti != t.end()) { + if (spanning) + return error(origin, std::format( + "[hooks].{}.timeout_seconds does not apply: this " + "command runs for as long as the build, which bounds " + "it", key)); + if (!ti->second.is_int() || ti->second.as_int() <= 0 + || ti->second.as_int() > kMaxHookTimeout) + return error(origin, std::format( + "[hooks].{}.timeout_seconds must be a positive integer " + "(seconds, at most {})", key, kMaxHookTimeout)); + out.timeoutSeconds = static_cast(ti->second.as_int()); + } + if (auto li = t.find("loop"); li != t.end()) { + if (!spanning) + return error(origin, std::format( + "[hooks].{}.loop does not apply: this command's " + "interval ends when it exits, so there is nothing to " + "restart. `during_build` is the event that spans the " + "build", key)); + if (!li->second.is_bool()) + return error(origin, + std::format("[hooks].{}.loop must be a boolean", key)); + out.loop = li->second.as_bool(); + } + + static constexpr std::string_view kSelfClosingKeys[] = { + "cmd", "timeout_seconds" }; + static constexpr std::string_view kSpanningKeys[] = { "cmd", "loop" }; + for (auto& [k, _] : t) { + bool known = false; + if (spanning) { + for (auto kk : kSpanningKeys) if (k == kk) known = true; + } else { + for (auto kk : kSelfClosingKeys) if (k == kk) known = true; + } + if (!known) + m.schemaWarnings.push_back(std::format( + "[hooks].{} has unsupported key '{}' (ignored). Keys: {}.", + key, k, spanning ? "cmd, loop" : "cmd, timeout_seconds")); + } + return std::nullopt; + }; + for (auto [key, out, spanning] : std::initializer_list< + std::tuple>{ + {"build_start", &m.hooks.buildStart, false}, + {"build_failed", &m.hooks.buildFailed, false}, + {"build_finished", &m.hooks.buildFinished, false}, + {"during_build", &m.hooks.duringBuild, true}}) { + if (auto e = read_command(key, *out, spanning)) + return std::unexpected(*e); + } + + if (auto it = ht->find("timeout_seconds"); it != ht->end()) { + if (!it->second.is_int() || it->second.as_int() <= 0 + || it->second.as_int() > kMaxHookTimeout) + return std::unexpected(error(origin, std::format( + "[hooks].timeout_seconds must be a positive integer " + "(seconds, at most {})", kMaxHookTimeout))); + m.hooks.timeoutSeconds = static_cast(it->second.as_int()); + } + + for (auto [key, out] : std::initializer_list< + std::pair>{ + {"enabled", &m.hooks.enabled}, + {"side_effect", &m.hooks.sideEffect}}) { + auto it = ht->find(key); + if (it == ht->end()) continue; + if (!it->second.is_bool()) + return std::unexpected(error(origin, std::format( + "[hooks].{} must be a boolean", key))); + *out = it->second.as_bool(); + } + + // ⚠️ THE EXPERIMENTAL GATE, AND THE WHOLE OF IT. + // + // `[hooks]` is experimental, so it may not decide whether a build + // succeeded. Asking for `side_effect = true` is refused rather than + // downgraded, because the two possible silent behaviours are both + // worse than an error: honouring it ships an experimental feature with + // a veto over every build, and ignoring it leaves a project believing + // its build is gated on a notifier when nothing is. + // + // Everything under this line already implements both values. Deleting + // this block is what promoting the feature consists of. + if (m.hooks.sideEffect) + return std::unexpected(error(origin, + "[hooks].side_effect = true is not available yet: [hooks] is " + "experimental and cannot decide whether a build succeeded. A " + "failing hook is reported as a warning and the build keeps its " + "own result. Remove the key (the default is false) — it is " + "reserved so that manifests do not have to change when the " + "feature is promoted.")); + + static constexpr std::string_view kKnownHookKeys[] = { + "build_start", "build_failed", "build_finished", "during_build", + "timeout_seconds", "enabled", "side_effect", + }; + for (auto& [k, _] : *ht) { + bool known = false; + for (auto kk : kKnownHookKeys) if (k == kk) { known = true; break; } + if (!known) + m.schemaWarnings.push_back(std::format( + "[hooks] has unsupported key '{}' (ignored). Keys: " + "build_start, build_failed, build_finished, during_build, " + "timeout_seconds, enabled, side_effect.", k)); + } + } + // [lib] — library root convention (cargo-style). if (auto v = doc->get_string("lib.path")) { m.lib.path = *v; diff --git a/modules/manifest/src/types.cppm b/modules/manifest/src/types.cppm index 9d3ec6a0..675957a4 100644 --- a/modules/manifest/src/types.cppm +++ b/modules/manifest/src/types.cppm @@ -919,6 +919,88 @@ struct WorkspaceConfig { bool present = false; }; +// `[hooks]` — project build lifecycle commands (#496). +// +// The commands are host-shell strings written by the project author, run by +// `mcpp build` around the build it performs. See docs/05-mcpp-toml.md §2.16. +// +// ⚠️ ONLY THE ROOT PROJECT'S HOOKS ARE EVER RUN. Every manifest mcpp parses +// carries this field, including a DEPENDENCY's — and `mcpp build` reaches the +// invoker (mcpp.hooks) with the root project's manifest alone. A dependency +// that declares hooks is inert by construction, which is the only reason +// `mcpp add` of a third-party package does not become "run their shell +// command on my next build". Anything that adds a second call site inherits +// that responsibility. +// One event's command. Spelled either as a bare string or as a table — the +// same string-or-table shape `[dependencies]` and `[resources].version-info` +// already use, so it adds no parsing semantics. +struct HookCommand { + std::string cmd; + // Per-event override of the table's `timeout_seconds`. 0 = inherit. Only + // meaningful for a self-closing interval; see below. + int timeoutSeconds = 0; + // Restart the command if it exits before its interval closes. Only + // `during_build` has an interval that can outlast a run, so this is + // REJECTED on the other events rather than accepted and ignored. + bool loop = false; + + bool empty() const { return cmd.empty(); } +}; + +// A hook is a command mcpp OWNS FOR AN INTERVAL; the event names the interval. +// +// build_start / build_finished / build_failed opens at the event, +// closes when the command exits +// during_build opens before the build, +// closes after it +// +// The first three are SELF-CLOSING, and "synchronous" is not a separate mode — +// it is what an interval closed by the command itself looks like. Everything +// that reads as a special case for `during_build` falls out of that one +// difference instead of being declared: `timeout_seconds` bounds one run and +// so does not apply where the build already bounds it; `loop` restarts a +// command that ended before its interval did, which a self-closing interval +// makes impossible. +// +// See .agents/docs/2026-08-30-project-build-hooks-owned-intervals.md. +struct Hooks { + HookCommand buildStart; + HookCommand buildFailed; + HookCommand buildFinished; + HookCommand duringBuild; + + int timeoutSeconds = 10; // default for one run of a hook command + bool enabled = true; // whole table + + // ⚠️ EXPERIMENTAL: FALSE, AND CURRENTLY THE ONLY VALUE. + // + // The key means "a hook failure fails the build". While `[hooks]` is + // experimental it does not get to decide that: a hook that fails is + // reported as a warning and the build keeps whatever result it earned on + // its own. `side_effect = true` is REJECTED by the parser rather than + // accepted and ignored — a project that believes its build is gated on a + // notifier, and is not, has been told something false. + // + // The mechanism below it is intact and is what the key will switch on when + // the feature graduates; the parser check is the whole of the gate, so + // removing it is the whole of the change. + bool sideEffect = false; + + // "This project has work for `mcpp build` to do." Distinct from `enabled`: + // a table that only sets policy keys declares no command, and must leave + // the build path it would otherwise divert (the fast path) untouched. + bool active() const { + return enabled && !(buildStart.empty() && buildFailed.empty() + && buildFinished.empty() && duringBuild.empty()); + } + + // The bound on one run of `c`. The per-event value wins; 0 means it was + // not given, which is what makes "inherit" expressible at all. + int timeout_for(const HookCommand& c) const { + return c.timeoutSeconds > 0 ? c.timeoutSeconds : timeoutSeconds; + } +}; + // [profile.] — bundled build settings (opt level, debug, lto, strip). struct Profile { std::string optLevel = "2"; @@ -985,6 +1067,7 @@ struct Manifest { Resources resources; // [resources] (mcpp#365) RuntimeConfig runtimeConfig; XlingsConfig xlings; // [xlings] build environment (L-1) + Hooks hooks; // [hooks] lifecycle commands (#496) std::vector conditionalConfigs; // [target.'cfg(...)'.build], deferred std::map profiles; // [profile.] // [features] — feature name → implied features ("default" = default set). diff --git a/modules/platform/src/process.cppm b/modules/platform/src/process.cppm index 3b13092f..657e49fa 100644 --- a/modules/platform/src/process.cppm +++ b/modules/platform/src/process.cppm @@ -112,6 +112,66 @@ int run_exec_deadline(const std::vector& argv, std::chrono::milliseconds deadline, bool* timed_out); +// Run one host-shell command with inherited stdio, a working directory and a +// real deadline. POSIX uses /bin/sh; Windows uses cmd.exe. This is for +// user-authored command strings such as project hooks — programmatic launches +// keep using the argv-based run_exec_deadline API above. +// +// `cwd` is where the command runs; empty means "inherit ours". It is a +// PARAMETER rather than something the caller arranges with a chdir: the +// process-wide working directory is shared state, and the launchers underneath +// already carry a per-child cwd (posix_spawn_file_actions_addchdir_np / +// CreateProcess's lpCurrentDirectory). +// +// Returns 127 when the shell itself could not be started — the same code a +// shell uses for a command it cannot find, and never confusable with a +// command that ran. +int run_shell_deadline(std::string_view command, + std::string_view cwd, + std::chrono::milliseconds deadline, + bool* timed_out); + +// ─── A shell command mcpp owns for longer than one call (#496) ─────────── +// +// `run_shell_deadline` owns its child for the length of the call. A project +// `[hooks] during_build` command is owned for the length of the BUILD, so it +// is started here, polled while the build runs, and stopped afterwards. +// +// The handle is opaque and carries whichever platform's identity is real: a +// process GROUP on POSIX, a job object on Windows. Neither is a pid, and that +// is deliberate — the command is user-authored shell, so the thing that must +// die is a tree, not a process. +struct BackgroundCommand { + bool ok = false; + long long group = 0; // POSIX: process-group id + unsigned long long job = 0; // Windows: job object + unsigned long long process = 0; // Windows: the child +}; + +// `inheritStdio == false` discards the child's output. That is the right +// default for anything running alongside the build: its writes would otherwise +// interleave into the middle of a compiler diagnostic. +BackgroundCommand start_shell_background(std::string_view command, + std::string_view cwd, + bool inheritStdio); + +// True while it is still up. When it has exited and `exitCode` is given, the +// code is written there — "the player finished the track" and "the command +// does not exist" are the same event to a poller that only answers yes/no, and +// the `loop` supervisor has to tell them apart. +bool background_running(const BackgroundCommand& child, int* exitCode = nullptr); + +// Asks, waits `grace`, then takes the tree. +void stop_background(const BackgroundCommand& child, + std::chrono::milliseconds grace); + +// Ctrl-C. Registering the child means an interrupted build does not leave it +// running — which, for the case this exists for, is a background player the +// user can no longer name. Only one command is guarded at a time; a build owns +// at most one. +void guard_background_on_signal(const BackgroundCommand& child); +void clear_background_guard(); + RunResult capture_exec_deadline( const std::vector& argv, const std::vector>& extraEnv, @@ -160,6 +220,23 @@ int extract_exit_code(int raw_status); std::string windows_command_from_argv(const std::vector& argv); std::string windows_wrap_for_cmd_c(std::string_view cmd); +// The command line that runs a USER-AUTHORED shell command through cmd.exe. +// +// ⚠️ NOT `windows_command_from_argv({"cmd.exe", "/d", "/s", "/c", command})`. +// That shape is for a program plus its argv, where CreateProcess's parsing is +// what has to be satisfied. cmd.exe is not parsed that way: its switches must +// arrive BARE (quoted, they are no longer switches), and the command tail is +// governed by the /C quote rule above rather than by argv quoting — so an +// argv-quoted command arrives carrying a pair cmd does not consume. That is +// #425 one layer up, and it is why this is its own shape: +// +// cmd.exe /d /s /c "" +// +// /s makes the rule unconditional (strip exactly the outer pair), so the +// command reaches cmd verbatim no matter how many quotes it contains; /d skips +// AutoRun so a user's registry-installed shell hook cannot alter it. +std::string windows_shell_command_line(std::string_view command); + } // namespace mcpp::platform::process // ─── Implementation ────────────────────────────────────────────────────── @@ -181,6 +258,12 @@ std::string windows_wrap_for_cmd_c(std::string_view cmd) { return "\"" + std::string(cmd) + "\""; } +std::string windows_shell_command_line(std::string_view command) { + // One derivation for the outer pair: the same wrap the /c rule above is + // written against. + return "cmd.exe /d /s /c " + windows_wrap_for_cmd_c(command); +} + namespace { // Append a non-interactive stdin redirect to prevent child processes from @@ -585,12 +668,17 @@ struct BoundedOutcome { // `capture == false` runs the child on the caller's stdio: live output, and a // real terminal for anything that checks. `run_exec_deadline` needs that; the // capturing variants need the pipe. +// `windowsCommandLine` overrides what the Windows branch launches. Empty (the +// normal case) means "derive it from argv". A shell command is the one caller +// that must NOT be derived that way — see windows_shell_command_line — and the +// POSIX branch is unaffected either way, because it never flattens argv. BoundedOutcome dispatch_bounded( const std::vector& argv, const std::vector>& extraEnv, std::string_view cwd, std::chrono::milliseconds deadline, - bool capture) + bool capture, + std::string_view windowsCommandLine = {}) { BoundedOutcome outcome; @@ -617,7 +705,9 @@ BoundedOutcome dispatch_bounded( : nullptr; if constexpr (mcpp::platform::is_windows) { - const auto cmd = windows_command_from_argv(argv); + const auto cmd = windowsCommandLine.empty() + ? windows_command_from_argv(argv) + : std::string(windowsCommandLine); auto r = mcpp::platform::winproc::capture_with_deadline( cmd.c_str(), envArg, envCount, cwdArg, ms, sink, &outcome.output); outcome.supported = r.supported; @@ -658,6 +748,95 @@ int run_exec_deadline(const std::vector& argv, return r.exit_code; } +int run_shell_deadline(std::string_view command, + std::string_view cwd, + std::chrono::milliseconds deadline, + bool* timed_out) +{ + if (timed_out) *timed_out = false; + if (command.empty() || deadline.count() <= 0) return 127; + + // argv is what the POSIX branch launches; the Windows branch takes the + // shaped command line instead. Both are built here so neither platform's + // spelling can drift into a launcher that does not use it. + const std::vector argv{"/bin/sh", "-c", std::string(command)}; + auto r = dispatch_bounded(argv, {}, cwd, deadline, /*capture=*/false, + windows_shell_command_line(command)); + // No fallback to the unbounded launcher here, unlike run_exec_deadline: a + // hook's deadline and its working directory are both part of what the + // caller asked for, and the unbounded path can honour neither. + if (!r.supported) return 127; + if (timed_out) *timed_out = r.timed_out; + return r.exit_code; +} + +// The same split as dispatch_bounded, for the same reason: POSIX names a +// program with an argv array, Windows with a single command line. Both +// spellings are built here so neither can drift into a launcher that does not +// use it. +BackgroundCommand start_shell_background(std::string_view command, + std::string_view cwd, + bool inheritStdio) +{ + BackgroundCommand out; + if (command.empty()) return out; + const std::string cwdStore(cwd); + const char* cwdArg = cwdStore.empty() ? nullptr : cwdStore.c_str(); + + if constexpr (mcpp::platform::is_windows) { + const auto line = windows_shell_command_line(command); + auto r = mcpp::platform::winproc::spawn_background( + line.c_str(), cwdArg, inheritStdio ? 1 : 0); + out.ok = r.ok; + out.job = r.job; + out.process = r.process; + } else { + const std::string cmdStore(command); + const char* argv[] = {"/bin/sh", "-c", cmdStore.c_str()}; + auto r = mcpp::platform::unixproc::spawn_background( + argv, 3, cwdArg, inheritStdio ? 1 : 0); + out.ok = r.ok; + out.group = r.group; + } + return out; +} + +bool background_running(const BackgroundCommand& child, int* exitCode) { + if (!child.ok) return false; + if constexpr (mcpp::platform::is_windows) + return mcpp::platform::winproc::background_running(child.process, + exitCode) == 1; + else + return mcpp::platform::unixproc::background_running(child.group, + exitCode) == 1; +} + +void stop_background(const BackgroundCommand& child, + std::chrono::milliseconds grace) +{ + if (!child.ok) return; + if constexpr (mcpp::platform::is_windows) + mcpp::platform::winproc::background_stop(child.job, child.process, + grace.count()); + else + mcpp::platform::unixproc::background_stop(child.group, grace.count()); +} + +void guard_background_on_signal(const BackgroundCommand& child) { + if (!child.ok) return; + if constexpr (mcpp::platform::is_windows) + mcpp::platform::winproc::guard_job_on_signal(child.job); + else + mcpp::platform::unixproc::guard_group_on_signal(child.group); +} + +void clear_background_guard() { + if constexpr (mcpp::platform::is_windows) + mcpp::platform::winproc::clear_job_guard(); + else + mcpp::platform::unixproc::clear_group_guard(); +} + RunResult capture_exec_deadline( const std::vector& argv, const std::vector>& extraEnv, diff --git a/modules/platform/src/unix/bounded_process.cppm b/modules/platform/src/unix/bounded_process.cppm index 75b49091..5aa0e50c 100644 --- a/modules/platform/src/unix/bounded_process.cppm +++ b/modules/platform/src/unix/bounded_process.cppm @@ -87,6 +87,56 @@ DeadlineRun capture_with_deadline(const char* const* argvEntries, OutputSink sink, void* ctx); +// ─── A child that outlives the call that started it (#496) ─────────────── +// +// `capture_with_deadline` owns its child for the length of one call. A project +// `[hooks] during_build` command is owned for the length of the BUILD, so the +// caller needs a handle it can poll and stop later. Every member is a builtin, +// same constraint as DeadlineRun. +// +// ⚠️ `group`, NOT a pid. The child is placed in a process group of its own +// (posix_spawnattr_setpgroup) and stopped with killpg, because the thing being +// started is a user-authored SHELL command: `sh -c 'player & wait'` makes the +// writer a grandchild, and `kill(pid)` reaches only the shell. A background +// player that survives its build, from a process the user cannot name, is the +// worst outcome this API has — so the group is the unit throughout. +struct BackgroundChild { + bool ok = false; + long long group = 0; // the child's process-group id (== its pid) +}; + +// `inheritStdio == 0` sends the child's output to /dev/null. A spanning hook +// writes CONCURRENTLY with ninja and would otherwise interleave into the middle +// of a compiler diagnostic. +BackgroundChild spawn_background(const char* const* argvEntries, + unsigned long argvCount, + const char* cwd, + int inheritStdio); + +// 1 = still running, 0 = exited, -1 = unknown. When it returns 0, `exitCode` +// (if given) receives the shell convention: the status, or 128+signal. +// +// The code is part of the answer rather than a second call, because the only +// caller that needs it — the `loop` supervisor — has to distinguish "the +// player finished the track" from "the command does not exist". Restarting the +// first forever is the feature; restarting the second forever is a spin. +int background_running(long long group, int* exitCode); + +// SIGTERM, `graceMs`, then SIGKILL — to the GROUP. Reaps the direct child. +void background_stop(long long group, long long graceMs); + +// ─── Ctrl-C ────────────────────────────────────────────────────────────── +// +// Its own process group is what makes killpg possible AND what stops the +// terminal's SIGINT from reaching the child: Ctrl-C would kill mcpp and leave +// the player running. Both halves are required, so the group that must not +// outlive us is registered here for the duration. +// +// The handler does the minimum that is async-signal-safe: killpg (which is), +// then the default action. +void guard_group_on_signal(long long group); +void clear_group_guard(); + } // namespace mcpp::platform::unixproc namespace mcpp::platform::unixproc { @@ -228,6 +278,135 @@ DeadlineRun capture_with_deadline(const char* const* argvEntries, return out; } +// ─── Background children ───────────────────────────────────────────────── + +namespace { + +// Read by a signal handler, so `volatile sig_atomic_t` and nothing else: the +// handler may run between any two instructions and may not lock, allocate, or +// call anything that is not async-signal-safe. 0 means "nothing to clean up". +volatile sig_atomic_t g_guardedGroup = 0; + +extern "C" void background_signal_handler(int sig) { + const auto group = g_guardedGroup; + // killpg is async-signal-safe. SIGKILL rather than SIGTERM: this is the + // path where mcpp is about to stop existing, and there is nobody left to + // escalate if the group ignores the polite request. + if (group > 0) ::killpg(static_cast(group), SIGKILL); + // Die of the signal we were sent, so the exit status is the one the shell + // and any outer script expect from a Ctrl-C. + ::signal(sig, SIG_DFL); + ::raise(sig); +} + +} // namespace + +BackgroundChild spawn_background(const char* const* argvEntries, + unsigned long argvCount, + const char* cwd, + int inheritStdio) +{ + BackgroundChild out; + if (argvCount == 0 || !argvEntries) return out; + + std::vector cargv; + cargv.reserve(argvCount + 1); + for (unsigned long i = 0; i < argvCount; ++i) + cargv.push_back(const_cast(argvEntries[i])); + cargv.push_back(nullptr); + + posix_spawn_file_actions_t fa; + ::posix_spawn_file_actions_init(&fa); + if (cwd && *cwd) + ::posix_spawn_file_actions_addchdir_np(&fa, cwd); + if (!inheritStdio) { + // /dev/null on all three: a spanning hook must not write into ninja's + // output, and must not be able to block on a terminal read either. + ::posix_spawn_file_actions_addopen(&fa, 0, "/dev/null", O_RDONLY, 0); + ::posix_spawn_file_actions_addopen(&fa, 1, "/dev/null", O_WRONLY, 0); + ::posix_spawn_file_actions_adddup2(&fa, 1, 2); + } + + // POSIX_SPAWN_SETPGROUP with pgroup 0: the child becomes the leader of a + // new group whose id is its pid. Standard POSIX, unlike SETSID. + posix_spawnattr_t attr; + ::posix_spawnattr_init(&attr); + ::posix_spawnattr_setpgroup(&attr, 0); + ::posix_spawnattr_setflags(&attr, POSIX_SPAWN_SETPGROUP); + + pid_t pid = 0; + const int sp = ::posix_spawnp(&pid, cargv[0], &fa, &attr, + cargv.data(), current_environ()); + ::posix_spawn_file_actions_destroy(&fa); + ::posix_spawnattr_destroy(&attr); + if (sp != 0) return out; + + out.ok = true; + out.group = pid; + return out; +} + +// ⚠️ DOES NOT REAP, and that is the whole point. +// +// A zombie still holds its pid, so an unreaped leader is what keeps the GROUP +// id from being recycled — and `background_stop` signals that group. Reaping +// here would hand the id back to the kernel between the poll and the kill, +// which on a busy machine is how a stop lands on somebody else's process. +// waitid(WNOWAIT) answers "has it exited?" without giving the id back. +int background_running(long long group, int* exitCode) { + if (group <= 0) return -1; + siginfo_t info{}; + info.si_pid = 0; + if (::waitid(P_PID, static_cast(group), &info, + WEXITED | WNOHANG | WNOWAIT) != 0) + return -1; + if (info.si_pid == 0) return 1; + if (exitCode) + *exitCode = (info.si_code == CLD_EXITED) + ? info.si_status + : 128 + info.si_status; // shell convention for a signal + return 0; +} + +void background_stop(long long group, long long graceMs) { + if (group <= 0) return; + const pid_t pgid = static_cast(group); + + // The group, not the pid. This is the line the design is about: `kill(pid)` + // reaches only the shell mcpp started, and `sh -c 'player & wait'` makes + // the player a grandchild — still holding the audio device afterwards. + ::killpg(pgid, SIGTERM); + + const auto until = std::chrono::steady_clock::now() + + std::chrono::milliseconds(graceMs); + while (background_running(group, nullptr) == 1 + && std::chrono::steady_clock::now() < until) { + struct timespec ts{0, 10'000'000}; // 10ms + ::nanosleep(&ts, nullptr); + } + + // Unconditional, and BEFORE the reap: the leader may have gone politely + // while something it forked has not, and the group is still addressable + // only for as long as the unreaped leader holds the id. + ::killpg(pgid, SIGKILL); + int status = 0; + ::waitpid(pgid, &status, 0); +} + +void guard_group_on_signal(long long group) { + g_guardedGroup = static_cast(group); + ::signal(SIGINT, background_signal_handler); + ::signal(SIGTERM, background_signal_handler); + ::signal(SIGHUP, background_signal_handler); +} + +void clear_group_guard() { + g_guardedGroup = 0; + ::signal(SIGINT, SIG_DFL); + ::signal(SIGTERM, SIG_DFL); + ::signal(SIGHUP, SIG_DFL); +} + #else DeadlineRun capture_with_deadline(const char* const*, unsigned long, @@ -237,6 +416,15 @@ DeadlineRun capture_with_deadline(const char* const*, unsigned long, return {}; } +BackgroundChild spawn_background(const char* const*, unsigned long, + const char*, int) { + return {}; +} +int background_running(long long, int*) { return -1; } +void background_stop(long long, long long) {} +void guard_group_on_signal(long long) {} +void clear_group_guard() {} + #endif } // namespace mcpp::platform::unixproc diff --git a/modules/platform/src/windows/bounded_process.cppm b/modules/platform/src/windows/bounded_process.cppm index 0b30e914..ce930b1a 100644 --- a/modules/platform/src/windows/bounded_process.cppm +++ b/modules/platform/src/windows/bounded_process.cppm @@ -97,6 +97,56 @@ DeadlineRun capture_with_deadline(const char* commandLine, OutputSink sink, void* ctx); +// ─── A child that outlives the call that started it (#496) ─────────────── +// +// The peer of unixproc::spawn_background, and the same contract: a project +// `[hooks] during_build` command is owned for the length of the BUILD, so the +// caller gets a handle it can poll and stop later. Builtins only, like +// DeadlineRun — HANDLEs travel as integers rather than as a std type. +// +// The job object does here what a process group does on POSIX, and does it +// better in one respect: JOB_OBJECT_LIMIT_KILL_ON_JOB_CLOSE means the tree +// dies when the last handle to the job closes, which includes mcpp exiting for +// ANY reason — Ctrl-C, an unhandled exception, or being killed outright. POSIX +// needs an explicit signal handler for the same guarantee and still cannot +// cover `kill -9`. +struct BackgroundChild { + bool ok = false; + unsigned long long job = 0; // HANDLE to the job object + unsigned long long process = 0; // HANDLE to the child +}; + +// `inheritStdio == 0` sends the child's output to NUL. A spanning hook writes +// CONCURRENTLY with ninja and would otherwise interleave into the middle of a +// compiler diagnostic. +BackgroundChild spawn_background(const char* commandLine, + const char* cwd, + int inheritStdio); + +// 1 = still running, 0 = exited, -1 = unknown. When it returns 0, `exitCode` +// (if given) receives the process's exit code. Same contract as the POSIX +// peer, and for the same caller: the `loop` supervisor has to tell "the +// player finished the track" from "the command does not exist". +int background_running(unsigned long long process, int* exitCode); + +// Closes the job, which terminates the whole tree at once. +// +// `graceMs` is accepted for signature parity with the POSIX peer and is NOT +// spent: Windows has no portable graceful stop for a child with no console and +// no window of its own, and the call that looks like one +// (GenerateConsoleCtrlEvent) addresses a process group attached to THIS +// console — see the implementation for what that cost. The asymmetry with the +// POSIX side's SIGTERM-then-grace is real and is declared here rather than +// papered over. +void background_stop(unsigned long long job, unsigned long long process, + long long graceMs); + +// Ctrl-C. The job already covers process death, so this exists only so that a +// deliberate interrupt stops the tree BEFORE mcpp unwinds, rather than as a +// side effect of it exiting. +void guard_job_on_signal(unsigned long long job); +void clear_job_guard(); + } // namespace mcpp::platform::winproc namespace mcpp::platform::winproc { @@ -315,6 +365,158 @@ DeadlineRun capture_with_deadline(const char* commandLine, return out; } +// ─── Background children ───────────────────────────────────────────────── + +namespace { + +// Read by a console control handler, which runs on a thread of the OS's +// choosing. Only the handle is shared, and closing a job handle is atomic from +// the caller's point of view. +volatile unsigned long long g_guardedJob = 0; + +BOOL WINAPI background_console_handler(DWORD) { + const auto job = g_guardedJob; + // TerminateJobObject, not CloseHandle: this handler races `background_stop` + // on the normal path, and terminating is idempotent while closing the same + // handle twice is not. The handle stays valid for whoever closes it. + if (job) + ::TerminateJobObject( + reinterpret_cast(static_cast(job)), 1); + return FALSE; // FALSE = also run the default handler, i.e. still exit +} + +} // namespace + +BackgroundChild spawn_background(const char* commandLine, + const char* cwd, + int inheritStdio) +{ + BackgroundChild out; + if (!commandLine || !*commandLine) return out; + + HANDLE job = ::CreateJobObjectA(nullptr, nullptr); + if (job) { + JOBOBJECT_EXTENDED_LIMIT_INFORMATION jeli{}; + jeli.BasicLimitInformation.LimitFlags = + JOB_OBJECT_LIMIT_KILL_ON_JOB_CLOSE; + ::SetInformationJobObject(job, JobObjectExtendedLimitInformation, + &jeli, sizeof(jeli)); + } + + SECURITY_ATTRIBUTES sa{}; + sa.nLength = sizeof(sa); + sa.bInheritHandle = TRUE; + + Handle nulIo; + STARTUPINFOA si{}; + si.cb = sizeof(si); + if (!inheritStdio) { + nulIo.h = ::CreateFileA("NUL", GENERIC_READ | GENERIC_WRITE, + FILE_SHARE_READ | FILE_SHARE_WRITE, &sa, + OPEN_EXISTING, 0, nullptr); + if (nulIo.h && nulIo.h != INVALID_HANDLE_VALUE) { + si.dwFlags = STARTF_USESTDHANDLES; + si.hStdInput = nulIo.h; + si.hStdOutput = nulIo.h; + si.hStdError = nulIo.h; + } + } + + PROCESS_INFORMATION pi{}; + std::string cmdBuf(commandLine); // CreateProcessA may modify it + + // CREATE_SUSPENDED so the child joins the job BEFORE it can spawn anything + // — a grandchild created in that gap would escape the kill, which for a + // background player is the difference between "stops" and "plays forever". + // + // CREATE_NEW_PROCESS_GROUP is the peer of POSIX_SPAWN_SETPGROUP: the child + // stops receiving the console's Ctrl-C, which is what makes the guard + // below necessary and what stops a stray Ctrl-C from half-killing the tree. + const BOOL ok = ::CreateProcessA( + nullptr, cmdBuf.data(), nullptr, nullptr, + /*bInheritHandles=*/TRUE, + CREATE_SUSPENDED | CREATE_NEW_PROCESS_GROUP + | (inheritStdio ? 0u : CREATE_NO_WINDOW), + nullptr, (cwd && *cwd) ? cwd : nullptr, &si, &pi); + if (!ok) { + if (job) ::CloseHandle(job); + return out; + } + if (job) ::AssignProcessToJobObject(job, pi.hProcess); + ::ResumeThread(pi.hThread); + ::CloseHandle(pi.hThread); + + out.ok = true; + out.job = static_cast( + reinterpret_cast(job)); + out.process = static_cast( + reinterpret_cast(pi.hProcess)); + return out; +} + +int background_running(unsigned long long process, int* exitCode) { + if (!process) return -1; + auto h = reinterpret_cast(static_cast(process)); + const DWORD r = ::WaitForSingleObject(h, 0); + if (r == WAIT_TIMEOUT) return 1; + if (r != WAIT_OBJECT_0) return -1; + if (exitCode) { + DWORD code = 0; + ::GetExitCodeProcess(h, &code); + *exitCode = static_cast(code); + } + return 0; +} + +void background_stop(unsigned long long job, unsigned long long process, + long long graceMs) +{ + auto procH = process ? reinterpret_cast( + static_cast(process)) + : nullptr; + auto jobH = job ? reinterpret_cast( + static_cast(job)) + : nullptr; + + // ⚠️ NO POLITE ASK HERE, AND `graceMs` IS DELIBERATELY UNSPENT. + // + // The obvious "ask first" is + // + // ::GenerateConsoleCtrlEvent(CTRL_BREAK_EVENT, ::GetProcessId(procH)); + // + // and it is wrong in a way that does not show up locally. That call + // addresses a process GROUP attached to this console, not a process; when + // the id does not name a live group of ours — which it does not, once the + // child has already exited, and `start /b`-style commands exit at once — + // the event reaches everything sharing the console instead. Measured on + // the Windows e2e runner: the whole suite died eleven seconds into the + // hooks test with exit code -1073741510 (0xC000013A, + // STATUS_CONTROL_C_EXIT) and printed no summary at all, because mcpp had + // Ctrl-Break'd its own console. + // + // Windows has no portable graceful stop for a child with no console and no + // window of its own. The job IS the mechanism; the POSIX peer's + // SIGTERM-then-grace has no equivalent here, and the asymmetry is stated + // in the declaration rather than faked with a call that reaches too far. + (void)graceMs; + + // Closing a KILL_ON_JOB_CLOSE job takes the whole tree. TerminateProcess + // on the child alone would leave whatever it started behind — which for + // `start /b cmd /c player` is the player. + if (jobH) ::CloseHandle(jobH); + if (procH) ::CloseHandle(procH); +} + +void guard_job_on_signal(unsigned long long job) { + g_guardedJob = job; + ::SetConsoleCtrlHandler(background_console_handler, TRUE); +} + +void clear_job_guard() { + g_guardedJob = 0; + ::SetConsoleCtrlHandler(background_console_handler, FALSE); +} + #else DeadlineRun capture_with_deadline(const char*, const char* const*, unsigned long, @@ -323,6 +525,12 @@ DeadlineRun capture_with_deadline(const char*, const char* const*, unsigned long return {}; } +BackgroundChild spawn_background(const char*, const char*, int) { return {}; } +int background_running(unsigned long long, int*) { return -1; } +void background_stop(unsigned long long, unsigned long long, long long) {} +void guard_job_on_signal(unsigned long long) {} +void clear_job_guard() {} + #endif } // namespace mcpp::platform::winproc diff --git a/src/build/execute.cppm b/src/build/execute.cppm index 8c5d25bd..1799ac68 100644 --- a/src/build/execute.cppm +++ b/src/build/execute.cppm @@ -808,6 +808,10 @@ struct FastPathIdentity { // `[build] target` — the project's DEFAULT cross target, and the reason // try_fast_run cannot assume the artifact runs here. See its use. std::string defaultTarget; + // `[hooks]` with at least one command (#496). Third field down riding on + // the same single manifest read, and the only one that can VETO the fast + // path rather than describe it — see try_fast_build. + bool hooksActive = false; }; std::optional @@ -822,6 +826,7 @@ fast_path_identity(const std::filesystem::path& projectRoot, m->resources.files, mcpp::extension_table_for(m->buildConfig.moduleExtensions), m->buildConfig.target, + m->hooks.active(), }; } @@ -835,6 +840,14 @@ export std::optional try_fast_build(const std::filesystem::path& projectRoo auto want = fast_path_identity(projectRoot); if (!want) return std::nullopt; + // #496. A project with build hooks always takes the full path. The fast + // path is defined as "skip preparation", and `build_start` is specified to + // run AFTER it — a hook program installed as an `[xlings] deps` entry does + // not exist until preparation has run. Declining here rather than in + // cmd_build keeps the decision next to the manifest that answers it; the + // full path then runs the hooks around run_build_plan. + if (want->hooksActive) return std::nullopt; + // P3: read multi-entry cache and find the entry matching this // (target, profile, cache mode) triple. Matching on the target alone served // the wrong profile's artifacts, and ignoring the cache mode replayed a diff --git a/src/cli/cmd_build.cppm b/src/cli/cmd_build.cppm index 13b53177..650f9df8 100644 --- a/src/cli/cmd_build.cppm +++ b/src/cli/cmd_build.cppm @@ -18,6 +18,7 @@ import mcpp.build.stage; import mcpp.build.schedule.detach_codegen; import mcpp.build.test_targets; import mcpp.dyndep; +import mcpp.hooks; import mcpp.log; import mcpp.project; import mcpp.manifest; @@ -42,6 +43,44 @@ workspace_fanout_members(bool wantAll, const std::string& package_filter) { return std::nullopt; } +// run_build_plan, wrapped in the project's `[hooks]` lifecycle (#496). +// +// The hooks come off the context's own manifest, so they are the ones belonging +// to the package being built — which in a workspace fan-out is the MEMBER, once +// per member. The lifecycle is deliberately paired: build_finished/build_failed +// are only ever reached after build_start has run, so a project that could not +// be prepared at all (bad manifest, unresolvable dependency, no toolchain) +// fires nothing — its hook programs may be exactly what preparation failed to +// install. +int run_build_with_hooks(mcpp::build::BuildContext& ctx, bool verbose, + bool no_cache, std::string_view targetOverride) { + auto const& hooks = ctx.manifest.hooks; + + // `during_build` opens first and closes last: its interval is the one that + // spans everything below. Its output is discarded unless --verbose, which + // is the only way it could interleave into a compiler diagnostic. + mcpp::hooks::Span span(hooks, ctx.projectRoot, /*inheritOutput=*/verbose); + if (!span.ok()) return 1; + + if (!mcpp::hooks::invoke(hooks, mcpp::hooks::Event::BuildStart, + ctx.projectRoot)) + return 1; + + int rc = mcpp::build::run_build_plan(ctx, verbose, no_cache, targetOverride); + + // Closed BEFORE the terminal hook. A "build finished" sound competing with + // the background music it replaces is the ordering this line settles. + bool spanOk = span.finish(); + + auto terminalEvent = rc == 0 ? mcpp::hooks::Event::BuildFinished + : mcpp::hooks::Event::BuildFailed; + // The build's own exit code outranks the hook's: `mcpp build` returning + // "the notifier failed" for a compile error would answer a question nobody + // asked. A hook failure only decides the exit code of a build that worked. + bool hookOk = mcpp::hooks::invoke(hooks, terminalEvent, ctx.projectRoot); + return rc != 0 ? rc : ((spanOk && hookOk) ? 0 : 1); +} + export int cmd_build(const mcpplibs::cmdline::ParsedArgs& parsed) { bool verbose = parsed.is_flag_set("verbose") || mcpp::log::is_verbose(); bool print_fp = parsed.is_flag_set("print-fingerprint"); @@ -118,7 +157,7 @@ export int cmd_build(const mcpplibs::cmdline::ParsedArgs& parsed) { auto ctx = mcpp::build::prepare_build(print_fp, /*includeDevDeps=*/false, /*extraTargets=*/{}, mo); if (!ctx) { std::println(stderr, "error: {}: {}", mp, ctx.error()); rc = 2; continue; } - int r = mcpp::build::run_build_plan(*ctx, verbose, no_cache, mo.target_triple); + int r = run_build_with_hooks(*ctx, verbose, no_cache, mo.target_triple); if (r != 0) rc = r; } return rc; @@ -137,6 +176,9 @@ export int cmd_build(const mcpplibs::cmdline::ParsedArgs& parsed) { && ov.cache_mode.empty()) { auto root = mcpp::project::find_manifest_root(std::filesystem::current_path()); if (root) { + // A project with active `[hooks]` declines the fast path from + // inside try_fast_build, where the manifest that says so is + // already loaded. if (auto rc = mcpp::build::try_fast_build(*root, verbose, no_cache)) { return *rc; } @@ -147,7 +189,7 @@ export int cmd_build(const mcpplibs::cmdline::ParsedArgs& parsed) { /*extraTargets=*/{}, ov); if (!ctx) { std::println(stderr, "error: {}", ctx.error()); return 2; } - return mcpp::build::run_build_plan(*ctx, verbose, no_cache, ov.target_triple); + return run_build_with_hooks(*ctx, verbose, no_cache, ov.target_triple); } export int cmd_run(const mcpplibs::cmdline::ParsedArgs& parsed, diff --git a/src/hooks.cppm b/src/hooks.cppm new file mode 100644 index 00000000..bd84c393 --- /dev/null +++ b/src/hooks.cppm @@ -0,0 +1,284 @@ +// mcpp.hooks — running the project's `[hooks]` lifecycle commands. +// +// ⚠️ EXPERIMENTAL. A hook cannot currently change whether a build succeeded: +// every failure is a warning, and `side_effect = true` is refused by the +// manifest parser. Only the ROOT project's hooks are ever run — a dependency's +// `[hooks]` is inert. See docs/05-mcpp-toml.md §2.16. +// +// The CONFIGURATION is not parsed here: `[hooks]` is a section of mcpp.toml +// and mcpp.toml has one parser (mcpp.manifest). What lives here is the part +// that is policy rather than grammar — when a command runs, what a failure +// means, and what the user is told. +// +// The model is one idea: a hook is a command mcpp OWNS FOR AN INTERVAL, and +// the event names the interval. `invoke` runs a SELF-CLOSING one (the interval +// ends when the command exits). `Span` owns a SPANNING one (`during_build`), +// whose interval is closed by the build. See +// .agents/docs/2026-08-30-project-build-hooks-owned-intervals.md. + +export module mcpp.hooks; + +import std; +import mcpp.manifest; +import mcpp.platform.process; +import mcpp.ui; + +export namespace mcpp::hooks { + +enum class Event { BuildStart, BuildFailed, BuildFinished }; + +// Run the command this event names, if the project declared one, and wait for +// it. The interval is self-closing: it ends when the command exits. +// +// Returns whether the BUILD may keep its own result. False means a hook failed +// while `side_effect` was on, i.e. the failure is the build's now. A missing +// command, a disabled table, and a hook that failed under `side_effect = false` +// all return true — from the build's point of view nothing happened. +bool invoke(const mcpp::manifest::Hooks& hooks, Event event, + const std::filesystem::path& projectRoot); + +// `during_build`: a command started before the build and stopped after it. +// +// RAII because the interval must close on every path out of the build, +// including the ones nobody writes down — an early return, an exception, a +// diagnostic that gives up. A background player that outlives its build is the +// failure this whole shape exists to prevent, so the destructor is the only +// place the stop can be promised from. +class Span { +public: + Span() = default; + Span(const mcpp::manifest::Hooks& hooks, + const std::filesystem::path& projectRoot, + bool inheritOutput); + ~Span(); + Span(const Span&) = delete; + Span& operator=(const Span&) = delete; + + // Whether the BUILD may proceed. False means the command could not be + // started and `side_effect` was on — the same verdict, in the same words, + // that a failed `build_start` produces. Already reported. + bool ok() const { return ok_; } + + // Close the interval and report what happened while it was open. Returns + // whether the build may keep its result. + // + // Called explicitly BEFORE the terminal hook rather than left to the + // destructor: a "build finished" sound playing over the background music + // it was supposed to replace is the ordering this exists for. + bool finish(); + +private: + void close(); + + mcpp::platform::process::BackgroundCommand child_; + std::string command_; + std::filesystem::path root_; + bool inheritOutput_ = false; + bool declared_ = false; + bool started_ = false; + bool closed_ = false; + bool sideEffect_ = false; + bool ok_ = true; + std::atomic stop_{false}; + std::atomic gaveUp_{false}; + std::thread supervisor_; +}; + +} // namespace mcpp::hooks + +namespace mcpp::hooks { + +namespace { + +namespace proc = mcpp::platform::process; + +// `loop` restarts a command that ended before its interval did. Two bounds, +// fixed rather than configurable, because a knob whose wrong value is a spin +// is not a knob: a typo in the command (`play /nonexistant`) would otherwise +// restart thousands of times per second for the length of the build. +constexpr auto kRestartDelay = std::chrono::milliseconds(250); +constexpr auto kTooShort = std::chrono::milliseconds(1000); +constexpr int kShortRunsBeforeGivingUp = 5; + +// How long a stopped command is given to leave on its own before it is taken. +// A player asked to stop should get to close its audio device. +constexpr auto kStopGrace = std::chrono::milliseconds(2000); + +std::string_view event_name(Event event) { + switch (event) { + case Event::BuildStart: return "build_start"; + case Event::BuildFailed: return "build_failed"; + case Event::BuildFinished: return "build_finished"; + } + return "unknown"; +} + +const mcpp::manifest::HookCommand& +event_command(const mcpp::manifest::Hooks& hooks, Event event) { + switch (event) { + case Event::BuildFailed: return hooks.buildFailed; + case Event::BuildFinished: return hooks.buildFinished; + case Event::BuildStart: break; + } + return hooks.buildStart; +} + +} // namespace + +// One place decides what a hook failure costs, so `side_effect` cannot be +// honoured on one failure mode and forgotten on another — and there are now +// four of them (cannot start, non-zero, timed out, failed to stay up). The +// message is identical either way; only its severity and the build's fate +// differ. +// +// ⚠️ WHILE `[hooks]` IS EXPERIMENTAL, `sideEffect` IS ALWAYS FALSE — the +// manifest parser refuses `side_effect = true` (see +// modules/manifest/src/toml.cppm). The `true` branch below is therefore +// unreachable today ON PURPOSE: it is the behaviour the key will select when +// the feature is promoted, and keeping it here means promotion is a deletion +// in the parser rather than a reconstruction here. +bool report_hook_failure_flag(bool sideEffect, const std::string& message) { + if (sideEffect) { + mcpp::ui::error(message); + return false; + } + mcpp::ui::warning(message); + return true; +} + +bool report_hook_failure(const mcpp::manifest::Hooks& hooks, + const std::string& message) { + return report_hook_failure_flag(hooks.sideEffect, message); +} + +bool invoke(const mcpp::manifest::Hooks& hooks, Event event, + const std::filesystem::path& projectRoot) { + if (!hooks.enabled) return true; + auto const& hook = event_command(hooks, event); + if (hook.empty()) return true; + + bool timedOut = false; + // The project root is passed to the launcher, not arranged with a chdir: + // mcpp's working directory is process-wide state, and a hook is not + // entitled to move it even briefly. + int rc = proc::run_shell_deadline( + hook.cmd, projectRoot.string(), + std::chrono::seconds(hooks.timeout_for(hook)), &timedOut); + + if (timedOut) + return report_hook_failure(hooks, std::format( + "hook '{}' timed out after {}s", event_name(event), + hooks.timeout_for(hook))); + if (rc != 0) + return report_hook_failure(hooks, std::format( + "hook '{}' exited with status {}", event_name(event), rc)); + return true; +} + +Span::Span(const mcpp::manifest::Hooks& hooks, + const std::filesystem::path& projectRoot, + bool inheritOutput) + : command_(hooks.duringBuild.cmd) + , root_(projectRoot) + , inheritOutput_(inheritOutput) + , declared_(hooks.enabled && !hooks.duringBuild.empty()) + , sideEffect_(hooks.sideEffect) +{ + if (!declared_) return; + + child_ = proc::start_shell_background(command_, root_.string(), + inheritOutput_); + if (!child_.ok) { + ok_ = report_hook_failure(hooks, + "hook 'during_build' could not be started"); + return; + } + started_ = true; + + // Its own process group is what makes a whole-tree stop possible and what + // stops the terminal's Ctrl-C from reaching it. Both halves are needed, so + // the group is registered for the duration. + proc::guard_background_on_signal(child_); + + if (!hooks.duringBuild.loop) return; + + // A supervisor exists ONLY for `loop`. Without it, a spanning hook is a + // spawn and a stop, and no thread is created. + supervisor_ = std::thread([this, runStarted = std::chrono::steady_clock::now()] + () mutable { + int shortFailures = 0; + while (!stop_.load()) { + int code = 0; + if (proc::background_running(child_, &code)) { + std::this_thread::sleep_for(std::chrono::milliseconds(50)); + continue; + } + // How long the run that JUST ENDED lasted. Measuring from the + // restart instead would compare a spawn against itself and read + // every run as instantaneous — which is a give-up on the fifth + // restart of a perfectly healthy command. + const auto ranFor = std::chrono::steady_clock::now() - runStarted; + + // Stop before restarting, not after: the run is over but whatever + // it forked may not be, and the group is addressable only while + // the unreaped leader still holds its id. + proc::stop_background(child_, std::chrono::milliseconds(0)); + proc::clear_background_guard(); + if (stop_.load()) return; + + // "Failed to stay up" is BOTH halves: short AND unsuccessful. A + // command that finishes quickly and cleanly — one `ding.mp3`, an + // `echo` — is doing exactly what `loop` was asked to repeat, and + // counting it here would turn the feature into its own kill switch. + if (code != 0 && ranFor < kTooShort) { + if (++shortFailures >= kShortRunsBeforeGivingUp) { + gaveUp_.store(true); + return; + } + } else { + shortFailures = 0; + } + + std::this_thread::sleep_for(kRestartDelay); + if (stop_.load()) return; + + runStarted = std::chrono::steady_clock::now(); + child_ = proc::start_shell_background(command_, root_.string(), + inheritOutput_); + if (!child_.ok) { + gaveUp_.store(true); + return; + } + proc::guard_background_on_signal(child_); + } + }); +} + +void Span::close() { + if (!declared_ || closed_) return; + closed_ = true; + stop_.store(true); + if (supervisor_.joinable()) supervisor_.join(); + if (started_) { + proc::clear_background_guard(); + proc::stop_background(child_, kStopGrace); + } +} + +bool Span::finish() { + const bool alreadyClosed = closed_; + close(); + if (alreadyClosed || !started_) return ok_; + if (!gaveUp_.load()) return ok_; + // Reported once, here, rather than from the supervisor thread: ui writes + // are not synchronised, and a thread printing into the middle of ninja's + // output is the interleaving `during_build` discards its child's stdout to + // avoid in the first place. + return report_hook_failure_flag(sideEffect_, std::format( + "hook 'during_build' failed to stay up: {} consecutive runs ended " + "within {}ms", kShortRunsBeforeGivingUp, kTooShort.count())); +} + +Span::~Span() { close(); } + +} // namespace mcpp::hooks diff --git a/tests/e2e/317_project_build_hooks.sh b/tests/e2e/317_project_build_hooks.sh new file mode 100644 index 00000000..0b4e6613 --- /dev/null +++ b/tests/e2e/317_project_build_hooks.sh @@ -0,0 +1,511 @@ +#!/usr/bin/env bash +# requires: +# Project-level build hooks (#496): lifecycle order, policy, timeout, the +# directory a hook runs in, and the two places nothing may fire. +set -e +source "$(dirname "$0")/_host_path.sh" + +TMP=$(mktemp -d) +trap 'rm -rf "$TMP"' EXIT +mkdir -p "$TMP/app/src" "$TMP/app/nested" + +cat > "$TMP/app/src/main.cpp" <<'EOF' +int main() { return 0; } +EOF + +write_manifest() { + cat > "$TMP/app/mcpp.toml" +} + +# cmd.exe writes CRLF and /bin/sh writes LF, so the line ENDINGS are the host's +# and the line CONTENT is the hook's. Compare the content. +# expect_log ... +expect_log() { + local file=$1; shift + local want got + want=$(printf '%s\n' "$@") + got=$(tr -d '\r' < "$file" 2>/dev/null || true) + if [[ "$got" != "$want" ]]; then + echo "FAIL: $file" + echo "--- expected ---"; printf '%s\n' "$want" + echo "--- actual ---"; printf '%s\n' "$got" + exit 1 + fi +} + +# A WHOLE line, matched as one: a substring would still pass if the message +# were reworded around it. No pipe into grep — a `grep -q` that exits on the +# first match SIGPIPEs its producer, which reads as failure the moment anyone +# adds `set -o pipefail` to this file. +expect_line() { # expect_line + local file=$1 line=$2 + local body + body=$(tr -d '\r' < "$file" 2>/dev/null || true) + if [[ $'\n'"$body"$'\n' != *$'\n'"$line"$'\n'* ]]; then + cat "$file" + echo "FAIL: '$line' is not a line of $file" + exit 1 + fi +} + +# ── The success lifecycle, twice ───────────────────────────────────────── +# +# Twice on purpose: the second `mcpp build` is the one that would otherwise +# take the no-op fast path, which skips preparation and would skip the hooks +# with it. A single run cannot tell the two paths apart. +write_manifest <<'EOF' +[package] +name = "hookprobe" +version = "0.1.0" + +[hooks] +build_start = "echo start>>hooks.log" +build_failed = "echo failed>>hooks.log" +build_finished = "echo finished>>hooks.log" +timeout_seconds = 10 +enabled = true +side_effect = false +EOF + +# Invoked BELOW the project root: a hook's relative paths belong to the root, +# not to wherever the user happened to type `mcpp build`. +cd "$TMP/app/nested" +"$MCPP" build > success-1.log 2>&1 || { + cat success-1.log; echo "FAIL: hooked build failed"; exit 1; } +"$MCPP" build > success-2.log 2>&1 || { + cat success-2.log; echo "FAIL: second hooked build failed"; exit 1; } +cd "$TMP/app" +[[ ! -e nested/hooks.log ]] || { + echo "FAIL: hook ran in the invocation directory, not the project root"; exit 1; } +expect_log hooks.log start finished start finished + +# ── A compiler failure chooses build_failed, never build_finished ──────── +cat > src/main.cpp <<'EOF' +#error "intentional hook failure path" +int main() { return 0; } +EOF +: > hooks.log +rc=0 +"$MCPP" build > build-failed.log 2>&1 || rc=$? +[[ $rc -ne 0 ]] || { cat build-failed.log; echo "FAIL: broken source built"; exit 1; } +expect_log hooks.log start failed + +cat > src/main.cpp <<'EOF' +int main() { return 0; } +EOF + +# ── A failing hook is REPORTED and the build keeps its own result ─────── +# +# While `[hooks]` is experimental it may not decide whether a build succeeded. +# The build below succeeds with a hook that cannot run at all, and the two +# assertions are both needed: the exit code says the hook had no vote, and the +# warning says it was not silently swallowed. +write_manifest <<'EOF' +[package] +name = "hookprobe" +version = "0.1.0" + +[hooks] +build_finished = "mcpp-hook-command-that-does-not-exist" +EOF +"$MCPP" build > ignored-hook-failure.log 2>&1 || { + cat ignored-hook-failure.log + echo "FAIL: a failing hook changed the build result"; exit 1; } +grep -q "warning: hook 'build_finished' exited with status" ignored-hook-failure.log || { + cat ignored-hook-failure.log; echo "FAIL: the hook failure was not reported"; exit 1; } +# `if grep` rather than `grep && { }`: a NEGATIVE assertion written with `&&` +# leaves the list's exit status as grep's, and reading that under `set -e` is +# an argument this file should not be having. +if grep -q "error: hook 'build_finished'" ignored-hook-failure.log; then + cat ignored-hook-failure.log + echo "FAIL: an experimental hook failure was reported as an error"; exit 1 +fi + +# The explicit spelling of the current behaviour keeps working, so a manifest +# does not have to change when the feature is promoted. +write_manifest <<'EOF' +[package] +name = "hookprobe" +version = "0.1.0" + +[hooks] +build_finished = "mcpp-hook-command-that-does-not-exist" +side_effect = false +EOF +"$MCPP" build > explicit-false.log 2>&1 || { + cat explicit-false.log; echo "FAIL: side_effect=false was not accepted"; exit 1; } + +# ── `side_effect = true` is REFUSED, not quietly downgraded ───────────── +# +# Honouring it would give an experimental feature a veto over every build; +# ignoring it would leave the project believing its build is gated on a +# notifier when nothing is. Both silent options are worse than an error. +write_manifest <<'EOF' +[package] +name = "hookprobe" +version = "0.1.0" + +[hooks] +build_finished = "echo done" +side_effect = true +EOF +rc=0 +"$MCPP" build > side-effect-true.log 2>&1 || rc=$? +[[ $rc -ne 0 ]] || { + cat side-effect-true.log; echo "FAIL: side_effect=true was accepted"; exit 1; } +grep -q "experimental" side-effect-true.log || { + cat side-effect-true.log + echo "FAIL: the refusal does not say why"; exit 1; } + +# ── enabled = false switches off a table that still names commands ────── +write_manifest <<'EOF' +[package] +name = "hookprobe" +version = "0.1.0" + +[hooks] +build_start = "echo disabled>>disabled.log" +build_finished = "echo disabled>>disabled.log" +enabled = false +EOF +rm -f disabled.log +"$MCPP" build > disabled.log.out 2>&1 || { + cat disabled.log.out; echo "FAIL: disabled hook build failed"; exit 1; } +[[ ! -e disabled.log ]] || { + cat disabled.log; echo "FAIL: enabled=false still ran hooks"; exit 1; } + +# ── The timeout is a real bound, on both host shells ──────────────────── +case "$(uname -s)" in + MINGW*|MSYS*|CYGWIN*) HOST_WINDOWS=1 ;; + *) HOST_WINDOWS=0 ;; +esac +if [[ $HOST_WINDOWS -eq 1 ]]; then + slow_command="ping -n 6 127.0.0.1 >NUL" + pause_2s="ping -n 3 127.0.0.1 >NUL" +else + slow_command="sleep 5" + pause_2s="sleep 2" +fi +write_manifest < timeout.log 2>&1 || { + cat timeout.log; echo "FAIL: a timed-out hook changed the build result"; exit 1; } +# The bound is real even though the build survives it: without the timeout the +# command would have run for five seconds, and the line below would be absent. +expect_line timeout.log "warning: hook 'build_finished' timed out after 1s" + +# ── An invalid value fails the manifest, before any command runs ──────── +# +# And it fails as a MANIFEST error naming the key — `[hooks]` is a section of +# mcpp.toml, so a bad value there is reported by the same parser and in the +# same words as a bad value anywhere else in the file. +write_manifest <<'EOF' +[package] +name = "hookprobe" +version = "0.1.0" + +[hooks] +build_start = "echo should-not-run>>invalid.log" +timeout_seconds = 0 +EOF +rm -f invalid.log +rc=0 +"$MCPP" build > invalid-config.log 2>&1 || rc=$? +[[ $rc -ne 0 ]] || { cat invalid-config.log; echo "FAIL: invalid hooks were accepted"; exit 1; } +[[ ! -e invalid.log ]] || { cat invalid.log; echo "FAIL: invalid hook ran"; exit 1; } +grep -q "\[hooks\].timeout_seconds must be a positive integer" invalid-config.log || { + cat invalid-config.log; echo "FAIL: invalid hook diagnostic is missing"; exit 1; } + +# ── An unknown key is a warning, and the known ones still run ─────────── +: > hooks.log +write_manifest <<'EOF' +[package] +name = "hookprobe" +version = "0.1.0" + +[hooks] +build_start = "echo start>>hooks.log" +build_finsihed = "echo typo>>hooks.log" +EOF +"$MCPP" build > unknown-key.log 2>&1 || { + cat unknown-key.log; echo "FAIL: unknown hook key was fatal"; exit 1; } +grep -q "\[hooks\] has unsupported key 'build_finsihed'" unknown-key.log || { + cat unknown-key.log; echo "FAIL: unknown hook key was not reported"; exit 1; } +expect_log hooks.log start + +# ── Preparation failing fires NOTHING ─────────────────────────────────── +# +# The lifecycle is paired: build_finished/build_failed are only reachable after +# build_start has run. A project that cannot be prepared has not started +# building — and its hook program may be exactly what preparation would have +# installed, so "report the failure" is not available here. +: > hooks.log +write_manifest <<'EOF' +[package] +name = "hookprobe" +version = "0.1.0" + +[hooks] +build_start = "echo start>>hooks.log" +build_failed = "echo failed>>hooks.log" + +[dependencies] +absent = { path = "no/such/dependency" } +EOF +rc=0 +"$MCPP" build > prepare-failed.log 2>&1 || rc=$? +[[ $rc -ne 0 ]] || { + cat prepare-failed.log; echo "FAIL: a missing path dependency built"; exit 1; } +expect_log hooks.log + +# ── `during_build`: an interval closed by the build, not by the command ── +# +# The assertions below are about STATE, never about a log line. "mcpp said it +# stopped the command" passes whether or not anything stopped; "the file it was +# appending to has not grown in a second" does not. +if [[ $HOST_WINDOWS -eq 1 ]]; then + cat > writer.cmd <<'EOF' +@echo off +:loop +echo tick>>beat.log +ping -n 2 127.0.0.1 >NUL +goto loop +EOF + # `start /b` returns immediately, so the command mcpp starts EXITS and the + # writer is left behind it — the job object is the only thing that can + # still reach it. + writer_with_grandchild="start /b cmd /c writer.cmd" +else + cat > writer.sh <<'EOF' +while :; do echo tick>>beat.log; sleep 0.2; done +EOF + # The shell mcpp starts forks the writer and waits, so the writer is a + # GRANDCHILD. This is the case `kill(pid)` misses and `killpg` catches, and + # it is the only assertion that tells the fix from the bug. + writer_with_grandchild="sh writer.sh & wait" +fi + +beat_is_frozen() { # the file grew, and then stopped growing + local before after + before=$(wc -c < beat.log 2>/dev/null || echo 0) + sleep 1 + after=$(wc -c < beat.log 2>/dev/null || echo 0) + [[ "$before" != "0" && "$before" == "$after" ]] +} + +: > hooks.log +rm -f beat.log +write_manifest < during-build.log 2>&1 || { + cat during-build.log; echo "FAIL: build with during_build failed"; exit 1; } +beat_is_frozen || { + echo "FAIL: during_build kept running after the build (or never ran)" + wc -c beat.log 2>/dev/null; exit 1; } +# The interval closed BEFORE the terminal hook, which is the ordering that +# keeps a "build finished" sound from playing over the music it replaces. +expect_log hooks.log finished + +# ── `loop` restarts a command that ends before its interval does ──────── +rm -f runs.log +write_manifest < loop-on.log 2>&1 || { + cat loop-on.log; echo "FAIL: looped during_build failed the build"; exit 1; } +looped=$(wc -l < runs.log | tr -d ' ') +[[ "$looped" -ge 2 ]] || { + cat loop-on.log; echo "FAIL: loop=true ran $looped time(s), expected >= 2"; exit 1; } + +# Without `loop`, the same command runs exactly once — the denominator that +# makes the count above mean something. +rm -f runs.log +write_manifest < loop-off.log 2>&1 || { + cat loop-off.log; echo "FAIL: unlooped during_build failed the build"; exit 1; } +once=$(wc -l < runs.log | tr -d ' ') +[[ "$once" -eq 1 ]] || { + cat loop-off.log; echo "FAIL: loop=false ran $once time(s), expected 1"; exit 1; } + +# ── A command that cannot stay up stops, rather than spinning ─────────── +# +# `loop` on a typo is a fork bomb otherwise. The bound is five consecutive +# runs that end unsuccessfully within a second. +write_manifest < loop-giveup.log 2>&1 || { + cat loop-giveup.log + echo "FAIL: a during_build that never ran changed the build result"; exit 1; } +grep -q "warning: hook 'during_build' failed to stay up" loop-giveup.log || { + cat loop-giveup.log; echo "FAIL: giving up was not reported"; exit 1; } + +# ── `loop` on a self-closing event is refused, not ignored ────────────── +write_manifest <<'EOF' +[package] +name = "hookprobe" +version = "0.1.0" + +[hooks] +build_start = { cmd = "echo start", loop = true } +EOF +rc=0 +"$MCPP" build > loop-misplaced.log 2>&1 || rc=$? +[[ $rc -ne 0 ]] || { + cat loop-misplaced.log; echo "FAIL: loop on build_start was accepted"; exit 1; } +grep -q "during_build" loop-misplaced.log || { + cat loop-misplaced.log + echo "FAIL: the diagnostic does not name the event that supports loop"; exit 1; } + +# ── An interrupted build leaves nothing running ───────────────────────── +# +# POSIX only, and the skip is printed rather than silent: sending Ctrl-C to +# another process on Windows needs a helper this suite does not have, and a +# test that cannot fail there would read as coverage. +if [[ $HOST_WINDOWS -eq 1 ]]; then + echo "SKIP(within test): Ctrl-C cleanup — no way to signal another process here" +else + rm -f beat.log + write_manifest < interrupted.log 2>&1 & + build_pid=$! + sleep 3 + kill -INT "$build_pid" 2>/dev/null || true + wait "$build_pid" 2>/dev/null || true + beat_is_frozen || { + echo "FAIL: Ctrl-C left during_build running"; exit 1; } +fi + +# ── A DEPENDENCY's hooks never run ────────────────────────────────────── +# +# Every manifest mcpp parses carries a `[hooks]` field, a dependency's +# included. Only the root project's is ever invoked, and that is the whole +# reason `mcpp add` of a third-party package is not "run their shell command +# on my next build". A comment cannot hold that; this can. +DEP="$TMP/dep" +DEP_HOST="$(host_path "$DEP")" +mkdir -p "$DEP/src" +cat > "$DEP/src/dep.cppm" <<'EOF' +export module dep; +export int dep_answer() { return 0; } +EOF +cat > "$DEP/mcpp.toml" <<'EOF' +[package] +name = "dep" +version = "0.1.0" + +[targets.dep] +kind = "lib" + +[hooks] +build_start = "echo dependency>>dependency-hook.log" +build_finished = "echo dependency>>dependency-hook.log" +EOF +: > hooks.log +rm -f "$DEP/dependency-hook.log" dependency-hook.log +write_manifest < src/main.cpp <<'EOF' +import dep; +int main() { return dep_answer(); } +EOF +"$MCPP" build > dependency-hooks.log 2>&1 || { + cat dependency-hooks.log; echo "FAIL: build with a hooked dependency failed"; exit 1; } +expect_log hooks.log start finished +[[ ! -e "$DEP/dependency-hook.log" && ! -e dependency-hook.log ]] || { + echo "FAIL: a dependency's hooks ran"; exit 1; } + +# ── A workspace runs each MEMBER's hooks, in that member's root ───────── +# +# Members are separate packages, so their hooks are separate too — and the +# workspace root's own `[hooks]` belongs to a node that builds nothing. +WS="$TMP/ws" +mkdir -p "$WS/alpha/src" "$WS/beta/src" +cat > "$WS/mcpp.toml" <<'EOF' +[workspace] +members = ["alpha", "beta"] + +[hooks] +build_start = "echo root>>root.log" +build_finished = "echo root>>root.log" +EOF +for member in alpha beta; do + cat > "$WS/$member/src/main.cpp" <<'EOF' +int main() { return 0; } +EOF + cat > "$WS/$member/mcpp.toml" < ws-build.log 2>&1 || { + cat ws-build.log; echo "FAIL: workspace build with hooks failed"; exit 1; } +expect_log alpha/hooks.log alpha-start alpha-finished +expect_log beta/hooks.log beta-start beta-finished +[[ ! -e root.log ]] || { + cat root.log; echo "FAIL: the workspace root's own hooks fired"; exit 1; } +[[ ! -e hooks.log ]] || { + cat hooks.log; echo "FAIL: a member hook ran in the workspace root"; exit 1; } + +echo "OK" diff --git a/tests/unit/test_manifest.cpp b/tests/unit/test_manifest.cpp index ffe3418a..5b0b1f9a 100644 --- a/tests/unit/test_manifest.cpp +++ b/tests/unit/test_manifest.cpp @@ -3965,3 +3965,271 @@ cmdline = { version = "0.0.x", features = ["a"] } if (w.find("not a requirement") != std::string::npos) found = true; EXPECT_TRUE(found); } + +// ─── [hooks] — project build lifecycle commands (#496) ─────────────────── +// +// The section is parsed here rather than by the module that runs the commands, +// so this is where its grammar is pinned. The invocation policy (what a +// failure costs, when each event fires) is e2e 314's subject. + +namespace { +constexpr std::string_view kHookPreamble = R"( +[package] +name = "x" +version = "0.1.0" +)"; + +std::expected +parse_with_hooks(std::string_view hooksSection) { + return mcpp::manifest::parse_string( + std::string(kHookPreamble) + std::string(hooksSection)); +} +} // namespace + +TEST(ManifestHooks, AbsentSectionIsInertButNotDisabled) { + auto m = mcpp::manifest::parse_string(kHookPreamble); + ASSERT_TRUE(m.has_value()) << (m ? "" : m.error().format()); + // `enabled` defaults to true and says nothing about whether there is work + // to do; `active()` is the predicate the fast path is allowed to consult. + EXPECT_TRUE(m->hooks.enabled); + EXPECT_FALSE(m->hooks.active()); + EXPECT_EQ(m->hooks.timeoutSeconds, 10); + // Experimental: a hook may not decide whether a build succeeded, so this + // defaults to false and is the only value the parser accepts. + EXPECT_FALSE(m->hooks.sideEffect); +} + +// ⚠️ The experimental gate. Refused rather than downgraded: honouring it would +// give an experimental feature a veto over every build, and ignoring it would +// leave the project believing its build is gated on a notifier when nothing +// is. Both silent options are worse than the error. +TEST(ManifestHooks, SideEffectTrueIsRefusedWhileTheFeatureIsExperimental) { + auto m = parse_with_hooks(R"( +[hooks] +build_finished = "echo done" +side_effect = true +)"); + ASSERT_FALSE(m.has_value()); + EXPECT_NE(m.error().message.find("experimental"), std::string::npos) + << m.error().message; + EXPECT_NE(m.error().message.find("side_effect"), std::string::npos) + << m.error().message; +} + +// The key stays in the vocabulary: writing the value that IS the current +// behaviour must keep working, so a manifest does not have to change when the +// feature is promoted. +TEST(ManifestHooks, SideEffectFalseIsAcceptedSoTheKeyStaysReserved) { + auto m = parse_with_hooks(R"( +[hooks] +build_finished = "echo done" +side_effect = false +)"); + ASSERT_TRUE(m.has_value()) << (m ? "" : m.error().format()); + EXPECT_FALSE(m->hooks.sideEffect); +} + +TEST(ManifestHooks, CommandsAndPolicyAreRead) { + auto m = parse_with_hooks(R"( +[hooks] +build_start = "echo start" +build_failed = "echo failed" +build_finished = "echo finished" +timeout_seconds = 30 +side_effect = false +)"); + ASSERT_TRUE(m.has_value()) << (m ? "" : m.error().format()); + EXPECT_EQ(m->hooks.buildStart.cmd, "echo start"); + EXPECT_EQ(m->hooks.buildFailed.cmd, "echo failed"); + EXPECT_EQ(m->hooks.buildFinished.cmd, "echo finished"); + EXPECT_EQ(m->hooks.timeoutSeconds, 30); + EXPECT_FALSE(m->hooks.sideEffect); + EXPECT_TRUE(m->hooks.active()); +} + +// `enabled = false` has to switch off a table that still names commands — +// otherwise the only way to silence hooks is to delete them. +TEST(ManifestHooks, DisabledTableIsNotActive) { + auto m = parse_with_hooks(R"( +[hooks] +build_start = "echo start" +enabled = false +)"); + ASSERT_TRUE(m.has_value()) << (m ? "" : m.error().format()); + EXPECT_EQ(m->hooks.buildStart.cmd, "echo start"); + EXPECT_FALSE(m->hooks.active()); +} + +// Policy without a command is not work: a table that only raises the timeout +// must leave the fast path alone. +TEST(ManifestHooks, PolicyOnlyTableIsNotActive) { + auto m = parse_with_hooks(R"( +[hooks] +timeout_seconds = 60 +)"); + ASSERT_TRUE(m.has_value()) << (m ? "" : m.error().format()); + EXPECT_FALSE(m->hooks.active()); + EXPECT_EQ(m->hooks.timeoutSeconds, 60); +} + +// Both ends of the documented range are the range: the largest accepted value +// is asserted next to the smallest rejected one, so a change to either is a +// change to a test. +TEST(ManifestHooks, TheDocumentedTimeoutCeilingIsAccepted) { + auto m = parse_with_hooks(R"( +[hooks] +build_start = "echo start" +timeout_seconds = 86400 +)"); + ASSERT_TRUE(m.has_value()) << (m ? "" : m.error().format()); + EXPECT_EQ(m->hooks.timeoutSeconds, 86400); +} + +TEST(ManifestHooks, ValueErrorsAreRejectedWithTheKeyNamed) { + struct Case { std::string_view section; std::string_view needle; }; + for (auto const& c : { + Case{R"([hooks] +timeout_seconds = 0)", "timeout_seconds"}, + Case{R"([hooks] +timeout_seconds = "10")", "timeout_seconds"}, + Case{R"([hooks] +build_start = "")", "build_start"}, + Case{R"([hooks] +build_finished = 7)", "build_finished"}, + Case{R"([hooks] +enabled = "yes")", "enabled"}, + Case{R"([hooks] +side_effect = 1)", "side_effect"}, + // The upper bound is documented (1–86400), so it is asserted. An + // undocumented ceiling and no ceiling look the same from outside. + Case{R"([hooks] +timeout_seconds = 86401)", "timeout_seconds"}, + }) { + auto m = parse_with_hooks(c.section); + ASSERT_FALSE(m.has_value()) << c.section; + EXPECT_NE(m.error().message.find(c.needle), std::string::npos) + << c.section << " -> " << m.error().message; + } +} + +// `hooks = "…"` is the shape a user reaches for when they want one command +// and no table. Saying so beats parsing nothing and running nothing. +TEST(ManifestHooks, AScalarHooksKeyIsRejected) { + auto m = mcpp::manifest::parse_string(R"( +hooks = "echo hi" + +[package] +name = "x" +version = "0.1.0" +)"); + ASSERT_FALSE(m.has_value()); + EXPECT_NE(m.error().message.find("[hooks]"), std::string::npos) + << m.error().message; +} + +// An unknown KEY is a warning, not an error — the same split [build] uses. +// A manifest written for a later mcpp (say, a `build_cancelled` event) still +// loads on this one, while a typo is still reported. +TEST(ManifestHooks, UnknownKeyWarnsInsteadOfFailing) { + auto m = parse_with_hooks(R"( +[hooks] +build_start = "echo start" +build_stared = "echo typo" +)"); + ASSERT_TRUE(m.has_value()) << (m ? "" : m.error().format()); + bool found = false; + for (auto const& w : m->schemaWarnings) + if (w.find("build_stared") != std::string::npos) found = true; + EXPECT_TRUE(found); + EXPECT_TRUE(m->hooks.active()); +} + +// ─── The table form, and the interval each event names ─────────────────── + +TEST(ManifestHooks, ABareStringIsSugarForATable) { + auto m = parse_with_hooks(R"( +[hooks] +build_start = "echo start" +)"); + ASSERT_TRUE(m.has_value()) << (m ? "" : m.error().format()); + EXPECT_EQ(m->hooks.buildStart.cmd, "echo start"); + EXPECT_EQ(m->hooks.buildStart.timeoutSeconds, 0); // 0 = inherit + EXPECT_FALSE(m->hooks.buildStart.loop); +} + +// The per-event value wins over the table's, and 0 is what makes "not given" +// expressible — without it there is no way to tell an inherited 10 from a +// declared one. +TEST(ManifestHooks, APerEventTimeoutOverridesTheTableDefault) { + auto m = parse_with_hooks(R"( +[hooks] +timeout_seconds = 10 +build_finished = { cmd = "slow-notifier", timeout_seconds = 45 } +build_start = "echo start" +)"); + ASSERT_TRUE(m.has_value()) << (m ? "" : m.error().format()); + EXPECT_EQ(m->hooks.timeout_for(m->hooks.buildFinished), 45); + EXPECT_EQ(m->hooks.timeout_for(m->hooks.buildStart), 10); +} + +TEST(ManifestHooks, DuringBuildIsReadAndMakesTheTableActive) { + auto m = parse_with_hooks(R"( +[hooks] +during_build = { cmd = "play bgm.mp3", loop = true } +)"); + ASSERT_TRUE(m.has_value()) << (m ? "" : m.error().format()); + EXPECT_EQ(m->hooks.duringBuild.cmd, "play bgm.mp3"); + EXPECT_TRUE(m->hooks.duringBuild.loop); + EXPECT_TRUE(m->hooks.active()); +} + +// ⭐ The two keys that only exist for one interval. Accepted-and-ignored is +// the failure mode this rejects: the user writes `loop` on `build_start`, +// nothing repeats, and the feature looks broken rather than misused. Each +// message names the event that does support the key. +TEST(ManifestHooks, AKeyOfferedToTheWrongIntervalIsRejected) { + auto loopOnSelfClosing = parse_with_hooks(R"( +[hooks] +build_start = { cmd = "play bgm.mp3", loop = true } +)"); + ASSERT_FALSE(loopOnSelfClosing.has_value()); + EXPECT_NE(loopOnSelfClosing.error().message.find("during_build"), + std::string::npos) << loopOnSelfClosing.error().message; + + auto timeoutOnSpanning = parse_with_hooks(R"( +[hooks] +during_build = { cmd = "play bgm.mp3", timeout_seconds = 30 } +)"); + ASSERT_FALSE(timeoutOnSpanning.has_value()); + EXPECT_NE(timeoutOnSpanning.error().message.find("timeout_seconds"), + std::string::npos) << timeoutOnSpanning.error().message; +} + +TEST(ManifestHooks, ATableWithoutCmdIsRejected) { + for (std::string_view section : { + R"([hooks] +during_build = { loop = true })", + R"([hooks] +build_start = { cmd = "" })", + R"([hooks] +build_start = 42)", + }) { + auto m = parse_with_hooks(section); + ASSERT_FALSE(m.has_value()) << section; + EXPECT_NE(m.error().message.find("cmd"), std::string::npos) + << section << " -> " << m.error().message; + } +} + +TEST(ManifestHooks, AnUnknownKeyInsideAnEventTableWarns) { + auto m = parse_with_hooks(R"( +[hooks] +during_build = { cmd = "play bgm.mp3", lopo = true } +)"); + ASSERT_TRUE(m.has_value()) << (m ? "" : m.error().format()); + bool found = false; + for (auto const& w : m->schemaWarnings) + if (w.find("lopo") != std::string::npos) found = true; + EXPECT_TRUE(found); + EXPECT_TRUE(m->hooks.active()); +} diff --git a/tests/unit/test_windows_command_line.cpp b/tests/unit/test_windows_command_line.cpp index 191acf95..a9c3656f 100644 --- a/tests/unit/test_windows_command_line.cpp +++ b/tests/unit/test_windows_command_line.cpp @@ -66,6 +66,51 @@ TEST(WindowsCommandLine, EmbeddedQuotesAreEscaped) { EXPECT_EQ(q, "\"a\\\"b\""); } +// ─── A user-authored shell command (project `[hooks]`, #496) ───────────── +// +// What cmd.exe does with `/c ` when /S is given: strip the first +// character and the last quote character, run the rest. Modelling it here is +// the whole point — the assertion is "the author's command arrives verbatim", +// not "the string looks plausible". +static std::string cmd_c_tail_under_slash_s(std::string_view line) { + constexpr std::string_view kPrefix = "cmd.exe /d /s /c "; + EXPECT_TRUE(line.starts_with(kPrefix)) << line; + std::string tail(line.substr(kPrefix.size())); + if (tail.empty() || tail.front() != '"') return tail; // rule does not fire + tail.erase(0, 1); + tail.erase(tail.rfind('"'), 1); + return tail; +} + +// The switches must arrive BARE. Quoted (`"cmd.exe" "/d" "/s" "/c" "..."`, +// which is what windows_command_from_argv produces for the same argv) they +// are no longer switches, and the command tail keeps a quote pair cmd never +// consumes — the CI failure this shape exists to prevent. +TEST(WindowsCommandLine, ShellCommandKeepsCmdSwitchesBare) { + auto line = proc::windows_shell_command_line("echo hi"); + EXPECT_TRUE(line.starts_with("cmd.exe /d /s /c ")) << line; + EXPECT_EQ(line.find("\"/c\""), std::string::npos) << line; +} + +// A redirect is the ordinary case for a hook that appends to a log, and it is +// also the case argv quoting destroys: `>` inside a quoted argument is a +// literal, not a redirect. +TEST(WindowsCommandLine, ShellCommandDeliversARedirectVerbatim) { + constexpr std::string_view command = "echo start>>hooks.log"; + EXPECT_EQ(cmd_c_tail_under_slash_s(proc::windows_shell_command_line(command)), + command); +} + +// More than one interior quote pair is exactly where the /C rule bites, and +// where the "wrap once" shape earns its keep: whatever the author wrote comes +// back byte for byte. +TEST(WindowsCommandLine, ShellCommandDeliversQuotedPathsVerbatim) { + constexpr std::string_view command = + R"("C:\Program Files\notify\notify.exe" --title "build done")"; + EXPECT_EQ(cmd_c_tail_under_slash_s(proc::windows_shell_command_line(command)), + command); +} + // The POSIX half must keep its own convention — a shared helper that // silently applied Windows quoting on Linux would break every sh command. TEST(WindowsCommandLine, PosixQuotingIsUnaffected) {