Skip to content

fix(ohos): generate QuickJS bindings and gate unsupported PTY tools#4470

Merged
Hmbown merged 1 commit into
Hmbown:mainfrom
shenjackyuanjie:fix/ohos
Jul 17, 2026
Merged

fix(ohos): generate QuickJS bindings and gate unsupported PTY tools#4470
Hmbown merged 1 commit into
Hmbown:mainfrom
shenjackyuanjie:fix/ohos

Conversation

@shenjackyuanjie

Copy link
Copy Markdown
Contributor

Summary

  • enable rquickjs bindgen for target_env = "ohos" and configure the OpenHarmony SDK libclang/sysroot in both environment setup scripts
  • keep portable-pty out of the OHOS dependency graph by omitting persistent terminal/* tools while retaining the non-PTY exec_shell path
  • document the bindgen setup and PTY capability difference in the HarmonyOS guide

Verification

  • cargo fmt --all -- --check
  • cargo check -p codewhale-tui --bin codewhale-tui --target aarch64-unknown-linux-ohos --locked
  • verified the OHOS all-features dependency graph contains no portable-pty/starlark/arboard/keyring/nix 0.28-0.29 chain
  • cargo test -p codewhale-tui --bin codewhale-tui --locked terminal_session
  • cargo build --release -p codewhale-cli -p codewhale-tui --target aarch64-unknown-linux-ohos --locked
  • validated both release outputs as AArch64 ELF64 binaries using /lib/ld-musl-aarch64.so.1

@shenjackyuanjie
shenjackyuanjie requested a review from Hmbown as a code owner July 17, 2026 07:37
Copilot AI review requested due to automatic review settings July 17, 2026 07:37

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

This PR improves OpenHarmony (OHOS) support by enabling rquickjs bindgen for target_env = "ohos" and ensuring OHOS builds don’t pull in unsupported PTY-related dependencies, while documenting the resulting capability differences.

Changes:

  • Configure OHOS environment setup scripts to locate the SDK libclang and provide bindgen/clang sysroot args for rquickjs-sys binding generation.
  • Enable rquickjs bindgen feature specifically for cfg(target_env = "ohos").
  • Gate PTY-backed terminal/* tools off on OHOS while keeping the non-PTY exec_shell tool path available, and document this behavior.

Reviewed changes

Copilot reviewed 6 out of 6 changed files in this pull request and generated no comments.

Show a summary per file
File Description
scripts/ohos-env.sh Adds SDK libclang discovery and exports bindgen-related environment variables for OHOS builds.
scripts/ohos-env.ps1 Ensures Windows OHOS setup validates libclang.dll and exports bindgen-related environment variables.
docs/HarmonyOS.md Documents bindgen/libclang setup and the OHOS PTY tool availability difference.
crates/workflow-js/Cargo.toml Enables rquickjs bindgen specifically on target_env = "ohos" for generated bindings.
crates/tui/src/tools/registry.rs Disables registration of persistent PTY terminal tools on OHOS via cfg(target_env = "ohos").
crates/tui/src/tools/mod.rs Excludes the terminal_session module from OHOS builds to keep PTY dependencies out of the graph.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@Hmbown Hmbown added this to the v0.9.1 milestone Jul 17, 2026
@Hmbown
Hmbown merged commit fdb72fd into Hmbown:main Jul 17, 2026
17 checks passed
pull Bot pushed a commit to TheTechOddBug/DeepSeek-TUI that referenced this pull request Jul 17, 2026
Extend check-ohos-deps.sh with a cargo-tree feature-graph assertion that
the OHOS target activates rquickjs's bindgen feature (forwarded to
rquickjs-sys) for codewhale-workflow-js — the only reason the crate
compiles for a target with no pre-generated QuickJS bindings. Pure
metadata: no SDK or target toolchain required. Positive path verified
against current main (bindgen landed in Hmbown#4470); negative path verified
against a non-OHOS target.

Carried over from the Hmbown#4459 review lane; the OHOS bindgen approach was
pioneered by @shenyongqing in Hmbown#4384.

Signed-off-by: Hunter B <hmbown@gmail.com>
pull Bot pushed a commit to TheTechOddBug/DeepSeek-TUI that referenced this pull request Jul 17, 2026
After Hmbown#4471 and Hmbown#4470 merged, the v0.9.1 contributor set grows to
include @nightt5879 (Solarized Light fix) and @shenjackyuanjie
(HarmonyOS workflow-js), with @AiurArtanis for the Solarized regression
report and @shenyongqing for the original HarmonyOS bindgen approach.

CHANGELOG.md, docs/CONTRIBUTORS.md, and web/lib/release-credits.ts are
updated together so the exact-parity website contract test keeps
passing (73/73).

Signed-off-by: Hunter B <hmbown@gmail.com>
Hmbown added a commit that referenced this pull request Jul 17, 2026
Point Cargo's Windows OHOS linker at a repository-local cmd launcher that delegates to the existing PowerShell clang wrapper. This keeps the target triple, SDK sysroot, and MUSL define on the final Rust link while preserving Cargo's arguments and the native linker exit status.

Extend the no-SDK release gate and documentation to protect the full linker and rquickjs bindgen contract. Follow-up evidence from @shenjackyuanjie's PR #4470 completes the Windows linker path alongside @shenyongqing's original bindgen approach in PR #4384.

Co-authored-by: shenjackyuanjie <54507071+shenjackyuanjie@users.noreply.github.com>
Signed-off-by: Hunter B <hmbown@gmail.com>
@Hmbown

Hmbown commented Jul 17, 2026

Copy link
Copy Markdown
Owner

Follow-up from @shenjackyuanjie’s OHOS findings is now merged on public main via #4503 at merge commit 8ab67a0bc03ac78406677339f3e4e3714e138885. Windows Cargo now selects the .cmd → PowerShell OHOS linker wrapper, which carries -target aarch64-linux-ohos, the SDK sysroot, and -D__MUSL__; the static OHOS gate covers bindgen, wrapper selection, forwarding, SDK normalization, and exit propagation. Credit for @shenjackyuanjie and @shenyongqing is preserved. Honest boundary: CI/static gates pass, but a real Windows OHOS SDK/device final link remains unverified.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants