refactor: Relax runtime checks in MergedRegistry (#43)

Co-authored-by: Aaron Craelius <aaron@regen.network>

CHANGELOG.md

@@ -4,6 +4,7 @@
 
 ### Improvements
 
+- [#43](https://github.com/cosmos/gogoproto/pull/43) Relax runtime linter checks introduced in #37: instead of throwing an error, simply log a warning to StdErr. Also provide a helper function `DebugFileDescriptorsMismatch` to debug these errors.
 - [#37](https://github.com/cosmos/gogoproto/pull/37) Add `MergedFileDescriptors` and `MergedRegistry` to retrieve a registry with merged file descriptors from both gogo and protoregistry.
 
 ### Bug Fixes

go.mod

@@ -3,16 +3,18 @@ module github.com/cosmos/gogoproto
 go 1.19
 
 require (
+	github.com/golang/protobuf v1.5.2
 	github.com/google/go-cmp v0.5.9
 	golang.org/x/exp v0.0.0-20230131160201-f062dba9d201
 	google.golang.org/grpc v1.53.0
 	google.golang.org/protobuf v1.28.1
 )
 
 require (
-	github.com/golang/protobuf v1.5.2 // indirect
-	golang.org/x/net v0.5.0 // indirect
-	golang.org/x/sys v0.4.0 // indirect
-	golang.org/x/text v0.6.0 // indirect
+	golang.org/x/mod v0.8.0 // indirect
+	golang.org/x/net v0.6.0 // indirect
+	golang.org/x/sys v0.5.0 // indirect
+	golang.org/x/text v0.7.0 // indirect
+	golang.org/x/tools v0.6.0 // indirect
 	google.golang.org/genproto v0.0.0-20230110181048-76db0878b65f // indirect
 )

go.sum

@@ -6,12 +6,16 @@ github.com/google/go-cmp v0.5.9 h1:O2Tfq5qg4qc4AmwVlvv0oLiVAGB7enBSJ2x2DqQFi38=
 github.com/google/go-cmp v0.5.9/go.mod h1:17dUlkBOakJ0+DkrSSNjCkIjxS6bF9zb3elmeNGIjoY=
 golang.org/x/exp v0.0.0-20230131160201-f062dba9d201 h1:BEABXpNXLEz0WxtA+6CQIz2xkg80e+1zrhWyMcq8VzE=
 golang.org/x/exp v0.0.0-20230131160201-f062dba9d201/go.mod h1:CxIveKay+FTh1D0yPZemJVgC/95VzuuOLq5Qi4xnoYc=
-golang.org/x/net v0.5.0 h1:GyT4nK/YDHSqa1c4753ouYCDajOYKTja9Xb/OHtgvSw=
-golang.org/x/net v0.5.0/go.mod h1:DivGGAXEgPSlEBzxGzZI+ZLohi+xUj054jfeKui00ws=
-golang.org/x/sys v0.4.0 h1:Zr2JFtRQNX3BCZ8YtxRE9hNJYC8J6I1MVbMg6owUp18=
-golang.org/x/sys v0.4.0/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg=
-golang.org/x/text v0.6.0 h1:3XmdazWV+ubf7QgHSTWeykHOci5oeekaGJBLkrkaw4k=
-golang.org/x/text v0.6.0/go.mod h1:mrYo+phRRbMaCq/xk9113O4dZlRixOauAjOtrjsXDZ8=
+golang.org/x/mod v0.8.0 h1:LUYupSeNrTNCGzR/hVBk2NHZO4hXcVaW1k4Qx7rjPx8=
+golang.org/x/mod v0.8.0/go.mod h1:iBbtSCu2XBx23ZKBPSOrRkjjQPZFPuis4dIYUhu/chs=
+golang.org/x/net v0.6.0 h1:L4ZwwTvKW9gr0ZMS1yrHD9GZhIuVjOBBnaKH+SPQK0Q=
+golang.org/x/net v0.6.0/go.mod h1:2Tu9+aMcznHK/AK1HMvgo6xiTLG5rD5rZLDS+rp2Bjs=
+golang.org/x/sys v0.5.0 h1:MUK/U/4lj1t1oPg0HfuXDN/Z1wv31ZJ/YcPiGccS4DU=
+golang.org/x/sys v0.5.0/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg=
+golang.org/x/text v0.7.0 h1:4BRB4x83lYWy72KwLD/qYDuTu7q9PjSagHvijDw7cLo=
+golang.org/x/text v0.7.0/go.mod h1:mrYo+phRRbMaCq/xk9113O4dZlRixOauAjOtrjsXDZ8=
+golang.org/x/tools v0.6.0 h1:BOw41kyTf3PuCW1pVQf8+Cyg8pMlkYB1oo9iJ6D/lKM=
+golang.org/x/tools v0.6.0/go.mod h1:Xwgl3UAJ/d3gWutnCtw505GrjyAbvKui8lOU390QaIU=
 golang.org/x/xerrors v0.0.0-20191204190536-9bdfabe68543/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0=
 google.golang.org/genproto v0.0.0-20230110181048-76db0878b65f h1:BWUVssLB0HVOSY78gIdvk1dTVYtT1y8SBWtPYuTJ/6w=
 google.golang.org/genproto v0.0.0-20230110181048-76db0878b65f/go.mod h1:RGgjbofJ8xD9Sq1VVhDM1Vok1vRONV+rg+CjzG4SZKM=

proto/merge.go

@@ -3,8 +3,10 @@ package proto
 import (
 	"bytes"
 	"compress/gzip"
+	"errors"
 	"fmt"
 	"io"
+	"os"
 	"strings"
 
 	"github.com/google/go-cmp/cmp"
@@ -21,15 +23,48 @@ import (
 // file descriptors registered with both gogoproto and protoregistry. When
 // merging it also performs the following checks:
 // - check that all file descriptors' import paths are correct (i.e. match
-// their fully-qualified package name).
+// their fully-qualified package name). A warning is logged if this check fails.
 // - check that if both gogo and protoregistry import the same file descriptor,
-// that these are identical under proto.Equal.
+// that these are identical under proto.Equal. A warning is logged if this
+// check fails. If there is a mismatch, the final merged file descriptor set will contain the
+// protoregistry file descriptor, and discard the gogo one.
 func MergedFileDescriptors() (*descriptorpb.FileDescriptorSet, error) {
+	return mergedFileDescriptors(false)
+}
+
+// DebugFileDescriptorsMismatch is a helper function to debug file descriptor
+// mismatches. It returns an error if there are any mismatches.
+func DebugFileDescriptorsMismatch() error {
+	_, err := mergedFileDescriptors(true)
+	return err
+}
+
+func mergedFileDescriptors(debug bool) (*descriptorpb.FileDescriptorSet, error) {
 	fds := &descriptorpb.FileDescriptorSet{}
 
+	// While combing through the file descriptors, we'll also log any errors
+	// we encounter.
+	var (
+		checkImportErr []string
+		diffErr        []string
+	)
+
+	// Add protoregistry file descriptors to our final file descriptor set.
+	protoregistry.GlobalFiles.RangeFiles(func(fileDescriptor protoreflect.FileDescriptor) bool {
+		fd := protodesc.ToFileDescriptorProto(fileDescriptor)
+		if fd.Name != nil && fd.Package != nil {
+			if err := CheckImportPath(*fd.Name, *fd.Package); err != nil {
+				checkImportErr = append(checkImportErr, err.Error())
+			}
+		}
+
+		fds.File = append(fds.File, protodesc.ToFileDescriptorProto(fileDescriptor))
+
+		return true
+	})
+
 	// load gogo proto file descriptors
 	gogoFds := AllFileDescriptors()
-	gogoFdsMap := map[string]*descriptorpb.FileDescriptorProto{}
 	for _, compressedBz := range gogoFds {
 		rdr, err := gzip.NewReader(bytes.NewReader(compressedBz))
 		if err != nil {
@@ -49,46 +84,49 @@ func MergedFileDescriptors() (*descriptorpb.FileDescriptorSet, error) {
 
 		err = CheckImportPath(*fd.Name, *fd.Package)
 		if err != nil {
-			return nil, err
-		}
-
-		fds.File = append(fds.File, fd)
-		gogoFdsMap[*fd.Name] = fd
-	}
-
-	// load any protoregistry file descriptors not in gogo
-	var (
-		checkImportErr []string
-		diffErr        error
-	)
-	protoregistry.GlobalFiles.RangeFiles(func(fileDescriptor protoreflect.FileDescriptor) bool {
-		fd := protodesc.ToFileDescriptorProto(fileDescriptor)
-		if fd.Name != nil && fd.Package != nil {
-			if err := CheckImportPath(*fd.Name, *fd.Package); err != nil {
-				checkImportErr = append(checkImportErr, err.Error())
-			}
+			checkImportErr = append(checkImportErr, err.Error())
 		}
 
-		gogoFd, found := gogoFdsMap[fileDescriptor.Path()]
+		// If it's not in the protoregistry file descriptors, add it.
+		protoregFd, err := protoregistry.GlobalFiles.FindFileByPath(*fd.Name)
 		// If we already loaded gogo's file descriptor, compare that the 2
-		// are strictly equal.
-		if found {
-			if !protov2.Equal(gogoFd, fd) {
-				diff := cmp.Diff(fd, gogoFd, protocmp.Transform())
-				diffErr = fmt.Errorf("got different file descriptors for %s; %s", *fd.Name, diff)
-				return false
+		// are strictly equal, or else log a warning.
+		if err != nil {
+			// If we have a NotFound error, we add this file descriptor to the
+			// final file descriptor set.
+			if errors.Is(err, protoregistry.NotFound) {
+				fds.File = append(fds.File, fd)
+			} else {
+				return nil, err
 			}
 		} else {
-			fds.File = append(fds.File, protodesc.ToFileDescriptorProto(fileDescriptor))
+			// If there's a mismatch, we log a warning. If there was no
+			// mismatch, then we do nothing, and take the protoregistry file
+			// descriptor as the correct one.
+			if !protov2.Equal(protodesc.ToFileDescriptorProto(protoregFd), fd) {
+				diff := cmp.Diff(protodesc.ToFileDescriptorProto(protoregFd), fd, protocmp.Transform())
+				diffErr = append(diffErr, fmt.Sprintf("Mismatch in %s:\n%s", *fd.Name, diff))
+			}
 		}
-
-		return true
-	})
-	if len(checkImportErr) > 0 {
-		return nil, fmt.Errorf("got %d file descriptor import path errors:\n%s", len(checkImportErr), strings.Join(checkImportErr, "\n"))
 	}
-	if diffErr != nil {
-		return nil, diffErr
+
+	if debug {
+		errStr := new(bytes.Buffer)
+		if len(checkImportErr) > 0 {
+			fmt.Fprintf(errStr, "Got %d file descriptor import path errors:\n\t%s\n", len(checkImportErr), strings.Join(checkImportErr, "\n\t"))
+		}
+		if len(diffErr) > 0 {
+			fmt.Fprintf(errStr, "Got %d file descriptor mismatches. Make sure gogoproto and protoregistry use the same .proto files. '-' lines are from protoregistry, '+' lines from gogo's registry.\n\n\t%s\n", len(diffErr), strings.Join(diffErr, "\n\t"))
+		}
+		if errStr.Len() > 0 {
+			return nil, fmt.Errorf(errStr.String())
+		}
+	} else {
+		// In production, we just log a warning to StdErr with the number of
+		// linter errors.
+		if len(checkImportErr) > 0 || len(diffErr) > 0 {
+			fmt.Fprintf(os.Stderr, "Got %d file descriptor import path errors and %d file descriptor mismatches. Run `proto.DebugFileDescriptorsMismatch` to debug them.\n", len(checkImportErr), len(diffErr))
+		}
 	}
 
 	slices.SortFunc(fds.File, func(x, y *descriptorpb.FileDescriptorProto) bool {
@@ -100,7 +138,7 @@ func MergedFileDescriptors() (*descriptorpb.FileDescriptorSet, error) {
 
 // MergedRegistry returns a *protoregistry.Files that acts as a single registry
 // which contains all the file descriptors registered with both gogoproto and
-// protoregistry.
+// protoregistry (the latter taking precendence if there's a mismatch).
 func MergedRegistry() (*protoregistry.Files, error) {
 	fds, err := MergedFileDescriptors()
 	if err != nil {

protobuf/Makefile

@@ -6,7 +6,13 @@ regenerate:
 
 	protoc-min-version \
 	--version="3.0.0" \
-	--gogotypes_out=../types/ \
+	--gogotypes_out=\
+	Mgoogle/protobuf/api.proto=github.com/cosmos/gogoproto/types,\
+	Mgoogle/protobuf/any.proto=github.com/cosmos/gogoproto/types,\
+	Mgoogle/protobuf/source_context.proto=github.com/cosmos/gogoproto/types,\
+	Mgoogle/protobuf/struct.proto=github.com/cosmos/gogoproto/types,\
+	Mgoogle/protobuf/type.proto=github.com/cosmos/gogoproto/types\
+	:../types/ \
 	-I=. \
 	google/protobuf/any.proto \
 	google/protobuf/type.proto \
@@ -19,8 +25,8 @@ regenerate:
 	google/protobuf/field_mask.proto \
 	google/protobuf/source_context.proto
 
-	mv google.golang.org/protobuf/types/known/*/*.pb.go ../types/ || true
-	rm -rf ../types/google.golang.org || true
+	mv ../types/google.golang.org/protobuf/types/known/*/*.pb.go ../types/
+	rm -rf ../types/google.golang.org
 
 	# Fix package names, because in gogo they are all in `types`.
 	cd ../types && sed -i.bak 's/package anypb/package types/' any.pb.go && rm any.pb.go.bak