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.

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

feat: try out golang.org/x/mobile for generating Java JNI bindings #1327

Closed
wants to merge 1 commit into from

Conversation

adamdecaf
Copy link
Member

https://docs.google.com/document/d/1y9hStonl9wpj-5VM-xWrSTuEJFUAxGOXOhxvAs7GZHE/edit

See: https://blog.dogan.io/2015/08/15/java-jni-jnr-go/
See: https://pkg.go.dev/golang.org/x/mobile/cmd/gobind
Related: golang/go#34885

Remaining Items

  • Generate compatibility package that we generate code from, can be copied from main Go interfaces.

Notes

@codecov-commenter
Copy link

Codecov Report

Merging #1327 (6b92979) into master (5957ca4) will not change coverage.
The diff coverage is n/a.

❗ Current head 6b92979 differs from pull request most recent head 10e244c. Consider uploading reports for the commit 10e244c to get more accurate results

❗ Your organization needs to install the Codecov GitHub app to enable full functionality.

Additional details and impacted files
@@           Coverage Diff           @@
##           master    #1327   +/-   ##
=======================================
  Coverage   89.02%   89.02%           
=======================================
  Files          75       75           
  Lines        7689     7689           
=======================================
  Hits         6845     6845           
  Misses        502      502           
  Partials      342      342           

@midepeter
Copy link

can i get to hop on this @adamdecaf ?

@adamdecaf
Copy link
Member Author

Yea go for it!

@midepeter
Copy link

midepeter commented Feb 22, 2024

@adamdecaf

I am trying to get a clear description of this very PR. What we are trying to achieve here it to use golang.org/x/mobile to generate Java JNI bindings so they will be able to use ach library exported functions from java android applications right.

Also, I just ran the two gobind commands you had in the initial commit that you places in the makefile

  1. the first gobind gobind -lang=java -outdir ./java/ github.com/moov-io/ach/internal/... worked
  2. there were some errors on the second one gobind -lang=java -outdir ./java/ github.com/moov-io/ach/ and this is because we are having three returned types for the function signatures .SegmentFile .NextEntry.

Was this the main problem you faced while working on it. I am just trying to get the state of the PR and what needs to be achieved.

@adamdecaf
Copy link
Member Author

Yes that's pretty much where I ended up as well. Java is a popular language so if we could offer native support (without having to rewrite Go to Java) that would help a lot of potential users of the library.

We probably have to write some middleware to convert functions like SegmentFile so they're compatible with Java / JNI. The Java interface would have a slightly different API than Go, but would be largely compatible.

@midepeter
Copy link

midepeter commented Feb 23, 2024

Yes,

that’s the constraint. Not sure I understand how exactly the middleware should be built. If you can shed more light to that

But the temporary thing I did was to duplicate the functions (SegmentFileJNI and NextEntryJNI) and now embed the return values in a single struct to align with gobind restrictions. It worked and generated successfully but the problem is that there is no way to ignore the normal function or suppress the errors from the normal functions during the bindings.

@adamdecaf
Copy link
Member Author

I think we will want to create a new package which calls functions from moov-io/ach but returns a struct of the multiple values like you did. That way we generate a single package and avoid trying to generate known bad code.

@adamdecaf
Copy link
Member Author

adamdecaf commented Feb 23, 2024

package javainterop

type EntryDetail ach.EntryDetail // reference every type

type File ach.File 

type SegmentFileResponse struct {
    CreditEntryFile *File
    DebitEntryFile *File 
}
func (f *File) SegmentFile(_ *SegmentFileConfiguration) (SegmentFileResponse, error)

Ideally I'd like to generate this or at least compare the API surface between moov-io/ach and the javainterop package so we can ensure every function is ported over.

@midepeter
Copy link

Alright

I will work on that.

@adamdecaf adamdecaf closed this Mar 12, 2024
@adamdecaf adamdecaf added this to the v2 milestone Aug 20, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants