protoc import should not conflict with option override anymore

[bennyscetbun]

Problem:
Protoc is appending the "-import" as "-option M". But with gogofaster compiler some "-option M" are added and will be override by the default import, which is not what we want.

Solution:
append the "-import" as "-option M" only if the "-option M" is not already set. This should avoid overriding compiler options.

[jayconrod]

Thanks for working on this.

Could you explain exactly what options gogofaster adds that will be overridden? Sorry I'm not all that familiar with gogo. I'm wondering if we should adjust which arguments are passed to the builder from the rules instead of fixing this in the builder. At minimum, this should be carefully explained in a comment somewhere.

[bennyscetbun]

I tried to avoid to change too much code.
Right now you ve got 2 different path that will lead to the same result.
GOGO_WELL_KNOWN_TYPE_REMAPS will be used to generate "-option M"
And dependencies will generate "-import" that will in the end generate "-option M" for the final call to the real protoc.
So the only perfect story would be to merge the "GOGO_WELL_KNOWN_TYPE_REMAPS to option" code and the import code. But that s mean a lot of changes

[jayconrod]

I think I understand. So if you're using @io_bazel_rules_go//proto:gogofaster_proto compiler, when you import, for example, google/protobuf/duration.proto, your generated code should import github.com/gogo/protobuf/types, but because of the remap in well_known_types.bzl it actually imports github.com/golang/protobuf/ptypes/duration because it's overridden by the implicit dependency on @io_bazel_rules_go//proto/wkt:duration_go_proto, which generates code with the default proto plugin.

I can see how this PR prevents that override, but I think you're still going to have the wrong dependency, and you'll end up linking code from two different proto systems.

Instead, I think we need to define a go_proto_library rule for each WKT and each gogo plugin in @com_github_gogo_protobuf and then have the go_proto_compiler rules declare implicit dependencies on those rules instead of the standard rules in //proto/wkt. That would solve both problems. This can be part of a fix for #1548, which is a plan for defining go_proto_library rules for WKTs and other frequently imported libraries.

Does that make sense? If so, I'll close this PR and add a note to #1548, then fix this in the next few weeks. Again, I don't have a complete understand of gogo protobuf,

[bennyscetbun]

the current pr is working perfectly fine. You can mix google and gogo imports.
But if you ve got a nicer solution with the same result, go for it :) and close this PR. We will use it internally until you ve released your fix.

[bennyscetbun]

Or you could merge this one and remove the code when your solution will be ready

[jayconrod]

I'm not sure it's safe to mix golang/protobuf and gogo/protobuf. This article mentions that they register with different backends, which is consistent with my understanding. It sounds like the situation is improving though.

If this change is working for you, I'd encourage you to continue using it internally until a fix is ready. I hope to have something ready before the next major release (first half of July).

[jayconrod]

I've added a note in #1548, and I'll close this PR now.

I'm sorry this is as broken as it is. I think we will end up with something much more useful soon though.