mozilla / rust-android-gradle

Apache License 2.0
1.03k stars 67 forks source link

[Question] `targetDirectory` does not work? #28

Open nisrulz opened 4 years ago

nisrulz commented 4 years ago

https://github.com/mozilla/rust-android-gradle#targetdirectory

I tried everything I could think of, but I couldn't get the *.so libs to appear in the specified path. Eventually I had to write my own groovy task to copy those files by creating a map. :man_shrugging:

Anything I am missing.

OliverCulleyDeLange commented 4 years ago

Looks like you can use the CARGO_TARGET_DIR env as a work around util this is fixed.

cargo {
 ...
    exec { spec, toolchain ->
        def home = project.gradle.gradleUserHomeDir
        spec.environment("CARGO_TARGET_DIR", "$home/cargo-target")
    }
}
OliverCulleyDeLange commented 4 years ago

I'm revisiting this as i've been having some problems.

Everything looked ok to me previously as the rustJniLibs folder still existed in my build directory. However, when i ran in CI - my app didn't contain the binaries. I added some logging to the plugin, and for some reason, the CARGO_TARGET_DIR env var is not being found by the plugin despite being set.

I'm using WSL and this complicates matters somewhat as CI is OSX so i have to support both.

My above suggestion works to the point of telling CARGO where to output files. However when the plugin tries to copy the files, it looks for target directory in this order:

  1. local.properties for rust.cargoTargetDir
  2. System level environment variables for CARGO_TARGET_DIR
  3. cargo { targetDirectory = "foo" }} property set in build.gradle

This is failing because i was only setting the environment variable for the exec spec, therefore it is not set at System level. This means the plugin is looking for outputs in the default location and not finding them.

Ideally, when we set targetDirectory in the cargo gradle config, it should pass that to the cargo command via --target-dir link rather than relying on CARGO_TARGET_DIR.

Also targetDirectory shouldn't be relative to the gradle project root as this makes it awkward to use for example "${project.gradle.gradleUserHomeDir}/cargo-out" as a root. If people want a relative path they can specify that in the gradle config like:

cargo {
    targetDirectory =  "${project.projectDir}/someroot"
}
OliverCulleyDeLange commented 4 years ago

I had a quick go at implementing this, but specifying a windows path which is then passed throught to WSL doesn't work for obvious reasons.

My hacky workaround at the moment is:

cargo {
    ...
    def cargoTarget = "${project.gradle.gradleUserHomeDir}/android-engine-build-outputs"
    def relativeCargoTarget = Paths.get(project.projectDir.absolutePath).relativize(Paths.get(cargoTarget))
    // Tells the plugin where to find cargo outputs, but its relative to project root
    targetDirectory = relativeCargoTarget
    exec { spec, toolchain ->
        println "Gradle plugin will look for cargo outputs here: $relativeCargoTarget"
        println "Telling cargo to output here: CARGO_TARGET_DIR: $cargoTarget"
        spec.environment("CARGO_TARGET_DIR", cargoTarget)
    }
}
AquilesCanta commented 1 year ago

I'm also affected by this but managed to work around this via https://github.com/mozilla/rust-android-gradle/issues/28#issuecomment-636881167. To be clear, what I'm observing is that changing cargo.targetDirectory doesn't have any effect.

If targetDirectory is indeed not working and we don't foresee a fix in the near future, it might make sense removing it from the README in order to save newcomers some time. Might be misunderstanding the situation though. Please ignore if so.

EDIT: I think my comment about "changing cargo.targetDirectory doesn't have any effect" is not accurate, and actually both targetDirectory and CARGO_TARGET_DIR need to be used simultaneously. I'm not sure what happened in my setup but at some point gradle stopped finding my .so files. Defining both targetDirectory and CARGO_TARGET_DIR as described in https://github.com/mozilla/rust-android-gradle/issues/28#issuecomment-638223295 seems to fix this.