Skip to content

Commit 9b9f026

Browse files
committed
review: one parser for mcpp.toml, and a cmd.exe line cmd.exe accepts
Follow-up to the [hooks] feature. Five things the first shape got wrong, in descending order of how loudly they fail. 1. Windows: the hook never ran. `run_shell_deadline` built the command line with `windows_command_from_argv({"cmd.exe","/d","/s","/c", command})`, which quotes every token — so the switches stopped being switches and the command arrived carrying a quote pair cmd.exe does not consume: '"echo start>>hooks.log' is not recognized as an internal or external command That is #425 one layer up (`cmd.exe /c` does not use CreateProcess argument quoting). A shell command now has its own host-independent shaper, `windows_shell_command_line` — bare switches plus the single outer pair /s strips — and its own tests in test_windows_command_line.cpp, which compile on every platform. The Windows branch is unreachable on the machines this is developed on; a unit test is the only thing that can fail there first. 2. `[hooks]` is a section of mcpp.toml, so mcpp.manifest parses it. The feature came with a second, independent reader of the same file. It cost a redundant parse on every build, and it reported ITS syntax errors in ITS vocabulary: any manifest typo, anywhere in the file, came out as `error: invalid hook configuration: ...` for every project whether or not it used hooks. `Manifest::hooks` now carries the config, unknown keys warn (and error under --strict) like every other section, and mcpp.hooks is left with the part that is policy rather than grammar. 3. A hook no longer moves mcpp's working directory. The launchers already carry a per-child cwd (posix_spawn_file_actions_addchdir_np, lpCurrentDirectory); `run_shell_deadline` takes one, and the chdir-and-restore dance around the call is gone. 4. The fast-path veto moved into try_fast_build, next to the manifest read that answers it — cmd_build no longer loads the manifest a second time to ask whether it may call a function that loads it again. 5. Coverage the claims did not have: workspace members (each member's own hooks, in that member's root, root manifest silent), preparation failing fires nothing, an unknown key warns and the known ones still run, and the e2e compares hook logs by CONTENT — cmd.exe writes CRLF, so the previous `$'start\nfinished'` comparison could not have passed on Windows even with the command line fixed. Plus seven manifest tests for the grammar. Docs (en + zh) state what was implicit: which commands run hooks and which deliberately do not, what a virtual workspace root does, that an active hook opts the project out of the fast path, that preparation failure is silent, and that a `[hooks]` table is executable content in a repository you may have just cloned.
1 parent 32437da commit 9b9f026

12 files changed

Lines changed: 821 additions & 337 deletions

File tree

docs/05-mcpp-toml.md

Lines changed: 43 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -2116,13 +2116,15 @@ side_effect = true
21162116
| `build_start` | string || Runs after project preparation, immediately before the build |
21172117
| `build_failed` | string || Runs when the build exits unsuccessfully |
21182118
| `build_finished` | string || Runs when the build exits successfully |
2119-
| `timeout_seconds` | positive integer | `10` | Maximum time for each command |
2119+
| `timeout_seconds` | integer, 1–86400 | `10` | Maximum time for each command |
21202120
| `enabled` | bool | `true` | Enables all commands in this table |
21212121
| `side_effect` | bool | `true` | Whether a hook failure makes the overall build fail |
21222122

2123-
Commands run synchronously in the current project's root directory through the
2124-
host shell (`/bin/sh` or `cmd.exe`). Standard input/output/error keep their
2125-
ordinary terminal behaviour. Missing event commands are skipped.
2123+
Commands run synchronously through the host shell (`/bin/sh` or `cmd.exe`),
2124+
with the **project root** as their working directory — not the directory
2125+
`mcpp build` was typed in, so a relative path in a hook means the same thing
2126+
wherever the build was started. Standard input/output/error keep their ordinary
2127+
terminal behaviour. Missing event commands are skipped.
21262128

21272129
The lifecycle is:
21282130

