Skip to content

feat(ffi): generate include/uda.h with cbindgen — Rust becomes the single source of truth - #7

Open
umbrella22 wants to merge 19 commits into
UniDesktop:developfrom
umbrella22:feat/cbindgen-generated-header
Open

umbrella22 wants to merge 19 commits into
UniDesktop:developfrom
umbrella22:feat/cbindgen-generated-header

Conversation

@umbrella22

@umbrella22 umbrella22 commented Oct 8, 2026 •

Copy link
Copy Markdown

Closes #6

概述

include/uda.h 现在是生成物:cbindgen 以 crates/uda-ffi/src/lib.rs 中导出函数(连同其文档注释)和 crates/uda-ffi/src/abi.rs 中的表面类型为输入生成头文件。Rust 源码由此成为 C-ABI 表面(签名、常量、文档)的唯一事实源——即 #6 中提出并同意的设计。

Commit 导览(建议按此顺序审阅)

  1. refactor(ffi) —— 全部 UDA_* 常量与两个托盘回调类型集中到 src/abi.rs(顺带消除 lib.rs 与 error.rs 之间的一份重复定义);回调类型改名为 C 侧名称(UdaTrayTextCallback、UdaTrayCheckboxCallback)。此 commit 不动 include/uda.h——C 侧零可见变化。
  2. docs(ffi) —— 手写头文件中更丰富的 API 文档合并进 Rust doc comment;导出函数按头文件的逻辑分组重排。函数体可证明未被改动:对 lib.rs 做剔除文档行后的行多重集 diff,结果为空。
  3. feat(ffi) —— 正式切换:cbindgen.toml(选项全部固定)、scripts/gen-header.sh(支持 --check)、重新生成的 include/uda.h、CI Cross-Compile Hygiene job 增加「重新生成 + diff」门禁、AGENTS.md 写入工作流(「永不手编 uda.h」)、CHANGELOG。
  4. fix(ffi) / docs(ffi) ×4 —— 多轮交叉验证(2 个独立验证代理 + 2 个 code-simplifier 代理,迭代至双验证器均判定 CLEAN)发现的修复,见下文。

关键配置说明

  • cpp_compat = true 是必须的:仅设 language = "c" 时 cbindgen 不会生成 #ifdef __cplusplus extern "C" 保护块,所有 C++ 使用者将无法链接。这是交叉验证第一轮抓到的问题。
  • 导出签名使用 i32(替代 c_int)并开启 usize_is_size_t = true,使生成的头文件与手写版一样写 int32_t / size_t。
  • scripts/gen-header.sh 固定 cbindgen 0.29.4(与 ci.yml 双处 pin),校验参数(拼写错误不会再落回重写分支静默覆盖头文件),并把负数 #define 值归一化为带括号形式——调用方写 -UDA_ERR_IO 仍能正确解析。
  • header_symbols.py / pe_exports.py 不变,继续作为独立的「头文件 ↔ 动态库符号」第二道门禁;新 CI 门禁关注点正交(头文件 ↔ Rust 源码)。

零语义变化证据

旧版(a74bd2c:include/uda.h)与生成版逐项比对:

  • 33 个 #define 值 —— 全部相等(7 个 UDA_SESSION_CAP_* 仅表示形式变化:0x00100000u → (1 << 20))
  • 32 个函数声明 —— 签名逐字相同
  • 2 个回调 typedef —— 类型相同(参数名被丢弃,系 cbindgen 限制)
  • extern "C" 保护块 —— 经 cpp_compat 恢复;用 C++ TU 编译验证(无 name mangling)
  • cc -Wall -Wextra -Werror -fsyntax-only 以 C 与 C++ 双模式通过(含对负数 define 取负、两个 typedef、状态码 static_assert)
  • python3 scripts/header_symbols.py include/uda.h 列出完全相同的 32 个符号
  • ./scripts/gen-header.sh --check 对已提交头文件幂等通过

主动披露的差异

  • 文档各层均承诺、代码却从未实现的行为,本次对齐(均在 CHANGELOG 披露):
    • uda_set_wallpaper 现在确定性地以 UDA_ERR_INVALID_ARGUMENT 拒绝空路径(文档一直这么承诺;此前 Windows 报 NOT_SUPPORTED,Linux 把空串透传给后端)。单元测试已钉死精确状态码。
    • uda_media_get_metadata 现按文档为未发布字段与「无播放器」写回 NULL(时长写 0)——docs/internals/media_specs.md、旧头文件、Python/Node 绑定注释均如此描述;原先的 unwrap_or_default() 写回指向空字符串的非 NULL 指针,还破坏了后端的 Ok(None) 归一化。
  • 文档纠错(旧头文件与实现不符处):uda_last_error_message 原文档称「库持有、存活到下一次调用」,实现实际每次调用交给调用者一个新分配串、读取即消费(文档已改为与代码一致);uda_tray_set_icon_rgba 不再声称 len == 0 时 data 可为 NULL;file:// URI 的接受范围限定为 Linux;Windows 托盘图标路径按 LoadImageW 实际支持的 .ico/.cur/.bmp 记载(非 .png);uda_media_send_command 的文档不再声称拒绝命令返回 UDA_ERR_NOT_SUPPORTED(拒绝实际映射为另一个负状态码,见下一点)。
  • 留给维护者决策(头文件与 media_specs.md 已刻意不站队):媒体命令被播放器拒绝时应视为成功(media_specs.md §3.4 原本的意图,Ok(()))还是错误(后端经 CommandFailed 实现)。头文件只承诺「任何非 UDA_OK 状态码都意味着未送达」。
  • 外观差异:每函数的 @return UDA_OK on success… 样板已删(序言统一说明一次)、#define 分节横幅 cbindgen 无法复现、回调 typedef 移到了函数区之前。

