Basic Android Toodle app + Rust JNA bindings #30

grigoryk · 2017-11-24T11:39:31Z

PR state as of 29/12/17 PST:

skeleton android app with "list" and "new item" activities
android logging support from Rust
JNA interface into Rust which allows creating items from Java and registers a callback that listens for new items
item sets are passed back to Java and are displayed as a list
an attempt to avoid leaking memory at the interface layer

TODO:

rebase on top of mentat store branch - no more SQL!
expand "ItemJNA" struct to also have due_date and completion_date
UI support for modifying these dates
label support

…e changes for kotlin

fluffyemily

I've been trying to compile this but when I come to building the android lib I get the following error:

cargo build --target i686-linux-android --release
   Compiling libsqlite3-sys v0.8.1
error: failed to run custom build command for `libsqlite3-sys v0.8.1`
process didn't exit successfully: `/Users/emilytoop/Development/cross-platform-rust/complex/cargo/target/release/build/libsqlite3-sys-af5f505c0fc46472/build-script-build` (exit code: 101)
--- stdout
TARGET = Some("i686-linux-android")
OPT_LEVEL = Some("3")
TARGET = Some("i686-linux-android")
HOST = Some("x86_64-apple-darwin")
TARGET = Some("i686-linux-android")
TARGET = Some("i686-linux-android")
HOST = Some("x86_64-apple-darwin")
CC_i686-linux-android = None
CC_i686_linux_android = None
TARGET_CC = None
CC = None
TARGET = Some("i686-linux-android")
HOST = Some("x86_64-apple-darwin")
CFLAGS_i686-linux-android = None
CFLAGS_i686_linux_android = None
TARGET_CFLAGS = None
CFLAGS = None
DEBUG = Some("false")
running: "i686-linux-android-gcc" "-O3" "-ffunction-sections" "-fdata-sections" "-fPIC" "-m32" "-DSQLITE_CORE" "-DSQLITE_DEFAULT_FOREIGN_KEYS=1" "-DSQLITE_ENABLE_API_ARMOR" "-DSQLITE_ENABLE_COLUMN_METADATA" "-DSQLITE_ENABLE_DBSTAT_VTAB" "-DSQLITE_ENABLE_FTS3" "-DSQLITE_ENABLE_FTS3_PARENTHESIS" "-DSQLITE_ENABLE_FTS5" "-DSQLITE_ENABLE_JSON1" "-DSQLITE_ENABLE_LOAD_EXTENSION=1" "-DSQLITE_ENABLE_MEMORY_MANAGEMENT" "-DSQLITE_ENABLE_RTREE" "-DSQLITE_ENABLE_STAT2" "-DSQLITE_ENABLE_STAT4" "-DSQLITE_HAVE_ISNAN" "-DSQLITE_SOUNDEX" "-DSQLITE_THREADSAFE=1" "-DSQLITE_USE_URI" "-DHAVE_USLEEP=1" "-Wall" "-Wextra" "-o" "/Users/emilytoop/Development/cross-platform-rust/complex/cargo/target/i686-linux-android/release/build/libsqlite3-sys-9c6be82b1e4f932e/out/sqlite3/sqlite3.o" "-c" "sqlite3/sqlite3.c"

--- stderr
thread 'main' panicked at '
Internal error occurred: Failed to find tool. Is `i686-linux-android-gcc` installed?

I've updated my Android Studio, my NDK and my build tools to try and resolve this, but it looks as if it's a problem with one of our dependencies. Did you encounter this? And if you did, how did you solve it?

fluffyemily · 2017-11-27T13:14:23Z

complex/cargo/src/lib.rs