@@ -2132,11 +2134,43 @@ build_start
21322134
└─ build fails → build_failed
21332135
```
21342136

2135-
`build_failed` and `build_finished` are mutually exclusive. A hook command
2136-
that cannot start, returns non-zero, or exceeds its timeout is a hook failure.
2137-
With `side_effect = false`, mcpp reports a warning and preserves the build's
2138-
result; with `true`, it returns failure. A hook's own failure does not trigger
2139-
another hook.
2137+
`build_failed` and `build_finished` are mutually exclusive, and both are
2138+
reachable only after `build_start` has run. A project that cannot be *prepared*
2139+
— an invalid manifest, an unresolvable dependency, no usable toolchain — fires
2140+
nothing: it has not started building, and its hook program may be exactly what
2141+
preparation would have installed.
2142+
2143+
A hook command that cannot start, returns non-zero, or exceeds its timeout is a
2144+
hook failure. With `side_effect = false`, mcpp reports a warning and preserves
2145+
the build's result; with `true`, it returns failure — but a build that failed on
2146+
its own keeps its own exit code, so `mcpp build` never reports a compile error
2147+
as a notifier problem. A hook's own failure does not trigger another hook.
2148+
2149+
Scope, precisely:
2150+
2151+
- Only `mcpp build` runs hooks. `mcpp run`, `mcpp test` and
2152+
`mcpp build --configure-only` build too, and deliberately do not.
2153+
- Hooks belong to the **package being built**. In a workspace fan-out that is
2154+
each member in turn — its own `[hooks]`, around its own build, in its own
2155+
root. A *virtual* workspace root (`[workspace]` with no `[package]`) builds
2156+
nothing, so a `[hooks]` table there never fires.
2157+
- A dependency's `[hooks]` is never run. Only the root project's — installing a
2158+
package cannot make its author's shell command part of the consuming
2159+
project's build.
2160+
- Declaring an active hook opts the project out of the no-op fast path, because
2161+
`build_start` is specified to run after preparation. Expect `mcpp build` on an
2162+
already-current hooked project to cost a preparation pass rather than
2163+
milliseconds.
2164+
2165+
An unrecognised key in `[hooks]` is a warning (an error under `--strict`), so a
2166+
manifest written for a newer mcpp still loads. An unrecognised *value* — a
2167+
non-string command, a `timeout_seconds` outside 1–86400 — is a manifest error.
2168+
2169+
> **A hook is code, and `mcpp.toml` is part of the repository.** Building a
2170+
> freshly cloned project runs whatever its `[hooks]` say, with the privileges
2171+
> of whoever invoked `mcpp build`. This is the same trust `build.mcpp` already
2172+
> asks for ([07 — build.mcpp](07-build-mcpp.md)); `[hooks]` widens its reach
2173+
> rather than introducing it.
21402174
21412175
Hook programs can be installed as ordinary xlings dependencies. For example,
21422176
an audio notifier can keep its sound files inside its own executable rather

docs/zh/05-mcpp-toml.md

Lines changed: 33 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1812,12 +1812,13 @@ side_effect = true
18121812
| `build_start` | 字符串 || 项目准备完成、正式构建开始前执行 |
18131813
| `build_failed` | 字符串 || 构建以失败状态结束时执行 |
18141814
| `build_finished` | 字符串 || 构建成功结束时执行 |
1815-
| `timeout_seconds` | 正整数 | `10` | 每条命令的最长执行时间 |
1815+
| `timeout_seconds` | 整数,1–86400 | `10` | 每条命令的最长执行时间 |
18161816
| `enabled` | 布尔 | `true` | 是否启用本表中的全部命令 |
18171817
| `side_effect` | 布尔 | `true` | Hook 失败是否让本次构建失败 |
18181818

1819-
命令在当前项目根目录中同步执行,使用宿主 Shell(`/bin/sh``cmd.exe`),标准输入、
1820-
输出和错误沿用普通终端行为。没有配置的事件直接跳过。
1819+
命令通过宿主 Shell(`/bin/sh``cmd.exe`)同步执行,工作目录是**项目根目录**——
1820+
不是敲 `mcpp build` 的那个目录,所以 Hook 里的相对路径在哪儿发起构建都指同一处。
1821+
标准输入、输出和错误沿用普通终端行为。没有配置的事件直接跳过。
18211822

18221823
生命周期为:
18231824

@@ -1827,9 +1828,35 @@ build_start
18271828
└─ 构建失败 → build_failed
18281829
```
18291830