验证

  • cargo check --workspace --all-targets + cargo clippy --workspace --all-targets -- -D warnings(x86_64-unknown-linux-gnu,另对 uda-ffi 加验 x86_64-pc-windows-msvc):0 错误 0 警告;cargo fmt --check 干净。完整测试套件在本 PR 的 Linux/Windows CI job 中运行。
  • 交叉验证:2 个独立评审验证代理 + 2 个 code-simplifier 代理,共三轮、轮间修复(第一轮抓到缺失的 extern "C" 保护块与 uda_last_error_message 错误所有权契约——均为 P0;第三轮一致性扫描修复六项文档契约问题;残留 nit 修复后复验)。最终双验证器判定:CLEAN。

给合并者的提示

…rc/abi.rs

The C-ABI constants were scattered across four modules and duplicated
between lib.rs and error.rs (the unit tests only asserted that the
duplicates agreed). All UDA_* constants and the two tray callback types
now live in src/abi.rs, the module cbindgen will turn into the generated
include/uda.h (UniDesktop#6); the modules that merely implement the
exports import what they need from there.

The callback types are renamed to their C names (UdaTrayTextCallback,
UdaTrayCheckboxCallback) so the generated typedef matches the Rust item
verbatim. No C-visible change: include/uda.h is untouched and the
exported functions, their signatures and every constant value are
exactly as before.
…mments

include/uda.h carried the richest documentation of the C-ABI surface,
which meant every API fact was written twice and the two copies had
started to drift. The header's per-function contracts, the tray handle
lifetime and callback threading model, and the platform specifics in the
session actions are now merged into the Rust doc comments of the
exported functions, ready to be carried into the generated header.

The exported functions are also reordered to match the header's logical
grouping (appearance, wallpaper, wake locks, notifications, media,
session, tray). Function bodies are untouched: a line-multiset diff of
the file with doc comments stripped is empty.

One documentation bug is fixed while merging: the header claimed
uda_tray_set_icon_rgba() accepts a NULL data pointer when len is zero,
but the implementation has always rejected NULL unconditionally. The
merged doc states the implemented contract.
include/uda.h becomes a generated artifact: cbindgen turns the exported
functions in crates/uda-ffi/src/lib.rs and the surface types in
src/abi.rs into the header, making the Rust sources the single source of
truth for the C ABI (UniDesktop#6).

- crates/uda-ffi/cbindgen.toml: language=c, doxygen-style documentation
  carried from the Rust doc comments, UDA_H_ include guard, a DO NOT
  EDIT warning, and a preamble with the status-code convention, memory
  ownership, thread-safety and ABI-stability notes.
- scripts/gen-header.sh regenerates the header (cbindgen pinned to
  0.29.4) and supports --check for CI; it normalises negative #define
  values into parentheses so a caller's -UDA_ERR_IO keeps parsing.
- CI's Cross-Compile Hygiene job regenerates the header and fails on
  drift; header_symbols.py / pe_exports.py stay as the independent
  header-to-library symbol gates.
- Exported signatures now use i32 (instead of c_int) and usize_is_size_t
  is enabled, so the generated header keeps saying int32_t/size_t exactly
  like the hand-written one; the tray callback aliases are Option<extern
  "C" fn ...> so cbindgen emits the function-pointer typedefs.

Zero semantic change for C callers: 33 constant values, 32 function
signatures and both callback typedefs were compared old-vs-new and are
equal; scripts/header_symbols.py lists the identical 32 symbols, and the
header compiles warning-free as both C and C++.
Findings from the double-validator cross-review of the cbindgen switch:

- P0: the generated header lost the C++ guards - language=c only wraps
  the declarations in extern "C" when cpp_compat is set. The hand-written
  header always guaranteed C++ linkability; it is restored.
- P0: uda_last_error_message's merged doc kept the wrong half of a
  self-contradictory old contract. The implementation leaks a fresh
  allocation per call for the caller to free (take + CString::into_raw),
  so the doc now says exactly that, including the consume-on-read
  behaviour, instead of the false "owned by the library" claim.
- P1: an empty wallpaper path is now rejected with UDA_ERR_INVALID_ARGUMENT
  by the FFI layer, honouring what the docs always claimed but the code
  never implemented (Windows failed with NOT_SUPPORTED, Linux passed the
  empty string through). Doc updated to scope the file:// URI note to
  Linux.
- P1: the tray icon path doc claimed Windows accepts .png; LoadImageW
  handles .ico/.cur/.bmp - corrected to the implemented set, and
  uda_tray_create's doc now mentions that an empty name also selects the
  library default.
- P1/P2: documentation_style is "doxy" again (the explicit "c" setting
  had downgraded the blocks from /** */ to plain /* */), the preamble no
  longer presents the two string-returning helpers as status-code
  functions, uda_wakelock_release documents the 0-handle rejection, the
  Rust range syntax and a stray markdown bold were rewritten for the C
  audience, and four stale crate::error constant links in module docs
  point at crate::abi again.

Re-verified after the fixes: 33 defines value-equal, 32 signatures equal,
both typedefs and both extern "C" guards present; the header compiles
warning-free as C and as C++, and gen-header.sh --check is idempotent.
Round-2 review nit: uda_last_error_message returns const char * while
uda_free_string takes char *, and the regenerated doc now (correctly)
makes the free obligation explicit - tell the C caller to cast away
const at the call, matching the old header's copy-then-free intent
without resurrecting its false ownership claim.
Findings from the code-simplifier and code-review pass over the branch:

- gen-header.sh now validates arguments (a typo like --chekc used to
  fall through to rewrite mode and silently overwrite include/uda.h),
  parses the mode once, and fails with an actionable message when
  cargo install succeeds but CARGO_HOME/bin is not on PATH.
- uda_media_send_command's merged doc claimed a refusing player reports
  UDA_ERR_NOT_SUPPORTED, but the backends map refusal to CommandFailed
  (-> UDA_ERR_INTERNAL); only "no player" is -2. The old hand-written
  header carried the same false claim. The header now only promises
  what the implementation guarantees: no-player is -2 and any non-OK
  status means "not delivered". (The deeper three-way disagreement -
  spec says refusal is UDA_OK, backends return -5 - is left for a
  maintainer decision; the header no longer takes a side.)
- uda_media_get_metadata now implements the documented NULL contract:
  unpublished fields and "no player" write NULL (plus zero durations)
  instead of non-NULL pointers to empty strings. The old unwrap_or_default
  defeated the backends' Ok(None) normalization and contradicted
  docs/internals/media_specs.md and the Python/Node binding comments,
  which all describe NULL. A unit test pins the helper's behaviour.
- util.rs imports UDA_OK alongside UDA_ERR_PANIC instead of mixing an
  import with a fully qualified use of its sibling.

All gates re-run: clippy -D warnings and fmt clean on
x86_64-unknown-linux-gnu, gen-header.sh --check green, generated header
in sync.
Six minor findings from the two code-review validators, all verified
against the code before fixing:

- The metadata export's doc now also states the negative-status case
  (no media backend at all), matching the asymmetry with
  uda_media_get_status's doc.
- The preamble's memory-ownership rule now carves out
  uda_status_message's static return (must not be freed), and its
  thread-safety rule now names the tray handle registry alongside the
  wake-lock registry; the lib.rs crate doc carries the same two fixes.
- "A handle is single-use" in the tray prose read as if a handle allowed
  only one call; it means destruction is one-shot. Reworded to "can be
  destroyed exactly once".
- error.rs's module doc still said the last-error message is available
  "until the next UDA call"; reading it actually consumes it (see
  take_last_message). Reworded to the consume-on-read contract.
- media.rs's module doc said metadata crosses as "three strings plus an
  out duration"; the export carries a position slot too.
- docs/internals/media_specs.md: the metadata row showed the old
  4-parameter signature and claimed the three string pointers accept
  NULL to skip fields - the export has always rejected NULL there
  (only out_position_ms is optional). The row now matches the shipped
  contract; the send_command row no longer claims a refused command
  returns UDA_OK (the implementation maps refusal to a negative status;
  the spec no longer takes a side, matching the header).

Gates re-run: clippy -D warnings and fmt clean on
x86_64-unknown-linux-gnu, regenerated include/uda.h committed,
gen-header.sh --check green.
@SRInternet
SRInternet self-requested a review October 8, 2026 05:33
@SRInternet SRInternet added the enhancement New feature or request label Oct 8, 2026
- The tray section's Rust-facing comment still said "A handle is
  single-use" after the header prose was reworded; use the same
  "destroyed exactly once" terminology.
- media_specs.md 3.4 still asserted the refusal maps to Ok(()) and
  HRESULT failures to UdaError::Internal, both contradicting the shipped
  mapping (false and HRESULT failures alike surface as CommandFailed)
  and the neutral table row above it. State the observed behaviour and
  mark the refusal-is-success question as the deferred design decision
  it is, matching the header's neutrality.

@SRInternet SRInternet left a comment •

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

审查完毕,达成合并条件

审查过程为离线测试。由于出现问题,暂未进行实机测试。
本次审查对象为来自 @umbrella22 的 8 commits based on tag v0.2.1


存在问题(Medium or higher,建议修复)

1. uda_media_get_metadata 的"空字符串"与"未发布"不再可区分

位于 crates/uda-ffi/src/lib.rs

c_string_or_null 对 Some("") 与 None 一律返回 NULL:

fn c_string_or_null(text: Option<&str>) -> *mut c_char {
    match text {
        Some(text) if !text.is_empty() => util::c_string_from(text),
        _ => std::ptr::null_mut(),
    }
}

对照旧实现(origin/main 的 lib.rs)为 media::active_metadata()?.unwrap_or_default() 后直接 c_string_from(&metadata.title),即"无播放器"时也会写出三个指向空串的非 NULL 指针。新版改为 None 分支写三个 NULL。

旧版"无播放器"时宿主若 printf("%s", title) 会打印空串;新版得到 NULL,未做空检查的宿主将打印 (null) 或崩溃,即使 Python 侧已实测 06_media.py 通过;Node 侧同理。


本 Comment 由人工编写,审查建议中包含 AI 给予的建议。

@SRInternet

Copy link
Copy Markdown
Member

请先同步分支 develop 最新内容后继续变更,并对分支 develop 发起 PR 而不是分支 main。

…npublished

Addresses the follow-up review on PR UniDesktop#7: c_string_or_null folded
Some("") and None into the same NULL, so a player publishing an empty
title became indistinguishable from one that never published it.
MediaMetadata's string fields are now Option<String>: None means the
player does not publish the field (NULL on the wire), Some carries the
published value - including an empty one - through as a real C string.
The Windows backend maps an empty SMTC read to None because the
protocol cannot tell an absent property from an empty one, which is
now documented on the struct.
@umbrella22
umbrella22 changed the base branch from main to develop October 8, 2026 15:14
0d273aa reworded the uda_media_get_metadata doc comment on the Rust
side but shipped without the regenerated header, so the Cross-Compile
Hygiene gate fails on the merge commit. Commit the regenerated header;
the diff is the doc paragraph only - no signature or constant changes.
Validation round on d6240c1 flagged three spots still describing the
pre-0d273aa behavior:

- media_specs.md 2.3 said a missing MPRIS key equals an empty string;
  it maps to None (NULL on the C-ABI) while a published empty value
  stays an empty string - the contract section 4 already states.
- media_specs.md section 6 named metadata_from_dbus /
  duration_from_ticks, functions that do not exist; the pure parsers
  are metadata_from_dict / duration_from_micros and
  milliseconds_from_ticks.
- The python/nodejs bindings' all-empty fold comments claimed the
  library normalizes every published-empty field to NULL; since 0d273aa
  only unpublished fields are NULL, so the comments now describe both
  cases honestly. Binding behavior is unchanged.
@umbrella22

Copy link
Copy Markdown
Author

@SRInternet 本轮的唯一 Medium+ 问题(c_string_or_null 把 Some("") 与 None 一律折叠为 NULL)已在 0d273aa 修复,此前的评论(同步 develop、retarget 到 develop)也已落实:

  • MediaMetadata 三个字符串字段改为 Option<String>:None(播放器未发布该字段,如电台流无专辑)→ NULL,宿主照旧跳过;Some("")(发布为空)→ 真实空 C 串,不再被折叠。C-ABI 上"已发布为空"与"未发布"重新可区分。单元测试 metadata_fields_distinguish_published_empty_from_unpublished 钉住三种情形(NULL / 空串 / 有值)。
  • 生产者侧语义:Linux MPRIS 仅在 xesam:* 键缺席时产生 None;空 artists 数组拼接后仍是 Some("")。Windows SMTC 中空串即线上唯一"未发布"的拼写,published_or_none 将其映射为 None(含文档说明)——该侧本就不可能出现 Some("")。
  • 绑定层:python/nodejs 对 NULL 跳过、对空串照常呈现;"全空归一为 None/null"的注释已按新契约改写(行为不变——绑定仍把"无播放器 / 全部未发布 / 发布为全空"归一,示例只关心有无可展示内容)。
  • 文档三处一致:include/uda.h 媒体段落(d6240c1 补交重新生成的头文件,修复了 Cross-Compile Hygiene 门禁在合并提交上的失败)、docs/internals/media_specs.md(顺带修正 2.3 节遗留的"缺席 == 空字符串"与 §6 的旧函数名)、CHANGELOG [Unreleased] 条目。

CI:Cross-Compile Hygiene / Lint & Format / Linux / Windows 四个 job 在 c0a0f7d 全绿,mergeStateStatus = CLEAN。

@SRInternet SRInternet left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

审查完毕,达成合并条件

审查过程包含 Windows Subsystem for Linux 2 (Ubuntu) 免登录实机测试 + 本地 Linux 目标编译与 dbus 沙箱自动化测试 + cbindgen 0.29.4 头文件生成门禁实证(含反向篡改验证)
本次审查对象为来自 @umbrella22 的 8b4d53c 至 c0a0f7d based on origin/develop(包含 4 项提交 / 11 项文件变动 / +903 / −68 变更)


摘要

本轮增量是一次针对上一轮 Medium 级发现存在问题,目前剩余问题仅一项 Medium(CHANGELOG 措辞自相矛盾)与若干 Low 级规范项,均不构成合并阻塞。

存在问题 (Medium or higher, 建议检验)

1. CHANGELOG 「零语义变化」断言与同段列出的行为修正自相矛盾 (文档误导/契约范围误用)

位于 CHANGELOG.md:22-25

第 22/23 行写「对 C 调用者零语义变化 / Zero semantic change for C callers」,并随即列举「33 个常量值、32 个函数签名与 2 个回调 typedef 逐一比对等价」作为佐证。该断言只在签名、常量与 typedef 这三个维度成立,不能外推到指针语义维度。而紧接的第 24/25 行本身就列出三处行为修正,其中 uda_media_get_metadata 的输出语义经历了两次变化:origin/main 为「三个指向空串的非 NULL 指针」,上一轮中间态收敛为「三个 NULL」,本轮最终态为「未发布 → NULL,已发布但为空 → 指向 "" 的非 NULL 指针」。也就是说,一个依赖 out_title == NULL 判定「有无元数据」的 C 宿主,在 origin/main 上永远收不到 NULL,在本轮代码上对「发布空串」的播放器同样收不到 NULL——这正是必须回归的场景。

三级问题 (Low, 建议规范或修复)

1. MediaMetadata 字符串字段 Option 化属 uda-core 破坏性 API 变更,未在 CHANGELOG 单列 (契约变更/版本策略)

位于 crates/uda-core/src/media.rs:62-95

title / artist / album 由 String 变为 Option<String>,同文件内 MediaMetadata::is_empty() 亦同步由三个 is_empty() 改为三个 is_none()。任何外部 Rust 代码书写 metadata.title(期望 String 或 &str)、metadata.title.clone() 或 format!("{}", metadata.title) 都将编译失败,需改为 metadata.title.as_deref().unwrap_or("") 等适配写法。CHANGELOG 仅在「三处文档与实现不符」那段内以附带句形式提及,未作为独立条目或 Breaking 段落列出。

2. Linux 后端 album 与 title / artist 的键处理不对称,缺少注释锁定设计意图 (可维护性/误重构风险)

位于 crates/uda-platform-linux/src/media.rs metadata_from_dict() 函数体(title / artist 走 .map()、album 走 .and_then(),由 commit 0d273aa 引入,请以该 commit 的行号为准)

title 与 artist 走 .map(...),即「键存在必得 Some」,即使值类型异常也经 title_from_value 容错为 Some("");album 走 .and_then(|v| v.downcast_ref::<String>().ok()),即「键存在但类型不符」折叠为 None。这一不对称是合理设计——MPRIS 规范规定 xesam:album 为单一字符串,而 xesam:title / xesam:artist 允许数组形态,故后者需要容错转换路径、前者不需要。metadata_from_dict 的 doc comment 也已正确声明 "a present key to Some(value), even when the value is an empty string"。

3. 生成头文件总述中 "the two string-returning helpers below" 位置指代含混 (文档规范)

位于 include/uda.h:14-18 crates/uda-ffi/src/lib.rs 模块级文档段(Rust 侧为 cbindgen.toml 的 header 模板之外的 lib.rs 顶部 doc comment;头文件侧行号 14-18 已实测锁定)

总述写 "except uda_free_string, which returns nothing, and the two string-returning helpers below"。数字与语义均正确(uda_free_string 返回 void,uda_last_error_message 与 uda_status_message 返回 const char *),但 "below" 一词指向的是文件中后部(约 596 / 605 行)的两个函数,与常量区相距数百行;在生成式头文件中位置感被进一步稀释。

4. gen-header.sh 的临时文件落在仓库外,与 .tmp/ 约定不一致 (工程规范)

位于 scripts/gen-header.sh:70-71

output="$(mktemp)" 默认落在 $TMPDIR(通常 /tmp)。仓库既有约定是临时产物一律落工作区 .tmp/(该目录已在 .gitignore 中),跨出工作区写入在严格执行 shell 边界策略的开发机上会被安全闸口拦截。

5. cbindgen 对非 pub 项每次生成都打印 WARN,训练开发者忽略 stderr (构建卫生)

位于 crates/uda-cli/src/main.rs(CLI_LOCK_SECONDS 定义处) crates/uda-ffi/cbindgen.toml

cbindgen 扫描 uda-ffi 时命中 CLI_LOCK_SECONDS 但因非 pub 而跳过,输出 WARN: Skip uda-ffi::CLI_LOCK_SECONDS - (not \pub`)。cbindgen.toml未用[export] include/exclude或[parse]` 限定扫描面。

6. 头文件与符号集合的一致性保证完全依赖外层脚本,cargo test 层无防护 (门禁前移)

位于 crates/uda-ffi/src/abi.rs scripts/gen-header.sh

abi.rs 已有 status_codes_match_the_documented_values 与 capability_bits_are_distinct_powers_of_two 两条守卫 ABI 位值的测试,质量很高;但没有任何一条测试断言「头文件声明的符号集合 == crate 的 #[no_mangle] 集合」。该保证目前完全由 gen-header.sh --check 这一外层脚本承担。本轮 d6240c1 的 commit message 自陈了一个正面证据:0d273aa 改了 Rust doc comment 却忘记重生成头文件,导致 Cross-Compile Hygiene job 红灯——新门禁确实按设计生效了,但也说明纯靠人工记忆仍会漏,且这个漂移在本地 cargo test 阶段毫无察觉。


本 Comment 由 Agent 根据 code-review-guidelines 编写,审查建议中包含人工给予的建议。

当 PR 仅剩余三级问题时, Agent 将会自动合并而无需其他操作。

…hygiene

Fourth review round on PR UniDesktop#7:

- The CHANGELOG's 'zero semantic change' claim is scoped to what was
  actually compared - signatures, constants, typedefs - and now says
  explicitly that pointer semantics changed in two intentional ways;
  the MediaMetadata Option flip gets its own Breaking entry with the
  migration pattern for Rust consumers.
- metadata_from_dict's title/artist-vs-album asymmetry (tolerant
  readers for the array-legal keys, strict downcast for the
  single-string album) is deliberate per the MPRIS spec; a comment now
  locks that intent so a refactor cannot homogenise it.
- The header preamble names the two string-returning helpers instead
  of pointing at 'below' - positional prose ages badly in a generated
  file.
- gen-header.sh writes its scratch file under the workspace .tmp/
  (gitignored) instead of /var/folders/3c/bxhhfjtx4mvcfvw1br4843qr0000gn/T/, matching the repo convention.
- CLI_LOCK_SECONDS carries a cbindgen:ignore annotation, silencing the
  per-run 'Skip (not pub)' warning without changing any export.
- A cargo-test-level gate now pins the header's symbol set against the
  crate's sources: every #[no_mangle] export, pub UDA_* constant and
  callback typedef in abi.rs must appear in include/uda.h and vice
  versa, so the drift that d6240c1 shipped (and CI caught) fails in
  plain 'cargo test -p uda-ffi' too.
@umbrella22

Copy link
Copy Markdown
Author

@SRInternet 本轮 1 个 Medium 与 6 个 Low 已全部处理,提交为 82256e9:

Medium 1:CHANGELOG「零语义变化」断言收敛范围

同意该断言只在签名/常量/typedef 三个维度成立。现改写为「ABI 表面零变化(仅限签名、常量与 typedef 三个维度)」,并显式声明指针语义维度有两处有意的行为修正(uda_media_get_metadata 的 NULL 回填、uda_set_wallpaper 的空路径拒绝),指明依赖旧行为的 C 宿主必须按新契约适配、不在「零变化」承诺范围内。英文同步改写。

Low 1:Option 化单列 Breaking 条目 — 新增 ### ⚠️ Breaking 小节,双语完整说明 title/artist/album 的 String → Option<String> 变更、is_empty() 语义变化、as_deref().unwrap_or("") 适配写法与动机(C 宿主需要区分「未发布」NULL 与「发布为空」空串);原「三处文档与实现不符」条目也加了指向 Breaking 条目的交叉引用。

Low 2:album 不对称锁定设计意图 — metadata_from_dict 补注释:title/artist 走容错读取(MPRIS 允许数组形态,Chromium 系发过数组 xesam:title),album 按规范是单一字符串故严格 downcast、类型不符折叠 None;标注该不对称是有意设计。

Low 3:"below" 指代含混 — 头文件总述改为直接点名两个 helper:uda_last_error_message()(本线程最近一次失败的诊断)与 uda_status_message()(渲染任意状态码,免二次 FFI 往返),源在 cbindgen.toml preamble,头文件已重新生成。

Low 4:mktemp 落点 — gen-header.sh 的临时文件改到工作区 .tmp/(已在 .gitignore),不再写 $TMPDIR。

Low 5:cbindgen WARN — 给 CLI_LOCK_SECONDS 加 /// cbindgen:ignore 注解(并附说明:实现细节、非 C-ABI 面)。实测每次生成的 WARN 不再出现,且无需维护 include 白名单。顺带说明:该常量不能移出 uda-ffi——acquire_cli 是跨平台编译的(Windows 上 spawn 失败即返回 -2),移到 platform-linux 会破坏 Windows 构建。

Low 6:cargo-test 层符号集门禁 — abi.rs 新增 the_generated_header_declares_exactly_the_exported_symbols:扫描 src/*.rs 的全部 #[no_mangle] 导出与 abi.rs 的 pub UDA_* 常量/回调 typedef,与 include/uda.h 的函数声明、#define、typedef 双向集合比对。从此纯 cargo test -p uda-ffi 就能抓住 d6240c1 那类漂移(改了 Rust 忘了重新生成),不必等 CI 的 Cross-Compile Hygiene。开发中该测试实际抓出了跨行声明解析与 UdaStatus 误报两类问题并已修正。

版本号说明:五个 crate 与 Cargo.lock、nodejs 示例包的 0.2.2 提升已在 PR #5(21431ae)完成——两个 PR 目标同一个 v0.2.2,先合 #5、本 PR rebase 后自然继承,避免双 PR 各自 bump 造成冲突。

验证:容器内 uda-ffi 92 测试全绿(含新门禁)、gen-header.sh --check 通过、clippy -D warnings 干净、fmt 干净;CI 四个 job 在 82256e9 全绿。

Four-way cross-validation (two reviewers, four validators) over the
full delta confirmed six minors, addressed:

- The header's 'published empty stays an empty string' promise is now
  scoped: it holds on Linux; on Windows an empty read also surfaces as
  NULL because SMTC has no unset marker on the wire. media_specs.md
  gains the same qualifier in both the MPRIS and SMTC sections.
- The parity test's scanner fails with the actual cause when
  #[no_mangle] is not immediately followed by its fn line, instead of a
  misleading 're-run gen-header.sh'; its hand-rolled parser limitations
  are documented as such (all currently unreachable, all failing
  loudly, never as silent drift).
- gen-header.sh fails the generation when an unwrapped negative define
  survives the wrapping step, so a future non-decimal negative constant
  cannot ship '--...' tokens to callers silently.
- The notify doc's availability probe is labelled Rust-only; the C-ABI
  surface has no notification-availability probe.
- Simplifier pass: dict_get returns a reference, not an owned Value
  (comment was factually wrong); a dangling 'safety-classification
  gate' reference is gone; UDA_MEDIA_UNKNOWN's UDA_OK gets its
  backtick; and a pre-existing private intra-doc link in uda-core's
  notification.rs broke the rustdoc gate - now a plain code span.

Verified in containers: uda-ffi 92 passed (parity test included),
header regenerated and --check green, clippy -D warnings, fmt, and the
uda-core rustdoc --locked gate all clean.
@umbrella22

Copy link
Copy Markdown
Author

补充说明:82256e9..4162d7f 是内部交叉审核轮的修复,不是对新的外部反馈的回应。要点:头文件"已发布为空保持空字符串"的承诺补上 Windows 限定(SMTC 无线上未发布标记,空读取同样回填 NULL;media_specs 两个章节同步);奇偶校验测试的扫描器在 #[no_mangle] 后未紧跟 fn 行时改为报真实原因,并记档三处当前不可达的解析盲区(失效方向均为响亮失败);gen-header.sh 对未包裹的负数 define 增加响亮后置校验;notify 文档的可用性探测标注为仅 Rust API。全部验证门保持绿色(uda-ffi 92 / clippy / fmt / gen-header --check / CI 四 job)。

…aStatus, pinned cbindgen

- the seven UDA_SESSION_CAP_* constants now spell 1u32 << n, so cbindgen
  emits (1u << n) and a C caller's ~MASK keeps the unsigned 32-bit
  semantics the hand-written header had (regenerated include/uda.h);
- restore the uda_ffi::UdaStatus rlib re-export that the abi.rs move
  dropped - existing `use uda_ffi::UdaStatus` hosts keep compiling;
- gen-header.sh exits with an error instead of warning when the installed
  cbindgen is not the pinned version, before touching any output;
- regressions: scripts/test-header.py (C11/C++11 static asserts for the
  capability macros, the Rust-only UdaStatus alias, and the version-gate
  behaviour), a public-API integration test, and a CI step running the
  header contract on the Linux job.
@umbrella22

Copy link
Copy Markdown
Author

终审交叉验证轮(用户授权的第一性原理复审)

本轮在 4162d7f 之上做了一次全量复审(并行简化器 + 独立审查,候选逐项独立验证后修复),提交 f00aa8e,CI 4/4 通过。修复的问题:

  1. UDA_SESSION_CAP_* 宏丢失无符号类型(C 语义变化):Rust 侧 1 << n 经 cbindgen 生成 (1 << n),在 C 里是有符号 int——(uint64_t)~UDA_SESSION_CAP_LOCK 在旧头为 0x00000000fffdffff、新头为 0xfffffffffffdffff(C11 _Static_assert 实测 base 过/head 挂)。修复:七个常量改写 1u32 << n,重生成后为 (1u << n),位值不变、无符号语义恢复。
  2. uda_ffi::UdaStatus 公开重导出被意外移除:lib.rs 重构时 pub use error::{status_message, UdaStatus} 只留下了 status_message,rlib 宿主的 use uda_ffi::UdaStatus 变 E0432。已恢复;生成器确认不会把它输出成 C typedef。
  3. gen-header.sh 版本不符只警告:安装了其它版本 cbindgen 时脚本继续用错误版本生成/校验。现在版本不符直接 exit 1(在任何临时文件创建与生成调用之前,--check 与默认模式一致)。

新增回归:scripts/test-header.py(C11/C++11 静态断言锁定七个宏的类型/数值/取反扩展语义、UdaStatus 不入 C 头、stub cbindgen 验证版本门先拒绝后生成)并已接入 CI linux-test job;crates/uda-ffi/tests/public_api.rs 从 crate 外部引用 UdaStatus 防回退。

已知项(非本轮引入):本分支 cargo doc --document-private-items 在 uda-platform-windows 的模块清单链接上仍报基线告警(CI 不跑 rustdoc 门);这些修复在另一分支(PR #5)已完成,本 PR 变基后自然消解。

验证:gen-header.sh --check 通过(生成确定),C/C++ 严格语法编译、test-header.py 3/3、public_api(linux 目标)通过、Windows 交叉编译通过;CI 4/4(f00aa8e)。修复后独立复审结论 CLEAN。

@SRInternet SRInternet left a comment •

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Already to Congratulations

审查过程包含 Windows Subsystem for Linux 2 (Ubuntu) 本地 Linux 目标编译与 dbus 沙箱自动化测试 + C/C++ 多标准头文件契约实证 + cbindgen 0.29.4 头文件生成门禁实证(含版本门禁与负数定义后置校验)
本次审查对象为来自 @umbrella22 的 8b4d53c 至 d22e565 based on origin/develop(包含 10 项提交 / 14 项文件变动 / +433 / −71 变更)

摘要

本轮增量把上一轮一项 Medium(CHANGELOG「零语义变化」的范围误用)与六项 Low 中的五项逐条闭环,且闭环方式值得称赞。本轮未发现 Medium 及以上问题,仅剩余一项 Low 为可选优化 🎉

三级问题 (Low, 建议规范或修复)

  1. test-header.py 的补码期望硬编码 0xFFFFFFFF 掩码,依赖宏恰好落在低 32 位 (测试可维护性/魔数)

位于 scripts/test-header.py:46 scripts/test-header.py:18-26

第 46 行 complement = 0xFFFFFFFF ^ (1 << bit) 把七项能力宏的补码期望固定在低 32 位取反。当前七个位是 16..22,掩码正确;一旦未来新增的会话能力位越过第 31 位(例如 1u32 << 31 之上的位在 unsigned int 中已不可能,但若把能力宏整体改为 uint64_t 字面量或改用 ULL 后缀),该期望值需要同步改写,而它与第 18-26 行的 CAPABILITIES 位表分居两处,改一处漏一处不会立刻报错——只会让 _Static_assert 以一个看似合理的错误值失败,排错方向被误导。属 Nice-to-fix。


本 Comment 由 Agent 根据 code-review-guidelines 编写,审查建议中包含人工给予的建议。

Tips: 根据 code-review-guidelines ,当 PR 仅剩余等级 <= Low 的问题时,可以忽略本 Review。

@SRInternet SRInternet added the 🎉 congratulations It's coming to an end label Oct 27, 2026
Addresses the cross-validated review findings: tray menu rows use push-returned ids; media status/wakelock/command docs match the implementations; Windows media read failures degrade to Unknown/None; capability header gates pin the canonical bit map and parser shapes; malformed icon geometry reports InvalidArgument with a non-vacuous test.
@umbrella22

Copy link
Copy Markdown
Author

复审与修复

当前 head:e0e95dc。这轮把 d22e565 重新过了一遍,主要改的是 cbindgen 头文件、媒体错误语义和示例绑定。

  • 托盘菜单行改用 push() 返回的 MenuItemId,去掉 entries().last() 的并发反查,避免并发 add 时把回调挂到别的行。
  • 媒体:uda_media_get_status、send_command、Python send() 的错误码说明按 Linux/Windows 实际映射收窄;Windows 的 GetPlaybackInfo/PlaybackStatus 读取失败降级为 UDA_OK + UNKNOWN,属性读取失败降级为 Ok(None)。
  • 头文件门禁:test-header.py 改用固定的 16..22 位表,解析允许空白,并增加 C++ 回调 typedef 用例;上一轮 review 提到的 0xFFFFFFFF 硬编码已去掉。header_symbols.py 改为匹配去掉缩进后的声明。
  • uda_tray_set_icon_rgba 的畸形 geometry 统一返回 InvalidArgument;测试会断言具体的 geometry 错误消息,不再被 handle 错误掩盖。
  • 主题常量在 abi.rs 用字面量钉住 0/1/2;头文件已重新生成,gen-header.sh --check 通过。

验证:

  • 本地:Linux/Windows 双目标 check、Linux clippy -D warnings、core 58、header gate 6/6 通过。
  • 容器(dbus-run-session):core 58、ffi 95 + 1 集成、platform-linux 84。
  • CI 4/4。

合并前仍需确认:PR #5 与 #7 在 FFI/头文件上有重叠,第二个合并需要手工解冲突。

Resolves the conflicts from PR UniDesktop#5 landing in develop. Semantics follow
develop (callback gate, push-returned menu ids, watcher classification,
session probing, refusal -> UDA_ERR_NOT_SUPPORTED); structure follows this
branch (every UDA_* constant in abi.rs, header generated by cbindgen).

PR UniDesktop#7 behavior kept on top of develop: MediaMetadata Option<String> with
NULL for unpublished and an empty C string for published-empty, Windows
media read failures degrading to Unknown/None, and the FFI geometry
validation reporting UDA_ERR_INVALID_ARGUMENT. include/uda.h regenerated
from the merged sources.
The merged util.rs carried an explicit target that now resolves through the
abi re-export, which the workspace rustdoc gate rejects under -D warnings.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

🎉 congratulations It's coming to an end enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

用 cbindgen 自动生成 include/uda.h:Rust 作为 C 头文件的唯一事实源

2 participants