Fix #30#31
Conversation
thomcc
left a comment
thomcc
left a comment
There was a problem hiding this comment.
Hrmmm... I think this is wrong if you're outputing for the current platform. We go through some effort to ensure that we don't pass the target arg to reuse as much of the existing build output.
|
What do you mean by "current platform?" |
|
I'm going to have to do some testing on this. Also I'm not fully sure I understand the issue... |
|
I mean that this doesn't work with the fix we have in place for #10. Edit for clarification: |
|
Sorry, my bad, I just realized this code is inside a branch that only runs for non-desktop builds. It probably still applies but I'm not really worried about people building android apps on android. That said, I'll still need to do some testing of it before I cut a release. |
|
Fixed. |
|
No further comments? |
|
Sorry, I’ve been sick, and I need to find time to test, probably later this week. I’d also like to better understand the motivation here. |
|
The workaround is from rust-lang/cargo#1706 (comment), the |
| File(project.rootProject.buildDir, "linker-wrapper/linker-wrapper.py").path) | ||
| environment("RUST_ANDROID_GRADLE_CC", cc) | ||
| environment("RUST_ANDROID_GRADLE_CC_LINK_ARG", "-Wl,-soname,lib${cargoExtension.libname!!}.so") | ||
| environment("RUST_ANDROID_GRADLE_CC_LINK_ARG", "-o,$cargoOutputDir/lib${cargoExtension.libname!!}.so") |
There was a problem hiding this comment.
Hmm, I wonder if we should be still setting the soname in addition to this. Do you know if the current version still ends up with the correct soname?
|
The new patch should fix things in a simpler way. Not sure why you would want to compile non-Android binaries with this plugin though. |
We compile the same code for Desktop platforms to run unit tests against it -- it's really nice :) |
|
I'll test this out (and cut a release assuming it works) later today or tomorrow, sorry for the delay here (I was sick, but am not anymore). |
|
Alright but if the unit tests are written in rust, probably there is no need to bundle them into apk so I guess my patch still makes sense. :) |
| spec.include("lib${libname}.dylib") | ||
| spec.include("${libname}.dll") | ||
| spec.include(libname!!) | ||
| } |
There was a problem hiding this comment.
Huh, is this right for non-linux platforms? macos uses .dylib and windows uses .dll.
There was a problem hiding this comment.
Oh maybe the issue is that I am compiling a binary to be bundled into the apk -- not a library.
There was a problem hiding this comment.
Right, for bundling an APK that's right, but rust-android-gradle also supports being used for other situations (namely unit tests on the host machine, as discussed in comments below).
Sorry for all the trouble here!
@ncalexan meant android unit tests. Example: https://github.com/mozilla/application-services/blob/master/components/logins/android/src/test/java/mozilla/appservices/logins/DatabaseLoginsStorageTest.kt is testing the rust code in this crate https://github.com/mozilla/application-services/tree/master/components/logins and it "just works" and automatically builds the shared library for the current platform and runs it via robolectric. |
|
Yes, this doesn't work for local unit tests. I'll update the code to the sample project in this repository that demonstrates this so that you can test against that. Again, sorry for all the issues here. |
|
@Mygod I saw email from you a few days ago, but now I can't find the comment on GH. I can't really tell what the issue you are trying to solve is. If I understand correctly, you are trying to put a Rust binary executable into an Android APK, and then to run that executable on a device? I have a spare half hour, I'll try to see if that can be done already. |
3 participants
Tested via shadowsocks/shadowsocks-android@5ea3e35.