Skip to content
This repository has been archived by the owner on Jul 3, 2020. It is now read-only.

The apk builder is now a Cargo subcommand #75

Merged
merged 29 commits into from
Apr 20, 2016
Merged

The apk builder is now a Cargo subcommand #75

merged 29 commits into from
Apr 20, 2016

Conversation

tomaka
Copy link
Contributor

@tomaka tomaka commented Apr 17, 2016

Turns the apk-builder into cargo-apk, which provides the cargo apk Cargo subcommand.

Using it will:

  • Build a android-artifacts directory inside target.
  • Compile your package for the given list of platforms (not configurable yet, defaults to just arm-android).
  • Compile android_native_glue for each platform.
  • Compile some glue that will make android_main call Rust's main.
  • Setup the build directory and run ant.

To do:

  • Add parsing of a CargoApk.toml file that can indicate various information about the package.
  • Add parsing the .cargo/config file to find out the path to the Android SDK and NDK.
  • Add various subcommands, like cargo apk install that would invoke adb.
  • Make everything cleaner. It can fail in several ways, and error messages aren't very explicit.

The idea is that it should now be much simpler to build an apk by just running cargo install cargo-apk followed with cargo apk, without having to modify your source code or anything fancy.
You would be able to configure your build with a CargoApk.toml file whose content remains to be determined. Another advantage is that the build directory wouldn't be destroyed, making it possible to easily tweak and debug builds.

The setup would also be greatly simplified by just having to install the NDK, the SDK, and a compatible rustc thanks to rustup. An NDK standalone toolchain would no longer be needed.

cc @ozkriff @larsbergstrom (@brson and @alexcrichton maybe as well?)

@tomaka
Copy link
Contributor Author

tomaka commented Apr 17, 2016

My initial idea to inject code was to build a file that would contain:

include!("/path/to/main.rs");

// injected code
pub extern fn android_main() { ... }

However this doesn't work in practice because include_bytes! and include_str! don't work anymore in the original source code.

The only proper way I can see is building a plugin and injecting it with -Z extra-plugins, but that means we would require nightly Rust.

The other technical difficulty is injecting android_glue in the project.
For the moment my idea is to run cargo rustc -p android_glue -- --print file-names.

There are then two possibilities: either the project or one of its dependencies depend on android_glue and we can inject the already-compiled library with --extern android_glue=..., or cargo rustc returns an error and we need to compile android_glue in a temporary directory before passing it with --extern.

@tomaka
Copy link
Contributor Author

tomaka commented Apr 17, 2016

Thinking about it, I think the best way is in fact to rework the way the glue works.

The glue would be split in two parts:

  • One library used as a regular dependency by Rust code and published on crates.io.
  • One library manually injected by cargo-apk.

The former would query stuff from the latter through unmangled symbols.

This way we are guaranteed that it will work every time.

@tomaka
Copy link
Contributor Author

tomaka commented Apr 17, 2016

#rust-internals told me that plugins couldn't inject code.

I'm going to try using traditional object files and linking, but I'm a bit dubious seeing how complex it was the last time I tried.

@tomaka
Copy link
Contributor Author

tomaka commented Apr 17, 2016

Things should work fine in theory, but I'm going to test it properly tomorrow.

@alexcrichton
Copy link

Awesome! This sounds like a pretty sweet subcommand. Let me know if there's anything Cargo can do to make this easier!

Add parsing of a CargoApk.toml file that can indicate various information about the package.

In Cargo we'd actually love to add a whitelisted area of [package.metadata] where arbitrary metadata could be stored and not warned about. Would that suffice for this use case? That way all configuration is in Cargo.toml and this subcommand could read something like [package.metadata.android]?

Add parsing the .cargo/config file to find out the path to the Android SDK and NDK.

I might recommend directly linking to the cargo crate and using the Config type to read out this information, but whatever works works!

@tomaka
Copy link
Contributor Author

tomaka commented Apr 17, 2016

In Cargo we'd actually love to add a whitelisted area of [package.metadata] where arbitrary metadata could be stored and not warned about. Would that suffice for this use case? That way all configuration is in Cargo.toml and this subcommand could read something like [package.metadata.android]?

Sure. The advantage of using CargoApk.toml is that I think that what is specific to cargo-apk should be easy to identify. But if everything is in its own subsection, I guess it's good.

I might recommend directly linking to the cargo crate and using the Config type to read out this information, but whatever works works!

That's what I wanted to do initially. However I think cargo itself is too annoying to build on Windows right now because you need to have a C compiler, cmake, openssl, and maybe others. If cargo-apk depends on cargo, then cargo-apk will be annoying to build as well.

@alexcrichton
Copy link

Yeah there's definitely some parts of Cargo that could be split out, like the configuration, errors, etc. Note though that OpenSSL isn't required on Windows, just cmake + a C compiler.

@tomaka
Copy link
Contributor Author

tomaka commented Apr 18, 2016

After the fixes I just pushed, it is now possible to do:

cargo new --bin test
cd test
cargo apk
adb install target/android-artifacts/build/rust-android-debug.apk

And the package works (and shows a black screen).

Adding cargo apk install is probably the next step.

@tomaka
Copy link
Contributor Author

tomaka commented Apr 19, 2016

Got vulkano's triangle example running after a few tweaks (the few tweaks being necessary because the example itself is badly written).

@brson
Copy link

brson commented Apr 19, 2016

Sweet.

@tomaka
Copy link
Contributor Author

tomaka commented Apr 20, 2016

I'm going to merge this since this code is already better than the old one, and Servo has their own fork of this repo. I don't think I'm going to break anyone's workflow, since the apk-builder is usually part of a git submodule and not a Cargo dependency.

Last commit pre-rework is 8aa355a if anyone wants to jump back to it.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants