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

go mod tidy fails #749

Closed
tbruyelle opened this issue Apr 18, 2023 · 5 comments · Fixed by #755
Closed

go mod tidy fails #749

tbruyelle opened this issue Apr 18, 2023 · 5 comments · Fixed by #755

Comments

@tbruyelle
Copy link
Contributor

Not sure since when, but it is currently failing on master.

$ go mod tidy
go: finding module for package github.com/gnolang/gno/tm2/pkg/amino/tests/proto3/proto
github.com/gnolang/gno/tm2/pkg/amino/tests/proto3 tested by
	github.com/gnolang/gno/tm2/pkg/amino/tests/proto3.test imports
	github.com/gnolang/gno/tm2/pkg/amino/tests/proto3/proto: no matching versions for query "latest"

$ echo $?
1
@thehowl
Copy link
Member

thehowl commented Apr 18, 2023

File was deleted in The Great Refactor (#585), probably because from the .gitignore, these should actually be ignored but they were already in the repository before the changes were added to gitignore).

This is fixed by adding the missing .pb.go file in the directory. There are a few other files which were deleted (pkgs/amino/tests/pb/tests.pb.go, pkgs/amino/tests/pbbindings.go, other than the one at hand, pkgs/amino/tests/proto3/proto/compat.pb.go). I know too little about amino to know if these were actually necessary for tests etc and what to do in this regard.

To generate the file
cd tm2/pkg/amino/tests/proto3/proto
protoc --go_out=. --go_opt=paths=source_relative \
  --go_opt=Mproto/compat.proto=github.com/gnolang/gno/tm2/pkg/amino/tests/proto3 \
  --go-grpc_opt=Mproto/compat.proto=github.com/gnolang/gno/tm2/pkg/amino/tests/proto3 \
  --go-grpc_out=. --go-grpc_opt=paths=source_relative proto/compat.proto

@tbruyelle
Copy link
Contributor Author

Awesome ty @thehowl it worked after I changed the initial path from tm2/pkg/amino/tests/proto3/proto to tm2/pkg/amino/tests/proto3/

We should probably add the proto gen to a makefile recipe.

@thehowl
Copy link
Member

thehowl commented Apr 18, 2023

Mhh. I'll reopen this because IMO go mod tidy should work without saying or changing anything for all commits on master. I think the question of the issue becomes: do we want to keep the gitignore rule? Do we want to document this somehow? Do we want to add the files back?

@thehowl thehowl reopened this Apr 18, 2023
@tbruyelle
Copy link
Contributor Author

Good point. As far as I know, most cosmos chains track their .pb.go files in VCS, and this is also true for the cosmos-sdk.
The benefit is the code source is complete after the clone, so devs can hack immediately. The downside is the code in *.pb.go is large and ugly, which disrupts the code review (when it changes).

For accessibility, I would say we should track them.

@moul
Copy link
Member

moul commented Apr 18, 2023

I think I may have forgotten something in my working branch, let me quickly check.
While I'm doing that, please take a look at misc/genproto, which is where the generator is located.

Edit: -> #755

Edit2: let's add the .pb.go files, and about reviews, no worries, we can set a .gitattributes telling GH and other tools that those files are generated.

Edit3: I may add *.pb.go back to .gitignore in my PR and selectively add missing files. Gno doesn't rely on Protobuf directly, but it does use Amino, which has some Protobuf features.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Development

Successfully merging a pull request may close this issue.

3 participants