Skip to content

Commit 0d8db93

Browse files
committed
feat(hooks): ship it experimental — a hook gets no vote on the build
`[hooks]` is marked experimental, and the marking is enforced rather than written down: `side_effect` now defaults to FALSE, and `side_effect = true` is REFUSED by the manifest parser. error: mcpp.toml: error: [hooks].side_effect = true is not available yet: [hooks] is experimental and cannot decide whether a build succeeded. … So every hook failure — cannot start, non-zero, timed out, failed to stay up — is a warning, and `mcpp build` keeps the result it earned on its own. Refused rather than quietly downgraded, because both silent behaviours are worse than an error. Honouring `true` would ship an experimental feature with a veto over every build. Ignoring it would leave a project believing its build is gated on a notifier when nothing is — which is the "accepted and does nothing" shape this design already rejects for `loop` on a self-closing event and for `timeout_seconds` on `during_build`. Making an exception for the one key whose wrong answer is invisible would be exactly backwards. The key stays in the schema, so a manifest written today does 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. Two further limits are documented as permanent rather than provisional, so they are not read as part of the experiment: a dependency's `[hooks]` is skipped always (there is one Span construction and there are two invoke call sites, all in run_build_with_hooks, all fed from the context's own manifest), and only `mcpp build` runs hooks. Criteria: the e2e now asserts BOTH halves — the exit code says the hook had no vote, and the warning says the failure was not swallowed. Checking only the exit code would pass just as well if hooks had stopped running altogether. The timeout and give-up cases flipped from "build fails with error:" to "build succeeds with warning:", and a negative assertion was rewritten as `if grep` rather than `grep && { }`, whose exit status under `set -e` is an argument this file should not be having. Hook context (MCPP_PROFILE, MCPP_TARGET, …) is deferred; the experimental window is where that interface can still be named deliberately.
1 parent fa14e6d commit 0d8db93

8 files changed

Lines changed: 236 additions & 49 deletions

File tree

.agents/docs/2026-08-30-project-build-hooks-owned-intervals.md

Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -178,6 +178,37 @@ would interleave into the middle of a compiler diagnostic. Its output is
178178
therefore discarded by default, and inherited under `--verbose` — no new schema
179179
key, and the answer to "why is there no music" is one flag away.
180180
181+
## Shipping it experimental
182+
183+
The feature ships with `side_effect` defaulting to **false** and `true`
184+
*refused* by the manifest parser. A hook therefore cannot change whether a
185+
build succeeded: every failure is a warning and `mcpp build` keeps the result
186+
it earned on its own.
187+
188+
Refused rather than downgraded, because both silent behaviours are worse than
189+
an error. Honouring `true` ships an experimental feature with a veto over every
190+
build. Ignoring it leaves a project believing its build is gated on a notifier
191+
when nothing is — the "accepted and does nothing" shape this design rejects
192+
everywhere else (`loop` on a self-closing event, `timeout_seconds` on
193+
`during_build`), so it would be inconsistent to make an exception for the one
194+
key whose wrong answer is invisible.
195+
196+
The key stays in the schema so manifests do not have to change when the feature
197+
is promoted, and the mechanism under it already implements both values — the
198+
`sideEffect == true` branch in `mcpp.hooks` is unreachable today ON PURPOSE.
199+
**Promotion is the deletion of one block in the parser**, not a
200+
reconstruction. That is the property to preserve when editing either side.
201+
202+
Two further limits are permanent rather than provisional and should not be read
203+
as part of the experiment:
204+
205+
- Only the ROOT project's hooks run. A dependency's `[hooks]` is inert by
206+
construction — there is exactly one `Span` construction and there are exactly
207+
two `invoke` call sites, all in `run_build_with_hooks`, all fed from the
208+
context's own manifest.
209+
- Only `mcpp build` runs hooks. `mcpp run`, `mcpp test` and
210+
`mcpp build --configure-only` build too, and deliberately do not.
211+
181212
## Criteria
182213
183214
The assertions this design has to earn are about *state*, not about log lines —
@@ -202,3 +233,7 @@ The assertions this design has to earn are about *state*, not about log lines
202233
rather than papered over with a test that passes vacuously.
203234
7. **A self-closing event rejects `loop`.** Otherwise the key is accepted,
204235
does nothing, and the user concludes the feature does not work.
236+
8. **`side_effect = true` is refused, and every failure mode is a warning.**
237+
Two assertions, not one: the exit code says the hook had no vote, and the
238+
warning says the failure was not swallowed. A test that checked only the
239+
exit code would pass just as well if hooks had stopped running altogether.

