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

go_package paths in proto files not supported #305

Closed
rutsky opened this issue Sep 2, 2018 · 2 comments
Closed

go_package paths in proto files not supported #305

rutsky opened this issue Sep 2, 2018 · 2 comments

Comments

@rutsky
Copy link
Contributor

rutsky commented Sep 2, 2018

I'm trying to generate build rules with Gazelle for a project that uses interdependent proto files and include them by package name, but Gazelle/rules_go fails in multiple ways.

I prepared small example to reproduce the issue: https://github.com/rutsky/gazelle_proto_import

├── BUILD
├── hello_proto
│   ├── BUILD.bazel
│   └── hello.proto
├── server_proto
│   ├── BUILD.bazel
│   └── server.proto
└── WORKSPACE

server.proto:

syntax = "proto3";

package server;

import "github.com/rutsky/gazelle_proto_import/hello_proto/hello.proto";

option go_package = "github.com/rutsky/gazelle_proto_import/server_proto;server"; 

service Greeter {
  rpc SayHello (hello.HelloRequest) returns (hello.HelloReply) {}
}

hello.proto:

syntax = "proto3";

package hello;

option go_package = "github.com/rutsky/gazelle_proto_import/hello_proto;hello"; 

message HelloRequest {
  string name = 1;
}

message HelloReply {
  string message = 1;
}

server.proto depends on hello.proto by using absolute package import path.

There are following issues.

  1. When I run bazel run //:gazelle, server_proto rule for server.proto gets deps = ["//github.com/rutsky/gazelle_proto_import/hello_proto:hello_proto_proto"], which clearly doesn't exists and build fails.

    I can fix this by manually setting proper path to dependency (e.g. rutsky/gazelle_proto_import@7c45b8d).

  2. With fixed dependency this still won't build, as bazel is not informed of what are those absolute packages names:

$ bazel build -s //server_proto
INFO: Analysed target //server_proto:server_proto (0 packages loaded).
INFO: Found 1 target...
SUBCOMMAND: # //server_proto:server_proto [action 'Generating Descriptor Set proto_library //server_proto:server_proto']
(cd /home/bob/.cache/bazel/_bazel_bob/b5a692e6e2b62f927f1e063c9b530723/execroot/gazelle_proto_import && \
  exec env - \
    LD_LIBRARY_PATH=/home/bob/.local/lib:/home/bob/.local/lib: \
    PATH=/home/bob/.local/share/umake/bin:/home/bob/bin:/home/bob/stuff/go//bin:/home/bob/bin:/home/bob/.nvm/versions/node/v6.4.0/bin:/home/bob/.local/bin:/home/bob/.gem/ruby/2.2.0/bin:/home/bob/bin:/home/bob/stuff/go//bin:/home/bob/bin:/home/bob/.local/bin:/home/bob/.gem/ruby/2.2.0/bin:/home/bob/bin:/home/bob/.local/share/umake/bin:/home/bob/bin:/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin:/usr/games:/usr/local/games:/snap/bin:/mnt/media/bob/Android/Sdk/tools:/mnt/media/bob/Android/Sdk/platform-tools:/mnt/media/bob/Android/Sdk/tools:/mnt/media/bob/Android/Sdk/platform-tools \
  bazel-out/host/bin/external/com_google_protobuf/protoc '--descriptor_set_out=bazel-out/k8-fastbuild/genfiles/server_proto/server_proto-descriptor-set.proto.bin' '-Iserver_proto/server.proto=server_proto/server.proto' '-Ihello_proto/hello.proto=hello_proto/hello.proto' --direct_dependencies hello_proto/hello.proto:server_proto/server.proto '--direct_dependencies_violation_msg=%s is imported, but //server_proto:server_proto doesn'\''t directly depend on a proto_library that '\''srcs'\'' it.' server_proto/server.proto)
ERROR: /home/bob/stuff/go/src/github.com/rutsky/gazelle_proto_import/server_proto/BUILD.bazel:4:1: Generating Descriptor Set proto_library //server_proto:server_proto failed (Exit 1)
github.com/rutsky/gazelle_proto_import/hello_proto/hello.proto: File not found.
server_proto/server.proto: Import "github.com/rutsky/gazelle_proto_import/hello_proto/hello.proto" was not found or had errors.
server_proto/server.proto:10:17: "hello.HelloRequest" is not defined.
server_proto/server.proto:10:46: "hello.HelloReply" is not defined.
Target //server_proto:server_proto failed to build
Use --verbose_failures to see the command lines of failed build steps.
INFO: Elapsed time: 0.306s, Critical Path: 0.06s
INFO: 0 processes.
FAILED: Build did NOT complete successfully

I expected that something similar by effect to this command would be generated:

protoc --go_out=plugins=grpc:. \
  -Igithub.com/rutsky/gazelle_proto_import/hello_proto/hello.proto=hello_proto/hello.proto \
  -Igithub.com/rutsky/gazelle_proto_import/hello_proto/server.proto=server_proto/server.proto \
  server_proto/server.proto 

I assume go_package paths should be supported by Gazelle (according to this comment they work and).
I saw that absolute includes work with some grpc/gogo built-ins (like import "google/protobuf/empty.proto";), though it seems this is hardcoded inside rules_go.

It seems this should be possible to implement at least as this was done for gogo support in rules_go (https://github.com/bazelbuild/rules_go/blob/8b0118fff2ea415c93f02670bd8fde5aa76d3404/proto/gogo.bzl#L34), though it seems too low level and overkill.

@jayconrod
Copy link
Contributor

I think you're running into a limitation in Bazel: bazelbuild/bazel#3867.

The proto_library rule (native to Bazel) is what runs protoc. It expects imports to be relative to the workspace root directory, either in a local workspace or in an external workspace. So you would need to write import hello_proto/hello.proto. There is no way that I know of to set the import directories.

If you can't update your .proto files (e.g., because some of them are vendored), I'd suggest pre-generating your .pb.go files outside of Bazel, and adding # gazelle:proto disable_global in your root build file to avoid generating proto_library and go_proto_library rules.

@jayconrod
Copy link
Contributor

Closing this because there's nothing Gazelle can do at the moment. When Bazel can build proto_library rules with imports like this, I'll update Gazelle to generate those rules.

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

2 participants