Readme "Usage" feedback

[MaulingMonkey]

Just got up and running using this, and figured you wouldn't mind some feedback from fresh eyes :). Focusing entirely on the https://github.com/mozilla/rust-android-gradle#usage section:

• Listing non-android targets first threw me through a bit of a loop, and had me wondering if I needed targets for the host as well. Moving this under the first cargo { ... } section (but still before ./gradlew cargoBuild) might help make it clearer that, no really, these are for targets - and would also give the # for arm comments more context.

• The usage cargo { ... } section specifies no libname and thus won't work. Might change to:

  cargo {
    module  = "../rust"        // Contains Cargo.toml
    libname = "rust"           // Matches Cargo.toml's [package] name.
    targets = ["arm", "x86"]
  }

• "Next add the cargo configuration to android project." made me feel like I was guessing when I added the cargo section to my app/build.gradle. Maybe reword as "Next add the cargo configuration to your project's build.gradle." ?

• The project's build.gradle also needs: apply plugin: 'org.mozilla.rust-android-gradle.rust-android'

• Default Android Studio build tasks don't invoke cargoBuild at all, which confused the heck out of me. Might suggest adding a line like preBuild.dependsOn "cargoBuild" ? (I have no idea if preBuild is an appropriate task to (ab)use, or if assembleDebug and friends might be better suited.)

Redone, this might look like:

Usage

Add the plugin to your root build.gradle, like:

buildscript {
    repositories {
        maven {
            url "https://plugins.gradle.org/m2/"
        }
    }
    dependencies {
        classpath 'gradle.plugin.org.mozilla.rust-android-gradle:plugin:0.8.1'
    }
}

In your project's build.gradle, apply plugin and add the cargo configuration:

android { ... }

apply plugin: 'org.mozilla.rust-android-gradle.rust-android'

cargo {
    module  = "../rust"       // Or whatever directory contains your Cargo.toml
    libname = "rust"          // Or whatever matches Cargo.toml's [package] name.
    targets = ["arm", "x86"]  // See bellow for a longer list of options
}

Install the rust toolchains for your target platforms:

rustup target add armv7-linux-androideabi   # for arm
rustup target add i686-linux-android        # for x86
rustup target add aarch64-linux-android     # for arm64
rustup target add x86_64-linux-android      # for x86_64
rustup target add x86_64-unknown-linux-gnu  # for linux-x86-64
rustup target add x86_64-apple-darwin       # for macOS (darwin)
rustup target add x86_64-pc-windows-gnu     # for win32-x86-64-gnu
rustup target add x86_64-pc-windows-msvc    # for win32-x86-64-msvc
...

Finally, run the cargoBuild task to cross compile:

./gradlew cargoBuild

Or add it as a dependency to one of your other build tasks, to build your rust code when you normally build your project:

preBuild.dependsOn "cargoBuild"

[MaulingMonkey]

(I can create a PR if any of these suggestions sound good...?)

[ncalexan]

Just got up and running using this, and figured you wouldn't mind some feedback from fresh eyes :). Focusing entirely on the https://github.com/mozilla/rust-android-gradle#usage section:

We certainly would -- and I'm thrilled you got up and running!

* Listing non-android targets first threw me through a bit of a loop, and had me wondering if I needed targets for the _host_ as well.  Moving this under the first `cargo { ... }` section (but still before `./gradlew cargoBuild`) might help make it clearer that, no really, these _are_ for targets - and would also give the `# for arm` comments more context.

Agreed. The unit testing stuff is really half-baked: see https://github.com/ncalexan/rust-android-gradle/issues/13 for a little more context. It's just enough to get Mozilla's needs moving at the moment.

* The usage `cargo { ... }` section specifies no libname and thus won't work.  Might change to:
  ```groovy-gradle
  cargo {
      module  = "../rust"        // Contains Cargo.toml
      libname = "rust"           // Matches Cargo.toml's [package] name.
      targets = ["arm", "x86"]
  }
  ```

👍

* "Next add the `cargo` configuration to android project." made me feel like I was guessing when I added the cargo section to my app/build.gradle.  Maybe reword as "Next add the `cargo` configuration to your _project_'s `build.gradle`." ?

👍

* The project's build.gradle also needs: `apply plugin: 'org.mozilla.rust-android-gradle.rust-android'`

👍

* Default Android Studio build tasks don't invoke cargoBuild at all, which confused the heck out of me.  Might suggest adding a line like `preBuild.dependsOn "cargoBuild"` ?  (I have no idea if `preBuild` is an appropriate task to (ab)use, or if `assembleDebug` and friends might be better suited.)

Yes, this is janky. See also https://github.com/ncalexan/rust-android-gradle/issues/29 for more on how this could be improved. I honestly don't know what we have working in https://github.com/mozilla/application-services for this at this time.

Redone, this might look like:

Usage

Add the plugin to your root build.gradle, like:

buildscript {
    repositories {
        maven {
            url "https://plugins.gradle.org/m2/"
        }
    }
    dependencies {
        classpath 'gradle.plugin.org.mozilla.rust-android-gradle:plugin:0.8.1'
    }
}

In your project's build.gradle, apply plugin and add the cargo configuration:

android { ... }

apply plugin: 'org.mozilla.rust-android-gradle.rust-android'

cargo {
    module  = "../rust"       // Or whatever directory contains your Cargo.toml
    libname = "rust"          // Or whatever matches Cargo.toml's [package] name.
    targets = ["arm", "x86"]  // See bellow for a longer list of options
}

Install the rust toolchains for your target platforms:

rustup target add armv7-linux-androideabi   # for arm
rustup target add i686-linux-android        # for x86
rustup target add aarch64-linux-android     # for arm64
rustup target add x86_64-linux-android      # for x86_64
rustup target add x86_64-unknown-linux-gnu  # for linux-x86-64
rustup target add x86_64-apple-darwin       # for macOS (darwin)
rustup target add x86_64-pc-windows-gnu     # for win32-x86-64-gnu
rustup target add x86_64-pc-windows-msvc    # for win32-x86-64-msvc
...

Finally, run the cargoBuild task to cross compile:

./gradlew cargoBuild

Or add it as a dependency to one of your other build tasks, to build your rust code when you normally build your project:

preBuild.dependsOn "cargoBuild"

I'd happily take a PR... Thanks again!

[MaulingMonkey]

See also ncalexan/rust-android-gradle#29 for more on how this could be improved.

Unforunately my gradle knowledge is very rusty... I'm not able to help out too much there, I think.

I honestly don't know what we have working in https://github.com/mozilla/application-services for this at this time.

Me either 😄

I'd happily take a PR... Thanks again!

Done! 👍

[thomcc]

A-S uses this https://github.com/mozilla/application-services/blob/c2b7f97ab43bfcf7b66b9745c4b072efc0317cbd/megazords/full/android/build.gradle#L90-L100 rather than preBuild.dependsOn (still). I think the main benefit over hooking onto preBuild is avoiding performing the cargo build invocation in cases where it's not necessary.