docs/05-mcpp-toml.md

Lines changed: 39 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -2094,7 +2094,15 @@ See [07 — build.mcpp](07-build-mcpp.md). Naming such a file in
20942094
command: nothing tracks it, and editing the file produces `ninja: no work to
20952095
do`.
20962096

2097-
### 2.16 `[hooks]` — Project Build Lifecycle Commands
2097+
### 2.16 `[hooks]` — Project Build Lifecycle Commands (experimental)
2098+
2099+
> **Experimental.** A hook cannot currently decide whether a build succeeded.
2100+
> Every hook failure is reported as a **warning** and `mcpp build` keeps the
2101+
> result it earned on its own; `side_effect = true` is refused with an error
2102+
> rather than honoured. The key stays in the schema so that manifests written
2103+
> today do not have to change when the feature is promoted. Two further limits
2104+
> are permanent rather than provisional: only the root project's hooks run, and
2105+
> only `mcpp build` runs them.
20982106
20992107
A hook is a command `mcpp build` **owns for an interval**, and the event names
21002108
the interval:
@@ -2111,7 +2119,7 @@ during_build = { cmd = "mcpp-hooks-audioplayer bgm", loop = true }
21112119
# Optional; these are the defaults.
21122120
timeout_seconds = 10
21132121
enabled = true
2114-
side_effect = true
2122+
side_effect = false # `true` is refused while this is experimental
21152123
```
21162124

21172125
| Key | Type | Default | The interval it names |
@@ -2122,7 +2130,7 @@ side_effect = true
21222130
| `during_build` | command || Opens before the build, closes after it |
21232131
| `timeout_seconds` | integer, 1–86400 | `10` | Bounds one run of a command |
21242132
| `enabled` | bool | `true` | Enables all commands in this table |
2125-
| `side_effect` | bool | `true` | Whether a hook failure makes the overall build fail |
2133+
| `side_effect` | bool | `false` | Whether a hook failure makes the build fail. **Reserved** — only `false` is accepted while this is experimental |
21262134

21272135
The first three intervals are **self-closing** — they end when the command
21282136
does. "Synchronous" is not a separate mode here; it is what a self-closing
@@ -2172,11 +2180,24 @@ hook failure. For `during_build` there is one more: a looped command that
21722180
**fails to stay up** — five consecutive runs ending unsuccessfully within a
21732181
second — stops being restarted and is reported. (A command that finishes
21742182
quickly and *successfully* is doing exactly what `loop` was asked to repeat,
2175-
and is not a failure.) With `side_effect = false`, mcpp reports a warning and
2176-
preserves the build's result; with `true`, it returns failure — but a build that
2177-
failed on its own keeps its own exit code, so `mcpp build` never reports a
2178-
compile error as a notifier problem. A hook's own failure does not trigger
2179-
another hook.
2183+
and is not a failure.) Every one of those is reported as a **warning**, and the
2184+
build keeps the result it earned on its own — while `[hooks]` is experimental
2185+
it does not get a vote. A hook's own failure does not trigger another hook.
2186+
2187+
`side_effect = true` is what will change that, and asking for it today is an
2188+
error:
2189+
2190+
```text
2191+
error: mcpp.toml: error: [hooks].side_effect = true is not available yet:
2192+
[hooks] is experimental and cannot decide whether a build succeeded. …
2193+
```
2194+
2195+
Refused rather than quietly downgraded, because both silent options are worse:
2196+
honouring it would give an experimental feature a veto over every build, and
2197+
ignoring it would leave a project believing its build is gated on a notifier
2198+
when nothing is. When the feature is promoted, `true` will mean "a hook failure
2199+
fails the build" — and a build that failed on its own will still keep its own
2200+
exit code, so `mcpp build` never reports a compile error as a notifier problem.
21802201

21812202
Two things are worth knowing about a `during_build` command specifically:
21822203

@@ -2197,9 +2218,11 @@ Scope, precisely:
21972218
each member in turn — its own `[hooks]`, around its own build, in its own
21982219
root. A *virtual* workspace root (`[workspace]` with no `[package]`) builds
21992220
nothing, so a `[hooks]` table there never fires.
2200-
- A dependency's `[hooks]` is never run. Only the root project's — installing a
2201-
package cannot make its author's shell command part of the consuming
2202-
project's build.
2221+
- A dependency's `[hooks]` is **skipped**, always. Only the root project's run.
2222+
Every manifest mcpp parses carries the section, a dependency's included, and
2223+
nothing reads it — which is what keeps `mcpp add` from meaning "run this
2224+
author's shell command on my next build". This is a property of the design,
2225+
not a default awaiting a switch.
22032226
- Declaring an active hook opts the project out of the no-op fast path, because
22042227
`build_start` is specified to run after preparation. Expect `mcpp build` on an
22052228
already-current hooked project to cost a preparation pass rather than
@@ -2231,9 +2254,11 @@ side_effect = false
22312254
deps = ["xim:mcpp-hooks-audioplayer@0.0.1"]
22322255
```
22332256

