Add CompilerCommand::crate_provides

[dzfranklin]

Relates to #411 and #233

annotation imports @0xc3b9fe42a83105cd (file) :List(Import);
# Allows specifying that the generated code for an import is located in another
# crate.
#
# You only need this if your code uses imported types whose generated code is
# in another crate. You can only use this annotation once in all the files
# compiled together.
#
#    using Json = import "/capnp/compat/json.capnp";
#
#    $Rust.imports([
#        (path = "/capnp/compat/json.capnp", crate = "capnp_json")
#    ]);

struct Import {
    path @0 :Text;
    crate @1 :Text;
}

I'm intending this as a stopgap until cargo supports non-linking DEP environment variables. It allows people to use generated code in external crates without us needing to add an annotation to the external .capnp file. This is important because without the change to cargo it would be awkward to require users use a modified json.capnp.

Unfortunately capnproto doesn't support annotations on import statements.

[dzfranklin]

The new api is like so

fn main() {
    capnpc::CompilerCommand::new()
        .link_override(0xe6f94f52f7be8fe2, "external_crate")
        .file("test.capnp")
        .run()
        .unwrap();
}

I chose to call it link_override instead of link because I think that'll be clearer when we also have #233 .

After sleeping on it I think "link" is a better word here than "import". "use_override" or "extern_override" might also make sense to mirror the rust syntax.

[dwrensha]

Hm... using the word "link" like this seems potentially confusing to me. This doesn't have anything to do with ld.
How about file_from_crate() instead of link_override()?

[dzfranklin]

Agree "link" is wrong. What about .crate_provides("capnp_json", [0x8ef99297a43a5e34]).

[dwrensha]

Works for me.

[dwrensha]

What should happen if a file in ctx.request.get_requested_files() has this ID too? Then the user is generating code for that file, and it seems to me that the crate-local version should win.

I think that, given an import ID X, the precedence should be:

1. If the user explicitly requests codegen a file with ID X, then that locally-generated version should be used, regardless of the contents of crates_provide_map.
2. Otherwise, if X appears in crates_provide_map, then that version of the name should be used.
3. Otherwise, if the referred-to file has a $Rust.crate annotation, then the Text crate name in that annotation should be used.
4. Otherwise, compile-time error (or graceful fallback).

I see that you removed the $Rust.crate annotation from this pull request. I do think we should add that annotation. Up to you whether you want it to be in this pull request or not.

[dzfranklin]

I'm tempted to say that if a user explicitly says a file should be provided by a crate and also explicitly asks to compile that file themselves we should fail with an error.

My guess is the main reason this would happen is that somebody was confused and thought they had to compile their imports. I can't think of a good reason you'd ever do this.

I agree $Rust.crate should be overridden by the build script option. I removed it because I was thinking it'd only be useful once we had the cargo feature but on second thought I think it's useful right away. I'll add it back.

[dwrensha]

I'm tempted to say that if a user explicitly says a file should be provided by a crate and also explicitly asks to compile that file themselves we should fail with an error.

Yeah that makes sense. I was forgetting that they need to explicitly opt into each individual file in crate_provides().

[dzfranklin]

In terms of fallbacks I don't think it's that important. c++.capnp doesn't have any annotations with non built-in types, so I think it's unlikely users will have such annotations in their code and not want to parse the annotations in Rust.

If it's a field instead of an annotation I can't think of a use case where a fallback would be useful.

Another possible API would be to fail with a helpful error message that suggests adding an explicit exclude_annotations(file_id) call.

[dwrensha]

I was imagining a schema created for C++ project, where it has a bunch of $Json annotations sprinkled in. A Rust project might not care about JSON and might not want to add a dependency on the capnp_json crate. So I think some way to ignore the annotations is needed. However, since there's probably more discussion to be had about it, and since nobody is actually complaining about it yet, I think it does not belong in the current PR.

[dwrensha]

$Rust.crate can be added in a latter pull reqeust.