Remove options in Managed Mode

[amckinney]

We recently had a feature request related to PHP code generation and protoc-gen-validate. In summary, PHP can't generate code for proto2 files, so anything that depends protoc-gen-validate will not compile.

We want users to incorporate protoc-gen-validate as they please, but it's really only relevant when generating the server implementation in the user's language of choice (go in this case).

We've discussed managing .proto options in general (not just file-level options), so this is a good first candidate to explore that space. In this case, we would need to remove all instances of a particular set of options (those provided by validate.rules) and we would need to remove the import validate/validate.proto; declaration so that the PHP stubs don't initialize the initOnce hook.

[bufdev]

For future reference, this is roughly what we want:

diff --git a/data/template/buf.go-client.gen.yaml b/data/template/buf.go-client.gen.yaml
index 68b84740..7b522a82 100644
--- a/data/template/buf.go-client.gen.yaml
+++ b/data/template/buf.go-client.gen.yaml
@@ -5,6 +5,12 @@ managed:
   enabled: true
   go_package_prefix:
     default: github.com/bufbuild/buf/private/gen/proto/go
+  options:
+    exclude:
+      - (validate.rules)
+  imports:
+    exclude:
+      - validate/validate.proto
 plugins:
   - name: go-grpc
     out: private/gen/proto/go

[amckinney]

At a high level, the proposal is fairly easy to implement - we can modify the Image just like the rest of the other Managed Mode modifiers, and strip each of the options/imports from the Image. Unfortunately, this means that the user could modify the Image in a way that makes it no longer compile.

For example, suppose that I strip the google/protobuf/descriptor.proto file from the buf.build/bufbuild/buf module. If I use this feature, the protoc-gen-go plugin yields the following (from the root of this repository):

$ buf generate proto --template data/template/buf.go.gen.yaml
protoc-gen-go: invalid FileDescriptorProto "buf/alpha/image/v1/image.proto": proto: message field "buf.alpha.image.v1.ImageFile.message_type" cannot resolve type: "google.protobuf.DescriptorProto" not found

We could consider this a user error and justify that the feature is working as expected, but it's still not an ideal user experience - the plugins could now receive a malformed Image from buf (which was previously a guarantee).

Removing options is also strange - we would need to clear the extension from the Image. This means we need to iterate over and interact with the relevant unknown fields (like we do in protosource, or something else along those lines). We would only affect the plugins that look for them (as intended). However, that still doesn't get us everything we want - the import statement would still remain which ends up affecting specific plugins. For example, protoc-gen-go will execute init hooks for unused imports unless the import is weak (link).

In order to satisfy both of these requirements, we'd need to verify that the Image compiles after we're applied the Managed Mode modifiers and return an error if it doesn't (before any of the plugins are invoked). This suggests a specific modifier ordering - should the options and imports be removed before any of the other standard modifiers are applied, or after? It probably makes sense to apply them before, but it still means we need to compile the image twice: once before it is modified, and again after it's been modified. With that said, we'd only need to re-compile the Image if certain modifiers were enabled (the import statement modifier in this case).

---

We should consider whether or not the complexity in this feature is worth it. There's already a fair amount of learning required for users to understand Managed Mode, and this adds even more questions into the mix. I'm not so worried about the implementation in this case - I'm only concerned about the UX.

[amckinney]

Another thing to keep in mind here -

We're beginning to see similar functionality appear in different parts of buf. Now that we have partial images with the --type flag, there's an argument that this kind of behavior would be there instead of in Managed Mode. The two features act similarly - they both modify an Image in some way. We should be careful with introducing functionality to one and not the other, or introducing this functionality to both places and have multiple ways to do the same thing.

Similarly, this might be a feature that we want to implement on the AST before it's compiled into an Image (kinda similar to a buf format-esque approach). The user might actually want to modify the source itself so it can be published in two different places, where one version contains the options and another doesn't.

TL;DR We'll need to think about this one a bit more to decide where it should go.

[rauanmayemir]

Another case where this feature would shine is docs and api spec generators like gnostic. You wouldn't need them in the language-specific runtime.