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

build: Allow specifying RUST_PROTOBUF_SRC_PROTOC env variable #13

Closed

Conversation

ParkMyCar
Copy link
Member

This PR allows users to specify a path to an existing instance of protoc, which will skip building the vendored version. This is useful for build processes where we don't want to use the build script (e.g. Bazel) or for processes that want to provide their own version of protoc.

@benesch
Copy link
Member

benesch commented Apr 3, 2024

I don’t think this is the right approach! The point of the protobuf-src crate is to build the vendored copy. If someone wants to opt out I think that should happen at a higher level. Ie, don’t depend on protobuf-src.

@ParkMyCar
Copy link
Member Author

I don’t think this is the right approach! The point of the protobuf-src crate is to build the vendored copy. If someone wants to opt out I think that should happen at a higher level. Ie, don’t depend on protobuf-src.

I get that! But I think this change is the only way to handle the scenario where for local dev you want to build a vendored version of protoc and maybe for CI or some other process you rely on an already built version. An alternative option would be in all of the crates that depend on protobuf-src you could make it an optional dependency and in your build script have something like:

#[cfg(feature = "protobuf-src")]
{
    std::env::set_var("PROTOC", protobuf_src::protoc());
}

but even this approach doesn't totally work since a transitive dependency could pull in protobuf-src and require it get built.

@ParkMyCar
Copy link
Member Author

ParkMyCar commented Apr 3, 2024

Hmmmm wait, another thought I just had is I could make a wrapping crate, e.g. mz-protoc which includes all of this logic in its own build script. Still doesn't get around the transitive dependency issue though, but maybe that doesn't matter in practice 🤔

Edit: Never mind this wouldn't work, because even when building mz-protoc you would still need to build protobuf-src since it's a dependency. I don't think there's anyway to skip builing a dependency based on an env var

@benesch
Copy link
Member

benesch commented Apr 4, 2024

Just noting for posterity that we chatted about this on Slack and I believe we found another viable path!

@ParkMyCar
Copy link
Member Author

Closing out because we found a different path!

@ParkMyCar ParkMyCar closed this Apr 4, 2024
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

Successfully merging this pull request may close these issues.

2 participants