feat: porting to HarmonyOS#2634
Conversation
|
Thanks @shenjackyuanjie for taking the time to contribute. This repository is currently observing a maintainer-managed contribution gate in dry-run mode, so this pull request is staying open. When enforcement is enabled, pull requests from contributors who are not listed in Please read |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request adds support for HarmonyOS (OHOS) targets by introducing custom compiler wrapper scripts, target configurations, and excluding OHOS from Linux-specific features like sandboxing, process hardening, and native clipboard integrations. Feedback focuses on improving build portability by avoiding hardcoded absolute Windows paths in .cargo/config.toml and the wrapper scripts, as well as enhancing code maintainability in clipboard.rs by abstracting the repetitive, complex platform-specific cfg attributes into a helper struct.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
| [target.aarch64-unknown-linux-ohos] | ||
| linker = "D:/githubs/hmnext/CodeWhale/ohos-clang.cmd" | ||
|
|
||
| [env] | ||
| AR_aarch64_unknown_linux_ohos = "D:/apps/DevEco Studio/app/sdk/default/openharmony/native/llvm/bin/llvm-ar.exe" | ||
| CC_aarch64_unknown_linux_ohos = "D:/githubs/hmnext/CodeWhale/ohos-clang.cmd" | ||
| CXX_aarch64_unknown_linux_ohos = "D:/githubs/hmnext/CodeWhale/ohos-clangxx.cmd" | ||
| CMAKE_TOOLCHAIN_FILE_aarch64_unknown_linux_ohos = "D:/apps/DevEco Studio/app/sdk/default/openharmony/native/build/cmake/ohos.toolchain.cmake" |
There was a problem hiding this comment.
The committed .cargo/config.toml contains hardcoded absolute Windows paths (D:/...) specific to a local development environment. This makes the build configuration non-portable and will break compilation for other developers or CI/CD pipelines attempting to build for the HarmonyOS target.
Recommended Improvements:
- Avoid committing absolute paths: Do not hardcode local directory paths in shared configuration files.
- Use environment variables: Instruct developers to set environment variables (e.g.,
CARGO_TARGET_AARCH64_UNKNOWN_LINUX_OHOS_LINKER) in their local environment instead of committing them. - Provide a template: Consider adding
.cargo/config.tomlto.gitignoreand providing a.cargo/config.toml.exampletemplate for local configuration.
| @@ -0,0 +1,3 @@ | |||
| @echo off | |||
| set SDK=D:\apps\DevEco Studio\app\sdk\default\openharmony\native | |||
There was a problem hiding this comment.
The SDK path is hardcoded to a specific local directory on Windows. To make this script portable and usable by other developers, allow overriding the SDK path via an environment variable (e.g., OHOS_NDK_HOME), while keeping the current path as a default fallback.
if "%OHOS_NDK_HOME%"=="" (set SDK=D:\\apps\\DevEco Studio\\app\\sdk\\default\\openharmony\\native) else (set SDK=%OHOS_NDK_HOME%)
| @@ -0,0 +1,3 @@ | |||
| @echo off | |||
| set SDK=D:\apps\DevEco Studio\app\sdk\default\openharmony\native | |||
There was a problem hiding this comment.
The SDK path is hardcoded to a specific local directory on Windows. To make this script portable and usable by other developers, allow overriding the SDK path via an environment variable (e.g., OHOS_NDK_HOME), while keeping the current path as a default fallback.
if "%OHOS_NDK_HOME%"=="" (set SDK=D:\\apps\\DevEco Studio\\app\\sdk\\default\\openharmony\\native) else (set SDK=%OHOS_NDK_HOME%)
| #[cfg(any( | ||
| test, | ||
| all( | ||
| not(test), | ||
| any( | ||
| target_os = "macos", | ||
| target_os = "windows", | ||
| all(target_os = "linux", not(target_env = "ohos")) | ||
| ) | ||
| ) | ||
| ))] | ||
| clipboard: Option<Clipboard>, | ||
| #[cfg(any( | ||
| test, | ||
| all( | ||
| not(test), | ||
| any( | ||
| target_os = "macos", | ||
| target_os = "windows", | ||
| all(target_os = "linux", not(target_env = "ohos")) | ||
| ) | ||
| ) | ||
| ))] | ||
| clipboard_init_attempted: bool, |
There was a problem hiding this comment.
The complex cfg condition for arboard clipboard support is repeated many times throughout this file (for imports, struct fields, initialization, and methods). This significantly reduces readability and maintainability.
Recommended Refactoring:
Abstract the system clipboard into a helper struct (e.g., SystemClipboard) with two conditional implementations:
- One using
arboardfor supported platforms. - A dummy/no-op implementation for unsupported platforms (like HarmonyOS).
This isolates the platform-specific conditional compilation to a single place and keeps ClipboardHandler clean and free of inline cfg clutter.
There was a problem hiding this comment.
Pull request overview
Note
Copilot was unable to run its full agentic suite in this review.
Adds OpenHarmony (OHOS) build support by excluding Linux-only integrations on the OHOS target and wiring up a Rustls crypto provider for reqwest after moving to rustls-no-provider.
Changes:
- Add OHOS toolchain helper scripts and a Cargo target config for
aarch64-unknown-linux-ohos. - Gate Linux-only behaviors (xdg-open, Landlock/seccomp/bwrap, prctl-based hardening, wl-copy/wl-paste, etc.) to exclude
target_env = "ohos". - Switch
reqwesttorustls-no-provider, addrustlsdependency, and install the ring provider at process startup (CLI + TUI).
Reviewed changes
Copilot reviewed 17 out of 19 changed files in this pull request and generated 7 comments.
Show a summary per file
| File | Description |
|---|---|
ohos-clang.cmd |
Adds a Windows wrapper script to invoke OHOS clang for cross-compilation. |
ohos-clangxx.cmd |
Adds a Windows wrapper script to invoke OHOS clang++ for cross-compilation. |
.cargo/config.toml |
Adds OHOS target linker + env var configuration for Cargo builds. |
.gitignore |
Un-ignores the two OHOS .cmd scripts so they can be committed. |
Cargo.toml |
Switches workspace reqwest to rustls-no-provider and adds workspace rustls config. |
crates/tui/Cargo.toml |
Updates reqwest features, adds rustls, and makes arboard target-specific (excluding OHOS). |
crates/release/Cargo.toml |
Adds rustls dependency (likely to ensure provider/code is linked consistently). |
crates/cli/Cargo.toml |
Adds rustls dependency for provider installation. |
crates/tui/src/main.rs |
Installs the Rustls ring provider at startup. |
crates/cli/src/lib.rs |
Installs the Rustls ring provider before running CLI logic. |
crates/tui/src/utils.rs |
Treats OHOS as “not Linux” for browser-open behavior and related tests. |
crates/tui/src/tui/clipboard.rs |
Excludes clipboard backends on OHOS and makes arboard/image usage conditional. |
crates/tui/src/tools/shell.rs |
Disables Linux-specific parent-death signal setup on OHOS. |
crates/tui/src/tools/diagnostics.rs |
Disables Linux-only probes (bwrap/cgroup) on OHOS. |
crates/tui/src/sandbox/mod.rs |
Excludes Linux sandbox types (Landlock/etc.) on OHOS. |
crates/tui/src/sandbox/process_hardening.rs |
Excludes Linux prctl/setrlimit hardening on OHOS. |
crates/secrets/src/lib.rs |
Excludes Linux keyring backend on OHOS via cfg gating. |
crates/secrets/Cargo.toml |
Makes keyring Linux dependency conditional on not(target_env = "ohos"). |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| fn install_rustls_crypto_provider() { | ||
| let _ = rustls::crypto::ring::default_provider().install_default(); | ||
| } |
| fn install_rustls_crypto_provider() { | ||
| let _ = rustls::crypto::ring::default_provider().install_default(); | ||
| } |
| #[cfg(any( | ||
| test, | ||
| all( | ||
| not(test), | ||
| any( | ||
| target_os = "macos", | ||
| target_os = "windows", | ||
| all(target_os = "linux", not(target_env = "ohos")) | ||
| ) | ||
| ) | ||
| ))] | ||
| use arboard::{Clipboard, ImageData}; |
| #[cfg(any( | ||
| test, | ||
| all( | ||
| not(test), | ||
| any( | ||
| target_os = "macos", | ||
| target_os = "windows", | ||
| all(target_os = "linux", not(target_env = "ohos")) | ||
| ) | ||
| ) | ||
| ))] | ||
| clipboard: Option<Clipboard>, | ||
| #[cfg(any( | ||
| test, | ||
| all( | ||
| not(test), | ||
| any( | ||
| target_os = "macos", | ||
| target_os = "windows", | ||
| all(target_os = "linux", not(target_env = "ohos")) | ||
| ) | ||
| ) | ||
| ))] | ||
| clipboard_init_attempted: bool, |
shenjackyuanjie
changed the title
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| #[cfg(not(all(target_os = "linux", not(target_env = "ohos"))))] | ||
| { | ||
| tracing::debug!("Process hardening skipped: not on Linux"); | ||
| } |
There was a problem hiding this comment.
The not(all(target_os = "linux", not(target_env = "ohos"))) guard is true when running on OHOS, because OHOS has target_os = "linux". OHOS builds will log "Process hardening skipped: not on Linux" — which is inaccurate. The message should distinguish the OHOS case.
| #[cfg(not(all(target_os = "linux", not(target_env = "ohos"))))] | |
| { | |
| tracing::debug!("Process hardening skipped: not on Linux"); | |
| } | |
| #[cfg(not(all(target_os = "linux", not(target_env = "ohos"))))] | |
| { | |
| #[cfg(all(target_os = "linux", target_env = "ohos"))] | |
| tracing::debug!("Process hardening skipped: not supported on HarmonyOS"); | |
| #[cfg(not(target_os = "linux"))] | |
| tracing::debug!("Process hardening skipped: not on Linux"); | |
| } |
|
using codex to replace hardcode path into $env |
|
Thanks @shenjackyuanjie. I saw the WIP HarmonyOS branch and your note about replacing hard-coded paths with env-driven paths. This is the right shape for the HarmonyOS work, but it touches broad build, sandbox, shell, clipboard, and dependency surfaces, so I am not pulling it into the v0.8.52 cleanup release. I will keep #2625 / this PR as the follow-up lane after 0.8.52 is published and verified. |
shenjackyuanjie
changed the title
Remove checked-in machine-specific DevEco SDK paths from Cargo and compiler wrappers. Add PowerShell and POSIX setup scripts that derive the OHOS toolchain from OHOS_NATIVE_SDK, plus HarmonyOS documentation linked from install docs and READMEs. Verified: cargo build --target aarch64-unknown-linux-ohos -p codewhale-cli passes with the local OpenHarmony SDK. codewhale-tui reaches the known rustyline/libc ioctl type mismatch on OHOS. Co-authored-by: Codex <codex@openai.com>
shenjackyuanjie
force-pushed
the
ohos-elf-port
branch
from
667c339 to
2c139b4
Compare
Summary
checklist
[x] make code in the repo compileale
[ ] patch
nixor wait for upstream update to make repo compileGreptile Summary
This PR ports CodeWhale to HarmonyOS/OpenHarmony (
aarch64-unknown-linux-ohos) by adding cross-compilation tooling and guarding Linux-specific features behindnot(target_env = \"ohos\")conditions. The previous hardcoded SDK paths are replaced with environment-variable-based setup scripts, addressing the concern raised in the prior review.reqwestfrom therustlsfeature (which pulled inaws-lc-rs/ CMake) torustls-no-providerwith an explicitring-backed provider installed at binary startup in bothcodewhale-cliandcodewhale-tui.target_env = \"ohos\"; clipboard falls back to OSC52 on OHOS; self-update returns an informative error at runtime.ohos-clang.{sh,ps1},ohos-clangxx.{sh,ps1},scripts/ohos-env.{sh,ps1}, anddocs/HarmonyOS.md; all driven by theOHOS_NATIVE_SDKenv var with no hardcoded paths.Confidence Score: 5/5
Safe to merge; no functional regressions on existing targets and OHOS falls back gracefully for unsupported features.
All Linux-specific features (sandbox, keyring, wl-copy clipboard, xdg-open, parent-death signal) are correctly guarded against the ohos target env. The crypto provider change from aws-lc-rs to ring is consistent across the workspace. No hardcoded paths remain in version-controlled config. The self-update and keyring paths return clear errors on OHOS rather than panicking or silently misbehaving. The only finding is a verbose but logically correct cfg pattern in clipboard.rs that could be simplified for readability.
crates/tui/src/tui/clipboard.rs has repeated verbose cfg expressions that could be simplified; the logic is correct but harder to audit at a glance.
Important Files Changed
rustlstorustls-no-providerand adds explicitrustlswith ring feature; removes aws-lc-rs/CMake build dependency which was blocking OHOS compilation.Flowchart
%%{init: {'theme': 'neutral'}}%% flowchart TD Build["cargo build --target aarch64-unknown-linux-ohos"] --> EnvCheck{OHOS_NATIVE_SDK set?} EnvCheck -- No --> Fail["Error: set OHOS_NATIVE_SDK"] EnvCheck -- Yes --> EnvScript["source scripts/ohos-env.sh\n(exports LINKER, AR, CC, CXX,\nCFLAGS, RUSTFLAGS, CMAKE_TOOLCHAIN)"] EnvScript --> RustBuild["rustc compiles with\ntarget_os=linux, target_env=ohos"] RustBuild --> Crypto["install_rustls_crypto_provider()\n→ ring::default_provider()"] RustBuild --> Sandbox["Sandbox: landlock/seccomp/bwrap\n disabled (cfg-gated)"] RustBuild --> Keyring["Keyring backend\n disabled → unsupported error"] RustBuild --> Clipboard["ClipboardHandler::write_text\n→ OSC52 fallback\nread() → None"] RustBuild --> Browser["browser_open_command\n→ unsupported error"] RustBuild --> SelfUpdate["run_update\n→ bail! not supported yet"]Reviews (4): Last reviewed commit: "disable selfupdate on harmonyos" | Re-trigger Greptile