Add rust_tonic_compile rule
#391
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
PR Review ChecklistDo not edit the content of this comment. The PR reviewer should simply update this comment by ticking each review item below, as they get completed. Code
Architecture
|
alexjpwalker
commented
Nov 21, 2022
| @@ -0,0 +1,33 @@ | |||
| # | |||
There was a problem hiding this comment.
This package contains the rust_tonic_compile rule and the rust_binary that powers it.
Ideally, we'd get this rule from an external repo such as rules_rust or rules_proto_grpc, but they don't have tonic implementations yet.
We may want to relocate this rule to another repo so it can be used by other Rust proto repos. We may also want to ask the rules_proto_grpc maintainers if they'd accept it as a PR. They do have an existing open PR that adds rust_tonic_compile (and more), so we'd start by commenting there.
alexjpwalker
commented
Nov 21, 2022
| @@ -0,0 +1,28 @@ | |||
| // | |||
There was a problem hiding this comment.
We pass in all the tonic_build configuration via environment variables, which are set by the rust_tonic_compile rule.
alexjpwalker
commented
Nov 21, 2022
| @@ -0,0 +1,62 @@ | |||
| # | |||
There was a problem hiding this comment.
The rule implementation is quite straightforward:
- Define the inputs for the compile script, namely all
.protosources and theprotocexecutable. - Define the outputs of the compile script. One output is generated for each unique
packagedefined in our.protofiles. Bazel can't (easily!) get these package names, so the user is required to supply them as a parameter. - Run the compile script and expose the outputs as source files that any
rust_librarycan depend on.
(The rust_library is then expected to depend on the tonic and prost crates, otherwise it will fail to compile. This is in line with Java, where we expect our java_library to depend on the correct gRPC + Protobuf JARs.)
alexjpwalker
commented
Nov 21, 2022
|
|
||
| load("@rules_rust//rust:repositories.bzl", "rules_rust_dependencies", "rust_register_toolchains") | ||
| rules_rust_dependencies() | ||
| rust_register_toolchains(edition = "2021", include_rustc_srcs = True) |
There was a problem hiding this comment.
Update to the latest Rust (fixes build issue in ortools, and also is just generally good practice)
alexjpwalker
commented
Nov 21, 2022
| "OUT_DIR": outputs[0].dirname, | ||
| "PROTOC": ctx.attr.protoc.files.to_list()[0].path, | ||
| "PROTOS": ";".join([src.path for src in protos]), | ||
| "PROTOS_ROOT": ctx.attr.srcs[0][ProtoInfo].proto_source_root, |
There was a problem hiding this comment.
OUT_DIR and PROTOC are both used internally by the tonic_build crate. So, for convenience, we use environment variables to represent the other necessary input arguments as well.
lolski
approved these changes
Nov 21, 2022
haikalpribadi
pushed a commit
to typedb/typedb-protocol
that referenced
this pull request
Nov 21, 2022
## What is the goal of this PR? We added crate distribution targets (`assemble_crate` and `deploy_crate`) for Rust, and added CI workflows for deploying snapshot and release crates. ## What are the changes implemented in this PR? We recently tried adding an `assemble_crate` rule to our existing Rust proto codebase to distribute Rust proto bindings, but it didn't work - it didn't contain any protobuf source code, because the `assemble_crate` rule doesn't support Cargo build scripts. But instead of actually adding that support, we had a rethink of the build architecture. The old idea was (following Tonic's own docs) for `typedb-protocol` to come bundled with the `.proto` sources and a Cargo build script that would allow the end-user to compile the protobufs on-the-fly. This would require them to have `protoc` installed in their environment, and would dramatically increase their compilation time and overhead. A better option would be to bundle the generated Rust sources. So we've created the `rust_tonic_compile` rule that enables just that in typedb/typedb-dependencies#391. Now, both `rust_library` and `assemble_crate` rules can depend on `typedb_protocol` seamlessly, so we can depend on `typedb_protocol` either through Bazel or Cargo. As this PR stabilises the release process, we've gone ahead and added the relevant CI workflows: `deploy-crate-snapshot` and `deploy-crate-release`.
jamesreprise
pushed a commit
to jamesreprise/vaticle-dependencies
that referenced
this pull request
Dec 5, 2022
## What is the goal of this PR? We added the `rust_tonic_compile` rule which compiles `.proto` files into Rust sources that depend on the `tonic` and `prost` crates. ## What are the changes implemented in this PR? We had an existing Rust binary for compiling proto files into a Tonic library - a Cargo build script in `typedb-protocol`. But Cargo users shouldn't have to run `protoc` on their machine to compile `typedb-client` (which is what happens with a Cargo build script). Rather, the `typedb-protocol` crate should contain the generated Rust gRPC + Protobuf sources. So we've created a rule, `rust_tonic_compile`, that internally runs a `rust_binary` (similarly to how many of our rules run Kotlin binaries), which uses `tonic_build` to compile `.proto` files into Rust sources, and exposes the outputs. Any `rust_library` can then depend on these generated sources. The API is similar to Java and Python (`java_grpc_compile` and `python_grpc_compile` respectively), and the `antlr` rule from `rules_antlr` behaves similarly, too. Ideally, we wouldn't need our own rule to do this, however, the Bazel rules repos that contain Rust gRPC + Protobuf rules only support the `grpc` crate, and not the more popular `tonic` crate. Moreover, all of them already have open PRs for adding such functionality: - rules-proto-grpc/rules_proto_grpc#143 - stackb/rules_proto#201 - bazelbuild/rules_rust#915 Given that our implementation is **very minimal** (~30 lines of code) it should incur a very low maintenance effort.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
What is the goal of this PR?
We added the
rust_tonic_compilerule which compiles.protofiles into Rust sources that depend on thetonicandprostcrates.What are the changes implemented in this PR?
We had an existing Rust binary for compiling proto files into a Tonic library - a Cargo build script in
typedb-protocol. But Cargo users shouldn't have to runprotocon their machine to compiletypedb-client(which is what happens with a Cargo build script). Rather, thetypedb-protocolcrate should contain the generated Rust gRPC + Protobuf sources.So we've created a rule,
rust_tonic_compile, that internally runs arust_binary(similarly to how many of our rules run Kotlin binaries), which usestonic_buildto compile.protofiles into Rust sources, and exposes the outputs. Anyrust_librarycan then depend on these generated sources. The API is similar to Java and Python (java_grpc_compileandpython_grpc_compilerespectively), and theantlrrule fromrules_antlrbehaves similarly, too.Ideally, we wouldn't need our own rule to do this, however, the Bazel rules repos that contain Rust gRPC + Protobuf rules only support the
grpccrate, and not the more populartoniccrate. Moreover, all of them already have open PRs for adding such functionality:Given that our implementation is very minimal (~30 lines of code) it should incur a very low maintenance effort.