2234-
Background music for the length of the build, a different sound for how it
2235-
ended, and `side_effect = false` so that a missing audio device is a warning
2236-
rather than a failed build.
2257+
Background music for the length of the build, and a different sound for how it
2258+
ended. `side_effect = false` is written out rather than left to the default:
2259+
it is the value this manifest wants on its own terms — a missing audio device
2260+
should never fail a build — so it will still say so once the key has more than
2261+
one accepted value.
22372262

22382263
## Appendix A. Schema Ownership Principle (admission criteria for new fields)
22392264

docs/zh/05-mcpp-toml.md

Lines changed: 30 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -1791,7 +1791,13 @@ o.arg("./mkblob.sh").arg("blob.bin").arg("${mcpp.out_dir}/blob.o")
17911791
但 ldflags 是链接命令里的一串字符:没有任何东西跟踪它,改了它得到的是
17921792
`ninja: no work to do`
17931793

1794-
### 2.16 `[hooks]` —— 项目构建生命周期命令
1794+
### 2.16 `[hooks]` —— 项目构建生命周期命令(实验性)
1795+
1796+
> **实验性。** Hook 目前**不能**决定一次构建成功与否。所有 Hook 失败都以
1797+
> **warning** 报出,`mcpp build` 保留它自己挣来的结果;`side_effect = true`
1798+
> 会被报错拒绝,而不是被采纳。这个键保留在 schema 里,这样今天写下的 manifest
1799+
> 在该功能转正时无需改动。另有两条限制是永久的、不是临时的:**只有根项目的 Hook
1800+
> 会执行**,而且**只有 `mcpp build` 会执行它们**
17951801
17961802
Hook 是 `mcpp build` **在一段区间内持有**的命令,事件名就是那段区间:
17971803

