Fix publishing dart packages for Android - #3522
Conversation
dosubot
Bot
added
the
size:M
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughThis PR restructures the Android Flutter plugin architecture by splitting the monolithic Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested labels
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request refactors the Android implementation by splitting the monolithic sherpa_onnx_android package into architecture-specific packages (arm64, armeabi, x86, x86_64) and updates the versioning across all platform packages to 1.12.39. Feedback focuses on the need for unique namespaces and package names in the build.gradle and AndroidManifest.xml files for each new architecture-specific package to prevent build collisions when multiple architectures are bundled together.
| apply plugin: "com.android.library" | ||
|
|
||
| android { | ||
| namespace 'com.k2fsa.sherpa.onnx' |
There was a problem hiding this comment.
The namespace is set to 'com.k2fsa.sherpa.onnx', which is identical across all architecture-specific packages. This will cause a build collision (duplicate classes/resources) when multiple architectures are bundled into the same APK. Each package must have a unique namespace.
namespace 'com.k2fsa.sherpa.onnx.arm64'
| @@ -0,0 +1,3 @@ | |||
| <manifest xmlns:android="http://schemas.android.com/apk/res/android" | |||
| package="com.k2fsa.sherpa.onnx"> | |||
There was a problem hiding this comment.
The package name in the manifest should be unique for each architecture-specific package to avoid collisions. It should match the namespace defined in the corresponding build.gradle file.
| package="com.k2fsa.sherpa.onnx"> | |
| package="com.k2fsa.sherpa.onnx.arm64"> |
| apply plugin: "com.android.library" | ||
|
|
||
| android { | ||
| namespace 'com.k2fsa.sherpa.onnx' |
There was a problem hiding this comment.
The namespace is set to 'com.k2fsa.sherpa.onnx', which will collide with other architecture packages. Please use a unique namespace for this package.
namespace 'com.k2fsa.sherpa.onnx.armeabi'
| @@ -0,0 +1,3 @@ | |||
| <manifest xmlns:android="http://schemas.android.com/apk/res/android" | |||
| package="com.k2fsa.sherpa.onnx"> | |||
There was a problem hiding this comment.
The package name in the manifest should be unique for this architecture-specific package.
| package="com.k2fsa.sherpa.onnx"> | |
| package="com.k2fsa.sherpa.onnx.armeabi"> |
| apply plugin: "com.android.library" | ||
|
|
||
| android { | ||
| namespace 'com.k2fsa.sherpa.onnx' |
There was a problem hiding this comment.
The namespace is set to 'com.k2fsa.sherpa.onnx', which will collide with other architecture packages. Please use a unique namespace for this package.
namespace 'com.k2fsa.sherpa.onnx.x86'
| @@ -0,0 +1,3 @@ | |||
| <manifest xmlns:android="http://schemas.android.com/apk/res/android" | |||
| package="com.k2fsa.sherpa.onnx"> | |||
There was a problem hiding this comment.
The package name in the manifest should be unique for this architecture-specific package.
| package="com.k2fsa.sherpa.onnx"> | |
| package="com.k2fsa.sherpa.onnx.x86"> |
| apply plugin: "com.android.library" | ||
|
|
||
| android { | ||
| namespace 'com.k2fsa.sherpa.onnx' |
There was a problem hiding this comment.
The namespace is set to 'com.k2fsa.sherpa.onnx', which will collide with other architecture packages. Please use a unique namespace for this package.
namespace 'com.k2fsa.sherpa.onnx.x86_64'
| @@ -0,0 +1,3 @@ | |||
| <manifest xmlns:android="http://schemas.android.com/apk/res/android" | |||
| package="com.k2fsa.sherpa.onnx"> | |||
There was a problem hiding this comment.
The package name in the manifest should be unique for this architecture-specific package.
| package="com.k2fsa.sherpa.onnx"> | |
| package="com.k2fsa.sherpa.onnx.x86_64"> |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (3)
.github/workflows/release-dart-package.yaml (2)
443-539: Nit:# if: falsecruft across all new ABI jobs.All four new per-ABI jobs include the commented-out
# if: falsescaffold on the line after the job name. Not a defect (it matches the existing pattern in this workflow), but worth removing during a later cleanup pass so the file doesn’t accumulate dead toggles.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/release-dart-package.yaml around lines 443 - 539, Remove the leftover commented scaffold "# if: false" that appears immediately after each new per-ABI job name (e.g., sherpa_onnx_android_arm64) across the four ABI jobs; edit the workflow YAML to delete that commented line for each job so the jobs remain unchanged functionally but the file no longer contains dead toggle cruft.
919-947: Aggregator now waits on four Android publishes before the meta-package publishes.
sleep 60beforeflutter pub publishon the meta-package was already marginal with a single Android dep; with four per-ABI Android packages that all need to be indexed by pub.dev before this job can resolve them via^1.12.39, occasional 404/resolution failures become more likely if pub.dev is slow to index. Consider either bumping the sleep (e.g.sleep 180) or replacing it with a retry loop aroundflutter pub get/pub depsthat polls pub.dev for each new ABI version before publishing.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/release-dart-package.yaml around lines 919 - 947, The current "Fix version" step uses a fixed short sleep (sleep 60) which is unreliable now that the meta-package waits on four Android ABI publishes; replace the single sleep with either a longer delay (e.g. sleep 180) or, preferably, implement a retry/poll loop in the "Fix version" step that runs flutter pub get (or dart pub deps) and retries until dependencies resolve or a timeout is reached—specifically target the logic around "sleep 60" in the "Fix version" step and use commands like flutter pub get / dart pub deps to confirm the newly-published ABI versions (the ones produced by new-release.sh) are visible before proceeding to flutter pub publish of the meta-package.flutter/sherpa_onnx_android_arm64/android/build.gradle (1)
3-4: Hardcoded Mavenversion = "1.0"is orthogonal to the pub.dev package version.The
group/versionhere are only meaningful for Maven-style Android publishing (which isn’t what the pub.dev flow uses), so this isn’t a functional issue. Consider either dropping these or parameterizingversionto matchpubspec.yamlto avoid misleading metadata inside the AAR in future. Same applies to the other three per-ABI Gradle files.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@flutter/sherpa_onnx_android_arm64/android/build.gradle` around lines 3 - 4, The build.gradle files currently hardcode Maven metadata (the group property and version = "1.0") which can become misleading; either remove the group/version entries entirely from android/build.gradle (and the three per-ABI Gradle files) or parameterize the version to read from the Flutter pubspec (so the AAR version matches pub.dev) by replacing the literal version with a Gradle property or injected value; update the same changes in the other three per-ABI Gradle files to keep metadata consistent.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.github/workflows/release-dart-package.yaml:
- Around line 808-815: Rename the ambiguous x64 artifact to match the Android
x86_64 ABI: change the tarball name from android_x64.tar.bz2 to
android_x86_64.tar.bz2 and update the upload-artifact step's name from
android-x64 to android-x86-64 (the actions/upload-artifact@v4 block and the tar
cjfv invocation that currently reference android_x64 should be updated) so the
artifact, directory (jniLibs/x86_64) and package naming
(sherpa_onnx_android_x86_64) are consistent.
In `@flutter/sherpa_onnx_android_arm64/android/build.gradle`:
- Line 28: The shared namespace declaration namespace 'com.k2fsa.sherpa.onnx' is
used across the per-ABI Gradle modules and must be made unique per ABI to avoid
AGP 9.0+ conflicts; update the namespace line in each ABI module's
android/build.gradle (the file that currently contains namespace
'com.k2fsa.sherpa.onnx') to include an ABI-specific suffix (for example change
to com.k2fsa.sherpa.onnx.arm64 for the arm64 module and analogously
com.k2fsa.sherpa.onnx.armeabi, com.k2fsa.sherpa.onnx.x86,
com.k2fsa.sherpa.onnx.x86_64 for the other modules), and ensure any
corresponding manifest package attributes or code references in those modules
are updated to match the new namespace.
---
Nitpick comments:
In @.github/workflows/release-dart-package.yaml:
- Around line 443-539: Remove the leftover commented scaffold "# if: false" that
appears immediately after each new per-ABI job name (e.g.,
sherpa_onnx_android_arm64) across the four ABI jobs; edit the workflow YAML to
delete that commented line for each job so the jobs remain unchanged
functionally but the file no longer contains dead toggle cruft.
- Around line 919-947: The current "Fix version" step uses a fixed short sleep
(sleep 60) which is unreliable now that the meta-package waits on four Android
ABI publishes; replace the single sleep with either a longer delay (e.g. sleep
180) or, preferably, implement a retry/poll loop in the "Fix version" step that
runs flutter pub get (or dart pub deps) and retries until dependencies resolve
or a timeout is reached—specifically target the logic around "sleep 60" in the
"Fix version" step and use commands like flutter pub get / dart pub deps to
confirm the newly-published ABI versions (the ones produced by new-release.sh)
are visible before proceeding to flutter pub publish of the meta-package.
In `@flutter/sherpa_onnx_android_arm64/android/build.gradle`:
- Around line 3-4: The build.gradle files currently hardcode Maven metadata (the
group property and version = "1.0") which can become misleading; either remove
the group/version entries entirely from android/build.gradle (and the three
per-ABI Gradle files) or parameterize the version to read from the Flutter
pubspec (so the AAR version matches pub.dev) by replacing the literal version
with a Gradle property or injected value; update the same changes in the other
three per-ABI Gradle files to keep metadata consistent.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: c75e3f00-3297-46f8-be13-6b9422c22670
📒 Files selected for processing (33)
.github/workflows/release-dart-package.yamlflutter/notes.mdflutter/sherpa_onnx/pubspec.yamlflutter/sherpa_onnx_android/pubspec.yamlflutter/sherpa_onnx_android_arm64/README.mdflutter/sherpa_onnx_android_arm64/android/build.gradleflutter/sherpa_onnx_android_arm64/android/settings.gradleflutter/sherpa_onnx_android_arm64/android/src/main/AndroidManifest.xmlflutter/sherpa_onnx_android_arm64/android/src/main/jniLibs/arm64-v8a/.gitkeepflutter/sherpa_onnx_android_arm64/pubspec.yamlflutter/sherpa_onnx_android_armeabi/README.mdflutter/sherpa_onnx_android_armeabi/android/build.gradleflutter/sherpa_onnx_android_armeabi/android/settings.gradleflutter/sherpa_onnx_android_armeabi/android/src/main/AndroidManifest.xmlflutter/sherpa_onnx_android_armeabi/android/src/main/jniLibs/armeabi-v7a/.gitkeepflutter/sherpa_onnx_android_armeabi/pubspec.yamlflutter/sherpa_onnx_android_x86/README.mdflutter/sherpa_onnx_android_x86/android/build.gradleflutter/sherpa_onnx_android_x86/android/settings.gradleflutter/sherpa_onnx_android_x86/android/src/main/AndroidManifest.xmlflutter/sherpa_onnx_android_x86/android/src/main/jniLibs/x86/.gitkeepflutter/sherpa_onnx_android_x86/pubspec.yamlflutter/sherpa_onnx_android_x86_64/README.mdflutter/sherpa_onnx_android_x86_64/android/build.gradleflutter/sherpa_onnx_android_x86_64/android/settings.gradleflutter/sherpa_onnx_android_x86_64/android/src/main/AndroidManifest.xmlflutter/sherpa_onnx_android_x86_64/android/src/main/jniLibs/x86_64/.gitkeepflutter/sherpa_onnx_android_x86_64/pubspec.yamlflutter/sherpa_onnx_ios/pubspec.yamlflutter/sherpa_onnx_linux/pubspec.yamlflutter/sherpa_onnx_macos/pubspec.yamlflutter/sherpa_onnx_windows/pubspec.yamlscripts/dart/sherpa-onnx-pubspec.yaml
| cd /tmp | ||
| tar cjfv android_x64.tar.bz2 ./to_be_published | ||
| ls -lh *.tar.bz2 | ||
|
|
||
| - uses: actions/upload-artifact@v4 | ||
| with: | ||
| name: android-x64 | ||
| path: /tmp/android_x64.tar.bz2 |
There was a problem hiding this comment.
Inconsistent artifact naming for the x86_64 job.
The other three ABI jobs produce android_arm64.tar.bz2, android_armeabi.tar.bz2, android_x86.tar.bz2 and upload artifacts named android-arm64/android-armeabi/android-x86. This job uses android_x64.tar.bz2 / android-x64, which is ambiguous (x64 is generally understood as desktop x86_64, but here it refers to the Android x86_64 ABI whose directory is x86_64). Renaming to android_x86_64.tar.bz2 / android-x86-64 would match the job, directory, and package naming (sherpa_onnx_android_x86_64, jniLibs/x86_64).
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.github/workflows/release-dart-package.yaml around lines 808 - 815, Rename
the ambiguous x64 artifact to match the Android x86_64 ABI: change the tarball
name from android_x64.tar.bz2 to android_x86_64.tar.bz2 and update the
upload-artifact step's name from android-x64 to android-x86-64 (the
actions/upload-artifact@v4 block and the tar cjfv invocation that currently
reference android_x64 should be updated) so the artifact, directory
(jniLibs/x86_64) and package naming (sherpa_onnx_android_x86_64) are consistent.
| apply plugin: "com.android.library" | ||
|
|
||
| android { | ||
| namespace 'com.k2fsa.sherpa.onnx' |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# Find all build.gradle files for the Android ABI modules
fd -e build.gradle -p "*android*" | grep -E "(armeabi|arm64|x86)" | head -20Repository: k2-fsa/sherpa-onnx
Length of output: 350
🏁 Script executed:
# Check the namespace declarations in each ABI module
for dir in flutter/sherpa_onnx_android_*/android; do
if [ -f "$dir/build.gradle" ]; then
echo "=== $dir ==="
grep -n "namespace" "$dir/build.gradle" || echo "No namespace found"
fi
doneRepository: k2-fsa/sherpa-onnx
Length of output: 426
🌐 Web query:
Android Gradle Plugin library module namespace requirements multiple AARs same app
💡 Result:
In Android Gradle Plugin (AGP) versions before 9.0, library modules (producing AARs) could share the same namespace (previously package attribute), and the manifest merger would handle it during app builds. Starting with AGP 9.0 (released around early 2026), namespace uniqueness is strictly enforced across all modules and library dependencies, including multiple AARs. If two or more AARs declare the same namespace in their AndroidManifest.xml or build.gradle (namespace DSL), the build fails during manifest processing with an error like: "Namespace 'com.example' is used in multiple modules and/or libraries: lib1, lib2". Requirements: - Each library module must declare a unique namespace in its module-level build.gradle: android { namespace = "com.unique.lib1" } - Namespaces should match the library's root Java/Kotlin package and be unique to avoid R class conflicts. - Official docs recommend this for generating unique R classes per library (AGP 7.0+). For apps using multiple AARs with conflicting namespaces (common in third-party SDKs like TensorFlow Lite, Zebra RFID, Pangle, LeakCanary, Razorpay as reported in 2026 issues): - Ideal fix: Library authors update to unique namespaces. - Temporary workaround: Add android.uniquePackageNames=false to gradle.properties (project-level). This disables strict enforcement, allowing duplicate namespaces (legacy behavior). Confirmed working in multiple issues without runtime issues, referenced in AGP 9.0 release notes. - Alternative: Use Fused Library plugin (AGP 8.12+ preview) to merge multiple library modules into one AAR with a single namespace before publishing. - Downgrade AGP/Gradle if feasible, but not recommended long-term. This change improves build reliability and prevents subtle resource/R class conflicts but breaks apps depending on legacy multi-AAR SDKs with shared namespaces.
Citations:
- 1: https://developer.android.com/build/releases/agp-8-0-0-release-notes
- 2: https://developer.android.com/build/publish-library/prep-lib-release
- 3: AGP 9 build fails when two libs share the same namespace "org.tensorflow.lite." tensorflow/tensorflow#109508
- 4: https://developer.zebra.com/content/manifest-merger-conflict-due-duplicate-namespace-zebra-rfid-sdk-aars
- 5: [AGP 9.0 Compatibility] Namespace collision between standard-core:1.7.6 and core:1.0.4 razorpay/razorpay-android-sample-app#388
- 6: AGP 9.0+ build fails due to duplicate com.bytedance namespace in Pangle SDK (workaround confirmed) googleads/googleads-mobile-android-mediation#889
- 7: Duplicate namespace between leakcanary-android and leakcanary-android-process breaks AGP 9 builds square/leakcanary#2792
- 8: https://developer.android.com/build/releases/agp-9-0-0-release-notes#android-gradle-plugin-new-dsl
🏁 Script executed:
# Look for gradle.properties and AGP version declarations
find . -name "gradle.properties" -o -name "build.gradle" | head -5 | xargs grep -l "gradle\|agp" 2>/dev/null | head -10Repository: k2-fsa/sherpa-onnx
Length of output: 100
🏁 Script executed:
# Check for documentation about AGP compatibility
find . -name "README*" -o -name "*.md" | xargs grep -i "agp\|gradle.*plugin" 2>/dev/null | head -15Repository: k2-fsa/sherpa-onnx
Length of output: 240
🏁 Script executed:
# Search for any existing gradle.properties at project root
cat gradle.properties 2>/dev/null || echo "No gradle.properties at root"Repository: k2-fsa/sherpa-onnx
Length of output: 90
🏁 Script executed:
# Check if any build.gradle defines AGP version via wrapper or plugin version
find . -name "build.gradle" -exec grep -l "com.android.application\|com.android.library" {} \; | head -3 | xargs grep -A2 "plugins\|gradle.*9\|gradle.*8" 2>/dev/null | head -20Repository: k2-fsa/sherpa-onnx
Length of output: 1459
Shared Android namespace across all four ABI AARs will cause build failures in AGP 9.0+.
The armeabi, arm64, x86, and x86_64 Gradle modules all declare namespace 'com.k2fsa.sherpa.onnx'. Android Gradle Plugin 9.0 (released early 2026) strictly enforces unique namespaces across library modules in an app. When a consuming app depends on sherpa_onnx with all four AARs and targets AGP 9.0 or later, the manifest merger will fail with "Namespace 'com.k2fsa.sherpa.onnx' is used in multiple modules" errors. The current repo uses AGP 7.3.1 (which tolerates shared namespaces), but this becomes a blocker as consumers upgrade to newer AGP versions.
Give each per-ABI package a distinct namespace:
🛠 Suggested changes
android {
- namespace 'com.k2fsa.sherpa.onnx'
+ namespace 'com.k2fsa.sherpa.onnx.arm64'Analogously for armeabi (...armeabi), x86 (...x86), and x86_64 (...x86_64).
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| namespace 'com.k2fsa.sherpa.onnx' | |
| android { | |
| namespace 'com.k2fsa.sherpa.onnx.arm64' |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@flutter/sherpa_onnx_android_arm64/android/build.gradle` at line 28, The
shared namespace declaration namespace 'com.k2fsa.sherpa.onnx' is used across
the per-ABI Gradle modules and must be made unique per ABI to avoid AGP 9.0+
conflicts; update the namespace line in each ABI module's android/build.gradle
(the file that currently contains namespace 'com.k2fsa.sherpa.onnx') to include
an ABI-specific suffix (for example change to com.k2fsa.sherpa.onnx.arm64 for
the arm64 module and analogously com.k2fsa.sherpa.onnx.armeabi,
com.k2fsa.sherpa.onnx.x86, com.k2fsa.sherpa.onnx.x86_64 for the other modules),
and ensure any corresponding manifest package attributes or code references in
those modules are updated to match the new namespace.
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
scripts/tauri/build-tauri-vad-asr.sh.in (2)
276-295: Minor: inconsistent portable archive naming between Windows and Linux.Line 283 hardcodes
windows-x64while line 292 uses${PLATFORM}(e.g.linux-x64,linux-aarch64). Functionally fine today becausedetect_platformonly emitswindows-x64, but using${PLATFORM}on both sides keeps the convention uniform and avoids a silent divergence if Windows ARM support is added later.♻️ Proposed tweak
- zip -r "${ARTIFACT_PREFIX}-windows-x64.zip" _portable/ + zip -r "${ARTIFACT_PREFIX}-${PLATFORM}.zip" _portable/🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@scripts/tauri/build-tauri-vad-asr.sh.in` around lines 276 - 295, The portable archive naming is inconsistent: the Windows branch hardcodes "windows-x64" while the Linux branch uses ${PLATFORM}; update the Windows zip creation to use the PLATFORM variable so names match convention — change the zip command that currently uses "${ARTIFACT_PREFIX}-windows-x64.zip" to "${ARTIFACT_PREFIX}-${PLATFORM}.zip" (refer to PORTABLE_DIR, ARTIFACT_PREFIX, PLATFORM and the zip/rm logic in the Windows branch).
170-170: Consider not silencingcperrors with2>/dev/null || true.Suppressing both stderr and the non-zero exit status will also hide genuine bundle-layout changes from Tauri (e.g., a missing
bundle/<type>/subdir on a new platform), producing empty artifacts that still publish as "successful". Since the globbed paths are predictable per platform, consider asserting at least one match is copied, or scope the suppression to the "no files matched" case only.Also applies to: 241-241, 262-262
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@scripts/tauri/build-tauri-vad-asr.sh.in` at line 170, The cp invocation that silences all errors (cp -r target/${RUST_TARGET}/release/bundle/*/* "${STAGE_DIR}/" 2>/dev/null || true) should be changed to detect the "no files matched" case explicitly and fail on other errors: check for matches under target/${RUST_TARGET}/release/bundle/*/* (e.g. expand into an array or test for existence), if none are found emit an error/exit non-zero; otherwise run cp without swallowing stderr so real copy errors bubble up. Update the same pattern at the other occurrences (lines with the same cp and variables RUST_TARGET and STAGE_DIR) so only the benign "no matches" case is handled gracefully while other failures surface.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@scripts/tauri/build-tauri-vad-asr.sh.in`:
- Around line 168-170: The script stages build artifacts into "${STAGE_DIR}" but
removes it with rm -rf and then runs cp -r ... "${STAGE_DIR}/" without
recreating the directory, causing cp to fail (masked by 2>/dev/null || true) and
subsequent cd "${STAGE_DIR}" to abort; fix by ensuring "${STAGE_DIR}" exists
before every cp that targets it (mirror the mkdir -p used near the second macOS
arch block) — add mkdir -p "${STAGE_DIR}" immediately before the cp commands
that reference STAGE_DIR in the first macOS arch block (around the cp at
target/${RUST_TARGET}/release/bundle/*/* "${STAGE_DIR}/"), the macOS universal
collection block, and the Linux/Windows branch so cp always has an existing
destination directory.
- Around line 113-120: Replace the bare inline python invocation that creates
icons/icon.ico with a guarded Python script call that (1) imports Pillow inside
a try/except and exits nonzero with a clear log message on ImportError or image
read/save errors, (2) checks icons/icon.png dimensions and, if smaller than
256x256, upscales it (e.g., using Image.resize with Image.LANCZOS) to at least
256x256 before saving so the ICO contains 16,32,48,256 variants, and (3) returns
a nonzero exit code on failure so the surrounding shell conditional (which runs
under set -e) fails with a descriptive message; keep the shell condition using
PLATFORM and the presence check for icons/icon.ico and ensure the shell
invocation appends an || { log "ERROR: failed to generate icon.ico (Pillow
missing or icons/icon.png invalid)"; exit 1; } handler to surface failures.
---
Nitpick comments:
In `@scripts/tauri/build-tauri-vad-asr.sh.in`:
- Around line 276-295: The portable archive naming is inconsistent: the Windows
branch hardcodes "windows-x64" while the Linux branch uses ${PLATFORM}; update
the Windows zip creation to use the PLATFORM variable so names match convention
— change the zip command that currently uses
"${ARTIFACT_PREFIX}-windows-x64.zip" to "${ARTIFACT_PREFIX}-${PLATFORM}.zip"
(refer to PORTABLE_DIR, ARTIFACT_PREFIX, PLATFORM and the zip/rm logic in the
Windows branch).
- Line 170: The cp invocation that silences all errors (cp -r
target/${RUST_TARGET}/release/bundle/*/* "${STAGE_DIR}/" 2>/dev/null || true)
should be changed to detect the "no files matched" case explicitly and fail on
other errors: check for matches under target/${RUST_TARGET}/release/bundle/*/*
(e.g. expand into an array or test for existence), if none are found emit an
error/exit non-zero; otherwise run cp without swallowing stderr so real copy
errors bubble up. Update the same pattern at the other occurrences (lines with
the same cp and variables RUST_TARGET and STAGE_DIR) so only the benign "no
matches" case is handled gracefully while other failures surface.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: c323e9f9-8dea-4068-b171-d12d83513ac6
📒 Files selected for processing (37)
.github/workflows/release-dart-package.yaml.github/workflows/tauri-vad-asr.yamlflutter/notes.mdflutter/sherpa_onnx/pubspec.yamlflutter/sherpa_onnx_android/pubspec.yamlflutter/sherpa_onnx_android_arm64/README.mdflutter/sherpa_onnx_android_arm64/android/build.gradleflutter/sherpa_onnx_android_arm64/android/settings.gradleflutter/sherpa_onnx_android_arm64/android/src/main/AndroidManifest.xmlflutter/sherpa_onnx_android_arm64/android/src/main/jniLibs/arm64-v8a/.gitkeepflutter/sherpa_onnx_android_arm64/pubspec.yamlflutter/sherpa_onnx_android_armeabi/README.mdflutter/sherpa_onnx_android_armeabi/android/build.gradleflutter/sherpa_onnx_android_armeabi/android/settings.gradleflutter/sherpa_onnx_android_armeabi/android/src/main/AndroidManifest.xmlflutter/sherpa_onnx_android_armeabi/android/src/main/jniLibs/armeabi-v7a/.gitkeepflutter/sherpa_onnx_android_armeabi/pubspec.yamlflutter/sherpa_onnx_android_x86/README.mdflutter/sherpa_onnx_android_x86/android/build.gradleflutter/sherpa_onnx_android_x86/android/settings.gradleflutter/sherpa_onnx_android_x86/android/src/main/AndroidManifest.xmlflutter/sherpa_onnx_android_x86/android/src/main/jniLibs/x86/.gitkeepflutter/sherpa_onnx_android_x86/pubspec.yamlflutter/sherpa_onnx_android_x86_64/README.mdflutter/sherpa_onnx_android_x86_64/android/build.gradleflutter/sherpa_onnx_android_x86_64/android/settings.gradleflutter/sherpa_onnx_android_x86_64/android/src/main/AndroidManifest.xmlflutter/sherpa_onnx_android_x86_64/android/src/main/jniLibs/x86_64/.gitkeepflutter/sherpa_onnx_android_x86_64/pubspec.yamlflutter/sherpa_onnx_ios/pubspec.yamlflutter/sherpa_onnx_linux/pubspec.yamlflutter/sherpa_onnx_macos/pubspec.yamlflutter/sherpa_onnx_windows/pubspec.yamlnew-release.shscripts/dart/sherpa-onnx-pubspec.yamlscripts/tauri/build-tauri-vad-asr.sh.intauri-examples/non-streaming-speech-recognition-from-file/src-tauri/tauri.conf.json
✅ Files skipped from review due to trivial changes (25)
- flutter/sherpa_onnx_android_armeabi/README.md
- flutter/sherpa_onnx_android_x86/android/settings.gradle
- flutter/sherpa_onnx_android_x86_64/README.md
- flutter/sherpa_onnx_android_x86/README.md
- flutter/sherpa_onnx_ios/pubspec.yaml
- flutter/sherpa_onnx_windows/pubspec.yaml
- flutter/sherpa_onnx_android_x86_64/android/settings.gradle
- flutter/sherpa_onnx_android_x86_64/android/src/main/AndroidManifest.xml
- flutter/sherpa_onnx_android_arm64/android/src/main/AndroidManifest.xml
- flutter/sherpa_onnx_android_armeabi/android/settings.gradle
- flutter/sherpa_onnx_android_arm64/README.md
- flutter/sherpa_onnx_macos/pubspec.yaml
- flutter/sherpa_onnx_android/pubspec.yaml
- flutter/sherpa_onnx_android_x86/android/src/main/AndroidManifest.xml
- flutter/notes.md
- flutter/sherpa_onnx_android_armeabi/android/src/main/AndroidManifest.xml
- flutter/sherpa_onnx_linux/pubspec.yaml
- flutter/sherpa_onnx_android_arm64/android/settings.gradle
- tauri-examples/non-streaming-speech-recognition-from-file/src-tauri/tauri.conf.json
- flutter/sherpa_onnx_android_x86/pubspec.yaml
- flutter/sherpa_onnx_android_x86_64/pubspec.yaml
- flutter/sherpa_onnx_android_armeabi/pubspec.yaml
- flutter/sherpa_onnx_android_x86_64/android/build.gradle
- flutter/sherpa_onnx_android_arm64/pubspec.yaml
- flutter/sherpa_onnx_android_x86/android/build.gradle
🚧 Files skipped from review as they are similar to previous changes (2)
- flutter/sherpa_onnx_android_arm64/android/build.gradle
- flutter/sherpa_onnx_android_armeabi/android/build.gradle
| # Windows needs icon.ico — generate from icon.png if missing | ||
| if [[ "$PLATFORM" == windows-* ]] && [ ! -f icons/icon.ico ]; then | ||
| python3 -c " | ||
| from PIL import Image | ||
| img = Image.open('icons/icon.png') | ||
| img.save('icons/icon.ico', sizes=[(16,16),(32,32),(48,48),(256,256)]) | ||
| " | ||
| fi |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
fd -t f 'icon\.png$' tauri-examples | while read -r f; do
echo "== $f =="
ls -lh "$f"
# PNG header + IHDR width/height
python3 - <<PY "$f"
import struct, sys
with open(sys.argv[1], "rb") as fh:
data = fh.read(24)
if data[:8] != b"\x89PNG\r\n\x1a\n":
print("NOT A PNG"); raise SystemExit
w, h = struct.unpack(">II", data[16:24])
print(f"dimensions: {w}x{h}")
PY
doneRepository: k2-fsa/sherpa-onnx
Length of output: 298
🏁 Script executed:
fd -t f 'build-tauri-vad-asr.sh.in' .Repository: k2-fsa/sherpa-onnx
Length of output: 44
🏁 Script executed:
if [ -f scripts/tauri/build-tauri-vad-asr.sh.in ]; then
sed -n '110,125p' scripts/tauri/build-tauri-vad-asr.sh.in
fiRepository: k2-fsa/sherpa-onnx
Length of output: 707
🏁 Script executed:
# Check if Pillow is even imported/available in the script context
rg -A 5 -B 5 'Image.save.*sizes=' scripts/tauri/Repository: k2-fsa/sherpa-onnx
Length of output: 44
🏁 Script executed:
# Look for other icon files or asset sources
find tauri-examples -type f \( -name "*icon*" -o -name "*.png" \) | head -20Repository: k2-fsa/sherpa-onnx
Length of output: 144
🏁 Script executed:
# Check if there's any documentation about icon generation
rg -l "icon|ico" scripts/tauri/*.md scripts/README.md 2>/dev/null | head -5Repository: k2-fsa/sherpa-onnx
Length of output: 44
🏁 Script executed:
# Verify PIL behavior with a quick test
python3 << 'PYTEST'
from PIL import Image
import io
# Create a 32x32 test image
img = Image.new('RGBA', (32, 32), color='red')
# Try to save as ICO with larger sizes requested
output = io.BytesIO()
img.save(output, format='ICO', sizes=[(16,16),(32,32),(48,48),(256,256)])
# Check what was actually included
output.seek(0)
ico_data = output.read()
print(f"ICO file size: {len(ico_data)} bytes")
# Parse ICO to see what sizes are actually in it
# ICO format: header (6 bytes) + directory entries (16 bytes each)
import struct
reserved, img_type, num_images = struct.unpack('<HHH', ico_data[:6])
print(f"Number of images in ICO: {num_images}")
for i in range(num_images):
offset = 6 + (i * 16)
width, height, colors, reserved, planes, bits, size, img_offset = struct.unpack('<BBBBHHII', ico_data[offset:offset+16])
width = width or 256 # 0 means 256
height = height or 256
print(f" Image {i}: {width}x{height}")
PYTESTRepository: k2-fsa/sherpa-onnx
Length of output: 177
🏁 Script executed:
# Check the tauri-examples project structure and any README
ls -la tauri-examples/non-streaming-speech-recognition-from-file/src-tauri/icons/Repository: k2-fsa/sherpa-onnx
Length of output: 284
🏁 Script executed:
# Look for any documentation about icon generation or requirements
find tauri-examples -name "README*" -o -name "*.md" | xargs grep -l "icon\|ico" 2>/dev/null | head -3Repository: k2-fsa/sherpa-onnx
Length of output: 129
🏁 Script executed:
# Check if there's a .gitignore that might hide proper icon assets
cat tauri-examples/non-streaming-speech-recognition-from-file/src-tauri/.gitignore 2>/dev/null || echo "No .gitignore found"Repository: k2-fsa/sherpa-onnx
Length of output: 70
🏁 Script executed:
# Check Tauri template structure for icon expectations
find tauri-examples/non-streaming-speech-recognition-from-file/src-tauri -type f | grep -i iconRepository: k2-fsa/sherpa-onnx
Length of output: 227
🏁 Script executed:
# Check the README to understand the context
cat tauri-examples/non-streaming-speech-recognition-from-file/README.md | head -40Repository: k2-fsa/sherpa-onnx
Length of output: 1357
🏁 Script executed:
# Check if there's any documentation about the build script itself
head -20 scripts/tauri/build-tauri-vad-asr.sh.inRepository: k2-fsa/sherpa-onnx
Length of output: 534
Improve error handling and icon sizing for Windows builds.
The inline python3 -c has no error handling and will silently produce a broken icon.ico. Because icon.png is only 32×32 pixels, PIL silently drops the 48×48 and 256×256 requests; the resulting .ico will only contain 16×16 and 32×32 variants, missing what Windows installers expect. Additionally, if Pillow is missing or the PNG is unreadable, the entire build aborts under set -e with a cryptic error.
Wrap the step with explicit error handling and enlarge icon.png to support the full requested range:
if [[ "$PLATFORM" == windows-* ]] && [ ! -f icons/icon.ico ]; then
python3 -c "..." || { log "ERROR: failed to generate icon.ico (Pillow missing or icons/icon.png invalid)"; exit 1; }
fi
Also ensure icon.png is at least 256×256 to avoid silent truncation of icon variants.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@scripts/tauri/build-tauri-vad-asr.sh.in` around lines 113 - 120, Replace the
bare inline python invocation that creates icons/icon.ico with a guarded Python
script call that (1) imports Pillow inside a try/except and exits nonzero with a
clear log message on ImportError or image read/save errors, (2) checks
icons/icon.png dimensions and, if smaller than 256x256, upscales it (e.g., using
Image.resize with Image.LANCZOS) to at least 256x256 before saving so the ICO
contains 16,32,48,256 variants, and (3) returns a nonzero exit code on failure
so the surrounding shell conditional (which runs under set -e) fails with a
descriptive message; keep the shell condition using PLATFORM and the presence
check for icons/icon.ico and ensure the shell invocation appends an || { log
"ERROR: failed to generate icon.ico (Pillow missing or icons/icon.png invalid)";
exit 1; } handler to surface failures.
1 participant
We separated it into different architectures to stay within the 100 MB file size limit.
Summary by CodeRabbit
New Features
Chores