+
+    #[no_mangle]
+    pub unsafe extern fn Java_com_mozilla_toodle_RustToodle_newToodle(env: JNIEnv, _: JClass, db_path: JString) -> jlong {
+        log("newToodle!");


Why not just pass the args into the existing new_toodle function to save having to replicate code? I think the fewer places we have to make changes the better.

fluffyemily · 2017-11-27T13:18:16Z

complex/cargo/list/src/items.rs

+
+    #[no_mangle]
+    pub unsafe extern fn Java_com_mozilla_toodle_RustToodle_itemSetDueDate(env: JNIEnv, _: JClass, item: *mut Item, due_date: JValue) {
+        let item = &mut*item;


again, just call item_set_due_date rather than having to do it all again here.

grigoryk · 2017-11-27T19:11:19Z

@fluffyemily for the build issue, I had to add the NDK/bin directories into my PATH.

I'm not sure why, this isn't what I'd expect to have to do (why doesn't .cargo/config work?). Also, a side-effect is that doing so breaks a regular Firefox build, for me anyway (but that's easily remedied by adjusting your PATH again).

grigoryk · 2017-11-29T07:33:37Z

@fluffyemily check out a basic JNA integration.

It all changed, need another review.

grigoryk · 2017-11-30T02:38:55Z

This really needs a rebase on top of @fluffyemily's mentat branch ASAP before things diverge too far.

fluffyemily

Just a few concerns about calling unsafe code from inside ListManager.

fluffyemily · 2017-11-30T16:31:15Z

complex/cargo/ffi-utils/src/lib.rs

@@ -24,6 +25,11 @@ pub mod strings {
        r_str.to_string()
    }

+    pub fn to_string(pointer: *const c_char) -> String {


this is exactly the same as c_char_to_string. Either overwrite the old one or update it to this. No point having 2 functions doing exactly the same thing,

fluffyemily · 2017-11-30T16:32:23Z

complex/cargo/list/src/items.rs

@@ -61,6 +62,16 @@ pub unsafe extern "C" fn item_destroy(item: *mut Item) {
    let _ = Box::from_raw(item);
 }

+// TODO Can these simpler android methods also work for swift?


yes, they can. Let's all use the same FFI if possible.

fluffyemily · 2017-11-30T16:41:32Z

complex/cargo/list/src/items.rs

+}
+
+pub extern "C" fn a_item_destroy(_: Box<Item>) {
+    // Rust will clean up for us automatically, since we own the Item.


This is not the case for iOS so we need to leave a destroy function around for that.

fluffyemily · 2017-11-30T16:43:50Z

complex/cargo/list/src/items.rs

+}
+
+#[no_mangle]
+pub unsafe extern "C" fn a_item_set_due_date(item: &mut Item, due_date: *const size_t) {


This is essentially the same code. How about we call a_item_set_due_date after casting the *mut Item pointer to a reference in item_set_due_date.

fluffyemily · 2017-11-30T16:46:38Z

complex/cargo/list/src/labels.rs

@@ -34,6 +34,11 @@ pub unsafe extern "C" fn label_destroy(label: *mut Label) {
    let _ = Box::from_raw(label);
 }

+#[no_mangle]
+pub unsafe extern "C" fn a_label_destroy(_: Box<Label>) {
+    // magic!


There are compiler flags for different architectures. #[cfg(target_os="android")] will compile a function just for Android, for example. For functions that have to be different between iOS and Android we should use these flags to ensure not everything is compiled every time and move as much shared code as possible into functions that these architecture specific ones can call into.

fluffyemily · 2017-11-30T16:48:37Z

complex/cargo/list/src/lib.rs

+use time::Timespec;
+
+#[no_mangle]
+pub unsafe extern "C" fn a_list_manager_create_item(manager: *const Arc<ListManager>, name: *const c_char, due_date: *const time_t) {


At this point the iOS app is broken already so I don't mind if you make a breaking change, as long as you check with Kit first to make sure he's not relying on it.

fluffyemily · 2017-11-30T17:07:54Z

complex/cargo/list/src/lib.rs

@@ -204,6 +207,23 @@ impl ListManager {
        }
    }

+    pub fn fetch_item_uuids(&self) -> Vec<*const c_char> {


I am not a huge fan of c types being used inside ListManager. string_to_c_char calls unsafe code and unsafe code should be kept to the FFI boundary as much as possible which allows this function to be called from elsewhere in Rust without safety worries.

Ah, good call! I'll revise this.

fluffyemily · 2017-11-30T17:10:54Z

complex/cargo/list/src/items.rs

@@ -32,6 +33,15 @@ pub struct Item {
    pub labels: Vec<Label>,
 }

+#[repr(C)]
+#[derive(Debug)]
+pub struct ItemJNA {


This is something I have been considering doing for all objects we pass over the boundary - converting them from complex Rust types to C structs. This would also make handling the objects in Swift much easier.

fluffyemily · 2017-11-30T17:14:58Z

complex/cargo/list/src/lib.rs

@@ -224,6 +225,44 @@ impl ListManager {
        uuids
    }

+    pub fn fetch_all_items(&self) -> Vec<ItemJNA> {


Take a look at the Into trait. https://doc.rust-lang.org/std/convert/trait.Into.html

This is a trait you can implement on structs to convert one thing into another. Think about implementing Into for Item to ItemJNA and Vec<Item> to Vec<ItemJNA>. That way the ListManager function can deal entirely with safe types and code and you can call list.into() from inside your FFI function when Boxing it for return.

What's nice about Into is that implementing Into automatically implements From which means when we pass an ItemJNA back into we can call item.from() to turn it back into an Item.

Grisha Kruglov added 15 commits November 24, 2017 03:00

Rename to Toodle, update gradle plugin version

260be9c

Rename classes

2943212

Proper package name

fce43be

RustToodle lifecycle

4abfe5c

'build all' helper script

b1baae2

symlinks for libtoodle.so

52ef88d

debug builds

cd30870

final rename

bf6eecf

Remove login manager stuff from java

7327b06

basic jni interface with android logging

e93dcce

comment

57bc234

Fix wrong SQL CREATE TABLE statement

3509efc

Remove debug comments

581187c

rename layout, misc AS stuff

e58ecef

Basic recyclerview list

388db42

grigoryk requested a review from fluffyemily November 24, 2017 11:39

Grisha Kruglov added 2 commits November 25, 2017 20:55

Dummy 'add' button + resources

6d347c8

new item activity & form; JNI bindings for item creation; build.gradl…

33c628b

…e changes for kotlin

grigoryk changed the title ~~Android app scaffolding + logging from rust~~ Basic Android Toodle app + Rust jni bindings Nov 27, 2017

fluffyemily previously requested changes Nov 27, 2017

View reviewed changes

Grisha Kruglov added 4 commits November 28, 2017 20:55

Switch to JNA; semi-clear java, messy rust

d60cd06

clean up

891cdea

more clean up

1b59b2e

basic native->java callback chain when new items appear

142a01e

Grisha Kruglov added 4 commits November 29, 2017 16:38

display a list of all uuids in the database

a0e74a5

get all items, display them

493d458

java side of the allItems

8feed34

slightly better memory management

0d40963

fluffyemily suggested changes Nov 30, 2017

View reviewed changes

grigoryk changed the title ~~Basic Android Toodle app + Rust jni bindings~~ Basic Android Toodle app + Rust JNA bindings Nov 30, 2017

ide churn.. should this be checked in even?

16d79f4

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Basic Android Toodle app + Rust JNA bindings #30

Basic Android Toodle app + Rust JNA bindings #30

grigoryk commented Nov 24, 2017 •

edited

Loading

fluffyemily left a comment

fluffyemily Nov 27, 2017

fluffyemily Nov 27, 2017

grigoryk commented Nov 27, 2017

grigoryk commented Nov 29, 2017

grigoryk commented Nov 30, 2017

fluffyemily left a comment

fluffyemily Nov 30, 2017

fluffyemily Nov 30, 2017

fluffyemily Nov 30, 2017

fluffyemily Nov 30, 2017

fluffyemily Nov 30, 2017

fluffyemily Nov 30, 2017

fluffyemily Nov 30, 2017

grigoryk Nov 30, 2017

fluffyemily Nov 30, 2017

fluffyemily Nov 30, 2017

fluffyemily Nov 30, 2017

Basic Android Toodle app + Rust JNA bindings #30

Are you sure you want to change the base?

Basic Android Toodle app + Rust JNA bindings #30

Conversation

grigoryk commented Nov 24, 2017 • edited Loading

fluffyemily left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

grigoryk commented Nov 27, 2017

grigoryk commented Nov 29, 2017

grigoryk commented Nov 30, 2017

fluffyemily left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

grigoryk commented Nov 24, 2017 •

edited

Loading