Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Rust] Add better support for "crate-per-schema" #8273

Open
adsnaider opened this issue Mar 31, 2024 · 2 comments
Open

[Rust] Add better support for "crate-per-schema" #8273

adsnaider opened this issue Mar 31, 2024 · 2 comments

Comments

@adsnaider
Copy link
Contributor

Currently, flatc allows using --rust-module-root-file and --gen-all to generate multiple schemas into a single crate with a top-level mod.rs. This is good but makes it really hard to use in many contexts since the best (only?) option to have inter-dependent schemas is to generate everything together into a single crate.

Ideally, we can generate each schema independently of the includes (as each include will be its own generated crate), and link them all at build time.

@adsnaider
Copy link
Contributor Author

I don't particularly care about doing this through flatc directly. We use Bazel downstream and I have some patches to implement this, but we're somewhat behind master so it might take me a bit of time to send a PR. It is on my radar though.

The big thing that needs to change is the use per module need to be more specific, for instance:

// foo.fbs
namespace foo;
// ... some definitions
// foobar.fbs
include "foo.fbs"
namespace foo.bar;
// ... some definitions

The generated code should be like

// foo_generated.rs

use foo_generated::*;

pub mod foo {
  use foo_generated::foo::*; // As opposed to currently `foo_generated::*;

  pub mod bar {
    // No need to include foo_generated here since it doesn't live in `foo.bar`
  }
}

I think the correct approach is to include [some_generated_dep]::[some_module_tree]::* if and only if [some_module_tree] is a subset of [some_generated_dep]'s namespace. I'm hoping someone will tell me if that doesn't work on every case :)

@adsnaider
Copy link
Contributor Author

Unfortunately, this doesn't work when the imports are ambiguous. For instance:

foo.fbs -> namespace a1.b1.c1 - includes bar and baz
bar.fbs -> namespace a1.b2.c1
baz.fbs -> namespace a1.b2.c1

This will result in bar::a1::* and baz::a1::* being imported into the same a1 module which result in an ambiguous include of b2.

We will probably have to change the code generation to use absolute paths instead for our use case. Would love to upstream our work once it's ready. Would this be a reasonable contribution or would this change to absolute paths break other use cases?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant