Add FFI and JNI support to Swift and Kotlin#11352
Add FFI and JNI support to Swift and Kotlin#11352tarrinneal wants to merge 91 commits intoflutter:mainfrom
Conversation
tarrinneal
added
CICD
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
![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 introduces FFI and JNI support for Swift and Kotlin respectively, updating Pigeon to version 26.4.0. This involved significant changes to the Dart, Swift, and Kotlin generators to handle native interop types, codecs, and API registration. New configuration files for ffigen and jnigen are added, and analysis_options.yaml is updated to ignore the new generated files. The isNullish function in Swift generated files was improved, and toString() methods were added to generated Dart classes. Review comments highlight a potential issue with the Kotlin version downgrade to 2.1.0, suggesting it might be a temporary JNI dependency that needs documentation. Additionally, a request was made to link the specific jnigen issue addressed by jnigen_fix.dart for better maintainability. A further comment points out that the Kotlin generator omits the package statement when useJni is true, which could lead to issues and suggests always generating it if a package name is provided.
|
|
||
| buildscript { | ||
| val kotlinVersion = "2.3.0" | ||
| ext.kotlin_version = '2.1.0' |
There was a problem hiding this comment.
The Kotlin version is being downgraded to 2.1.0 from 2.3.0. This is a significant change that could cause issues for users on newer Kotlin versions. Is this a temporary requirement for the JNI dependencies? If so, this limitation should be clearly documented.
There was a problem hiding this comment.
This is required for jnigen use currently.
| if (generatorOptions.package != null && !generatorOptions.useJni) { | ||
| indent.writeln('package ${generatorOptions.package}'); | ||
| } |
There was a problem hiding this comment.
The package statement is omitted when useJni is true. This can lead to the generated Kotlin files being placed in the default package, which is discouraged and can cause issues in larger projects. It would be better to always generate the package statement if a package name is provided in the options.
| if (generatorOptions.package != null && !generatorOptions.useJni) { | |
| indent.writeln('package ${generatorOptions.package}'); | |
| } | |
| if (generatorOptions.package != null) { | |
| indent.writeln('package ${generatorOptions.package}'); | |
| } |
There was a problem hiding this comment.
There may be a work around for this, but the way jnigen is set up (at least within this project) I couldn't "find" the classes unless they were in the default package. Maybe @liamappelbe knows how?
1 participant
title