Allow PROTOC env to be overridden at runtime

[gabrielrussoc]

The docs state

If set, prost-build uses the PROTOC and PROTOC_INCLUDE environment variables for locating protoc and the Protobuf includes directory.

This is currently only done at build time, essentially hardwiring a path to the compiled binary and requiring a rebuild in order to change the PROTOC location. This PR adds the ability to use the runtime environment for fetching both PROTOC and PROTOC_INCLUDE, making it more flexible.

Why

On our particular case, we have a binary that uses prost-build but is not compiled with Cargo. This will make it possible for us to cache this binary and just point to the correct protoc location on different environments.

[dfreese]

What sort of system are you using? We're using Bazel and I hadn't noticed this PR when I submitted #309.

I add a feature and cfg! some things out in that PR, but then add some options to the builder. I had considered going a similar direction as this one. I discuss it a bit there, but my first preference would be to change the env! macros to optional_env! macros, and then let them be overridden by the builder.

Not having to run a build.rs file works a lot better with cargo raze in order to make things mesh with Bazel.

[gabrielrussoc]

@dfreese we use Buck, which is somewhat similar to Bazel.

[danburkert]

This PR looks good to me, I can't think of any downsides to letting this be set at runtime, and it seems like this may be what most people expect.

my first preference would be to change the env! macros to optional_env! macros, and then let them be overridden by the builder.

@dfreese can you elaborate on that? I'm not sure what optional_env! is referencing, or 'builder'. I'm going to go ahead and merge this, since I assume your +1 on the PR means it's a net positive for your bazel usecase.

[danburkert]

I don't know how to GitHub, and I want to push some changes to fix the tests, so I'm going to re-open as a new PR momentarily.

[danburkert]

https://github.com/danburkert/prost/pull/324

[dfreese]

tldr; I'm trying to deal with two concerns:

1. Using env! macros requires setting a bogus value in environments that don't run build.rs
2. There's currently no public logic to be able to write your own transform from FileDescriptorSet to String of generated code

I had +1 it since it was in a similar vein to what I was looking at in #309. There are some differences though. (and builder == prost_build::Config struct, since it looks like a builder pattern)

It's difficult to get build scripts to integrate well with a bazel environment, so I was mainly interested in exposing enough functionality from prost-build to be able to write a binary that can turn a FileDescriptorSet into generated code. Mainly just the generate(...) function

Since I don't use build scripts, using env! macros means I need to special case PROTOC at build time to some bogus value. With #309 the logic for this PR could be handled by:

Config::new()
    .protoc(std::env::var("PROTOC")?)
    .compile_protos(...);

without still requiring PROTOC to be set to some value at build time to be able to compile. The optional_env! macro would be another way of getting around setting a bogus value.

I had put up a plugin in #313 as a way of exposing the generate functionality. That would be one way of exposing this logic, though not currently my preferred.

It would be great to move most of the underlying logic of prost-build back into a prost-codegen crate. That would let it expose the generate function logic, and then prost-build could be focused on being used in build scripts. I'd be happy to do this, if it's something you'd be okay with. Other ways of going about this might be exposing the generate function as public, which would be a little awkward, but would still work.

[danburkert]

I had put up a plugin in #313 as a way of exposing the generate functionality. That would be one way of exposing this logic, though not currently my preferred.

Can you expand on why a protoc plugin isn't preferred? My main concern with a prost-codegen crate is just additional work to maintain, and an additional public API. I suppose it could be as simple as a single public fn from CodeGeneratorRequest to CodeGeneratorResponse, though.

[danburkert]

It's tricky, though. That API for instance wouldn't play well with how prost handles ignoring files, e.g. Config::extern_path.

[dfreese]

So in #313 I ended up adding an API that does that CodeGeneratorRequest -> Response conversion in run_plugin(...).

That still allows for use of extern_path.

Config::new()
    .external_path(...)
    .run_plugin(request);

It isn't as preferable, as it loses a little bit of the detail about the rust module that is being created (i.e. the Vec used to represent ["google", "protobuf"] gets encoded into "google.protobuf" in the file name), and the error handling becomes a little odd, but that being said, it's still workable.