@@ -1807,7 +1813,7 @@ during_build = { cmd = "mcpp-hooks-audioplayer bgm", loop = true }
18071813
# 可选;以下是默认值。
18081814
timeout_seconds = 10
18091815
enabled = true
1810-
side_effect = true
1816+
side_effect = false # 实验期内 `true` 会被拒绝
18111817
```
18121818

18131819
|| 类型 | 默认值 | 它命名的区间 |
@@ -1818,7 +1824,7 @@ side_effect = true
18181824
| `during_build` | 命令 || 构建开始前开启,构建结束后闭合 |
18191825
| `timeout_seconds` | 整数,1–86400 | `10` | 单次运行的时限 |
18201826
| `enabled` | 布尔 | `true` | 是否启用本表中的全部命令 |
1821-
| `side_effect` | 布尔 | `true` | Hook 失败是否让本次构建失败 |
1827+
| `side_effect` | 布尔 | `false` | Hook 失败是否让本次构建失败**保留键**——实验期内只接受 `false` |
18221828

18231829
前三个区间是**自闭合**的——命令退出,区间就结束。"同步"在这里不是一种单独的模式,
18241830
它就是自闭合区间的样子。`during_build` 是唯一由别的东西闭合的区间,而那两个只对其中
@@ -1858,10 +1864,21 @@ build_start
18581864

18591865
命令无法启动、返回非零或超过时限均视为 Hook 失败。`during_build` 还多一种:开了 `loop`
18601866
的命令**起不来**——连续五次在一秒内以非零状态结束——就不再重启,并被报出来。(很快就
1861-
成功结束的命令,正是 `loop` 被要求重复的那件事,不算失败。)`side_effect = false`
1862-
mcpp 报 warning 并保留原构建结果;设为 `true` 时返回失败——但构建自身失败时保留它自己
1863-
的退出码,所以 `mcpp build` 不会把一次编译错误报成通知程序的问题。Hook 自身失败不会再
1864-
触发另一个 Hook。
1867+
成功结束的命令,正是 `loop` 被要求重复的那件事,不算失败。)以上每一种都以 **warning**
1868+
报出,构建保留它自己挣来的结果——`[hooks]` 还在实验期,它没有投票权。Hook 自身失败不会
1869+
再触发另一个 Hook。
1870+
1871+
改变这一点的正是 `side_effect = true`,而今天写它是一个错误:
1872+
1873+
```text
1874+
error: mcpp.toml: error: [hooks].side_effect = true is not available yet:
1875+
[hooks] is experimental and cannot decide whether a build succeeded. …
1876+
```
1877+
1878+
是拒绝而不是悄悄降级,因为两种沉默的做法都更糟:采纳它等于让一个实验性功能对每一次
1879+
构建都有否决权;忽略它则让项目以为自己的构建被通知程序把着关,而实际上没有。功能转正
1880+
后,`true` 的含义是"Hook 失败让构建失败"——而构建自身失败时仍保留它自己的退出码,所以
1881+
`mcpp build` 不会把一次编译错误报成通知程序的问题。
18651882

18661883
关于 `during_build` 有两件事值得单独知道:
18671884

@@ -1878,8 +1895,9 @@ mcpp 报 warning 并保留原构建结果;设为 `true` 时返回失败——但
18781895
- Hook 属于**被构建的那个包**。workspace 展开时就是逐个成员:各自的 `[hooks]`
18791896
各自的构建、各自的根目录。**虚拟** workspace 根(只有 `[workspace]` 没有
18801897
`[package]`)不构建任何东西,写在那里的 `[hooks]` 永不触发。
1881-
- 依赖的 `[hooks]` 永不执行,只有根项目的会——装一个包不会把包作者的 Shell 命令
1882-
变成消费方构建的一部分。
1898+
- 依赖的 `[hooks]` **一律跳过**,只有根项目的会执行。mcpp 解析的每一份 manifest 都
1899+
带着这一节,依赖的也带,而没有任何东西去读它——这正是"装一个包"不会变成"在我下次
1900+
构建时跑包作者的 Shell 命令"的原因。这是设计的性质,不是一个等着被打开的默认值。
18831901
- 声明了生效的 Hook 就等于让项目放弃空转快路径,因为 `build_start` 规定在准备阶段之后
18841902
执行。对已经是最新状态的带 Hook 项目,`mcpp build` 的代价是一次准备,而不是毫秒级。
18851903

@@ -1906,8 +1924,9 @@ side_effect = false
19061924
deps = ["xim:mcpp-hooks-audioplayer@0.0.1"]
19071925
```
19081926

1909-
构建全程的背景音乐,加上一段区分结果的提示音;`side_effect = false` 让缺少音频设备
1910-
只是一条 warning,而不是一次失败的构建。
1927+
构建全程的背景音乐,加上一段区分结果的提示音。`side_effect = false` 写出来而不是靠
1928+
默认值:它是这份 manifest 自己就想要的值——缺个音频设备不该让构建失败——所以等这个键
1929+
有了不止一个可接受的值之后,它仍然会这么写。
19111930

19121931
## 附录 A. Schema 所有权原则(新字段准入标准)
19131932

modules/manifest/src/toml.cppm

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1651,6 +1651,26 @@ std::expected<Manifest, ManifestError> parse_string(std::string_view content,
16511651
*out = it->second.as_bool();
16521652
}
16531653

