Make M option overwrite `option go_package` in the `.proto` file (as opposed to return an error)

[janpfeifer]

Is your feature request related to a problem? Please describe.

Yes. I need to use protos from a C/C++ repository that I have no control over. The "M" option (--go_opt=M<...file.proto>=<package_path>) solves most of the problems ... but one of the files has as left-over option go_package defined ... that breaks my generation of the proto Go bindings in my own package. When I try to overwrite that option go_package with an M option I get the error:

$ protoc --go_out=. -I=/home/janpf/src/xla -I=/home/janpf/src/xla/third_party/tsl --go_opt=Mxla/autotune_results.proto=github.com/gomlx/gopjrt/pjrt --go_opt=Mxla/autotuning.proto=github.com/g
omlx/gopjrt/pjrt --go_opt=Mxla/pjrt/compile_options.proto=github.com/gomlx/gopjrt/pjrt --go_opt=Mxla/service/hlo.proto=github.com/gomlx/gopjrt/pjrt --go_opt=Mxla/stream_executor/device_descri
ption.proto=github.com/gomlx/gopjrt/pjrt --go_opt=Mxla/xla.proto=github.com/gomlx/gopjrt/pjrt --go_opt=Mxla/xla_data.proto=github.com/gomlx/gopjrt/pjrt xla/pjrt/compile_options.proto         

protoc-gen-go: Go package "github.com/gomlx/gopjrt/pjrt" has inconsistent names pjrt (xla/autotune_results.proto) and for_core_protos_go_proto (xla/autotuning.proto)                          
--go_out: protoc-gen-go: Plugin failed with status code 1. 

Describe the solution you'd like

I want the Go proto generator to use as import path the one I'm providing with the M option.

Generally, allowing M to overwrite option go_package allows down-stream users of libraries to not depend on (potentially out-dated or in my case simply non-existant) Go generate packages on the location defined by option go_package.

Describe alternatives you've considered Script the patching of the source repository in a local clone removing the offending option go_package line.

A clear and concise description of any alternative solutions or features you've considered.

Additional context

I don't use often protocol buffers in Go. Maybe this functionality already exists and I missed it. Or some other nuance I'm not catching.

[janpfeifer]

Same issue in another file of the same repository setting to a go_package that doesn't exist or is maintained, and I have no way to generate.

[znkr]

Sorry for this inconvenience, but we're not convinced that M should override the go_package in a .proto file. We do expect that people only compile protos that they do control. I suggest you find a way change the files (e.g. via a pull request).

[janpfeifer]

hi @znkr , thanks for the reply. Let me argue in favor of the feature request once (I'll close the issue if you are not convinced):

"People only complile protos that they do control" --> that seems a limiting assumption. Meaning if some project exposes a public API with some complex proto structure, a downstream user won't be able to (easily) use it in their own way -- my case.

I understand Google is a mono-repo set up, so this type of problem never arises.

But outside in a world of different build systems, it gets tricky to integrate across repositories. Each project depends on dozens of other projects, and it is time consuming to try to do PRs for various reasons in each of the sub-projects. So making the "glue" between across projects more flexible (like allowing M to overwrite the pkg path) leads to time saving, and less need of external PRs :)

Also overwriting the go_package can be convenient for testing in some cases.

Not to say it needs to be with M, it could be something else, or only enabled by an optional flag ?

[znkr]

This is a fair point, but it's not something that we should address in an ad-hoc fashion. Either way, Your use case, a split between the proto definition and how it's build, doesn't sound like something we want to support at all. There are too many ways this can go wrong. I suggest to find a better way of building the protos you need.

[janpfeifer]

Thanks for considering it, I'm closing the issue.

I do think the enforcement of having the build parameters and definition together creates more problems than it solves ... but it's not up to me :)