andrewhickman / protox

A pure-rust protobuf compiler, designed for use with prost-build
Apache License 2.0
68 stars 5 forks source link

Abstraction for FileSystem #40

Closed ssddOnTop closed 7 months ago

ssddOnTop commented 8 months ago

There can be cases where people have different IO then std::fs. For example in my case, I am using cloudflare workers so I have an abstraction which uses cloudflare r2 as virtual fs. So adding an abstraction, it would be much easier to add tests this way.

But I am a bit confused about IO of protox/protobuf dir (which you defined in a macro).

andrewhickman commented 8 months ago

Hi, this should already be possible through the FileResolver trait.

You can define your own CloudflareR2FileResolver and pass it to Compiler::with_file_resolver, and all file I/O will go through your implementation.

GoogleFileResolver does not read from the filesystem - the well known protobuf files are packaged with the library using include_str!. If using your own file resolver, you can chain it with GoogleFileResolver in the same way as Compiler::new().

ssddOnTop commented 8 months ago

Yeah, I didn't think that way earlier.. thank you for responding, and I need 1 more help

I have a struct called Foo which is similar to IncludeFileResolver and instantiated as

    let mut resolver = ChainFileResolver::new();
    resolver.add(Foo::add(PathBuf::from(parent_dir)));
    resolver.add(GoogleFileResolver::new());

    let mut compiler = protox::Compiler::with_file_resolver(resolver);
    let compiler = compiler.include_imports(true).open_file(proto_path)?;

But for it tries to open the google proto files from Foo instead of the GoogleFileResolver

andrewhickman commented 8 months ago

ChainFileResolver will attempt each resolver in the order they are added, and it only tries the next one if it gets a "not found" error. You can either return Error::file_not_found() from your Foo implementation, so that it moves on to GoogleFileResolver, or swap the order in which they are added to the ChainFileResolver so that it tries the google one first.

ssddOnTop commented 8 months ago

thanks @andrewhickman fixed it.

ssddOnTop commented 8 months ago

One last thing, my fileio is async, and the open_file is not async function, how about moving the open_file function to async function?

Or people will have to read files separately and store it somewhere like in hashmap and use that instead

andrewhickman commented 8 months ago

I think pre-loading the files into a hashmap sounds like a good idea.

Async support is tricky since this crate is primarily aimed at build scripts, and it would be super inconvenient to need an async runtime to use it. Sadly rust doesn't give us great ways to write code that works for both sync and async.