proto: revert strict erroring of deterministic and custom marshalers

[dsnet]

PR #650 added strict checking of the use of deterministic with custom marshalers
since it is impossible to know whether a custom marshalers actually do produce
deterministic output or not.

However, this check is breaking hundreds of targets that already rely on
determinism along with custom marshalers. In every case, the custom marshaler
already produced deterministic output, so it did not really matter.

If deterministic is specified and a custom marshaler is not actually
deterministic, then the output is obviously not deterministic,
and setting the flag was a lie. However, there is not much we can do with the
current API. A redesign of the proto API will resolve this tension.

[dsnet]

\cc @jmarais @awalterschulze

[awalterschulze]

This is very unfortunate.

I don't know what to do for gogoprotobuf to ensure compatibility with golang/protobuf, since we can generate a Marshal method that is either deterministic or not, depending on the extension specified by the user per message.

Another use case: The user with control over a top level Marshal method, might not have control over a 3rd party lower level.

Also when a custom Marshal method is hand coded there is no way to tell if it is deterministic or not or even to tell what the coder intended. If this method had a name like DeterministicMarshal or a deterministic bool parameter, I would be much more comfortable, to defer to user intention, but for now I don't know.

I do get that you had to be pragmatic, but this issue has been a lot of work for us and after much discussion, returning an error was the only solution we could come up with.

[awalterschulze]

Really you couldn't have punched me in the gut harder than you have just done ;)

[dsnet]

Really you couldn't have punched me in the gut harder than you have just done ;)

Sorry :(

I don't know what to do for gogoprotobuf to ensure compatibility with golang/protobuf, since we can generate a Marshal method that is either deterministic or not, depending on the extension specified by the user per message.

In this case, I think it's fine for gogo/protobuf and golang/protobuf to diverge in behavior. The situation is unfortunate. When I looked at the failures inside Google, most of the cases of custom Marshalers and Unmarshalers exist because of lack of protobuf reflection. I could spend several weeks cleaning those up hacking around them, or the time could be better spent developing the new reflection APIs.

Furthermore, the interfaces defined in the protoiface package would provide a much more scalable way to specify options and detect support for both golang/protobuf and gogo/protobuf.

I CC'd you on golang/go#26723, where we will be pushing a lot of the reflection code that we have been working on. Personally, I think there's a bright future for Go protobuf interoperability ahead, but there'll be some bumps in the road along the way.

[awalterschulze]

In this case, I think it's fine for gogo/protobuf and golang/protobuf to diverge in behavior.

Fair enough. I think we have decided the same thing. Although it is not ideal, it is pragmatic.