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

Robert/update protogen #1455

Closed
wants to merge 4 commits into from
Closed

Robert/update protogen #1455

wants to merge 4 commits into from

Conversation

robert-zaremba
Copy link
Contributor

What is the purpose of the change

  • updating protogen tooling to the buf based setup

Documentation and Release Note

  • Does this pull request introduce a new feature or user-facing behavior changes? NO
  • Is a relevant changelog entry added to the Unreleased section in CHANGELOG.md? (yes / no)
  • How is the feature or change documented? (not applicable / specification (x/<module>/spec/) / Osmosis docs repo / not documented)

@robert-zaremba
Copy link
Contributor Author

There are some proto build errors related to imports.

@codecov-commenter
Copy link

codecov-commenter commented May 9, 2022

Codecov Report

Merging #1455 (c7e596d) into main (f56fbe5) will decrease coverage by 0.31%.
The diff coverage is 18.70%.

@@            Coverage Diff             @@
##             main    #1455      +/-   ##
==========================================
- Coverage   19.82%   19.51%   -0.32%     
==========================================
  Files         202      229      +27     
  Lines       27685    32097    +4412     
==========================================
+ Hits         5489     6263     +774     
- Misses      21175    24704    +3529     
- Partials     1021     1130     +109     
Impacted Files Coverage Δ
x/epochs/client/cli/query.go 0.00% <ø> (ø)
x/gamm/client/cli/query.go 36.74% <0.00%> (-0.36%) ⬇️
x/gamm/pool-models/stableswap/msgs.go 0.00% <0.00%> (ø)
x/gamm/pool-models/stableswap/pool.go 0.00% <0.00%> (ø)
x/gamm/pool-models/stableswap/stableswap_pool.go 0.00% <0.00%> (ø)
.../gamm/pool-models/stableswap/stableswap_pool.pb.go 0.58% <0.00%> (-0.09%) ⬇️
x/gamm/types/pool.go 0.00% <ø> (ø)
x/incentives/client/cli/query.go 0.00% <ø> (ø)
x/incentives/keeper/distribute.go 61.00% <0.00%> (ø)
x/incentives/keeper/hooks.go 50.00% <0.00%> (ø)
... and 53 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update ddaa866...c7e596d. Read the comment docs.

@ValarDragon
Copy link
Member

Thanks for updating Osmosis to the newer setup! Is there something I can read up on to understand the differences?

@faddat
Copy link
Member

faddat commented May 10, 2022

Thanks Robert!

at least 15x better than the docker way.

:)

--grpc-gateway_out=logtostderr=true:. \
$(find "${dir}" -maxdepth 1 -name '*.proto')
done
go mod tidy -compat=1.17
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can we make this:

go mod tidy -go=1.18

?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

no, because we want to use it in 0.46

@faddat
Copy link
Member

faddat commented May 19, 2022

Robert, does this script you added also depend on a protocgen2.sh file?

I have like

!/usr/bin/env bash

#== Requirements ==
#
## make sure your `go env GOPATH` is in the `$PATH`
## Install:
## + latest buf (v1.0.0-rc11 or later)
## + protobuf v3
#
## All protoc dependencies must be installed not in the module scope
## currently we must use grpc-gateway v1
# cd ~
# go install google.golang.org/protobuf/cmd/protoc-gen-go@latest
# go install google.golang.org/grpc/cmd/protoc-gen-go-grpc@latest
# go install github.com/grpc-ecosystem/grpc-gateway/[email protected]
# go install github.com/cosmos/cosmos-proto/cmd/protoc-gen-go-pulsar@latest
# go get github.com/regen-network/cosmos-proto@latest # doesn't work in install mode
# go get github.com/regen-network/cosmos-proto/[email protected]

set -eo pipefail

echo "Generating gogo proto code"
cd proto
proto_dirs=$(find . \( -path ./third_party -o -path ./vendor \) -prune -o -name '*.proto' -print0 | xargs -0 -n1 dirname | sort | uniq)

for dir in $proto_dirs; do
  for file in $(find "${dir}" -maxdepth 1 -name '*.proto'); do
    if grep "option go_package" $file &> /dev/null ; then
      buf generate --template buf.gen.gogo.yaml $file
    fi
  done
done

cd ..

# generate codec/testdata proto code
(cd testutil/testdata; buf generate)

# move proto files to the right places
cp -r github.com/cosmos/cosmos-sdk/* ./
rm -rf github.com

go mod tidy -compat=1.17

./scripts/protocgen2.sh

@robert-zaremba
Copy link
Contributor Author

No, it doesn't need protocgen2.sh - because the Pulsar is not used by Osmosis yet. That would require bigger migration, which can be done step by step.

mergify bot pushed a commit that referenced this pull request Jun 2, 2022
A re-attempt of #1455, but I've reached the capacity of my knowledge here. Maybe @robert-zaremba or @aaronc can chime in on how to get this to the finish line.

Currently, running `make proto-gen` yields:

```
make proto-gen
Generating Protobuf files
Generating gogo proto code
WARNING: field Pool.ScalingFactor is a repeated non-nullable native type, nullable=false has no effect
WARNING: field Pool.ScalingFactor is a repeated non-nullable native type, nullable=false has no effect
WARNING: field MsgStableSwapAdjustScalingFactors.ScalingFactors is a repeated non-nullable native type, nullable=false has no effect
Cleaning API directory
./scripts/protocgen2.sh: cd: line 14: can't cd to api: No such file or directory
make: *** [proto-gen] Error 2
```

It also produces an erroneous `v7/` dir in the root.
@robert-zaremba
Copy link
Contributor Author

Superseded by #1589

@robert-zaremba robert-zaremba deleted the robert/update-protogen branch June 2, 2022 22:30
puneet2019 pushed a commit to persistenceOne/persistence-sdk that referenced this pull request Jul 30, 2022
A re-attempt of osmosis-labs/osmosis#1455, but I've reached the capacity of my knowledge here. Maybe @robert-zaremba or @aaronc can chime in on how to get this to the finish line.

Currently, running `make proto-gen` yields:

```
make proto-gen
Generating Protobuf files
Generating gogo proto code
WARNING: field Pool.ScalingFactor is a repeated non-nullable native type, nullable=false has no effect
WARNING: field Pool.ScalingFactor is a repeated non-nullable native type, nullable=false has no effect
WARNING: field MsgStableSwapAdjustScalingFactors.ScalingFactors is a repeated non-nullable native type, nullable=false has no effect
Cleaning API directory
./scripts/protocgen2.sh: cd: line 14: can't cd to api: No such file or directory
make: *** [proto-gen] Error 2
```

It also produces an erroneous `v7/` dir in the root.
larry0x pushed a commit to larry0x/wasmd that referenced this pull request Jul 17, 2023
A re-attempt of osmosis-labs/osmosis#1455, but I've reached the capacity of my knowledge here. Maybe @robert-zaremba or @aaronc can chime in on how to get this to the finish line.

Currently, running `make proto-gen` yields:

```
make proto-gen
Generating Protobuf files
Generating gogo proto code
WARNING: field Pool.ScalingFactor is a repeated non-nullable native type, nullable=false has no effect
WARNING: field Pool.ScalingFactor is a repeated non-nullable native type, nullable=false has no effect
WARNING: field MsgStableSwapAdjustScalingFactors.ScalingFactors is a repeated non-nullable native type, nullable=false has no effect
Cleaning API directory
./scripts/protocgen2.sh: cd: line 14: can't cd to api: No such file or directory
make: *** [proto-gen] Error 2
```

It also produces an erroneous `v7/` dir in the root.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

4 participants