protobuf

protobuf copied to clipboard

Consider the following set of files.

In file test1.proto:

syntax = "proto2";
import public "google/protobuf/descriptor.proto";

In file test2.proto:

syntax = "proto2";
import public "test1.proto";

In file test3.proto:

syntax = "proto2";
import public "test2.proto";

message Foo { optional google.protobuf.FileDescriptorProto field = 1; }

Compiling these with protoc --go_out=. test3.proto produces:

2018/09/03 22:46:30 protoc-gen-go: WARNING: failed finding publicly imported dependency for .google.protobuf.FileDescriptorProto, used in test3.proto
2018/09/03 22:46:30 protoc-gen-go: WARNING: failed finding publicly imported dependency for .google.protobuf.FileDescriptorProto, used in test3.proto
2018/09/03 22:46:30 protoc-gen-go: WARNING: failed finding publicly imported dependency for .google.protobuf.FileDescriptorProto, used in test3.proto

However, C++ is able to resolve google.protobuf.FileDescriptorProto just fine when running protoc --cpp_out=. test3.proto.

It seems that the behavior is that indirectly publicly import declarations are exported.

\cc @neild

It seems that this can be addressed in the new API by changing protoreflect.FileDescriptor.DescriptorByName: https://github.com/golang/protobuf/blob/b4e370ef3a86cd5a0f62874fe112e24e855be711/reflect/protoreflect/type.go#L181-L183 to also search the name within any files that were publicly imported (by also calling DescriptorByName which will have the desired recursive effect).

I'm fairly certain that the v2 API resolves this since prototype.findMessageDescriptor is responsible for resolving the message name, for which it searches the protoregistry.Files for all declarations that have been imported thus far: https://github.com/golang/protobuf/blob/01ab29648ebc1cc521a4d439d9a76668ee01f165/reflect/prototype/protofile_desc.go#L303-L320

However, this behavior is a more liberal than necessary, and validation (when implemented) will need to make sure not to accidentally break this again.

FYI, the definition of "import public" back when it was introduced (circa 2010) was not transitive. Maybe something changed, but it's possible that C++ is only "accidentally" supporting it.