ndk_glue::main assumes ndk-glue is in Cargo.toml

[francesca64]

My use case involves exporting ndk-glue from another crate, so proc-macro-crate being used to get the crate name makes this macro unusable for me.

It would be nice to have an option to manually specify the crate name, though honestly I'd be content just to have the ndk_glue! macro still be available as an alternative...

[dvc94ch]

I like the ndk_glue! macro, it's simple and concise. This does seem like a case of over engineering to me, but since @katyo 's pr was clean and well tested, I didn't feel comfortable rejecting it.

[dvc94ch]

@francesca64 can you come up with a solution that makes everyone happy?

[francesca64]

@dvc94ch I believe so. I recall you mentioned that you no longer depend on Rust's Android stack? I work for a company that intends to use this library in production, so I can definitely take a more active role here if you'd like.

@katyo can you make a stronger case for the merits of your proc macro? It replaces a ~20 line declarative macro with ~750 new lines of code, which is nothing to sneeze at. While it does look rather clean, and I commend you for adding extensive tests, having more code nonetheless represents a maintenance burden.

I love proc macros, but their complexity can be an obstacle for new contributors (or just for people reading the code), and syn is becoming increasingly unpopular with the "fewer dependencies for faster build times" crowd. However, I can't argue against the power proc macros offer, so I'm just asking you to make the case that said power is warranted. It looks like you've made logging and backtraces configurable, which I imagine would be difficult to implement as elegantly using declarative macros. But even ignoring that, are you confident that having an attribute is substantially more convenient for users?

[dvc94ch]

@francesca64 that would be great. I'm working on completely different stuff at the moment so if you want to take over that's great.

[katyo]

@francesca64 Attribute macros is a proper way to apply transformations to Rust code. Almost all crates use something like that (see async_std::main, tokio::main).
I planned to re-implement ndk-macro using darling. In spite that it little bit breaks current API (ex. macro will accepts backtrace = "full" instead of backtrace(full)) but source code will looks much shorter and simpler.

Because currently the things like $crate is missing for proc_macro I use proc_macro_crate::crate_name() to get the name of ndk-glue crate for code gen. Because proc_macro_crate lookups crates in current Cargo.toml so this crate should be listed there. Alternatively we can add extra property to ndk_glue::main which overrides path to re-exported ndk_glue explicitly.

[katyo]

I completely re-implemented ndk_glue::main macro using darling in pr #54.

[katyo]

@francesca64 I does not sense your use case so clear. Can you provide sample code to reproduce this issue? I may try to fix or work around it.

[francesca64]

@katyo

I planned to re-implement ndk-macro using darling. In spite that it little bit breaks current API (ex. macro will accepts backtrace = "full" instead of backtrace(full)) but source code will looks much shorter and simpler.

I'd be happy with that, especially since darling is a cute name.

Because proc_macro_crate lookups crates in current Cargo.toml so this crate should be listed there. Alternatively we can add extra property to ndk_glue::main which overrides path to re-exported ndk_glue explicitly.

Adding it to Cargo.toml isn't possible for my use case, so I'd be very happy to have an optional property added for that.

I completely re-implemented ndk_glue::main macro using darling in pr #54.

Awesome! I'll review that.

Can you provide sample code to reproduce this issue? I may try to fix or work around it.

Thanks for the offer. I use an attribute macro, which presently expands into this:

#[cfg(target_os = "android")]
use framework::platform_ffi::android::ndk_glue;

fn stop_unwind<F: FnOnce() -> T, T>(f: F) -> T {
    match std::panic::catch_unwind(std::panic::AssertUnwindSafe(f)) {
        Ok(t) => t,
        Err(err) => {
            eprintln!("attempt to unwind out of `rust` with err: {:?}", err);
            std::process::abort()
        }
    }
}

// As an aside, if `ndk_glue::main` can accept `extern` functions, 
// then I won't need to have this extra function here.
fn _start_app() {
    #func
    stop_unwind(|| #name());
}

#[no_mangle]
#[inline(never)]
pub extern "C" fn start_app() {
    _start_app()
}

#[cfg(target_os = "android")]
ndk_glue::ndk_glue!(_start_app);

It generates boilerplate for both iOS and Android. The user depends on our framework, but for the sake of convenience, doesn't directly depend on ndk-glue.

[katyo]

Well, we can just use default crate names when it is not found.
Also I can introduce corresponding props to override this names.

[francesca64]

Fixed by #54