Simplify proto gen #52
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
|
Wow awesome I'll take a look! |
michaelx11
approved these changes
Apr 16, 2023
There was a problem hiding this comment.
This is an amazing improvement. I was hunting for the reference to our protos, but it's in build.rs pointing at https://buf.build/xmtp/proto right?
Having a separate crate for protos is probably a better idea too!
michaelx11
reviewed
Apr 16, 2023
| @@ -0,0 +1,25 @@ | |||
| // @generated | |||
| pub mod xmtp { | |||
There was a problem hiding this comment.
I feel like a moron for having to handwrite this file myself in the original branch. I felt like a genius at the time for figuring out how to deal with our package names though 😅
There was a problem hiding this comment.
Oh man, I never would have figured that out on my own.
|
Feel free to merge whenever! |
michaelx11
pushed a commit
that referenced
this pull request
Apr 17, 2023
* feat: get gRPC compiling into networking crate * fix: attempt to fix weird package name nesting situation * fix: actually got it right * feat: add silly grpc client test * feat: added crappy grpc test * wip: need to run local node to test * wip: checkpoint, getting connection error detected: frame with invalid size, trying to add tls now * feat: rountrip publish/query in Rust * docs: fix up comments * feat: do a full grpc roundtrip as a Swift test * docs: remove warnings * fix: return uint16 status code for grpc test too * fix: chunk up auth token to avoid false positives in secret detection * feat: everything builds, Generated code is populated, now need to hook up to xmtp-ios * feat: Swift-Bridge, manually copy headers into include * fix: strip out corecrypto for now, framework down to 230MB * feat: add library size optimizations * feat: refactor into publish and query with serialized versions * feat: cleaned up and implemented query and publish * style: ran clippy * feat: full e2e build working, if jank * test: added a failing base64 bytes encoding test * fix: do a serde encoding override to base64 encode bytes fields in MessageApi * feat: implement subscribe_once api * Simplify proto gen (#52) * Initial commit * Include lib * Add features * Correct import * Add feature * Add pbjson * Almost passing * Add build.rs * Add .gitignore * Rename folder * Remove generated code * Gitignore generated * Automatically install required dependency * fix: Makefile change target dir to ./target workaround Cargo workspace * Update gitignore * Delete .gitmodules * Delete Cargo.lock * fix: remove bindings/xmtp_rust_swift from cargo workspace * fix: remove xmtp_rust_swift from workspace to use size-optimized profiles --------- Co-authored-by: Nicholas Molnar <65710+neekolas@users.noreply.github.com> Co-authored-by: Nicholas Molnar <nicholas@xmtp.com>
neekolas
added a commit
that referenced
this pull request
Apr 19, 2023
* feat: get gRPC compiling into networking crate * fix: attempt to fix weird package name nesting situation * fix: actually got it right * feat: add silly grpc client test * feat: added crappy grpc test * wip: need to run local node to test * wip: checkpoint, getting connection error detected: frame with invalid size, trying to add tls now * feat: rountrip publish/query in Rust * docs: fix up comments * feat: do a full grpc roundtrip as a Swift test * docs: remove warnings * fix: return uint16 status code for grpc test too * fix: chunk up auth token to avoid false positives in secret detection * feat: everything builds, Generated code is populated, now need to hook up to xmtp-ios * feat: Swift-Bridge, manually copy headers into include * fix: strip out corecrypto for now, framework down to 230MB * feat: add library size optimizations * feat: refactor into publish and query with serialized versions * feat: cleaned up and implemented query and publish * style: ran clippy * feat: full e2e build working, if jank * test: added a failing base64 bytes encoding test * fix: do a serde encoding override to base64 encode bytes fields in MessageApi * feat: implement subscribe_once api * Simplify proto gen (#52) * Initial commit * Include lib * Add features * Correct import * Add feature * Add pbjson * Almost passing * Add build.rs * Add .gitignore * Rename folder * Remove generated code * Gitignore generated * Automatically install required dependency * fix: Makefile change target dir to ./target workaround Cargo workspace * Update gitignore * Delete .gitmodules * Delete Cargo.lock * fix: remove bindings/xmtp_rust_swift from cargo workspace * fix: remove xmtp_rust_swift from workspace to use size-optimized profiles * Rename repo * Streaming support * Rename back to subscription * Make tests pass * Update bindings * Fix merge conflicts * Include libxmtp-core and wasm bindings * Make poll_subscription non-async * Re-pin --------- Co-authored-by: Michael Xu <michaeljxu11@gmail.com> Co-authored-by: Michael Xu <michaelx@xmtp.com>
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.
Summary
Trying a different approach to generating protobuf code.
Advantages are:
build.rsconfig with no special overrides needed for serialization.Notes
Some things to watch out for:
.serde.rsfile for all the protos. One side effect of this is that zero'd proto fields are no longer present in JSON output. I had to update one of your tests to accomodate the change.bytesfields are serialized as base64 strings, which this generated Serde setup seems to do by default. But there may be other less desirable changes that I haven't noticed yet.xmtp-networking, but my knowledge of Rust modules is just not there to debug the issue. If you can make it work, that's probably better.