-
Notifications
You must be signed in to change notification settings - Fork 1
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
Add Gradle and GitHub Actions tooling to package for Maven Central #4
Add Gradle and GitHub Actions tooling to package for Maven Central #4
Conversation
a06c67e
to
a491ede
Compare
8cca401
to
be1fb77
Compare
once I get a general ACK, I'll add a commit to change everything to point to |
38aac93
to
c04f28f
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great, thanks for having a look at this!
Here are some comments from a first pass. Generally, I'm afraid that lightningdevkit#25 could get huge if we merge everything into it. Would you therefore mind reopening this as a separate PR upstream, based on lightningdevkit#25?
Btw, we already have the |
Oh sweet, yeah. That will be nice. It seems like we may not be publishing on Sonatype (?) but I'll check out the Java bindings to see how this PR needs to be updated to be more aligned on publishing LDK-branded releases |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The publishing workflows look like they'll run (given that the shell scripts work and your secrets are correctly structured).
One issue you'll run into is the use of the latest NDK for the Android library. This is an ongoing issue with the Rust compiler itself as I found out a few days ago. I think at this point the short-term fix might be to stick with NDK 21 (it's the PR I will probably open on bdk-ffi). An example of the breaks it creates is when you use the android library on an x86_64 architecture (for example on emulators) as @ConorOkus found out on Monday. Using NDK 21 is a bit of a pain and requires downloading it onto the CI image (see our workflow from a few weeks back here). The other problem with that is that so far I have not been able to make this NDK work with the macOS GitHub CI image (see bitcoindevkit/bdk-ffi#282 and bitcoindevkit/bdk-ffi#243) This means you cannot use Android connected tests to run the Android tests in the emulator on the CI (you can run them locally they work fine, I just don't know why I can't seem to use the old NDK on macOS even after following GitHub's instructions).
At this point even Mozilla doesn't have a good, clean fix for it. See bitcoindevkit/bdk-ffi#242 for a bunch of links tracking how this is developing.
29f9d2b
to
c54d4d5
Compare
thanks for all the review! closing in favor of PR to upstream lightningdevkit#59 |
Tested and working for...
ldk-node-android
ldk-node-jvm
Based on work done by BDK (thanks @thunderbiscuit!)
Things to do before merge
lightningdevkit/ldk-node
inldk-node-android/lib/build.gradle.kts