Add maven examples for Java API - #3783
Conversation
|
Caution The consumer version of Gemini Code Assist on GitHub has been sunset. All code review activity has officially ceased. |
📝 WalkthroughWalkthroughThe PR adds Maven publication setup and a cross-platform Maven example, introduces matrix-based Maven packaging tests, updates JitPack artifact installation, and changes Java jar versioning and release handling. The previous Java API Maven metadata and guide files are removed. ChangesMaven Java packaging
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant MavenCI
participant MavenExample
participant JitPack
participant ShadedJar
MavenCI->>MavenExample: select platform dependency and run mvn package
MavenExample->>JitPack: resolve Sherpa-ONNX JVM and native artifacts
JitPack-->>MavenExample: return Maven dependencies
MavenExample->>ShadedJar: create shaded executable jar
ShadedJar-->>MavenCI: verify native binaries and execute VersionTest
Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ 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.
Actionable comments posted: 2
🧹 Nitpick comments (3)
.github/workflows/run-java-test.yaml (3)
432-432: 🔒 Security & Privacy | 🔵 Trivial | ⚡ Quick winNew checkout steps don't set
persist-credentials: false.Both new jobs check out the repo and then run
mvn, resolving dependencies from a third-party JitPack repository, while the default GitHub token remains persisted in.git/configfor the rest of the job.🔒 Suggested fix
- uses: actions/checkout@v4 + with: + persist-credentials: falseAlso applies to: 547-547
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.github/workflows/run-java-test.yaml at line 432, Update both new actions/checkout@v4 steps in the affected jobs to set persist-credentials to false, ensuring the GitHub token is not retained in the repository’s Git configuration while subsequent Maven dependency resolution runs.Source: Linters/SAST tools
445-492: 📐 Maintainability & Code Quality | 🔵 Trivial | 🏗️ Heavy liftRegex-based pom.xml mutation is brittle.
The python script matches native-lib dependency blocks purely on the exact 8-space indentation and comment formatting of
pom.xml. Any future reformatting ofpom.xmlwill silently fail to uncomment the target block (no error raised), leaving the build without a native-lib dependency for that platform. Consider using Maven profiles (selected via-P<profile>) activated per platform instead of text-mangling the pom.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.github/workflows/run-java-test.yaml around lines 445 - 492, Replace the regex-based pom.xml mutation in the “Update pom.xml for this platform” workflow step with Maven profile selection using -P<profile> for matrix.native_lib. Define or reuse platform-specific profiles so the selected profile activates its native-lib dependency without relying on indentation or comment formatting, and remove the Python text-mangling logic.
560-634: 📐 Maintainability & Code Quality | 🔵 Trivial | 🏗️ Heavy liftApproach‑1 pom.xml is duplicated inline instead of reused.
This heredoc re-declares the compiler/jar/shade plugin versions that already exist in
java-api-examples/maven-examples/pom.xml's Approach‑1 comment block. Any future plugin-version bump in the checked-inpom.xmlwon't propagate here, and the two will drift.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.github/workflows/run-java-test.yaml around lines 560 - 634, Replace the duplicated heredoc in the “Write pom.xml for Approach 1” workflow step with reuse of the checked-in Approach-1 configuration from java-api-examples/maven-examples/pom.xml. Ensure the generated pom preserves the existing dependency and build behavior while sourcing compiler, jar, and shade plugin versions from the canonical file so future updates stay synchronized.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.github/workflows/run-java-test.yaml:
- Around line 515-517: Update the native-library verification commands near the
“Native libs in jar” checks to fail when the archive contains no .so, .dylib, or
.dll entries. Remove the unconditional `|| true` suppression and add an explicit
non-empty match assertion, preserving successful execution when at least one
native library is found; apply the same change to both verification locations.
In `@jitpack.yml`:
- Around line 4-20: Update the before_install downloads and all mvn
install:install-file commands to use JitPack’s provided version variable instead
of hardcoded 1.13.4. Set the installed artifacts’ groupId to
com.github.k2-fsa.sherpa-onnx so they match the example Maven dependencies, and
ensure the Approach 1 sherpa-onnx dependency declares type aar.
---
Nitpick comments:
In @.github/workflows/run-java-test.yaml:
- Line 432: Update both new actions/checkout@v4 steps in the affected jobs to
set persist-credentials to false, ensuring the GitHub token is not retained in
the repository’s Git configuration while subsequent Maven dependency resolution
runs.
- Around line 445-492: Replace the regex-based pom.xml mutation in the “Update
pom.xml for this platform” workflow step with Maven profile selection using
-P<profile> for matrix.native_lib. Define or reuse platform-specific profiles so
the selected profile activates its native-lib dependency without relying on
indentation or comment formatting, and remove the Python text-mangling logic.
- Around line 560-634: Replace the duplicated heredoc in the “Write pom.xml for
Approach 1” workflow step with reuse of the checked-in Approach-1 configuration
from java-api-examples/maven-examples/pom.xml. Ensure the generated pom
preserves the existing dependency and build behavior while sourcing compiler,
jar, and shade plugin versions from the canonical file so future updates stay
synchronized.
🪄 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: dc653307-bdec-4e76-926f-1d17f9c12537
📒 Files selected for processing (10)
.github/workflows/jar.yaml.github/workflows/run-java-test.yamljava-api-examples/maven-examples/.gitignorejava-api-examples/maven-examples/README.mdjava-api-examples/maven-examples/pom.xmljava-api-examples/maven-examples/src/main/java/com/k2fsa/sherpa/onnx/example/VersionTest.javajitpack.ymlsherpa-onnx/java-api/pom.xmlsherpa-onnx/java-api/readme.mdsherpa-onnx/java-api/readme.zh.md
💤 Files with no reviewable changes (3)
- sherpa-onnx/java-api/pom.xml
- sherpa-onnx/java-api/readme.md
- sherpa-onnx/java-api/readme.zh.md
| echo "=== Native libs in jar ===" | ||
| unzip -l target/sherpa-onnx-maven-example-1.0-SNAPSHOT.jar | grep -E "\.so$|\.dylib$|\.dll$" || true | ||
|
|
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Native-lib verification step can't actually fail.
grep ... || true means this step always exits 0 even when zero native binaries are found in the jar, so it silently passes regardless of whether the jar actually bundles the expected .so/.dylib/.dll. Since the stated purpose of this step is to verify native binaries are present, an empty match should fail the job (e.g., assert non-zero line count) rather than be swallowed.
✅ Suggested fix
- echo "=== Native libs in jar ==="
- unzip -l target/sherpa-onnx-maven-example-1.0-SNAPSHOT.jar | grep -E "\.so$|\.dylib$|\.dll$" || true
+ echo "=== Native libs in jar ==="
+ unzip -l target/sherpa-onnx-maven-example-1.0-SNAPSHOT.jar | grep -E "\.so$|\.dylib$|\.dll$"Also applies to: 657-659
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In @.github/workflows/run-java-test.yaml around lines 515 - 517, Update the
native-library verification commands near the “Native libs in jar” checks to
fail when the archive contains no .so, .dylib, or .dll entries. Remove the
unconditional `|| true` suppression and add an explicit non-empty match
assertion, preserving successful execution when at least one native library is
found; apply the same change to both verification locations.
| before_install: | ||
| - wget https://github.com/k2-fsa/sherpa-onnx/releases/download/v1.13.4/sherpa-onnx-1.13.4.aar | ||
| - wget https://github.com/k2-fsa/sherpa-onnx/releases/download/v1.13.4/sherpa-onnx-jvm-1.13.4.jar | ||
| - wget https://github.com/k2-fsa/sherpa-onnx/releases/download/v1.13.4/sherpa-onnx-native-lib-linux-aarch64-1.13.4.jar | ||
| - wget https://github.com/k2-fsa/sherpa-onnx/releases/download/v1.13.4/sherpa-onnx-native-lib-linux-x64-1.13.4.jar | ||
| - wget https://github.com/k2-fsa/sherpa-onnx/releases/download/v1.13.4/sherpa-onnx-native-lib-osx-aarch64-1.13.4.jar | ||
| - wget https://github.com/k2-fsa/sherpa-onnx/releases/download/v1.13.4/sherpa-onnx-native-lib-osx-x64-1.13.4.jar | ||
| - wget https://github.com/k2-fsa/sherpa-onnx/releases/download/v1.13.4/sherpa-onnx-native-lib-win-x64-1.13.4.jar | ||
|
|
||
| install: | ||
| - FILE="-Dfile=sherpa-onnx-1.13.4.aar" | ||
| - mvn install:install-file $FILE -DgroupId=com.k2fsa.sherpa.onnx -DartifactId=sherpa-onnx -Dversion=1.13.4 -Dpackaging=aar -DgeneratePom=true | ||
| - mvn install:install-file -Dfile=sherpa-onnx-1.13.4.aar -DgroupId=com.github.k2-fsa -DartifactId=sherpa-onnx -Dversion=1.13.4 -Dpackaging=aar -DgeneratePom=true | ||
| - mvn install:install-file -Dfile=sherpa-onnx-jvm-1.13.4.jar -DgroupId=com.github.k2-fsa -DartifactId=sherpa-onnx-jvm -Dversion=1.13.4 -Dpackaging=jar -DgeneratePom=true | ||
| - mvn install:install-file -Dfile=sherpa-onnx-native-lib-linux-aarch64-1.13.4.jar -DgroupId=com.github.k2-fsa -DartifactId=sherpa-onnx-native-lib-linux-aarch64 -Dversion=1.13.4 -Dpackaging=jar -DgeneratePom=true | ||
| - mvn install:install-file -Dfile=sherpa-onnx-native-lib-linux-x64-1.13.4.jar -DgroupId=com.github.k2-fsa -DartifactId=sherpa-onnx-native-lib-linux-x64 -Dversion=1.13.4 -Dpackaging=jar -DgeneratePom=true | ||
| - mvn install:install-file -Dfile=sherpa-onnx-native-lib-osx-aarch64-1.13.4.jar -DgroupId=com.github.k2-fsa -DartifactId=sherpa-onnx-native-lib-osx-aarch64 -Dversion=1.13.4 -Dpackaging=jar -DgeneratePom=true | ||
| - mvn install:install-file -Dfile=sherpa-onnx-native-lib-osx-x64-1.13.4.jar -DgroupId=com.github.k2-fsa -DartifactId=sherpa-onnx-native-lib-osx-x64 -Dversion=1.13.4 -Dpackaging=jar -DgeneratePom=true | ||
| - mvn install:install-file -Dfile=sherpa-onnx-native-lib-win-x64-1.13.4.jar -DgroupId=com.github.k2-fsa -DartifactId=sherpa-onnx-native-lib-win-x64 -Dversion=1.13.4 -Dpackaging=jar -DgeneratePom=true |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🔴 Critical | ⚡ Quick win
Align the published Maven coordinates with the example dependencies.
jitpack.ymlinstallssherpa-onnx-jvmandsherpa-onnx-native-lib-*undercom.github.k2-fsa, butjava-api-examples/maven-examples/pom.xmlexpectscom.github.k2-fsa.sherpa-onnx.- The Approach 1
sherpa-onnxdependency also needstype=aar; without it, Maven will resolve a JAR and miss the published artifact. - Hardcoding
1.13.4here makes the install script diverge from the requested ref/version; use the JitPack-provided version variable instead.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@jitpack.yml` around lines 4 - 20, Update the before_install downloads and all
mvn install:install-file commands to use JitPack’s provided version variable
instead of hardcoded 1.13.4. Set the installed artifacts’ groupId to
com.github.k2-fsa.sherpa-onnx so they match the example Maven dependencies, and
ensure the Approach 1 sherpa-onnx dependency declares type aar.
Fixes #2439
Fixes #2665
Note that we use jitpack.
cc @Chamukuy
Summary by CodeRabbit
New Features
Documentation
Tests