Skip to content

fix(build): Prevent duplicated client/server generated code#121

Merged
LucioFranco merged 4 commits intohyperium:masterfrom
steele:bugfix/build-service-import
Nov 9, 2019
Merged

fix(build): Prevent duplicated client/server generated code#121
LucioFranco merged 4 commits intohyperium:masterfrom
steele:bugfix/build-service-import

Conversation

@steele
Copy link
Copy Markdown
Contributor

@steele steele commented Nov 7, 2019

Motivation

The tonic-build process collects up RPC services as they are provided by prost, before writing them out as part of the finalization step for a given protocol buffer package.

In the case of imported protocol buffer packages, there may be RPC services included by import in addition to those in the top-level package. Therefore it is necessary to make sure each set of client/server services gathered by tonic-build is cleared after the finalization process for a given protocol buffer package, otherwise they will be incorrectly aggregated as the generation process proceeds through the subsequent packages.

This incorrect behaviour is currently observed in tonic-build.

Solution

Reset the TokenStreams that capture the services for client and server use-cases after the finalization process has written out the generated code.

Copy link
Copy Markdown
Member

@LucioFranco LucioFranco left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for this! I think you may need to rebase against master since we are updating our toolchain to stable.

The tonic-build process collects up RPC services as they are provided by
prost, before writing them out as part of the finalization step for a
given protocol buffer package.

In the case of imported protocol buffer packages, there may be RPC
services included by import in addition to those in the top-level
package. Therefore it is necessary to make sure each set of
client/server services gathered by tonic-build is cleared after the
finalization process for a given protocol buffer package, otherwise they
will be incorrectly aggregated as the generation process proceeds
through the subsequent packages.
A simple test case that will fail to build without a fix to prevent
RPC services being duplicated into inappropriate modules (that related
to particular protocol buffer packages).
Introduces an additional case that captures making sure services defined
before including a package with additional services doesn't
incidentially clear such precursor services from the including package.
@steele steele force-pushed the bugfix/build-service-import branch from 9b2d659 to c748fb2 Compare November 9, 2019 11:27
@steele
Copy link
Copy Markdown
Contributor Author

steele commented Nov 9, 2019

Thanks for this! I think you may need to rebase against master since we are updating our toolchain to stable.

Should be rebased to master that uses the stable toolchain now

@LucioFranco LucioFranco merged commit b02b4b2 into hyperium:master Nov 9, 2019
@steele
Copy link
Copy Markdown
Contributor Author

steele commented Nov 9, 2019

👍 thanks @LucioFranco!

@steele steele deleted the bugfix/build-service-import branch November 9, 2019 19:17
@rlabrecque
Copy link
Copy Markdown

Nice work!

rabbitinspace pushed a commit to satelit-project/tonic that referenced this pull request Jan 1, 2020
…#121)

* fix(build): Prevent duplicated client/server generated code

The tonic-build process collects up RPC services as they are provided by
prost, before writing them out as part of the finalization step for a
given protocol buffer package.

In the case of imported protocol buffer packages, there may be RPC
services included by import in addition to those in the top-level
package. Therefore it is necessary to make sure each set of
client/server services gathered by tonic-build is cleared after the
finalization process for a given protocol buffer package, otherwise they
will be incorrectly aggregated as the generation process proceeds
through the subsequent packages.

* Test case for duplicated client/server generated code

A simple test case that will fail to build without a fix to prevent
RPC services being duplicated into inappropriate modules (that related
to particular protocol buffer packages).

* Additional test case for included_service

Introduces an additional case that captures making sure services defined
before including a package with additional services doesn't
incidentially clear such precursor services from the including package.

* Fix unnecessary newline to keep `cargo fmt` happy
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.

3 participants