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: better dev ux #789

Closed
tac0turtle opened this issue Feb 26, 2021 · 9 comments
Closed

proto: better dev ux #789

tac0turtle opened this issue Feb 26, 2021 · 9 comments

Comments

@tac0turtle
Copy link
Contributor

tac0turtle commented Feb 26, 2021

Description

Right now when a user uses startport with protobuf they are missing quite a few files if they would like to do something a little more complicated. If a user wants to import a sdk type, a very common thing, they need to go through copying over countless files from the sdk and even then protoc doesn't show the correct error many times.

Proposal 1

Include third_party in scaffolding

Proposal 2

Add a buf.yaml file. Protobuf is notoriously tricky to work with, buf.build aims to help with this.
Example: https://github.com/interchainberlin/ica/blob/master/buf.yaml

Proposal 3

Use docker to generate the protobuf binaries. The cosmos-sdk uses gogoproto which is apiv1 of go-protobuf. This is not compatible with apiv2, we should avoid dependency hell by just saying to generate the proto stubs you need docker.

Example:

proto-gen:
	@echo "Generating Protobuf files"
	$(DOCKER) run --rm -v $(CURDIR):/workspace --workdir /workspace tendermintdev/sdk-proto-gen sh ./scripts/protocgen.sh
@okwme
Copy link
Contributor

okwme commented Feb 26, 2021

big fans of #1 and #2!

i'm a little worried about #3 being a burden on starport users. I don' think it's likely that most starport users will have docker already installed and running, which would be a big barrier of entry to new-comers. Is it not possible to ensure the correct version of gogoproto is installed locally? maybe we could put version checkers somewhere instead of relying on docker?

@seantking
Copy link

Nice! I see your point about #3 @okwme but I don't think it's a big ask to expect people to install docker. I think so long as there is some error handling if docker is not found. Docker is pretty much a standard dev requirement these days.

Personally, I hate polluting my local environment with project-specific dependencies so it would be a nicer DUX. I actually already ran into some issues with this.

@fadeev
Copy link
Contributor

fadeev commented Feb 26, 2021

Starport has all standard cosmos, google and tendermint proto files embedded in the binary. This was intentional to reduce the amount of third-party code users see in their project. Users shouldn't worry about these extra dependencies. If they want other third party proto, they can drop them into proto_vendor dir.

Even though Docker is widely used, we prefer not to have it as a required dependency. buf.build is nice, but we can't use it programmatically. We can, however, use protoc programmatically.

@marbar3778 were there any issues you had with not being able to use third-party proto?

@tac0turtle
Copy link
Contributor Author

Starport has all standard cosmos, google and tendermint proto files embedded in the binary. This was intentional to reduce the amount of third-party code users see in their project. Users shouldn't worry about these extra dependencies. If they want other third party proto, they can drop them into proto_vendor dir.

hmm this isnt in the docs, I couldn't find it. I understand the want to not pollute the devs env, but this is a convention adopted by the proto industry. I would prefer to just offer what is standard instead of writing custom logic. But then again I don't use starport when developing modules and apps so I'm not the best person to have a say.

Even though Docker is widely used, we prefer not to have it as a required dependency. buf.build is nice, but we can't use it programmatically. We can, however, use protoc programmatically.

Do you also install --gocosmos_out -grpc-gateway_out and -doc_out. The docker file is used to abstract all these. If so, the dev is believing in the scaffolding tool to handle more things that may be necessary.

How does a user add a custom generator?

Buf forces users to comply with proto standards and helps them avoid costly mistakes. I highly recommend passing this on to the user. Best and standardized processes are there because people have gone through the hard aches of using software.

@marbar3778 were there any issues you had with not being able to use third-party proto?

in the repo mentioned above https://github.com/interchainberlin/ica/tree/master/third_party/proto we had to explicitly add it. Does this proto_vendor style only work with newly generated apps or can you migrate an old app?

@fadeev
Copy link
Contributor

fadeev commented Feb 26, 2021

Do you also install --gocosmos_out -grpc-gateway_out and -doc_out. The docker file is used to abstract all these. If so, the dev is believing in the scaffolding tool to handle more things that may be necessary.

Yes, we will be generating client code in Starport and abstracting these things from the user.

@fadeev
Copy link
Contributor

fadeev commented Feb 26, 2021

These are good points. We need to think about this to find a good balances between how experienced developers may be using Starport and how to keep approachable for new developers as well.

