rust-mobile / android-rs-glue

Glue between Rust and Android
Apache License 2.0
905 stars 111 forks source link

Crates with inner attributes in root module no longer build #229

Closed philip-alldredge closed 5 years ago

philip-alldredge commented 5 years ago

The Problem

The build system merged in #223 fails to build crates which have inner attributes in the root module.

Incomplete Example

// Inner attributes
#![no_std]
#![cfg_attr(
not(any(
    feature = "example_feature"
    )),
    allow(dead_code, unused_extern_crates, unused_imports)
)]

// macro_use attribute is only allowed at the crate root.
#[macro_use]
extern crate lazy_static;

mod child_mod;

fn main() {
    child_mod::do_something();
}

References

There is a proposed solution at the bottom of the issue. The following sections are included for discussion and documentation purposes.

Things that don't work.

Nested module with path attribute

For most uses of include!, this can be worked around by declaring an inner module and use the path attribute such at:

#[path="../real/main.rs"]
mod inner;

However, this solution is not feasible for our use case because:

Embed main.rs into lib.rs

Simply moving the contents of the root module such as main.rs into lib.rs does not work. Since lib.rs is in a separate directory, it is unable to find any additional modules.

Hacky Solution

Example:

// Inner attributes
#![no_std]
#![cfg_attr(
not(any(
feature = "example_feature"
)),
    allow(dead_code, unused_extern_crates, unused_imports)
)]

// macro_use attribute is only allowed at the crate root.
#[macro_use]
extern crate lazy_static;

#[path="../real/"]
mod example {
    mod child_mod;

    // pub added to main
    pub fn main() {
        child_mod::do_something();
    }
}

use example::*;

Proposed Solution - Create temporary file in user's source directory.

This solution is one I avoided experimenting with because it requires producing files outside of the target folder. However, it is the simplest. Because of is simplicity, it should be less fragile. I believe this solution should be implemented unless another one is proposed.

Does anyone have thoughts about a better way to handle this case? @mb64

mb64 commented 5 years ago

My understanding of how it used to work is that it would tell rustc to build an executable, then intercept the linker call and link a dynamic library instead (and link in all the extra stuff, such as android_native_app_glue). This worked, but with an ugly linker wrapper.

With #223, it makes a new file lib.rs (roughly replacing glue_obj.rs) that looks like

include!("/path/to/src/main.rs"); // Side note: I'm pleasantly surprised this resolves modules correctly

#[no_mangle]
pub extern "C" fn android_main(...) { /* stuff with main() */ }

and it tells rustc to compile this as a static library.

It would be possible to go back to intercepting linker calls. There might be a nicer way to do it than a wrapper executable; I'm not familiar with the Cargo library.

Aside from copying the whole source directory (which might be bad if things include!("../../file_outside_of_source_dir")), I think a temporary file in the source directory might be the best solution. As long as it gets deleted afterward, and it doesn't conflict with other files in the source directory, it won't have a real impact on users.