1830-
`build_failed``build_finished` 互斥。命令无法启动、返回非零或超过时限均视为
1831-
Hook 失败。`side_effect = false` 时 mcpp 报 warning 并保留原构建结果;设为 `true`
1832-
时返回失败。Hook 自身失败不会再触发另一个 Hook。
1831+
`build_failed``build_finished` 互斥,而且两者都只在 `build_start` 已经执行之后
1832+
才可达。项目**准备**阶段就失败的情况——manifest 非法、依赖无法解析、没有可用工具链
1833+
——一个 Hook 都不触发:此时构建尚未开始,而 Hook 程序本身可能正是准备阶段要装的东西。
1834+
1835+
命令无法启动、返回非零或超过时限均视为 Hook 失败。`side_effect = false` 时 mcpp 报
1836+
warning 并保留原构建结果;设为 `true` 时返回失败——但构建自身失败时保留它自己的退出码,
1837+
所以 `mcpp build` 不会把一次编译错误报成通知程序的问题。Hook 自身失败不会再触发另一个
1838+
Hook。
1839+
1840+
作用范围:
1841+
1842+
- 只有 `mcpp build` 执行 Hook。`mcpp run``mcpp test`
1843+
`mcpp build --configure-only` 同样会构建,但有意不执行。
1844+
- Hook 属于**被构建的那个包**。workspace 展开时就是逐个成员:各自的 `[hooks]`
1845+
各自的构建、各自的根目录。**虚拟** workspace 根(只有 `[workspace]` 没有
1846+
`[package]`)不构建任何东西,写在那里的 `[hooks]` 永不触发。
1847+
- 依赖的 `[hooks]` 永不执行,只有根项目的会——装一个包不会把包作者的 Shell 命令
1848+
变成消费方构建的一部分。
1849+
- 声明了生效的 Hook 就等于让项目放弃空转快路径,因为 `build_start` 规定在准备阶段之后
1850+
执行。对已经是最新状态的带 Hook 项目,`mcpp build` 的代价是一次准备,而不是毫秒级。
1851+
1852+
`[hooks]` 里不认识的****是 warning(`--strict` 下为错误),所以为更新版 mcpp 写的
1853+
manifest 在这一版仍能加载;不认识的****——命令不是字符串、`timeout_seconds` 不在
1854+
1–86400 之间——是 manifest 错误。
1855+
1856+
> **Hook 是代码,而 `mcpp.toml` 是仓库的一部分。** 构建一个刚克隆下来的项目,会以
1857+
> 执行 `mcpp build` 的那个账户的权限,运行它 `[hooks]` 里写的任何东西。这与
1858+
> `build.mcpp`([07 — build.mcpp](07-build-mcpp.md))已经要求的信任是同一份;
1859+
> `[hooks]` 扩大的是它的范围,而不是引入了一份新的信任。
18331860
18341861
Hook 程序可以作为普通 xlings 依赖安装。例如,音频通知程序可以把音频内置进自己的
18351862
可执行文件,无需让 mcpp 处理媒体资源:

modules/manifest/src/toml.cppm

Lines changed: 75 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1520,6 +1520,81 @@ std::expected<Manifest, ManifestError> parse_string(std::string_view content,
15201520
}
15211521
}
15221522

