Add no_std feature

[nicholastmosher]

Hey there! I've been wanting to have capnproto-rust available on no_std for awhile (per #71), and I've given it a few shots in the past but I think now I'm actually pretty close to nailing it. I wanted to open up a PR to get some feedback on my approach and ask some questions about things I've gotten hung up on.

Things I've done:

• Added a no_std feature flag to the capnp crate. This feature is used in a few #[cfg(...)] conditional compilations around the codebase.
• Included the core_io crate as an optional dependency to be included when the no_std feature flag is set. This crate is mostly autogenerated in that it applies patches against std::io to take out all std usages and replace them using the alloc crate.
• Replaced all references to std:: within the capnp crate to use core:: instead. In capnp's lib.rs file, there is a conditional import based on #[cfg(feature = "no_std")]. If no_std is enabled, then the name core:: references core_io:: items. If no_std is not enabled, then core:: references std:: items. A similar strategy handles deciding between e.g. std::string and alloc::string and between std::str and alloc::str.

Problems I'm having now:

It seems that everything in the capnp crate is now building properly both when no_std is enabled and disabled. However, when I add the capnpc::CompilerCommand::new().file("schema.capnp").run().unwrap(); buildscript to a no_std project of mine, there's an interesting compilation problem. capnp needs to be compiled as no_std in order to comply with my no_std application, but since that is the case, the std-compiled capnpc seems to now be linking against the no_std-compiled capnp, whereas I think it should build a separate instance of capnp (std-compiled) to link against. The problem that this creates is that e.g. in capnpc/src/lib.rs:81: let mut p = command.spawn()?;, the spawn() call returns a Result in which the error type is std::io::Error. Typically, when capnp is std-compiled, it would implement From<std::io::Error> for capnp::Error. However, since it is linking against the no_std-compiled version of capnp, the implementation that is actually implemented is From<core_io::Error> for capnp::Error. It seems that I can get around this by simply mapping the error each time capnpc deals with an io::Result, e.g. like this:

let mut p = command.spawn().map_err(|err| capnp::Error::failed(err.to_string()))?;

There are only perhaps five or so instances of this problem, but I figured I should ask if anybody has any suggestions on a strategy to take. I don't think it makes sense to force the no_std requirements to leak up into capnpc, but I also don't necessarily want to destroy ergonomics by forcing the client to use map_err everywhere. I think I'll probably work on adding a commit that does do that, just in order to get a working solution together, but any alternative suggestions are certainly welcome!

Edit:

I forgot to mention, since the core_io crate is built by applying patches against certain builds of std::io, you'll need a fixed version of nightly rust to build it using the no_std feature. You should be able to satisfy it using this override which corresponds to the last patch of the core_io crate:

rustup override set nightly-2019-07-01

[dwrensha]

Thanks for working on this!
I'm a bit overwhelmed with other tasks at the moment, but I'd like to find time before too long to review this...

[nicholastmosher]

I've added a commit to resolve the incompatible Result types I mentioned above (mapping the capnp::Error to std::io::Error) but there's one instance of incompatible types that I've been unable to resolve. The tricky part is that the error does not manifest when you build this library crate, only when you include it into a no_std binary. To give a concrete example of this, I've uploaded my work-in-progress example to this branch in my project. (Note that you need to build that project with the capnproto-rust repository adjacent to it on your filesystem and with this branch no_std-20190701 checked out).

The error produced by building that project is the following:

cargo build
   Compiling capnpc v0.10.1 (/home/nick/Documents/capnproto-rust/capnpc)
error[E0277]: the trait bound `T: core_io::io::Read` is not satisfied
    --> /home/nick/Documents/capnproto-rust/capnpc/src/codegen.rs:1852:19
     |
1852 |     let message = serialize::read_message(&mut inp, capnp::message::ReaderOptions::new())?;
     |                   ^^^^^^^^^^^^^^^^^^^^^^^ the trait `core_io::io::Read` is not implemented for `T`
     |
     = help: consider adding a `where T: core_io::io::Read` bound
     = note: required by `capnp::serialize::read_message`
error: aborting due to previous error
For more information about this error, try `rustc --explain E0277`.
error: Could not compile `capnpc`.
To learn more, run the command again with --verbose.

If we look at the type signature of the parent function, we'll see this:

// capnpc/src/codegen.rs:1846
pub fn generate_code<T>(mut inp: T, out_dir: &::std::path::Path) -> ::capnp::Result<()>
    where T: ::std::io::Read
{ ... }

