[Tauri] Unify serial implementation #4683
Conversation
|
Important Review skippedDraft detected. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the WalkthroughThis pull request introduces Android APK signing workflows, upgrades Java toolchain to version 21 across CI and release pipelines, downgrades tauri-plugin-serialplugin to exact version 2.15.0, removes Android-specific serial port functionality, and simplifies the main Tauri application initialization by consolidating plugin handling. Changes
Sequence Diagram(s)sequenceDiagram
participant Release as Release Workflow
participant Secrets as GitHub Secrets
participant Gradle as Gradle Build
participant Sign as Signing Logic
participant Upload as Artifact Upload
Release->>Secrets: Check for release keystore secrets
alt All 4 release secrets present
Secrets-->>Sign: Provide keystore credentials
Sign->>Gradle: Sign APK with release key
Gradle-->>Sign: APK signed
Sign->>Gradle: Sign AAB with release key
Gradle-->>Sign: AAB signed
Sign->>Upload: Upload *-signed.apk & *-signed.aab
else Secrets missing
Sign->>Gradle: Generate debug keystore
Gradle-->>Sign: Keystore created
Sign->>Gradle: Sign APK with debug key
Gradle-->>Sign: APK signed
Sign->>Upload: Upload -debug-signed.apk only
end
Upload->>Upload: Release complete
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes
Suggested labels
Suggested reviewers
Pre-merge checks and finishing touches❌ Failed checks (2 warnings)
✅ Passed checks (1 passed)
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 |
haslinghuis
changed the title
haslinghuis
force-pushed
the
tauri-serial
branch
2 times, most recently
from
767aaea to
dc2b2a4
Compare
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
src-tauri/Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (6)
.github/ANDROID_SIGNING.md(1 hunks).github/workflows/ci.yml(2 hunks).github/workflows/release.yml(3 hunks)src-tauri/Cargo.toml(1 hunks)src-tauri/src/main.rs(1 hunks)src-tauri/src/serial_android.rs(0 hunks)
💤 Files with no reviewable changes (1)
- src-tauri/src/serial_android.rs
🔇 Additional comments (9)
.github/ANDROID_SIGNING.md (2)
24-34: LGTM! Keystore creation command is correct.The keytool command is properly formatted with appropriate parameters for production use (RSA 2048-bit, 10000 days validity).
76-84: LGTM! APK verification instructions are accurate.The verification commands using jarsigner and keytool are correct and will help users confirm their APKs are properly signed.
.github/workflows/ci.yml (3)
172-191: LGTM! Debug keystore generation follows Android conventions.The debug keystore generation is correctly implemented with:
- Standard Android debug credentials
- Proper idempotency check
- Correct keytool parameters matching Android debug standards
193-214: LGTM! APK signing and zipalign process is correct.The signing workflow properly:
- Signs the unsigned APK in-place with jarsigner
- Creates an aligned copy with zipalign using the debug-signed filename
- Uses appropriate cryptographic algorithms (SHA256withRSA)
- Dynamically detects the build-tools version for zipalign
129-133: Update ANDROID.md to document Java 21 compatibility status or clarify choice.While Java 21 is technically compatible with Gradle 8.10.2 and Android Gradle Plugin 8.8.1 (which exceed the 8.5.1+ minimum), Tauri still recommends using Java 17 as the safer/default JDK. The project's ANDROID.md currently only documents "JDK 17+" without acknowledging Java 21. Either update the documentation to explain the Java 21 upgrade and potential deprecation warnings, or align CI with the documented Java 17 recommendation.
.github/workflows/release.yml (3)
144-216: LGTM! Signing logic correctly handles both release and debug workflows.The conditional signing flow is well-implemented:
- Properly falls back to debug keystore when release secrets are unavailable
- Uses file existence check (more reliable than env check)
- Correctly signs APK with jarsigner and zipalign
- Appropriately handles AAB signing (which doesn't require zipalign)
- Proper error handling and logging
One minor note: The AAB signing on line 211 uses
mvto rename the unsigned AAB to signed. This is correct since jarsigner modifies the file in-place, andmvsimply renames it to reflect its signed status.
217-225: LGTM! Artifact upload paths correctly match signed outputs.The upload paths properly use wildcards to match both debug-signed and release-signed artifacts, providing flexibility based on whether release secrets are configured.
82-96: No compatibility concerns - Java 21 is already configured in the build.The ANDROID.md documentation explicitly requires "JDK 17+", which includes Java 21. The project's Capacitor-generated build configuration already specifies
JavaVersion.VERSION_21for both source and target compatibility. The workflow setup ofjava-version: '21'correctly aligns with these existing requirements and no version conflicts are present in the build configuration.src-tauri/src/main.rs (1)
7-9: Changes are correct and properly scoped to desktop builds.The unconditional serialplugin registration in main.rs is appropriate since this is the desktop-only binary entry point. The mobile entry point (src/lib.rs) correctly omits serialplugin registration to avoid platform incompatibilities. The pinned version 2.15.0 in Cargo.toml addresses known Android/Kotlin API compatibility issues. No runtime concerns detected.
| - name: Ensure JitPack repository (for usb-serial-for-android) | ||
| shell: bash | ||
| run: | | ||
| set -euo pipefail | ||
| FILE="src-tauri/gen/android/build.gradle.kts" | ||
| echo "Ensuring JitPack repository is present in $FILE" | ||
| if ! grep -q 'jitpack.io' "$FILE"; then | ||
| printf '\nallprojects {\n repositories {\n maven(url = "https://jitpack.io")\n }\n}\n' >> "$FILE" | ||
| fi | ||
| echo "Repositories block in $FILE now contains:" | ||
| grep -n "jitpack.io" "$FILE" || true |
There was a problem hiding this comment.
Review the repository injection approach for potential Gradle syntax conflicts.
The current implementation appends an allprojects block to build.gradle.kts if JitPack is not found. This could create duplicate allprojects blocks or improper nesting if the file already contains such a block, potentially leading to Gradle build failures.
Consider these alternatives:
- Check for existing
allprojectsblock and append within it:
run: |
set -euo pipefail
FILE="src-tauri/gen/android/build.gradle.kts"
echo "Ensuring JitPack repository is present in $FILE"
if ! grep -q 'jitpack.io' "$FILE"; then
- printf '\nallprojects {\n repositories {\n maven(url = "https://jitpack.io")\n }\n}\n' >> "$FILE"
+ if grep -q 'allprojects' "$FILE"; then
+ # Insert into existing allprojects block (requires more complex sed/awk)
+ echo "Warning: allprojects block exists, manual insertion needed"
+ else
+ printf '\nallprojects {\n repositories {\n maven(url = "https://jitpack.io")\n }\n}\n' >> "$FILE"
+ fi
fi- Use Gradle init script instead of modifying generated files:
mkdir -p ~/.gradle/init.d
cat > ~/.gradle/init.d/jitpack.gradle.kts << 'EOF'
allprojects {
repositories {
maven(url = "https://jitpack.io")
}
}
EOFThe Gradle init script approach is cleaner as it avoids modifying generated files.
🤖 Prompt for AI Agents
.github/workflows/ci.yml around lines 154-164: the workflow currently appends a
new allprojects block to src-tauri/gen/android/build.gradle.kts which can create
duplicate blocks or invalid nesting; replace this with a safer approach: detect
an existing allprojects { ... } block and insert the jitpack maven entry into
its repositories section if present (or create a repositories block inside that
allprojects block if missing), otherwise fallback to creating a Gradle init
script under ~/.gradle/init.d/jitpack.gradle.kts that adds the allprojects {
repositories { maven(url = "https://jitpack.io") } } entry so generated files
are not modified; update the CI step to perform the detection-and-insert logic
and, if insertion into the generated file isn’t possible, create the init.d file
instead.
| - name: Ensure JitPack repository (for usb-serial-for-android) | ||
| shell: bash | ||
| run: | | ||
| set -euo pipefail | ||
| FILE="src-tauri/gen/android/build.gradle.kts" | ||
| if [ -f "$FILE" ]; then | ||
| echo "Ensuring JitPack repository is present in $FILE" | ||
| if ! grep -q 'jitpack.io' "$FILE"; then | ||
| printf '\nallprojects {\n repositories {\n maven(url = "https://jitpack.io")\n }\n}\n' >> "$FILE" | ||
| fi | ||
| grep -n "jitpack.io" "$FILE" || true | ||
| else | ||
| echo "Warning: $FILE not found (will be generated by Tauri on first build)." | ||
| fi |
There was a problem hiding this comment.
Same repository injection concern as ci.yml.
This has the same potential issue with appending allprojects blocks as noted in the ci.yml workflow (lines 154-164). Consider using a Gradle init script instead of modifying generated files.
The file existence check is a good addition, but the append approach can still create duplicate blocks if the file already contains an allprojects section. Refer to the review comment on ci.yml lines 154-164 for suggested alternatives.
🤖 Prompt for AI Agents
.github/workflows/release.yml lines 108-121: the workflow appends an allprojects
{ repositories { maven(url = "https://jitpack.io") } } block directly into
src-tauri/gen/android/build.gradle.kts which can create duplicate or conflicting
allprojects sections; instead make the change idempotent by either (a) creating
a Gradle init script (e.g. in .github/workflows or repo root) that adds the
jitpack repository for all builds and reference that in CI, or (b) if you must
edit the generated file, detect an existing allprojects block and inject/merge
the jitpack repository into that block rather than appending a new allprojects
section, ensuring the check is robust (search for an existing allprojects { ...
} and for jitpack.io) before writing.
| - name: Setup release keystore (if available) | ||
| if: ${{ env.ANDROID_KEYSTORE_BASE64 != '' }} | ||
| env: | ||
| ANDROID_KEYSTORE_BASE64: ${{ secrets.ANDROID_KEYSTORE_BASE64 }} | ||
| ANDROID_KEYSTORE_PASSWORD: ${{ secrets.ANDROID_KEYSTORE_PASSWORD }} | ||
| ANDROID_KEY_ALIAS: ${{ secrets.ANDROID_KEY_ALIAS }} | ||
| ANDROID_KEY_PASSWORD: ${{ secrets.ANDROID_KEY_PASSWORD }} | ||
| shell: bash | ||
| run: | | ||
| set -euo pipefail | ||
| echo "Setting up release keystore from secrets" | ||
| echo "$ANDROID_KEYSTORE_BASE64" | base64 -d > release.keystore | ||
| echo "KEYSTORE_PATH=$(pwd)/release.keystore" >> $GITHUB_ENV | ||
| echo "KEYSTORE_PASSWORD=$ANDROID_KEYSTORE_PASSWORD" >> $GITHUB_ENV | ||
| echo "KEY_ALIAS=$ANDROID_KEY_ALIAS" >> $GITHUB_ENV | ||
| echo "KEY_PASSWORD=$ANDROID_KEY_PASSWORD" >> $GITHUB_ENV | ||
There was a problem hiding this comment.
Fix the conditional check for release keystore setup.
The condition if: ${{ env.ANDROID_KEYSTORE_BASE64 != '' }} checks an environment variable that hasn't been set yet. The env block within the step sets environment variables from secrets, but the if condition is evaluated before the step runs.
Apply this diff to fix the conditional:
- name: Setup release keystore (if available)
- if: ${{ env.ANDROID_KEYSTORE_BASE64 != '' }}
+ if: ${{ secrets.ANDROID_KEYSTORE_BASE64 != '' }}
env:
ANDROID_KEYSTORE_BASE64: ${{ secrets.ANDROID_KEYSTORE_BASE64 }}
ANDROID_KEYSTORE_PASSWORD: ${{ secrets.ANDROID_KEYSTORE_PASSWORD }}This ensures the step only runs when the secret is actually configured in the repository.
🤖 Prompt for AI Agents
.github/workflows/release.yml around lines 127-143: the step's if condition is
checking env.ANDROID_KEYSTORE_BASE64 which isn't set until the step runs; update
the condition to check the repository secret instead by changing the if to use
the secrets context (for example: if: ${{ secrets.ANDROID_KEYSTORE_BASE64 != ''
}} ), keep the env block that maps secrets into environment variables, and leave
the rest of the step unchanged so the keystore is only created when the secret
is present.
| # Pin exact version to avoid pulling 2.21.x which is incompatible with current Android/Kotlin setup | ||
| # Try an older plugin version that matches tauri-android 2.9 API surface (avoid onDetach override) | ||
| # Further downgrade to avoid onDetach override incompatibility | ||
| tauri-plugin-serialplugin = "=2.15.0" |
There was a problem hiding this comment.
🧩 Analysis chain
Consolidate redundant comments and verify the pinned version.
The three comment lines explaining the version pin are somewhat redundant. Consider consolidating them for clarity.
Apply this diff to consolidate the comments:
-# Pin exact version to avoid pulling 2.21.x which is incompatible with current Android/Kotlin setup
-# Try an older plugin version that matches tauri-android 2.9 API surface (avoid onDetach override)
-# Further downgrade to avoid onDetach override incompatibility
+# Pinned to 2.15.0: versions 2.21+ have onDetach override incompatibility with tauri-android 2.9 API
tauri-plugin-serialplugin = "=2.15.0"Verify that version 2.15.0 exists and check for any security advisories:
Additionally, you may want to verify the plugin version on crates.io:
🏁 Script executed:
#!/bin/bash
# Description: Check if tauri-plugin-serialplugin 2.15.0 exists and look for security issues
# Search for any published versions or release info in the repository
rg -n "2\.15\.0|2\.21" -g "*.md" -g "*.toml" -C 2
# Check Cargo.lock for the resolved version
if [ -f "Cargo.lock" ]; then
grep -A 5 "name = \"tauri-plugin-serialplugin\"" Cargo.lock | head -10
else
echo "No Cargo.lock found"
fiLength of output: 625
🌐 Web query:
What versions of tauri-plugin-serialplugin are available on crates.io, and are there any known issues with version 2.15.0?
💡 Result:
Short answer:
-
Available releases (per the plugin repo tags): 2.16.0, 2.10.0, 2.4.14, 2.4.12, v2.1.0 (the repository’s tags/releases page shows 2.16.0 as the newest). [1]
-
Known issues for 2.15.0: I could not find any open/recorded issues specifically referencing version 2.15.0 in the plugin’s issue tracker. No repo issues specifically mention 2.15.0. If you want a definitive crates.io list and publish dates or any crate-specific bug reports tied to the crates.io release, I can fetch the crates.io API page and scan issue trackers again. [1][2]
Sources:
- GitHub tags/releases for s00d/tauri-plugin-serialplugin (tags list includes 2.16.0 etc.). [turn6view0].
- Repository issues overview (no matches for “2.15.0” in open issues). [turn2view1].
Fix the pinned version: 2.15.0 does not exist on crates.io.
The web search confirms that available versions are 2.16.0, 2.10.0, 2.4.14, 2.4.12, and v2.1.0—version 2.15.0 is not among them. This will cause build failures.
Issues:
- Exact version pin "=2.15.0" targets a non-existent release
- Must use an available version (likely 2.10.0 or 2.16.0) that satisfies the Android/Kotlin compatibility constraints mentioned in the comments
- The three comment lines should also be consolidated once the correct version is identified
Action required: Verify which available version resolves the onDetach override incompatibility, update the pin accordingly, and consolidate the comments into a single line explaining the constraint.
haslinghuis
force-pushed
the
tauri-serial
branch
from
50fc733 to
3189a9b
Compare
|
@coderabbitai removed last commit (and moved to separate PR which now enables installable APK) so we can test further on a real device. |
|
Tip For best results, initiate chat on the files or code changes. @haslinghuis, thank you for the update! That's a good approach to separate the installable APK functionality into its own PR. This keeps the current PR focused on the serial implementation unification while allowing you to test the APK installation changes independently on a real device. Let me know if you'd like me to review any specific aspects of the serial unification changes in this PR, or if you need assistance with the separate APK PR once you're ready. |
haslinghuis
force-pushed
the
tauri-serial
branch
from
3189a9b to
ddbe6a5
Compare
|
2 participants
Summary by CodeRabbit
Documentation
Chores
Refactor