pubref/rules_protobuf

gogo_proto_library with the "well known types" (google/protobuf)

robfig opened this issue · 12 comments

I'm having trouble getting anything working with gogo_proto_library importing "google/protobuf/wrappers.proto"

I found this documentation / example:
https://github.com/pubref/rules_protobuf/blob/master/examples/wkt/go/BUILD

But I'm having trouble getting it to work. Here's a repo that demonstrates the build failure:
https://github.com/robfig/go_protobuf_test

Build failure:

ERROR: /Users/robfig/gogo_protobuf/BUILD:11:1: no such target '@com_github_golang_protobuf//:ptypes/any': target 'ptypes/any' not declared in package ''; however, a source directory of this name exists.  (Perhaps add 'exports_files(["ptypes/any"])' to /BUILD, or define a filegroup?) defined by /private/var/tmp/_bazel_robfig/fc5fe57c8a7eeabe79b8a6469f8a658b/external/com_github_golang_protobuf/BUILD.bazel and referenced by '//:protolib'
... many other similar ones ...

Are there any examples you can point me to?

Thank you,
Rob

I got this to work by applying this change:
robfig/go_protobuf_test@220f977

However, it logs these warnings which seem worth resolving:

GoLink: warning: package "github.com/golang/protobuf/ptypes/any" is provided by more than one rule:
    @io_bazel_rules_go//proto/wkt:any_go_proto
    @com_github_golang_protobuf//ptypes/any:go_default_library
Set "importmap" to different paths in each library.
This will be an error in the future.

(and similar errors for other WKTs)

I'm still not able to get it to build with gogo_proto_library. Here's the closest I've gotten so far:
https://github.com/robfig/go_protobuf_test/blob/2aa903c51082ad8891070a3afc3c49deb8572685/BUILD

Building results in this error:

GoCompile: missing strict dependencies:
	/private/var/tmp/_bazel_robfig/fc5fe57c8a7eeabe79b8a6469f8a658b/sandbox/darwin-sandbox/1/execroot/go_protobuf_test/bazel-out/darwin-fastbuild/genfiles/wkt.pb.go: import of "google/protobuf"
	/private/var/tmp/_bazel_robfig/fc5fe57c8a7eeabe79b8a6469f8a658b/sandbox/darwin-sandbox/1/execroot/go_protobuf_test/bazel-out/darwin-fastbuild/genfiles/wkt.pb.go: import of "google/protobuf/compiler"

My guess is that it needs to add these WKT parameters as a prefix to --gogo_out, e.g.:

---gogo_out=Mgoogle/protobuf/timestamp.proto=github.com/gogo/protobuf/types

Setting verbose in the BUILD rule shows that this is not set, but it is required in the usual build process. I would have expected the "importmap" property to control it, but it doesn't appear to.

pcj commented

@robfig I've just recently rewritten these rules to use the native proto_library and I think it's a better approach to dependencies. I actually just banged out the gogo support this morning if you care to take a look.

https://github.com/pubref/rules_proto/tree/master/github.com/gogo/protobuf

BTW, thanks for robfig/soy, it's awesome!

Oh, cool! I'll give it a go.

I also just realized my error; I had been using a fork of pubref/rules_protobuf updated to replace go_prefix with go_importpath (since upstream rules_go no longer accepts go_prefix), and I had mistakenly left importpath out of the gogo_proto_library rule. I pushed a fix that should allow me to prevail in this vein
yext@09bdd14

Cool, I didn't realize anyone else actually used soy. Cheers!

Where do I specify the importmap?
https://github.com/robfig/go_protobuf_test/blob/rules_proto/BUILD

That doesn't seem to be supported by either proto_library or proto_compile, unless I'm missing something

(Also, it seems to be missing the standard "gogo" flavor. I never investigated to see what gogoslick, gogofast, etc are)

pcj commented

Do you still need the importmap feature? It may be obsolete.

Oh, you are correct! Wonderful, thank you

FYI, I made some changes and have rules_proto working with gogo and WKTs
pubref/rules_proto@master...robfig:master
https://github.com/robfig/go_protobuf_test/blob/rules_proto/BUILD

This shouldn't be merged as-is due to the code duplication in github.com/gogo/protobuf/library.bzl and the now-different interface that gogo_proto_library exposes. Just wanted to share.

Thanks

Oh, one last note - I noticed that gogo provides a go library for the WKTs here:
"@com_github_gogo_protobuf//types:go_default_library",

However, I didn't find a way to make protoc generate those imports (maybe that's what the importmap could be used for?). We don't use that today so I didn't pursue that.

Doh, I think I do need importmap. Here's the scenario:

  1. We have a monorepo and proto files are sprinkled around, e.g. //src/com/corp/foo/foo.proto.
  2. Our Go code is in a GOPATH rooted inside the repo at //gocode
  3. Our pre-bazel build process (a shell script) for Go protobufs generates them into a git-ignored directory: //gocode/src/CORP/src/com/corp/foo/foo.pb.go

I implement that in our new bazel rules by specifying:

go_library(
    ...
    importpath = "CORP/" + PACKAGE_NAME
)

But that information is not available to protoc when it's generating the pb.go, so for protobufs that depend on other protobufs, it incorrectly generates:

import "src/com/corp/foo"

instead of

import "CORP/src/com/corp/foo"

I believe this is the use case for importmap? Any ideas for how to implement this?

pcj commented

How many proto files are you talking about? One way to implement this in the new design:

  • The proto_plugin is the supplier of options for the plugin, so to add custom options one can declare one as needed. For example consider the implementation to support the importpath feature, which greatly affects the location of the generated files:
def go_proto_compile(**kwargs):
    # If importpath specified, declare a custom plugin that should correctly
    # predict the output location.
    importpath = kwargs.get("importpath")
    if importpath and not kwargs.get("plugins"):
        name = kwargs.get("name")
        name_plugin = name + "_plugin"
        proto_plugin(
            name = name_plugin,
            outputs = ["{package}/%s/{basename}.pb.go" % importpath],
            tool = "@com_github_golang_protobuf//protoc-gen-go",
        )
        kwargs["plugins"] = [name_plugin]
        kwargs.pop("importpath")
    # Define the default plugin if still not defined
    if not kwargs.get("plugins"):
        kwargs["plugins"] = [str(Label("//go:go"))]

    proto_compile(
        **kwargs
    )

So, one could check for an importmappings dict in the kwargs, construct a list of options like Msrc/com/corp/foo.proto=CORP/src/com/corp/foo.proto, and declare the plugin as above.

If this look OK we can put it into the go rules more generically.

pcj commented

However, the drawback with this approach is that it requires up-front knowledge of the remainder of the proto_plugin attributes that would result in code duplication.