feat(tonic-build): add ability to configure tonic crate path#2676
Open
lukasweber wants to merge 1 commit into
Open
feat(tonic-build): add ability to configure tonic crate path#2676lukasweber wants to merge 1 commit into
lukasweber wants to merge 1 commit into
Conversation
|
|
4d95c95 to
10d422d
Compare
8607471 to
7c12e24
Compare
This file contains hidden or 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
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.
Hi guys,
thanks for the great work :)
Motivation
Following the issue #699, I would like to add the capability to configure the path to the
toniccrate in generated code intonic-build. This allows other library crates building on top oftonicto re-exporttonic, so users of the library (usingtonic) don't have to to include a dependency totonic.My use case:
I'm currently building a library for the SiLA protocol on top of tonic here and it would be really nice to ditch the tonic dependency from crates using that library.
Solution
Inspired by how prost is doing it with prost_path, I added a new method
tonic_paththat allows to set the path totonicin the modulecode_gen(cratetonic_build). The same I did for the publicmanualmodule where the path is then passed further to theCodeGenBuilder. If the builder-method is not called::tonicis used by default. I also added the method totonic-prost-build.I then made sure this path is passed down to client and server code generation, so there is no "hardcoded" tonic-path anymore.
Additional Notes
I set the default path to
::tonicsince I sawprostis doing it the same way. Although probably not a too big deal, I feel like it makes more sense to make it absolute for generated code to have the path crystal clear. That's also why I had to change many of the code ingenerated. Let me know if you see a problem in that. I can easily revert it. It pollutes the changeset quite a bit.In the test, I also re-exported
tonic_prostand configured the path usingcodec_path(...)which appears to be enough to also ditch thetonic_prostdependency frommy_application. Let me know if you feel like I should only focus on re-exportingtonic.For some methods, I had to add an additional
#[allow(clippy::too_many_arguments)]where I thought it's probably ok given that it is also used in many similar cases in that module already.In
tonic-prost-build, I removed some unnecessary clones for parameters being passed that were right next to thetonic_pathparameter.