1654+
// ⚠️ THE EXPERIMENTAL GATE, AND THE WHOLE OF IT.
1655+
//
1656+
// `[hooks]` is experimental, so it may not decide whether a build
1657+
// succeeded. Asking for `side_effect = true` is refused rather than
1658+
// downgraded, because the two possible silent behaviours are both
1659+
// worse than an error: honouring it ships an experimental feature with
1660+
// a veto over every build, and ignoring it leaves a project believing
1661+
// its build is gated on a notifier when nothing is.
1662+
//
1663+
// Everything under this line already implements both values. Deleting
1664+
// this block is what promoting the feature consists of.
1665+
if (m.hooks.sideEffect)
1666+
return std::unexpected(error(origin,
1667+
"[hooks].side_effect = true is not available yet: [hooks] is "
1668+
"experimental and cannot decide whether a build succeeded. A "
1669+
"failing hook is reported as a warning and the build keeps its "
1670+
"own result. Remove the key (the default is false) — it is "
1671+
"reserved so that manifests do not have to change when the "
1672+
"feature is promoted."));
1673+
16541674
static constexpr std::string_view kKnownHookKeys[] = {
16551675
"build_start", "build_failed", "build_finished", "during_build",
16561676
"timeout_seconds", "enabled", "side_effect",

modules/manifest/src/types.cppm

Lines changed: 14 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -971,9 +971,20 @@ struct Hooks {
971971

972972
int timeoutSeconds = 10; // default for one run of a hook command
973973
bool enabled = true; // whole table
974-
// Whether a hook failure fails the build. False downgrades it to a
975-
// warning and preserves whatever the build itself returned.
976-
bool sideEffect = true;
974+
975+
// ⚠️ EXPERIMENTAL: FALSE, AND CURRENTLY THE ONLY VALUE.
976+
//
977+
// The key means "a hook failure fails the build". While `[hooks]` is
978+
// experimental it does not get to decide that: a hook that fails is
979+
// reported as a warning and the build keeps whatever result it earned on
980+
// its own. `side_effect = true` is REJECTED by the parser rather than
981+
// accepted and ignored — a project that believes its build is gated on a
982+
// notifier, and is not, has been told something false.
983+
//
984+
// The mechanism below it is intact and is what the key will switch on when
985+
// the feature graduates; the parser check is the whole of the gate, so
986+
// removing it is the whole of the change.
987+
bool sideEffect = false;
977988

978989
// "This project has work for `mcpp build` to do." Distinct from `enabled`:
979990
// a table that only sets policy keys declares no command, and must leave

src/hooks.cppm

Lines changed: 13 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,10 @@
11
// mcpp.hooks — running the project's `[hooks]` lifecycle commands.
22
//
3+
// ⚠️ EXPERIMENTAL. A hook cannot currently change whether a build succeeded:
4+
// every failure is a warning, and `side_effect = true` is refused by the
5+
// manifest parser. Only the ROOT project's hooks are ever run — a dependency's
6+
// `[hooks]` is inert. See docs/05-mcpp-toml.md §2.16.
7+
//
38
// The CONFIGURATION is not parsed here: `[hooks]` is a section of mcpp.toml
49
// and mcpp.toml has one parser (mcpp.manifest). What lives here is the part
510
// that is policy rather than grammar — when a command runs, what a failure
@@ -72,7 +77,7 @@ private:
7277
bool declared_ = false;
7378
bool started_ = false;
7479
bool closed_ = false;
75-
bool sideEffect_ = true;
80+
bool sideEffect_ = false;
7681
bool ok_ = true;
7782
std::atomic<bool> stop_{false};
7883
std::atomic<bool> gaveUp_{false};
@@ -125,6 +130,13 @@ event_command(const mcpp::manifest::Hooks& hooks, Event event) {
125130
// four of them (cannot start, non-zero, timed out, failed to stay up). The
126131
// message is identical either way; only its severity and the build's fate
127132
// differ.
133+
//
134+
// ⚠️ WHILE `[hooks]` IS EXPERIMENTAL, `sideEffect` IS ALWAYS FALSE — the
135+
// manifest parser refuses `side_effect = true` (see
136+
// modules/manifest/src/toml.cppm). The `true` branch below is therefore
137+
// unreachable today ON PURPOSE: it is the behaviour the key will select when
138+
// the feature is promoted, and keeping it here means promotion is a deletion
139+
// in the parser rather than a reconstruction here.
128140
bool report_hook_failure_flag(bool sideEffect, const std::string& message) {
129141
if (sideEffect) {
130142
mcpp::ui::error(message);

0 commit comments

Comments
 (0)