Skip to content

Update workflow-js Cargo.toml for HarmonyOS build#4384

Closed
shenyongqing wants to merge 1 commit into
Hmbown:mainfrom
shenyongqing:fix-workflow-js-oh
Closed

Update workflow-js Cargo.toml for HarmonyOS build#4384
shenyongqing wants to merge 1 commit into
Hmbown:mainfrom
shenyongqing:fix-workflow-js-oh

Conversation

@shenyongqing

Copy link
Copy Markdown

rquickjs doesn't ship pre-generated bindings for HarmonyOS/OpenHarmony, need to generate there.

Summary

Testing

  • cargo fmt --all -- --check
  • cargo clippy --workspace --all-targets --all-features
  • cargo test --workspace --all-features

Checklist

  • Updated docs or comments as needed
  • Added or updated tests where relevant
  • Verified TUI behavior manually if UI changes
  • Harvested/co-authored credit uses a GitHub numeric noreply address

rquickjs doesn't ship pre-generated bindings for HarmonyOS/OpenHarmony,
need to generate there.
@shenyongqing
shenyongqing requested a review from Hmbown as a code owner July 16, 2026 08:20
@github-actions

Copy link
Copy Markdown
Contributor

Thanks @shenyongqing for taking the time to contribute.

This repository is observing a maintainer-managed PR intake gate in dry-run mode, so this pull request is staying open. This note helps maintainers prepare the allowlist before any enforcement is considered.

Please read CONTRIBUTING.md for the expected contribution shape. A maintainer can grant recurring PR access by commenting /lgtm on a pull request.

@Hmbown Hmbown added this to the v0.9.1 milestone Jul 16, 2026 — with ChatGPT Codex Connector
@Hmbown Hmbown added v0.9.1 Targeting v0.9.1 bug Something isn't working reliability Reliability, flaky behavior, retries, fallbacks, and robustness workflow-runtime Workflow IR, executor, control flow, and replay runtime labels Jul 16, 2026 — with ChatGPT Codex Connector

Hmbown commented Jul 16, 2026

Copy link
Copy Markdown
Owner

Roadmap triage: this PR is now in v0.9.1 and linked to #2970 as the rquickjs/workflow-js build slice. It does not by itself close the tier-2 target issue; CI, sandbox, clipboard, and docs work remain there.

@Hmbown

Hmbown commented Jul 16, 2026

Copy link
Copy Markdown
Owner

Thanks @shenyongqing — this is the right narrow shape for the OHOS fix, and I’ve placed it in the v0.9.1 queue.

I replayed the commit onto current main (17ac2681) and verified:

  • rustc --print cfg --target aarch64-unknown-linux-ohos reports target_env="ohos"
  • cargo fmt --all -- --check
  • cargo metadata --no-deps --format-version 1
  • cargo check -p codewhale-workflow-js
  • git diff --check origin/main...HEAD

All of those pass. Before this is merge-ready, could you please:

  1. amend the commit with a Signed-off-by trailer (git commit --amend --signoff && git push --force-with-lease), and
  2. post the exact cargo check --target aarch64-unknown-linux-ohos -p codewhale-workflow-js receipt from an environment with OHOS_NATIVE_SDK configured?

I don’t have an OHOS SDK in this review environment, so I’m treating the target check as the remaining proof rather than claiming it from the host-only build.

@Hmbown

Hmbown commented Jul 17, 2026

Copy link
Copy Markdown
Owner

Retained for v0.9.1. The six-line HarmonyOS rquickjs bindgen change is isolated and still relevant, but this 163-behind head lacks a Signed-off-by line, current-base CI, and an exact OHOS-target build receipt with OHOS_NATIVE_SDK. We should land it as a credited fresh harvest if the contributor branch cannot be updated; #2970 must remain open because this covers only the workflow-js slice.

@Hmbown

Hmbown commented Jul 17, 2026

Copy link
Copy Markdown
Owner

Hi @shenyongqing — thank you for this contribution! The approach in this PR (activating rquickjs's bindgen feature for target_env = "ohos" so QuickJS bindings are generated at build time) is exactly right, and we want to land it with your authorship fully intact.

Rather than merging this PR directly, we've carried your commit onto a maintainer replacement lane in #4459. That lane rebases your work onto current main, reruns the focused gates, and adds one SDK-free CI guard (scripts/release/check-ohos-deps.sh now proves the OHOS target keeps the rquickjs-sys bindgen feature edge, using only cargo tree metadata). Your commit is preserved as-is — author Shen Yongqing <26758802+shenyongqing@users.noreply.github.com>, same change, no squash — and you're credited in the changelog, docs/CONTRIBUTORS.md, and the replacement PR body.

The replacement stays DRAFT until someone produces a real HarmonyOS SDK receipt (source scripts/ohos-env.sh && cargo check --locked --target aarch64-unknown-linux-ohos -p codewhale-workflow-js), so nothing is landing ahead of verification. This covers the workflow-js slice of #2970 only; the issue stays open.

We're leaving this PR open for the steward to close once the replacement lands. Thanks again for the clean fix! 🐋

@Hmbown

Hmbown commented Jul 17, 2026

Copy link
Copy Markdown
Owner

Closing with the lineage recorded: the OHOS bindgen approach you pioneered here has landed on main through #4470 (merged), which implements the same cfg(target_env = "ohos") bindgen targeting plus portable-pty gating and the PowerShell libclang setup, with a real-SDK check receipt.

You are credited in docs/CONTRIBUTORS.md for the original approach, and the SDK-free bindgen-edge guard from the #4459 review lane is carried forward in #4472.

The replacement was needed because the original PR required a rebase plus review-hardening; thank you for the groundwork — the HarmonyOS slice of #2970 is better for it.

@Hmbown Hmbown closed this Jul 17, 2026
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>
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

The remaining Windows final-link wrapper gap from the OHOS bindgen work is now merged in #4503 at 8ab67a0bc03ac78406677339f3e4e3714e138885. @shenyongqing’s original bindgen approach and @shenjackyuanjie’s validation remain credited; static cross-platform gates pass, with real SDK/device linking still explicitly pending.

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

Labels

bug Something isn't working reliability Reliability, flaky behavior, retries, fallbacks, and robustness v0.9.1 Targeting v0.9.1 workflow-runtime Workflow IR, executor, control flow, and replay runtime

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants