Vitess protocol buffer namespace conflict

[ryanpbrewster]

Overview of the Issue

Pulling in both this Vitess fork and the normal Vitess repo simultaneously will cause protocol buffer namespace conflicts. This results in a runtime panic that looks like

panic: proto: file "vttime.proto" has a name conflict over vttime.Time
See https://developers.google.com/protocol-buffers/docs/reference/go/faq#namespace-conflict


goroutine 1 [running]:
google.golang.org/protobuf/reflect/protoregistry.glob..func1({0x1792700, 0xc000451580}, {0x1792700, 0xc000451580})
        /Users/ryanpbrewster/go/pkg/mod/google.golang.org/protobuf@v1.27.1/reflect/protoregistry/registry.go:54 +0x1f4
google.golang.org/protobuf/reflect/protoregistry.(*Files).RegisterFile.func1({0x17b5d08, 0xc0003ad860})
        /Users/ryanpbrewster/go/pkg/mod/google.golang.org/protobuf@v1.27.1/reflect/protoregistry/registry.go:152 +0x28f
google.golang.org/protobuf/reflect/protoregistry.rangeTopLevelDescriptors({0x17bb488, 0xc000321180}, 0xc00039d600)
        /Users/ryanpbrewster/go/pkg/mod/google.golang.org/protobuf@v1.27.1/reflect/protoregistry/registry.go:415 +0x154
google.golang.org/protobuf/reflect/protoregistry.(*Files).RegisterFile(0xc0001b2018, {0x17bb488, 0xc000321180})
        /Users/ryanpbrewster/go/pkg/mod/google.golang.org/protobuf@v1.27.1/reflect/protoregistry/registry.go:147 +0x745
google.golang.org/protobuf/internal/filedesc.Builder.Build({{0x15add42, 0x23}, {0x1b99e80, 0xc5, 0xc5}, 0x0, 0x2, 0x0, 0x0, {0x179b010, ...}, ...})
        /Users/ryanpbrewster/go/pkg/mod/google.golang.org/protobuf@v1.27.1/internal/filedesc/build.go:113 +0x1d6
google.golang.org/protobuf/internal/filetype.Builder.Build({{{0x15add42, 0x23}, {0x1b99e80, 0xc5, 0xc5}, 0x0, 0x2, 0x0, 0x0, {0x0, ...}, ...}, ...})
        /Users/ryanpbrewster/go/pkg/mod/google.golang.org/protobuf@v1.27.1/internal/filetype/build.go:139 +0x198
vitess.io/vitess/go/vt/proto/vttime.file_vttime_proto_init()
        /Users/ryanpbrewster/go/pkg/mod/vitess.io/vitess@v0.13.1/go/vt/proto/vttime/vttime.pb.go:239 +0x198
vitess.io/vitess/go/vt/proto/vttime.init.0()
        /Users/ryanpbrewster/go/pkg/mod/vitess.io/vitess@v0.13.1/go/vt/proto/vttime/vttime.pb.go:195 +0x17
FAIL    command-line-arguments  0.240s
FAIL

Reproduction Steps

I have a single-source reproduction of this issue:

package my_test

import (
  "fmt"
  "testing"

  "github.com/dolthub/go-mysql-server/memory"
  topodatapb "vitess.io/vitess/go/vt/proto/topodata"
)

func Test_Collision(t *testing.T) {
  fmt.Println(memory.NewDatabase("mydb"))
  fmt.Println(topodatapb.CellInfo{})
}

panics with the error message posted at the top of this issue.

There are two possibilities I can see to fix this:

1. Refactor this repo to depend on the upstream Vitess protos instead of vendoring them. This a bit limiting.
2. "shade" these protos by giving them a different package name. Today these proto files look like

syntax = "proto3";
option go_package = "github.com/dolthub/vitess/go/vt/proto/vttime";

package vttime;

and I think if that were switched to package dolthub.vitess.vttime; instead, it would resolve the conflicts.

Operating system and Environment details

The associated go.mod file looks like

module rpb.dev/vitess-proto-sample

go 1.17

require (
  github.com/cespare/xxhash v1.1.0 // indirect
  github.com/davecgh/go-spew v1.1.1 // indirect
  github.com/dolthub/go-mysql-server v0.11.0 // indirect
  github.com/dolthub/vitess v0.0.0-20211013185428-a8845fb919c1 // indirect
  github.com/go-kit/kit v0.9.0 // indirect
  github.com/golang/glog v0.0.0-20160126235308-23def4e6c14b // indirect
  github.com/golang/protobuf v1.5.2 // indirect
  github.com/google/uuid v1.2.0 // indirect
  github.com/hashicorp/golang-lru v0.5.3 // indirect
  github.com/konsorten/go-windows-terminal-sequences v1.0.2 // indirect
  github.com/lestrrat-go/strftime v1.0.1 // indirect
  github.com/mitchellh/hashstructure v1.0.0 // indirect
  github.com/oliveagle/jsonpath v0.0.0-20180606110733-2e52cf6e6852 // indirect
  github.com/opentracing/opentracing-go v1.1.0 // indirect
  github.com/pkg/errors v0.9.1 // indirect
  github.com/pmezard/go-difflib v1.0.0 // indirect
  github.com/shopspring/decimal v0.0.0-20191130220710-360f2bc03045 // indirect
  github.com/sirupsen/logrus v1.4.2 // indirect
  github.com/src-d/go-oniguruma v1.1.0 // indirect
  github.com/stretchr/testify v1.7.0 // indirect
  golang.org/x/net v0.0.0-20210825183410-e898025ed96a // indirect
  golang.org/x/sync v0.0.0-20210220032951-036812b2e83c // indirect
  golang.org/x/sys v0.0.0-20210823070655-63515b42dcdf // indirect
  google.golang.org/genproto v0.0.0-20210701191553-46259e63a0a9 // indirect
  google.golang.org/grpc v1.39.0 // indirect
  google.golang.org/protobuf v1.27.1 // indirect
  gopkg.in/src-d/go-errors.v1 v1.0.0 // indirect
  gopkg.in/yaml.v3 v3.0.0-20210107192922-496545a6307b // indirect
  vitess.io/vitess v0.13.1 // indirect
)

[ryanpbrewster]

I managed to reproduce this from within this repo, and in the process discovered that my proposed fix is not quite sufficient. The protoregistry actually expects all .proto files to have globally unique files paths relative to their proto root. In this repo's case, the proto root is proto/, so proto/topodata.proto will conflict with any other topodata.proto that exists in any other dependency.

I think there is a very simple fix here: move all the proto files from proto/*.proto to proto/dolthub/vt/*.proto. That ensures that they will only conflict with other projects that also lay out their protos in proto/dolthub/vt/ directory structure.

[ryanpbrewster]

Fixed by #166

[ryanpbrewster]

Confirming that when I upgrade from

dolthub/go-mysql-server@0.11.0

to

dolthub/go-mysql-server@707f05909f9544d323f6223948ec01e686d18a37

(the commit from a few hours ago)
the sample test I provided above now passes

$ go test foo_test.go 
ok      command-line-arguments  0.005s