1523+
// [hooks] — project build lifecycle commands (#496). Parsed HERE rather
1524+
// than by the module that runs them, for the reason Appendix A of
1525+
// docs/05-mcpp-toml.md states: mcpp.toml has one grammar and one parser.
1526+
// A second reader of the same file would report ITS syntax errors in its
1527+
// own vocabulary — a typo in [package] arriving as "invalid hook
1528+
// configuration" — and would sit outside the warning/--strict policy every
1529+
// other section is subject to.
1530+
if (auto* hooksValue = doc->get("hooks");
1531+
hooksValue && !hooksValue->is_table()) {
1532+
return std::unexpected(error(origin,
1533+
"[hooks] must be a table of lifecycle commands"));
1534+
}
1535+
if (auto* ht = doc->get_table("hooks")) {
1536+
// Values are the author's own and visible in front of them: a wrong
1537+
// type is an error, not a silent default. An unrecognised KEY is a
1538+
// warning (--strict makes it an error), same split as [build] — so a
1539+
// manifest written for a later mcpp still loads on this one.
1540+
auto read_command = [&](std::string_view key, std::string& out)
1541+
-> std::optional<ManifestError> {
1542+
auto it = ht->find(key);
1543+
if (it == ht->end()) return std::nullopt;
1544+
if (!it->second.is_string() || it->second.as_string().empty())
1545+
return error(origin, std::format(
1546+
"[hooks].{} must be a non-empty command string", key));
1547+
out = it->second.as_string();
1548+
return std::nullopt;
1549+
};
1550+
for (auto [key, out] : std::initializer_list<
1551+
std::pair<std::string_view, std::string*>>{
1552+
{"build_start", &m.hooks.buildStart},
1553+
{"build_failed", &m.hooks.buildFailed},
1554+
{"build_finished", &m.hooks.buildFinished}}) {
1555+
if (auto e = read_command(key, *out)) return std::unexpected(*e);
1556+
}
1557+
1558+
if (auto it = ht->find("timeout_seconds"); it != ht->end()) {
1559+
// Bounded above as well as below: the value becomes a
1560+
// std::chrono::seconds deadline, and "a timeout so large it is not
1561+
// one" is a mistake worth naming rather than honouring.
1562+
constexpr std::int64_t kMaxTimeout = 24 * 60 * 60;
1563+
if (!it->second.is_int() || it->second.as_int() <= 0
1564+
|| it->second.as_int() > kMaxTimeout)
1565+
return std::unexpected(error(origin, std::format(
1566+
"[hooks].timeout_seconds must be a positive integer "
1567+
"(seconds, at most {})", kMaxTimeout)));
1568+
m.hooks.timeoutSeconds = static_cast<int>(it->second.as_int());
1569+
}
1570+
1571+
for (auto [key, out] : std::initializer_list<
1572+
std::pair<std::string_view, bool*>>{
1573+
{"enabled", &m.hooks.enabled},
1574+
{"side_effect", &m.hooks.sideEffect}}) {
1575+
auto it = ht->find(key);
1576+
if (it == ht->end()) continue;
1577+
if (!it->second.is_bool())
1578+
return std::unexpected(error(origin, std::format(
1579+
"[hooks].{} must be a boolean", key)));
1580+
*out = it->second.as_bool();
1581+
}
1582+
1583+
static constexpr std::string_view kKnownHookKeys[] = {
1584+
"build_start", "build_failed", "build_finished",
1585+
"timeout_seconds", "enabled", "side_effect",
1586+
};
1587+
for (auto& [k, _] : *ht) {
1588+
bool known = false;
1589+
for (auto kk : kKnownHookKeys) if (k == kk) { known = true; break; }
1590+
if (!known)
1591+
m.schemaWarnings.push_back(std::format(
1592+
"[hooks] has unsupported key '{}' (ignored). Keys: "
1593+
"build_start, build_failed, build_finished, "
1594+
"timeout_seconds, enabled, side_effect.", k));
1595+
}
1596+
}
1597+
15231598
// [lib] — library root convention (cargo-style).
15241599
if (auto v = doc->get_string("lib.path")) {
15251600
m.lib.path = *v;

modules/manifest/src/types.cppm

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -919,6 +919,37 @@ struct WorkspaceConfig {
919919
bool present = false;
920920
};
921921

922+
// `[hooks]` — project build lifecycle commands (#496).
923+
//
924+
// The commands are host-shell strings written by the project author, run by
925+
// `mcpp build` around the build it performs. See docs/05-mcpp-toml.md §2.16.
926+
//
927+
// ⚠️ ONLY THE ROOT PROJECT'S HOOKS ARE EVER RUN. Every manifest mcpp parses
928+
// carries this field, including a DEPENDENCY's — and `mcpp build` reaches the
929+
// invoker (mcpp.hooks) with the root project's manifest alone. A dependency
930+
// that declares hooks is inert by construction, which is the only reason
931+
// `mcpp add` of a third-party package does not become "run their shell
932+
// command on my next build". Anything that adds a second call site inherits
933+
// that responsibility.
934+
struct Hooks {
935+
std::string buildStart;
936+
std::string buildFailed;
937+
std::string buildFinished;
938+
int timeoutSeconds = 10; // per command
939+
bool enabled = true; // whole table
940+
// Whether a hook failure fails the build. False downgrades it to a
941+
// warning and preserves whatever the build itself returned.
942+
bool sideEffect = true;
943+
944+
// "This project has work for `mcpp build` to do." Distinct from `enabled`:
945+
// a table that only sets policy keys declares no command, and must leave
946+
// the build path it would otherwise divert (the fast path) untouched.
947+
bool active() const {
948+
return enabled && !(buildStart.empty() && buildFailed.empty()
949+
&& buildFinished.empty());
950+
}
951+
};
952+
922953
// [profile.<name>] — bundled build settings (opt level, debug, lto, strip).
923954
struct Profile {
924955
std::string optLevel = "2";
@@ -985,6 +1016,7 @@ struct Manifest {
9851016
Resources resources; // [resources] (mcpp#365)
9861017
RuntimeConfig runtimeConfig;
9871018
XlingsConfig xlings; // [xlings] build environment (L-1)
1019+
Hooks hooks; // [hooks] lifecycle commands (#496)
9881020
std::vector<ConditionalConfig> conditionalConfigs; // [target.'cfg(...)'.build], deferred
9891021
std::map<std::string, Profile> profiles; // [profile.<name>]
9901022
// [features] — feature name → implied features ("default" = default set).

0 commit comments

Comments
 (0)