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

Unnecessary protobuf requirement in Libra #5251

Closed
garious opened this issue Jul 23, 2019 · 7 comments

Comments

@garious
Copy link
Member

commented Jul 23, 2019

Problem

The libra codebase uses Google protobuf to send data to its metrics server. We stuffed Golang and protobuf into our Docker containers to move forward, but neither is actually used. Furthermore, those dependencies aren't on our benchmarking servers and not on developer machines. Until we get those dependencies out, we can't integrate Move into our top-level build.

Proposed Solution

Add a metrics feature to libra that is on by default. Then patch libra to only use protobuf when that feature is present. Then when we add dependencies to libra packages, we should use no-default-features and add metrics.

Also, try to upstream that patch. If lucky, they'll want the feature off by default so that dev_setup.sh becomes optional for the dev community.

@garious garious added this to the Waimea v0.17.0 milestone Jul 23, 2019

@TristanDebrunner TristanDebrunner self-assigned this Jul 23, 2019

@TristanDebrunner

This comment has been minimized.

Copy link
Contributor

commented Jul 23, 2019

Libra appears to use protobuf for any (de)serialization, so the metrics feature won't entirely remove the dependency. I think we also need a feature (such as proto-serde?) that enables that.

@garious

This comment has been minimized.

Copy link
Member Author

commented Jul 24, 2019

Sounds good. metrics name just a placeholder for whatever makes the most sense.

@garious

This comment has been minimized.

Copy link
Member Author

commented Jul 25, 2019

Also consider Rust protobuf implementations:

https://www.google.com/search?q=rust+protocol+buffers

whatever makes our protobuf woes go away fastest

@TristanDebrunner

This comment has been minimized.

Copy link
Contributor

commented Jul 25, 2019

So the problem is protoc, not protobuf itself?

@garious

This comment has been minimized.

Copy link
Member Author

commented Jul 25, 2019

You tell us. The problem is we don't want Solana CI depending on installing Go and protobuf. How to make those dependencies go away, preferably today, is up to you.

@garious

This comment has been minimized.

Copy link
Member Author

commented Jul 25, 2019

Two options were experimenting with here:

  • Adding the generated Rust files to the libra git repo: solana-labs/libra#2
  • Publishing crates that include the generated Rust
@garious

This comment has been minimized.

Copy link
Member Author

commented Jul 27, 2019

We added the generated code to git

@garious garious closed this Jul 27, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants
You can’t perform that action at this time.