Skip to content
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.

Sign up for GitHub

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

[java] Setting Java module name to com.microsoft.onnxruntime.extensions #730

Merged
merged 1 commit into from
May 28, 2024
Merged

[java] Setting Java module name to com.microsoft.onnxruntime.extensions #730

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion java/build.gradle
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -113,7 +113,7 @@ if (cmakeBuildDir != null) {
// Overwrite jar location
task allJar(type: Jar) {
manifest {
attributes('Automatic-Module-Name': project.group,
attributes('Automatic-Module-Name': "com.microsoft.onnxruntime.extensions",
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should this change or should project.group be set to com.microsoft.onnxruntime.extensions? Not sure what is preferred.

Does build-android.gradle need updating? It doesn't seem to set Automatic-Module-Name. That Android package is used in example apps without any issues AFAIK.

https://github.com/microsoft/onnxruntime-inference-examples/blob/1c2199208a9084ec0e4ceb188cb5192608d75938/mobile/examples/super_resolution/android/app/build.gradle#L49
https://github.com/microsoft/onnxruntime-inference-examples/blob/1c2199208a9084ec0e4ceb188cb5192608d75938/mobile/examples/super_resolution/android/app/src/main/java/ai/onnxruntime/example/superresolution/MainActivity.kt#L4

Copy link
Contributor Author

@Craigacp Craigacp May 28, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

project.group sets the group name for Maven Central, and that can be com.microsoft.onnxruntime. Maven has both a group and artifact name, so it's fine for it to be group com.microsoft.onnxruntime and artifact onnxruntime-extensions. The namespacing will keep things together on Maven - https://central.sonatype.com/namespace/com.microsoft.onnxruntime. Maven Central's name doesn't matter for the Java platform module check.

I don't believe Android cares about Java modules, but anyway ORT-extensions targets a version of Android which only supports Java 8 and modules came in Java 9. Adding the automatic module name wouldn't cause problems if added to the build-android.gradle as it'll ignore things in the manifest it doesn't care about, but it probably wouldn't do anything either.

The problem in Java occurs if the user has modularized their application and then tries to run it with both the ORT and ORT-extensions jars as dependencies. As ORT-extensions (for non-Android Java) isn't on Maven Central yet there aren't too many people doing that, but it'll likely cause problems when it gets there. For example the stable diffusion example I wrote (https://github.com/oracle/sd4j) won't compile properly when migrated to use ORT-extensions from Maven Central without this change.

'Implementation-Title': 'onnxruntime-extensions',
'Implementation-Version': project.version)
}
Expand Down
Loading