mozilla / rust-android-gradle

Apache License 2.0
1.03k stars 67 forks source link

0.9.3 incompatible with AGP 7.4.1 #118

Open Leojangh opened 1 year ago

Leojangh commented 1 year ago

It can't package compiled rust jni libs to aar.So java.lang.UnsatisfiedLinkError not found ocurrs. AGP 7.3.1 has no problem.

ncalexan commented 1 year ago

Thanks for the report. The limited testing that we do should be able to also test AGP 7.4.x around https://github.com/mozilla/rust-android-gradle/blob/4fba4b9db16d56ba4e4f9aef2c028a4c2d6a9126/plugin/build.gradle#L30-L39. I'd love a patch to at least try newer versions of Gradle and AGP to see if we can get failing tests, and eventually to fix them. So: help wanted!

Thanks for the report!

ghost commented 1 year ago

Seconding the breakage. Not sure if I'll be able to spend time debugging it any time soon though.

bqbs commented 1 year ago

It's working for me under AGP 7.4.1 and 8.0.0-beta03 both.But I'm just doing some test..so my code are in the same source set.

src ├── main    ├── cpp    ├── java    └── rust

In case you need.Here is some different. Cargo.toml

...
[lib]
crate_type = ["lib", "cdylib"]

and build.gradle

/*
tasks.whenTaskAdded { task ->
    if ((task.name == 'javaPreCompileDebug' || task.name == 'javaPreCompileRelease')) {
        task.dependsOn 'cargoBuild'
    }
}
*/

tasks.whenTaskAdded { task ->
    if ((task.name == 'mergeDebugJniLibFolders' || task.name == 'mergeReleaseJniLibFolders')) {
        task.dependsOn 'cargoBuild'
    }
}
ghost commented 1 year ago

Yeah, it seems like the problem went away for me in the past few days. I'm sending in two PRs to add 7.4.2 to the tests.

Leojangh commented 1 year ago

OK,It repreduces in Gradle 8.0-rc02 and AGP 7.4.1. It seems like the problem went away after Bumping Gradle to 8.0.1. I found that the compiled rust lib(librust.so) is about 4.2MB in dir build/rustJniLibs/android/arm64-v8a,but the aar contains two different librust.so,one is in aar's assets/arm64-v8a/librust.so which is 980KB and the other is in jni/arm64-v8a/librust.so which is 132KB. Why there is two .so file in aar? Is it normal?

ghost commented 1 year ago

I've been getting it again, I think there's some caching interfering with whether or not it works. I can get it to work under 7.4.2 sometimes, but if I wipe caches and try again then I'll get the problem again. Trying to dig in a bit more. Gradle 7.6 and AGP 7.4.2.

ghost commented 1 year ago

Some more detail. EDIT: Lots more detail, I'm just just adding updates while I wait for builds to run. I'm checking for the bug, where the JNI doesn't make it onto the device and we get java.lang.UnsatisfiedLinkError: dlopen failed: library "lib<LIB_NAME>.so" not found. I'm doing this by running the connectedAndroidTest task for my test app (Validation), which has an implementation dependency on the Android library that builds and includes the Rust JNI library.

If I switch my AGP version from 7.3.1 to 7.4.2, do a clean and a re-build, then the build/outputs/apk/debug/validation-debug.apk file is missing it's lib directory and the the test fails with the "library not found" error. But I just got it to work with 7.4.2, I added android.sourceSets.getByName("debug") { jniLibs.srcDir("$buildDir/rustJniLibs/android") } to my Android Library's build.gradle.kts file, re-ran connectedAndroidTest and the test passed, with the JNI library being in the debug.apk file's lib directory.

I did some rebuilds, including clearing Android Studio's cache, and it seemed to be working. At this point I had this in my build.gradle.kts:

