protoc-gen-go: import public should be recursive

[dsnet]

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

[dsnet]

It seems that this can be addressed in the new API by changing protoreflect.FileDescriptor.DescriptorByName:

protobuf/reflect/protoreflect/type.go

Lines 181 to 183 in b4e370e

	// DescriptorByName looks up any descriptor declared within this file
	// by full name. It returns nil if not found.
	DescriptorByName(FullName) Descriptor

to also search the name within any files that were publicly imported (by also calling DescriptorByName which will have the desired recursive effect).

[dsnet]

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:

protobuf/reflect/prototype/protofile_desc.go

Lines 303 to 320 in 01ab296

	func findMessageDescriptor(s string, r *protoregistry.Files) (protoreflect.MessageDescriptor, error) {
	if !strings.HasPrefix(s, ".") {
	return nil, errors.New("identifier name must be fully qualified with a leading dot: %v", s)
	}
	name := protoreflect.FullName(strings.TrimPrefix(s, "."))
	switch m, err := r.FindDescriptorByName(name); {
	case err == nil:
	m, ok := m.(protoreflect.MessageDescriptor)
	if !ok {
	return nil, errors.New("resolved wrong type: got %v, want message", typeName(m))
	}
	return m, nil
	case err == protoregistry.NotFound:
	return PlaceholderMessage(name), nil
	default:
	return nil, err
	}
	}

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

[dsymonds]

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.