We see that the inp value implements std::io::Read, but that serialize::read_message(&mut inp, ...) requires a type that satisfies core_io::io::Read. I believe this discrepancy is due to the problem I described before (disclaimer: I'm not certain I'm interpreting the problem correctly, but here's my take on it). Since the capnp crate is being used as a dependency in the no_std binary, it is being compiled with the new no_std feature flag and therefore all of the methods in capnp are being compiled using the core_io::io::Read trait. However, the build-time dependency on capnp should be compiled for a regular target (i.e. not no_std), which would mean that the capnp instance running at build-time should be compiled to use std::io::Read. Since this is not happening, we're seeing this type-mismatch error.

If this is indeed what's happening, then either I'm missing a configuration option which can separate these dependencies, or it's a use case which cargo isn't equipped to handle at the moment.

[dwrensha]

Does it work if you explicitly specify your target when building, e.g. cargo build --target x86_64-unknown-linux-gnu ...? This reminds me of https://github.com/rust-fuzz/cargo-fuzz/blob/dd1de87a47ac21914256e477e953ad781609ed5c/src/main.rs#L295-L296

[nicholastmosher]

It doesn't seem to help. In my example project (atsam4s16b-examples) I tried building using cargo build --target thumbv7em-none-eabi and it still failed with

   Compiling capnpc v0.10.1 (/home/nick/Documents/capnproto-rust/capnpc)
error[E0277]: the trait bound `T: core_io::io::Read` is not satisfied
    --> /home/nick/Documents/capnproto-rust/capnpc/src/codegen.rs:1852:19
     |
1852 |     let message = serialize::read_message(&mut inp, capnp::message::ReaderOptions::new())?;
     |                   ^^^^^^^^^^^^^^^^^^^^^^^ the trait `core_io::io::Read` is not implemented for `T`
     |
     = help: consider adding a `where T: core_io::io::Read` bound
     = note: required by `capnp::serialize::read_message`
error: aborting due to previous error
For more information about this error, try `rustc --explain E0277`.
error: Could not compile `capnpc`.

I also tried building with cargo build --target x86_64-unknown-linux-gnu with the same result.

[nicholastmosher]

I really think this is a conditional compilation problem. I made a post on the users forum to see if anybody has any suggestions.

[nicholastmosher]

So I think I'm going to start playing with a few alternate strategies to see if I can at least get this to a working state. Some things I think I might try:

• Inverting the no_std flag to be a std flag.
  I originally did the flag as no_std because I wanted to be able to list core_io as an optional dependency that's only included when the no_std feature flag is enabled. For now, I think I'll deal without the optional-dependency mechanism, then we can perhaps revisit this later.

• Try adding a std feature flag to core_io.
  I think this is straying quite far into the territory of hacky workarounds, but based on the various discussions about build dependencies being bundled together with regular dependencies, it seems like there's no way to avoid the core_io compiled capnp being linked into the std-compiled capnpc. So, I'm thinking if I add a bunch of blanket impls behind a std feature flag in core_io, we might be able to make the two sets of traits compatible. E.g.

#[cfg(feature = "std)]
impl core_io::io::Read for T where T: std::io::Read { ... }

Which should allow us to pass &mut inp from generate_code as the argument for serialize::read_message.

• Converting capnpc to use core_io.
  I originally was specifically avoiding this since capnpc is a build-time tool which will pretty much always be running on a std environment, but if bubbling up the types causes all of the pieces to work, I might try it.

I'll post back with progress once I've tried some of these out.

[nicholastmosher]

Hm, it seems that since core_io is now included by default (as opposed to being a non-default optional dependency before), the main build would only succeed using nightly-2019-07-01 (which core_io requires). Previously, this wouldn't break the CI because the feature wasn't activated and so it wouldn't try to build core_io.

I think the ideal-world solution would be some sort of "feature negation", but I don't believe that exists right now. For example, previously I had this:

# Cargo.toml
[dependencies]
core_io = { version = "0.1.20190701", features = [ "alloc", "collections" ], optional = true }

[features]
no_std = ["core_io"]

This would only add core_io as a dependency when the no_std flag was activated. As far as I know, there's no way to do this in reverse, like this:

# Cargo.toml
[dependencies]
core_io = { version = "0.1.20190701", features = [ "alloc", "collections" ], optional = true }

[features]
std = ["!core_io"]

I think this is unfortunately an instance where the accommodations for no_std aren't mature enough to handle weird use cases like this. The need to even use core_io in the first place is an unfortunate consequence that the io traits were not originally designed to be compatible with no_std.

I think I'll push a revert commit and try something else.

[dwrensha]

The need to even use core_io in the first place is an unfortunate consequence that the io traits were not originally designed to be compatible with no_std

I wonder whether it would help if the capnp::serialize and capnp::serialize_packed modules were moved out into a separate crate. I think those modules are the only places whether the base crate needs std::io.

(I'm sorry that I still haven't gotten around to giving this pull request as in-depth of a review as it deserves...)

[bbqsrc]

@dwrensha I have just looked into this, and I have a branch for it on bbqsrc/capnproto-rust that takes an approach of providing a minimal subset of the Read and Write traits just to get things building.

I note that due to async support not functioning on no_std, I have replaced the one instance of its use in the codebase with a todo!() :)

I am now investigating moving the message serialisation functionality into its own crate. It seems possible that if this is achieved, we could have the message serialisation/deserialisation not require alloc as well, which is ideal for microcontrollers, particularly where we do not need the RPC or will be using a crippled subset.