cargo {
    module = "../crates/mylib"
    targetDirectory = "../target"
    targetIncludes = arrayOf("libmylib.so")
    libname = "mylib"
    targets =
        listOf(
            "arm", // Older Physical Android Devices
            "arm64", // Recent Physical Android Devices
            "x86", // Older Emulated devices, including the ATD Android Test device
            "x86_64", // Most Emulated Android Devices
        )
    features { defaultAnd(arrayOf("android")) }
}

android.sourceSets.getByName("androidTest") { jniLibs.srcDir("$buildDir/rustJniLibs/android") }
android.sourceSets.getByName("debug") { jniLibs.srcDir("$buildDir/rustJniLibs/android") }

// Must manually configure that Android build to depend on the JNI artifacts
tasks.withType<com.android.build.gradle.tasks.MergeSourceSetFolders>().configureEach {
    if (this.name.contains("Jni")) {
        this.dependsOn(tasks.named("cargoBuild"))
    }
}
afterEvaluate { tasks.named("preBuild").dependsOn(tasks.named("cargoBuild")) }

So so far so good?

I commented out the line that added it to the debug source set and the test failed, then added it back and it worked again. Then I tried launching the debug build of the app and that worked, then ran the "release" build of the app, and it the app crashed, library not found. Added android.sourceSets.getByName("release") { jniLibs.srcDir("$buildDir/rustJniLibs/android") } and the release version of the app worked fine.

Ok, so it's a source set issue?

I commented the sourceSet lines out, confirmed that the test failed, then added android.sourceSets.getByName("main") { jniLibs.srcDir("$buildDir/rustJniLibs/android") }, and the test passed. Checked the readme for this plugin and confirmed that it doesn't say you need to add the build directory to the sourcesets.

