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

Proto-build fails for SDK 0.46 #318

Closed
larry0x opened this issue Jan 13, 2023 · 18 comments
Closed

Proto-build fails for SDK 0.46 #318

larry0x opened this issue Jan 13, 2023 · 18 comments

Comments

@larry0x
Copy link

larry0x commented Jan 13, 2023

The cosmos-sdk repo since v0.46.0 no longer includes a third_party/proto folder. Instead, it uses a buf.yml config file to define dependencies. However, proto-build does not support this; executing cargo run now results in the following error:

thread 'main' panicked at 'called `Result::unwrap()` on an `Err` value: Custom { kind: Other, error: "protoc failed: gogoproto/gogo.proto: File not found.\ngoogle/protobuf/any.proto:35:1: Import \"gogoproto/gogo.proto\" was not found or had errors.\ngoogle/api/annotations.proto: File not found.\ncosmos_proto/cosmos.proto: File not found.\ncosmos/auth/v1beta1/auth.proto:4:1: Import \"cosmos_proto/cosmos.proto\" was not found or had errors.\ncosmos/auth/v1beta1/auth.proto:5:1: Import \"gogoproto/gogo.proto\" was not found or had errors.\ncosmos/auth/v1beta1/auth.proto:6:1: Import \"google/protobuf/any.proto\" was not found or had errors.\ncosmos/auth/v1beta1/auth.proto:21:3: \"google.protobuf.Any\" is not defined.\ncosmos/auth/v1beta1/query.proto:5:1: Import \"gogoproto/gogo.proto\" was not found or had errors.\ncosmos/auth/v1beta1/query.proto:6:1: Import \"google/protobuf/any.proto\" was not found or had errors.\ncosmos/auth/v1beta1/query.proto:7:1: Import \"google/api/annotations.proto\" was not found or had errors.\ncosmos/auth/v1beta1/query.proto:8:1: Import \"cosmos/auth/v1beta1/auth.proto\" was not found or had errors.\ncosmos/auth/v1beta1/query.proto:9:1: Import \"cosmos_proto/cosmos.proto\" was not found or had errors.\ncosmos/auth/v1beta1/query.proto:86:12: \"google.protobuf.Any\" is not defined.\ncosmos/auth/v1beta1/query.proto:104:3: \"google.protobuf.Any\" is not defined.\ncosmos/auth/v1beta1/query.proto:113:3: \"Params\" is not defined.\ncosmos/auth/v1beta1/query.proto:125:12: \"google.protobuf.Any\" is not defined.\ncosmos/auth/v1beta1/query.proto:135:3: \"google.protobuf.Any\" is not defined.\n" }', proto-build/src/main.rs:254:10

Apparently, it cannot locate the gogoproto dependency.

I do not know a good solution to this. Any suggestion?

@larry0x larry0x changed the title Proto-build fails since sdk 0.46, Proto-build fails for SDK 0.46 Jan 13, 2023
@tony-iqlusion
Copy link
Member

proto-build needs to be updated accordingly

@xoac
Copy link
Contributor

xoac commented Feb 22, 2023

Is there any estimation when support for version 0.46 can be expected?

@apollo-sturdy
Copy link
Contributor

I had the same issue when trying to generate types for wasmd >= 0.29.0. It seems that both cosmos-sdk and wasmd have a proto-gen script in their Makefile that runs a bash script in a docker container. @tony-iqlusion Would it be reasonable to have proto-build run this script and would it be acceptable for users to need to have docker installed? If you think this is a good approach I can try to make a PR with these changes.

@apollo-sturdy
Copy link
Contributor

apollo-sturdy commented Feb 24, 2023

So I got it to work for wasmd >= 0.29.0 by generating the files via buf with the help of this repo: https://github.com/neoeinstein/protoc-gen-prost

My changes can be found here: https://github.com/apollodao/cosmos-rust/tree/dev/generate-via-buf
I will open a PR for this.