@ilgooz
Copy link
Member

ilgooz commented Feb 26, 2021

Starport aims to provide a first class proto support with the best UX possible. We need every feedback we can get, I'm really excited to see that we have some momentum on this topic!

Let me go through how we deal with proto right now and how we aim to improve it.

Starport basically wants to make proto easier for SDK apps and devs during all kind of code generation processes. Such as Go code generation (comforting sdk.Msg), JS/TS client code generation, gRPC Gateway, gRPC Web code generation and OpenAPI specs.

Not limited to these, we also aim to generate code for the high level actors that are also based on proto e.g. generating Vuex stores for modules.

We have a few branches improving on the proto and code generation right now, one of them is feat/codegen-dep-modules. You can check out the generic packages we're implementing here, including the ones related to our subject. But please keep in mind that some other branches might be more up-to-day concerning different proto/code generation related packages under pkg/.

1-

protoanalysis is for understanding proto files. It is basically for analysing an app's proto folder and to create some metadata so, we can use it later during code generation. We also have protopath and protoc helpers.

2-

cosmosanalysis is a dedicated package that analyses an SDK app's source code and its proto folder. cosmosanalysis/module makes a static analysis on an app to find out the types in the source code that implements sdk.Msg and associates them to their proto representatives. On another branch, we have a more slightly improved version of this which also provides the info about which proto file a message is defined in like this.

3-

cosmosgen is the entry point to generate any code for any SDK based chain.

It is a generic Go pkg, not tied to Starport and has high level APIs to generate Go, JS, TS, gRPC Gateway/Web code and OpenAPI specs. Also generates a wrapper around generated JS, gRPC Gateway/Web code to provide a high level pure JS client that can be used by the browsers and NodeJS.

It is also responsible to install any required protoc plugins (Go ones) automatically including the JS dependencies through nodetime.

We have this thing called nodetime (generated by our script) meaning a nodejs runtime that enables us to benefit from nodejs ecosystem for JS related code generation. We directly embed NodeJS and tools we use tsc, ts-proto and sta into Starport for both linux and darwin, this adds around ~40MB to our total binary size. These tools are available within their own Go packages, can be imported and used with a high level API.

We're also thinking to embed protoc and google's proto inside Starport. This way, there will be no dependencies that developer will ever need to install while dealing with proto and code generation.

_

All starport commands (serve, build, type, module, app) are able to generate needed Go code by the app itself and do other optional code generation such as generating JS clients and Vuex stores, all can be controlled through the config.yml (tho, if an app doesn't have this config file, we do our best and still do Go code generation when build and serve commands are used).

Code generation is made for each module inside the app and the 3rd party modules that are used by the app, including SDK(not any version of the SDK, the one that locked inside go.mod). Under the hood, we basically go though app's go.mod to find out which modules it depends on by using our cosmosanalysis pkg and generate code for them in addition to generating code for app's own modules.

Conserning cosmosgen, it automatically includes SDK's (the one that it's version locked inside app's go.mod) proto path to protoc or other code generation tools we use while generating code for different targets. SDK's proto is available out of the box in the same way how google's protos are made available by default with use of protoc.

In context of Starport, it automatically recognizes a proto folder and these include paths (in case users want to copy-paste some 3rd party protos). These are configurable through config.yml.

Please keep in mind that things quickly change and we have a bunch of branches right now related to proto and code generation. We're actively working on implementing JS related code generation and refactoring what we have. The goal is to grow cosmosgen into something that everyone can use by directly importing as a Go package without installing dependencies. So, folks can write CLIs that uses it (e.g. Starport! :)) or they can do code generation with advanced configs to meet their custom needs.

@tac0turtle
Copy link
Contributor Author

tac0turtle commented Feb 26, 2021

Amazing to see all this work. I was mainly pointing out how things are done, so you wouldn't need to recreate the wheel.

The flow makes sense, if a user is new to protobuf it makes sense to do this, but I can see people who have worked with protobuf being confused and opting to not use it. Not a problem either way.

I don't think any of these tools do what buf does(?) Buf is adopted as a standard for working with protobuf, I still recommend using it to instill best practices.

Hopping on a call next week may be easier than github, thoughts?

@fadeev
Copy link
Contributor

fadeev commented Apr 9, 2021

Resolved during a call. We've opted to internalize proto-related processes and will gradually expose customization options.

@fadeev fadeev closed this as completed Apr 9, 2021
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