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

[WIP] Try the latest protobuf code generators #40

Closed
wants to merge 5 commits into from

Conversation

kzys
Copy link
Member

@kzys kzys commented Sep 8, 2021

Signed-off-by: Kazuyoshi Kato katokazu@amazon.com

@estesp
Copy link
Member

estesp commented Sep 8, 2021

just FYI, I noticed something looking at this run--the go.mod module name was never changed to the containerd repo: https://github.com/containerd/protobuild/blob/main/go.mod#L1

@kzys kzys force-pushed the protoc-upgrade branch 7 times, most recently from 08abd43 to eb6c190 Compare September 8, 2021 21:51
Signed-off-by: Kazuyoshi Kato <katokazu@amazon.com>
Signed-off-by: Kazuyoshi Kato <katokazu@amazon.com>
- plugins are fully deprecated.
- packages may work, but gogo wouldn't.

Signed-off-by: Kazuyoshi Kato <katokazu@amazon.com>
Signed-off-by: Kazuyoshi Kato <katokazu@amazon.com>
@kzys kzys requested a review from stevvooe September 8, 2021 22:13
@@ -5,11 +5,6 @@ version = "unstable"
# example that selects the ctrd vanity binary.
# generator = "gogoctrd"
Copy link
Member Author

Choose a reason for hiding this comment

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

According to https://grpc.io/docs/languages/go/quickstart/#regenerate-grpc-code, the new command line options for generating gRPC code is like

$ protoc --go_out=. --go_opt=paths=source_relative \
    --go-grpc_out=. --go-grpc_opt=paths=source_relative \
    helloworld/helloworld.proto

So we may need to have multiple generators instead.

# Plugins allows one to specify one or more plugins for use in generation.
#
# The common example grpc is provided below.
plugins = ["grpc"]
Copy link
Member Author

Choose a reason for hiding this comment

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

The new Go code generator doesn't support plugins. We may need to deprecate the field.

@@ -53,10 +48,6 @@ plugins = ["grpc"]
#
# We have a few examples to map packages from the examples.
[packages]
Copy link
Member Author

Choose a reason for hiding this comment

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

The package replacement still works, but gogo would not work with the latest protobuf. We may need another packages as an example.

https://developers.google.com/protocol-buffers/docs/reference/go-generated#package

@@ -35,7 +35,7 @@ var (
{{- if $index}}+{{end}}
{{- $plugin}}
{{- end -}}
,{{- end -}}import_path={{.ImportPath}}
,{{- end -}}
Copy link
Member Author

Choose a reason for hiding this comment

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

I'm not really sure the effect of this removal. Apparently protobuild works without the flag.

@kzys kzys changed the title Run protobuild on GitHub Actions [WIP] Try the latest protobuf code generators Sep 8, 2021
@kzys kzys force-pushed the protoc-upgrade branch 2 times, most recently from e6b57d6 to ad41928 Compare September 8, 2021 22:54
Signed-off-by: Kazuyoshi Kato <katokazu@amazon.com>
@kzys
Copy link
Member Author

kzys commented Sep 8, 2021

The build is green now! Let me extract small PRs from here.

@dmcgowan dmcgowan added this to New in Code Review Sep 27, 2021
@kzys
Copy link
Member Author

kzys commented Oct 20, 2021

#44, #45 and #48 replace this PR.

@kzys kzys closed this Oct 20, 2021
Code Review automation moved this from New to Done Oct 20, 2021
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 this pull request may close these issues.

None yet

2 participants