go: add go_package options to proto files

[tmc]

The helper code that just landed temporarily writes file-level go_package options -- is there any reason these don't get added directly to the .proto files?

[suyashkumar]

Hi @tmc -- great question. If I recall correctly, the team initially looked into including the go_package options in the protocol buffer generator itself--but that required some other changes to inject the right context to generate the go_package path. We also didn't want to manually add the go_package option into the protos if it wasn't automatically added from the core proto generator, because re-running the generator as is would produce different output.

That's why we went with the interim approach to unblock the go modules migration asap--a generation script that manages building the go protos, which handles inserting the right go_package reliably during the generation process for now. This script needed to exist anyway to help manage rebuilding the go protos after any proto change (and hopefully as a user, it handles everything for you regardless of where the go_package temporarily lived).

We're planning on implementing the relevant changes needed to add the option to proto generator as well in the near future. We can keep this issue open and post updates here as that comes to fruition.

Let us know if you have any other context on your use cases that are impacted by this! Thank you!

[OliverCardoza]

Building protos which import the FHIR protos is pretty burdensome in cases where generating Go libraries is needed. IIUC the only way is to add hundreds of --go_opt and --go_grpc_opts args in the protoc call.

for resource in resources:
  go_opts+=("--go_opt=Mproto/google/fhir/proto/r4/core/resources/${resource}.proto=${GOOGLE_FHIR_PATH}/google/fhir/proto/r4/core/resources/${resource}_go_proto")
done

protoc "${go_opts[@]}" [...]

Other prominent Google protos declare go_package in the proto file:

• protocolbuffers/protobuf - any.proto: link
• googleapis/googleapis - fhir.proto: link

I recognize complexity to add this but fwiw I think all Go users will thank you 😄

[aivajoe]

Thanks for your excellent work on this proto representation of FHIR!

Are there any plans to add go_package to the .proto files? We're encountering this issue. It's not practical to have an "M" line for every single .proto file added to the compile step for us plebs who aren't using Bazel or similar tooling, so it would be a godsend for us. 😃

I can provide more context on our use case via email: joe@aivainc.com

[nikklassen]

I'm taking a look at this, regenerating all of the protos may be a bit tricky, does anyone need anything other than the R4 (and future) core resources/datatypes

[tmc]

I still have some code working over stu3 types (unfortunately).

[nikklassen]

Unfortunately some of the STU3 proto generation code was removed recently in c9ab5d2. This may come down to adding the annotations by hand to those protos. I only have experience on the Go code, I'm ramping up to how the proto generator works

[OliverCardoza]

does anyone need anything other than the R4 (and future) core resources/datatypes?

Nope :)

[aivajoe]

FWIW, protoc-gen-go itself emits warnings about this for all fhir protos, e.g.

2023/08/31 12:26:02 WARNING: Missing 'go_package' option in "proto/google/fhir/proto/r4/core/resources/structure_definition.proto",
please specify it with the full Go package path as a future release of protoc-gen-go will require this be specified.
See https://developers.google.com/protocol-buffers/docs/reference/go-generated#package for more information.

[nikklassen]

I submitted #243 for the R4 protos. If anyone has any issues please let me know

[tmc]

It would still be useful to get go_package options added to the rest of the proto tree.

So, a few asks/questions:

• Add go_package lines to the (I believe) hand authored protos in this directory: https://github.com/google/fhir/tree/master/proto/google/fhir/proto and this directory: https://github.com/google/fhir/tree/master/proto/google/fhir/proto/r4 files
• Add go_package to the r4/core/profiles subtree.

[nikklassen]

@tmc are you using r4/core/profiles or do you just want to use a glob that includes all these files? I tried to focus on all the files I expected people to need. If you need more we can reopen this, but I'll have to wait until I get time again to do that.

[aivajoe]

This ended up working out for my use case, I don't need to pass a huge number of "M" lines to protoc anymore. Thanks!