... And then I decided to do some confirmation by doing a clean and test with no build-cache, and got an error about having duplicate libraries... (task mergeDebugJniLibFolders failed, duplicate resources, with both paths listed seeming to point to the same file.) I removed the sourceset addition line (so no manual adding to the source set and it worked again.

So, ok, we've got some serious build cache interference here and most of what's above is suspect.

In the end I commented out everything except the cargo configuration block and the "depends on CargoBuild" block". So just this:

cargo {
...
}
tasks.withType<com.android.build.gradle.tasks.MergeSourceSetFolders>().configureEach {
    if (this.name.contains("Jni")) {
        this.dependsOn(tasks.named("cargoBuild"))
    }
}

I ran that with gradle clean connectedDebugAndroidTest --no-build-cache and it worked, thought I succeeded.

But after some more testing it failed...

So I'm still not sure, but my best guess is that, when starting with a fully clean cache, the first build will fail and it'll continue to fail until you make a change to your build.gradle file which causes one of the tasks to run again. I think there's some kind of build dependency issue, maybe something changed related to when source sets are added. But I need to get going so that's where this stands at the moment...

Key takeaway: all tests here should be done without cache, after invalidating Android Studio's cache.

stan-irl commented 1 year ago

im seeing something kinda similar.

Im trying to run gradle on a docker container and if i run gradle build before gradle publish then I get the duplicate resource error. this doesnt happen on my local machine.

If I only run gradle publish then I dont get the error but the published aar doesnt contain any native libs. I checked the build artifacts and I can see that rustJniLibs is populated.

I dont use AGP at all though

stan-irl commented 1 year ago

oh wait im on 7.3.1. i have an issue with my tooling which is updating me to 7.4 i think

stan-irl commented 1 year ago

Theres an issue tracking it here. looks like an AGP bug?

stan-irl commented 1 year ago

I've solved my issue - it was actually the issue discussed here. Interesting that it can trigger the duplicate resource error.

SupernaviX commented 1 year ago

I was running into (the same?) issues with my project after upgrading to gradle 8.0.2, where changes to my rust files weren't getting picked up in the android app. I hacked together a fix by registering the cargoBuild output folders as inputs to the mergeDebugJniLibFolders and mergeReleaseJniLibFolders tasks.

project.afterEvaluate {
    // collect all the target dirs we compile to
    def jniTargetDirs = []
    tasks.withType(com.nishtahir.CargoBuildTask).forEach {
        jniTargetDirs += new File("$buildDir/rustJniLibs", it.toolchain.folder)
    }

    // make sure we rerun the "merge jni lib folders" tasks any time they change
    tasks.matching { it.name.matches(/merge.*JniLibFolders/) }.forEach {
        jniTargetDirs.forEach { dir -> it.inputs.dir(dir) }
    }
}
ghost commented 1 year ago

So a few weeks back I looked at the code and it's not in a great shape. IIRC pretty much everything is created afterEvaluation which makes it extra tricky to get things right.

The Android Gradle Plugin has stabilized a set of APIs that are meant for properly hooking in things like this (https://developer.android.com/studio/build/extend-agp). In the next few weeks I'm planning on basically re-implementing the Rust plugin in my project using those APIs (if anything so that we can upgrade past AGP 7.3) and I'll be able to share that code, which could probably be ported back into this plugin.

Also, it's worth noting that the tests for the plugin only check to make sure that the rust library is built, not whether it gets packaged into an apk or aar, which is why the extra tests that I tried to add weren't catching it.

chenzhenjia commented 1 year ago

I use this configuration to fix the problem that run will not package rustJniLibs

tasks.whenObjectAdded {
   if ((this.name == "mergeDebugJniLibFolders" || this.name == "mergeReleaseJniLibFolders")) {
        this.dependsOn("cargoBuild")
       // fix mergeDebugJniLibFolders  UP-TO-DATE
        this.inputs.dir(buildDir.resolve("rustJniLibs/android"))
    }
}
MarijnS95 commented 8 months ago

I couldn't get the above to work [^1] for various obscure reasons (timing between various callbacks, things being null, io::File::resolve() no longer existing in at least Java 21), but piecing together what was written in here and https://github.com/mozilla/rust-android-gradle/issues/85 lead me to either of these that seem to work consistently:

tasks.matching { it.name.matches(/merge.*JniLibFolders/) }.configureEach {
    it.inputs.dir(new File(buildDir, "rustJniLibs/android"))
    it.dependsOn("cargoBuild")
}
project.afterEvaluate {
    tasks.withType(com.nishtahir.CargoBuildTask).forEach { buildTask ->
        tasks.withType(com.android.build.gradle.tasks.MergeSourceSetFolders).configureEach {
            it.inputs.dir(new File(new File(buildDir, "rustJniLibs"), buildTask.toolchain.folder))
            it.dependsOn(buildTask)
        }
    }
}

Thanks all for posting your snippets! It's sad to see that this project has been abandoned, would be great to spend some time to get a fix for this merged instead.

[^1]: By "working" I am referring to the APK containing the latest up-to-date libxxx.so built from Rust code. The above comments fix any errors that were occurring but didn't update the final library in the APK unless a clean build is performed every time.

klcantrell commented 8 months ago

Thanks @MarijnS95, I kept running into java.lang.UnsatisfiedLinkError: Error looking up function errors trying to rerun my app after modifying the Rust source. The following snippet from https://github.com/mozilla/rust-android-gradle/issues/118#issuecomment-1777952460 is what fixed it for me.

tasks.matching { it.name.matches(/merge.*JniLibFolders/) }.configureEach {
    it.inputs.dir(new File(buildDir, "rustJniLibs/android"))
    it.dependsOn("cargoBuild")
}
ianthetechie commented 7 months ago

Adding my 2 bytes of information in case it helps others.

I am currently on AGP 8.1.2 and frequently hit the same issue as @klcantrell with UnsatisfiedLinkErrors. Strangely, if I simply ran the Android Tests target (or similar), after gradle clean, then all worked fine. But if I ran gradle build first, I hit the issues. This, in my case, had nothing to do with underlying rust source changes btw; it occurred simply as a result of certain gradle orderings.

Using the same fix mentioned in that comment seem to solve the issues for my case.

ngoquang2708 commented 7 months ago

My alternative is to use cmake with corrossion.