Fix `go_package` option behaviour on go proto API v2

Question

Fix `go_package` option behaviour on go proto API v2

arunk-s opened this issue 5 years ago · 10 comments

First, A big Thanks for open-sourcing and maintaining prototool.

Since the release of new protobuf API v2 for Golang, option go_package is now required to provide full import path.

This collides with the current recommendations from prototool.

When using protoc-gen-go APIv1 implemented in the terms of APIv2 at version 1.4.0 , prototool generates the following warning.

2020-04-10T13:57:41.333+0200    WARN    protoc returned a line we do not understand, please file this as an issue at https://github.com/uber/prototool/issues/new       {"protocLine": "2020/04/10 13:57:41 WARNING: Deprecated use of 'go_package' option without a full import path in \"foo/bar/baz.proto\", please specify:"}
2020-04-10T13:57:41.333+0200    WARN    protoc returned a line we do not understand, please file this as an issue at https://github.com/uber/prototool/issues/new       {"protocLine": "option go_package = \"foo/bar/baz;foo_bar_baz\";"}
2020-04-10T13:57:41.333+0200    WARN    protoc returned a line we do not understand, please file this as an issue at https://github.com/uber/prototool/issues/new       {"protocLine": "A future release of protoc-gen-go will require the import path be specified."}
2020-04-10T13:57:41.333+0200    WARN    protoc returned a line we do not understand, please file this as an issue at https://github.com/uber/prototool/issues/new       {"protocLine": "See https://developers.google.com/protocol-buffers/docs/reference/go-generated#package for more information."}

Note that there is a separate option to provide paths=source_relative flag to protoc-gen-go which changes the path of the output fields to relative paths. But that doesn't help with the above warnings.

I'll leave the final decision of how prototool will incorporate the changes required for the new APIv2 on the maintainers.

But what I would love to see as a suggested change is:

Removal of existing warnings by providing full import paths whether in terms of combination of import_path or removal of the import_path entirely.
Provide a lint option for not providing full import paths.
Maybe a separate option for ignoring the above warnings on new protoc-gen-go versions >1.3.4 if the change in behaviour of import_path is backward incompatible for prototool as long as the generated code is not impacted.

Answer 1 · 2020-04-17T03:21:02.000Z

meet the same issue

Answer 2 · 2020-04-30T17:45:29.000Z

I have the same issue

Answer 3 · 2020-04-30T19:28:09.000Z

it seems that there is a conflict between this import_path, output dir, and defining the path in the proto file. The weirdness can be seen in the import patth of the generated files, which import other generated files. I worked around this problem with bash (moving files up one directory)

Answer 4 · 2020-05-28T15:29:04.000Z

I would like to chime in on this issue. We have found another issue with linting rules, specifically FILE_OPTIONS_EQUAL_GO_PACKAGE_V2_SUFFIX. The problem is it checks for equality only. So when you disable FILE_OPTIONS_GO_PACKAGE_NOT_LONG_FORM, you cannot use syntax like option go_package "foo/bar/v1;barv1 together with FILE_OPTIONS_EQUAL_GO_PACKAGE_V2_SUFFIX.

Since API v2 also requires full path in go_package, it would seem like fixing the issue above is also a requirement. Most likely by introducing FILE_OPTIONS_SUFFIX_GO_PACKAGE_V2_SUFFIX that checks go_package suffix or something like that.

Answer 5 · 2020-06-03T09:58:21.000Z

We have the same issue.

Answer 6 · 2020-06-05T13:26:58.000Z

Seeing the same issue!
And protobuf doc about this https://developers.google.com/protocol-buffers/docs/reference/go-generated#invocation

Answer 7 · 2020-06-05T15:00:42.000Z

We found a workaround though I'm not sure if this works for everyone else. For everyone facing a similar issue they can try it out, if it fits with their project setup.

Note that this workaround require to have full go_package path defined in protobuf files in the recommended format prior to running prototool.

Ideally it'll be nice if the package path in proto files matches the import_path but since prototool explicitly uses -M option to map module names while generation, it should be okay.

generate:
  go_options:
    import_path: github.com/foo/protobuf
    extra_modifiers:
        google/api/annotations.proto: google.golang.org/genproto/googleapis/api/annotations
        google/api/http.proto: google.golang.org/genproto/googleapis/api/annotations
  plugins:
    # using relative path ../ and generate the protobuf/grpc code in the parent directory
  - name: go
    type: go
    flags: plugins=grpc,paths=source_relative
    output: ../protobuf

  - name: grpc-gateway
    type: go
    flags: paths=source_relative
    output: ../protobuf

On a side note, there is a new flag --go-opt=module to protoc that specifically tries to provide the same thing as prototool tries to do with overriding every module import with -M flag.

For more information on source_relative vs --go-opt=module see this issue.

Answer 8 · 2020-07-06T08:12:16.000Z

Please consider this issue for v1.11.0

Answer 9 · 2020-07-16T07:28:42.000Z

We found a workaround though I'm not sure if this works for everyone else. For everyone facing a similar issue they can try it out, if it fits with their project setup.

Note that this workaround require to have full go_package path defined in protobuf files in the recommended format prior to running prototool.

Ideally it'll be nice if the package path in proto files matches the import_path but since prototool explicitly uses -M option to map module names while generation, it should be okay.
generate:
  go_options:
    import_path: github.com/foo/protobuf
    extra_modifiers:
        google/api/annotations.proto: google.golang.org/genproto/googleapis/api/annotations
        google/api/http.proto: google.golang.org/genproto/googleapis/api/annotations
  plugins:
    # using relative path ../ and generate the protobuf/grpc code in the parent directory
  - name: go
    type: go
    flags: plugins=grpc,paths=source_relative
    output: ../protobuf

  - name: grpc-gateway
    type: go
    flags: paths=source_relative
    output: ../protobuf
On a side note, there is a new flag --go-opt=module to protoc that specifically tries to provide the same thing as prototool tries to do with overriding every module import with -M flag.

For more information on source_relative vs --go-opt=module see this issue.

This solution does not work for me.

Answer 10 · 2020-09-02T13:44:47.000Z

Managing to mostly work around this by applying a similar configuration to @arunk-s, but disabling the relevant linting rules:

protoc:
  version: 3.6.1 # could do with an update here... haven't tested with newer versions
generate:
  go_options:
    import_path: ... # my package here
  plugins:
    - name: go
      type: go
      flags: plugins=grpc # didn't appear to need 'paths=source_relative', but outputting to a different location
      output: ../generated/grpc
lint:
  group: uber2
  rules:
    remove:
      # Protogen now expects the full package path to be specified, which conflicts with the Uber2 style/linting
      # https://developers.google.com/protocol-buffers/docs/reference/go-generated#package
      - FILE_OPTIONS_EQUAL_GO_PACKAGE_V2_SUFFIX
      - FILE_OPTIONS_GO_PACKAGE_NOT_LONG_FORM

With this, code generation and linting checks are completing successfully, with package declarations like option go_package = "foo/bar/feature/v1;featurev1";

We are also running prototool format over our proto files however, and the formatting wipes out the changed go_package paths, reverting them to option go_package = "featurev1";