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

Protos flow #1798

Closed
3 tasks
faddat opened this issue Jul 27, 2022 · 2 comments
Closed
3 tasks

Protos flow #1798

faddat opened this issue Jul 27, 2022 · 2 comments

Comments

@faddat
Copy link
Contributor

faddat commented Jul 27, 2022

Summary of Bug

make proto-update-deps
make proto-all

Doesn't work quite right.

Has a change to makefile that fetches the correct ics23 .proto file, but we should work on the flow till it is buttery smooth.

Vuong uses arch, that works.

I use a mac, it doesn't.

The ubuntu .gitpod.yml container that comes by default, also doesn't. ghcr.io/notional-labs/cosmos does work.

Expected Behaviour

Version

Steps to Reproduce


For Admin Use

  • Not duplicate issue
  • Appropriate labels applied
  • Appropriate contributors tagged/assigned
@nicopernas
Copy link
Contributor

nicopernas commented Jul 28, 2022

I can repro the issue you are having on macOS caused by the sed command in the Makefile. This here should fix it and should work for both macOS and GNU sed

diff --git a/Makefile b/Makefile
index c8a42e0e..656cfd6a 100644
--- a/Makefile
+++ b/Makefile
@@ -462,7 +462,7 @@ proto-update-deps:

 ## insert go package option into proofs.proto file
 ## Issue link: https://github.com/confio/ics23/issues/32
-       @sed -i '4ioption go_package = "github.com/confio/ics23/go";' $(CONFIO_TYPES)/proofs.proto
+       @sed -ri -e 's|^(package ics23;)$$|\1\noption go_package = "github.com/confio/ics23/go";|' $(CONFIO_TYPES)/proofs.proto

With that, make proto-update-deps starts working. However, the proto-lint target fails because confio's proof.proto has issues.

I'm happy to send a PR for the former issue.

@crodriguezvega
Copy link
Contributor

I run both

make proto-update-deps
make proto-all

on my mac on main and they work just fine, so I think I will close the issue for now. If the problem re-appears we can open a new issue.

Thanks very much to both of you!

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

3 participants