From 8fb9a439d08401f86fbe5c801e43f253e4779e88 Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 26 Sep 2026 23:41:06 +0000 Subject: [PATCH] feat(xim): recipe packaging revision and the install_targets record (2026.9.27.1) A version entry may now state `revision = N` (libxpkg 0.0.58): a change to what a recipe installs under an unchanged upstream version. Until now the store could only answer "is 2.44.3 here", so a fixed recipe never reached a machine that had installed the broken payload: `install` of that version returned on the strength of the `installed` hook or the version database. Revision (#620) - The payload stamp records the revision it was built from (`"revision": N` in `.xpkg-install.json`, always written). - One verdict, `payload_revision_verdict` (install_state): a payload is current iff its recorded revision equals the recipe's. A stamp without the field reads as revision 0, so only a recipe that states revision >= 1 reinstalls, and it reaches every older payload. No stamp at all, or a stamp that records a failed install, gives no verdict. - The resolver plans a stale payload as not installed (its artifact is downloaded and verified first); the installer asks the same verdict again after its own fast paths and says why: `reinstalling glibc@2.44.3: recipe revision 1, installed revision 0`. - The replacement never leaves the version without a payload: the old tree is renamed to `/stale/`, the new one is installed at the real path (recipes embed install_dir()), and the old tree is deleted on success or renamed back on any failure (`PayloadReplacement`, payload.cppm). Files other tools wrote into the old tree go with it. - `install_plan` entries carry the revision as a third element and the reinstall reason as their note. install_targets (interface protocol 1.1, additive) - The top-level install reports, per request and in request order, `{request, namespace, name, version, revision, status, payload_dir}` with status `installed | already_present | failed`, on every path: fresh installs, "everything already installed" (which previously put nothing on the wire), and failures. Nested installs requested by recipes and dry runs do not report. Registered as interface-only for the terminal. - A node whose payload was present and current is recorded as present rather than installed, so `install_summary.success` counts what the run installed, as its comment already stated. - docs/spec/interface-ndjson-v1.md documents install_plan, install_summary (as emitted) and install_targets; docs/spec/xpkg-manifest-v1.md documents `revision`. Crash recovery for an interrupted reinstall - `PayloadReplacement`'s rollback is a C++ destructor, so it only runs within the same process. A kill (SIGKILL, power loss) between the rename into `/stale/` and that destructor left the parked directory there forever: nothing else ever revisited it, and every crash during a revision reinstall leaked one directory. - Fixed with a marker written next to the parked directory before the rename that creates it (`.origin`, atomic write-temp-then-rename; `write_parked_payload_marker`), recording the payload's original path and the pid doing the parking. At the start of every `install` -- once per command, where `Installer::execute` first touches the store, not once per package -- `recover_parked_payloads` sweeps `/stale/`: an entry whose pid is no longer running is moved back when the original path is missing or is the empty placeholder `set_aside_payload` leaves behind, or discarded with its marker when the original path already holds something else. An entry whose pid is still alive is left alone. Recovered and discarded entries are logged at the same level as other install diagnostics. - The in-process rollback is unchanged; a successful replacement deletes the marker along with the parked directory. - The message for a rename that fails on a file held open (Windows only -- POSIX `rename(2)` on a directory does not depend on what is open inside it) now names the specific file, not only the two directories being exchanged. - docs/spec/xpkg-manifest-v1.md gets one sentence on the recovery. Version-grammar conformance vectors - tests/data/semver-vectors.tsv: request, available versions, active version, expected result; driven through the resolver by tests/unit/test_semver_vectors.cpp. Self-describing for vendoring. Also: xlings 2026.9.27.1, mcpplibs.xpkg 0.0.58, E2E-121 (install_revision_test.sh, now with a crash-recovery case). Refs: #620 mcpp.lock is regenerated against the published libxpkg 0.0.58 with mcpp 2026.9.26.2 (the earlier edit only renamed the version and kept 0.0.57's hash). Co-Authored-By: Claude Opus 5.5 --- .../2026-09-27-release-2026.9.27.1-notes.md | 24 ++ .agents/skills/xlings-usage/SKILL.md | 4 +- AGENTS.md | 2 +- docs/design/interface-protocol.md | 2 +- docs/spec/interface-ndjson-v1.md | 93 ++++- docs/spec/xpkg-manifest-v1.md | 30 ++ examples/clients/README.md | 2 +- examples/clients/node_client.mjs | 3 +- examples/clients/python_client.py | 4 +- mcpp.lock | 30 +- mcpp.toml | 4 +- src/agent/text_renderer.cpp | 5 +- src/cli.cpp | 3 + src/core/config.cppm | 2 +- src/core/xim/commands.cpp | 161 +++++++- src/core/xim/install_state.cpp | 17 + src/core/xim/install_state.cppm | 36 ++ src/core/xim/installer.cpp | 66 +++- src/core/xim/libxpkg/types/type.cppm | 12 + src/core/xim/payload.cpp | 346 +++++++++++++++++- src/core/xim/payload.cppm | 165 ++++++++- src/core/xim/resolver.cpp | 21 +- src/interface.cppm | 6 +- tests/data/semver-vectors.tsv | 105 ++++++ tests/e2e/install_revision_test.sh | 342 +++++++++++++++++ tests/e2e/run_all.sh | 4 + tests/unit/test_install_state.cpp | 103 ++++++ tests/unit/test_interface_protocol.cpp | 40 +- tests/unit/test_payload_remove.cpp | 286 +++++++++++++++ tests/unit/test_semver_vectors.cpp | 132 +++++++ 30 files changed, 1995 insertions(+), 55 deletions(-) create mode 100644 .agents/docs/2026-09-27-release-2026.9.27.1-notes.md create mode 100644 tests/data/semver-vectors.tsv create mode 100755 tests/e2e/install_revision_test.sh create mode 100644 tests/unit/test_semver_vectors.cpp diff --git a/.agents/docs/2026-09-27-release-2026.9.27.1-notes.md b/.agents/docs/2026-09-27-release-2026.9.27.1-notes.md new file mode 100644 index 000000000..0f30df76e --- /dev/null +++ b/.agents/docs/2026-09-27-release-2026.9.27.1-notes.md @@ -0,0 +1,24 @@ +# 2026.9.27.1 发布说明 + +> 相关：xlings#620（配方打包修订号）、#621（glibc 的 locale 与 gconv）；libxpkg 0.0.58；xim-pkgindex#892。 +> 本文记录本版的变化与客户端可见的差异；发布后的验证结果追加在 §3。 + +## 1. 变化 + +| 项 | 内容 | +|---|---| +| 打包修订号 | 版本条目可写 `revision`（libxpkg 0.0.58 解析）。载荷的安装记录 `.xpkg-install.json` 写入修订号；记录的修订号低于配方时视为未安装，`install` 就地重装，并输出 `recipe revision , installed revision ` | +| 就地重装 | 旧载荷先移入 `/stale/`，新载荷装到原路径；成功后删除旧载荷，失败时移回。配方会把 `install_dir()` 写进所装文件，因此不能先装到临时目录再改名 | +| 中断恢复 | 移出旧载荷之前，在其旁写入标记 `.origin`（原路径与所属进程，原子写入）。每条 `install` 命令开始时处理 `/stale/`：所属进程已不存在的条目，原路径缺失或为空占位时移回，否则连同标记删除；进程仍存活的条目不动。进程内的析构回滚保持不变 | +| `install_targets` 事件 | 接口协议 1.1：顶层安装在所有路径上报告每个请求解析到的命名空间、名称、版本、修订号、状态与载荷目录。mcpp 2026.9.27.1 优先读取这一记录 | +| 版本选择一致性向量 | `tests/data/semver-vectors.tsv` 由单元测试驱动；mcpp 以同一份向量测试其版本选择 | +| 重命名失败的提示 | Windows 上目录中的文件被占用而无法移动时，提示点名被占用的文件 | + +## 2. 客户端可见的差异 + +- `install_summary.success` 只计本次实际安装的数量；已是当前修订的载荷报告为 `already_present`，不计入。 +- 旧版 xlings 忽略 `revision`：持有旧载荷的机器要升级到本版后才会重装。 + +## 3. 发布后验证 + +（发布后追加。） diff --git a/.agents/skills/xlings-usage/SKILL.md b/.agents/skills/xlings-usage/SKILL.md index 4ae1a2476..ed0d5a67f 100644 --- a/.agents/skills/xlings-usage/SKILL.md +++ b/.agents/skills/xlings-usage/SKILL.md @@ -142,8 +142,8 @@ xlings subos new agent-ws-2 --from subos:dev-env@latest ```bash xlings interface -# NDJSON protocol v1.0 over stdio -# → {"protocol":"1.0","capabilities":[...]} +# NDJSON protocol v1.1 over stdio +# → {"protocol":"1.1","capabilities":[...]} ``` ### One-shot command execution diff --git a/AGENTS.md b/AGENTS.md index f37129c31..09eb786a4 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -17,7 +17,7 @@ Core capabilities: src/ ├── main.cpp # Entry point ├── cli.cppm # CLI dispatch (positional + flag parsing) -├── interface.cppm # NDJSON programmatic interface (protocol v1.0) +├── interface.cppm # NDJSON programmatic interface (protocol v1.1) ├── core/ │ ├── config.cppm # 3-layer config (global → subos → project) │ ├── subos.cppm # SubOS lifecycle (create/use/fork/remove/stop) diff --git a/docs/design/interface-protocol.md b/docs/design/interface-protocol.md index f1b6533e1..b5b05d689 100644 --- a/docs/design/interface-protocol.md +++ b/docs/design/interface-protocol.md @@ -27,7 +27,7 @@ xlings interface --version 返回： ```json -{"protocol_version":"1.0"} +{"protocol_version":"1.1"} ``` ### 能力发现 diff --git a/docs/spec/interface-ndjson-v1.md b/docs/spec/interface-ndjson-v1.md index 3185998c1..8fde847a2 100644 --- a/docs/spec/interface-ndjson-v1.md +++ b/docs/spec/interface-ndjson-v1.md @@ -1,10 +1,18 @@ -> 编写日期: 2026-05-17 | 版本: 0.4.36 +> 编写日期: 2026-05-17 | 版本: 2026.9.27.1 -# NDJSON 接口协议规范 v1.0 +# NDJSON 接口协议规范 v1.1 ## 1. 概述 -`xlings interface` 提供面向程序的结构化 API，使外部客户端（IDE 插件、CI 脚本、AI agent 等）可通过标准 IO 与 xlings 交互。协议版本为 **1.0**，基于 NDJSON（Newline-Delimited JSON）。 +`xlings interface` 提供面向程序的结构化 API，使外部客户端（IDE 插件、CI 脚本、AI agent 等）可通过标准 IO 与 xlings 交互。协议版本为 **1.1**，基于 NDJSON（Newline-Delimited JSON）。 + +| 协议版本 | xlings | 变化 | +|----------|--------|------| +| 1.0 | 0.4.36 起 | 初始版本 | +| 1.1 | 2026.9.27.1 起 | 增补：`install_targets` 事件；`install_plan` 条目的第三个元素 `revision`（§6.3.1） | + +次版本号的变化只做增补，1.0 客户端无需修改即可读取 1.1 的输出。客户端应通过**探测能力** +判断服务端是否提供某项功能（例如 `install_targets` 事件是否出现），而不是比较版本号。 ## 2. 传输层 @@ -34,7 +42,7 @@ xlings interface --version 服务端输出一行后退出： ```json -{"protocol_version":"1.0"} +{"protocol_version":"1.1"} ``` ### 3.2 查询可用能力 @@ -46,7 +54,7 @@ xlings interface --list 服务端输出能力清单后退出： ```json -{"protocol_version":"1.0","capabilities":[{"name":"install_packages","description":"...","destructive":true,"inputSchema":{...},"outputSchema":{...}}, ...]} +{"protocol_version":"1.1","capabilities":[{"name":"install_packages","description":"...","destructive":true,"inputSchema":{...},"outputSchema":{...}}, ...]} ``` ### 3.3 执行能力 @@ -158,6 +166,70 @@ xlings interface install_packages --args '{"targets":["gcc@14"],"yes":true}' | dataKind | string | 数据类别标识 | | payload | object/string | 结构化数据；若原始 JSON 解析失败则为字符串 | +#### 6.3.1 安装相关的 data 事件 + +`install_packages`（以及 CLI 的 `xlings install`）按以下顺序输出三类 data 事件。 + +**`install_plan`**：待安装的节点（依赖已展开，已安装且为当前载荷的节点不列出）。全部目标均已 +安装时不输出。`plan_install`（dry-run）输出此事件后结束。 + +```json +{"kind":"data","dataKind":"install_plan","payload":{"packages":[["xim:glibc@2.44.3","reinstall: recipe revision 1, installed revision 0",1],["xim:node@22.4.0","",0]]}} +``` + +`packages` 的每一项为数组 `[坐标, 说明, 修订号]`： + +| 位置 | 类型 | 说明 | +|------|------|------| +| 0 | string | `:@` | +| 1 | string | 说明；磁盘上已有该版本但修订号不同时为重装原因，否则为空串 | +| 2 | integer | 配方对该版本声明的打包修订号（1.1 起；见 xpkg 规范 5.1.1） | + +**`install_summary`**：本次运行的计数。全部目标均已安装时不输出。 + +```json +{"kind":"data","dataKind":"install_summary","payload":{"success":1,"failed":0}} +``` + +| 字段 | 类型 | 说明 | +|------|------|------| +| success | integer | 本次运行实际安装的节点数；载荷已存在且为当前载荷的节点不计入 | +| failed | integer | 失败的节点数 | + +**`install_targets`**（1.1 起）：每个请求目标解析到的版本与载荷位置。由顶层安装调用在 +**每一条路径**上输出一次，包括全部目标均已安装、解析失败和安装失败；配方在安装过程中 +通过 `pkgmanager.install()` 发起的嵌套安装不输出此事件。`update_packages` 升级到新版本时 +内部执行的安装同样输出此事件；`plan_install`（dry-run）不输出。 + +```json +{"kind":"data","dataKind":"install_targets","payload":{"targets":[{"request":"glibc@2.44","namespace":"xim","name":"glibc","version":"2.44.3","revision":1,"status":"installed","payload_dir":"/home/u/.xlings/data/xpkgs/xim-x-glibc/2.44.3"}]}} +``` + +`targets` 与请求中的 `targets` 一一对应、顺序相同。`status` 描述本次运行是否满足了该请求： +运行在生成安装计划之前终止时（例如另一个目标无法解析），每一项均为 `failed`，已解析的项仍 +填写 `namespace`、`name`、`version`。每一项包含： + +| 字段 | 类型 | 说明 | +|------|------|------| +| request | string | 调用方请求的原始字符串 | +| namespace | string | 解析到的包命名空间；未能解析时为空串 | +| name | string | 解析到的包名；未能解析时为空串 | +| version | string | 实际解析到的版本；未能解析时为空串 | +| revision | integer | 配方对该版本声明的打包修订号 | +| status | string | `installed`（本次运行安装）/ `already_present`（载荷已存在且为当前载荷）/ `failed` | +| payload_dir | string | 载荷目录的绝对路径；`status` 为 `failed` 时为空串 | + +三种情形的示例： + +```jsonc +// 全新安装 +{"kind":"data","dataKind":"install_targets","payload":{"targets":[{"request":"xim:node@22","namespace":"xim","name":"node","version":"22.4.0","revision":0,"status":"installed","payload_dir":"/home/u/.xlings/data/xpkgs/xim-x-node/22.4.0"}]}} +// 全部已安装（此时不输出 install_plan 与 install_summary） +{"kind":"data","dataKind":"install_targets","payload":{"targets":[{"request":"xim:node@22","namespace":"xim","name":"node","version":"22.4.0","revision":0,"status":"already_present","payload_dir":"/home/u/.xlings/data/xpkgs/xim-x-node/22.4.0"}]}} +// 失败（此处为目标不存在；解析成功而安装失败时 namespace/name/version 仍会填写） +{"kind":"data","dataKind":"install_targets","payload":{"targets":[{"request":"xim:nosuchpkg","namespace":"","name":"","version":"","revision":0,"status":"failed","payload_dir":""}]}} +``` + ### 6.4 prompt 服务端向客户端请求用户输入。 @@ -207,13 +279,15 @@ xlings interface install_packages --args '{"targets":["gcc@14"],"yes":true}' 终止行，标志会话结束。每次执行恰好输出一行。 ```json -{"kind":"result","exitCode":0,"data":{"installed":["gcc@14.2.0"]}} +{"kind":"result","exitCode":0} ``` | 字段 | 类型 | 说明 | |------|------|------| | exitCode | integer | 0 成功，非 0 失败，130 表示取消 | -| data | object? | 能力返回的结构化结果（可选） | +| data | object? | 能力返回值中 `exitCode` 以外的字段（可选；返回值只有 `exitCode` 时省略） | + +安装结果不在 `result` 行中，而在 `install_targets` 事件中（§6.3.1）。 ### 6.7 heartbeat @@ -332,8 +406,9 @@ stdout 输出（每行一个 JSON）： {"kind":"progress","phase":"downloading","percent":80,"message":"node-v22.4.0-linux-x64.tar.xz"} {"kind":"log","level":"info","message":"extracting node-v22.4.0-linux-x64.tar.xz"} {"kind":"progress","phase":"installing","percent":90,"message":"linking shims"} -{"kind":"data","dataKind":"install_summary","payload":{"installed":["node@22.4.0"]}} -{"kind":"result","exitCode":0,"data":{"installed":["node@22.4.0"]}} +{"kind":"data","dataKind":"install_summary","payload":{"success":1,"failed":0}} +{"kind":"data","dataKind":"install_targets","payload":{"targets":[{"request":"node@22","namespace":"xim","name":"node","version":"22.4.0","revision":0,"status":"installed","payload_dir":"/home/u/.xlings/data/xpkgs/xim-x-node/22.4.0"}]}} +{"kind":"result","exitCode":0} ``` 客户端收到 `kind: result` 后即可关闭 stdin 并退出。 diff --git a/docs/spec/xpkg-manifest-v1.md b/docs/spec/xpkg-manifest-v1.md index 06a591e6a..0a14f2a90 100644 --- a/docs/spec/xpkg-manifest-v1.md +++ b/docs/spec/xpkg-manifest-v1.md @@ -59,6 +59,7 @@ xpkg 包描述文件是一个 Lua 脚本（`.lua`），用于声明软件包的 | `url` | string / table | 下载地址；table 时为镜像表 `{GLOBAL=..., CN=...}` | | `sha256` | string / nil | 校验和；nil 表示暂未计算 | | `ref` | string | 引用另一版本（别名），如 `latest` → 实际版本 | +| `revision` | integer | 打包修订号，非负整数，缺省为 0；见 5.1.1 | 示例： @@ -70,6 +71,35 @@ xpkg 包描述文件是一个 Lua 脚本（`.lua`），用于声明软件包的 }, ``` +#### 5.1.1 打包修订号 `revision`（xlings 2026.9.27.1 / libxpkg 0.0.58） + +上游版本不变而安装内容发生变化时（重新构建的归档、修正后的安装钩子），版本条目以 +`revision` 递增表达这一变化。未声明或取值不是非负整数时按 0 读取；`ref` 别名不携带 +修订号，其值取自别名最终指向的条目。 + +xlings 安装载荷时把修订号写入载荷目录的 `.xpkg-install.json`（`"revision": N`）。 +判定规则只有一条：载荷记录的修订号等于配方对该版本声明的修订号时，载荷为当前载荷； +否则视为未安装，下一次 `xlings install` 重新安装该版本并输出原因，例如 +`reinstalling glibc@2.44.3: recipe revision 1, installed revision 0`。 + +- 不含 `revision` 字段的记录（2026.9.27.1 之前的客户端写入）按修订号 0 读取，因此 + 配方声明 `revision = 1` 即覆盖所有旧载荷，未声明修订号的配方不触发重装。 +- 载荷目录中没有 `.xpkg-install.json` 时不作判定，行为与此前相同。 +- 重装过程中载荷始终存在：先下载并校验归档，再把旧载荷移至 `/stale/`，在原路径 + 安装新载荷（配方会把 `install_dir()` 写入所安装的文件，因此不能在临时目录安装后改名）； + 成功后删除旧载荷，失败时删除不完整的新载荷并移回旧载荷。其他工具写入旧载荷目录的文件 + （例如 mcpp 的 `.mcpp_ok`、`.mcpp-fixup.json`）随旧载荷一并删除。 +- 移出旧载荷之前，在 `/stale/` 中写入记录原路径与所属进程的标记。进程被强制终止时， + 后续的 `install` 在该进程已不存在后处理遗留的旧载荷：原路径缺失或为空时移回，否则删除。 + +```lua +["2.44.3"] = { + url = "https://example.com/glibc-2.44.3-linux-x86_64.tar.gz", + sha256 = "abcdef...", + revision = 1, +}, +``` + ### 5.2 平台级字段 除版本条目外，平台 table 还可包含以下键（非版本条目，加载器会跳过）： diff --git a/examples/clients/README.md b/examples/clients/README.md index 308bfefe4..0a2f79dc4 100644 --- a/examples/clients/README.md +++ b/examples/clients/README.md @@ -10,7 +10,7 @@ Three minimal clients exercising the v1 NDJSON protocol from different runtimes: Each client does the same flow: -1. Probe `xlings interface --version` → assert `protocol_version == "1.0"` +1. Probe `xlings interface --version` → assert the `protocol_version` major is `1` (a minor bump is additive) 2. Probe `--list` → print capability count 3. Call `env` capability → print active sub-OS + paths 4. Call `list_subos` capability → print sub-OSs diff --git a/examples/clients/node_client.mjs b/examples/clients/node_client.mjs index cf735cc95..3f7b54cb4 100644 --- a/examples/clients/node_client.mjs +++ b/examples/clients/node_client.mjs @@ -53,7 +53,8 @@ console.log(`→ using ${xlings}`); const { events } = callXlings(xlings, "interface", "--version"); const v = events[0]?.protocol_version; console.log(`protocol_version: ${v}`); - if (v !== "1.0") { + // Major version only: a minor bump is additive. + if (typeof v !== "string" || v.split(".")[0] !== "1") { console.error(`ERROR: unexpected version ${v}`); process.exit(1); } diff --git a/examples/clients/python_client.py b/examples/clients/python_client.py index 3f9fbf954..f00c660dd 100755 --- a/examples/clients/python_client.py +++ b/examples/clients/python_client.py @@ -57,7 +57,9 @@ def main() -> int: # 1. protocol_version (one-line JSON, not NDJSON) [version_obj] = list(call(xlings, "interface", "--version")) print(f"protocol_version: {version_obj['protocol_version']}") - assert version_obj["protocol_version"] == "1.0" + # Major version only: a minor bump is additive, and a capability is + # detected by its presence on the wire, not by this number. + assert version_obj["protocol_version"].split(".")[0] == "1" # 2. capability list (one big JSON object) [list_obj] = list(call(xlings, "interface", "--list")) diff --git a/mcpp.lock b/mcpp.lock index 1fce5eea1..412ee03e4 100644 --- a/mcpp.lock +++ b/mcpp.lock @@ -9,77 +9,77 @@ version = 2 namespace = "compat" version = "1.0.8" source = "index+compat@1.0.8" -hash = "fnv1a:3672c2797c5f2faf" +hash = "fnv1a:e0e808c73d8f562d" [package."compat.ftxui"] namespace = "compat" version = "6.1.9" source = "index+compat@6.1.9" -hash = "fnv1a:f4f0f58dc9107814" +hash = "fnv1a:2798f2ba9c6400b1" [package."compat.libarchive"] namespace = "compat" version = "3.8.7" source = "index+compat@3.8.7" -hash = "fnv1a:e412742c0533bef7" +hash = "fnv1a:c4f73335678a009e" [package."compat.lua"] namespace = "compat" version = "5.4.7" source = "index+compat@5.4.7" -hash = "fnv1a:6bda787e2c850eac" +hash = "fnv1a:62ff6fb178577847" [package."compat.lz4"] namespace = "compat" version = "1.10.0" source = "index+compat@1.10.0" -hash = "fnv1a:77a204c2fbefcdc5" +hash = "fnv1a:c1f0fef08251ee13" [package."compat.mbedtls"] namespace = "compat" version = "3.6.1" source = "index+compat@3.6.1" -hash = "fnv1a:83dea67ac1379be3" +hash = "fnv1a:bb700e69e973ccd0" [package."compat.xz"] namespace = "compat" version = "5.8.3" source = "index+compat@5.8.3" -hash = "fnv1a:3c6c899487663f43" +hash = "fnv1a:8556dee38703a791" [package."compat.zlib"] namespace = "compat" version = "1.3.2" source = "index+compat@1.3.2" -hash = "fnv1a:bb0a6ebd39272a80" +hash = "fnv1a:9f13dbeaae3ef4c0" [package."compat.zstd"] namespace = "compat" version = "1.5.7" source = "index+compat@1.5.7" -hash = "fnv1a:bfb039f8b4a4b3f3" +hash = "fnv1a:cc855914d41f9bfb" [package."mcpplibs.cmdline"] namespace = "mcpplibs" version = "0.0.2" source = "index+mcpplibs@0.0.2" -hash = "fnv1a:8ee8a8e51ac69885" +hash = "fnv1a:aad61299abc2c4b7" [package."tinyhttps"] namespace = "mcpplibs" version = "0.2.9" source = "index+mcpplibs@0.2.9" -hash = "fnv1a:7dcd2b0fcf679705" +hash = "fnv1a:a01132893163f775" [package."mcpplibs.xpkg"] namespace = "mcpplibs" -version = "0.0.57" -source = "index+mcpplibs@0.0.57" -hash = "fnv1a:91399fb9d7d59ebc" +version = "0.0.58" +source = "index+mcpplibs@0.0.58" +hash = "fnv1a:86a48e702be90204" [package."mcpplibs.capi.lua"] namespace = "mcpplibs.capi" version = "0.0.3" source = "index+mcpplibs.capi@0.0.3" -hash = "fnv1a:c8e7ef9d60a1a3a6" +hash = "fnv1a:f3031630d0b22d94" diff --git a/mcpp.toml b/mcpp.toml index dfb884188..49fe0a76e 100644 --- a/mcpp.toml +++ b/mcpp.toml @@ -29,7 +29,7 @@ members = [ [package] name = "xlings" -version = "2026.9.26.3" +version = "2026.9.27.1" description = "Universal package management infrastructure tool with SubOS isolation" license = "Apache-2.0" repo = "https://github.com/openxlings/xlings" @@ -126,7 +126,7 @@ libarchive = "3.8.7" [dependencies.mcpplibs] cmdline = "0.0.2" -xpkg = "0.0.57" +xpkg = "0.0.58" capi.lua = "0.0.3" [dev-dependencies.compat] diff --git a/src/agent/text_renderer.cpp b/src/agent/text_renderer.cpp index 628311309..791bd588a 100644 --- a/src/agent/text_renderer.cpp +++ b/src/agent/text_renderer.cpp @@ -208,8 +208,9 @@ void render_data_event(const DataEvent& e) { } } } - // download_progress, system_info, env, repo_list, subos_shims: - // these are either noisy (progress) or already covered by info_panel. + // download_progress, system_info, env, repo_list, subos_shims, + // install_targets: these are either noisy (progress), already covered by + // info_panel, or a structured duplicate of the plan and the summary. // Skip silently — the log messages carry the essential info. } diff --git a/src/cli.cpp b/src/cli.cpp index 3b815cc6a..5b707c956 100644 --- a/src/cli.cpp +++ b/src/cli.cpp @@ -65,6 +65,9 @@ bool kind_is_interface_only_(std::string_view kind) { }; static constexpr std::string_view kWireDuplicate[] = { "remove_blocked", "update_plan", "update_summary", + // What each request of an install resolved to (protocol 1.1); the + // terminal already shows the plan, the progress and the summary. + "install_targets", }; return std::ranges::contains(kCapabilityOnly, kind) || std::ranges::contains(kWireDuplicate, kind); diff --git a/src/core/config.cppm b/src/core/config.cppm index f7b1e0a75..c8b459b7a 100644 --- a/src/core/config.cppm +++ b/src/core/config.cppm @@ -10,7 +10,7 @@ import xlings.core.xvm.db; namespace xlings { export struct Info { - static constexpr std::string_view VERSION = "2026.9.26.3"; + static constexpr std::string_view VERSION = "2026.9.27.1"; static constexpr std::string_view REPO = "https://github.com/openxlings/xlings"; }; diff --git a/src/core/xim/commands.cpp b/src/core/xim/commands.cpp index 60d5b245d..97083d513 100644 --- a/src/core/xim/commands.cpp +++ b/src/core/xim/commands.cpp @@ -288,7 +288,90 @@ int cmd_why(const std::string& target, const std::string& dep, return 0; } +// ── install_targets: what each request of one install resolved to ────── +// +// A client that asks for `libglvnd@1.7` has, without this, no way to learn +// which version that selected or where its payload is, except by running the +// version grammar a second time on its own side -- a second answerer, which +// disagrees with this one the first time either grammar changes. And when +// everything was already installed, nothing at all reached the wire. +// +// So the top-level invocation reports one entry per request, in request +// order, on every path: success, "everything was already installed", and +// every failure, including those before resolution (the entry then carries +// the request and nothing else). Nested installs -- the ones a recipe asks +// for through pkgmanager.install() -- write into the same event stream and +// do not report: their requests are not the caller's. A dry run does not +// report either: it installs nothing, and a status would be a prediction. +// +// Protocol 1.1 (additive). Clients detect it by the event's presence. +struct InstallTargetReport_ { + std::string request; + std::string namespaceName; + std::string name; + std::string version; + int revision { 0 }; + std::string status { "failed" }; // installed | already_present | failed + std::filesystem::path payloadDir; // published unless failed +}; + +std::string install_targets_payload_(std::span targets) { + nlohmann::json list = nlohmann::json::array(); + for (const auto& t : targets) { + std::string payloadDir; + if (t.status != "failed" && !t.payloadDir.empty()) { + std::error_code ec; + const auto absolute = std::filesystem::absolute(t.payloadDir, ec); + payloadDir = (ec ? t.payloadDir : absolute).lexically_normal().string(); + } + list.push_back({ + {"request", t.request}, + {"namespace", t.namespaceName}, + {"name", t.name}, + {"version", t.version}, + {"revision", t.revision}, + {"status", t.status}, + {"payload_dir", payloadDir}, + }); + } + nlohmann::json payload; + payload["targets"] = std::move(list); + return payload.dump(); +} + +int install_(std::span targets, bool yes, bool noDeps, + EventStream& stream, bool forceGlobal, CancellationToken* cancel, + bool dryRun, bool useAfterInstall, + std::vector* report); + int cmd_install(std::span targets, bool yes, bool noDeps, EventStream& stream, bool forceGlobal, CancellationToken* cancel, bool dryRun, bool useAfterInstall) { + std::vector report; + report.reserve(targets.size()); + for (const auto& target : targets) { + report.push_back(InstallTargetReport_{ .request = target }); + } + const auto emit_report = [&] { + if (dryRun) return; + stream.emit(DataEvent{"install_targets", install_targets_payload_(report)}); + }; + int rc = 0; + try { + rc = install_(targets, yes, noDeps, stream, forceGlobal, cancel, + dryRun, useAfterInstall, &report); + } catch (...) { + // A cancellation arrives as an exception; the report still goes out, + // with whatever this run had established before it. + emit_report(); + throw; + } + emit_report(); + return rc; +} + +int install_(std::span targets, bool yes, bool noDeps, + EventStream& stream, bool forceGlobal, CancellationToken* cancel, + bool dryRun, bool useAfterInstall, + std::vector* report) { // An emulated build installs emulated packages -- correctly, since they // have to match this process's ABI, and slowly, since every one of them // then runs under Rosetta / WOW64 / qemu. The user is the only one who can @@ -627,6 +710,14 @@ int cmd_install(std::span targets, bool yes, bool noDeps, Eve if (!match->version.empty()) { target += "@" + match->version; } + // One match per request, in request order, so the count so far is + // this request's index. + if (report && requestedMatches.size() < report->size()) { + auto& entry = (*report)[requestedMatches.size()]; + entry.namespaceName = match->namespaceName; + entry.name = match->name; + entry.version = match->version; + } requestedMatches.push_back(*match); } @@ -674,11 +765,23 @@ int cmd_install(std::span targets, bool yes, bool noDeps, Eve auto plan_key = [](const PackageMatch& match) { return match.canonicalName + "@" + match.version; }; + const auto plan_node_of = [&](const PackageMatch& match) -> const PlanNode* { + const auto key = plan_key(match); + for (const auto& node : plan.nodes) { + if (detail_::plan_key_(node) == key) return &node; + } + return nullptr; + }; + // The planner's verdict where it has one: it also knows a payload whose + // packaging revision is stale (#620), which the match -- "a payload + // directory exists" -- cannot. std::unordered_map requestedAlreadyInstalled; for (auto& match : requestedMatches) { - requestedAlreadyInstalled[plan_key(match)] = - match.installed && !match.payloadForeign; + const auto* node = plan_node_of(match); + requestedAlreadyInstalled[plan_key(match)] = node + ? node->alreadyInstalled + : match.installed && !match.payloadForeign; } // ── What this run actually did, per node. One record, many readers. ── @@ -724,6 +827,31 @@ int cmd_install(std::span targets, bool yes, bool noDeps, Eve }; const auto failed_count = [&] { return count_status(NodeStatus::Failed); }; + // The install_targets entries, read off the same record. A request with no + // outcome was not satisfied by this run and reports `failed`. + const auto record_report = [&] { + if (!report) return; + for (std::size_t i = 0; + i < requestedMatches.size() && i < report->size(); ++i) { + const auto& match = requestedMatches[i]; + auto& entry = (*report)[i]; + if (const auto* node = plan_node_of(match)) { + entry.revision = node->revision; + const auto storeRoot = node->storeRoot.empty() + ? Config::paths().dataDir / "xpkgs" : node->storeRoot; + entry.payloadDir = storeRoot + / package_store_name(node->namespaceName, node->name) + / node->version; + } + const auto it = outcomes.find(plan_key(match)); + entry.status = it == outcomes.end() ? "failed" + : it->second.status == NodeStatus::Installed ? "installed" + : it->second.status == NodeStatus::AlreadyPresent ? "already_present" + : "failed"; + } + }; + record_report(); + auto activate_requested_targets = [&]() { auto db = Config::versions(); for (auto& match : requestedMatches) { @@ -772,13 +900,22 @@ int cmd_install(std::span targets, bool yes, bool noDeps, Eve } // Show install plan with themed UI + // + // Each entry is `[coordinate, note, revision]`. The note says why a + // payload that is on disk is installed again (#620); the third element is + // the recipe's packaging revision, appended so that readers of the first + // two are unaffected. if (!allAlreadyInstalled) { nlohmann::json planPackages = nlohmann::json::array(); for (auto& node : plan.nodes) { if (!node.alreadyInstalled) { std::string nameVer = node.canonicalName; if (!node.version.empty()) nameVer += "@" + node.version; - planPackages.push_back({nameVer, ""}); + std::string note; + if (!node.staleReason.empty()) { + note = "reinstall: " + node.staleReason; + } + planPackages.push_back({nameVer, note, node.revision}); } } nlohmann::json planPayload; @@ -847,9 +984,14 @@ int cmd_install(std::span targets, bool yes, bool noDeps, Eve break; case InstallPhase::Done: log::debug("[{}] done", status.name); + // A node whose payload was present and current also ends + // in Done (its config hook runs again); it is recorded as + // present, not as installed by this run. if (!status.planKey.empty()) { - outcomes[status.planKey] = - { NodeStatus::Installed, {}, {}, {} }; + outcomes[status.planKey] = { + status.payloadReused ? NodeStatus::AlreadyPresent + : NodeStatus::Installed, + {}, {}, {} }; } break; case InstallPhase::Failed: @@ -887,9 +1029,11 @@ int cmd_install(std::span targets, bool yes, bool noDeps, Eve if (req.op == "install") { log::debug("installing sub-dependency: {}", req.target); std::vector subTargets = { req.target }; - cmd_install(subTargets, /*yes=*/true, /*noDeps=*/false, stream, - forceGlobal, /*cancel=*/nullptr, /*dryRun=*/false, - useAfterInstall); + // Nested: no install_targets report of its own (see + // cmd_install). + install_(subTargets, /*yes=*/true, /*noDeps=*/false, stream, + forceGlobal, /*cancel=*/nullptr, /*dryRun=*/false, + useAfterInstall, /*report=*/nullptr); } else if (req.op == "remove") { log::debug("removing sub-dependency: {}", req.target); // force: an xpkg hook asking for a removal has already @@ -901,6 +1045,7 @@ int cmd_install(std::span targets, bool yes, bool noDeps, Eve } }, dlRenderer, cancel, useAfterInstall); + record_report(); if (!result) { // #374: structured error on the wire (was a swallowed log::error) diff --git a/src/core/xim/install_state.cpp b/src/core/xim/install_state.cpp index 443dd78d4..5cc9428c3 100644 --- a/src/core/xim/install_state.cpp +++ b/src/core/xim/install_state.cpp @@ -91,6 +91,18 @@ bool unverifiable_stamped_payload(const LedgerIndex& ledger, payloadDir / std::filesystem::path(kPayloadStampFile)); } +RevisionVerdict payload_revision_verdict(const std::filesystem::path& payloadDir, + int recipeRevision) { + RevisionVerdict verdict; + verdict.recipe = std::max(recipeRevision, 0); + if (!payload_has_content(payloadDir)) return verdict; + if (stamped_incomplete(payloadDir)) return verdict; + verdict.recorded = stamped_revision(payloadDir); + verdict.stale = verdict.recorded != kRevisionUnrecorded + && verdict.recorded != verdict.recipe; + return verdict; +} + } @@ -110,6 +122,11 @@ namespace xlings::xim { return state != InstallState::Installed; } +[[nodiscard]] std::string RevisionVerdict::reason() const { + return std::format("recipe revision {}, installed revision {}", + recipe, recorded); +} + LedgerIndex::LedgerIndex(const xvm::VersionDB& db, const std::string& xlingsHome) { for (const auto& [target, info] : db) { for (const auto& [versionKey, data] : info.versions) { diff --git a/src/core/xim/install_state.cppm b/src/core/xim/install_state.cppm index 169946a0e..4325e5d29 100644 --- a/src/core/xim/install_state.cppm +++ b/src/core/xim/install_state.cppm @@ -135,4 +135,40 @@ bool unverifiable_stamped_payload(const LedgerIndex& ledger, std::string_view version, const std::filesystem::path& payloadDir); +// Whether a payload is the recipe's CURRENT packaging of its version +// (openxlings/xlings#620). +// +// A recipe states `revision = N` on a version entry when what it installs +// changes and the upstream version does not (a rebuilt tarball, a fixed +// install hook). Without it the store can only answer "is 2.44.3 here", and a +// machine that installed 2.44.3 before the fix keeps that payload for good, +// because every probe says it is installed. +// +// The rule, decided here only -- the resolver plans with it, and the +// installer asks it again after its own fast paths (the `installed` hook, the +// version database), each of which would otherwise answer "installed": +// +// a payload is current iff the revision its stamp records equals the +// recipe's revision for that version. +// +// `stamped_revision` reads a stamp without the field as 0, so a recipe that +// states revision 1 reaches every payload installed before revisions existed, +// and a recipe that states none reinstalls nothing. Two shapes give no verdict +// and are never stale: +// +// * no stamp at all -- nothing was observed about that install; +// * a stamp that records a failed install -- that payload is Incomplete +// (`installation_state`), and the incomplete path reinstalls it already. +struct RevisionVerdict { + bool stale { false }; + int recorded { kRevisionUnrecorded }; // what the stamp records + int recipe { 0 }; // what the recipe states + + // "recipe revision 1, installed revision 0" + [[nodiscard]] std::string reason() const; +}; + +RevisionVerdict payload_revision_verdict(const std::filesystem::path& payloadDir, + int recipeRevision); + } // namespace xlings::xim diff --git a/src/core/xim/installer.cpp b/src/core/xim/installer.cpp index 3af471b68..537ac8f94 100644 --- a/src/core/xim/installer.cpp +++ b/src/core/xim/installer.cpp @@ -2550,6 +2550,16 @@ std::expected Installer::execute(const InstallPlan& plan, con } auto dataDir = Config::paths().dataDir; + + // Finish what an earlier, interrupted reinstall (#620) left parked + // under /stale/: PayloadReplacement's rollback is a C++ + // destructor, which never runs when the process is killed (SIGKILL, + // power loss) instead of unwound, so nothing else ever revisits what it + // parked there. Once per execute() call -- this is the store's first + // touch for the install, not the per-node loop below -- and never on a + // dry run, which returns before execute() is ever called. + recover_parked_payloads(dataDir / "stale"); + auto platform = detect_platform_(); // Phase 1: Collect download tasks for non-installed packages @@ -3094,6 +3104,40 @@ std::expected Installer::execute(const InstallPlan& plan, con } } + // A payload of another packaging revision (#620). Asked again here, + // not only by the planner, because the fast paths above -- the + // `installed` hook and the version database -- each answer + // "installed" about it, which is how `install` of an already + // installed version returned without looking at the recipe. + // + // The old tree is moved aside only now: the artifact was downloaded + // and verified in phase 1 and extracted above, so every step that + // can fail without touching the store has already run. From here to + // `commit` below, any early exit of this iteration destroys + // `replacement`, which removes the partial tree and puts the old one + // back (PayloadReplacement, payload.cppm). + std::optional replacement; + if (const auto revision = foreignPayload + ? RevisionVerdict{} + : payload_revision_verdict(ctx.install_dir, node.revision); + revision.stale) { + log::info("reinstalling {}@{}: {}", + node.name, node.version, revision.reason()); + auto aside = set_aside_payload(ctx.install_dir); + if (!aside) { + const auto message = std::format( + "{}; the installed payload (revision {}) is unchanged", + aside.error(), revision.recorded); + log::error("[{}@{}] {}", node.name, node.version, message); + if (onStatus) { + onStatus({ node.name, InstallPhase::Failed, 0.0f, message }); + } + continue; + } + replacement.emplace(std::move(*aside)); + payloadInstalled = false; + } + // Run install hook if (!payloadInstalled && executor.has_hook(mcpplibs::xpkg::HookType::Install)) { log::debug("installing {}...", node.name); @@ -3306,7 +3350,8 @@ std::expected Installer::execute(const InstallPlan& plan, con if (!payloadInstalled && node.pkgType != 3 /* Config */ && classify_payload_content(ctx.install_dir) != PayloadPlatform::Foreign) { - write_payload_stamp(ctx.install_dir, node.version); + write_payload_stamp(ctx.install_dir, node.version, + kRegisteredUnrecorded, node.revision); } // Apply elfpatch auto-patching if the install hook enabled it @@ -3518,11 +3563,17 @@ std::expected Installer::execute(const InstallPlan& plan, con // Without it, "the run reached the end" and "the run did its work" // are the same output, which is how a stack that wired nothing // reported `installed` on a real home. + // + // The recipe's revision is recorded on this path too. A payload this + // run did not install reached here either current by the revision + // check above, which makes the two numbers equal, or with no stamp + // and so no verdict, which the recipe's revision now adopts. if (node.pkgType != 3 /* Config */) { const auto registered = count_ledger_registrations( Config::versions(), Config::paths().homeDir.string(), node.namespaceName, node.name, node.version); - write_payload_stamp(ctx.install_dir, node.version, registered); + write_payload_stamp(ctx.install_dir, node.version, registered, + node.revision); } if (catalog_) { @@ -3542,9 +3593,16 @@ std::expected Installer::execute(const InstallPlan& plan, con index_->mark_installed(node.name, true); } + // Every step that can fail has run: the stale payload, if one was set + // aside, is no longer needed. + if (replacement) replacement->commit(); + if (onStatus) { - onStatus({ node.name, InstallPhase::Done, 1.0f, - payloadInstalled ? "already installed" : "" }); + onStatus({ .name = node.name, + .phase = InstallPhase::Done, + .progress = 1.0f, + .message = payloadInstalled ? "already installed" : "", + .payloadReused = payloadInstalled }); } if (payloadInstalled) { log::debug("{}@{} mapping to current subos", node.name, node.version); diff --git a/src/core/xim/libxpkg/types/type.cppm b/src/core/xim/libxpkg/types/type.cppm index d0354358a..ab5fd913f 100644 --- a/src/core/xim/libxpkg/types/type.cppm +++ b/src/core/xim/libxpkg/types/type.cppm @@ -51,6 +51,12 @@ struct InstallStatus { // "free some space" (openxlings/xlings#376). std::string errorCode; std::string hint; + + // Done only: the payload was already present and current, so this run + // installed nothing into it (its config hook and registrations may still + // have run). What separates `already_present` from `installed` in the + // install_targets record, without reading the free-text `message`. + bool payloadReused { false }; }; // How a node was reached in the dep graph. Determines whether the @@ -118,6 +124,12 @@ struct PlanNode { bool isSystemPM { false }; PackageScope scope { PackageScope::Global }; int pkgType { 0 }; // 0=Package, 1=Script, 2=Template, 3=Config, 4=Subos + // The recipe's packaging revision for this version (`revision` on the + // version entry, 0 when it states none). Recorded in the payload stamp. + int revision { 0 }; + // Non-empty when a payload of this version is on disk but stale: the + // resolver's `payload_revision_verdict`, phrased for the plan. + std::string staleReason; // Explicit special members to work around GCC 15 module linker bug PlanNode() = default; diff --git a/src/core/xim/payload.cpp b/src/core/xim/payload.cpp index 55a5127e7..274bb75ad 100644 --- a/src/core/xim/payload.cpp +++ b/src/core/xim/payload.cpp @@ -3,6 +3,7 @@ module xlings.core.xim.payload; import std; import xlings.platform; import xlings.core.config; +import xlings.core.log; namespace xlings::xim { @@ -224,7 +225,30 @@ int stamped_registration_count(const std::filesystem::path& dir) { return any ? value : kRegisteredUnrecorded; } -void write_payload_stamp(const std::filesystem::path& dir, std::string_view version, int registered) { +int stamped_revision(const std::filesystem::path& dir) { + namespace fs = std::filesystem; + std::error_code ec; + const auto stamp = dir / std::filesystem::path(kPayloadStampFile); + if (!fs::is_regular_file(stamp, ec)) return kRevisionUnrecorded; + // Scanned by hand, for the reason given at classify_payload_platform. + const auto content = platform::read_file_to_string(stamp.string()); + const auto key = content.find("\"revision\""); + if (key == std::string::npos) return 0; + const auto colon = content.find(':', key); + if (colon == std::string::npos) return 0; + std::size_t i = colon + 1; + while (i < content.size() && (content[i] == ' ' || content[i] == '\t')) ++i; + long long value = 0; + while (i < content.size() && content[i] >= '0' && content[i] <= '9') { + value = value * 10 + (content[i] - '0'); + if (value > std::numeric_limits::max()) return 0; + ++i; + } + return static_cast(value); +} + +void write_payload_stamp(const std::filesystem::path& dir, std::string_view version, + int registered, int revision) { namespace fs = std::filesystem; std::error_code ec; if (!fs::is_directory(dir, ec)) return; @@ -241,6 +265,7 @@ void write_payload_stamp(const std::filesystem::path& dir, std::string_view vers if (registered != kRegisteredUnrecorded) { text += std::format(",\n \"registered\": {}", registered); } + text += std::format(",\n \"revision\": {}", std::max(revision, 0)); text += "\n}\n"; platform::write_string_to_file( (dir / std::filesystem::path(kPayloadStampFile)).string(), text); @@ -323,6 +348,31 @@ void clear_readonly_(const fs::path& root) { } } +// Best-effort: name the one file that a whole-tree rename refuses to say +// anything concrete about. `std::error_code::message()` from a failed +// rename of a DIRECTORY is whatever the OS hands back for the directory +// entry operation itself -- on Windows, "Access is denied" and nothing +// else -- and does not say which file inside made it so. `cl.exe` leaves +// `vctip.exe`/`mspdbsrv.exe` running inside the toolset that launched them +// for tens of seconds after exit (see remove_payload_dir above), which is +// the shape this exists for: a file opened elsewhere without +// FILE_SHARE_DELETE fails to move even though renaming a directory does not +// touch its files' contents at all. +// +// Unreachable by construction on POSIX: rename(2) on a directory changes +// one directory entry and does not care whether anything underneath is +// open, so a whole-tree rename there never fails for this reason -- only +// for a directory permission bit, which this function correctly reports +// nothing about (there is no one file to blame). +fs::path locate_locked_file_(const fs::path& root) { + for (const auto& file : regular_files_(root)) { + std::fstream probe(file, std::ios::in | std::ios::out | std::ios::binary); + if (probe.is_open()) continue; + return file; + } + return {}; +} + } // namespace fs::path payload_trash_root(const fs::path& payloadDir) { @@ -430,4 +480,298 @@ RemoveOutcome settle_removal(const fs::path& root, std::string_view version) { "uninstall could not remove every file -- something is holding one"); return RemoveOutcome::Partial; } + +// ── replacing a payload that is installed but stale ────────────────── + +fs::path stale_payload_parking(const fs::path& payloadDir) { + const auto pid = std::to_string(platform::get_pid()); + fs::path base; + for (auto dir = payloadDir; dir.has_relative_path(); dir = dir.parent_path()) { + if (dir.filename() == "xpkgs") { + base = dir.parent_path() / "stale" / + (payloadDir.parent_path().filename().string() + "-" + + payloadDir.filename().string() + ".stale-" + pid); + break; + } + if (dir.parent_path() == dir) break; + } + if (base.empty()) { + base = payloadDir.parent_path() / + (payloadDir.filename().string() + ".stale-" + pid); + } + std::error_code ignore; + auto candidate = base; + for (int n = 1; fs::exists(candidate, ignore) && n < 1000; ++n) { + candidate = base; + candidate += "-" + std::to_string(n); + } + return candidate; +} + +// ── recovering a parked payload after a crash ───────────────────────── +// +// The marker is hand-written and hand-parsed for the same reason +// `classify_payload_platform` above is (see there): instantiating the JSON +// parser in this module trips a GCC 16 modules failure that names an +// unrelated module. A filesystem path is the one field here that cannot +// simply be scanned for without escaping -- a Windows path is full of the +// backslashes JSON uses for its own escapes -- so it gets a real (if +// minimal) escape/unescape pair instead of the sanitise-and-forget approach +// `write_payload_failure_marker` uses for its free-text reason. +namespace { + +std::string json_escape_path_(std::string_view s) { + std::string out; + out.reserve(s.size()); + for (const char c : s) { + if (c == '\\' || c == '"') out.push_back('\\'); + out.push_back(c); + } + return out; +} + +std::string json_unescape_path_(std::string_view s) { + std::string out; + out.reserve(s.size()); + for (std::size_t i = 0; i < s.size(); ++i) { + if (s[i] == '\\' && i + 1 < s.size()) { ++i; } + out.push_back(s[i]); + } + return out; +} + +} // namespace + +fs::path parked_marker_path(const fs::path& parkedDir) { + return fs::path(parkedDir.string() + ".origin"); +} + +void write_parked_payload_marker(const fs::path& parkedDir, + const fs::path& payloadDir, int pid) { + const auto text = std::format( + "{{\n \"payload_dir\": \"{}\",\n \"pid\": {}\n}}\n", + json_escape_path_(payloadDir.string()), pid); + // Atomic: write_file_atomic writes a staging file and renames over the + // destination, so a crash mid-write leaves either no marker (the usual + // case, nothing was here before) or the complete one -- never a torn + // file `read_parked_payload_marker` could misparse as valid. + platform::write_file_atomic(parked_marker_path(parkedDir).string(), text); +} + +std::optional read_parked_payload_marker( + const fs::path& parkedDir) { + std::error_code ec; + const auto markerPath = parked_marker_path(parkedDir); + if (!fs::is_regular_file(markerPath, ec)) return std::nullopt; + const auto content = platform::read_file_to_string(markerPath.string()); + + const auto pdKey = content.find("\"payload_dir\""); + if (pdKey == std::string::npos) return std::nullopt; + auto colon = content.find(':', pdKey); + auto open = colon == std::string::npos + ? std::string::npos : content.find('"', colon); + if (open == std::string::npos) return std::nullopt; + std::size_t close = open + 1; + while (close < content.size() && content[close] != '"') { + close += (content[close] == '\\' && close + 1 < content.size()) ? 2 : 1; + } + if (close >= content.size()) return std::nullopt; + const auto rawPath = content.substr(open + 1, close - open - 1); + + const auto pidKey = content.find("\"pid\""); + if (pidKey == std::string::npos) return std::nullopt; + colon = content.find(':', pidKey); + if (colon == std::string::npos) return std::nullopt; + std::size_t i = colon + 1; + while (i < content.size() && (content[i] == ' ' || content[i] == '\t')) ++i; + long long value = 0; + bool any = false; + while (i < content.size() && content[i] >= '0' && content[i] <= '9') { + value = value * 10 + (content[i] - '0'); + ++i; + any = true; + if (value > std::numeric_limits::max()) return std::nullopt; + } + if (!any) return std::nullopt; + + ParkedPayloadMarker marker; + marker.payloadDir = fs::path(json_unescape_path_(rawPath)); + marker.pid = static_cast(value); + return marker; +} + +void remove_parked_payload_marker(const fs::path& parkedDir) { + std::error_code ec; + fs::remove(parked_marker_path(parkedDir), ec); +} + +int recover_parked_payloads(const fs::path& staleRoot) { + std::error_code ec; + if (staleRoot.empty() || !fs::is_directory(staleRoot, ec)) return 0; + + int resolved = 0; + for (auto it = fs::directory_iterator(staleRoot, ec); + !ec && it != std::default_sentinel; it.increment(ec)) { + std::error_code dec; + if (!it->is_directory(dec) || dec) continue; // markers are files + const auto parked = it->path(); + + auto marker = read_parked_payload_marker(parked); + if (!marker) continue; // nothing says this one is ours to touch + if (platform::is_process_alive(marker->pid)) continue; // still running + + std::error_code oec; + const bool originalMissing = !fs::exists(marker->payloadDir, oec); + const bool originalEmpty = !originalMissing + && fs::is_directory(marker->payloadDir, oec) + && fs::is_empty(marker->payloadDir, oec); + + if (originalMissing || originalEmpty) { + std::error_code rec; + fs::create_directories(marker->payloadDir.parent_path(), rec); + rec.clear(); + fs::remove(marker->payloadDir, rec); // the empty placeholder, if any + rec.clear(); + fs::rename(parked, marker->payloadDir, rec); + if (rec) { + log::warn("could not restore the parked payload at {} to {}: " + "{} (pid {} is no longer running)", + parked.string(), marker->payloadDir.string(), + rec.message(), marker->pid); + continue; + } + remove_parked_payload_marker(parked); + log::info("recovered a parked payload: {} is back at {} " + "(the install that parked it, pid {}, did not finish)", + parked.string(), marker->payloadDir.string(), marker->pid); + } else { + // Something already occupies the original path -- a later + // install that finished cleanly, or the same crashed + // replacement's leftovers already resolved once. Either way + // the parked copy is superseded, not a second source of truth. + (void)remove_payload_dir(parked); + remove_parked_payload_marker(parked); + log::info("discarded a leftover parked payload at {} " + "(pid {} is no longer running, and {} already holds a " + "payload)", + parked.string(), marker->pid, marker->payloadDir.string()); + } + ++resolved; + } + std::error_code rmroot; + fs::remove(staleRoot, rmroot); // only when now empty + return resolved; +} + +std::expected +set_aside_payload(const fs::path& payloadDir) { + const auto parked = stale_payload_parking(payloadDir); + std::error_code ec; + fs::create_directories(parked.parent_path(), ec); + // Written before the rename below, not after: the marker's entire + // reason to exist is to survive a crash that lands between the two, and + // a marker written afterward could not do that. + write_parked_payload_marker(parked, payloadDir, platform::get_pid()); + ec.clear(); + fs::rename(payloadDir, parked, ec); + if (ec) { + remove_parked_payload_marker(parked); // nothing was parked + // Windows only (see locate_locked_file_): name the specific file + // that would not move, when there is one to name. + if (const auto locked = locate_locked_file_(payloadDir); !locked.empty()) { + return std::unexpected(std::format( + "cannot move {} aside to {}: {} -- {} could not be moved", + payloadDir.string(), parked.string(), ec.message(), + locked.string())); + } + return std::unexpected(std::format( + "cannot move {} aside to {}: {}", + payloadDir.string(), parked.string(), ec.message())); + } + fs::create_directories(payloadDir, ec); + if (ec || !fs::is_directory(payloadDir, ec)) { + const auto why = ec.message(); + std::error_code back; + fs::remove(payloadDir, back); + back.clear(); + fs::rename(parked, payloadDir, back); + if (!back) remove_parked_payload_marker(parked); // moved back already + return std::unexpected(std::format( + "cannot recreate {}: {}", payloadDir.string(), why)); + } + return PayloadReplacement(payloadDir, parked); +} + +PayloadReplacement::PayloadReplacement(fs::path payloadDir, fs::path parkedDir) + : payloadDir_(std::move(payloadDir)), parkedDir_(std::move(parkedDir)) {} + +PayloadReplacement::PayloadReplacement(PayloadReplacement&& other) noexcept + : payloadDir_(std::move(other.payloadDir_)), + parkedDir_(std::move(other.parkedDir_)), + settled_(other.settled_) { + other.settled_ = true; +} + +PayloadReplacement::~PayloadReplacement() { + if (settled_) return; + try { + const auto left = rollback(); + if (left.empty()) { + log::info("kept the previous payload at {}", payloadDir_.string()); + } else { + log::warn("the reinstall into {} did not complete, and {}", + payloadDir_.string(), left); + } + } catch (...) { + // A destructor reports nothing it cannot report safely. + } +} + +const fs::path& PayloadReplacement::parked() const { return parkedDir_; } + +void PayloadReplacement::commit() { + if (settled_) return; + settled_ = true; + (void)remove_payload_dir(parkedDir_); + remove_parked_payload_marker(parkedDir_); + std::error_code ignore; + fs::remove(parkedDir_.parent_path(), ignore); // only when now empty +} + +std::string PayloadReplacement::rollback() { + if (settled_) return {}; + settled_ = true; + // A partial tree that cannot be deleted is left stamped incomplete by + // remove_payload_dir, and the old one stays parked: renaming over a + // non-empty directory is not possible, and merging the two would produce + // a payload neither install wrote. The marker is deliberately NOT + // removed on this branch: it still names a parked directory that + // genuinely exists, and `recover_parked_payloads` -- once this process + // is gone -- is the same rule that would otherwise apply to it, run one + // command later instead of never. + if (remove_payload_dir(payloadDir_) != RemoveOutcome::Removed) { + return std::format("the previous payload is kept at {}", + parkedDir_.string()); + } + std::error_code ec; + fs::remove(payloadDir_, ec); + ec.clear(); + fs::rename(parkedDir_, payloadDir_, ec); + if (ec) { + // Windows only (see locate_locked_file_): name the specific file + // that would not move, when there is one to name. + if (const auto locked = locate_locked_file_(parkedDir_); !locked.empty()) { + return std::format("the previous payload is kept at {} ({} -- " + "{} could not be moved)", + parkedDir_.string(), ec.message(), + locked.string()); + } + return std::format("the previous payload is kept at {} ({})", + parkedDir_.string(), ec.message()); + } + remove_parked_payload_marker(parkedDir_); + fs::remove(parkedDir_.parent_path(), ec); // only when now empty + return {}; +} + } diff --git a/src/core/xim/payload.cppm b/src/core/xim/payload.cppm index a9a45d767..d8fd95c78 100644 --- a/src/core/xim/payload.cppm +++ b/src/core/xim/payload.cppm @@ -104,6 +104,27 @@ bool stamped_incomplete(const std::filesystem::path& dir); int stamped_registration_count(const std::filesystem::path& dir); +// The packaging revision the install that wrote this stamp recorded +// (openxlings/xlings#620). +// +// A recipe may state `revision = N` on a version entry: a change to what it +// installs that keeps the upstream version. The stamp records the revision +// the payload was built from, and `payload_revision_verdict` +// (install_state.cppm) compares the two. +// +// Three answers, and the difference between the last two is deliberate: +// * no stamp at all -> kRevisionUnrecorded: no observation, and +// so no verdict (a pre-stamp payload is not +// reinstalled on this account); +// * a stamp without the field -> 0: the install happened, before revisions +// existed, and a recipe that states none is +// revision 0. A recipe that states 1 or more +// therefore reaches every older payload; +// * the field -> its value. +constexpr int kRevisionUnrecorded = -1; + +int stamped_revision(const std::filesystem::path& dir); + // Record what this platform installed, so the next run does not have to guess. // // `registered` is the count of xvm nodes the install produced. It is what @@ -112,9 +133,14 @@ int stamped_registration_count(const std::filesystem::path& dir); // does. Measured on a real home: 29 payloads carried a stamp, reported // `installed`, and had no ledger entry at all -- the whole graphics stack // among them. See .agents/docs/2026-08-11-five-issues-triage-and-plan.md. +// +// `revision` is the recipe's packaging revision for this version; see +// `stamped_revision`. It is always written, 0 included, so a stamp from this +// client never relies on the absent-field reading. void write_payload_stamp(const std::filesystem::path& dir, std::string_view version, - int registered = kRegisteredUnrecorded); + int registered = kRegisteredUnrecorded, + int revision = 0); // Mark this payload as the residue of an install that did not finish. // @@ -224,4 +250,141 @@ RemoveOutcome settle_removal(const std::filesystem::path& root, // implying the store is clean. int sweep_payload_trash(const std::filesystem::path& trashRoot); +// ── replacing a payload that is installed but stale ────────────────── +// +// A payload whose recipe revision moved on (#620) is replaced, and the +// replacement must never leave a window in which the version has no payload: +// a failed reinstall that had already deleted the old tree turns "outdated" +// into "absent", for a package other payloads may link against. +// +// The new payload is installed into the REAL path, never into a temporary +// directory renamed afterwards: recipes write install_dir() into the files +// they install (gcc's specs, pkg-config files, rpaths), so a payload built +// anywhere else names a directory that does not hold it. The old tree is +// moved out of the way instead, and that move is what this type owns: +// +// set_aside_payload renames the old tree to its parking place and leaves +// an empty directory at the real path; +// commit the reinstall succeeded: the parked tree is deleted; +// rollback it did not: the partial tree is deleted and the +// parked one renamed back. The destructor does the same +// when neither was called, so every early exit of an +// install step restores the old payload. +// +// Files another tool wrote into the old tree (mcpp's `.mcpp_ok` and +// `.mcpp-fixup.json`) are deleted with it on commit. They describe the old +// payload, and carrying them over would assert them of the new one. +// +// The parking place is `/stale/`, beside `xpkgs` and so on the same +// filesystem. Not a sibling of the version directory: every reader of a store +// takes each subdirectory of `xpkgs//` for a version (see +// `payload_trash_root`). Not the trash root either: `remove` sweeps it, and a +// recipe may request a removal between its install and config hooks, which +// is inside this window. +class PayloadReplacement { +public: + PayloadReplacement(std::filesystem::path payloadDir, + std::filesystem::path parkedDir); + PayloadReplacement(PayloadReplacement&& other) noexcept; + PayloadReplacement& operator=(PayloadReplacement&&) = delete; + PayloadReplacement(const PayloadReplacement&) = delete; + PayloadReplacement& operator=(const PayloadReplacement&) = delete; + ~PayloadReplacement(); + + [[nodiscard]] const std::filesystem::path& parked() const; + + void commit(); + + // Empty when the old payload is back at its path; otherwise a sentence + // naming where it was left. + std::string rollback(); + +private: + std::filesystem::path payloadDir_; + std::filesystem::path parkedDir_; + bool settled_ { false }; +}; + +// Where `payloadDir` is parked while it is replaced: under `/stale/` +// for a payload inside an `xpkgs` store, a `.stale-` sibling otherwise. +// Unique within this process. +std::filesystem::path stale_payload_parking(const std::filesystem::path& payloadDir); + +// Moves the payload aside and recreates its directory empty. Refused, with the +// payload untouched, when the move is not possible (a held file on Windows, a +// store split across filesystems): a reinstall that cannot keep the old +// payload recoverable does not start. +std::expected +set_aside_payload(const std::filesystem::path& payloadDir); + +// ── recovering a parked payload after a crash ───────────────────────── +// +// PayloadReplacement's rollback is a destructor: it runs when the process +// that called `set_aside_payload` unwinds, by any path, without `commit()`. +// A kill (SIGKILL, power loss) between the rename in `set_aside_payload` and +// that destructor never runs it, and nothing else ever looks under +// `/stale/` again -- `sweep_payload_trash` walks a different root +// (`/trash`, `payload_trash_root`), for a different leftover +// (something `remove` displaced, not something an install parked). Every +// crash at that point leaks one directory, forever. +// +// The fix is a record written before the fact, because there is nothing to +// read after the fact: once the process is gone, the only thing that still +// says a parked directory was ever meant to go somewhere is whatever was +// written down while it still could be. `write_parked_payload_marker` is +// called from `set_aside_payload`, before the rename that creates the +// parked directory, so a crash in the smallest possible window after it +// still leaves the marker findable. It is written atomically +// (`platform::write_file_atomic`: temp file + rename), so a crash WHILE +// writing it leaves either the old marker (absent, the first time) or the +// new one -- never a torn file a later reader could misparse as valid. +struct ParkedPayloadMarker { + std::filesystem::path payloadDir; // where the parked directory came from + int pid { 0 }; // the process that parked it +}; + +// The marker's path for a given parked directory: a sibling of it (never a +// child), so writing or removing the marker is never mistaken for a change +// to the parked payload itself, and the marker survives exactly as long as +// the question it answers is still open. +std::filesystem::path parked_marker_path(const std::filesystem::path& parkedDir); + +// Written before `parkedDir` exists. `pid` is the parking process's own, +// e.g. `platform::get_pid()`. +void write_parked_payload_marker(const std::filesystem::path& parkedDir, + const std::filesystem::path& payloadDir, + int pid); + +// `std::nullopt` when there is no marker, or what is there cannot be read +// as one -- treated identically by `recover_parked_payloads`: nothing here +// says it is safe to touch a directory whose marker cannot be read. +std::optional read_parked_payload_marker( + const std::filesystem::path& parkedDir); + +// Idempotent: removing an already-absent marker is not an error. +void remove_parked_payload_marker(const std::filesystem::path& parkedDir); + +// Finishes what a crashed reinstall left parked under `staleRoot` +// (`/stale`), for every entry whose marker names a process that is no +// longer running: +// +// * the original payload path is missing, or is the empty directory +// `set_aside_payload` recreates right after parking the old one -- +// nothing replaced it, so the parked payload moves back; +// * anything else is already at the original path -- a later install +// that finished cleanly, or a leftover from the SAME interrupted +// replacement that this same rule already discarded once -- so the +// parked payload and its marker are removed rather than guessed about +// a second time. +// +// An entry with no readable marker, or whose owning process is still alive, +// is left exactly as found: the first because nothing says it is ours to +// touch, the second because that process may yet run its own rollback. +// +// Meant to run once per command, at the point installation first touches +// the store -- not once per package, and not on a dry run, which never +// touches the store at all. Returns how many entries were resolved, so a +// caller can log a count instead of nothing. +int recover_parked_payloads(const std::filesystem::path& staleRoot); + } // namespace xlings::xim diff --git a/src/core/xim/resolver.cpp b/src/core/xim/resolver.cpp index 7db56f0cc..aca4491e7 100644 --- a/src/core/xim/resolver.cpp +++ b/src/core/xim/resolver.cpp @@ -150,12 +150,13 @@ resolve(PackageCatalog& catalog, std::span targets, const std // an incomplete payload, while a planner that still called it // installed printed "nothing to do" and then "already installed" in // the same run that reinstalled it. Same predicate, both places. + const auto payloadDir = match.storeRoot + / package_store_name(match.namespaceName, match.name) + / match.version; node.alreadyInstalled = match.installed && !match.payloadForeign && !installation_state( ledgerIndex, match.namespaceName, match.name, match.version, - match.storeRoot - / package_store_name(match.namespaceName, match.name) - / match.version) + payloadDir) .is_incomplete(); node.kind = kind; @@ -167,6 +168,20 @@ resolve(PackageCatalog& catalog, std::span targets, const std // enumerates its architectures and does not list this one -- a // package-level `archs` union is never enough to stop a plan. const auto* entry = find_entry(*pkg, platform, node.version); + + // A payload of another packaging revision plans as not installed, + // like a foreign or an incomplete one, so its artifact is + // downloaded and the installer replaces it (#620). Same verdict + // function as the installer's check. + if (entry) node.revision = entry->revision; + if (node.alreadyInstalled) { + const auto verdict = + payload_revision_verdict(payloadDir, node.revision); + if (verdict.stale) { + node.alreadyInstalled = false; + node.staleReason = verdict.reason(); + } + } const auto compatibility = check_target_compatibility( *pkg, entry, platform, hostArch); if (!compatibility.supported) { diff --git a/src/interface.cppm b/src/interface.cppm index 4686efc49..9277609f7 100644 --- a/src/interface.cppm +++ b/src/interface.cppm @@ -22,7 +22,11 @@ import xlings.runtime; namespace xlings::interface { -export constexpr const char* kProtocolVersion = "1.0"; +// 1.1 (2026.9.27.1, additive): the `install_targets` data event, and the +// recipe revision as a third element of each `install_plan` entry. A 1.0 +// client ignores both. Clients detect a capability by its presence on the +// wire, not by comparing this string. +export constexpr const char* kProtocolVersion = "1.1"; // Convert any Event variant to one NDJSON line (no trailing newline). // Returns "" for events not surfaced to wire (e.g. CompletedEvent — the diff --git a/tests/data/semver-vectors.tsv b/tests/data/semver-vectors.tsv new file mode 100644 index 000000000..20def3207 --- /dev/null +++ b/tests/data/semver-vectors.tsv @@ -0,0 +1,105 @@ +# xlings version-resolution conformance vectors +# +# Which version an install request selects from the versions a recipe +# declares. The unit test tests/unit/test_semver_vectors.cpp drives the +# resolver xlings installs with (pin_target_to_subos, then the catalog's +# version selection) with every vector below; another implementation of the +# same grammar can vendor this file and run it unchanged. +# +# FORMAT +# +# One vector per line, four columns separated by a single TAB: +# +# request the version part of a request, as written after `@` +# (`1.7`, `>=1.2 <2`, `^1.2.3`); `-` for a bare name. +# May contain spaces, never a tab. +# available the version keys the recipe declares for the platform, +# comma-separated, no spaces. Every key names a concrete +# version (no aliases, no `latest`). +# active the version active in the workspace, `-` for none. +# expected the version selected, or `none` when nothing is. +# +# Lines starting with `#` and blank lines carry no vector. +# +# RULES THE VECTORS EXERCISE +# +# 1. A request that equals a declared key selects that key. +# 2. Otherwise the highest declared version satisfying the request wins. +# 3. A bare version of three or more segments is written-prefix equality, +# floored at three segments: 1.7.0 matches 1.7.0 and 1.7.0.1, never +# 1.7.1; 1.8.12 never selects 1.9.0. +# 4. A bare version of one or two segments is the prefix range [v, next): +# 1.2 is [1.2, 1.3), 1 is [1, 2). 1.1 never matches 1.10. +# 5. A prerelease is exact; a release request never selects a prerelease. +# 6. Operators: >=, >, <=, < (a space-separated list is a conjunction), +# ^ (up to the next release of the first nonzero segment), ~ (up to the +# next second segment), and the wildcards 1.2.* and 1.*. Ordering +# compares segment by segment, numerically, a missing segment being 0. +# 7. A bare name selects the highest declared version. +# 8. An active version that satisfies the request, and that the recipe +# declares, is selected; otherwise resolution proceeds as if nothing +# were active. +# +# request available active expected + +# rule 1: a declared key is selected as written +1.2 1.2,1.2.0,1.2.5 - 1.2 +1.7.0 1.7.0,1.7.0.1 - 1.7.0 +2.15.0.1 2.15.0.1,2.15.0.2 - 2.15.0.1 +2026.7.31.2 2026.7.31.1,2026.7.31.2,2026.8.1.1 - 2026.7.31.2 +25.0.4+7 25.0.3+9,25.0.4+7 - 25.0.4+7 + +# rule 3: three or more segments are written-prefix equality +1.7.0 1.7.0.1,1.7.1 - 1.7.0.1 +1.7.0 1.7.0.1,1.7.0.2,1.7.1 - 1.7.0.2 +12.9.1 12.9.1.2,12.9.1.4,12.9.2.0 - 12.9.1.4 +2.15.0 2.15.0.1,2.15.0.2,2.15.1.0 - 2.15.0.2 +1.8.12 1.8.11,1.9.0 - none +1.8.12 1.8.12.4,1.8.13,1.9.0 - 1.8.12.4 +15.1.0 15.1.1,16.1.0 - none + +# rule 4: one or two segments are a prefix range +1.7 1.7.0.1,1.8.0.0 - 1.7.0.1 +1.7 1.6.9.9,1.7.0.1,1.7.2.3,1.8.0.0 - 1.7.2.3 +1.2 1.1.9,1.2.0,1.2.9,1.3.0 - 1.2.9 +1.2 1.3.0,1.10.0 - none +1.1 1.1.0,1.10.0 - 1.1.0 +1 0.9.0,1.0.0,1.9.9,2.0.0 - 1.9.9 +15 14.2.0,15.1.0,15.2.0,16.1.0 - 15.2.0 +2.39 2.38,2.40 - none + +# rule 5: a prerelease is exact +1.0.0-rc1 1.0.0-rc1,1.0.0-rc2,1.0.0 - 1.0.0-rc1 +1.0.0-rc1 1.0.0-rc2,1.0.0 - none +1.0.0 1.0.0-rc1,1.0.0-rc2 - none + +# rule 6: operators +>=1.2 1.1.0,1.2.0,2.0.0 - 2.0.0 +>=1.2 <2 1.1.0,1.2.0,1.9.3,2.0.0 - 1.9.3 +>1.2.0 1.2.0,1.2.1 - 1.2.1 +<=1.5 1.4.0,1.5.0,1.5.1 - 1.5.0 +<2 1.9.9,2.0.0 - 1.9.9 +>=2.38 2.37,2.39,2.40 - 2.40 +>=2.15.0.1 2.15.0.0,2.15.0.1,2.15.0.3 - 2.15.0.3 +>=3 1.0.0,2.9.9 - none +^1.2.3 1.2.2,1.2.3,1.9.0,2.0.0 - 1.9.0 +^0.2.3 0.2.3,0.2.9,0.3.0 - 0.2.9 +~1.2.3 1.2.3,1.2.8,1.3.0 - 1.2.8 +1.2.* 1.2.0,1.2.7,1.3.0 - 1.2.7 +1.* 1.0.0,1.9.0,2.0.0 - 1.9.0 + +# rule 7: a bare name selects the highest declared version +- 1.0.0,2.0.0,1.10.0 - 2.0.0 +- 0.0.9,0.0.11,0.0.100 - 0.0.100 +- 2026.7.31.2,2026.7.31.10 - 2026.7.31.10 + +# rule 8: an active version that satisfies the request is selected +- 1.0.0,2.0.0 1.0.0 1.0.0 +1 1.5.0,1.9.0 1.5.0 1.5.0 +^1.2 1.2.0,1.10.0,1.11.0 1.10.0 1.10.0 +>=2.38 2.39,2.40 2.39 2.39 +12.9.1 12.9.1.2,12.9.1.4 12.9.1.2 12.9.1.2 +1.7 1.7.0.1,1.7.2.3 1.7.0.1 1.7.0.1 +1.1 1.1.0,1.10.0 1.10.0 1.1.0 +2 1.5.0,2.1.0 1.5.0 2.1.0 +- 1.0.0,2.0.0 0.9.0 2.0.0 diff --git a/tests/e2e/install_revision_test.sh b/tests/e2e/install_revision_test.sh new file mode 100755 index 000000000..dd8df9614 --- /dev/null +++ b/tests/e2e/install_revision_test.sh @@ -0,0 +1,342 @@ +#!/usr/bin/env bash +# E2E: a recipe's packaging revision, and the install_targets record. +# +# WHAT THIS DEFENDS (openxlings/xlings#620, interface protocol 1.1) +# +# 1. `revision` on a version entry counts changes to what a recipe installs +# under an unchanged upstream version. A payload is current iff the revision +# its stamp records equals the recipe's. Without that, `install` of a version +# already on disk returned on the strength of the version database and the +# `installed` probe, and a fixed recipe never reached a machine that had +# installed the broken one. +# 2. A stamp written before the field existed reads as revision 0: a recipe +# that states none reinstalls nothing, one that states 1 reaches it. +# 3. The reinstall never leaves the version without a payload: a failing +# reinstall puts the previous payload back. +# 4. The top-level install reports what each request resolved to +# (`install_targets`) on every path, including "already installed" and +# failures, exactly once per invocation; a dry run does not report. +# 5. The reinstall's rollback is a C++ destructor and never runs when the +# process is killed instead of unwound: a marker written before the old +# payload is parked lets the NEXT install finish the job -- put the +# parked payload back, or discard it -- instead of leaking it under +# /stale/ forever. +# +# Offline: the fixture is hook-only (no download). +set -euo pipefail + +# shellcheck source=./project_test_lib.sh +source "$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)/project_test_lib.sh" + +require_fixture_index + +RUNTIME_DIR="$ROOT_DIR/tests/e2e/runtime/install_revision" +HOME_DIR="$RUNTIME_DIR/home" +LOCAL_INDEX_DIR="$RUNTIME_DIR/xim-pkgindex" +RECIPE="$LOCAL_INDEX_DIR/pkgs/r/revpkg.lua" + +cleanup() { + chmod -R u+w "$RUNTIME_DIR" 2>/dev/null || true + rm -rf "$RUNTIME_DIR" +} +trap cleanup EXIT +cleanup + +XLINGS_BIN="$(find_xlings_bin)" + +RUN() { + ( cd /tmp && env -i HOME="$HOME" PATH=/usr/bin:/bin \ + XLINGS_HOME="$HOME_DIR" XLINGS_ACTIVE_SUBOS=default \ + XLINGS_NON_INTERACTIVE=1 \ + "$XLINGS_BIN" "$@" ) +} + +# write_recipe +# `-` writes an entry that states no revision at all. +# `fail` makes the install hook write part of a payload and then fail. +write_recipe() { + local rev="$1" mode="$2" entry label + if [[ "$rev" == "-" ]]; then entry="{}"; label="r0" + else entry="{ revision = $rev }"; label="r$rev"; fi + mkdir -p "$(dirname "$RECIPE")" + cat > "$RECIPE" < key=value ... +PY_EXPECT=' +import json, sys +payload = json.loads(sys.argv[1]) +target = payload["targets"][int(sys.argv[2])] +bad = [] +for pair in sys.argv[3:]: + key, want = pair.split("=", 1) + got = target.get(key) + if str(got) != want: + bad.append("%s: want %r, got %r" % (key, want, got)) +if bad: + sys.exit("; ".join(bad) + "\n entry: " + json.dumps(target)) +' +targets_of() { python3 -c "$PY_TARGETS" "${2:-1}" <<<"$1"; } +expect_target() { + local payload="$1" index="$2"; shift 2 + python3 -c "$PY_EXPECT" "$payload" "$index" "$@" \ + || fail "install_targets entry $index does not match" +} +has_kind() { grep -q "\"dataKind\":\"$2\"" <<<"$1"; } + +mkdir -p "$HOME_DIR/subos/default/bin" +cp -r "$FIXTURE_INDEX_DIR" "$LOCAL_INDEX_DIR" +printf 'xim_indexrepos = {}\n' > "$LOCAL_INDEX_DIR/xim-indexrepos.lua" +write_recipe - ok + +cp "$XLINGS_BIN" "$HOME_DIR/xlings" +cat > "$HOME_DIR/.xlings.json" </dev/null 2>&1 || fail "self init failed" +mkdir -p "$HOME_DIR/data/xim-index-repos" +printf '{}\n' > "$HOME_DIR/data/xim-index-repos/xim-indexrepos.json" + +PAYLOAD="$HOME_DIR/data/xpkgs/xim-x-revpkg/1.0" +STAMP="$PAYLOAD/.xpkg-install.json" +ARGS='{"targets":["revpkg@1.0"],"yes":true}' + +# ── R1: fresh install ──────────────────────────────────────────────── +log "R1: a fresh install reports installed, with the payload directory" +OUT="$(RUN interface install_packages --args "$ARGS" 2>/dev/null)" \ + || fail "R1: install failed: +$OUT" +T="$(targets_of "$OUT")" || fail "R1: $OUT" +expect_target "$T" 0 request=revpkg@1.0 namespace=xim name=revpkg \ + version=1.0 revision=0 status=installed "payload_dir=$PAYLOAD" +[[ "$(cat "$PAYLOAD/bin/revpkg")" == "revpkg-1.0-r0" ]] \ + || fail "R1: the payload is not on disk" +grep -q '"revision": 0' "$STAMP" \ + || fail "R1: the stamp does not record the revision: $(cat "$STAMP")" +python3 -c ' +import json, sys +for line in sys.stdin: + e = json.loads(line) if line.startswith("{") else {} + if e.get("dataKind") == "install_plan": + entry = e["payload"]["packages"][0] + if len(entry) != 3 or entry[2] != 0: + sys.exit("install_plan entry carries no revision: %r" % entry) + sys.exit(0) +sys.exit("no install_plan event") +' <<<"$OUT" || fail "R1: install_plan" +log " ok" + +# ── R2: already installed ──────────────────────────────────────────── +log "R2: everything already installed still reports, as already_present" +OUT="$(RUN interface install_packages --args "$ARGS" 2>/dev/null)" \ + || fail "R2: install failed: +$OUT" +T="$(targets_of "$OUT")" || fail "R2: $OUT" +expect_target "$T" 0 request=revpkg@1.0 version=1.0 revision=0 \ + status=already_present "payload_dir=$PAYLOAD" +if has_kind "$OUT" install_plan; then fail "R2: a plan for nothing to do"; fi +log " ok" + +# ── R3: a stamp from before revisions existed ──────────────────────── +log "R3: a stamp without the field is revision 0 -- nothing is reinstalled" +python3 - "$STAMP" <<'PY' +import json, sys +path = sys.argv[1] +stamp = json.load(open(path)) +stamp.pop("revision", None) +stamp["xlings_version"] = "2026.9.26.3" +open(path, "w").write(json.dumps(stamp, indent=2) + "\n") +PY +if grep -q '"revision"' "$STAMP"; then fail "R3: fixture still carries the field"; fi +printf 'sentinel\n' > "$PAYLOAD/keep-me" +OUT="$(RUN interface install_packages --args "$ARGS" 2>/dev/null)" \ + || fail "R3: install failed" +T="$(targets_of "$OUT")" || fail "R3: $OUT" +expect_target "$T" 0 status=already_present revision=0 +[[ -f "$PAYLOAD/keep-me" ]] || fail "R3: a revision-less recipe reinstalled" +log " ok" + +# ── R4: the recipe states revision 1 ───────────────────────────────── +log "R4: revision 1 reinstalls the version in place, and says why" +write_recipe 1 ok +printf 'ok\n' > "$PAYLOAD/.mcpp_ok" +OUT="$(RUN install revpkg@1.0 -y 2>&1 | strip_ansi)" \ + || fail "R4: the reinstall failed: +$OUT" +echo "$OUT" | sed 's/^/ /' +grep -qF "reinstalling revpkg@1.0: recipe revision 1, installed revision 0" \ + <<<"$OUT" || fail "R4: the output does not say why it reinstalled" +[[ "$(cat "$PAYLOAD/bin/revpkg")" == "revpkg-1.0-r1" ]] \ + || fail "R4: the payload was not replaced -- the fast path answered" +grep -q '"revision": 1' "$STAMP" || fail "R4: stamp: $(cat "$STAMP")" +[[ ! -e "$PAYLOAD/keep-me" ]] || fail "R4: the old tree was kept" +[[ ! -e "$PAYLOAD/.mcpp_ok" ]] \ + || fail "R4: a marker about the old payload survived into the new one" +[[ ! -e "$HOME_DIR/data/stale" ]] \ + || fail "R4: the parked payload was not removed: $(ls -R "$HOME_DIR/data/stale")" +log " ok" + +log "R5: the reinstalled payload is current" +OUT="$(RUN interface install_packages --args "$ARGS" 2>/dev/null)" \ + || fail "R5: install failed" +T="$(targets_of "$OUT")" || fail "R5: $OUT" +expect_target "$T" 0 status=already_present revision=1 "payload_dir=$PAYLOAD" +log " ok" + +# ── R6: a reinstall that fails keeps the previous payload ──────────── +log "R6: a failing reinstall puts the previous payload back" +write_recipe 2 fail +set +e +OUT="$(RUN interface install_packages --args "$ARGS" 2>/dev/null)" +RC=$? +set -e +[[ "$RC" -ne 0 ]] || fail "R6: a failed reinstall exited 0" +T="$(targets_of "$OUT")" || fail "R6: $OUT" +expect_target "$T" 0 request=revpkg@1.0 version=1.0 revision=2 \ + status=failed payload_dir= +[[ "$(cat "$PAYLOAD/bin/revpkg" 2>/dev/null)" == "revpkg-1.0-r1" ]] \ + || fail "R6: the previous payload is gone after a failed reinstall" +[[ ! -e "$PAYLOAD/bin/half-written" ]] || fail "R6: the partial payload stayed" +grep -q '"revision": 1' "$STAMP" || fail "R6: stamp: $(cat "$STAMP")" +if grep -q '"incomplete"' "$STAMP"; then + fail "R6: the restored payload is marked incomplete" +fi +[[ ! -e "$HOME_DIR/data/stale" ]] || fail "R6: a parked payload was left behind" + +set +e +OUT="$(RUN install revpkg@1.0 -y 2>&1 | strip_ansi)" +set -e +grep -qF "reinstalling revpkg@1.0: recipe revision 2, installed revision 1" \ + <<<"$OUT" || fail "R6: no reason printed: +$OUT" +grep -qF "kept the previous payload" <<<"$OUT" \ + || fail "R6: the restore is not reported: +$OUT" +[[ "$(cat "$PAYLOAD/bin/revpkg")" == "revpkg-1.0-r1" ]] \ + || fail "R6: the CLI run lost the previous payload" +log " ok" + +# ── R7: a request that resolves to nothing ─────────────────────────── +log "R7: an unresolvable request is reported as failed" +set +e +OUT="$(RUN interface install_packages \ + --args '{"targets":["xim:revpkg-nosuch"],"yes":true}' 2>/dev/null)" +set -e +T="$(targets_of "$OUT")" || fail "R7: $OUT" +expect_target "$T" 0 request=xim:revpkg-nosuch namespace= name= version= \ + status=failed payload_dir= +log " ok" + +# ── R8: a dry run does not report ──────────────────────────────────── +log "R8: plan_install emits no install_targets" +write_recipe 1 ok +OUT="$(RUN interface plan_install --args '{"targets":["revpkg@1.0"]}' 2>/dev/null)" \ + || fail "R8: plan_install failed" +targets_of "$OUT" 0 >/dev/null || fail "R8: a dry run reported install_targets" +log " ok" + +# ── R9: a killed reinstall's parked payload is recovered ───────────── +# +# set_aside_payload's rollback (PayloadReplacement) is a C++ destructor: it +# never runs when the process is killed instead of unwound, so a crash right +# after the rename into /stale/ used to leak that directory forever. +# This fabricates exactly that shape -- a parked payload, a marker naming a +# dead pid, and the original path missing (the crash landed before +# set_aside_payload even recreated the empty placeholder) -- and asserts +# that the NEXT install notices and finishes the job. +log "R9: a killed reinstall's parked payload is recovered on the next install" +[[ "$(cat "$PAYLOAD/bin/revpkg")" == "revpkg-1.0-r1" ]] \ + || fail "R9: precondition: the payload from R6 is not what this case expects" + +STALE_ROOT="$HOME_DIR/data/stale" +PARKED="$STALE_ROOT/xim-x-revpkg-1.0.stale-999999" +rm -rf "$STALE_ROOT" +mkdir -p "$PARKED/bin" +cp "$PAYLOAD/bin/revpkg" "$PARKED/bin/revpkg" +cp "$STAMP" "$PARKED/.xpkg-install.json" +# Written by ONLY this fabricated parked copy, never by the recipe's own +# install() -- proves a later assertion sees the RESTORED payload, not one a +# fresh install hook happened to reproduce identically. +printf 'parked\n' > "$PARKED/bin/PARKED_MARKER" + +# A pid guaranteed not to be running: spawn a no-op subshell and reap it +# before writing the marker, rather than guessing a number. +( : ) & DEADPID=$! +wait "$DEADPID" 2>/dev/null || true + +cat > "${PARKED}.origin" <(in), {}); + } + EXPECT_NE(text.find("\"revision\": 0"), std::string::npos) << text; + fs::remove_all(home); +} + +TEST(InstallStateRevision, AStampWithoutTheFieldIsRevisionZero) { + const auto home = make_temp_root("rev-absent"); + const auto dir = payload_dir(home, "xim", "glibc", "2.44.3"); + write_file(dir / "lib" / "libc.so.6", "payload"); + write_prerevision_stamp(dir, "2.44.3"); + + EXPECT_EQ(xim::stamped_revision(dir), 0); + + // A recipe that states none leaves every older payload alone ... + EXPECT_FALSE(xim::payload_revision_verdict(dir, 0).stale); + // ... and one that states 1 reaches every older payload. + const auto bumped = xim::payload_revision_verdict(dir, 1); + EXPECT_TRUE(bumped.stale); + EXPECT_EQ(bumped.recorded, 0); + EXPECT_EQ(bumped.recipe, 1); + EXPECT_EQ(bumped.reason(), "recipe revision 1, installed revision 0"); + fs::remove_all(home); +} + +TEST(InstallStateRevision, CurrentIffTheTwoRevisionsAreEqual) { + const auto home = make_temp_root("rev-equal"); + const auto dir = payload_dir(home, "xim", "demo", "1.0"); + write_file(dir / "bin" / "demo", "payload"); + xim::write_payload_stamp(dir, "1.0", /*registered=*/1, /*revision=*/2); + + EXPECT_FALSE(xim::payload_revision_verdict(dir, 2).stale); + EXPECT_TRUE(xim::payload_revision_verdict(dir, 3).stale); + // Equality, not order: a recipe that went back to an earlier packaging + // is still a different packaging from the one on disk. + EXPECT_TRUE(xim::payload_revision_verdict(dir, 1).stale); + fs::remove_all(home); +} + +// The falsification half: nothing observed, nothing decided. +TEST(InstallStateRevision, NoStampGivesNoVerdict) { + const auto home = make_temp_root("rev-nostamp"); + const auto dir = payload_dir(home, "xim", "linux-headers", "5.11.1"); + write_file(dir / "include" / "linux" / "types.h", "payload"); + + const auto verdict = xim::payload_revision_verdict(dir, 4); + EXPECT_FALSE(verdict.stale); + EXPECT_EQ(verdict.recorded, xim::kRevisionUnrecorded); + EXPECT_EQ(xim::stamped_revision(dir), xim::kRevisionUnrecorded); + fs::remove_all(home); +} + +// An incomplete payload is reinstalled by the incomplete path; the revision +// verdict does not claim it a second time, and an absent payload is not stale. +TEST(InstallStateRevision, IncompleteOrAbsentPayloadIsNotStale) { + const auto home = make_temp_root("rev-incomplete"); + const auto dir = payload_dir(home, "xim", "demo", "1.0"); + write_file(dir / "lib" / "libdemo.so", "partial"); + xim::write_payload_failure_marker(dir, "1.0", "install hook returned false"); + EXPECT_FALSE(xim::payload_revision_verdict(dir, 1).stale); + + const auto empty = payload_dir(home, "xim", "demo", "2.0"); + EXPECT_FALSE(xim::payload_revision_verdict(empty, 1).stale); + EXPECT_FALSE(xim::payload_revision_verdict(home / "missing", 1).stale); + fs::remove_all(home); +} diff --git a/tests/unit/test_interface_protocol.cpp b/tests/unit/test_interface_protocol.cpp index c2bb6ed33..7c4455b33 100644 --- a/tests/unit/test_interface_protocol.cpp +++ b/tests/unit/test_interface_protocol.cpp @@ -192,7 +192,7 @@ TEST(InterfaceProtocol, VersionFlagPrintsProtocolVersion) { auto j = nlohmann::json::parse(out, nullptr, false); ASSERT_FALSE(j.is_discarded()) << "non-JSON output: " << out; ASSERT_TRUE(j.contains("protocol_version")); - EXPECT_EQ(j["protocol_version"].get(), "1.0"); + EXPECT_EQ(j["protocol_version"].get(), "1.1"); std::filesystem::remove_all(home); } @@ -597,7 +597,45 @@ TEST(InterfaceProtocol, PlanInstallMissingPackageEmitsStructuredError) { EXPECT_TRUE(saw_err) << "non-zero plan_install exit produced no error event (issue #374): " << out; + // A dry run installs nothing, so it reports no install_targets. + for (auto& e : events) { + EXPECT_NE(e.value("dataKind", std::string{}), "install_targets") << out; + } + + std::filesystem::remove_all(home); +} + +// Protocol 1.1: every top-level install reports, per request, what it +// resolved to -- on the failure paths too, including one that stops before +// anything resolves. A client reads the entry for its request instead of +// inferring "nothing happened" from an event that never came. +TEST(InterfaceProtocol, InstallPackagesReportsEveryRequestEvenWhenNothingResolves) { + auto home = make_sandbox_home_(); + auto [out, _rc] = run_xlings_({"interface", "install_packages", "--args", + R"({"targets":["xim:nonexistent_pkg_xyz"],"yes":true})"}, home); std::filesystem::remove_all(home); + + auto events = parse_ndjson_(out); + ASSERT_FALSE(events.empty()) << out; + EXPECT_NE(events.back().value("exitCode", 0), 0) << out; + + int reports = 0; + for (auto& e : events) { + if (e.value("dataKind", std::string{}) != "install_targets") continue; + ++reports; + ASSERT_TRUE(e["payload"].is_object()) << out; + const auto& targets = e["payload"]["targets"]; + ASSERT_TRUE(targets.is_array()) << out; + ASSERT_EQ(targets.size(), 1u) << out; + const auto& t = targets[0]; + EXPECT_EQ(t.value("request", std::string{}), "xim:nonexistent_pkg_xyz"); + EXPECT_EQ(t.value("status", std::string{}), "failed"); + EXPECT_EQ(t.value("payload_dir", std::string{"?"}), ""); + EXPECT_EQ(t.value("version", std::string{"?"}), ""); + EXPECT_TRUE(t.contains("revision") && t["revision"].is_number_integer()); + EXPECT_TRUE(t.contains("namespace") && t.contains("name")); + } + EXPECT_EQ(reports, 1) << "exactly one install_targets per invocation: " << out; } // #464: a params object that does not satisfy the capability's own declared diff --git a/tests/unit/test_payload_remove.cpp b/tests/unit/test_payload_remove.cpp index 52cac34f7..291b85c72 100644 --- a/tests/unit/test_payload_remove.cpp +++ b/tests/unit/test_payload_remove.cpp @@ -35,6 +35,7 @@ import std; import xlings.core.xim.payload; +import xlings.platform; namespace xim = xlings::xim; namespace fs = std::filesystem; @@ -292,3 +293,288 @@ TEST(PayloadRemove, SweepingNothingIsFine) { EXPECT_EQ(xim::sweep_payload_trash(xim::payload_trash_root(fx.payload())), 0); EXPECT_EQ(xim::sweep_payload_trash({}), 0); } + +// ── replacing a stale payload (#620) ───────────────────────────────── +// +// The reinstall of a payload whose recipe revision moved on must never leave +// the version without a payload: the old tree is set aside, the new one is +// installed at the real path, and the old one is either deleted (commit) or +// put back (rollback, or destruction without commit). + +namespace { + +std::string read_(const fs::path& p) { + std::ifstream in(p); + return {std::istreambuf_iterator(in), {}}; +} + +} // namespace + +TEST(PayloadReplace, SetAsideLeavesAnEmptyPathAndParksOutsideTheStore) { + StoreFixture fx{"replace-aside"}; + fx.file("bin/node", "old"); + fx.file(".mcpp_ok", "mcpp"); + + auto aside = xim::set_aside_payload(fx.payload()); + ASSERT_TRUE(aside) << aside.error(); + + EXPECT_TRUE(fs::is_directory(fx.payload())); + EXPECT_TRUE(fs::is_empty(fx.payload())) + << "the install hook needs its install_dir, and an empty one"; + EXPECT_EQ(read_(aside->parked() / "bin" / "node"), "old"); + EXPECT_EQ(aside->parked().parent_path(), fx.root / "data" / "stale"); + + // Parked where no store scan reads it as a version of this package. + EXPECT_EQ(fx.versions_as_seen_by_the_store(), + std::vector{"22.17.1"}); + aside->commit(); +} + +TEST(PayloadReplace, CommitDeletesTheOldTreeWithWhatOtherToolsWroteIntoIt) { + StoreFixture fx{"replace-commit"}; + fx.file("bin/node", "old"); + fx.file(".mcpp_ok", "mcpp"); + fx.file(".mcpp-fixup.json", "{}"); + + auto aside = xim::set_aside_payload(fx.payload()); + ASSERT_TRUE(aside) << aside.error(); + const auto parked = aside->parked(); + fx.file("bin/node", "new"); + aside->commit(); + + EXPECT_EQ(read_(fx.payload() / "bin" / "node"), "new"); + EXPECT_FALSE(fs::exists(fx.payload() / ".mcpp_ok")) + << "a marker written about the old payload must not describe the new one"; + EXPECT_FALSE(fs::exists(fx.payload() / ".mcpp-fixup.json")); + EXPECT_FALSE(fs::exists(parked)); + EXPECT_FALSE(fs::exists(fx.root / "data" / "stale")) + << "an empty parking directory is noise in the data dir"; +} + +TEST(PayloadReplace, RollbackPutsTheOldPayloadBackAndDropsThePartialOne) { + StoreFixture fx{"replace-rollback"}; + fx.file("bin/node", "old"); + fx.file(".mcpp_ok", "mcpp"); + + auto aside = xim::set_aside_payload(fx.payload()); + ASSERT_TRUE(aside) << aside.error(); + fx.file("bin/node", "partial"); + fx.file("lib/half-written.so", "partial"); + + EXPECT_EQ(aside->rollback(), ""); + EXPECT_EQ(read_(fx.payload() / "bin" / "node"), "old"); + EXPECT_TRUE(fs::exists(fx.payload() / ".mcpp_ok")); + EXPECT_FALSE(fs::exists(fx.payload() / "lib" / "half-written.so")); + EXPECT_FALSE(fs::exists(fx.root / "data" / "stale")); +} + +// Every early exit of an install step is a scope exit. The destructor is what +// makes all of them restore the old payload without each remembering to. +TEST(PayloadReplace, LeavingScopeWithoutCommitRollsBack) { + StoreFixture fx{"replace-scope"}; + fx.file("bin/node", "old"); + { + auto aside = xim::set_aside_payload(fx.payload()); + ASSERT_TRUE(aside) << aside.error(); + fx.file("bin/node", "partial"); + } + EXPECT_EQ(read_(fx.payload() / "bin" / "node"), "old"); +} + +TEST(PayloadReplace, AMoveThatIsRefusedLeavesThePayloadInPlace) { + if (running_as_root()) GTEST_SKIP() << "root ignores directory write bits"; + StoreFixture fx{"replace-refused"}; + fx.file("bin/node", "old"); + freeze_(fx.pkgDir()); + + auto aside = xim::set_aside_payload(fx.payload()); + thaw_(fx.pkgDir()); + + EXPECT_FALSE(aside) << "a reinstall that cannot keep the old payload " + "recoverable must not start"; + EXPECT_EQ(read_(fx.payload() / "bin" / "node"), "old"); +} + +// ── recovering a parked payload after a crash ───────────────────────── +// +// PayloadReplacement's rollback is a destructor: a kill between the rename +// in `set_aside_payload` and that destructor never runs it, and nothing else +// ever revisits `/stale/` again. These tests drive the marker and the +// sweep that finishes the job on a LATER run, once the parking process is +// gone. + +namespace { + +// A pid that is guaranteed not to be running: spawn a trivial child and +// reap it before returning, rather than guessing a number. Cross-platform +// through the same process primitives the rest of xlings uses to run one. +int dead_pid_() { + auto handle = xlings::platform::spawn_command("exit 0"); + auto [code, output] = xlings::platform::wait_or_kill( + handle, nullptr, std::chrono::seconds(5)); + (void)code; (void)output; + return handle.pid; +} + +fs::path make_stale_root_(std::string_view tag) { + auto root = fs::temp_directory_path() / + ("xlings-payload-stale-" + std::string(tag)); + std::error_code ec; + fs::remove_all(root, ec); + fs::create_directories(root, ec); + return root; +} + +} // namespace + +TEST(ParkedPayloadMarker, RoundTrips) { + auto stale = make_stale_root_("marker-roundtrip"); + const auto parked = stale / "xim-x-node-22.17.1.stale-4242"; + // A path with characters JSON would otherwise treat as its own escapes, + // to exercise the escape/unescape pair rather than only the common case. + const auto original = fs::temp_directory_path() / + "xpkgs" / "xim-x-node" / "a \"quoted\" \\ name" / "22.17.1"; + + xim::write_parked_payload_marker(parked, original, 4242); + auto marker = xim::read_parked_payload_marker(parked); + ASSERT_TRUE(marker.has_value()); + EXPECT_EQ(marker->payloadDir, original); + EXPECT_EQ(marker->pid, 4242); + + xim::remove_parked_payload_marker(parked); + EXPECT_FALSE(xim::read_parked_payload_marker(parked).has_value()); + + std::error_code ec; + fs::remove_all(stale, ec); +} + +TEST(ParkedPayloadMarker, NoMarkerReadsAsNullopt) { + auto stale = make_stale_root_("marker-absent"); + EXPECT_FALSE(xim::read_parked_payload_marker(stale / "nothing-here") + .has_value()); + std::error_code ec; + fs::remove_all(stale, ec); +} + +TEST(RecoverParkedPayloads, RestoresAParkedPayloadWhoseOriginalPathIsMissing) { + auto stale = make_stale_root_("recover-missing"); + const auto parked = stale / "xim-x-node-22.17.1.stale-1"; + std::error_code ec; + fs::create_directories(parked / "bin", ec); + std::ofstream(parked / "bin" / "node") << "old"; + + const auto original = fs::temp_directory_path() / + "xlings-payload-stale-recover-missing-orig" / "22.17.1"; + fs::remove_all(original.parent_path(), ec); // make sure it is absent + + xim::write_parked_payload_marker(parked, original, dead_pid_()); + + EXPECT_EQ(xim::recover_parked_payloads(stale), 1); + EXPECT_TRUE(fs::exists(original / "bin" / "node")) + << "the parked payload should have moved back to its original path"; + EXPECT_EQ(read_(original / "bin" / "node"), "old"); + EXPECT_FALSE(fs::exists(parked)) << "the parked directory should be gone"; + EXPECT_FALSE(fs::exists(stale)) + << "an emptied stale root is noise in the data dir"; + + fs::remove_all(original.parent_path(), ec); +} + +TEST(RecoverParkedPayloads, + RestoresAParkedPayloadWhoseOriginalIsTheEmptyPlaceholder) { + // The exact shape set_aside_payload leaves behind when a crash lands + // between the rename and the destructor running: the parked copy holds + // the old payload, and the real path holds the empty directory + // set_aside_payload recreates right after parking it. + auto stale = make_stale_root_("recover-placeholder"); + const auto parked = stale / "xim-x-node-22.17.1.stale-1"; + std::error_code ec; + fs::create_directories(parked / "bin", ec); + std::ofstream(parked / "bin" / "node") << "old"; + + const auto original = fs::temp_directory_path() / + "xlings-payload-stale-recover-placeholder-orig" / "22.17.1"; + fs::remove_all(original.parent_path(), ec); + fs::create_directories(original, ec); // empty placeholder + + xim::write_parked_payload_marker(parked, original, dead_pid_()); + + EXPECT_EQ(xim::recover_parked_payloads(stale), 1); + EXPECT_EQ(read_(original / "bin" / "node"), "old"); + EXPECT_FALSE(fs::exists(parked)); + + fs::remove_all(original.parent_path(), ec); +} + +TEST(RecoverParkedPayloads, DeletesAParkedPayloadWhoseOriginalHoldsANewerPayload) { + auto stale = make_stale_root_("recover-superseded"); + const auto parked = stale / "xim-x-node-22.17.1.stale-1"; + std::error_code ec; + fs::create_directories(parked / "bin", ec); + std::ofstream(parked / "bin" / "node") << "old"; + + const auto original = fs::temp_directory_path() / + "xlings-payload-stale-recover-superseded-orig" / "22.17.1"; + fs::remove_all(original.parent_path(), ec); + fs::create_directories(original / "bin", ec); + std::ofstream(original / "bin" / "node") << "new"; // already replaced + + xim::write_parked_payload_marker(parked, original, dead_pid_()); + + EXPECT_EQ(xim::recover_parked_payloads(stale), 1); + EXPECT_FALSE(fs::exists(parked)) + << "the superseded parked copy should be discarded"; + EXPECT_EQ(read_(original / "bin" / "node"), "new") + << "recovery must never overwrite what already replaced the parked " + "payload"; + + fs::remove_all(original.parent_path(), ec); +} + +TEST(RecoverParkedPayloads, EntriesOfALiveProcessAreUntouched) { + auto stale = make_stale_root_("recover-live"); + const auto parked = stale / "xim-x-node-22.17.1.stale-1"; + std::error_code ec; + fs::create_directories(parked / "bin", ec); + std::ofstream(parked / "bin" / "node") << "old"; + + const auto original = fs::temp_directory_path() / + "xlings-payload-stale-recover-live-orig" / "22.17.1"; + fs::remove_all(original.parent_path(), ec); // absent, same as the + // crash case above + + xim::write_parked_payload_marker(parked, original, xlings::platform::get_pid()); + + EXPECT_EQ(xim::recover_parked_payloads(stale), 0) + << "an entry whose owning process is still running must not be " + "touched -- it may yet run its own rollback"; + EXPECT_TRUE(fs::exists(parked)); + EXPECT_TRUE(xim::read_parked_payload_marker(parked).has_value()); + EXPECT_FALSE(fs::exists(original)) + << "recovery must not act on this entry at all"; + + fs::remove_all(stale, ec); + fs::remove_all(original.parent_path(), ec); +} + +TEST(RecoverParkedPayloads, AnEntryWithNoMarkerIsLeftAlone) { + // Not every directory under stale/ is necessarily ours: nothing says a + // marker-less one is safe to touch, so it is left exactly as found. + auto stale = make_stale_root_("recover-no-marker"); + const auto mystery = stale / "something-else.stale-1"; + std::error_code ec; + fs::create_directories(mystery, ec); + std::ofstream(mystery / "file") << "x"; + + EXPECT_EQ(xim::recover_parked_payloads(stale), 0); + EXPECT_TRUE(fs::exists(mystery / "file")); + + fs::remove_all(stale, ec); +} + +TEST(RecoverParkedPayloads, SweepingAnAbsentStaleRootIsFine) { + EXPECT_EQ(xim::recover_parked_payloads({}), 0); + EXPECT_EQ(xim::recover_parked_payloads( + fs::temp_directory_path() / "xlings-no-such-stale-root"), + 0); +} diff --git a/tests/unit/test_semver_vectors.cpp b/tests/unit/test_semver_vectors.cpp new file mode 100644 index 000000000..51ab41bc1 --- /dev/null +++ b/tests/unit/test_semver_vectors.cpp @@ -0,0 +1,132 @@ +// Version-resolution conformance vectors (tests/data/semver-vectors.tsv). +// +// The vectors are the published statement of which version an install +// request selects; the file's header states the format and the rules. They +// are data rather than code so that a second implementation of the grammar -- +// mcpp vendors the file at the xlings version it pins -- is checked against +// the same statement instead of a transcription of it. +// +// Each vector is driven through the functions `xlings install` resolves with, +// in the order cmd_install and the resolver apply them: the active-version +// pin, the target parser, and the catalog's version selection, with the +// fallback to the unpinned target when the pinned one selects nothing. + +#include + +import std; +import mcpplibs.xpkg; +import xlings.core.xim.catalog; +import xlings.core.xim.resolver; + +namespace fs = std::filesystem; +namespace xim = xlings::xim; + +namespace { + +struct Vector { + int line { 0 }; + std::string request; + std::vector available; + std::string active; + std::string expected; +}; + +fs::path vectors_file_() { + for (const auto& candidate : { + fs::current_path() / "tests" / "data" / "semver-vectors.tsv", + fs::current_path() / ".." / ".." / "tests" / "data" / "semver-vectors.tsv", + }) { + std::error_code ec; + if (fs::is_regular_file(candidate, ec)) return candidate; + } + return {}; +} + +std::vector split_(std::string_view s, char sep) { + std::vector out; + std::size_t start = 0; + while (true) { + const auto at = s.find(sep, start); + out.emplace_back(s.substr(start, at == std::string_view::npos + ? std::string_view::npos + : at - start)); + if (at == std::string_view::npos) break; + start = at + 1; + } + return out; +} + +std::vector load_vectors_(const fs::path& file) { + std::vector vectors; + std::ifstream in(file); + std::string line; + int number = 0; + while (std::getline(in, line)) { + ++number; + if (!line.empty() && line.back() == '\r') line.pop_back(); + if (line.empty() || line.front() == '#') continue; + const auto columns = split_(line, '\t'); + if (columns.size() != 4) { + ADD_FAILURE() << file.string() << ":" << number + << ": expected 4 tab-separated columns, got " + << columns.size(); + continue; + } + vectors.push_back({ + .line = number, + .request = columns[0], + .available = split_(columns[1], ','), + .active = columns[2] == "-" ? std::string{} : columns[2], + .expected = columns[3] == "none" ? std::string{} : columns[3], + }); + } + return vectors; +} + +// The resolution `xlings install vec@` performs against a recipe +// declaring `available`, with `active` active in the workspace. +std::string resolve_(const Vector& v) { + mcpplibs::xpkg::Package pkg; + pkg.name = "vec"; + pkg.spec = "1"; + for (const auto& key : v.available) { + mcpplibs::xpkg::PlatformResource entry; + entry.url = "https://example.invalid/vec-" + key + ".tar.gz"; + pkg.xpm.entries["linux"][key] = entry; + } + + const std::string target = v.request == "-" ? "vec" : "vec@" + v.request; + const auto activeOf = [&](const std::string& name) { + return name == "vec" ? v.active : std::string{}; + }; + const auto select = [&](const std::string& t) { + const auto parsed = xim::detail_::parse_target_(t); + return xim::detail_::select_version_(pkg, "linux", parsed.version); + }; + + const auto pinned = xim::pin_target_to_subos(target, activeOf); + auto selected = select(pinned); + if (selected.empty() && pinned != target) selected = select(target); + return selected; +} + +} // namespace + +TEST(SemverVectors, EveryVectorResolvesAsStated) { + const auto file = vectors_file_(); + ASSERT_FALSE(file.empty()) + << "tests/data/semver-vectors.tsv not found from " << fs::current_path(); + + const auto vectors = load_vectors_(file); + // A floor, so a file that parsed to nothing cannot pass. + ASSERT_GE(vectors.size(), 40u) << "too few vectors read from " << file; + + for (const auto& v : vectors) { + EXPECT_EQ(resolve_(v), v.expected) + << file.filename().string() << ":" << v.line + << " request '" << v.request << "'" + << (v.active.empty() ? std::string{} : ", active " + v.active) + << " -> expected " + << (v.expected.empty() ? std::string("none") : v.expected); + } +}