Cargo apk build system rework and other enhancements

[philip-alldredge]

See #212

Changes

• Updates dependencies to newer version. For example uses serde instead of rustc-serialize.
• Reworks build process
  - Use a custom Executor with cargo::ops::compile_with_exec and injected code build a static libraries for each build target(ABI) and cargo target(binaries and examples). This allows support for multiple cargo targets.
  - Uses ndk-build to build the shared library for each build target. This gets rid of the need for the linker work around and fixes issues related to compatibility with newer NDKs which do not include GCC.
  - Updates supported build targets to match those supported by the current NDK.
  - Minor change to injector glue to work with new system for calling main().
  - Use SDK provided tools to build and sign APKs. This removes the gradle dependency.
  - If an Android debug keystore exists then it uses this to sign the APK. Otherwise, it creates it using the keytool provided by the Java JRE or JDK. This was previously handled by the gradle build.
• Fixes injected glue warnings related to 2018 edition.
• Formats code using rustfmt. This does result in extra noise in the diff but it should help when reviewing future contributions by myself or others.
• Adds support for building and installing multiple cargo targets.
• Adds support for customing uses-feature and uses-permission portions of Android manifest via Cargo.toml.
• Removes hard coded uses-permission from manifest. Thie avoid unintended permission request.
• Adds an example demonstrating additional settings in Cargo.toml.
• Adds an example of a crate which includes multiple cargo targets(binaries and examples).
• Adds an integration test which uses cargo apk to build all the examples
• Updates android_version(compile version) to 29 and target sdk version to 29. The min_sdk_version remains at 18.
• Updates Dockerfile to be based on the latest official rust docker image. It also updates to the latest Android SDK and NDK. This makes it easier to update SDK and NDK versions moving forward.
• Updates Dockerfile to run tests as part of the build process. Recommend building it as part of CI at some point.
• Updates README to match changes. Links to and references the docker image provided at https://hub.docker.com/r/philipalldredge/cargo-apk. If merged, it would likely be better to setup an official rust-windowing organization on github and automatically build the docker image based on that. The linked docker image automatically updates based on my fork.

Tested using the window example in https://github.com/mb64/winit/tree/android-eventloop-2.0 referenced in rust-windowing/winit#1001

Potential Compatibility Changes

Most of this shouldn't be a factor for users of the Docker image but it may require others to upgrade their Android SDK/NDK. I have not performed testing on a large number of Android SDK and NDK combinations.

• The default android version and target sdk version requires a newer Android platform. This prevents warnings when running on newer Android devices. The setting can be adjusted as appropriate. In general it is recommended to target the latest android devces.
• The default and supported build targets has changed. The default is to build on all targets supported by the current NDK. arm-linux-androideabi was removed. See README for supported targets.
• The removal of the hard coded uses-permission may result in minor issues when targeting older versions of android. Users will need to add the permission to the Cargo.toml. For newer version of android, users will also need to request permission at runtime using a Java API. A rust API that uses JNI to do that is needed but that is a separate concern from the build process.

Related Issues

Closes #212, #208, #191, #138, #123, #211, #202, #198, #192, #164, #134,
(Reason for some of the closing is removal of gradle. Some of them may have been resolved by other pull requests but should be resolved by this as well.)

Likely fixes: #109

[philip-alldredge]

This does not update the circleci configuration. I hadn't really looked at it. It looks like its tests have been failing for a while. This pull request rolls testing into the building of the docker image and removes the need for a base docker image. This should make it simpler to migrate to travis for consistency with other rust-windowing projects. All travis needs to do is build the docker image. On merging into master, it could push the docker image to docker hub and also publish to crates.io. Related to #221

[goddessfreya]

R? @mb64

[mb64]

I looked over most of the peripheral changes, without examining the specifics of the cargo-wizardry. I'll get back to those when I have the time – I'm not familiar with the Cargo library so it'll take a bit.

A few overall comments:

• I like the build procedure of Rust static library → ndk-build; it uses all the tools for what they're supposed to be used, and doesn't re-implement too much.
• I haven't built and tested it yet, but from a CLI usability standpoint, it'd be nice to resolve the issues from Propagate additional cargo build options #205.
• It's not a new change, but I don't like not supporting Android 17 and earlier. Again, I haven't tested it yet, but I have an old 4.2.2 (SDK 17) device lying around that I'll try it on.

Sorry that some of those bullet points weren't actually related to this PR. I'll try to get to the actual meat of the change tomorrow, look through the code and run it myself.

[mb64]

This seems odd. Why does rustc not support platforms before 18? This seems odd – I would think the compiler should be agnostic to the SDK version required. It would be great to support below 18, because otherwise we're forcing any android apps using the glue out of more than 3 percent of all devices.

And even if it doesn't support compiling for SDK versions below 18, does that necessarily mean that it can't support them (compile with higher, support lower)? The comments in the AndroidConfig from before indicate that this might be the case.

[philip-alldredge]

The reference to the minimum being 18 was in the original readme. I suspect that the current rustc will support any platform version as long as the device runs a supported ABI. I found this this referring to platform 18 with regards to rustc. It appears to be an issue with the combination to armv7 and platform < 18 that has been fixed.

https://github.com/rust-lang/rust/issues/9700

[philip-alldredge]

I think most of the comments should be addressed now. I'm not sure what we should do about the platform 18 issue. Like I mentioned in the comment, I suspect that the issue has been resolved. However, I'm unable to test it. If we can confirm that it works with lower platform levels, then we should change the default setting and those comments.

@mb64 can you retest #205? I am unable to reproduce. I only tested the color option and it seems to work as expected.

[mb64]

A few more things:

• Yeah, this seems to propagate arguments to Cargo fine.
• If you want the examples to compile fine with cargo build, try a .cargo/config file with something like:

  [build]
  target = "arm-linux-androideabi"

• When the package name has a dash in it (I just tried with build-test), the generated static library has underscores instead (libbuild_test.a). Cargo apk, however, looks for libbuild-test.a.

[philip-alldredge]

Good catch on the build name. Fix has been pushed.

[mb64]

Despite all my nitpicking, I'm overall really happy with this – it's nicer to use than before, and it builds faster and fixes a lot of things (including #172, which is not among the listed issues). Thank you for your work on it.

I'd like to merge this, but I'll wait until I get your go-ahed in case there's anything else you want to add/change/discuss.

[philip-alldredge]

@mb64 I made some additional changes with regards to how the package_name and label values are handled. Secondary targets no longer use the value defined in package.metadata.android and will always use the default value unless otherwise specified. I also modified the default package name for examples. It made sense(to me) that the label behaved this way. The package_name is more important because there could be issued related to uniqueness otherwise. The readme has been updated to describe them.

Unless you object to those changes, I think this is ready to merge.

[philip-alldredge]

@mb64 actually, hold off of any merging. I'm looking into something.

[philip-alldredge]

The issue has been fixed. I was concerned that some of the config values in one of the examples wasn't what I expected. It was due to using the wrong name for a secondary target. It would be nice to report a warning in such cases but I think that can wait.

[philip-alldredge]

I had a few minor issues with the examples and readme that were detected once deny_unknown_fields was enabled. I was in a hurry and forgot to run all the tests. The fixes have been pushed and all tests pass.

[mb64]

I'm merging this.

I'll create an issue to list the further changes.