search

mozilla / rust-android-gradle

Apache License 2.0
1.03k stars 67 forks source link

Fix #30 #31

Open Mygod opened 4 years ago

Mygod commented 4 years ago

Tested via https://github.com/shadowsocks/shadowsocks-android/commit/5ea3e3559adfb7b779ba6f3f633a02b3bcf50145.

Mygod commented 4 years ago

What do you mean by "current platform?"

thomcc commented 4 years ago

I'm going to have to do some testing on this. Also I'm not fully sure I understand the issue...

thomcc commented 4 years ago

I mean that this doesn't work with the fix we have in place for https://github.com/mozilla/rust-android-gradle/issues/10.

Edit for clarification: target/$toolchain/$profile/... is wrong if you're on e.g. macos and doing a build for macos desktop, it would just be target/$profile/....

thomcc commented 4 years ago

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.

Mygod commented 4 years ago

Fixed.

Mygod commented 4 years ago

No further comments?

thomcc commented 4 years ago

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.

Mygod commented 4 years ago

The workaround is from https://github.com/rust-lang/cargo/issues/1706#issuecomment-563889929, the soname flags did not seem to work for us.

Mygod commented 4 years ago

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.

ncalexan commented 4 years ago

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 :)

thomcc commented 4 years ago

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).

Mygod commented 4 years ago

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. :)

thomcc commented 4 years ago

Alright but if the unit tests are written in rust

@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.

thomcc commented 4 years ago

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.

ncalexan commented 2 years ago

@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.

Mygod commented 2 years ago

Yes! See this example: https://github.com/shadowsocks/shadowsocks-android/blob/c59af093de3ad6e65b57605904a5617a932655b6/core/build.gradle.kts#L35-L51