Fix building Flutter Android APPs - #3559
Conversation
|
Caution Review failedPull request was closed or merged during review 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 (9)
📝 WalkthroughWalkthroughAndroid build configurations for four architecture variants of sherpa_onnx (arm64, armeabi, x86, x86_64) are updated to use architecture-specific package namespaces, appending suffixes like Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related PRs
Suggested labels
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (4 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 |
There was a problem hiding this comment.
Code Review
This pull request updates the Android namespaces and package names for architecture-specific Flutter plugins (arm64, armeabi, x86, and x86_64) to ensure unique identifiers across different builds. The review feedback identifies an inconsistency in the documentation changes within notes.md, noting that the updated flutter create commands would generate redundant package names and should be reverted to maintain consistency with the manual namespace configurations used in the project.
| flutter create --template plugin_ffi --platforms android --org com.k2fsa.sherpa.onnx.arm64 sherpa_onnx_android_arm64 | ||
| flutter create --template plugin_ffi --platforms android --org com.k2fsa.sherpa.onnx.armeabi sherpa_onnx_android_armeabi | ||
| flutter create --template plugin_ffi --platforms android --org com.k2fsa.sherpa.onnx.x86 sherpa_onnx_android_x86 | ||
| flutter create --template plugin_ffi --platforms android --org com.k2fsa.sherpa.onnx.x86_64 sherpa_onnx_android_x86_64 |
There was a problem hiding this comment.
The updated flutter create commands are inconsistent with the actual namespace values set in the build.gradle files. Running these commands would result in redundant package names like com.k2fsa.sherpa.onnx.arm64.sherpa_onnx_android_arm64, whereas the code in this PR uses com.k2fsa.sherpa.onnx.arm64.
Actually, the original commands (using --org com.k2fsa.sherpa.onnx) already produced unique namespaces by default (e.g., com.k2fsa.sherpa.onnx.sherpa_onnx_android_arm64). The collision likely occurred because the namespaces were manually overridden in the code. It is recommended to revert these documentation changes to keep the commands clean, as manual adjustment is required anyway to match the specific namespace structure used in this repository.
| flutter create --template plugin_ffi --platforms android --org com.k2fsa.sherpa.onnx.arm64 sherpa_onnx_android_arm64 | |
| flutter create --template plugin_ffi --platforms android --org com.k2fsa.sherpa.onnx.armeabi sherpa_onnx_android_armeabi | |
| flutter create --template plugin_ffi --platforms android --org com.k2fsa.sherpa.onnx.x86 sherpa_onnx_android_x86 | |
| flutter create --template plugin_ffi --platforms android --org com.k2fsa.sherpa.onnx.x86_64 sherpa_onnx_android_x86_64 | |
| flutter create --template plugin_ffi --platforms android --org com.k2fsa.sherpa.onnx sherpa_onnx_android_arm64 | |
| flutter create --template plugin_ffi --platforms android --org com.k2fsa.sherpa.onnx sherpa_onnx_android_armeabi | |
| flutter create --template plugin_ffi --platforms android --org com.k2fsa.sherpa.onnx sherpa_onnx_android_x86 | |
| flutter create --template plugin_ffi --platforms android --org com.k2fsa.sherpa.onnx sherpa_onnx_android_x86_64 |
Fixes #3557
Summary by CodeRabbit