[bbqsrc]

@dwrensha so a team at @technocreatives will begin working on splitting the serialization infra into a separate crate next week. We'll make a pull request with the outcome.

The goal is to make it possible to use the serialization on microcontrollers like NRF52. Hopefully we'll achieve that. 😄

[dwrensha]

we could have the message serialisation/deserialisation not require alloc as well

Hm. Eliminating the alloc dependency sounds difficult to me. That means we would need to stop using Vec is used in private/arena.rs, right?

splitting the serialization infra into a separate crate

I wonder how we could achieve this while minimizing the annoyance to people who don't care about no_std. Like, will everyone need to add both capnp and capnp_serialize to their Cargo.toml? Maybe some fancy feature flags and export facades could work around that, but would it be worth the complexity?

I have a branch for it on bbqsrc/capnproto-rust that takes an approach of providing a minimal subset of the Read and Write traits just to get things building

This approach seems promising to me, and much simpler than using core_io, which seems to involve some complicated machinery.

[dwrensha]

I'm looking at https://github.com/technocreatives/capnproto-rust/blob/c331b74f8def6ffc7bccdb2991a85adcf8179f95/capnp/src/serialize.rs#L31, which defines Read and Write traits in a local module :

#[cfg(not(feature = "std"))]
pub mod io { ... }

I wonder whether it would make more sense for this module to be unconditionally included, and to have the serialize and serialize_packed functions always operate on objects implementing these traits. Then the conditional compilation could be restricted to impl blocks. In the feature="std" case, we would have impl <T> Read for T where T: std::io::Read, and for the not(feature="std") case would just provide the impl for &[u8] and maybe a few other concrete types. This way, the feature flag would truly be "additive", so I think there would be less risk of weird breakage.

[bbqsrc]

Hm. Eliminating the alloc dependency sounds difficult to me. That means we would need to stop using Vec is used in private/arena.rs, right?

Not necessarily. There's a few pragmatic options:

1. Require providing a global allocator in no_std codebases (not very nice but feasible)
2. Using a crate like heapless
3. Using instead pre-allocated fixed size buffers in certain places where that size if somehow configurable

Obviously, there'll be some experimentation here, falling back to option 1 if everything else is wrong. 😄

I wonder how we could achieve this while minimizing the annoyance to people who don't care about no_std. Like, will everyone need to add both capnp and capnp_serialize to their Cargo.toml? Maybe some fancy feature flags and export facades could work around that, but would it be worth the complexity?

My current idea is that by default capnp would include capnp_serialize. The only case where you'd want to not use capnp directly is if you only wanted to serialise messages and not use the RPC aspects. The other pattern of the community is to use a std feature which is included in the default list, so that no_std users simply do no-default-features and std only stuff is disabled. Is that amenable to what you want?

I wonder whether it would make more sense for this module to be unconditionally included, and to have the serialize and serialize_packed functions always operate on objects implementing these traits.

Thanks for reviewing my experiment! I was thinking the same thing. I just did the bare minimum to get this to compile. We'll review this as a team and try to come up with a really good solution

[bbqsrc]

Hey! I have some unfortunate news.

The team assessed the amount of work that it would take for us to become familiar enough with the codebase to successfully separate out the components, and at this stage it would take us longer than a week, so we're no longer pursuing work in this space.

Global allocator is still the best option for now.

[CameronNemo]

Converting capnpc to use core_io.

This seems like the nicest solution. Was it ever tried?

[CameronNemo]

Another potential solution to the capnpc problem has come up.

Assuming this is the same issue as you are facing here: rust-lang/cargo#2589

A more capable dependency resolver was added to cargo to address the problem: rust-lang/cargo#7820

[dwrensha]

Thanks for the links, @CameronNemo.

I think for now it's probably acceptable for no_std to mean that you need to use a pre-installed version of capnpc-rust, so that its std feature flag does not interfere with your project's capnp feature flags.

I'm still skeptical of core_io. It looks very complicated to me.

I still like the plan I outlined in #138 (comment). That is, the capnp crate can define its own local versions of the Read and Write traits, and a std feature flag optionally enables blanket impls:

impl capnp::Read for T where T: std::io::Read { ... }
impl capnp::Write for T where T: std::io::Write { ... }

[dwrensha]

I've opened a new pull request for no-std support: #184. I'd be curious to hear anyone's thoughts on it. I'm aiming to include this change in a 0.13 release in about two weeks, once rustc 1.44 is out and we can use async and no-std together (though note that the initial no-std support will just be for the base capnp crate, not for capnp-futures or capnp-rpc).

[dwrensha]

#184 has landed, incorporating many of the ideas brought up by @nicholastmosher, @bbqsrc, and others. Thanks for the pull request and discussion! Please open new issues if there are any problems.

[dwrensha]

And note that with #184, capnpc does not require the "std" feature of capnp crate, so there should be no problem with generating code in a build.rs in a project being compiled for no_std.