-
Notifications
You must be signed in to change notification settings - Fork 14
feat: add iOS TTS support using ONNX Runtime #378
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
98b803f
8304cc4
0c2e5b4
c79189f
a617282
ff027db
48d3b5b
d95a2df
e472312
518a04b
2a8f319
9e47edf
7268eb7
998a539
a78ed58
51f548d
91137c6
f06513c
d146705
2faebdd
23d36a6
645d869
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -53,6 +53,63 @@ jobs: | |||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| working-directory: ./frontend | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| run: bun install | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| - name: Cache ONNX Runtime iOS build | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| uses: actions/cache@v4 | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| id: cache-onnxruntime | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| with: | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| path: | | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| frontend/src-tauri/onnxruntime-ios | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| frontend/src-tauri/onnxruntime-build | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| key: onnxruntime-ios-built-1.22.2-v1 | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| restore-keys: | | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| onnxruntime-ios-built-1.22.2- | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+56
to
+66
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Cache key should include build script hash to prevent stale builds. The cache key is completely static ( 🔧 Proposed fix: include build script hash in cache key - name: Cache ONNX Runtime iOS build
uses: actions/cache@v4
id: cache-onnxruntime
with:
path: |
frontend/src-tauri/onnxruntime-ios
frontend/src-tauri/onnxruntime-build
- key: onnxruntime-ios-built-1.22.2-v1
+ key: onnxruntime-ios-built-1.22.2-${{ hashFiles('frontend/src-tauri/scripts/build-ios-onnxruntime.sh') }}
restore-keys: |
onnxruntime-ios-built-1.22.2-📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| - name: Build ONNX Runtime for iOS from source | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| if: steps.cache-onnxruntime.outputs.cache-hit != 'true' | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| working-directory: ./frontend/src-tauri | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| run: | | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| chmod +x scripts/build-ios-onnxruntime.sh | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| ./scripts/build-ios-onnxruntime.sh 1.22.2 | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| timeout-minutes: 90 | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| - name: Verify ONNX Runtime files | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| run: | | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This verification step expects
Suggested change
Prompt To Fix With AIThis is a comment left during a code review.
Path: .github/workflows/mobile-build.yml
Line: 76:76
Comment:
This verification step expects `libonnxruntime.a`, but the download script saves files as `onnxruntime.a` (without the `lib` prefix). After fixing the script, update this to match:
```suggestion
file ${{ github.workspace }}/frontend/src-tauri/onnxruntime-ios/onnxruntime.xcframework/ios-arm64/onnxruntime.a || echo "File not found or not a valid archive"
```
How can I resolve this? If you propose a fix, please make it concise. |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| echo "Checking ONNX Runtime xcframework..." | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| ls -la ${{ github.workspace }}/frontend/src-tauri/onnxruntime-ios/onnxruntime.xcframework/ | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| ls -la ${{ github.workspace }}/frontend/src-tauri/onnxruntime-ios/onnxruntime.xcframework/ios-arm64/ | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| echo "" | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| echo "Library info:" | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| file ${{ github.workspace }}/frontend/src-tauri/onnxruntime-ios/onnxruntime.xcframework/ios-arm64/libonnxruntime.a | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| echo "" | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| echo "Checking for Abseil symbols (should be included):" | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| nm ${{ github.workspace }}/frontend/src-tauri/onnxruntime-ios/onnxruntime.xcframework/ios-arm64/libonnxruntime.a 2>/dev/null | grep -i "absl" | head -20 || echo "No abseil symbols found (they may be internal)" | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
coderabbitai[bot] marked this conversation as resolved.
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| - name: Configure Cargo for iOS ONNX Runtime | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| run: | | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| # Create cargo config with absolute paths for iOS builds | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| # This overrides ort-sys's build script to use our built-from-source library | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| WORKSPACE="${{ github.workspace }}" | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| mkdir -p "${WORKSPACE}/frontend/src-tauri/.cargo" | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| cat > "${WORKSPACE}/frontend/src-tauri/.cargo/config.toml" << EOF | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| # Auto-generated cargo config for iOS ONNX Runtime linking | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| # Uses absolute paths because xcodebuild may run cargo from different directories | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| [target.aarch64-apple-ios.onnxruntime] | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [P2] Incorrect TOML format: Section headers in TOML should be Prompt To Fix With AIThis is a comment left during a code review.
Path: .github/workflows/mobile-build.yml
Line: 97:97
Comment:
[P2] Incorrect TOML format: Section headers in TOML should be `[target.aarch64-apple-ios]`, not `[target.aarch64-apple-ios.onnxruntime]`. The `.onnxruntime` suffix makes this an invalid section name. The build script in build.rs uses standard cargo directives (`rustc-link-search`, `rustc-link-lib`) which don't require this custom section format. This config may be ignored by cargo.
How can I resolve this? If you propose a fix, please make it concise.
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Invalid Cargo config syntax: Remove Prompt To Fix With AIThis is a comment left during a code review.
Path: .github/workflows/mobile-build.yml
Line: 97:97
Comment:
Invalid Cargo config syntax: `[target.aarch64-apple-ios.onnxruntime]` is not a standard Cargo config section. Cargo only supports `[target.<triple>]` sections with keys like `rustc-link-search` and `rustc-link-lib`. The `.onnxruntime` suffix will be ignored by Cargo.
Remove `.onnxruntime` from all three target sections (lines 97, 101, 105).
How can I resolve this? If you propose a fix, please make it concise. |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| rustc-link-search = ["${WORKSPACE}/frontend/src-tauri/onnxruntime-ios/onnxruntime.xcframework/ios-arm64"] | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+88
to
+98
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The Cargo config table names Cargo config only supports specific known table names under If the intent is to configure linking for the Prompt To Fix With AIThis is a comment left during a code review.
Path: .github/workflows/mobile-build.yml
Line: 88:98
Comment:
The Cargo config table names `[target.aarch64-apple-ios.onnxruntime]`, `[target.aarch64-apple-ios-sim.onnxruntime]`, and `[target.x86_64-apple-ios.onnxruntime]` are not valid Cargo configuration syntax.
Cargo config only supports specific known table names under `[target.<triple>]`. The `.onnxruntime` suffix is not a recognized table and will be ignored by Cargo, meaning these linker settings will have no effect.
If the intent is to configure linking for the `ort` crate, this approach won't work. The linker flags in `build.rs` (which uses `cargo:rustc-link-search` and `cargo:rustc-link-lib`) are the correct approach and should be sufficient. This entire config generation step may be unnecessary.
How can I resolve this? If you propose a fix, please make it concise. |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| rustc-link-lib = ["static=onnxruntime"] | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| [target.aarch64-apple-ios-sim.onnxruntime] | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| rustc-link-search = ["${WORKSPACE}/frontend/src-tauri/onnxruntime-ios/onnxruntime.xcframework/ios-arm64-simulator"] | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [P0] Path mismatch with build script output. The build-ios-onnxruntime.sh creates Prompt To Fix With AIThis is a comment left during a code review.
Path: .github/workflows/mobile-build.yml
Line: 102:102
Comment:
[P0] Path mismatch with build script output. The build-ios-onnxruntime.sh creates `ios-arm64-simulator` but this expects `ios-arm64-simulator`. If using build script, this should be `ios-arm64-simulator` (which still needs fixing in build script), or if using setup script, keep as `ios-arm64_x86_64-simulator`.
How can I resolve this? If you propose a fix, please make it concise.
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Directory name mismatch:
Suggested change
Prompt To Fix With AIThis is a comment left during a code review.
Path: .github/workflows/mobile-build.yml
Line: 102:102
Comment:
Directory name mismatch: `build-ios-onnxruntime.sh` creates `ios-arm64-simulator` but workflow references `ios-arm64-simulator`.
```suggestion
rustc-link-search = ["${WORKSPACE}/frontend/src-tauri/onnxruntime-ios/onnxruntime.xcframework/ios-arm64-simulator"]
```
How can I resolve this? If you propose a fix, please make it concise. |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| rustc-link-lib = ["static=onnxruntime"] | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+88
to
+103
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [P1] Cargo config uses non-standard TOML section Prompt To Fix With AIThis is a comment left during a code review.
Path: .github/workflows/mobile-build.yml
Line: 88:103
Comment:
[P1] Cargo config uses non-standard TOML section `[target.*.onnxruntime]` which cargo doesn't recognize. The standard way is `[target.*.rustflags]` or environment variables. This config likely won't affect the build and the linking directives in `build.rs` will be used instead.
How can I resolve this? If you propose a fix, please make it concise. |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| [target.x86_64-apple-ios.onnxruntime] | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| rustc-link-search = ["${WORKSPACE}/frontend/src-tauri/onnxruntime-ios/onnxruntime.xcframework/ios-arm64-simulator"] | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [P0] Path mismatch. Same issue as line 102 - verify simulator directory name matches actual build output. Prompt To Fix With AIThis is a comment left during a code review.
Path: .github/workflows/mobile-build.yml
Line: 106:106
Comment:
[P0] Path mismatch. Same issue as line 102 - verify simulator directory name matches actual build output.
How can I resolve this? If you propose a fix, please make it concise.
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Same directory mismatch as line 102 - should be
Suggested change
Prompt To Fix With AIThis is a comment left during a code review.
Path: .github/workflows/mobile-build.yml
Line: 106:106
Comment:
Same directory mismatch as line 102 - should be `ios-arm64-simulator` not `ios-arm64-simulator`.
```suggestion
rustc-link-search = ["${WORKSPACE}/frontend/src-tauri/onnxruntime-ios/onnxruntime.xcframework/ios-arm64-simulator"]
```
How can I resolve this? If you propose a fix, please make it concise.
Comment on lines
+101
to
+106
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Path mismatch with build script output: These paths reference There's an inconsistency:
This will cause build failures depending on which path is used (download vs build from source). Prompt To Fix With AIThis is a comment left during a code review.
Path: .github/workflows/mobile-build.yml
Line: 101:106
Comment:
Path mismatch with build script output: These paths reference `ios-arm64-simulator`, but the `build-ios-onnxruntime.sh` script creates this directory (line 172), while `setup-ios-onnxruntime.sh` creates `ios-arm64_x86_64-simulator`. The build.rs file expects `ios-arm64_x86_64-simulator` (line 12).
There's an inconsistency:
- setup script → `ios-arm64_x86_64-simulator`
- build script → `ios-arm64-simulator`
- build.rs → `ios-arm64_x86_64-simulator`
- workflows → `ios-arm64-simulator`
This will cause build failures depending on which path is used (download vs build from source).
How can I resolve this? If you propose a fix, please make it concise. |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| rustc-link-lib = ["static=onnxruntime"] | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+88
to
+107
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [P2] This dynamically generated Prompt To Fix With AIThis is a comment left during a code review.
Path: .github/workflows/mobile-build.yml
Line: 88:107
Comment:
[P2] This dynamically generated `.cargo/config.toml` uses `[target.*.onnxruntime]` table names, but standard Cargo config uses section names without dots after the target triple. The correct format should be `[target.aarch64-apple-ios]` with `rustc-link-search` directly under it, not nested under `onnxruntime`. This non-standard syntax may be ignored by Cargo, making the configuration ineffective.
How can I resolve this? If you propose a fix, please make it concise.
Comment on lines
+97
to
+107
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The cargo config sections use an invalid format: Standard Cargo only supports
Suggested change
Additionally, line 106 has the wrong path - it should point to Prompt To Fix With AIThis is a comment left during a code review.
Path: .github/workflows/mobile-build.yml
Line: 97:107
Comment:
The cargo config sections use an invalid format: `[target.aarch64-apple-ios.onnxruntime]`. This was already noted in previous review threads. The `.onnxruntime` suffix is not a valid Cargo config section and will be ignored by Cargo.
Standard Cargo only supports `[target.<triple>]` sections. The correct format should be:
```suggestion
[target.aarch64-apple-ios]
rustc-link-search = ["${WORKSPACE}/frontend/src-tauri/onnxruntime-ios/onnxruntime.xcframework/ios-arm64"]
rustc-link-lib = ["static=onnxruntime"]
[target.aarch64-apple-ios-sim]
rustc-link-search = ["${WORKSPACE}/frontend/src-tauri/onnxruntime-ios/onnxruntime.xcframework/ios-arm64-simulator"]
rustc-link-lib = ["static=onnxruntime"]
[target.x86_64-apple-ios]
rustc-link-search = ["${WORKSPACE}/frontend/src-tauri/onnxruntime-ios/onnxruntime.xcframework/ios-arm64-simulator"]
rustc-link-lib = ["static=onnxruntime"]
```
Additionally, line 106 has the wrong path - it should point to `ios-arm64-simulator` not `ios-arm64-simulator` (matching the build-ios-onnxruntime.sh script output).
How can I resolve this? If you propose a fix, please make it concise.
Comment on lines
+97
to
+107
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Invalid Cargo config format: The custom section suffix The
Suggested change
Prompt To Fix With AIThis is a comment left during a code review.
Path: .github/workflows/mobile-build.yml
Line: 97:107
Comment:
Invalid Cargo config format: The custom section suffix `.onnxruntime` (e.g., `[target.aarch64-apple-ios.onnxruntime]`) is not valid in Cargo's config format. Standard Cargo only recognizes `[target.<triple>]` sections with keys like `rustc-link-search` and `rustc-link-lib`.
The `.onnxruntime` suffix will be silently ignored by Cargo, meaning these linker flags won't be applied, causing build failures.
```suggestion
[target.aarch64-apple-ios]
rustc-link-search = ["${WORKSPACE}/frontend/src-tauri/onnxruntime-ios/onnxruntime.xcframework/ios-arm64"]
rustc-link-lib = ["static=onnxruntime"]
[target.aarch64-apple-ios-sim]
rustc-link-search = ["${WORKSPACE}/frontend/src-tauri/onnxruntime-ios/onnxruntime.xcframework/ios-arm64-simulator"]
rustc-link-lib = ["static=onnxruntime"]
[target.x86_64-apple-ios]
rustc-link-search = ["${WORKSPACE}/frontend/src-tauri/onnxruntime-ios/onnxruntime.xcframework/ios-arm64-simulator"]
rustc-link-lib = ["static=onnxruntime"]
```
How can I resolve this? If you propose a fix, please make it concise.
Comment on lines
+97
to
+107
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Cargo config sections use invalid custom suffix Prompt To Fix With AIThis is a comment left during a code review.
Path: .github/workflows/mobile-build.yml
Line: 97:107
Comment:
Cargo config sections use invalid custom suffix `.onnxruntime` which Cargo ignores. The standard format is `[target.<triple>]` with keys like `rustc-link-search` and `rustc-link-lib`. This configuration won't affect the build—the actual linking is handled by `build.rs` (line 18-19) and the `ORT_LIB_LOCATION` environment variable (line 150). Consider removing these non-functional config sections or restructuring them to valid Cargo target configuration if they're intended to override ort-sys behavior.
How can I resolve this? If you propose a fix, please make it concise.
Comment on lines
+97
to
+107
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Invalid Cargo configuration: The Issue: Cargo only recognizes Impact: The linking configuration won't be applied, which may cause:
Suggested change
Note: Also fixed directory name to Prompt To Fix With AIThis is a comment left during a code review.
Path: .github/workflows/mobile-build.yml
Line: 97:107
Comment:
Invalid Cargo configuration: The `.onnxruntime` suffix in section names like `[target.aarch64-apple-ios.onnxruntime]` is not valid Cargo TOML syntax.
**Issue**: Cargo only recognizes `[target.<triple>]` sections with keys like `rustc-link-search` and `rustc-link-lib`. The `.onnxruntime` suffix will be ignored, causing the generated config to be ineffective.
**Impact**: The linking configuration won't be applied, which may cause:
- Build failures due to missing ONNX Runtime library
- Falls back to `ort-sys` attempting to download/build ONNX Runtime itself
```suggestion
[target.aarch64-apple-ios]
rustc-link-search = ["${WORKSPACE}/frontend/src-tauri/onnxruntime-ios/onnxruntime.xcframework/ios-arm64"]
rustc-link-lib = ["static=onnxruntime"]
[target.aarch64-apple-ios-sim]
rustc-link-search = ["${WORKSPACE}/frontend/src-tauri/onnxruntime-ios/onnxruntime.xcframework/ios-arm64_x86_64-simulator"]
rustc-link-lib = ["static=onnxruntime"]
[target.x86_64-apple-ios]
rustc-link-search = ["${WORKSPACE}/frontend/src-tauri/onnxruntime-ios/onnxruntime.xcframework/ios-arm64_x86_64-simulator"]
rustc-link-lib = ["static=onnxruntime"]
```
Note: Also fixed directory name to `ios-arm64_x86_64-simulator` to match `build.rs`.
How can I resolve this? If you propose a fix, please make it concise. |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| EOF | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| echo "Generated cargo config:" | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| cat "${WORKSPACE}/frontend/src-tauri/.cargo/config.toml" | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+87
to
+112
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Don’t blindly overwrite Suggested adjustments:
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| - name: Setup Xcode | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| uses: maxim-lobanov/setup-xcode@v1 | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| with: | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
@@ -89,6 +146,8 @@ jobs: | |||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| VITE_OPEN_SECRET_API_URL: https://enclave.trymaple.ai | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| VITE_MAPLE_BILLING_API_URL: https://billing.opensecret.cloud | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| VITE_CLIENT_ID: ba5a14b5-d915-47b1-b7b1-afda52bc5fc6 | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| # ONNX Runtime location for ort-sys crate | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| ORT_LIB_LOCATION: ${{ github.workspace }}/frontend/src-tauri/onnxruntime-ios/onnxruntime.xcframework/ios-arm64 | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The The build.rs sets
Suggested change
Prompt To Fix With AIThis is a comment left during a code review.
Path: .github/workflows/mobile-build.yml
Line: 109:109
Comment:
The `ORT_LIB_LOCATION` path is incorrect. According to the build.rs file (line 26), `ORT_LIB_LOCATION` should point to the `onnxruntime-ios` directory, not to the `ios-arm64` subdirectory within the xcframework.
The build.rs sets `ORT_LIB_LOCATION={ort_dir}` where `ort_dir = "{manifest_dir}/onnxruntime-ios"`, and then constructs the lib_path by appending the appropriate xcframework subdirectory based on the target.
```suggestion
ORT_LIB_LOCATION: ${{ github.workspace }}/frontend/src-tauri/onnxruntime-ios
```
How can I resolve this? If you propose a fix, please make it concise.
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Prompt To Fix With AIThis is a comment left during a code review.
Path: .github/workflows/mobile-build.yml
Line: 115:115
Comment:
`ORT_LIB_LOCATION` points to device-specific path (`ios-arm64`) but the workflow only builds for device, not simulator. If simulator builds are needed in the future, this will cause linker errors. The path should be consistent with what `build.rs` selects based on target (device vs simulator). Consider whether this env var is even needed since `build.rs` and `.cargo/config.toml` already handle this.
How can I resolve this? If you propose a fix, please make it concise.
Comment on lines
+149
to
+150
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [P3] Potentially incorrect ORT_LIB_LOCATION path: The environment variable points to a specific architecture directory (ios-arm64) rather than the xcframework root. The build.rs expects Prompt To Fix With AIThis is a comment left during a code review.
Path: .github/workflows/mobile-build.yml
Line: 149:150
Comment:
[P3] Potentially incorrect ORT_LIB_LOCATION path: The environment variable points to a specific architecture directory (ios-arm64) rather than the xcframework root. The build.rs expects `ORT_LIB_LOCATION` to point to the xcframework root directory (line 7 shows `format!("{}/onnxruntime-ios", manifest_dir)` and line 26 sets it to `ort_dir`). This should likely be `${{ github.workspace }}/frontend/src-tauri/onnxruntime-ios` to match build.rs expectations.
How can I resolve this? If you propose a fix, please make it concise. |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| - name: Upload iOS App | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| uses: actions/upload-artifact@v4 | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
@@ -99,7 +158,11 @@ jobs: | |||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| retention-days: 5 | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| - name: Submit to TestFlight | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| if: github.event_name == 'push' && github.ref == 'refs/heads/master' | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| # TODO: Remove ios-tts condition after PR #378 is merged | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| # For this PR, we want to test TestFlight submissions on every build | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| if: | | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| (github.event_name == 'push' && github.ref == 'refs/heads/master') || | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| (github.event_name == 'pull_request' && github.head_ref == 'ios-tts') | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| run: | | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| # Find the actual path of the IPA file | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| IPA_PATH=$(find frontend/src-tauri/gen/apple/build -name "*.ipa" | head -n 1) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -3,6 +3,12 @@ name: TestFlight on Comment | |||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| on: | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| issue_comment: | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| types: [created] | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| # TODO: Remove this push trigger after PR #378 is merged | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| # This is a temporary workaround because issue_comment workflows run from master, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| # so they don't pick up workflow changes from the PR branch | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| push: | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| branches: | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| - ios-tts | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| permissions: | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| contents: read | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
@@ -11,7 +17,8 @@ permissions: | |||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| jobs: | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| check-comment: | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| if: github.event.issue.pull_request && contains(github.event.comment.body, 'testflight build') | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| # Only run on issue_comment events, not on push | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| if: github.event_name == 'issue_comment' && github.event.issue.pull_request && contains(github.event.comment.body, 'testflight build') | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Comment on lines
19
to
+21
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Proposed fix (split push vs comment paths) jobs:
check-comment:
# Only run on issue_comment events, not on push
if: github.event_name == 'issue_comment' && github.event.issue.pull_request && contains(github.event.comment.body, 'testflight build')
runs-on: ubuntu-latest
@@
deploy-testflight:
- needs: check-comment
- # Run on push to ios-tts branch OR when triggered by comment with approval
- if: github.event_name == 'push' || needs.check-comment.outputs.should-deploy == 'true'
+ # Run on push to ios-tts branch OR when triggered by comment with approval
+ if: github.event_name == 'push' || needs.check-comment.outputs.should-deploy == 'true'
+ needs: check-comment
runs-on: macos-latest-xlargeIf this still skips on push in practice, the robust pattern is:
Also applies to: 72-75 🤖 Prompt for AI Agents |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| runs-on: ubuntu-latest | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| outputs: | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| should-deploy: ${{ steps.check.outputs.should-deploy }} | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
@@ -63,12 +70,14 @@ jobs: | |||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| deploy-testflight: | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| needs: check-comment | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| if: needs.check-comment.outputs.should-deploy == 'true' | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| # Run on push to ios-tts branch OR when triggered by comment with approval | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| if: github.event_name == 'push' || needs.check-comment.outputs.should-deploy == 'true' | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| runs-on: macos-latest-xlarge | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| steps: | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| - uses: actions/checkout@v4 | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| with: | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| ref: ${{ format('refs/pull/{0}/head', needs.check-comment.outputs.pr-number) }} | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| # For push events, use the current ref; for comment events, use the PR ref | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| ref: ${{ github.event_name == 'push' && github.ref || format('refs/pull/{0}/head', needs.check-comment.outputs.pr-number) }} | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| - name: Setup Bun | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| uses: oven-sh/setup-bun@v1 | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
@@ -111,6 +120,63 @@ jobs: | |||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| working-directory: ./frontend | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| run: bun install | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| - name: Cache ONNX Runtime iOS build | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| uses: actions/cache@v4 | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| id: cache-onnxruntime | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| with: | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| path: | | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| frontend/src-tauri/onnxruntime-ios | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| frontend/src-tauri/onnxruntime-build | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| key: onnxruntime-ios-built-1.22.2-v1 | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| restore-keys: | | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| onnxruntime-ios-built-1.22.2- | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| - name: Build ONNX Runtime for iOS from source | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| if: steps.cache-onnxruntime.outputs.cache-hit != 'true' | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| working-directory: ./frontend/src-tauri | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| run: | | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| chmod +x scripts/build-ios-onnxruntime.sh | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| ./scripts/build-ios-onnxruntime.sh 1.22.2 | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| timeout-minutes: 90 | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| - name: Verify ONNX Runtime files | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| run: | | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Same issue as in mobile-build.yml - this expects
Suggested change
Prompt To Fix With AIThis is a comment left during a code review.
Path: .github/workflows/testflight-on-comment.yml
Line: 134:134
Comment:
Same issue as in mobile-build.yml - this expects `libonnxruntime.a` but the script saves `onnxruntime.a`:
```suggestion
file ${{ github.workspace }}/frontend/src-tauri/onnxruntime-ios/onnxruntime.xcframework/ios-arm64/onnxruntime.a || echo "File not found or not a valid archive"
```
How can I resolve this? If you propose a fix, please make it concise. |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| echo "Checking ONNX Runtime xcframework..." | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| ls -la ${{ github.workspace }}/frontend/src-tauri/onnxruntime-ios/onnxruntime.xcframework/ | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| ls -la ${{ github.workspace }}/frontend/src-tauri/onnxruntime-ios/onnxruntime.xcframework/ios-arm64/ | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| echo "" | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| echo "Library info:" | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| file ${{ github.workspace }}/frontend/src-tauri/onnxruntime-ios/onnxruntime.xcframework/ios-arm64/libonnxruntime.a | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| echo "" | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| echo "Checking for Abseil symbols (should be included):" | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| nm ${{ github.workspace }}/frontend/src-tauri/onnxruntime-ios/onnxruntime.xcframework/ios-arm64/libonnxruntime.a 2>/dev/null | grep -i "absl" | head -20 || echo "No abseil symbols found (they may be internal)" | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| - name: Configure Cargo for iOS ONNX Runtime | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| run: | | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| # Create cargo config with absolute paths for iOS builds | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| # This overrides ort-sys's build script to use our built-from-source library | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| WORKSPACE="${{ github.workspace }}" | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| mkdir -p "${WORKSPACE}/frontend/src-tauri/.cargo" | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| cat > "${WORKSPACE}/frontend/src-tauri/.cargo/config.toml" << EOF | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| # Auto-generated cargo config for iOS ONNX Runtime linking | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| # Uses absolute paths because xcodebuild may run cargo from different directories | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+153
to
+163
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Same issue as mobile-build.yml: custom section Prompt To Fix With AIThis is a comment left during a code review.
Path: .github/workflows/testflight-on-comment.yml
Line: 144:154
Comment:
Same issue as mobile-build.yml: custom section `[target.aarch64-apple-ios.onnxruntime]` is not valid Cargo syntax. The `.onnxruntime` suffix will be ignored. Remove `.onnxruntime` from all three target sections.
How can I resolve this? If you propose a fix, please make it concise. |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| [target.aarch64-apple-ios.onnxruntime] | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [P2] Incorrect TOML format: Section headers in TOML should be Prompt To Fix With AIThis is a comment left during a code review.
Path: .github/workflows/testflight-on-comment.yml
Line: 155:155
Comment:
[P2] Incorrect TOML format: Section headers in TOML should be `[target.aarch64-apple-ios]`, not `[target.aarch64-apple-ios.onnxruntime]`. The `.onnxruntime` suffix makes this an invalid section name. The build script in build.rs uses standard cargo directives which don't require this custom section format. This config may be ignored by cargo.
How can I resolve this? If you propose a fix, please make it concise.
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Same invalid Cargo config syntax as mobile-build.yml - remove Prompt To Fix With AIThis is a comment left during a code review.
Path: .github/workflows/testflight-on-comment.yml
Line: 164:164
Comment:
Same invalid Cargo config syntax as mobile-build.yml - remove `.onnxruntime` suffix.
How can I resolve this? If you propose a fix, please make it concise. |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| rustc-link-search = ["${WORKSPACE}/frontend/src-tauri/onnxruntime-ios/onnxruntime.xcframework/ios-arm64"] | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+155
to
+165
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Same issue as mobile-build.yml - these Cargo config table names with Prompt To Fix With AIThis is a comment left during a code review.
Path: .github/workflows/testflight-on-comment.yml
Line: 146:156
Comment:
Same issue as mobile-build.yml - these Cargo config table names with `.onnxruntime` suffix are not valid and will be ignored by Cargo. The linker configuration in `build.rs` should be sufficient.
How can I resolve this? If you propose a fix, please make it concise. |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| rustc-link-lib = ["static=onnxruntime"] | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| [target.aarch64-apple-ios-sim.onnxruntime] | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| rustc-link-search = ["${WORKSPACE}/frontend/src-tauri/onnxruntime-ios/onnxruntime.xcframework/ios-arm64-simulator"] | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Directory mismatch - should be
Suggested change
Prompt To Fix With AIThis is a comment left during a code review.
Path: .github/workflows/testflight-on-comment.yml
Line: 169:169
Comment:
Directory mismatch - should be `ios-arm64-simulator`.
```suggestion
rustc-link-search = ["${WORKSPACE}/frontend/src-tauri/onnxruntime-ios/onnxruntime.xcframework/ios-arm64-simulator"]
```
How can I resolve this? If you propose a fix, please make it concise. |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| rustc-link-lib = ["static=onnxruntime"] | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| [target.x86_64-apple-ios.onnxruntime] | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| rustc-link-search = ["${WORKSPACE}/frontend/src-tauri/onnxruntime-ios/onnxruntime.xcframework/ios-arm64-simulator"] | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Directory mismatch - should be
Suggested change
Prompt To Fix With AIThis is a comment left during a code review.
Path: .github/workflows/testflight-on-comment.yml
Line: 173:173
Comment:
Directory mismatch - should be `ios-arm64-simulator`.
```suggestion
rustc-link-search = ["${WORKSPACE}/frontend/src-tauri/onnxruntime-ios/onnxruntime.xcframework/ios-arm64-simulator"]
```
How can I resolve this? If you propose a fix, please make it concise.
Comment on lines
+168
to
+173
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Path mismatch with build script: Same directory name inconsistency as in mobile-build.yml. The workflows reference Prompt To Fix With AIThis is a comment left during a code review.
Path: .github/workflows/testflight-on-comment.yml
Line: 168:173
Comment:
Path mismatch with build script: Same directory name inconsistency as in mobile-build.yml. The workflows reference `ios-arm64-simulator` but build.rs expects `ios-arm64_x86_64-simulator`.
How can I resolve this? If you propose a fix, please make it concise. |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| rustc-link-lib = ["static=onnxruntime"] | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+164
to
+174
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Same issue as in mobile-build.yml: The cargo config sections use an invalid format with The correct format should be:
Suggested change
Additionally, line 164 has the wrong path - it should point to Prompt To Fix With AIThis is a comment left during a code review.
Path: .github/workflows/testflight-on-comment.yml
Line: 155:165
Comment:
Same issue as in mobile-build.yml: The cargo config sections use an invalid format with `.onnxruntime` suffix which will be ignored by Cargo.
The correct format should be:
```suggestion
[target.aarch64-apple-ios]
rustc-link-search = ["${WORKSPACE}/frontend/src-tauri/onnxruntime-ios/onnxruntime.xcframework/ios-arm64"]
rustc-link-lib = ["static=onnxruntime"]
[target.aarch64-apple-ios-sim]
rustc-link-search = ["${WORKSPACE}/frontend/src-tauri/onnxruntime-ios/onnxruntime.xcframework/ios-arm64-simulator"]
rustc-link-lib = ["static=onnxruntime"]
[target.x86_64-apple-ios]
rustc-link-search = ["${WORKSPACE}/frontend/src-tauri/onnxruntime-ios/onnxruntime.xcframework/ios-arm64-simulator"]
rustc-link-lib = ["static=onnxruntime"]
```
Additionally, line 164 has the wrong path - it should point to `ios-arm64-simulator` (matching the build-ios-onnxruntime.sh script output).
How can I resolve this? If you propose a fix, please make it concise.
Comment on lines
+164
to
+174
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Invalid Cargo config format: Same issue as mobile-build.yml - the
Suggested change
Prompt To Fix With AIThis is a comment left during a code review.
Path: .github/workflows/testflight-on-comment.yml
Line: 164:174
Comment:
Invalid Cargo config format: Same issue as mobile-build.yml - the `.onnxruntime` suffix in section names like `[target.aarch64-apple-ios.onnxruntime]` is not valid Cargo syntax and will be ignored.
```suggestion
[target.aarch64-apple-ios]
rustc-link-search = ["${WORKSPACE}/frontend/src-tauri/onnxruntime-ios/onnxruntime.xcframework/ios-arm64"]
rustc-link-lib = ["static=onnxruntime"]
[target.aarch64-apple-ios-sim]
rustc-link-search = ["${WORKSPACE}/frontend/src-tauri/onnxruntime-ios/onnxruntime.xcframework/ios-arm64-simulator"]
rustc-link-lib = ["static=onnxruntime"]
[target.x86_64-apple-ios]
rustc-link-search = ["${WORKSPACE}/frontend/src-tauri/onnxruntime-ios/onnxruntime.xcframework/ios-arm64-simulator"]
rustc-link-lib = ["static=onnxruntime"]
```
How can I resolve this? If you propose a fix, please make it concise.
Comment on lines
+164
to
+174
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Same issue as mobile-build.yml—these cargo config sections with Prompt To Fix With AIThis is a comment left during a code review.
Path: .github/workflows/testflight-on-comment.yml
Line: 164:174
Comment:
Same issue as mobile-build.yml—these cargo config sections with `.onnxruntime` suffix are non-standard and will be ignored by Cargo.
How can I resolve this? If you propose a fix, please make it concise.
Comment on lines
+164
to
+174
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Invalid Cargo configuration: Same Additionally, the simulator directory path should be
Suggested change
Prompt To Fix With AIThis is a comment left during a code review.
Path: .github/workflows/testflight-on-comment.yml
Line: 164:174
Comment:
Invalid Cargo configuration: Same `.onnxruntime` suffix issue as in mobile-build.yml. The section names are invalid and will be ignored by Cargo.
Additionally, the simulator directory path should be `ios-arm64_x86_64-simulator` to match `build.rs` expectations.
```suggestion
[target.aarch64-apple-ios]
rustc-link-search = ["${WORKSPACE}/frontend/src-tauri/onnxruntime-ios/onnxruntime.xcframework/ios-arm64"]
rustc-link-lib = ["static=onnxruntime"]
[target.aarch64-apple-ios-sim]
rustc-link-search = ["${WORKSPACE}/frontend/src-tauri/onnxruntime-ios/onnxruntime.xcframework/ios-arm64_x86_64-simulator"]
rustc-link-lib = ["static=onnxruntime"]
[target.x86_64-apple-ios]
rustc-link-search = ["${WORKSPACE}/frontend/src-tauri/onnxruntime-ios/onnxruntime.xcframework/ios-arm64_x86_64-simulator"]
rustc-link-lib = ["static=onnxruntime"]
```
How can I resolve this? If you propose a fix, please make it concise. |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| EOF | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+154
to
+175
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🧩 Analysis chain🏁 Script executed: # First, find the workflow file and check all similar sections
fd -n "testflight-on-comment.yml" .
wc -l .github/workflows/testflight-on-comment.ymlRepository: OpenSecretCloud/Maple Length of output: 360 🏁 Script executed: # Verify whether any repo code parses the custom .cargo/config.toml tables
rg -n "target\.[a-zA-Z0-9_\-]+\.onnxruntime" --hidden
rg -n "rustc-link-search|rustc-link-lib" --hidden
rg -n "ORT_LIB_LOCATION" --hiddenRepository: OpenSecretCloud/Maple Length of output: 3630 🏁 Script executed: # Check for any build.rs files that might read cargo config
fd -n "build\.rs" .
# Check for references to .cargo/config or CARGO_HOME
rg -n "\.cargo/config|CARGO_HOME" --hidden -t toml -t rs -t yamlRepository: OpenSecretCloud/Maple Length of output: 322 🏁 Script executed: # Look at lines 216-218 in the workflow to see if similar code exists
sed -n '210,225p' .github/workflows/testflight-on-comment.ymlRepository: OpenSecretCloud/Maple Length of output: 901 🏁 Script executed: # Get the full build.rs file to understand the complete linking strategy
cat -n frontend/src-tauri/build.rsRepository: OpenSecretCloud/Maple Length of output: 1647 🏁 Script executed: # Check if there's any documentation about this setup
rg -n "cargo.*config|\.cargo.*config" --hidden -t mdRepository: OpenSecretCloud/Maple Length of output: 47 🌐 Web query:
💡 Result: Short answer: Yes — Cargo's config.toml supports target-specific subtables of the form [target.] (and custom target-spec paths are normalized to a triple name), but the config's conditional [target.'cfg(...)'] form has limits (notably it does not observe crate-level --features when evaluated). References:
If you want, tell me the exact triple or cfg expression you want to use and I’ll show the exact config.toml snippet. Remove or replace the non-standard The generated config file uses non-standard Cargo syntax: custom subtables like Note: The x86_64 simulator path is correct— 🤖 Prompt for AI Agents |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| echo "Generated cargo config:" | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| cat "${WORKSPACE}/frontend/src-tauri/.cargo/config.toml" | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| - name: Setup Xcode | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| uses: maxim-lobanov/setup-xcode@v1 | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| with: | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
@@ -147,6 +213,8 @@ jobs: | |||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| VITE_OPEN_SECRET_API_URL: https://enclave.trymaple.ai | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| VITE_MAPLE_BILLING_API_URL: https://billing.opensecret.cloud | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| VITE_CLIENT_ID: ba5a14b5-d915-47b1-b7b1-afda52bc5fc6 | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| # ONNX Runtime location for ort-sys crate | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| ORT_LIB_LOCATION: ${{ github.workspace }}/frontend/src-tauri/onnxruntime-ios/onnxruntime.xcframework/ios-arm64 | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The
Suggested change
Prompt To Fix With AIThis is a comment left during a code review.
Path: .github/workflows/testflight-on-comment.yml
Line: 167:167
Comment:
The `ORT_LIB_LOCATION` path is incorrect - same issue as in mobile-build.yml. According to the build.rs file (line 26), `ORT_LIB_LOCATION` should point to the `onnxruntime-ios` directory, not to the `ios-arm64` subdirectory within the xcframework.
```suggestion
ORT_LIB_LOCATION: ${{ github.workspace }}/frontend/src-tauri/onnxruntime-ios
```
How can I resolve this? If you propose a fix, please make it concise.
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Same Prompt To Fix With AIThis is a comment left during a code review.
Path: .github/workflows/testflight-on-comment.yml
Line: 173:173
Comment:
Same `ORT_LIB_LOCATION` issue as in `mobile-build.yml` - hardcoded to device path but doesn't match the dynamic selection in `build.rs`. This creates maintenance issues and potential build failures if simulator builds are ever needed.
How can I resolve this? If you propose a fix, please make it concise.
Comment on lines
+216
to
+217
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [P3] Potentially incorrect ORT_LIB_LOCATION path: The environment variable points to a specific architecture directory (ios-arm64) rather than the xcframework root. The build.rs expects Prompt To Fix With AIThis is a comment left during a code review.
Path: .github/workflows/testflight-on-comment.yml
Line: 207:208
Comment:
[P3] Potentially incorrect ORT_LIB_LOCATION path: The environment variable points to a specific architecture directory (ios-arm64) rather than the xcframework root. The build.rs expects `ORT_LIB_LOCATION` to point to the xcframework root directory (it formats paths like `{ort_dir}/onnxruntime.xcframework/ios-arm64`). This should likely be `${{ github.workspace }}/frontend/src-tauri/onnxruntime-ios` to match build.rs expectations.
How can I resolve this? If you propose a fix, please make it concise. |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| - name: Submit to TestFlight | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| run: | | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
|
|
@@ -53,5 +53,22 @@ futures-util = "0.3" | |||||
| dirs = "5.0" | ||||||
| sha2 = "0.10" | ||||||
|
|
||||||
| [target.'cfg(target_os = "ios")'.dependencies] | ||||||
| # TTS dependencies (Supertonic) - iOS | ||||||
| # We build ONNX Runtime 1.22.2 from source for iOS (see scripts/build-ios-onnxruntime.sh) | ||||||
| # We disable download-binaries and copy-dylibs since we link our own xcframework | ||||||
| # Need "std" for Error trait impl and file operations, "ndarray" for tensor creation | ||||||
| ort = { version = "2.0.0-rc.10", default-features = false, features = ["std", "ndarray"] } | ||||||
|
Comment on lines
+56
to
+61
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Comment says "We build ONNX Runtime 1.22.2" but the PR description mentions 1.20.1. Also, comment says "We disable download-binaries and copy-dylibs" but the Prompt To Fix With AIThis is a comment left during a code review.
Path: frontend/src-tauri/Cargo.toml
Line: 56:61
Comment:
Comment says "We build ONNX Runtime 1.22.2" but the PR description mentions 1.20.1. Also, comment says "We disable download-binaries and copy-dylibs" but the `ort` dependency line doesn't explicitly disable these features. Verify the comment is accurate or add explicit feature flags.
How can I resolve this? If you propose a fix, please make it concise. |
||||||
| ndarray = { version = "0.16" } | ||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. iOS build missing Prompt To Fix With AIThis is a comment left during a code review.
Path: frontend/src-tauri/Cargo.toml
Line: 62:62
Comment:
iOS build missing `rayon` feature for `ndarray` - desktop has it on line 45. Without parallel processing, TTS inference will be significantly slower on iOS (5-10x slower for tensor operations). Consider adding `features = ["rayon"]` unless there's a specific reason to exclude it.
How can I resolve this? If you propose a fix, please make it concise.
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The iOS version of ndarray is missing the "rayon" feature that is included in the desktop version (line 46). This inconsistency could cause performance degradation on iOS since parallel operations won't be available. The desktop version has: ndarray = { version = "0.16", features = ["rayon"] }But the iOS version only has: ndarray = { version = "0.16" }Impact: TTS synthesis on iOS may be significantly slower than on desktop due to lack of parallel processing via rayon.
Suggested change
Prompt To Fix With AIThis is a comment left during a code review.
Path: frontend/src-tauri/Cargo.toml
Line: 62:62
Comment:
The iOS version of ndarray is missing the "rayon" feature that is included in the desktop version (line 46). This inconsistency could cause performance degradation on iOS since parallel operations won't be available.
The desktop version has:
```toml
ndarray = { version = "0.16", features = ["rayon"] }
```
But the iOS version only has:
```toml
ndarray = { version = "0.16" }
```
**Impact**: TTS synthesis on iOS may be significantly slower than on desktop due to lack of parallel processing via rayon.
```suggestion
ndarray = { version = "0.16", features = ["rayon"] }
```
How can I resolve this? If you propose a fix, please make it concise. |
||||||
| rand = "0.8" | ||||||
| rand_distr = "0.4" | ||||||
| hound = "3.5" | ||||||
| unicode-normalization = "0.1" | ||||||
| regex = "1.10" | ||||||
| reqwest = { version = "0.12", features = ["stream"] } | ||||||
| futures-util = "0.3" | ||||||
| dirs = "5.0" | ||||||
| sha2 = "0.10" | ||||||
|
Comment on lines
+56
to
+71
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🧩 Analysis chain🌐 Web query:
💡 Result: Short answer: 2.0.0-rc.10 is a release-candidate (pre-release), not a GA release—so it’s not recommended as a drop-in production build for iOS without your own validation and testing. The ort 2.0.0-rc.10 Rust wrapper exists, and ONNX Runtime provides iOS build instructions and supported iOS platforms, but RCs can contain breaking changes or unresolved bugs and should be treated as test/pre‑production. [1][2] References
🌐 Web query:
💡 Result: As of Jan 9, 2026 the latest stable ONNX Runtime release is 1.23.2 (released Oct 2025). It supports iOS (arm64 devices and x86_64 simulator); the docs list iOS 12 as the tested minimum and note tvOS/watchOS and 32-bit architectures are not supported. [1][2] Sources:
🌐 Web query:
💡 Result: Short answer: There is no ONNX Runtime "2.0.0 final" release or official iOS xcframework available (ONNX Runtime releases are still in the 1.x series). See the official releases page and the iOS build docs. [1][2] If you need an iOS xcframework now, you can either:
References
Would you like step-by-step commands to build an iOS xcframework from the current ONNX Runtime release? Replace ONNX Runtime release candidate with a stable version. Using Either upgrade to the latest stable release or document the specific reason this RC version is necessary for iOS compatibility. 🤖 Prompt for AI Agents |
||||||
|
|
||||||
| [target.'cfg(target_os = "android")'.dependencies] | ||||||
| openssl = { version = "0.10", default-features = false, features = ["vendored"] } | ||||||
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -1,3 +1,33 @@ | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| fn main() { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| // iOS-specific build configuration for ONNX Runtime | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| #[cfg(target_os = "ios")] | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| // Get the path to the ONNX Runtime xcframework | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| let manifest_dir = std::env::var("CARGO_MANIFEST_DIR").unwrap(); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Using Recommendation: Use Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time! Prompt To Fix With AIThis is a comment left during a code review.
Path: frontend/src-tauri/build.rs
Line: 6:6
Comment:
Using `.unwrap()` in build scripts provides poor error messages when failures occur. While `CARGO_MANIFEST_DIR` should always be set during builds, if it's not set for any reason, the build will panic with an unhelpful error message.
**Recommendation**: Use `.expect("CARGO_MANIFEST_DIR not set")` to provide a more descriptive error message, or use `.context()` from anyhow if error handling is needed.
<sub>Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!</sub>
How can I resolve this? If you propose a fix, please make it concise.
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The
Suggested change
Prompt To Fix With AIThis is a comment left during a code review.
Path: frontend/src-tauri/build.rs
Line: 6:6
Comment:
The `unwrap()` call on line 6 will panic if `CARGO_MANIFEST_DIR` is not set. This is a build-time failure that will cause confusing error messages. Should use `expect()` with a descriptive message or handle the error gracefully.
```suggestion
let manifest_dir = std::env::var("CARGO_MANIFEST_DIR").expect("CARGO_MANIFEST_DIR not set - this is required by Cargo");
```
How can I resolve this? If you propose a fix, please make it concise. |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| let ort_dir = format!("{}/onnxruntime-ios", manifest_dir); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| // Check if building for simulator or device | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| let target = std::env::var("TARGET").unwrap_or_default(); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| let lib_path = if target.contains("sim") || target.contains("x86_64") { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| format!("{}/onnxruntime.xcframework/ios-arm64_x86_64-simulator", ort_dir) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The simulator xcframework path Impact: iOS simulator builds will fail with a linker error because the library cannot be found at this path. Root cause: The Fix needed: Change this line to:
Suggested change
Also update the GitHub workflows (mobile-build.yml lines 102, 106 and testflight-on-comment.yml lines 160, 164) to use Prompt To Fix With AIThis is a comment left during a code review.
Path: frontend/src-tauri/build.rs
Line: 12:12
Comment:
The simulator xcframework path `ios-arm64_x86_64-simulator` in build.rs does not match the actual path created by `build-ios-onnxruntime.sh` which creates `ios-arm64-simulator`. This will cause build failures when building for iOS simulator.
**Impact**: iOS simulator builds will fail with a linker error because the library cannot be found at this path.
**Root cause**: The `build-ios-onnxruntime.sh` script (line 205) creates the directory as `ios-arm64-simulator`, but this build.rs expects `ios-arm64_x86_64-simulator` (matching the setup script but not the build script).
**Fix needed**: Change this line to:
```suggestion
format!("{}/onnxruntime.xcframework/ios-arm64-simulator", ort_dir)
```
Also update the GitHub workflows (mobile-build.yml lines 102, 106 and testflight-on-comment.yml lines 160, 164) to use `ios-arm64-simulator` instead of `ios-arm64-simulator` in the cargo config generation.
How can I resolve this? If you propose a fix, please make it concise.
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [P0] Directory name mismatch will cause simulator builds to fail. The build script at line 172 creates
Suggested change
Prompt To Fix With AIThis is a comment left during a code review.
Path: frontend/src-tauri/build.rs
Line: 12:12
Comment:
[P0] Directory name mismatch will cause simulator builds to fail. The build script at line 172 creates `ios-arm64-simulator` but this code expects `ios-arm64_x86_64-simulator`. Change to:
```suggestion
format!("{}/onnxruntime.xcframework/ios-arm64-simulator", ort_dir)
```
How can I resolve this? If you propose a fix, please make it concise.
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Directory name mismatch:
Suggested change
Prompt To Fix With AIThis is a comment left during a code review.
Path: frontend/src-tauri/build.rs
Line: 12:12
Comment:
Directory name mismatch: `build-ios-onnxruntime.sh` creates `ios-arm64-simulator` but `build.rs` expects `ios-arm64_x86_64-simulator`.
```suggestion
format!("{}/onnxruntime.xcframework/ios-arm64-simulator", ort_dir)
```
How can I resolve this? If you propose a fix, please make it concise.
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Critical path mismatch: This code expects This will cause linker failures when building from source (cache miss) because the library won't be found at the expected path.
Suggested change
Prompt To Fix With AIThis is a comment left during a code review.
Path: frontend/src-tauri/build.rs
Line: 12:12
Comment:
Critical path mismatch: This code expects `ios-arm64_x86_64-simulator` but the build script (`build-ios-onnxruntime.sh`) creates `ios-arm64-simulator` (line 172). The setup script downloads to `ios-arm64_x86_64-simulator` which matches this code, but the build script uses a different directory name.
This will cause linker failures when building from source (cache miss) because the library won't be found at the expected path.
```suggestion
format!("{}/onnxruntime.xcframework/ios-arm64-simulator", ort_dir)
```
How can I resolve this? If you propose a fix, please make it concise. |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } else { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| format!("{}/onnxruntime.xcframework/ios-arm64", ort_dir) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+11
to
+14
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [P2] Simulator path inconsistency: Prompt To Fix With AIThis is a comment left during a code review.
Path: frontend/src-tauri/build.rs
Line: 11:14
Comment:
[P2] Simulator path inconsistency: `build.rs` uses `ios-arm64_x86_64-simulator` for sim/x86_64 targets, but `build-ios-onnxruntime.sh` creates only `ios-arm64-simulator`. This mismatch will cause linker failures when building for x86_64 simulator. Either update `build.rs` to use `ios-arm64-simulator` or update the build script to create the `ios-arm64_x86_64-simulator` directory.
How can I resolve this? If you propose a fix, please make it concise. |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| }; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+6
to
+15
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Missing validation: No check if xcframework exists before setting linker flags. If download step fails silently or is skipped, the build fails with cryptic linker errors. Consider adding a check with a helpful error message: let ort_dir = format!("{}/onnxruntime-ios", manifest_dir);
let xcframework_path = format!("{}/onnxruntime.xcframework", ort_dir);
if !std::path::Path::new(&xcframework_path).exists() {
panic!("ONNX Runtime xcframework not found at {}. Run scripts/setup-ios-onnxruntime.sh first.", xcframework_path);
}Prompt To Fix With AIThis is a comment left during a code review.
Path: frontend/src-tauri/build.rs
Line: 6:15
Comment:
Missing validation: No check if xcframework exists before setting linker flags. If download step fails silently or is skipped, the build fails with cryptic linker errors. Consider adding a check with a helpful error message:
```rust
let ort_dir = format!("{}/onnxruntime-ios", manifest_dir);
let xcframework_path = format!("{}/onnxruntime.xcframework", ort_dir);
if !std::path::Path::new(&xcframework_path).exists() {
panic!("ONNX Runtime xcframework not found at {}. Run scripts/setup-ios-onnxruntime.sh first.", xcframework_path);
}
```
How can I resolve this? If you propose a fix, please make it concise.
Comment on lines
+3
to
+15
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The build script doesn't verify that the ONNX Runtime directory exists before attempting to use it. If the setup script fails or hasn't been run, this will cause cryptic linker errors that are difficult to debug. Consider adding a check with a helpful error message:
Suggested change
Prompt To Fix With AIThis is a comment left during a code review.
Path: frontend/src-tauri/build.rs
Line: 3:15
Comment:
The build script doesn't verify that the ONNX Runtime directory exists before attempting to use it. If the setup script fails or hasn't been run, this will cause cryptic linker errors that are difficult to debug.
Consider adding a check with a helpful error message:
```suggestion
#[cfg(target_os = "ios")]
{
// Get the path to the ONNX Runtime xcframework
let manifest_dir = std::env::var("CARGO_MANIFEST_DIR").unwrap();
let ort_dir = format!("{}/onnxruntime-ios", manifest_dir);
// Verify the xcframework exists before proceeding
let xcframework_path = format!("{}/onnxruntime.xcframework", ort_dir);
if !std::path::Path::new(&xcframework_path).exists() {
panic!(
"ONNX Runtime xcframework not found at: {}\n\
Please run: ./scripts/setup-ios-onnxruntime.sh",
xcframework_path
);
}
// Check if building for simulator or device
let target = std::env::var("TARGET").unwrap_or_default();
let lib_path = if target.contains("sim") || target.contains("x86_64") {
format!("{}/onnxruntime.xcframework/ios-arm64_x86_64-simulator", ort_dir)
} else {
format!("{}/onnxruntime.xcframework/ios-arm64", ort_dir)
};
```
How can I resolve this? If you propose a fix, please make it concise.
Comment on lines
+10
to
+15
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Path logic checks for "sim" or "x86_64" but maps to Prompt To Fix With AIThis is a comment left during a code review.
Path: frontend/src-tauri/build.rs
Line: 10:15
Comment:
Path logic checks for "sim" or "x86_64" but maps to `ios-arm64_x86_64-simulator` directory. However, the build script creates `ios-arm64-simulator` directory (build-ios-onnxruntime.sh:172), causing a mismatch. The setup script (setup-ios-onnxruntime.sh:49) creates `ios-arm64_x86_64-simulator` matching this code, but the build script differs. Which directory structure is correct—build script creates ios-arm64-simulator while this expects ios-arm64_x86_64-simulator?
How can I resolve this? If you propose a fix, please make it concise. |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| // Tell cargo where to find the ONNX Runtime static library | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| println!("cargo:rustc-link-search=native={}", lib_path); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| println!("cargo:rustc-link-lib=static=onnxruntime"); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+17
to
+19
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The build script tells cargo to link against the ONNX Runtime library without verifying that the path actually exists. If the Add a check to verify the library exists before proceeding:
Suggested change
Prompt To Fix With AIThis is a comment left during a code review.
Path: frontend/src-tauri/build.rs
Line: 17:19
Comment:
The build script tells cargo to link against the ONNX Runtime library without verifying that the path actually exists. If the `onnxruntime-ios` directory was not downloaded (e.g., the setup script failed silently or was skipped), the build will fail with an obscure linker error rather than a clear error message about the missing file.
Add a check to verify the library exists before proceeding:
```suggestion
// Tell cargo where to find the ONNX Runtime static library
if !std::path::Path::new(&lib_path).exists() {
panic!("ONNX Runtime library not found at {}. Did you run setup-ios-onnxruntime.sh?", lib_path);
}
println!("cargo:rustc-link-search=native={}", lib_path);
```
How can I resolve this? If you propose a fix, please make it concise. |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| // Link required iOS frameworks | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| println!("cargo:rustc-link-lib=framework=Foundation"); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| println!("cargo:rustc-link-lib=framework=Accelerate"); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| // Set ORT_LIB_LOCATION for the ort crate | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| println!("cargo:rustc-env=ORT_LIB_LOCATION={}", ort_dir); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Inconsistent Prompt To Fix With AIThis is a comment left during a code review.
Path: frontend/src-tauri/build.rs
Line: 26:26
Comment:
Inconsistent `ORT_LIB_LOCATION` configuration. This sets it to `{manifest_dir}/onnxruntime-ios` (base directory), but `.cargo/config.toml` line 11 sets it to `onnxruntime-ios/onnxruntime.xcframework/ios-arm64` (subdirectory), and GitHub Actions sets it to the full absolute path with subdirectory. The `.cargo/config.toml` value will override this, making this line ineffective. Either remove this line or ensure all three locations use consistent paths.
How can I resolve this? If you propose a fix, please make it concise.
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [P2] The Prompt To Fix With AIThis is a comment left during a code review.
Path: frontend/src-tauri/build.rs
Line: 26:26
Comment:
[P2] The `ORT_LIB_LOCATION` set here points to `onnxruntime-ios` directory, but the GitHub Actions workflow sets it to `onnxruntime-ios/onnxruntime.xcframework/ios-arm64`. The workflow environment variable will override this build.rs setting, which creates inconsistency between local and CI builds. Consider either removing this line or aligning the paths.
How can I resolve this? If you propose a fix, please make it concise.
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The The workflow's environment variable will override this build script value, but during local development builds this could cause confusion. Consider whether Prompt To Fix With AIThis is a comment left during a code review.
Path: frontend/src-tauri/build.rs
Line: 26:26
Comment:
The `ORT_LIB_LOCATION` is set to `ort_dir` (the base `onnxruntime-ios` directory), but the GitHub Actions workflow sets it to the architecture-specific path (`ios-arm64`). This inconsistency could cause issues depending on what the `ort` crate expects.
The workflow's environment variable will override this build script value, but during local development builds this could cause confusion. Consider whether `ORT_LIB_LOCATION` should point to the base directory or the architecture-specific directory, and make it consistent across build.rs and the workflows.
How can I resolve this? If you propose a fix, please make it concise. |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| // Rerun if the onnxruntime directory changes | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| println!("cargo:rerun-if-changed={}", ort_dir); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| tauri_build::build() | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[P2] Cache key references version 1.22.2 but PR description mentions 1.20.1. The build script uses 1.22.2 by default, so the cache key is correct, but the documentation is inconsistent.
Prompt To Fix With AI