*: update to use full package name for `option go_package` in protos #97272

rickystewart · 2023-02-16T22:38:36Z

Everywhere in the entire tree we use option go_package statements like
the following:

option go_package = "build";

We do this instead of using the entire fully-qualified package name,
like:

option go_package = "github.com/cockroachdb/cockroach/pkg/build";

This is apparently just an error. All the documentation I have seen
(https://github.com/cockroachdb/gogoproto/blob/master/README) suggests
that the fully-qualified package name should be used here. Further, this
caused the make build to break after a refactor as the code generator
doesn't know how to import a package like "roachpb" (since it should
instead be importing "github.com/cockroachdb/cockroach/pkg/roachpb").

Correct this problem everywhere and update the Makefile to set
paths=source_relative to tell protoc that output files should be
placed next to input files.

Epic: none
Release note: None

cockroach-teamcity · 2023-02-16T22:38:46Z

This change is

RaduBerinde

Out of sheer paranoia, I would double-check that the generated files show no other differences (like they would if we changed the proto package names).

Reviewable status: complete! 1 of 0 LGTMs obtained (waiting on @healthy-pod)

rickystewart · 2023-02-21T17:49:19Z

Out of sheer paranoia, I would double-check that the generated files show no other differences (like they would if we changed the proto package names).

There are actually diffs in the FileDescriptorProto:

cockroach$ diff pkg/roachpb/internal.pb.go internal.pb.go 
207,235c207,233
< 	// 433 bytes of a gzipped FileDescriptorProto
< 	0x1f, 0x8b, 0x08, 0x00, 0x00, 0x00, 0x00, 0x00, .......

As far as I can tell this is because the go_package option is included in the actual FileDescriptorProto itself.

rickystewart · 2023-02-21T18:32:04Z

PR still has some issues with code generation, I'll take care of that.

Everywhere in the entire tree we use `option go_package` statements like the following: ``` option go_package = "build"; ``` We do this *instead of* using the entire fully-qualified package name, like: ``` option go_package = "github.com/cockroachdb/cockroach/pkg/build"; ``` This is apparently just an error. All the documentation I have seen (https://github.com/cockroachdb/gogoproto/blob/master/README) suggests that the fully-qualified package name should be used here. Further, this caused the `make` build to break after a refactor as the code generator doesn't know how to import a package like `"roachpb"` (since it should instead be importing `"github.com/cockroachdb/cockroach/pkg/roachpb"`). Correct this problem *everywhere* and update the `Makefile` to set `paths=source_relative` to tell `protoc` that output files should be placed next to input files. Epic: none Release note: None

rickystewart · 2023-02-22T16:07:25Z

bors r=rail,stevendanna,RaduBerinde

craig · 2023-02-22T16:26:46Z

This PR was included in a batch that was canceled, it will be automatically retried

craig · 2023-02-22T17:29:44Z

Build succeeded:

Bazel Essential CI (Cockroach)

rickystewart force-pushed the roachpbgopackage branch from d60ce6a to 885e5d8 Compare February 17, 2023 21:13

rickystewart changed the title ~~roachpb: update to use full package name for roachpb in go_package~~ *: update to use full package name for option go_package in protos Feb 17, 2023

rickystewart force-pushed the roachpbgopackage branch 3 times, most recently from 900091a to 922bc37 Compare February 17, 2023 21:50

rickystewart requested a review from healthy-pod February 17, 2023 21:59

RaduBerinde approved these changes Feb 17, 2023

View reviewed changes

rickystewart force-pushed the roachpbgopackage branch from 922bc37 to d8baef9 Compare February 21, 2023 17:40

rickystewart marked this pull request as ready for review February 21, 2023 17:49