Unfourtunately it seems there are manual changes needed to cosmrs to support cosmos-sdk 0.46. Since I don't need it and am not familiar with the codebase I will leave these changes to someone else. I did however test generating the protos for cosmos-sdk 0.46.10 via buf and it works. I also included a buf yaml config file in my fork for those who want to make the changes needed to cosmrs.

@tony-iqlusion
Copy link
Member

I just got back from vacation. Will take a look at this (and cutting another release) next week

@tony-iqlusion
Copy link
Member

Looking at @apollo-sturdy's branch it seems like protoc-gen-prost is probably the best way forward to me if upstream Cosmos SDK has switched to using buf.

I can try to put together a PR for that unless someone else is particularly interested.

I can also merge #289 to unblock people until we have fully working automation again, at least if the test failures were fixed.

@tony-iqlusion
Copy link
Member

I think what might make sense is to cut another release to get tendermint-rs upgraded, then circle back on this issue for the next release.

That will let us focus on coordinating on ADR 012: Multi-Version Support as part of the upgrade.

@mzabaluev
Copy link

mzabaluev commented Mar 3, 2023

If that's of any help, I had a PR on Hermes/ibc-rs where I converted the proto builder used in that project to pull the dependencies as listed in buf.lock using one of the auxiliary buf commands.

Edit: initially pointed to a different PR informalsystems/hermes#2199, where more extensive use of buf was attempted.

@tony-iqlusion
Copy link
Member

Those are both fairly large PRs. @mzabaluev can you point to some specific changes you think would be useful?

FWIW #332 contains some initial steps but still needs a rebase cc @apollo-sturdy

@mzabaluev
Copy link

Those are both fairly large PRs. @mzabaluev can you point to some specific changes you think would be useful?

Look at the changes in ibc-proto-compiler.

@tony-iqlusion
Copy link
Member

tony-iqlusion commented Mar 16, 2023

@mzabaluev it looks like you didn't use protoc-gen-prost, but instead just shell out to buf directly. Is that right?

@mzabaluev
Copy link

@mzabaluev it looks like you didn't use protoc-gen-prost, but instead just shell out to buf directly. Is that right?

Yes, I used one of the auxiliary buf commands to fetch the dependencies at the commits listed in buf.lock.
The proper thing do to would be to switch to buf also for code generation using the prost plugin.

@tony-iqlusion
Copy link
Member

I opened #392 which switches to using buf + protoc-gen-prost + protoc-gen-tonic.

It seems to work but I could definitely use help validating the changes and ensuring there's not anything missing.

@tony-iqlusion
Copy link
Member

Oops, I realized #392 didn't actually update to SDK v0.46.

Upon attempting to do that, I get the following error from buf:

[info] Compiling proto definitions and clients for GRPC services!
../cosmos-sdk-go/third_party/proto/google/protobuf/any.proto:35:8:gogoproto/gogo.proto: does not exist

I see the following in cosmos-sdk's proto/buf.yaml:

deps:
  - buf.build/cosmos/cosmos-proto
  - buf.build/cosmos/gogo-proto
  - buf.build/googleapis/googleapis

...so I'm not sure why it isn't being pulled in?

@tony-iqlusion
Copy link
Member

I split migrating to buf out into its own PR which doesn't attempt to upgrade the SDK version: #393

That should make it easier for others to experiment with the upgrade, namely figuring out the buf.gen.yaml and/or buf CLI arguments needed to get the prerequisite packages to work.

I'm curious if, ala #289, we might just need to vendor the relevant gogoproto protos.

@tony-iqlusion
Copy link
Member

Okay, think I finally got it: #395

I was passing cosmos-sdk-go instead of cosmos-sdk-go/proto as the input path for buf generate. When using the latter, everything works as expected.

@tony-iqlusion
Copy link
Member

tony-iqlusion commented Apr 20, 2023

Closing this as completed by #395.

If anyone would like to do some advance testing, I'm cutting cosmos-sdk-proto v0.19.0-pre.0 which includes Cosmos SDK v0.46-compatible protos: #396

@tony-iqlusion
Copy link
Member

With tendermint-rs v0.32 about to ship, this is the last call on any feedback on the newly generated protos before the next release.

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

No branches or pull requests

5 participants